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 <hello@hidde.co>
This commit is contained in:
parent
1ea9999c2c
commit
67b10aad22
|
@ -184,15 +184,19 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, repository sour
|
||||||
|
|
||||||
// determine auth method
|
// determine auth method
|
||||||
var auth transport.AuthMethod
|
var auth transport.AuthMethod
|
||||||
authStrategy := git.AuthSecretStrategyForURL(repository.Spec.URL)
|
if repository.Spec.SecretRef != nil {
|
||||||
if repository.Spec.SecretRef != nil && authStrategy != nil {
|
authStrategy, err := git.AuthSecretStrategyForURL(repository.Spec.URL)
|
||||||
|
if err != nil {
|
||||||
|
return sourcev1.GitRepositoryNotReady(repository, sourcev1.AuthenticationFailedReason, err.Error()), err
|
||||||
|
}
|
||||||
|
|
||||||
name := types.NamespacedName{
|
name := types.NamespacedName{
|
||||||
Namespace: repository.GetNamespace(),
|
Namespace: repository.GetNamespace(),
|
||||||
Name: repository.Spec.SecretRef.Name,
|
Name: repository.Spec.SecretRef.Name,
|
||||||
}
|
}
|
||||||
|
|
||||||
var secret corev1.Secret
|
var secret corev1.Secret
|
||||||
err := r.Client.Get(ctx, name, &secret)
|
err = r.Client.Get(ctx, name, &secret)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
err = fmt.Errorf("auth secret error: %w", err)
|
err = fmt.Errorf("auth secret error: %w", err)
|
||||||
return sourcev1.GitRepositoryNotReady(repository, sourcev1.AuthenticationFailedReason, err.Error()), err
|
return sourcev1.GitRepositoryNotReady(repository, sourcev1.AuthenticationFailedReason, err.Error()), err
|
||||||
|
|
|
@ -18,7 +18,7 @@ package git
|
||||||
|
|
||||||
import (
|
import (
|
||||||
"fmt"
|
"fmt"
|
||||||
"strings"
|
"net/url"
|
||||||
|
|
||||||
"github.com/go-git/go-git/v5/plumbing/transport"
|
"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/http"
|
||||||
|
@ -28,14 +28,21 @@ import (
|
||||||
"github.com/fluxcd/pkg/ssh/knownhosts"
|
"github.com/fluxcd/pkg/ssh/knownhosts"
|
||||||
)
|
)
|
||||||
|
|
||||||
func AuthSecretStrategyForURL(url string) AuthSecretStrategy {
|
const defaultPublicKeyAuthUser = "git"
|
||||||
switch {
|
|
||||||
case strings.HasPrefix(url, "http"):
|
func AuthSecretStrategyForURL(URL string) (AuthSecretStrategy, error) {
|
||||||
return &BasicAuth{}
|
u, err := url.Parse(URL)
|
||||||
case strings.HasPrefix(url, "ssh"):
|
if err != nil {
|
||||||
return &PublicKeyAuth{}
|
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 {
|
type AuthSecretStrategy interface {
|
||||||
|
@ -58,7 +65,9 @@ func (s *BasicAuth) Method(secret corev1.Secret) (transport.AuthMethod, error) {
|
||||||
return auth, nil
|
return auth, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
type PublicKeyAuth struct{}
|
type PublicKeyAuth struct {
|
||||||
|
user string
|
||||||
|
}
|
||||||
|
|
||||||
func (s *PublicKeyAuth) Method(secret corev1.Secret) (transport.AuthMethod, error) {
|
func (s *PublicKeyAuth) Method(secret corev1.Secret) (transport.AuthMethod, error) {
|
||||||
identity := secret.Data["identity"]
|
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)
|
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 {
|
if err != nil {
|
||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|
|
@ -66,19 +66,25 @@ var (
|
||||||
|
|
||||||
func TestAuthSecretStrategyForURL(t *testing.T) {
|
func TestAuthSecretStrategyForURL(t *testing.T) {
|
||||||
tests := []struct {
|
tests := []struct {
|
||||||
name string
|
name string
|
||||||
url string
|
url string
|
||||||
want AuthSecretStrategy
|
want AuthSecretStrategy
|
||||||
|
wantErr bool
|
||||||
}{
|
}{
|
||||||
{"HTTP", "http://git.example.com/org/repo.git", &BasicAuth{}},
|
{"HTTP", "http://git.example.com/org/repo.git", &BasicAuth{}, false},
|
||||||
{"HTTPS", "https://git.example.com/org/repo.git", &BasicAuth{}},
|
{"HTTPS", "https://git.example.com/org/repo.git", &BasicAuth{}, false},
|
||||||
{"SSH", "ssh://git.example.com:2222/org/repo.git", &PublicKeyAuth{}},
|
{"SSH", "ssh://git.example.com:2222/org/repo.git", &PublicKeyAuth{}, false},
|
||||||
{"unsupported", "protocol://example.com", nil},
|
{"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 {
|
for _, tt := range tests {
|
||||||
t.Run(tt.name, func(t *testing.T) {
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
got := AuthSecretStrategyForURL(tt.url)
|
got, err := AuthSecretStrategyForURL(tt.url)
|
||||||
if reflect.TypeOf(got) != reflect.TypeOf(tt.want) {
|
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)
|
t.Errorf("AuthSecretStrategyForURL() got = %v, want %v", got, tt.want)
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
|
|
Loading…
Reference in New Issue