diff --git a/controllers/gitrepository_controller.go b/controllers/gitrepository_controller.go index ae9f621d..c45da376 100644 --- a/controllers/gitrepository_controller.go +++ b/controllers/gitrepository_controller.go @@ -184,15 +184,19 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, repository sour // determine auth method var auth transport.AuthMethod - authStrategy := git.AuthSecretStrategyForURL(repository.Spec.URL) - if repository.Spec.SecretRef != nil && authStrategy != nil { + if repository.Spec.SecretRef != nil { + authStrategy, err := git.AuthSecretStrategyForURL(repository.Spec.URL) + if err != nil { + return sourcev1.GitRepositoryNotReady(repository, sourcev1.AuthenticationFailedReason, err.Error()), err + } + name := types.NamespacedName{ Namespace: repository.GetNamespace(), Name: repository.Spec.SecretRef.Name, } var secret corev1.Secret - err := r.Client.Get(ctx, name, &secret) + err = r.Client.Get(ctx, name, &secret) if err != nil { err = fmt.Errorf("auth secret error: %w", err) return sourcev1.GitRepositoryNotReady(repository, sourcev1.AuthenticationFailedReason, err.Error()), err diff --git a/pkg/git/transport.go b/pkg/git/transport.go index b542a454..a298e1f3 100644 --- a/pkg/git/transport.go +++ b/pkg/git/transport.go @@ -18,7 +18,7 @@ package git import ( "fmt" - "strings" + "net/url" "github.com/go-git/go-git/v5/plumbing/transport" "github.com/go-git/go-git/v5/plumbing/transport/http" @@ -28,14 +28,21 @@ import ( "github.com/fluxcd/pkg/ssh/knownhosts" ) -func AuthSecretStrategyForURL(url string) AuthSecretStrategy { - switch { - case strings.HasPrefix(url, "http"): - return &BasicAuth{} - case strings.HasPrefix(url, "ssh"): - return &PublicKeyAuth{} +const defaultPublicKeyAuthUser = "git" + +func AuthSecretStrategyForURL(URL string) (AuthSecretStrategy, error) { + u, err := url.Parse(URL) + if err != nil { + return nil, fmt.Errorf("failed to parse URL to determine auth strategy: %w", err) + } + switch { + case u.Scheme == "http", u.Scheme == "https": + return &BasicAuth{}, nil + case u.Scheme == "ssh": + return &PublicKeyAuth{user: u.User.Username()}, nil + default: + return nil, fmt.Errorf("no auth secret strategy for scheme %s", u.Scheme) } - return nil } type AuthSecretStrategy interface { @@ -58,7 +65,9 @@ func (s *BasicAuth) Method(secret corev1.Secret) (transport.AuthMethod, error) { return auth, nil } -type PublicKeyAuth struct{} +type PublicKeyAuth struct { + user string +} func (s *PublicKeyAuth) Method(secret corev1.Secret) (transport.AuthMethod, error) { identity := secret.Data["identity"] @@ -67,7 +76,12 @@ func (s *PublicKeyAuth) Method(secret corev1.Secret) (transport.AuthMethod, erro return nil, fmt.Errorf("invalid '%s' secret data: required fields 'identity' and 'known_hosts'", secret.Name) } - pk, err := ssh.NewPublicKeys("git", identity, "") + user := s.user + if user == "" { + user = defaultPublicKeyAuthUser + } + + pk, err := ssh.NewPublicKeys(user, identity, "") if err != nil { return nil, err } diff --git a/pkg/git/transport_test.go b/pkg/git/transport_test.go index d77e11fc..a73e34f0 100644 --- a/pkg/git/transport_test.go +++ b/pkg/git/transport_test.go @@ -66,19 +66,25 @@ var ( func TestAuthSecretStrategyForURL(t *testing.T) { tests := []struct { - name string - url string - want AuthSecretStrategy + name string + url string + want AuthSecretStrategy + wantErr bool }{ - {"HTTP", "http://git.example.com/org/repo.git", &BasicAuth{}}, - {"HTTPS", "https://git.example.com/org/repo.git", &BasicAuth{}}, - {"SSH", "ssh://git.example.com:2222/org/repo.git", &PublicKeyAuth{}}, - {"unsupported", "protocol://example.com", nil}, + {"HTTP", "http://git.example.com/org/repo.git", &BasicAuth{}, false}, + {"HTTPS", "https://git.example.com/org/repo.git", &BasicAuth{}, false}, + {"SSH", "ssh://git.example.com:2222/org/repo.git", &PublicKeyAuth{}, false}, + {"SSH with username", "ssh://example@git.example.com:2222/org/repo.git", &PublicKeyAuth{user: "example"}, false}, + {"unsupported", "protocol://example.com", nil, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := AuthSecretStrategyForURL(tt.url) - if reflect.TypeOf(got) != reflect.TypeOf(tt.want) { + got, err := AuthSecretStrategyForURL(tt.url) + if (err != nil) != tt.wantErr { + t.Errorf("AuthSecretStrategyForURL() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { t.Errorf("AuthSecretStrategyForURL() got = %v, want %v", got, tt.want) } })