From affea40b436a3e1bcddc53646399acaf75bae939 Mon Sep 17 00:00:00 2001 From: Somtochi Onyekwere Date: Mon, 7 Aug 2023 20:41:44 +0100 Subject: [PATCH 1/4] add .spec.serviceAccountName to helm repository for type oci Signed-off-by: Somtochi Onyekwere --- api/v1beta2/helmrepository_types.go | 6 +++ ...ce.toolkit.fluxcd.io_helmrepositories.yaml | 5 ++ internal/helm/getter/client_opts.go | 42 +++++++++++++++- internal/helm/getter/client_opts_test.go | 50 ++++++++++++++++--- 4 files changed, 96 insertions(+), 7 deletions(-) diff --git a/api/v1beta2/helmrepository_types.go b/api/v1beta2/helmrepository_types.go index 4dcf0a454..3bb09f176 100644 --- a/api/v1beta2/helmrepository_types.go +++ b/api/v1beta2/helmrepository_types.go @@ -104,6 +104,12 @@ type HelmRepositorySpec struct { // +optional Type string `json:"type,omitempty"` + // ServiceAccountName is the name of the Kubernetes ServiceAccount used to authenticate + // the OCI image pull if the service account has attached pull secrets. For more information: + // https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/#add-imagepullsecrets-to-a-service-account + // +optional + ServiceAccountName string `json:"serviceAccountName,omitempty"` + // Provider used for authentication, can be 'aws', 'azure', 'gcp' or 'generic'. // This field is optional, and only taken into account if the .spec.type field is set to 'oci'. // When not specified, defaults to 'generic'. diff --git a/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml b/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml index 8af5734be..23eae2378 100644 --- a/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml +++ b/config/crd/bases/source.toolkit.fluxcd.io_helmrepositories.yaml @@ -345,6 +345,11 @@ spec: required: - name type: object + serviceAccountName: + description: 'ServiceAccountName is the name of the Kubernetes ServiceAccount + used to authenticate the OCI image pull if the service account has + attached pull secrets. For more information: https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/#add-imagepullsecrets-to-a-service-account' + type: string suspend: description: Suspend tells the controller to suspend the reconciliation of this HelmRepository. diff --git a/internal/helm/getter/client_opts.go b/internal/helm/getter/client_opts.go index 2af928c8e..90101a264 100644 --- a/internal/helm/getter/client_opts.go +++ b/internal/helm/getter/client_opts.go @@ -26,6 +26,7 @@ import ( "github.com/fluxcd/pkg/oci" "github.com/google/go-containerregistry/pkg/authn" + "github.com/google/go-containerregistry/pkg/authn/k8schain" helmgetter "helm.sh/helm/v3/pkg/getter" helmreg "helm.sh/helm/v3/pkg/registry" corev1 "k8s.io/api/core/v1" @@ -80,6 +81,39 @@ func GetClientOpts(ctx context.Context, c client.Client, obj *helmv1.HelmReposit var authSecret *corev1.Secret var deprecatedTLSConfig bool + + if ociRepo { + if obj.Spec.ServiceAccountName != "" { + serviceAccount := corev1.ServiceAccount{} + // Lookup service account + if err := c.Get(ctx, types.NamespacedName{ + Namespace: obj.GetNamespace(), + Name: obj.Spec.ServiceAccountName, + }, &serviceAccount); err != nil { + return nil, fmt.Errorf("failed to get serviceaccout: %s", err) + } + + if len(serviceAccount.ImagePullSecrets) > 0 { + imagePullSecrets := make([]corev1.Secret, len(serviceAccount.ImagePullSecrets)) + for i, ips := range serviceAccount.ImagePullSecrets { + var saAuthSecret corev1.Secret + if err := c.Get(ctx, types.NamespacedName{ + Namespace: obj.GetNamespace(), + Name: ips.Name, + }, &saAuthSecret); err != nil { + return nil, fmt.Errorf("failed to get image pull secret '%s' for serviceaccount '%s': %w", + ips.Name, obj.Spec.ServiceAccountName, err) + } + imagePullSecrets[i] = saAuthSecret + } + hrOpts.Keychain, err = k8schain.NewFromPullSecrets(ctx, imagePullSecrets) + if err != nil { + return nil, fmt.Errorf("error constructing keychain from image pull secrets: %w", err) + } + } + } + } + if obj.Spec.SecretRef != nil { authSecret, err = fetchSecret(ctx, c, obj.Spec.SecretRef.Name, obj.GetNamespace()) if err != nil { @@ -107,10 +141,16 @@ func GetClientOpts(ctx context.Context, c client.Client, obj *helmv1.HelmReposit } if ociRepo { - hrOpts.Keychain, err = registry.LoginOptionFromSecret(url, *authSecret) + keychain, err := registry.LoginOptionFromSecret(url, *authSecret) if err != nil { return nil, fmt.Errorf("failed to configure login options: %w", err) } + + if hrOpts.Keychain != nil { + hrOpts.Keychain = authn.NewMultiKeychain(keychain, hrOpts.Keychain) + } else { + hrOpts.Keychain = keychain + } } } else if obj.Spec.Provider != helmv1.GenericOCIProvider && obj.Spec.Type == helmv1.HelmRepositoryTypeOCI && ociRepo { authenticator, authErr := soci.OIDCAuth(ctx, obj.Spec.URL, obj.Spec.Provider) diff --git a/internal/helm/getter/client_opts_test.go b/internal/helm/getter/client_opts_test.go index 2231e2a52..bdd255b1f 100644 --- a/internal/helm/getter/client_opts_test.go +++ b/internal/helm/getter/client_opts_test.go @@ -44,12 +44,13 @@ func TestGetClientOpts(t *testing.T) { } tests := []struct { - name string - certSecret *corev1.Secret - authSecret *corev1.Secret - afterFunc func(t *WithT, hcOpts *ClientOpts) - oci bool - err error + name string + certSecret *corev1.Secret + authSecret *corev1.Secret + serviceAccount *corev1.ServiceAccount + afterFunc func(t *WithT, hcOpts *ClientOpts) + oci bool + err error }{ { name: "HelmRepository with certSecretRef discards TLS config in secretRef", @@ -117,6 +118,39 @@ func TestGetClientOpts(t *testing.T) { }, oci: true, }, + { + name: "OCI HelmRepository with serviceaccount name", + serviceAccount: &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-sa", + }, + ImagePullSecrets: []corev1.LocalObjectReference{ + { + Name: "pull-secret", + }, + }, + }, + authSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pull-secret", + }, + Type: corev1.SecretTypeDockerConfigJson, + Data: map[string][]byte{ + corev1.DockerConfigJsonKey: []byte(`{"auths":{"ghcr.io":{"username":"user","password":"pass","auth":"dXNlcjpwYXNz"}}}`), + }, + }, + afterFunc: func(t *WithT, hcOpts *ClientOpts) { + repo, err := name.NewRepository("ghcr.io/dummy") + t.Expect(err).ToNot(HaveOccurred()) + authenticator, err := hcOpts.Keychain.Resolve(repo) + t.Expect(err).ToNot(HaveOccurred()) + config, err := authenticator.Authorization() + t.Expect(err).ToNot(HaveOccurred()) + t.Expect(config.Username).To(Equal("user")) + t.Expect(config.Password).To(Equal("pass")) + }, + oci: true, + }, } for _, tt := range tests { @@ -147,6 +181,10 @@ func TestGetClientOpts(t *testing.T) { Name: tt.certSecret.Name, } } + if tt.serviceAccount != nil { + clientBuilder.WithObjects(tt.serviceAccount.DeepCopy()) + helmRepo.Spec.ServiceAccountName = tt.serviceAccount.Name + } c := clientBuilder.Build() clientOpts, err := GetClientOpts(context.TODO(), c, helmRepo, "https://ghcr.io/dummy") From 2dfa0420182f2ef1aa164f466a5e3c18086f6627 Mon Sep 17 00:00:00 2001 From: Somtochi Onyekwere Date: Mon, 7 Aug 2023 21:23:50 +0100 Subject: [PATCH 2/4] get image pull secrets in different func Signed-off-by: Somtochi Onyekwere --- internal/helm/getter/client_opts.go | 61 +++++++++++++++++------------ 1 file changed, 35 insertions(+), 26 deletions(-) diff --git a/internal/helm/getter/client_opts.go b/internal/helm/getter/client_opts.go index 90101a264..993167605 100644 --- a/internal/helm/getter/client_opts.go +++ b/internal/helm/getter/client_opts.go @@ -84,32 +84,9 @@ func GetClientOpts(ctx context.Context, c client.Client, obj *helmv1.HelmReposit if ociRepo { if obj.Spec.ServiceAccountName != "" { - serviceAccount := corev1.ServiceAccount{} - // Lookup service account - if err := c.Get(ctx, types.NamespacedName{ - Namespace: obj.GetNamespace(), - Name: obj.Spec.ServiceAccountName, - }, &serviceAccount); err != nil { - return nil, fmt.Errorf("failed to get serviceaccout: %s", err) - } - - if len(serviceAccount.ImagePullSecrets) > 0 { - imagePullSecrets := make([]corev1.Secret, len(serviceAccount.ImagePullSecrets)) - for i, ips := range serviceAccount.ImagePullSecrets { - var saAuthSecret corev1.Secret - if err := c.Get(ctx, types.NamespacedName{ - Namespace: obj.GetNamespace(), - Name: ips.Name, - }, &saAuthSecret); err != nil { - return nil, fmt.Errorf("failed to get image pull secret '%s' for serviceaccount '%s': %w", - ips.Name, obj.Spec.ServiceAccountName, err) - } - imagePullSecrets[i] = saAuthSecret - } - hrOpts.Keychain, err = k8schain.NewFromPullSecrets(ctx, imagePullSecrets) - if err != nil { - return nil, fmt.Errorf("error constructing keychain from image pull secrets: %w", err) - } + hrOpts.Keychain, err = getKeychainFromSAImagePullSecrets(ctx, c, obj.GetNamespace(), obj.Spec.ServiceAccountName) + if err != nil { + return nil, fmt.Errorf("failed to get keychain from service account: %w", err) } } } @@ -234,3 +211,35 @@ func TLSClientConfigFromSecret(secret corev1.Secret, repositoryUrl string) (*tls return tlsConf, nil } + +// getKeychainFromSAImagePullSecrets returns an authn.Keychain gotten from the image pull secrets attached to a +// service account. +func getKeychainFromSAImagePullSecrets(ctx context.Context, c client.Client, ns, saName string) (authn.Keychain, error) { + serviceAccount := corev1.ServiceAccount{} + // Lookup service account + if err := c.Get(ctx, types.NamespacedName{ + Namespace: ns, + Name: saName, + }, &serviceAccount); err != nil { + return nil, fmt.Errorf("failed to get serviceaccout: %s", err) + } + + if len(serviceAccount.ImagePullSecrets) > 0 { + imagePullSecrets := make([]corev1.Secret, len(serviceAccount.ImagePullSecrets)) + for i, ips := range serviceAccount.ImagePullSecrets { + var saAuthSecret corev1.Secret + if err := c.Get(ctx, types.NamespacedName{ + Namespace: ns, + Name: ips.Name, + }, &saAuthSecret); err != nil { + return nil, fmt.Errorf("failed to get image pull secret '%s' for serviceaccount '%s': %w", + ips.Name, saName, err) + } + imagePullSecrets[i] = saAuthSecret + } + + return k8schain.NewFromPullSecrets(ctx, imagePullSecrets) + } + + return nil, nil +} From 6a2df7603b76ab8d3e7007bc38653c8ae23bc816 Mon Sep 17 00:00:00 2001 From: Somtochi Onyekwere Date: Tue, 8 Aug 2023 17:26:50 +0100 Subject: [PATCH 3/4] only use provider auth if sa and secretRef aren't set Signed-off-by: Somtochi Onyekwere --- internal/helm/getter/client_opts.go | 46 ++++++++++--------- internal/helm/getter/client_opts_test.go | 56 ++++++++++++++++++++---- 2 files changed, 74 insertions(+), 28 deletions(-) diff --git a/internal/helm/getter/client_opts.go b/internal/helm/getter/client_opts.go index 993167605..84f6159d0 100644 --- a/internal/helm/getter/client_opts.go +++ b/internal/helm/getter/client_opts.go @@ -24,7 +24,6 @@ import ( "fmt" "net/url" - "github.com/fluxcd/pkg/oci" "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/authn/k8schain" helmgetter "helm.sh/helm/v3/pkg/getter" @@ -33,6 +32,7 @@ import ( "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/fluxcd/pkg/oci" helmv1 "github.com/fluxcd/source-controller/api/v1beta2" "github.com/fluxcd/source-controller/internal/helm/registry" soci "github.com/fluxcd/source-controller/internal/oci" @@ -82,15 +82,6 @@ func GetClientOpts(ctx context.Context, c client.Client, obj *helmv1.HelmReposit var authSecret *corev1.Secret var deprecatedTLSConfig bool - if ociRepo { - if obj.Spec.ServiceAccountName != "" { - hrOpts.Keychain, err = getKeychainFromSAImagePullSecrets(ctx, c, obj.GetNamespace(), obj.Spec.ServiceAccountName) - if err != nil { - return nil, fmt.Errorf("failed to get keychain from service account: %w", err) - } - } - } - if obj.Spec.SecretRef != nil { authSecret, err = fetchSecret(ctx, c, obj.Spec.SecretRef.Name, obj.GetNamespace()) if err != nil { @@ -118,28 +109,43 @@ func GetClientOpts(ctx context.Context, c client.Client, obj *helmv1.HelmReposit } if ociRepo { - keychain, err := registry.LoginOptionFromSecret(url, *authSecret) + hrOpts.Keychain, err = registry.LoginOptionFromSecret(url, *authSecret) if err != nil { return nil, fmt.Errorf("failed to configure login options: %w", err) } + } + } + + if ociRepo { + if obj.Spec.ServiceAccountName != "" { + keychain, err := getKeychainFromSAImagePullSecrets(ctx, c, obj.GetNamespace(), obj.Spec.ServiceAccountName) + if err != nil { + return nil, fmt.Errorf("failed to get keychain from service account: %w", err) + } if hrOpts.Keychain != nil { - hrOpts.Keychain = authn.NewMultiKeychain(keychain, hrOpts.Keychain) + hrOpts.Keychain = authn.NewMultiKeychain(hrOpts.Keychain, keychain) } else { hrOpts.Keychain = keychain } } - } else if obj.Spec.Provider != helmv1.GenericOCIProvider && obj.Spec.Type == helmv1.HelmRepositoryTypeOCI && ociRepo { - authenticator, authErr := soci.OIDCAuth(ctx, obj.Spec.URL, obj.Spec.Provider) - if authErr != nil && !errors.Is(authErr, oci.ErrUnconfiguredProvider) { - return nil, fmt.Errorf("failed to get credential from '%s': %w", obj.Spec.Provider, authErr) + + var hasKeychain bool + if hrOpts.Keychain != nil { + _, ok := hrOpts.Keychain.(soci.Anonymous) + hasKeychain = !ok } - if authenticator != nil { - hrOpts.Authenticator = authenticator + + if !hasKeychain && obj.Spec.Provider != helmv1.GenericOCIProvider { + authenticator, authErr := soci.OIDCAuth(ctx, obj.Spec.URL, obj.Spec.Provider) + if authErr != nil && !errors.Is(authErr, oci.ErrUnconfiguredProvider) { + return nil, fmt.Errorf("failed to get credential from '%s': %w", obj.Spec.Provider, authErr) + } + if authenticator != nil { + hrOpts.Authenticator = authenticator + } } - } - if ociRepo { hrOpts.RegLoginOpt, err = registry.NewLoginOption(hrOpts.Authenticator, hrOpts.Keychain, url) if err != nil { return nil, err diff --git a/internal/helm/getter/client_opts_test.go b/internal/helm/getter/client_opts_test.go index bdd255b1f..3049aef39 100644 --- a/internal/helm/getter/client_opts_test.go +++ b/internal/helm/getter/client_opts_test.go @@ -44,13 +44,15 @@ func TestGetClientOpts(t *testing.T) { } tests := []struct { - name string - certSecret *corev1.Secret - authSecret *corev1.Secret - serviceAccount *corev1.ServiceAccount - afterFunc func(t *WithT, hcOpts *ClientOpts) - oci bool - err error + name string + certSecret *corev1.Secret + authSecret *corev1.Secret + imagePullSecret *corev1.Secret + serviceAccount *corev1.ServiceAccount + provider string + afterFunc func(t *WithT, hcOpts *ClientOpts) + oci bool + err error }{ { name: "HelmRepository with certSecretRef discards TLS config in secretRef", @@ -130,7 +132,41 @@ func TestGetClientOpts(t *testing.T) { }, }, }, - authSecret: &corev1.Secret{ + imagePullSecret: &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pull-secret", + }, + Type: corev1.SecretTypeDockerConfigJson, + Data: map[string][]byte{ + corev1.DockerConfigJsonKey: []byte(`{"auths":{"ghcr.io":{"username":"user","password":"pass","auth":"dXNlcjpwYXNz"}}}`), + }, + }, + afterFunc: func(t *WithT, hcOpts *ClientOpts) { + repo, err := name.NewRepository("ghcr.io/dummy") + t.Expect(err).ToNot(HaveOccurred()) + authenticator, err := hcOpts.Keychain.Resolve(repo) + t.Expect(err).ToNot(HaveOccurred()) + config, err := authenticator.Authorization() + t.Expect(err).ToNot(HaveOccurred()) + t.Expect(config.Username).To(Equal("user")) + t.Expect(config.Password).To(Equal("pass")) + }, + oci: true, + }, + { + name: "OCI HelmRepository with serviceaccount name and provider (serviceaccount takes precedence)", + provider: helmv1.AzureOCIProvider, + serviceAccount: &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-sa", + }, + ImagePullSecrets: []corev1.LocalObjectReference{ + { + Name: "pull-secret", + }, + }, + }, + imagePullSecret: &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "pull-secret", }, @@ -159,6 +195,7 @@ func TestGetClientOpts(t *testing.T) { helmRepo := &helmv1.HelmRepository{ Spec: helmv1.HelmRepositorySpec{ + Provider: tt.provider, Timeout: &metav1.Duration{ Duration: time.Second, }, @@ -181,6 +218,9 @@ func TestGetClientOpts(t *testing.T) { Name: tt.certSecret.Name, } } + if tt.imagePullSecret != nil { + clientBuilder.WithObjects(tt.imagePullSecret.DeepCopy()) + } if tt.serviceAccount != nil { clientBuilder.WithObjects(tt.serviceAccount.DeepCopy()) helmRepo.Spec.ServiceAccountName = tt.serviceAccount.Name From f555115f8f9129ab17b29a76c6d7a9ee9d6fc4cd Mon Sep 17 00:00:00 2001 From: Somtochi Onyekwere Date: Wed, 9 Aug 2023 18:03:41 +0100 Subject: [PATCH 4/4] add docs for new field Signed-off-by: Somtochi Onyekwere --- api/v1beta2/helmrepository_types.go | 1 + docs/spec/v1beta2/helmrepositories.md | 10 ++++++++++ 2 files changed, 11 insertions(+) diff --git a/api/v1beta2/helmrepository_types.go b/api/v1beta2/helmrepository_types.go index 3bb09f176..8fd797320 100644 --- a/api/v1beta2/helmrepository_types.go +++ b/api/v1beta2/helmrepository_types.go @@ -108,6 +108,7 @@ type HelmRepositorySpec struct { // the OCI image pull if the service account has attached pull secrets. For more information: // https://kubernetes.io/docs/tasks/configure-pod-container/configure-service-account/#add-imagepullsecrets-to-a-service-account // +optional + // This field is only considered for Helm Repositories of type oci ServiceAccountName string `json:"serviceAccountName,omitempty"` // Provider used for authentication, can be 'aws', 'azure', 'gcp' or 'generic'. diff --git a/docs/spec/v1beta2/helmrepositories.md b/docs/spec/v1beta2/helmrepositories.md index b48f4ff4a..3bca0afb5 100644 --- a/docs/spec/v1beta2/helmrepositories.md +++ b/docs/spec/v1beta2/helmrepositories.md @@ -509,6 +509,16 @@ data: caFile: ``` + +### Service Account Name + +*Note:* This field is only taken into account for Helm Repository of +type `oci`. + +`.spec.serviceAccountName` is an optional field to specify a name of a +ServiceAccount in the same namespace as the HelmRepository, which has image +pull secrets that can be used for authentication to the OCI image repository. + ### Pass credentials `.spec.passCredentials` is an optional field to allow the credentials from the