From 86330bfecf6a7ed9c83004d349a7df0318c8df68 Mon Sep 17 00:00:00 2001 From: Soule BA Date: Mon, 26 Sep 2022 16:10:53 +0200 Subject: [PATCH] Refactor to use authn for authentication as OCIrepository does If implemented the oras registry loginOption will only be used internaly with the specific ChartRepo struct. This will permit reusing more easily feature developped with googlecontainerregistry authn. Signed-off-by: Soule BA --- controllers/helmchart_controller.go | 93 +++++++++++-------- controllers/helmrepository_controller_oci.go | 84 ++++++++++------- internal/helm/registry/auth.go | 56 +++++++++-- internal/helm/registry/auth_test.go | 21 +++-- .../helm/repository/oci_chart_repository.go | 1 + 5 files changed, 166 insertions(+), 89 deletions(-) diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index 965ddcedc..3cc7cc66a 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -56,6 +56,7 @@ import ( "github.com/fluxcd/pkg/runtime/patch" "github.com/fluxcd/pkg/runtime/predicates" "github.com/fluxcd/pkg/untar" + "github.com/google/go-containerregistry/pkg/authn" sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" "github.com/fluxcd/source-controller/internal/cache" @@ -454,8 +455,9 @@ func (r *HelmChartReconciler) reconcileSource(ctx context.Context, obj *sourcev1 func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *sourcev1.HelmChart, repo *sourcev1.HelmRepository, b *chart.Build) (sreconcile.Result, error) { var ( - tlsConfig *tls.Config - loginOpts []helmreg.LoginOption + tlsConfig *tls.Config + authenticator authn.Authenticator + keychain authn.Keychain ) // Used to login with the repository declared provider ctxTimeout, cancel := context.WithTimeout(ctx, repo.Spec.Timeout.Duration) @@ -480,10 +482,10 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * } // Build client options from secret - opts, err := getter.ClientOptionsFromSecret(*secret) + opts, tls, err := r.clientOptionsFromSecret(secret, normalizedURL) if err != nil { e := &serror.Event{ - Err: fmt.Errorf("failed to configure Helm client with secret data: %w", err), + Err: err, Reason: sourcev1.AuthenticationFailedReason, } conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) @@ -491,20 +493,10 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * return sreconcile.ResultEmpty, e } clientOpts = append(clientOpts, opts...) - - tlsConfig, err = getter.TLSClientConfigFromSecret(*secret, normalizedURL) - if err != nil { - e := &serror.Event{ - Err: fmt.Errorf("failed to create TLS client config with secret data: %w", err), - Reason: sourcev1.AuthenticationFailedReason, - } - conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) - // Requeue as content of secret might change - return sreconcile.ResultEmpty, e - } + tlsConfig = tls // Build registryClient options from secret - loginOpt, err := registry.LoginOptionFromSecret(normalizedURL, *secret) + keychain, err = registry.LoginOptionFromSecret(normalizedURL, *secret) if err != nil { e := &serror.Event{ Err: fmt.Errorf("failed to configure Helm client with secret data: %w", err), @@ -514,10 +506,8 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * // Requeue as content of secret might change return sreconcile.ResultEmpty, e } - - loginOpts = append([]helmreg.LoginOption{}, loginOpt) } else if repo.Spec.Provider != sourcev1.GenericOCIProvider && repo.Spec.Type == sourcev1.HelmRepositoryTypeOCI { - auth, authErr := oidcAuthFromAdapter(ctxTimeout, repo.Spec.URL, repo.Spec.Provider) + auth, authErr := oidcAuth(ctxTimeout, repo.Spec.URL, repo.Spec.Provider) if authErr != nil && !errors.Is(authErr, oci.ErrUnconfiguredProvider) { e := &serror.Event{ Err: fmt.Errorf("failed to get credential from %s: %w", repo.Spec.Provider, authErr), @@ -527,10 +517,20 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * return sreconcile.ResultEmpty, e } if auth != nil { - loginOpts = append([]helmreg.LoginOption{}, auth) + authenticator = auth } } + loginOpt, err := makeLoginOption(authenticator, keychain, normalizedURL) + if err != nil { + e := &serror.Event{ + Err: err, + Reason: sourcev1.AuthenticationFailedReason, + } + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) + return sreconcile.ResultEmpty, e + } + // Initialize the chart repository var chartRepo repository.Downloader switch repo.Spec.Type { @@ -544,7 +544,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * // this is needed because otherwise the credentials are stored in ~/.docker/config.json. // TODO@souleb: remove this once the registry move to Oras v2 // or rework to enable reusing credentials to avoid the unneccessary handshake operations - registryClient, credentialsFile, err := r.RegistryClientGenerator(loginOpts != nil) + registryClient, credentialsFile, err := r.RegistryClientGenerator(loginOpt != nil) if err != nil { e := &serror.Event{ Err: fmt.Errorf("failed to construct Helm client: %w", err), @@ -573,8 +573,8 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * // If login options are configured, use them to login to the registry // The OCIGetter will later retrieve the stored credentials to pull the chart - if loginOpts != nil { - err = ociChartRepo.Login(loginOpts...) + if keychain != nil { + err = ociChartRepo.Login(loginOpt) if err != nil { e := &serror.Event{ Err: fmt.Errorf("failed to login to OCI registry: %w", err), @@ -940,8 +940,9 @@ func (r *HelmChartReconciler) garbageCollect(ctx context.Context, obj *sourcev1. func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Context, name, namespace string) chart.GetChartDownloaderCallback { return func(url string) (repository.Downloader, error) { var ( - tlsConfig *tls.Config - loginOpts []helmreg.LoginOption + tlsConfig *tls.Config + authenticator authn.Authenticator + keychain authn.Keychain ) normalizedURL := repository.NormalizeURL(url) repo, err := r.resolveDependencyRepository(ctx, url, namespace) @@ -971,37 +972,39 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont if err != nil { return nil, err } - opts, err := getter.ClientOptionsFromSecret(*secret) + + // Build client options from secret + opts, tls, err := r.clientOptionsFromSecret(secret, normalizedURL) if err != nil { return nil, err } clientOpts = append(clientOpts, opts...) - - tlsConfig, err = getter.TLSClientConfigFromSecret(*secret, normalizedURL) - if err != nil { - return nil, fmt.Errorf("failed to create TLS client config for HelmRepository '%s': %w", repo.Name, err) - } + tlsConfig = tls // Build registryClient options from secret - loginOpt, err := registry.LoginOptionFromSecret(normalizedURL, *secret) + keychain, err = registry.LoginOptionFromSecret(normalizedURL, *secret) if err != nil { return nil, fmt.Errorf("failed to create login options for HelmRepository '%s': %w", repo.Name, err) } - loginOpts = append([]helmreg.LoginOption{}, loginOpt) } else if repo.Spec.Provider != sourcev1.GenericOCIProvider && repo.Spec.Type == sourcev1.HelmRepositoryTypeOCI { - auth, authErr := oidcAuthFromAdapter(ctxTimeout, repo.Spec.URL, repo.Spec.Provider) + auth, authErr := oidcAuth(ctxTimeout, repo.Spec.URL, repo.Spec.Provider) if authErr != nil && !errors.Is(authErr, oci.ErrUnconfiguredProvider) { return nil, fmt.Errorf("failed to get credential from %s: %w", repo.Spec.Provider, authErr) } if auth != nil { - loginOpts = append([]helmreg.LoginOption{}, auth) + authenticator = auth } } + loginOpt, err := makeLoginOption(authenticator, keychain, normalizedURL) + if err != nil { + return nil, err + } + var chartRepo repository.Downloader if helmreg.IsOCI(normalizedURL) { - registryClient, credentialsFile, err := r.RegistryClientGenerator(loginOpts != nil) + registryClient, credentialsFile, err := r.RegistryClientGenerator(loginOpt != nil) if err != nil { return nil, fmt.Errorf("failed to create registry client for HelmRepository '%s': %w", repo.Name, err) } @@ -1026,8 +1029,8 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont // If login options are configured, use them to login to the registry // The OCIGetter will later retrieve the stored credentials to pull the chart - if loginOpts != nil { - err = ociChartRepo.Login(loginOpts...) + if keychain != nil { + err = ociChartRepo.Login(loginOpt) if err != nil { errs = append(errs, fmt.Errorf("failed to login to OCI chart repository for HelmRepository '%s': %w", repo.Name, err)) // clean up the credentialsFile @@ -1077,6 +1080,20 @@ func (r *HelmChartReconciler) resolveDependencyRepository(ctx context.Context, u return nil, fmt.Errorf("no HelmRepository found for '%s' in '%s' namespace", url, namespace) } +func (r *HelmChartReconciler) clientOptionsFromSecret(secret *corev1.Secret, normalizedURL string) ([]helmgetter.Option, *tls.Config, error) { + opts, err := getter.ClientOptionsFromSecret(*secret) + if err != nil { + return nil, nil, fmt.Errorf("failed to configure Helm client with secret data: %w", err) + } + + tlsConfig, err := getter.TLSClientConfigFromSecret(*secret, normalizedURL) + if err != nil { + return nil, nil, fmt.Errorf("failed to create TLS client config with secret data: %w", err) + } + + return opts, tlsConfig, nil +} + func (r *HelmChartReconciler) getHelmRepositorySecret(ctx context.Context, repository *sourcev1.HelmRepository) (*corev1.Secret, error) { if repository.Spec.SecretRef == nil { return nil, nil diff --git a/controllers/helmrepository_controller_oci.go b/controllers/helmrepository_controller_oci.go index 5d60d2b1c..72bbf55a7 100644 --- a/controllers/helmrepository_controller_oci.go +++ b/controllers/helmrepository_controller_oci.go @@ -45,6 +45,7 @@ import ( helper "github.com/fluxcd/pkg/runtime/controller" "github.com/fluxcd/pkg/runtime/patch" "github.com/fluxcd/pkg/runtime/predicates" + "github.com/google/go-containerregistry/pkg/authn" "github.com/fluxcd/source-controller/api/v1beta2" sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" @@ -263,36 +264,21 @@ func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, obj *v1beta } conditions.Delete(obj, meta.StalledCondition) - var loginOpts []helmreg.LoginOption + var ( + authenticator authn.Authenticator + keychain authn.Keychain + err error + ) // Configure any authentication related options. if obj.Spec.SecretRef != nil { - // Attempt to retrieve secret. - name := types.NamespacedName{ - Namespace: obj.GetNamespace(), - Name: obj.Spec.SecretRef.Name, - } - var secret corev1.Secret - if err := r.Client.Get(ctx, name, &secret); err != nil { - e := fmt.Errorf("failed to get secret '%s': %w", name.String(), err) - conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, e.Error()) - result, retErr = ctrl.Result{}, e - return - } - - // Construct login options. - loginOpt, err := registry.LoginOptionFromSecret(obj.Spec.URL, secret) + keychain, err = authFromSecret(ctx, r.Client, obj) if err != nil { - e := fmt.Errorf("failed to configure Helm client with secret data: %w", err) - conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, e.Error()) - result, retErr = ctrl.Result{}, e + conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, err.Error()) + result, retErr = ctrl.Result{}, err return } - - if loginOpt != nil { - loginOpts = append(loginOpts, loginOpt) - } } else if obj.Spec.Provider != sourcev1.GenericOCIProvider && obj.Spec.Type == sourcev1.HelmRepositoryTypeOCI { - auth, authErr := oidcAuthFromAdapter(ctxTimeout, obj.Spec.URL, obj.Spec.Provider) + auth, authErr := oidcAuth(ctxTimeout, obj.Spec.URL, obj.Spec.Provider) if authErr != nil && !errors.Is(authErr, oci.ErrUnconfiguredProvider) { e := fmt.Errorf("failed to get credential from %s: %w", obj.Spec.Provider, authErr) conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, e.Error()) @@ -300,12 +286,19 @@ func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, obj *v1beta return } if auth != nil { - loginOpts = append(loginOpts, auth) + authenticator = auth } } + loginOpt, err := makeLoginOption(authenticator, keychain, obj.Spec.URL) + if err != nil { + conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, err.Error()) + result, retErr = ctrl.Result{}, err + return + } + // Create registry client and login if needed. - registryClient, file, err := r.RegistryClientGenerator(loginOpts != nil) + registryClient, file, err := r.RegistryClientGenerator(loginOpt != nil) if err != nil { e := fmt.Errorf("failed to create registry client: %w", err) conditions.MarkFalse(obj, meta.ReadyCondition, meta.FailedReason, e.Error()) @@ -332,8 +325,8 @@ func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, obj *v1beta conditions.Delete(obj, meta.StalledCondition) // Attempt to login to the registry if credentials are provided. - if loginOpts != nil { - err = chartRepo.Login(loginOpts...) + if loginOpt != nil { + err = chartRepo.Login(loginOpt) if err != nil { e := fmt.Errorf("failed to login to registry '%s': %w", obj.Spec.URL, err) conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, e.Error()) @@ -375,16 +368,37 @@ func (r *HelmRepositoryOCIReconciler) eventLogf(ctx context.Context, obj runtime r.Eventf(obj, eventType, reason, msg) } -// oidcAuthFromAdapter generates the OIDC credential authenticator based on the specified cloud provider. -func oidcAuthFromAdapter(ctx context.Context, url, provider string) (helmreg.LoginOption, error) { - auth, err := oidcAuth(ctx, url, provider) +// authFromSecret returns an authn.Keychain for the given HelmRepository. +// If the HelmRepository does not specify a secretRef, an anonymous keychain is returned. +func authFromSecret(ctx context.Context, client client.Client, obj *sourcev1.HelmRepository) (authn.Keychain, error) { + // Attempt to retrieve secret. + name := types.NamespacedName{ + Namespace: obj.GetNamespace(), + Name: obj.Spec.SecretRef.Name, + } + var secret corev1.Secret + if err := client.Get(ctx, name, &secret); err != nil { + return nil, fmt.Errorf("failed to get secret '%s': %w", name.String(), err) + } + + // Construct login options. + keychain, err := registry.LoginOptionFromSecret(obj.Spec.URL, secret) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to configure Helm client with secret data: %w", err) + } + return keychain, nil +} + +// makeLoginOption returns a registry login option for the given HelmRepository. +// If the HelmRepository does not specify a secretRef, a nil login option is returned. +func makeLoginOption(auth authn.Authenticator, keychain authn.Keychain, registryURL string) (helmreg.LoginOption, error) { + if auth != nil { + return registry.AuthAdaptHelper(auth) } - if auth == nil { - return nil, fmt.Errorf("could not validate OCI provider %s with URL %s", provider, url) + if keychain != nil { + return registry.KeychainAdaptHelper(keychain)(registryURL) } - return registry.OIDCAdaptHelper(auth) + return nil, nil } diff --git a/internal/helm/registry/auth.go b/internal/helm/registry/auth.go index e45d05172..c5d49998a 100644 --- a/internal/helm/registry/auth.go +++ b/internal/helm/registry/auth.go @@ -23,16 +23,31 @@ import ( "github.com/docker/cli/cli/config" "github.com/docker/cli/cli/config/credentials" + "github.com/fluxcd/source-controller/internal/oci" "github.com/google/go-containerregistry/pkg/authn" "helm.sh/helm/v3/pkg/registry" corev1 "k8s.io/api/core/v1" ) +// helper is a subset of the Docker credential helper credentials.Helper interface used by NewKeychainFromHelper. +type helper struct { + registry string + username, password string + err error +} + +func (h helper) Get(serverURL string) (string, string, error) { + if serverURL != h.registry { + return "", "", fmt.Errorf("unexpected serverURL: %s", serverURL) + } + return h.username, h.password, h.err +} + // LoginOptionFromSecret derives authentication data from a Secret to login to an OCI registry. This Secret // may either hold "username" and "password" fields or be of the corev1.SecretTypeDockerConfigJson type and hold // a corev1.DockerConfigJsonKey field with a complete Docker configuration. If both, "username" and "password" are // empty, a nil LoginOption and a nil error will be returned. -func LoginOptionFromSecret(registryURL string, secret corev1.Secret) (registry.LoginOption, error) { +func LoginOptionFromSecret(registryURL string, secret corev1.Secret) (authn.Keychain, error) { var username, password string if secret.Type == corev1.SecretTypeDockerConfigJson { dockerCfg, err := config.LoadFromReader(bytes.NewReader(secret.Data[corev1.DockerConfigJsonKey])) @@ -63,19 +78,34 @@ func LoginOptionFromSecret(registryURL string, secret corev1.Secret) (registry.L } switch { case username == "" && password == "": - return nil, nil + return oci.Anonymous{}, nil case username == "" || password == "": return nil, fmt.Errorf("invalid '%s' secret data: required fields 'username' and 'password'", secret.Name) } - return registry.LoginOptBasicAuth(username, password), nil + return authn.NewKeychainFromHelper(helper{registry: registryURL, username: username, password: password}), nil } -// OIDCAdaptHelper returns an ORAS credentials callback configured with the authorization data -// from the given authn authenticator. This allows for example to make use of credential helpers from +// KeyChainAdaptHelper returns an ORAS credentials callback configured with the authorization data +// from the given authn keychain. This allows for example to make use of credential helpers from // cloud providers. // Ref: https://github.com/google/go-containerregistry/tree/main/pkg/authn -func OIDCAdaptHelper(authenticator authn.Authenticator) (registry.LoginOption, error) { - authConfig, err := authenticator.Authorization() +func KeychainAdaptHelper(keyChain authn.Keychain) func(string) (registry.LoginOption, error) { + return func(registryURL string) (registry.LoginOption, error) { + authenticator, err := keyChain.Resolve(resource{registryURL}) + if err != nil { + return nil, fmt.Errorf("unable to resolve credentials for registry '%s': %w", registryURL, err) + } + + return AuthAdaptHelper(authenticator) + } +} + +// AuthAdaptHelper returns an ORAS credentials callback configured with the authorization data +// from the given authn authenticator This allows for example to make use of credential helpers from +// cloud providers. +// Ref: https://github.com/google/go-containerregistry/tree/main/pkg/authn +func AuthAdaptHelper(auth authn.Authenticator) (registry.LoginOption, error) { + authConfig, err := auth.Authorization() if err != nil { return nil, fmt.Errorf("unable to get authentication data from OIDC: %w", err) } @@ -91,3 +121,15 @@ func OIDCAdaptHelper(authenticator authn.Authenticator) (registry.LoginOption, e } return registry.LoginOptBasicAuth(username, password), nil } + +type resource struct { + registry string +} + +func (r resource) String() string { + return r.registry +} + +func (r resource) RegistryStr() string { + return r.registry +} diff --git a/internal/helm/registry/auth_test.go b/internal/helm/registry/auth_test.go index 58dbd04bf..30191ee89 100644 --- a/internal/helm/registry/auth_test.go +++ b/internal/helm/registry/auth_test.go @@ -24,6 +24,8 @@ import ( corev1 "k8s.io/api/core/v1" ) +const repoURL = "example.com" + func TestLoginOptionFromSecret(t *testing.T) { testURL := "oci://registry.example.com/foo/bar" testUser := "flux" @@ -131,33 +133,34 @@ func TestLoginOptionFromSecret(t *testing.T) { } } -func TestOIDCAdaptHelper(t *testing.T) { - auth := &authn.Basic{ - Username: "flux", - Password: "flux_password", +func TestKeychainAdaptHelper(t *testing.T) { + auth := helper{ + username: "flux", + password: "flux_password", + registry: repoURL, } tests := []struct { name string - auth authn.Authenticator + auth authn.Keychain expectedLogin bool wantErr bool }{ { name: "Login from basic auth with empty auth", - auth: &authn.Basic{}, + auth: authn.NewKeychainFromHelper(helper{}), expectedLogin: false, wantErr: false, }, { name: "Login from basic auth", - auth: auth, + auth: authn.NewKeychainFromHelper(auth), expectedLogin: true, wantErr: false, }, { name: "Login with missing password", - auth: &authn.Basic{Username: "flux"}, + auth: authn.NewKeychainFromHelper(helper{username: "flux", registry: repoURL}), expectedLogin: false, wantErr: true, }, @@ -166,7 +169,7 @@ func TestOIDCAdaptHelper(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { g := NewWithT(t) - loginOpt, err := OIDCAdaptHelper(tt.auth) + loginOpt, err := KeychainAdaptHelper(tt.auth)(repoURL) if tt.wantErr { g.Expect(err).To(HaveOccurred()) return diff --git a/internal/helm/repository/oci_chart_repository.go b/internal/helm/repository/oci_chart_repository.go index 417a52818..a037e6b40 100644 --- a/internal/helm/repository/oci_chart_repository.go +++ b/internal/helm/repository/oci_chart_repository.go @@ -215,6 +215,7 @@ func (r *OCIChartRepository) DownloadChart(chart *repo.ChartVersion) (*bytes.Buf // Login attempts to login to the OCI registry. // It returns an error on failure. func (r *OCIChartRepository) Login(opts ...registry.LoginOption) error { + // Get login credentials from keychain err := r.RegistryClient.Login(r.URL.Host, opts...) if err != nil { return err