From d9e97943fe4d0e93e80a77b2b3625006a9badb9b Mon Sep 17 00:00:00 2001 From: Matthias Diester Date: Tue, 30 Nov 2021 14:36:50 +0100 Subject: [PATCH 1/3] Avoid using `git config` Using `git config` has the disadvantage that it also changes the Git config in the development system when running the test cases. Rename `addtlCredArgs` to `addtlGitArgs` to be more generic. Use `-c` Git flag to supply specific configuration instead of using Git config. --- cmd/git/main.go | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/cmd/git/main.go b/cmd/git/main.go index e0c35c3e96..4a44128fa5 100644 --- a/cmd/git/main.go +++ b/cmd/git/main.go @@ -244,7 +244,7 @@ func clone(ctx context.Context) error { } } - var addtlCredArgs []string + var addtlGitArgs []string if flagValues.secretPath != "" { credType, err := checkCredentials() if err != nil { @@ -291,7 +291,7 @@ func clone(ctx context.Context) error { ) } - addtlCredArgs = append(addtlCredArgs, + addtlGitArgs = append(addtlGitArgs, "-c", fmt.Sprintf(`core.sshCommand=%s`, strings.Join(sshCmd, " ")), ) @@ -325,9 +325,10 @@ func clone(ctx context.Context) error { return err } - if _, err := git(ctx, "config", "--global", "credential.helper", fmt.Sprintf("store --file %s", credHelperFile.Name())); err != nil { - return err - } + addtlGitArgs = append(addtlGitArgs, + "-c", + fmt.Sprintf("credential.helper=%s", fmt.Sprintf("store --file %s", credHelperFile.Name())), + ) } } @@ -351,19 +352,14 @@ func clone(ctx context.Context) error { } if hostname != "" { - _, err := git(ctx, - "config", - "--global", - fmt.Sprintf("url.ssh://git@%s/.insteadOf", hostname), - fmt.Sprintf("https://%s/", hostname)) - - if err != nil { - return err - } + addtlGitArgs = append(addtlGitArgs, + "-c", + fmt.Sprintf("url.ssh://git@%s/.insteadOf=https://%s/", hostname, hostname), + ) } } - cloneArgs = append(cloneArgs, addtlCredArgs...) + cloneArgs = append(cloneArgs, addtlGitArgs...) cloneArgs = append(cloneArgs, "--", flagValues.url, flagValues.target) if _, err := git(ctx, cloneArgs...); err != nil { return err @@ -376,7 +372,7 @@ func clone(ctx context.Context) error { } submoduleArgs := []string{"-C", flagValues.target} - submoduleArgs = append(submoduleArgs, addtlCredArgs...) + submoduleArgs = append(submoduleArgs, addtlGitArgs...) submoduleArgs = append(submoduleArgs, "submodule", "update", "--init", "--recursive") if useDepthForSubmodule && flagValues.depth > 0 { submoduleArgs = append(submoduleArgs, "--depth", fmt.Sprintf("%d", flagValues.depth)) From 22a4f5880f72d3fb888fad052b5c2edd945334f1 Mon Sep 17 00:00:00 2001 From: Matthias Diester Date: Tue, 30 Nov 2021 14:45:13 +0100 Subject: [PATCH 2/3] Move Git URL rewrite into private key section The Git URL rewrite logic was located outside of the credentials section of the code, which means it was used in any case. However, it does not make sense to be used in combination with a HTTPS URL and basic auth. Move section as-is into the private key section and setup. --- cmd/git/main.go | 59 +++++++++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 27 deletions(-) diff --git a/cmd/git/main.go b/cmd/git/main.go index 4a44128fa5..8d0a0b5585 100644 --- a/cmd/git/main.go +++ b/cmd/git/main.go @@ -296,6 +296,38 @@ func clone(ctx context.Context) error { fmt.Sprintf(`core.sshCommand=%s`, strings.Join(sshCmd, " ")), ) + // When the Git URL rewrite is enabled, additional Git config + // options are required to introduce a rewrite rule so that + // HTTPS URLs are rewritten into Git+SSH URLs on the fly for + // the main clone as well as the submodule operations. This + // only makes sense in case a private key is configured. + if flagValues.gitURLRewrite { + var hostname string + switch { + case strings.HasPrefix(flagValues.url, "git@"): + trimmed := strings.TrimPrefix(flagValues.url, "git@") + splitted := strings.SplitN(trimmed, ":", 2) + hostname = splitted[0] + + case strings.HasPrefix(flagValues.url, "http"): + repoURL, err := url.Parse(flagValues.url) + if err != nil { + return err + } + hostname = repoURL.Host + + default: + log.Printf("Failed to setup Git URL rewrite, unknown/unsupported URL type: %q\n", flagValues.url) + } + + if hostname != "" { + addtlGitArgs = append(addtlGitArgs, + "-c", + fmt.Sprintf("url.ssh://git@%s/.insteadOf=https://%s/", hostname, hostname), + ) + } + } + case typeUsernamePassword: repoURL, err := url.Parse(flagValues.url) if err != nil { @@ -332,33 +364,6 @@ func clone(ctx context.Context) error { } } - if flagValues.gitURLRewrite { - var hostname string - switch { - case strings.HasPrefix(flagValues.url, "git@"): - trimmed := strings.TrimPrefix(flagValues.url, "git@") - splitted := strings.SplitN(trimmed, ":", 2) - hostname = splitted[0] - - case strings.HasPrefix(flagValues.url, "http"): - repoURL, err := url.Parse(flagValues.url) - if err != nil { - return err - } - hostname = repoURL.Host - - default: - log.Printf("Failed to setup Git URL rewrite, unknown/unsupported URL type: %q\n", flagValues.url) - } - - if hostname != "" { - addtlGitArgs = append(addtlGitArgs, - "-c", - fmt.Sprintf("url.ssh://git@%s/.insteadOf=https://%s/", hostname, hostname), - ) - } - } - cloneArgs = append(cloneArgs, addtlGitArgs...) cloneArgs = append(cloneArgs, "--", flagValues.url, flagValues.target) if _, err := git(ctx, cloneArgs...); err != nil { From 21e06d84b97a28322efb07f801bf94351e38da6a Mon Sep 17 00:00:00 2001 From: Matthias Diester Date: Tue, 30 Nov 2021 15:04:39 +0100 Subject: [PATCH 3/3] Fix Git URL rewrite issue for HTTPS URLs Fix credentials checks to allow for the combination of: - HTTPS URL, - private SSH key, and - Git URL rewrite enabled. --- cmd/git/main.go | 4 ++++ cmd/git/main_test.go | 20 +++++++++++++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/cmd/git/main.go b/cmd/git/main.go index 8d0a0b5585..48ab57a04a 100644 --- a/cmd/git/main.go +++ b/cmd/git/main.go @@ -450,10 +450,14 @@ func checkCredentials() (credentialType, error) { // in which case there is a file called ssh-privatekey hasPrivateKey := hasFile(flagValues.secretPath, "ssh-privatekey") isSSHGitURL := sshGitURLRegEx.MatchString(flagValues.url) + isGitURLRewriteSet := flagValues.gitURLRewrite switch { case hasPrivateKey && isSSHGitURL: return typePrivateKey, nil + case hasPrivateKey && !isSSHGitURL && isGitURLRewriteSet: + return typePrivateKey, nil + case hasPrivateKey && !isSSHGitURL: return typeUndef, &ExitError{Code: 110, Message: "Credential/URL inconsistency: SSH credentials provided, but URL is not a SSH Git URL"} diff --git a/cmd/git/main_test.go b/cmd/git/main_test.go index 81ff1c6f25..a5c375add4 100644 --- a/cmd/git/main_test.go +++ b/cmd/git/main_test.go @@ -252,7 +252,25 @@ var _ = Describe("Git Resource", func() { }) }) - It("should Git clone a private repository using a SSH private key that contains a HTTPS submodule", func() { + It("should Git clone a private repository using HTTPS URL and a SSH private key provided via a secret when Git URL rewrite is enabled", func() { + withTempDir(func(secret string) { + // Mock the filesystem state of `kubernetes.io/ssh-auth` type secret volume mount + file(filepath.Join(secret, "ssh-privatekey"), 0400, []byte(sshPrivateKey)) + + withTempDir(func(target string) { + Expect(run( + "--url", "https://github.com/shipwright-io/sample-nodejs-private.git", + "--secret-path", secret, + "--target", target, + "--git-url-rewrite", + )).ToNot(HaveOccurred()) + + Expect(filepath.Join(target, "README.md")).To(BeAnExistingFile()) + }) + }) + }) + + It("should Git clone a private repository using a SSH private key that contains a HTTPS submodule when Git URL rewrite is enabled", func() { withTempDir(func(secret string) { // Mock the filesystem state of `kubernetes.io/ssh-auth` type secret volume mount file(filepath.Join(secret, "ssh-privatekey"), 0400, []byte(sshPrivateKey))