git: refactor transport into strategies

Author: hiddeco

controllers/gitrepository_controller.go

@@ -149,6 +149,7 @@ func (r *GitRepositoryReconciler) sync(ctx context.Context, repository sourcev1.
 	}
 
 	// determine auth method
+	strategy := intgit.AuthSecretStrategyForURL(repository.Spec.URL)
 	var auth transport.AuthMethod
 	if repository.Spec.SecretRef != nil {
 		name := types.NamespacedName{
@@ -163,12 +164,11 @@ func (r *GitRepositoryReconciler) sync(ctx context.Context, repository sourcev1.
 			return sourcev1.GitRepositoryNotReady(repository, sourcev1.AuthenticationFailedReason, err.Error()), err
 		}
 
-		method, err := intgit.AuthMethodFromSecret(repository.Spec.URL, secret)
+		auth, err = strategy.Method(secret)
 		if err != nil {
 			err = fmt.Errorf("auth error: %w", err)
 			return sourcev1.GitRepositoryNotReady(repository, sourcev1.AuthenticationFailedReason, err.Error()), err
 		}
-		auth = method
 	}
 
 	// create tmp dir for the Git clone

internal/git/transport.go

@@ -28,17 +28,23 @@ import (
 	"github.com/fluxcd/source-controller/internal/crypto/ssh/knownhosts"
 )
 
-func AuthMethodFromSecret(url string, secret corev1.Secret) (transport.AuthMethod, error) {
+func AuthSecretStrategyForURL(url string) AuthSecretStrategy {
 	switch {
 	case strings.HasPrefix(url, "http"):
-		return BasicAuthFromSecret(secret)
+		return &BasicAuth{}
 	case strings.HasPrefix(url, "ssh"):
-		return PublicKeysFromSecret(secret)
+		return &PublicKeyAuth{}
 	}
-	return nil, nil
+	return nil
 }
 
-func BasicAuthFromSecret(secret corev1.Secret) (*http.BasicAuth, error) {
+type AuthSecretStrategy interface {
+	Method(secret corev1.Secret) (transport.AuthMethod, error)
+}
+
+type BasicAuth struct{}
+
+func (s *BasicAuth) Method(secret corev1.Secret) (transport.AuthMethod, error) {
 	auth := &http.BasicAuth{}
 	if username, ok := secret.Data["username"]; ok {
 		auth.Username = string(username)
@@ -52,7 +58,9 @@ func BasicAuthFromSecret(secret corev1.Secret) (*http.BasicAuth, error) {
 	return auth, nil
 }
 
-func PublicKeysFromSecret(secret corev1.Secret) (*ssh.PublicKeys, error) {
+type PublicKeyAuth struct{}
+
+func (s *PublicKeyAuth) Method(secret corev1.Secret) (transport.AuthMethod, error) {
 	identity := secret.Data["identity"]
 	knownHosts := secret.Data["known_hosts"]
 	if len(identity) == 0 || len(knownHosts) == 0 {

internal/git/transport_test.go

@@ -22,7 +22,6 @@ import (
 
 	"github.com/go-git/go-git/v5/plumbing/transport"
 	"github.com/go-git/go-git/v5/plumbing/transport/http"
-	"github.com/go-git/go-git/v5/plumbing/transport/ssh"
 	corev1 "k8s.io/api/core/v1"
 )
 
@@ -65,39 +64,33 @@ var (
 	}
 )
 
-func TestAuthMethodFromSecret(t *testing.T) {
+func TestAuthSecretStrategyForURL(t *testing.T) {
 	tests := []struct {
-		name    string
-		url     string
-		secret  corev1.Secret
-		want    transport.AuthMethod
-		wantErr bool
+		name string
+		url  string
+		want AuthSecretStrategy
 	}{
-		{"HTTP", "http://git.example.com/org/repo.git", basicAuthSecretFixture, &http.BasicAuth{}, false},
-		{"HTTPS", "https://git.example.com/org/repo.git", basicAuthSecretFixture, &http.BasicAuth{}, false},
-		{"SSH", "ssh://git.example.com:2222/org/repo.git", privateKeySecretFixture, &ssh.PublicKeys{}, false},
-		{"unsupported", "protocol://git.example.com/org/repo.git", corev1.Secret{}, nil, false},
+		{"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},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := AuthMethodFromSecret(tt.url, tt.secret)
-			if (err != nil) != tt.wantErr {
-				t.Errorf("AuthMethodFromSecret() error = %v, wantErr %v", err, tt.wantErr)
-				return
-			}
+			got := AuthSecretStrategyForURL(tt.url)
 			if reflect.TypeOf(got) != reflect.TypeOf(tt.want) {
-				t.Errorf("AuthMethodFromSecret() got = %v, want %v", got, tt.want)
+				t.Errorf("AuthSecretStrategyForURL() got = %v, want %v", got, tt.want)
 			}
 		})
 	}
 }
 
-func TestBasicAuthFromSecret(t *testing.T) {
+func TestBasicAuthStrategy_Method(t *testing.T) {
 	tests := []struct {
 		name    string
 		secret  corev1.Secret
 		modify  func(secret *corev1.Secret)
-		want    *http.BasicAuth
+		want    transport.AuthMethod
 		wantErr bool
 	}{
 		{"username and password", basicAuthSecretFixture, nil, &http.BasicAuth{Username: "git", Password: "password"}, false},
@@ -111,19 +104,20 @@ func TestBasicAuthFromSecret(t *testing.T) {
 			if tt.modify != nil {
 				tt.modify(secret)
 			}
-			got, err := BasicAuthFromSecret(*secret)
+			s := &BasicAuth{}
+			got, err := s.Method(*secret)
 			if (err != nil) != tt.wantErr {
-				t.Errorf("BasicAuthFromSecret() error = %v, wantErr %v", err, tt.wantErr)
+				t.Errorf("Method() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
 			if !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("BasicAuthFromSecret() got = %v, want %v", got, tt.want)
+				t.Errorf("Method() got = %v, want %v", got, tt.want)
 			}
 		})
 	}
 }
 
-func TestPublicKeysFromSecret(t *testing.T) {
+func TestPublicKeyStrategy_Method(t *testing.T) {
 	tests := []struct {
 		name    string
 		secret  corev1.Secret
@@ -143,9 +137,10 @@ func TestPublicKeysFromSecret(t *testing.T) {
 			if tt.modify != nil {
 				tt.modify(secret)
 			}
-			_, err := PublicKeysFromSecret(*secret)
+			s := &PublicKeyAuth{}
+			_, err := s.Method(*secret)
 			if (err != nil) != tt.wantErr {
-				t.Errorf("PublicKeysFromSecret() error = %v, wantErr %v", err, tt.wantErr)
+				t.Errorf("Method() error = %v, wantErr %v", err, tt.wantErr)
 				return
 			}
 		})