search

carvel-dev / vendir

Easy way to vendor portions of git repos, github releases, helm charts, docker image contents, etc. declaratively
https://carvel.dev/vendir
Apache License 2.0
269 stars 47 forks source link

Fix tar.gz support #280

Closed reegnz closed 9 months ago

reegnz commented 11 months ago

Fixes #285

reegnz commented 11 months ago

I haven't tried with other tar.gz files, but it definitely fixed the issue for this particular one of downloading a tar.gz of a github repo. Also the fix was inspired by this code in the go codebase: https://cs.opensource.google/go/x/build/+/2c66d68b:internal/untar/untar.go;l=34-139, particularly line 132 and 133.

neil-hickey commented 11 months ago

hey @reegnz , so this does appear to work on the surface. I think to really validate we are going to want some more e2e tests that cover each use-case. Is this something you would be able to contribute?

reegnz commented 11 months ago

@neil-hickey If you can point me to what exactly you'd like to see I can give it a go.

joaopapereira commented 10 months ago

I think we need something similar to https://github.com/carvel-dev/vendir/blob/develop/test/e2e/github_release_test.go but for HTTP workflow. We might need to standup a simple HTTP server and serve a tar.gz file from it to test it out.

reegnz commented 10 months ago

I think I'll just fetch a tar.gz from github (from this repo), that's one of those tar.gz-s what's causing the issue anyway, and that's the most life-like test.

reegnz commented 10 months ago

Pushed the e2e test (already had this in the making today, before the local http server idea).

reegnz commented 10 months ago

@joaopapereira fixed.

joaopapereira commented 10 months ago

@reegnz you need to sign your commits 😄

reegnz commented 9 months ago

@joaopapereira rebased, added signoff to commits.

kumaritanushree commented 9 months ago

LGTM