diff --git a/README.md b/README.md index 85884fad4..ba4df6f4b 100644 --- a/README.md +++ b/README.md @@ -280,6 +280,16 @@ None * `depth` - The Git clone depth. The provided number specifies the last `n` revisions to clone from the repository. + +The `git` getter accepts both URL-style SSH addresses like +`git::ssh://git@example.com/foo/bar`, and "scp-style" addresses like +`git::git@example.com/foo/bar`. In the latter case, omitting the `git::` +force prefix is allowed if the username prefix is exactly `git@`. + +The "scp-style" addresses _cannot_ be used in conjunction with the `ssh://` +scheme prefix, because in that case the colon is used to mark an optional +port number to connect on, rather than to delimit the path from the host. + ### Mercurial (`hg`) * `rev` - The Mercurial revision to checkout. diff --git a/detect_git_test.go b/detect_git_test.go index 0d1b9540c..ad6331160 100644 --- a/detect_git_test.go +++ b/detect_git_test.go @@ -9,7 +9,10 @@ func TestGitDetector(t *testing.T) { Input string Output string }{ - {"git@github.com:hashicorp/foo.git", "git::ssh://git@github.com/hashicorp/foo.git"}, + { + "git@github.com:hashicorp/foo.git", + "git::ssh://git@github.com/hashicorp/foo.git", + }, { "git@github.com:org/project.git?ref=test-branch", "git::ssh://git@github.com/org/project.git?ref=test-branch", @@ -38,21 +41,35 @@ func TestGitDetector(t *testing.T) { "git@github.xyz.com:org/project.git//module/a?ref=test-branch", "git::ssh://git@github.xyz.com/org/project.git//module/a?ref=test-branch", }, + { + // Using the scp-like form with the ssh:// prefix is invalid, so + // it passes through as-is. + "git::ssh://git@github.xyz.com:org/project.git", + "git::ssh://git@github.xyz.com:org/project.git", + }, + { + // Already in the canonical form, so no rewriting required + // When the ssh: protocol is used explicitly, we recognize it as + // URL form rather than SCP-like form, so the part after the colon + // is a port number, not part of the path. + "git::ssh://git@git.example.com:2222/hashicorp/foo.git", + "git::ssh://git@git.example.com:2222/hashicorp/foo.git", + }, } pwd := "/pwd" f := new(GitDetector) - for i, tc := range cases { - output, ok, err := f.Detect(tc.Input, pwd) - if err != nil { - t.Fatalf("err: %s", err) - } - if !ok { - t.Fatal("not ok") - } + ds := []Detector{f} + for _, tc := range cases { + t.Run(tc.Input, func (t *testing.T) { + output, err := Detect(tc.Input, pwd, ds) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } - if output != tc.Output { - t.Fatalf("%d: bad: %#v", i, output) - } + if output != tc.Output { + t.Errorf("wrong result\ninput: %s\ngot: %s\nwant: %s", tc.Input, output, tc.Output) + } + }) } } diff --git a/get_git.go b/get_git.go index 2ff00d20f..67e8b2f49 100644 --- a/get_git.go +++ b/get_git.go @@ -34,6 +34,15 @@ func (g *GitGetter) Get(dst string, u *url.URL) error { return fmt.Errorf("git must be available and on the PATH") } + // The port number must be parseable as an integer. If not, the user + // was probably trying to use a scp-style address, in which case the + // ssh:// prefix must be removed to indicate that. + if portStr := u.Port(); portStr != "" { + if _, err := strconv.ParseUint(portStr, 10, 16); err != nil { + return fmt.Errorf("invalid port number %q; if using the \"scp-like\" git address scheme where a colon introduces the path instead, remove the ssh:// portion and use just the git:: prefix", portStr) + } + } + // Extract some query parameters we use var ref, sshKey string var depth int @@ -90,26 +99,6 @@ func (g *GitGetter) Get(dst string, u *url.URL) error { } } - // For SSH-style URLs, if they use the SCP syntax of host:path, then - // the URL will be mangled. We detect that here and correct the path. - // Example: host:path/bar will turn into host/path/bar - if u.Scheme == "ssh" { - if idx := strings.Index(u.Host, ":"); idx > -1 { - // Copy the URL so we don't modify the input - var newU url.URL = *u - u = &newU - - // Path includes the part after the ':'. - u.Path = u.Host[idx+1:] + u.Path - if u.Path[0] != '/' { - u.Path = "/" + u.Path - } - - // Host trims up to the : - u.Host = u.Host[:idx] - } - } - // Clone or update the repository _, err := os.Stat(dst) if err != nil && !os.IsNotExist(err) { diff --git a/get_git_test.go b/get_git_test.go index 25d9570a6..413b689e6 100644 --- a/get_git_test.go +++ b/get_git_test.go @@ -298,6 +298,130 @@ func TestGitGetter_sshKey(t *testing.T) { } } +func TestGitGetter_sshSCPStyle(t *testing.T) { + if !testHasGit { + t.Skip("git not found, skipping") + } + + g := new(GitGetter) + dst := tempDir(t) + + encodedKey := base64.StdEncoding.EncodeToString([]byte(testGitToken)) + + // avoid getting locked by a github authenticity validation prompt + os.Setenv("GIT_SSH_COMMAND", "ssh -o StrictHostKeyChecking=no") + defer os.Setenv("GIT_SSH_COMMAND", "") + + // This test exercises the combination of the git detector and the + // git getter, to make sure that together they make scp-style URLs work. + client := &Client{ + Src: "git@github.com:hashicorp/test-private-repo?sshkey=" + encodedKey, + Dst: dst, + Pwd: ".", + + Mode: ClientModeDir, + + Detectors: []Detector{ + new(GitDetector), + }, + Getters: map[string]Getter{ + "git": g, + }, + } + + if err := client.Get(); err != nil { + t.Fatalf("client.Get failed: %s", err) + } + + readmePath := filepath.Join(dst, "README.md") + if _, err := os.Stat(readmePath); err != nil { + t.Fatalf("err: %s", err) + } +} + +func TestGitGetter_sshExplicitPort(t *testing.T) { + if !testHasGit { + t.Skip("git not found, skipping") + } + + g := new(GitGetter) + dst := tempDir(t) + + encodedKey := base64.StdEncoding.EncodeToString([]byte(testGitToken)) + + // avoid getting locked by a github authenticity validation prompt + os.Setenv("GIT_SSH_COMMAND", "ssh -o StrictHostKeyChecking=no") + defer os.Setenv("GIT_SSH_COMMAND", "") + + // This test exercises the combination of the git detector and the + // git getter, to make sure that together they make scp-style URLs work. + client := &Client{ + Src: "git::ssh://git@github.com:22/hashicorp/test-private-repo?sshkey=" + encodedKey, + Dst: dst, + Pwd: ".", + + Mode: ClientModeDir, + + Detectors: []Detector{ + new(GitDetector), + }, + Getters: map[string]Getter{ + "git": g, + }, + } + + if err := client.Get(); err != nil { + t.Fatalf("client.Get failed: %s", err) + } + + readmePath := filepath.Join(dst, "README.md") + if _, err := os.Stat(readmePath); err != nil { + t.Fatalf("err: %s", err) + } +} + + +func TestGitGetter_sshSCPStyleInvalidScheme(t *testing.T) { + if !testHasGit { + t.Skip("git not found, skipping") + } + + g := new(GitGetter) + dst := tempDir(t) + + encodedKey := base64.StdEncoding.EncodeToString([]byte(testGitToken)) + + // avoid getting locked by a github authenticity validation prompt + os.Setenv("GIT_SSH_COMMAND", "ssh -o StrictHostKeyChecking=no") + defer os.Setenv("GIT_SSH_COMMAND", "") + + // This test exercises the combination of the git detector and the + // git getter, to make sure that together they make scp-style URLs work. + client := &Client{ + Src: "git::ssh://git@github.com:hashicorp/test-private-repo?sshkey=" + encodedKey, + Dst: dst, + Pwd: ".", + + Mode: ClientModeDir, + + Detectors: []Detector{ + new(GitDetector), + }, + Getters: map[string]Getter{ + "git": g, + }, + } + + err := client.Get() + if err == nil { + t.Fatalf("get succeeded; want error") + } + + if got, want := err.Error(), `invalid port number "hashicorp"`; !strings.Contains(got, want) { + t.Fatalf("wrong error\ngot: %s\nwant: %s", got, want) + } +} + func TestGitGetter_submodule(t *testing.T) { if !testHasGit { t.Skip("git not found, skipping")