Allow SCP-style URLs for Git #129
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This detects URLs in the format of git@my.custom.git:dir1/dir2 and convert them to git::ssh://git@my.custom.git/dir1/dir2.
If a fully qualified URL like ssh://user@host:path is given, then the detectors don't run (since the URL is valid). Therefore we must also correct this directly in the getter.
This was referenced Nov 17, 2018
mitchellh
commented
Nov 17, 2018
| { | ||
| "git@github.com:hashicorp/foo.git?foo=bar", | ||
| "git::ssh://git@github.com/hashicorp/foo.git?foo=bar", | ||
| }, |
There was a problem hiding this comment.
These tests were moved to detect_git_test.go, not removed.
paultyng
approved these changes
Nov 17, 2018
apparentlymart
approved these changes
Nov 19, 2018
apparentlymart
added a commit
that referenced
this pull request
May 10, 2019
The "scp-style" remote URL scheme in git is ambiguous with the standard URL syntax in our previous design where we'd add/accept the ssh:// prefix in both cases; ssh://git@example.com:222/foo could be understood as either: - connect to example.com on port 222 and clone from path "foo" - connect to example.com on port 22 and clone from path "222/foo" Originally we used the first interpretation. but in #129 we switched to the second in order to support a strange hybrid form where no port number was specified, but in the process we broke handling of the standard URL form when a non-standard port is required, as is sometimes the case for internal git servers running on machines where port 22 is already used for administrative access. git itself resolves this ambiguity by not permitting the ssh:// scheme to be used in conjunction with the scp-like form. Here we mimic that by supporting the scp-like form _only_ in the detector, and only allowing it when the ssh:// prefix isn't present. That allows for the following two scp-like forms to be accepted: - git@example.com:foo/bar when the username is "git" - git::anything@example.com:foo/bar for any other username We no longer support using the scp-like form with the ssh:// prefix, so git::ssh://anything.example.com:foo/bar is invalid. To use that form, the ssh:// part must be omitted, which is consistent with git's own behavior. The detector already rewrites the scp-like form into the standard URL form, when no ssh: scheme is given so the getter does not need to do anything special to deal with it. In the git getter, we now also check to see if the port number seems to be non-decimal and return a more actionable error if so, since that is likely to be caused by someone incorrectly using the ssh:// scheme with a scp-style address.
apparentlymart
added a commit
that referenced
this pull request
May 12, 2019
The "scp-style" remote URL scheme in git is ambiguous with the standard URL syntax in our previous design where we'd add/accept the ssh:// prefix in both cases; ssh://git@example.com:222/foo could be understood as either: - connect to example.com on port 222 and clone from path "foo" - connect to example.com on port 22 and clone from path "222/foo" Originally we used the first interpretation. but in #129 we switched to the second in order to support a strange hybrid form where no port number was specified, but in the process we broke handling of the standard URL form when a non-standard port is required, as is sometimes the case for internal git servers running on machines where port 22 is already used for administrative access. git itself resolves this ambiguity by not permitting the ssh:// scheme to be used in conjunction with the scp-like form. Here we mimic that by supporting the scp-like form _only_ in the detector, and only allowing it when the ssh:// prefix isn't present. That allows for the following two scp-like forms to be accepted: - git@example.com:foo/bar when the username is "git" - git::anything@example.com:foo/bar for any other username We no longer support using the scp-like form with the ssh:// prefix, so git::ssh://anything.example.com:foo/bar is invalid. To use that form, the ssh:// part must be omitted, which is consistent with git's own behavior. The detector already rewrites the scp-like form into the standard URL form, when no ssh: scheme is given so the getter does not need to do anything special to deal with it. In the git getter, we now also check to see if the port number seems to be non-decimal and return a more actionable error if so, since that is likely to be caused by someone incorrectly using the ssh:// scheme with a scp-style address.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes both #38 and #124, and should also address the 4 TF issues referenced there as well. First, thanks @paultyng for the failing cases as well as a stab at a solution. You can see I took from that! I also took the failing cases from #38 (now passing).
I dug into the linked Terraform issues and there were actually two issues at play, both fixed in this PR:
1.) Detector for SCP-like Git URLs:
The heuristic we use here is the
git@username. We can extend this to support more usernames in the future (the code is ready for it) but all the issues seem to use this as the user and this limits the blast radius of this change significantly.This detects things in the form of
git@foo.com:pathand converts it togit::ssh://git@foo.com/pathand Git will clone this properly.Note that if a user other than "git" is required, the go-getter user can still use full forced URLs like
git::ssh://admin@host.com/path.2.) Fixing up the SCP-like URLs directly in the Git getter:
If a full, valid URL is given to go-getter, then the detectors are not run. So if a user gave the URL
git::ssh://git@foo.com:path(note the colon), then the detector won't run to fix that up at all. Therefore, I also added a little fixup directly in the Git getter that looks for 1.) an SSH URL and 2.) with ":" in the host (how Go'snet/urlparses it) and fixes it up.This should fix the issues where the direct URL is used.
Note to @apparentlymart: Re: your comment, I verified that forced protocols will always force. This is done here:
go-getter/detect.go
Line 93 in 9fd5bfc
Backwards Compatibility
This should not break any backwards compatibility.
These SCP-like URLs were being detected as "file" URLs below and its highly unlikely that was ever the intended behavior. Users who may have intended this can force it with
file::, though I strongly believe the number of users impacted by that will be zero.Future Stuff
The groundwork is now laid (
detect_ssh.go) to support an SCP-getter directly as well. So we can consider doing that one day by expanding this detector directly into SSH URLs that use an SCP lib or something.