Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

git: Handle scp-style git addresses in detector, not getter #180

Merged
merged 1 commit into from
May 12, 2019

Conversation

apparentlymart
Copy link
Contributor

@apparentlymart apparentlymart commented 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 of the following:

  • 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 the ssh:// prefix could introduce scp-like form, 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'll now 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", as seems common in various git server solutions
  • git::anything@example.com:foo/bar for any 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. This is a breaking change to go-getter for scp-like addresses with the scheme and no port number, though.

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. Explicitly prohibiting that ambiguous form seems better than breaking the ability to use non-standard port numbers.


This is for hashicorp/terraform#21257. As we vendor this fix into Terraform we'll also need to update its module sources documentation, which is largely a description of go-getter's behavior. Any other application that supports installing from git using go-getter will probably need to make similar documentation changes, as well as recording this as a breaking change.

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.
Copy link
Contributor

@mitchellh mitchellh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm good with this. I think the alignment with git makes sense.

@apparentlymart apparentlymart merged commit f9ec369 into master May 12, 2019
@apparentlymart
Copy link
Contributor Author

I've landed this in master. Will chat more next week to figure out what makes sense for releasing this... technically it's a breaking change so a major version bump would be warranted, but it's API-compatible and we've made behavior changes within the existing API before without a major version bump, so will need to think about the tradeoffs for how we represent this change for callers.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants