From ad397bd0a66e6865ea0ffc4d06c0d1ca204a49c8 Mon Sep 17 00:00:00 2001 From: Tyler McGoffin Date: Tue, 26 Nov 2024 16:08:51 -0800 Subject: [PATCH] Fix typos and add tests for CredentialPatternFrom* functions --- git/client.go | 32 +++++++++--------- git/client_test.go | 83 +++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 98 insertions(+), 17 deletions(-) diff --git a/git/client.go b/git/client.go index 8cf4dff33..9526d1249 100644 --- a/git/client.go +++ b/git/client.go @@ -96,21 +96,21 @@ func (c *Client) Command(ctx context.Context, args ...string) (*Command, error) } // CredentialPattern is used to indicate to AuthenticatedCommand which patterns git should match -// against when trying to find credentials. It is a little overengineered as a type because we +// against when trying to find credentials. It is a little over-engineered as a type because we // want AuthenticatedCommand to have a clear complication error when this is not provided, // as opposed to using a string which might compile with `client.AuthenticatedCommand(ctx, "fetch")`. // // It is only usable when constructed by another function in the package because the empty pattern, // without insecure set to true, will result in an error in AuthenticatedCommand. // -// Callers can currently opt-in to an insecure mode for backwards compatability by using +// Callers can currently opt-in to an insecure mode for backwards compatibility by using // InsecureAllMatchingCredentialsPattern. type CredentialPattern struct { insecure bool // should only be constructable via InsecureAllMatchingCredentialPattern pattern string } -// InsecureAllMatchingCredentialsPattern allows for opting in to an insecure mode for backwards compatability. +// InsecureAllMatchingCredentialsPattern allows for opting in to an insecure mode for backwards compatibility. var InsecureAllMatchingCredentialsPattern = CredentialPattern{insecure: true, pattern: ""} var disallowedCredentialPattern = CredentialPattern{insecure: false, pattern: ""} @@ -126,6 +126,18 @@ func CredentialPatternFromRemote(ctx context.Context, c *Client, remote string) return CredentialPatternFromGitURL(gitURL) } +// Cool cool cool... +// YOLO +func CredentialPatternFromGitURL(gitURL string) (CredentialPattern, error) { + normalizedURL, err := ParseURL(gitURL) + if err != nil { + return CredentialPattern{}, fmt.Errorf("failed to parse remote URL: %w", err) + } + return CredentialPattern{ + pattern: strings.TrimSuffix(ghinstance.HostPrefix(normalizedURL.Host), "/"), + }, nil +} + // AuthenticatedCommand is a wrapper around Command that included configuration to use gh // as the credential helper for git. func (c *Client) AuthenticatedCommand(ctx context.Context, credentialPattern CredentialPattern, args ...string) (*Command, error) { @@ -137,7 +149,7 @@ func (c *Client) AuthenticatedCommand(ctx context.Context, credentialPattern Cre var preArgs []string if credentialPattern == disallowedCredentialPattern { - return nil, fmt.Errorf("empty credential pattern is not allowed execept explicitly") + return nil, fmt.Errorf("empty credential pattern is not allowed unless provided explicitly") } else if credentialPattern == InsecureAllMatchingCredentialsPattern { preArgs = []string{"-c", "credential.helper="} preArgs = append(preArgs, "-c", fmt.Sprintf("credential.helper=%s", credHelper)) @@ -789,15 +801,3 @@ var globReplacer = strings.NewReplacer( func escapeGlob(p string) string { return globReplacer.Replace(p) } - -// Cool cool cool... -// YOLO -func CredentialPatternFromGitURL(gitURL string) (CredentialPattern, error) { - normalizedURL, err := ParseURL(gitURL) - if err != nil { - return CredentialPattern{}, fmt.Errorf("failed to parse remote URL: %w", err) - } - return CredentialPattern{ - pattern: strings.TrimSuffix(ghinstance.HostPrefix(normalizedURL.Host), "/"), - }, nil -} diff --git a/git/client_test.go b/git/client_test.go index 9cacf4e65..72a780ae6 100644 --- a/git/client_test.go +++ b/git/client_test.go @@ -88,7 +88,7 @@ func TestClientAuthenticatedCommand(t *testing.T) { { name: "errors when attempting to use an empty pattern that isn't marked insecure", pattern: CredentialPattern{insecure: false, pattern: ""}, - wantErr: fmt.Errorf("empty credential pattern is not allowed execept explicitly"), + wantErr: fmt.Errorf("empty credential pattern is not allowed except explicitly"), }, } for _, tt := range tests { @@ -1554,6 +1554,87 @@ func TestHelperProcess(t *testing.T) { os.Exit(0) } +func TestCredentialPatternFromGitURL(t *testing.T) { + tests := []struct { + name string + gitURL string + wantErr bool + wantCredentialPattern CredentialPattern + }{ + { + name: "Given a well formed gitURL, it returns the corresponding CredentialPattern", + gitURL: "https://github.com/OWNER/REPO", + wantCredentialPattern: CredentialPattern{ + pattern: "https://github.com", + insecure: false, + }, + }, + { + name: "Given a malformed gitURL, it returns an error", + // This pattern is copied from the tests in ParseURL + // Unexpectedly, a non URL-like string did not error in ParseURL + gitURL: "ssh://git@[/tmp/git-repo", + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + credentialPattern, err := CredentialPatternFromGitURL(tt.gitURL) + if tt.wantErr { + assert.ErrorContains(t, err, "failed to parse remote URL") + } else { + assert.NoError(t, err) + assert.Equal(t, tt.wantCredentialPattern, credentialPattern) + } + }) + } +} + +func TestCredentialPatternFromRemote(t *testing.T) { + tests := []struct { + name string + remote string + wantCredentialPattern CredentialPattern + wantErr bool + }{ + { + name: "Given a well formed remote, it returns the corresponding CredentialPattern", + remote: "https://github.com/OWNER/REPO", + wantCredentialPattern: CredentialPattern{ + pattern: "https://github.com", + insecure: false, + }, + }, + { + name: "Given an error from GetRemoteURL, it returns that error", + remote: "foo remote", + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var cmdCtx func(ctx context.Context, name string, args ...string) *exec.Cmd + if tt.wantErr { + _, cmdCtx = createCommandContext(t, 1, tt.remote, "GetRemoteURL error") + } else { + _, cmdCtx = createCommandContext(t, 0, tt.remote, "") + } + + client := Client{ + GitPath: "path/to/git", + commandContext: cmdCtx, + } + credentialPattern, err := CredentialPatternFromRemote(context.Background(), &client, tt.remote) + if tt.wantErr { + assert.ErrorContains(t, err, "GetRemoteURL error") + } else { + assert.NoError(t, err) + assert.Equal(t, tt.wantCredentialPattern, credentialPattern) + } + }) + } +} + func createCommandContext(t *testing.T, exitStatus int, stdout, stderr string) (*exec.Cmd, commandCtx) { cmd := exec.CommandContext(context.Background(), os.Args[0], "-test.run=TestHelperProcess", "--") cmd.Env = []string{