Respect configured user in SSH Git repository URL

We had a hardcoded assumption that the SSH user for a Git repository is
always "git". This is however not true in all scenarios, for example
when one is making use of Gerrit for team code collaboration, as users
there have their own username for (SSH) Git operations.

This commit changes the logic of the auth strategy helpers to:

1. Select the auth strategy based on the protocol of the parsed URL,
   instead of a simple rely on a correct prefix.
2. Use the user information from the parsed URL to configure the user
   for the public key authentication strategy, with a fallback to `git`
   if none is defined.

Signed-off-by: Hidde Beydals <[email protected]>

Author: hiddeco

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

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 {
+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 strings.HasPrefix(url, "http"):
-		return &BasicAuth{}
-	case strings.HasPrefix(url, "ssh"):
-		return &PublicKeyAuth{}
+	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
 	}

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://[email protected]: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)
 			}
 		})