diff --git a/controllers/gitrepository_controller.go b/controllers/gitrepository_controller.go index 6c5f56bc..4dea3e4c 100644 --- a/controllers/gitrepository_controller.go +++ b/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 diff --git a/internal/git/transport.go b/internal/git/transport.go index a796ef3b..ca0d20a7 100644 --- a/internal/git/transport.go +++ b/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 { diff --git a/internal/git/transport_test.go b/internal/git/transport_test.go index 224d5bd9..16254cd3 100644 --- a/internal/git/transport_test.go +++ b/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 } })