Skip to content
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

Upgrade go-getter #5445

Closed
prologic opened this issue Mar 20, 2019 · 10 comments
Closed

Upgrade go-getter #5445

prologic opened this issue Mar 20, 2019 · 10 comments
Assignees
Milestone

Comments

@prologic
Copy link

Need to upgrade the version of go-getter used to include the fix in hashicorp/go-getter#171

Also somewhat related (because of dependency management) #5444

@ExalDraen
Copy link

ExalDraen commented May 21, 2019

The new version of go-getter now also supports GCS as a storage backend (see hashicorp/go-getter#170) and contains a number of fixes, e.g. for SCP style URLs when using the git provider (see hashicorp/go-getter#180).

I'd be happy to help here if needed - what's needed to help push this over the line?
Am I right that besides bumping the version in vendor.json the main effort here is testing?

@cgbaker
Copy link
Contributor

cgbaker commented May 21, 2019

There's an outstanding PR for updating go-getter: #5446
I'll talk to the team about getting this in.

@prologic
Copy link
Author

The status on this is "on hold". I haven't had the bandwidth to do the work stated in the comments. Summary of what's needed as follows:

  • Rewrite the go-getter patch using securejoin
  • Add tests that demonstrate it is impossible to escape the chroot'd path

@ExalDraen
Copy link

Ah, that does seem like a bit more effort than I thought! I'd like to help where I can but it will take some learning to familiarize myself with the Nomad build and tests. It will be easier for me to help change go-getter which #5446 suggests is required.

@prologic
When you mention securejoin do you mean https://github.com/cyphar/filepath-securejoin? Are you intending to update go-getter to use securejoin instead of filepath.join?

@prologic
Copy link
Author

Yes! See this comment and this one

@aclowkey
Copy link

Any progress with this?
This is a major blocker for migrating to Nomad, if artifacts are stored in GCS.

@preetapan preetapan added this to the 0.10.0 milestone Aug 13, 2019
@langmartin
Copy link
Contributor

@aclowkey #6215 enables GCS support by updating to go-getter master(ish). @prologic the master of go-getter does not yet include your reworked securejoin symlink support. I'll see if we can get that support merged in go-getter and then I can redo the update. Cheers

@langmartin langmartin self-assigned this Aug 28, 2019
@nickethier nickethier modified the milestones: 0.10.0, 0.10.1 Sep 3, 2019
@DrorBuhnik
Copy link

It seems that just updating go-getter isn't enough

supported = []string{"http", "https", "s3", "hg", "git"}

should have 'gcs' in the supported list

-       supported = []string{"http", "https", "s3", "hg", "git"}
+       supported = []string{"http", "https", "s3", "hg", "git", "gcs"}

@langmartin
Copy link
Contributor

go-getter has been upgraded to support GCS as an artifact source, but still does not support symlinks in tarballs. I'm closing the rest of this issue as duplicate of #2292

@github-actions
Copy link

I'm going to lock this issue because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

10 participants