From 5ff65d7ae731dff55efb429c7fea0425d7917064 Mon Sep 17 00:00:00 2001 From: Philip Laine Date: Mon, 30 Nov 2020 21:37:26 +0100 Subject: [PATCH] Add user support to git2go implementation Signed-off-by: Philip Laine --- controllers/gitrepository_controller.go | 8 ++++-- pkg/git/common/common.go | 5 ++-- pkg/git/git.go | 2 +- pkg/git/v1/transport.go | 4 +-- pkg/git/v2/transport.go | 35 ++++++++++++++++++------- pkg/git/v2/trasnport_test.go | 24 ++++++++++------- 6 files changed, 51 insertions(+), 27 deletions(-) diff --git a/controllers/gitrepository_controller.go b/controllers/gitrepository_controller.go index 403e82fa..5593d7c5 100644 --- a/controllers/gitrepository_controller.go +++ b/controllers/gitrepository_controller.go @@ -182,8 +182,12 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, repository sour // determine auth method auth := &common.Auth{} - authStrategy := git.AuthSecretStrategyForURL(repository.Spec.URL, repository.Spec.GitProtocolV2Compatibility) - if repository.Spec.SecretRef != nil && authStrategy != nil { + if repository.Spec.SecretRef != nil { + authStrategy, err := git.AuthSecretStrategyForURL(repository.Spec.URL, repository.Spec.GitProtocolV2Compatibility) + if err != nil { + return sourcev1.GitRepositoryNotReady(repository, sourcev1.AuthenticationFailedReason, err.Error()), err + } + name := types.NamespacedName{ Namespace: repository.GetNamespace(), Name: repository.Spec.SecretRef.Name, diff --git a/pkg/git/common/common.go b/pkg/git/common/common.go index 4827791e..6f502043 100644 --- a/pkg/git/common/common.go +++ b/pkg/git/common/common.go @@ -25,8 +25,9 @@ import ( ) const ( - DefaultOrigin = "origin" - DefaultBranch = "master" + DefaultOrigin = "origin" + DefaultBranch = "master" + DefaultPublicKeyAuthUser = "git" ) type Commit interface { diff --git a/pkg/git/git.go b/pkg/git/git.go index 64c50a65..93416fe4 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -35,7 +35,7 @@ func CheckoutStrategyForRef(ref *sourcev1.GitRepositoryRef, useGitV2 bool) commo return gitv1.CheckoutStrategyForRef(ref) } -func AuthSecretStrategyForURL(url string, useGitV2 bool) common.AuthSecretStrategy { +func AuthSecretStrategyForURL(url string, useGitV2 bool) (common.AuthSecretStrategy, error) { if useGitV2 { return gitv2.AuthSecretStrategyForURL(url) } diff --git a/pkg/git/v1/transport.go b/pkg/git/v1/transport.go index 33976f97..66124060 100644 --- a/pkg/git/v1/transport.go +++ b/pkg/git/v1/transport.go @@ -28,8 +28,6 @@ import ( "github.com/fluxcd/source-controller/pkg/git/common" ) -const defaultPublicKeyAuthUser = "git" - func AuthSecretStrategyForURL(URL string) (common.AuthSecretStrategy, error) { u, err := url.Parse(URL) if err != nil { @@ -75,7 +73,7 @@ func (s *PublicKeyAuth) Method(secret corev1.Secret) (*common.Auth, error) { user := s.user if user == "" { - user = defaultPublicKeyAuthUser + user = common.DefaultPublicKeyAuthUser } pk, err := ssh.NewPublicKeys(user, identity, "") diff --git a/pkg/git/v2/transport.go b/pkg/git/v2/transport.go index 8450336f..d39b13b6 100644 --- a/pkg/git/v2/transport.go +++ b/pkg/git/v2/transport.go @@ -22,6 +22,7 @@ import ( "crypto/sha1" "fmt" "golang.org/x/crypto/ssh" + "net/url" "strings" "github.com/fluxcd/source-controller/pkg/git/common" @@ -29,14 +30,20 @@ import ( corev1 "k8s.io/api/core/v1" ) -func AuthSecretStrategyForURL(url string) common.AuthSecretStrategy { - switch { - case strings.HasPrefix(url, "http"): - return &BasicAuth{} - case strings.HasPrefix(url, "ssh"): - return &PublicKeyAuth{} +func AuthSecretStrategyForURL(URL string) (common.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 BasicAuth struct{} @@ -65,7 +72,9 @@ func (s *BasicAuth) Method(secret corev1.Secret) (*common.Auth, error) { return &common.Auth{CredCallback: credCallback, CertCallback: nil}, nil } -type PublicKeyAuth struct{} +type PublicKeyAuth struct { + user string +} func (s *PublicKeyAuth) Method(secret corev1.Secret) (*common.Auth, error) { identity := secret.Data["identity"] @@ -79,14 +88,20 @@ func (s *PublicKeyAuth) Method(secret corev1.Secret) (*common.Auth, error) { return nil, err } - // Need to validate private key + // Need to validate private key as it is not + // done by git2go when loading the key _, err = ssh.ParsePrivateKey(identity) if err != nil { return nil, err } + user := s.user + if user == "" { + user = common.DefaultPublicKeyAuthUser + } + credCallback := func(url string, username_from_url string, allowed_types git2go.CredType) (*git2go.Cred, error) { - cred, err := git2go.NewCredSshKeyFromMemory("git", "", string(identity), "") + cred, err := git2go.NewCredSshKeyFromMemory(user, "", string(identity), "") if err != nil { return nil, err } diff --git a/pkg/git/v2/trasnport_test.go b/pkg/git/v2/trasnport_test.go index 31b2e7f1..5729b47b 100644 --- a/pkg/git/v2/trasnport_test.go +++ b/pkg/git/v2/trasnport_test.go @@ -66,19 +66,25 @@ var ( func TestAuthSecretStrategyForURL(t *testing.T) { tests := []struct { - name string - url string - want common.AuthSecretStrategy + name string + url string + want common.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) } })