diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index 3f6b8504..73662da0 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" @@ -455,8 +456,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) @@ -481,10 +483,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()) @@ -492,20 +494,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), @@ -515,10 +507,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), @@ -528,10 +518,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 { @@ -545,7 +545,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), @@ -574,8 +574,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), @@ -941,8 +941,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) @@ -972,37 +973,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) } @@ -1027,8 +1030,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 @@ -1078,6 +1081,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 d42154d6..7e383e0c 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) } - - if auth == nil { - return nil, fmt.Errorf("could not validate OCI provider %s with URL %s", provider, url) - } - - return registry.OIDCAdaptHelper(auth) + 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 keychain != nil { + return registry.KeychainAdaptHelper(keychain)(registryURL) + } + + return nil, nil } diff --git a/internal/helm/registry/auth.go b/internal/helm/registry/auth.go index e45d0517..4914c568 100644 --- a/internal/helm/registry/auth.go +++ b/internal/helm/registry/auth.go @@ -23,27 +23,42 @@ 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 + parsedURL, err := url.Parse(registryURL) + if err != nil { + return nil, fmt.Errorf("unable to parse registry URL '%s' while reconciling Secret '%s': %w", + registryURL, secret.Name, err) + } if secret.Type == corev1.SecretTypeDockerConfigJson { dockerCfg, err := config.LoadFromReader(bytes.NewReader(secret.Data[corev1.DockerConfigJsonKey])) if err != nil { return nil, fmt.Errorf("unable to load Docker config from Secret '%s': %w", secret.Name, err) } - parsedURL, err := url.Parse(registryURL) - if err != nil { - return nil, fmt.Errorf("unable to parse registry URL '%s' while reconciling Secret '%s': %w", - registryURL, secret.Name, err) - } authConfig, err := dockerCfg.GetAuthConfig(parsedURL.Host) if err != nil { return nil, fmt.Errorf("unable to get authentication data from Secret '%s': %w", secret.Name, err) @@ -63,19 +78,38 @@ 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: parsedURL.Host, 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) { + parsedURL, err := url.Parse(registryURL) + if err != nil { + return nil, fmt.Errorf("unable to parse registry URL '%s'", registryURL) + } + authenticator, err := keyChain.Resolve(resource{parsedURL.Host}) + 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 +125,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 58dbd04b..14942a5b 100644 --- a/internal/helm/registry/auth_test.go +++ b/internal/helm/registry/auth_test.go @@ -17,6 +17,7 @@ limitations under the License. package registry import ( + "net/url" "testing" "github.com/google/go-containerregistry/pkg/authn" @@ -24,6 +25,8 @@ import ( corev1 "k8s.io/api/core/v1" ) +const repoURL = "https://example.com" + func TestLoginOptionFromSecret(t *testing.T) { testURL := "oci://registry.example.com/foo/bar" testUser := "flux" @@ -131,33 +134,40 @@ func TestLoginOptionFromSecret(t *testing.T) { } } -func TestOIDCAdaptHelper(t *testing.T) { - auth := &authn.Basic{ - Username: "flux", - Password: "flux_password", +func TestKeychainAdaptHelper(t *testing.T) { + g := NewWithT(t) + reg, err := url.Parse(repoURL) + if err != nil { + g.Expect(err).ToNot(HaveOccurred()) + } + + auth := helper{ + username: "flux", + password: "flux_password", + registry: reg.Host, } 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: reg.Host}), expectedLogin: false, wantErr: true, }, @@ -166,7 +176,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 417a5281..a037e6b4 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