New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support xz-encoded images #3160
Support xz-encoded images #3160
Conversation
how is octect stream request to xz?? afaik vSphere ova is also octect stream and that is not xz compressed. I think we need to probably use the correct application type? Also did this not have a closer call? |
I think there was a catch wrt the octet-stream format, deferring to @vrutkovs |
This should be fixed by FCOS server really, it serves all files as |
@vrutkovs do you know where that code lives offhand? Also, do we need the closer call Abhinav mentioned? |
The xz.Reader does not have a Close method. |
http.DetectContentType only knows about MIME types in this spec: https://mimesniff.spec.whatwg.org/ That list is very much browser focused and a type only gets added there with proper browser support and testing, which I don't expect to happen for As any type not included in their list will default to I'm thinking we should look at the file extension if we get the |
Updated to use |
i'm fine with the new vendor dep, but currently the commits are in mix state.. can you make sure you squash all non-vendor in one or make sure switch and add happen in separate ones ? |
d4018de
to
dfe0b41
Compare
updated :) |
/retest |
looks like a CI hickup. |
/retest |
1 similar comment
/retest |
/refresh |
/retest |
@abhinavdahiya I put the go.mod update in the first commit now. Is this what you meant or would you rather have it alongside the |
/test e2e-openstack transient network issue? |
dfe0b41
to
944703f
Compare
nope, the newly added lib casts the MIME type as |
944703f
to
ec9bdf3
Compare
I've updated the commit msg to include a note about the gzip vs x-gzip issue |
pkg/tfvars/internal/cache/cache.go
Outdated
|
||
"golang.org/x/sys/unix" | ||
) | ||
|
||
const ( | ||
applicationName = "openshift-installer" | ||
imageDataType = "image" | ||
gzipFileType = "application/x-gzip" | ||
gzipFileType = "application/gzip" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it possible to use the consts like https://github.com/h2non/filetype/blob/8241aa2c448465326c0002ccc6df7b910658960d/matchers/archive.go#L8 ?
Switches to "github.com/h2non/filetype/matchers" for MIME type matching and uses "github.com/ulikunitz/xz" for xz decompression.
ec9bdf3
to
2832acb
Compare
Updated according to @pierreprinetti's proposal |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abhinavdahiya The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
we're experiencing infra issues on openstack's CI /test e2e-openstack |
/retest |
/lgtm |
/retest Please review the full test history for this PR and help us cut down flakes. |
@LorbusChris: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Uses
github.com/h2non/filetype
for MIME type checkingand
github.com/ulikunitz/xz
to uncompress artifacts,identified by
application/x-xz