diff --git a/go.mod b/go.mod index 630ece7bb..68ef2bd15 100644 --- a/go.mod +++ b/go.mod @@ -23,10 +23,11 @@ require ( github.com/docker/go-units v0.5.0 github.com/elazarl/goproxy v1.8.4 github.com/fluxcd/cli-utils v1.4.0 + github.com/fluxcd/pkg/apis/crypto v0.4.1-0.20261019184916-07ce7833ecaf github.com/fluxcd/pkg/apis/event v0.32.0 - github.com/fluxcd/pkg/apis/meta v1.35.0 + github.com/fluxcd/pkg/apis/meta v1.36.0 github.com/fluxcd/pkg/artifact v0.24.0 - github.com/fluxcd/pkg/auth v0.62.0 + github.com/fluxcd/pkg/auth v0.63.1-0.20261019184916-07ce7833ecaf github.com/fluxcd/pkg/cache v0.17.0 github.com/fluxcd/pkg/git v0.55.0 github.com/fluxcd/pkg/gittestserver v0.32.0 @@ -34,7 +35,7 @@ require ( github.com/fluxcd/pkg/http/transport v0.9.0 github.com/fluxcd/pkg/masktoken v0.10.0 github.com/fluxcd/pkg/oci v0.72.0 - github.com/fluxcd/pkg/runtime v0.119.0 + github.com/fluxcd/pkg/runtime v0.120.1-0.20261019184916-07ce7833ecaf github.com/fluxcd/pkg/sourceignore v0.21.0 github.com/fluxcd/pkg/ssh v0.28.0 github.com/fluxcd/pkg/tar v1.3.0 @@ -326,7 +327,7 @@ require ( github.com/spf13/cast v1.10.0 // indirect github.com/spf13/cobra v1.10.2 // indirect github.com/spf13/viper v1.21.0 // indirect - github.com/spiffe/go-spiffe/v2 v2.7.0 // indirect + github.com/spiffe/go-spiffe/v2 v2.9.0 // indirect github.com/subosito/gotenv v1.6.0 // indirect github.com/syndtr/goleveldb v1.0.1-0.20220721030215-126854af5e6d // indirect github.com/tetratelabs/wabin v0.0.0-20230304001439-f6f874872834 // indirect diff --git a/go.sum b/go.sum index d4cceb29c..d4e82475a 100644 --- a/go.sum +++ b/go.sum @@ -350,14 +350,16 @@ github.com/fluxcd/gitkit v0.6.0 h1:iNg5LTx6ePo+Pl0ZwqHTAkhbUHxGVSY3YCxCdw7VIFg= github.com/fluxcd/gitkit v0.6.0/go.mod h1:svOHuKi0fO9HoawdK4HfHAJJseZDHHjk7I3ihnCIqNo= github.com/fluxcd/pkg/apis/acl v0.12.0 h1:UWpZDqKxpKDW38wdXIMHV8Uv3POPu2vUrra0JH1frH0= github.com/fluxcd/pkg/apis/acl v0.12.0/go.mod h1:PJrGIu5Bw/6/siO95uhS+pi5oKSU+gVJqHqsPHb8Kok= +github.com/fluxcd/pkg/apis/crypto v0.4.1-0.20261019184916-07ce7833ecaf h1:hAkYvuN01e6JcSubaWz39lZs6OrgCxwVfGuswD8JuPY= +github.com/fluxcd/pkg/apis/crypto v0.4.1-0.20261019184916-07ce7833ecaf/go.mod h1:bEwkDgBjLXMVJ2BwmVbbj32PkirmNSicabgl2Jvw7Dk= github.com/fluxcd/pkg/apis/event v0.32.0 h1:3zyjdUspwQN+EQJPlUXwCvCMbeqtB+azlNmeenAcbuE= github.com/fluxcd/pkg/apis/event v0.32.0/go.mod h1:ALma8dTqo+NbI+z4BawSzt0pAPjetCSNNUSSS/9p0VU= -github.com/fluxcd/pkg/apis/meta v1.35.0 h1:/k5d7uEFrxkz/hf6JTrdsKBr3UQWVHW2J4G6VSSae/k= -github.com/fluxcd/pkg/apis/meta v1.35.0/go.mod h1:1MLvK2Xu3UdTKBzg+Q1Oj7H9PHeiKsUB1UW3uwxAwiM= +github.com/fluxcd/pkg/apis/meta v1.36.0 h1:sgT0N/tXPTLYPzYjO3WHt3LKaRBOlQIP+/dTrJRzvDQ= +github.com/fluxcd/pkg/apis/meta v1.36.0/go.mod h1:1MLvK2Xu3UdTKBzg+Q1Oj7H9PHeiKsUB1UW3uwxAwiM= github.com/fluxcd/pkg/artifact v0.24.0 h1:j/3JrNDThWRwtkkU9u2N7wOFkSVwZjnMYQWHNG51PEA= github.com/fluxcd/pkg/artifact v0.24.0/go.mod h1:D1VCIh95degPgcfOOfVmRDHRqJqgRb2Z+PCCVODcHws= -github.com/fluxcd/pkg/auth v0.62.0 h1:ZhrkfoLTaH5Q095sNGLZ47zLYw/AHw4nVnPAspGwT/c= -github.com/fluxcd/pkg/auth v0.62.0/go.mod h1:46lKYjDvx5Q7fsq09ZC1BxXJcstTOfm2X/NFLLFn7wA= +github.com/fluxcd/pkg/auth v0.63.1-0.20261019184916-07ce7833ecaf h1:+R6qBa2c9Nxi/6jkzdX0K5QUC7wCYCTHTq51GFZ0KsE= +github.com/fluxcd/pkg/auth v0.63.1-0.20261019184916-07ce7833ecaf/go.mod h1:PvYIN2/ZUbka62QGIXJ2LR4blhmmo0aCh3w9YA1vWGI= github.com/fluxcd/pkg/cache v0.17.0 h1:197ST7bSJD2Nw4G01y5cMjtWQpwQsyjpl2ox4MaGeU8= github.com/fluxcd/pkg/cache v0.17.0/go.mod h1:OW3fncHhPnPZFZav5FdEa//cyMHSndI0JS+IM69LXOU= github.com/fluxcd/pkg/git v0.55.0 h1:jMdW0uAYuzcwIL5rJq5pmDd+jSrgOe6eIOZfdrcAvO8= @@ -374,8 +376,8 @@ github.com/fluxcd/pkg/masktoken v0.10.0 h1:jY9tuBIS1T7z9F8nJc9ozT0QrI391Se9vMu0e github.com/fluxcd/pkg/masktoken v0.10.0/go.mod h1:zOCZp3trYmD5/csLOizvIGTi4gn76nnkTFu+vKOJufo= github.com/fluxcd/pkg/oci v0.72.0 h1:nUTGHTd/2qq2yT66anHfXiwE19uJl6VL36zm74iFwbU= github.com/fluxcd/pkg/oci v0.72.0/go.mod h1:5ngbB6SDvo6daD29HZtBbC6CcnHxuI+5JJR/YqtZhKM= -github.com/fluxcd/pkg/runtime v0.119.0 h1:cY2KpQ4WwpQKG+pFToslSIXUjKK8ckHQ7cvPUc/QHiY= -github.com/fluxcd/pkg/runtime v0.119.0/go.mod h1:7LJFbZPXwl++ATADx6bt63Jh9CverBoXQxbluxPwspw= +github.com/fluxcd/pkg/runtime v0.120.1-0.20261019184916-07ce7833ecaf h1:ceYaUQAHZjFEEbx0K1qPLxH9bbDxWMIHwhOip40rufo= +github.com/fluxcd/pkg/runtime v0.120.1-0.20261019184916-07ce7833ecaf/go.mod h1:Cw0CQ/tVKS9lgJoyKVouZr1qVnoJcYnSHw3QDzIOOjA= github.com/fluxcd/pkg/sourceignore v0.21.0 h1:p65j81RzXAeL4KJKgbkFRou+xiA0841CUdAsOCSAb5o= github.com/fluxcd/pkg/sourceignore v0.21.0/go.mod h1:N9NFuxbEMm+UPtP9t+Oyx5LEs0uHu7W+WVPmdlIbCeY= github.com/fluxcd/pkg/ssh v0.28.0 h1:DbqDtvJGKzrSfXfB8Hi9T2mbTnNBodijpRZRfxx59wE= @@ -903,8 +905,8 @@ github.com/spf13/pflag v1.0.10 h1:4EBh2KAYBwaONj6b2Ye1GiHfwjqyROoF4RwYO+vPwFk= github.com/spf13/pflag v1.0.10/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/spf13/viper v1.21.0 h1:x5S+0EU27Lbphp4UKm1C+1oQO+rKx36vfCoaVebLFSU= github.com/spf13/viper v1.21.0/go.mod h1:P0lhsswPGWD/1lZJ9ny3fYnVqxiegrlNrEmgLjbTCAY= -github.com/spiffe/go-spiffe/v2 v2.7.0 h1:uXe1MflJoHw58wAUvxVlcM7WpKtijWG7I1UidcGh6g4= -github.com/spiffe/go-spiffe/v2 v2.7.0/go.mod h1:47Q0Q9/AqGha8QLHp+kxpH4Wca7X7EnOtlIJy3mxZ3U= +github.com/spiffe/go-spiffe/v2 v2.9.0 h1:g5qTg3qIP1HXV+GzuyEfSJFVxUCpEP5Ui1OINVJHPB4= +github.com/spiffe/go-spiffe/v2 v2.9.0/go.mod h1:w2CLWKLMTX/PPYUEUPv3ltH0RXsw5S8suwNF46w9/Aw= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= diff --git a/internal/controller/bucket_controller.go b/internal/controller/bucket_controller.go index 8044875b8..b6553f0ec 100644 --- a/internal/controller/bucket_controller.go +++ b/internal/controller/bucket_controller.go @@ -41,6 +41,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" + cryptov1 "github.com/fluxcd/pkg/apis/crypto/v1" eventv1 "github.com/fluxcd/pkg/apis/event/v1" "github.com/fluxcd/pkg/apis/meta" intdigest "github.com/fluxcd/pkg/artifact/digest" @@ -132,6 +133,10 @@ type BucketReconciler struct { ControllerName string TokenCache *cache.TokenCache + DefaultServiceAccount string + + ObjectLevelWorkloadIdentity bool + patchOptions []patch.Option } @@ -435,7 +440,7 @@ func (r *BucketReconciler) reconcileStorage(ctx context.Context, sp *patch.Seria func (r *BucketReconciler) reconcileSource(ctx context.Context, sp *patch.SerialPatcher, obj *sourcev1.Bucket, index *index.Digester, dir string) (sreconcile.Result, error) { usesObjectLevelWorkloadIdentity := obj.Spec.Provider != "" && obj.Spec.Provider != sourcev1.BucketProviderGeneric && obj.Spec.ServiceAccountName != "" if usesObjectLevelWorkloadIdentity { - if !auth.IsObjectLevelWorkloadIdentityEnabled() { + if !r.ObjectLevelWorkloadIdentity { const gate = auth.FeatureGateObjectLevelWorkloadIdentity const msgFmt = "to use spec.serviceAccountName for provider authentication please enable the %s feature gate in the controller" err := fmt.Errorf(msgFmt, gate) @@ -839,35 +844,78 @@ func (r *BucketReconciler) setupCredentials(ctx context.Context, obj *sourcev1.B // createBucketProvider creates a provider-specific bucket client using the given credentials and configuration. // It handles different bucket providers (AWS, GCP, Azure, generic) and returns the appropriate client. func (r *BucketReconciler) createBucketProvider(ctx context.Context, obj *sourcev1.Bucket, creds *bucketCredentials) (BucketProvider, error) { - authOpts := []auth.Option{ - auth.WithClient(r.Client), - auth.WithServiceAccountNamespace(obj.GetNamespace()), + // Determine the cryptov1 service provider and resolve the ServiceAccount + // once, applying the default ServiceAccount when the object does not + // specify one. The generic provider does not support a ServiceAccount. + var serviceProvider string + switch obj.Spec.Provider { + case sourcev1.BucketProviderAmazon: + serviceProvider = cryptov1.ProviderAWS + case sourcev1.BucketProviderGoogle: + serviceProvider = cryptov1.ProviderGCP + case sourcev1.BucketProviderAzure: + serviceProvider = cryptov1.ProviderAzure } - if obj.Spec.ServiceAccountName != "" { - authOpts = append(authOpts, auth.WithServiceAccountName(obj.Spec.ServiceAccountName)) + var sa *corev1.ServiceAccount + if serviceProvider != "" { + saName := obj.Spec.ServiceAccountName + if saName == "" { + saName = r.DefaultServiceAccount + } + if saName != "" { + sa = &corev1.ServiceAccount{} + saRef := client.ObjectKey{Namespace: obj.GetNamespace(), Name: saName} + if err := r.Get(ctx, saRef, sa); err != nil { + return nil, fmt.Errorf("failed to get service account '%s': %w", saRef, err) + } + } } - if r.TokenCache != nil { - involvedObject := cache.InvolvedObject{ - Kind: sourcev1.BucketKind, - Name: obj.GetName(), - Namespace: obj.GetNamespace(), - Operation: cache.OperationReconcile, + var cryptoSpec *cryptov1.Spec + if sa != nil { + buildOpts := []cryptov1.Option{ + cryptov1.WithAnnotations(sa.Annotations), + cryptov1.WithServiceProvider(serviceProvider), + } + if sts := obj.Spec.STS; sts != nil { + buildOpts = append(buildOpts, cryptov1.WithSTSEndpoint(sts.Provider, sts.Endpoint)) + } + if obj.Spec.CertSecretRef != nil || + (obj.Spec.STS != nil && obj.Spec.STS.CertSecretRef != nil) { + buildOpts = append(buildOpts, cryptov1.WithCertSecretRef()) + } + var err error + cryptoSpec, err = cryptov1.BuildSpec(buildOpts...) + if err != nil { + return nil, fmt.Errorf("failed to build spec for service account '%s/%s': %w", + sa.Namespace, sa.Name, err) } - authOpts = append(authOpts, auth.WithCache(*r.TokenCache, involvedObject)) } + var authOpts []auth.Option + if cryptoSpec != nil { + authOpts = auth.OptionsFromSpec(cryptoSpec.Auth) + } + authOpts = append(authOpts, + auth.WithClient(r.Client), + auth.WithObject(sourcev1.GroupVersion.Group, sourcev1.BucketKind, obj)) + if sa != nil { + authOpts = append(authOpts, auth.WithServiceAccountName(sa.Name)) + } + if r.TokenCache != nil { + authOpts = append(authOpts, + auth.WithCache(*r.TokenCache), + auth.WithCacheOperation(cache.OperationReconcile)) + } if creds.proxyURL != nil { authOpts = append(authOpts, auth.WithProxyURL(*creds.proxyURL)) } - if obj.Spec.Region != "" { - authOpts = append(authOpts, auth.WithSTSRegion(obj.Spec.Region)) + authOpts = append(authOpts, auth.WithAWSRegion(obj.Spec.Region)) } - if sts := obj.Spec.STS; sts != nil { - authOpts = append(authOpts, auth.WithSTSEndpoint(sts.Endpoint)) + authOpts = append(authOpts, auth.WithAWSSTSEndpoint(sts.Endpoint)) } switch obj.Spec.Provider { diff --git a/internal/controller/bucket_controller_test.go b/internal/controller/bucket_controller_test.go index 494c53303..cff3da826 100644 --- a/internal/controller/bucket_controller_test.go +++ b/internal/controller/bucket_controller_test.go @@ -40,7 +40,6 @@ import ( "github.com/fluxcd/pkg/apis/meta" intdigest "github.com/fluxcd/pkg/artifact/digest" "github.com/fluxcd/pkg/artifact/storage" - "github.com/fluxcd/pkg/auth" "github.com/fluxcd/pkg/runtime/conditions" conditionscheck "github.com/fluxcd/pkg/runtime/conditions/check" "github.com/fluxcd/pkg/runtime/events" @@ -1438,16 +1437,11 @@ func TestBucketReconciler_reconcileSource_gcs(t *testing.T) { } r := &BucketReconciler{ - Recorder: events.NewFakeRecorder(32, false), - Client: clientBuilder.Build(), - Storage: testStorage, - patchOptions: getPatchOptions(bucketReadyCondition.Owned, "sc"), - } - - // Handle ObjectLevelWorkloadIdentity feature gate - if !tt.disableObjectLevelWorkloadIdentity { - auth.EnableObjectLevelWorkloadIdentity() - t.Cleanup(auth.DisableObjectLevelWorkloadIdentity) + Recorder: events.NewFakeRecorder(32, false), + Client: clientBuilder.Build(), + Storage: testStorage, + patchOptions: getPatchOptions(bucketReadyCondition.Owned, "sc"), + ObjectLevelWorkloadIdentity: !tt.disableObjectLevelWorkloadIdentity, } tmpDir := t.TempDir() diff --git a/internal/controller/gitrepository_controller.go b/internal/controller/gitrepository_controller.go index 8228037e3..cb7f4e733 100644 --- a/internal/controller/gitrepository_controller.go +++ b/internal/controller/gitrepository_controller.go @@ -49,6 +49,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/predicate" "sigs.k8s.io/controller-runtime/pkg/reconcile" + cryptov1 "github.com/fluxcd/pkg/apis/crypto/v1" eventv1 "github.com/fluxcd/pkg/apis/event/v1" "github.com/fluxcd/pkg/apis/meta" "github.com/fluxcd/pkg/artifact/storage" @@ -66,7 +67,6 @@ import ( sourcev1 "github.com/fluxcd/source-controller/api/v1" serror "github.com/fluxcd/source-controller/internal/error" - "github.com/fluxcd/source-controller/internal/features" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" "github.com/fluxcd/source-controller/internal/reconcile/summarize" "github.com/fluxcd/source-controller/internal/util" @@ -173,8 +173,11 @@ type GitRepositoryReconciler struct { ControllerName string TokenCache *cache.TokenCache + DefaultServiceAccount string + + ObjectLevelWorkloadIdentity bool + requeueDependency time.Duration - features map[string]bool patchOptions []patch.Option } @@ -193,10 +196,6 @@ func (r *GitRepositoryReconciler) SetupWithManager(mgr ctrl.Manager, opts GitRep r.requeueDependency = opts.DependencyRequeueInterval - if r.features == nil { - r.features = features.FeatureGates() - } - return ctrl.NewControllerManagedBy(mgr). For(&sourcev1.GitRepository{}, builder.WithPredicates( predicate.Or(predicate.GenerationChangedPredicate{}, predicates.ReconcileRequestedPredicate{}), @@ -707,45 +706,92 @@ func (r *GitRepositoryReconciler) getAuthOpts(ctx context.Context, obj *sourcev1 } // Configure provider authentication if specified. + provider := obj.GetProvider() + withCerts := len(opts.ClientCert) > 0 || len(opts.ClientKey) > 0 || len(opts.CAFile) > 0 + + // Check the object-level workload identity feature gate for the providers + // that use the ServiceAccount for authentication. + switch provider { + case sourcev1.GitProviderAzure, sourcev1.GitProviderAWS: + if obj.Spec.ServiceAccountName != "" && !r.ObjectLevelWorkloadIdentity { + const gate = auth.FeatureGateObjectLevelWorkloadIdentity + const msgFmt = "to use spec.serviceAccountName for provider authentication please enable the %s feature gate in the controller" + err := serror.NewStalling(fmt.Errorf(msgFmt, gate), meta.FeatureGateDisabledReason) + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, meta.FeatureGateDisabledReason, "%s", err) + return nil, err + } + } + + // Resolve the ServiceAccount once, applying the default ServiceAccount when + // the object does not specify one. + saName := obj.Spec.ServiceAccountName + if saName == "" { + saName = r.DefaultServiceAccount + } + var sa *corev1.ServiceAccount + if saName != "" { + sa = &corev1.ServiceAccount{} + saRef := types.NamespacedName{Namespace: obj.GetNamespace(), Name: saName} + if err := r.Get(ctx, saRef, sa); err != nil { + e := serror.NewGeneric( + fmt.Errorf("failed to get service account '%s': %w", saRef, err), + sourcev1.AuthenticationFailedReason, + ) + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e) + return nil, e + } + } + + // Build the authentication spec. This is done even when no + // ServiceAccount is specified, as some inputs are validated regardless. + var cryptoSpec *cryptov1.Spec + if sa != nil || (provider != "" && provider != sourcev1.GitProviderGeneric) { + buildOpts := []cryptov1.Option{ + cryptov1.WithServiceProvider(provider), + } + if sa != nil { + buildOpts = append(buildOpts, cryptov1.WithAnnotations(sa.Annotations)) + } + if withCerts { + buildOpts = append(buildOpts, cryptov1.WithSecretRefCerts()) + } + if provider == sourcev1.GitProviderGitHub && len(authData[githubapp.KeyAppBaseURL]) > 0 { + buildOpts = append(buildOpts, cryptov1.WithGitHubEnterpriseServer()) + } + var err error + cryptoSpec, err = cryptov1.BuildSpec(buildOpts...) + if err != nil { + e := serror.NewGeneric( + fmt.Errorf("failed to validate configuration: %w", err), + sourcev1.AuthenticationFailedReason, + ) + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e) + return nil, e + } + } + var getCreds func() (*auth.GitCredentials, error) - switch provider := obj.GetProvider(); provider { + switch provider { // If other providers (GCP, etc.) are added in the future they can be added here separated by a comma. case sourcev1.GitProviderAzure, sourcev1.GitProviderAWS: getCreds = func() (*auth.GitCredentials, error) { - opts := []auth.Option{ + authOpts := auth.OptionsFromSpec(cryptoSpec.Auth) + authOpts = append(authOpts, auth.WithClient(r.Client), - auth.WithServiceAccountNamespace(obj.GetNamespace()), auth.WithGitURL(u), + auth.WithObject(sourcev1.GroupVersion.Group, sourcev1.GitRepositoryKind, obj)) + if sa != nil { + authOpts = append(authOpts, auth.WithServiceAccountName(sa.Name)) } - - if obj.Spec.ServiceAccountName != "" { - // Check object-level workload identity feature gate. - if !auth.IsObjectLevelWorkloadIdentityEnabled() { - const gate = auth.FeatureGateObjectLevelWorkloadIdentity - const msgFmt = "to use spec.serviceAccountName for provider authentication please enable the %s feature gate in the controller" - err := serror.NewStalling(fmt.Errorf(msgFmt, gate), meta.FeatureGateDisabledReason) - conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, meta.FeatureGateDisabledReason, "%s", err) - return nil, err - } - // Set ServiceAccountName only if explicitly specified - opts = append(opts, auth.WithServiceAccountName(obj.Spec.ServiceAccountName)) - } - if r.TokenCache != nil { - involvedObject := cache.InvolvedObject{ - Kind: sourcev1.GitRepositoryKind, - Name: obj.GetName(), - Namespace: obj.GetNamespace(), - Operation: cache.OperationReconcile, - } - opts = append(opts, auth.WithCache(*r.TokenCache, involvedObject)) + authOpts = append(authOpts, + auth.WithCache(*r.TokenCache), + auth.WithCacheOperation(cache.OperationReconcile)) } - if proxyURL != nil { - opts = append(opts, auth.WithProxyURL(*proxyURL)) + authOpts = append(authOpts, auth.WithProxyURL(*proxyURL)) } - - return authutils.GetGitCredentials(ctx, provider, opts...) + return authutils.GetGitCredentials(ctx, provider, authOpts...) } case sourcev1.GitProviderGitHub: // if provider is github, but secret ref is not specified diff --git a/internal/controller/gitrepository_controller_test.go b/internal/controller/gitrepository_controller_test.go index 4a18c8e37..252c3b919 100644 --- a/internal/controller/gitrepository_controller_test.go +++ b/internal/controller/gitrepository_controller_test.go @@ -62,7 +62,6 @@ import ( sourcev1 "github.com/fluxcd/source-controller/api/v1" serror "github.com/fluxcd/source-controller/internal/error" - "github.com/fluxcd/source-controller/internal/features" sreconcile "github.com/fluxcd/source-controller/internal/reconcile" "github.com/fluxcd/source-controller/internal/reconcile/summarize" ) @@ -1103,10 +1102,10 @@ func TestGitRepositoryReconciler_getAuthOpts_provider(t *testing.T) { obj := &sourcev1.GitRepository{} r := &GitRepositoryReconciler{ - Recorder: events.NewFakeRecorder(32, false), - Client: clientBuilder.Build(), - features: features.FeatureGates(), - patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"), + Recorder: events.NewFakeRecorder(32, false), + Client: clientBuilder.Build(), + ObjectLevelWorkloadIdentity: false, + patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"), } url, err := url.Parse(tt.url) @@ -1528,10 +1527,10 @@ func TestGitRepositoryReconciler_reconcileArtifact(t *testing.T) { resetChmod(tt.dir, 0o750, 0o600) r := &GitRepositoryReconciler{ - Recorder: events.NewFakeRecorder(32, false), - Storage: testStorage, - features: features.FeatureGates(), - patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"), + Recorder: events.NewFakeRecorder(32, false), + Storage: testStorage, + ObjectLevelWorkloadIdentity: true, + patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"), } obj := &sourcev1.GitRepository{ @@ -1678,12 +1677,12 @@ func TestGitRepositoryReconciler_reconcileInclude(t *testing.T) { } r := &GitRepositoryReconciler{ - Client: clientBuilder.Build(), - Recorder: events.NewFakeRecorder(32, false), - Storage: storage, - requeueDependency: dependencyInterval, - features: features.FeatureGates(), - patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"), + Client: clientBuilder.Build(), + Recorder: events.NewFakeRecorder(32, false), + Storage: storage, + requeueDependency: dependencyInterval, + ObjectLevelWorkloadIdentity: true, + patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"), } obj := &sourcev1.GitRepository{ @@ -1936,10 +1935,10 @@ func TestGitRepositoryReconciler_reconcileStorage(t *testing.T) { WithScheme(testEnv.GetScheme()). WithStatusSubresource(&sourcev1.GitRepository{}). Build(), - Recorder: events.NewFakeRecorder(32, false), - Storage: testStorage, - features: features.FeatureGates(), - patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"), + Recorder: events.NewFakeRecorder(32, false), + Storage: testStorage, + ObjectLevelWorkloadIdentity: true, + patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"), } obj := &sourcev1.GitRepository{ @@ -1990,10 +1989,10 @@ func TestGitRepositoryReconciler_reconcileDelete(t *testing.T) { g := NewWithT(t) r := &GitRepositoryReconciler{ - Recorder: events.NewFakeRecorder(32, false), - Storage: testStorage, - features: features.FeatureGates(), - patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"), + Recorder: events.NewFakeRecorder(32, false), + Storage: testStorage, + ObjectLevelWorkloadIdentity: true, + patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"), } obj := &sourcev1.GitRepository{ @@ -2782,10 +2781,10 @@ func TestGitRepositoryReconciler_verifySignature(t *testing.T) { } r := &GitRepositoryReconciler{ - Recorder: events.NewFakeRecorder(32, false), - Client: clientBuilder.Build(), - features: features.FeatureGates(), - patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"), + Recorder: events.NewFakeRecorder(32, false), + Client: clientBuilder.Build(), + ObjectLevelWorkloadIdentity: true, + patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"), } obj := &sourcev1.GitRepository{ @@ -2938,11 +2937,11 @@ func TestGitRepositoryReconciler_ConditionsUpdate(t *testing.T) { WithStatusSubresource(&sourcev1.GitRepository{}) r := &GitRepositoryReconciler{ - Client: clientBuilder.Build(), - Recorder: events.NewFakeRecorder(32, false), - Storage: testStorage, - features: features.FeatureGates(), - patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"), + Client: clientBuilder.Build(), + Recorder: events.NewFakeRecorder(32, false), + Storage: testStorage, + ObjectLevelWorkloadIdentity: true, + patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"), } key := client.ObjectKeyFromObject(obj) @@ -3346,9 +3345,9 @@ func TestGitRepositoryReconciler_notify(t *testing.T) { } reconciler := &GitRepositoryReconciler{ - Recorder: recorder, - features: features.FeatureGates(), - patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"), + Recorder: recorder, + ObjectLevelWorkloadIdentity: true, + patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"), } reconciler.notify(ctx, oldObj, newObj, tt.commit, tt.res, tt.resErr) diff --git a/internal/controller/ocirepository_controller.go b/internal/controller/ocirepository_controller.go index cbd4f7777..2df88a7d5 100644 --- a/internal/controller/ocirepository_controller.go +++ b/internal/controller/ocirepository_controller.go @@ -47,6 +47,7 @@ import ( "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/reconcile" + cryptov1 "github.com/fluxcd/pkg/apis/crypto/v1" eventv1 "github.com/fluxcd/pkg/apis/event/v1" "github.com/fluxcd/pkg/apis/meta" "github.com/fluxcd/pkg/artifact/storage" @@ -144,7 +145,11 @@ type OCIRepositoryReconciler struct { ControllerName string TokenCache *cache.TokenCache CosignVerifierFactory *scosign.CosignVerifierFactory - requeueDependency time.Duration + DefaultServiceAccount string + + ObjectLevelWorkloadIdentity bool + + requeueDependency time.Duration patchOptions []patch.Option } @@ -340,8 +345,65 @@ func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch conditions.Delete(obj, sourcev1.SourceVerifiedCondition) } - // Generate the registry credential keychain either from static credentials or using cloud OIDC - keychain, err := r.keychain(ctx, obj) + // Apply the default ServiceAccount when the object does not specify one, so + // that both the keychain and the auth library are aware of it. + saName := obj.Spec.ServiceAccountName + if saName == "" { + saName = r.DefaultServiceAccount + } + + // Fetch the ServiceAccount once. + var sa *corev1.ServiceAccount + if saName != "" { + sa = &corev1.ServiceAccount{} + saRef := types.NamespacedName{Namespace: obj.GetNamespace(), Name: saName} + if err := r.Get(ctx, saRef, sa); err != nil { + e := serror.NewGeneric( + fmt.Errorf("failed to get service account '%s': %w", saRef, err), + sourcev1.AuthenticationFailedReason, + ) + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e) + return sreconcile.ResultEmpty, e + } + } + + // Build the authentication spec. This is done even when no + // ServiceAccount is specified, as some inputs are validated regardless. + var cryptoSpec *cryptov1.Spec + if sa != nil || (obj.Spec.Provider != "" && obj.Spec.Provider != sourcev1.GenericOCIProvider) { + buildOpts := []cryptov1.Option{ + cryptov1.WithServiceProvider(obj.Spec.Provider), + cryptov1.WithOCI(), + } + if sa != nil { + buildOpts = append(buildOpts, cryptov1.WithAnnotations(sa.Annotations)) + if len(sa.ImagePullSecrets) > 0 { + buildOpts = append(buildOpts, cryptov1.WithImagePullSecrets()) + } + } + if obj.Spec.CertSecretRef != nil { + buildOpts = append(buildOpts, cryptov1.WithCertSecretRef()) + } + var err error + cryptoSpec, err = cryptov1.BuildSpec(buildOpts...) + if err != nil { + e := serror.NewGeneric( + fmt.Errorf("failed to validate configuration: %w", err), + sourcev1.AuthenticationFailedReason, + ) + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, "%s", e) + return sreconcile.ResultEmpty, e + } + } + + // Generate the registry credential keychain either from static credentials + // or using cloud OIDC. The ServiceAccount is only used for its image pull + // secrets when it has any. + var keychainSA *corev1.ServiceAccount + if sa != nil && len(sa.ImagePullSecrets) > 0 { + keychainSA = sa + } + keychain, err := r.keychain(ctx, obj, keychainSA) if err != nil { e := serror.NewGeneric( fmt.Errorf("failed to get credential: %w", err), @@ -369,30 +431,25 @@ func (r *OCIRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch } if _, ok := keychain.(soci.Anonymous); obj.Spec.Provider != "" && obj.Spec.Provider != sourcev1.GenericOCIProvider && ok { - opts := []auth.Option{ - auth.WithClient(r.Client), - auth.WithServiceAccountNamespace(obj.GetNamespace()), + // Check the object-level workload identity feature gate. + if sa != nil && !r.ObjectLevelWorkloadIdentity { + const gate = auth.FeatureGateObjectLevelWorkloadIdentity + const msgFmt = "to use spec.serviceAccountName for provider authentication please enable the %s feature gate in the controller" + err := fmt.Errorf(msgFmt, gate) + return sreconcile.ResultEmpty, serror.NewStalling(err, meta.FeatureGateDisabledReason) } - if obj.Spec.ServiceAccountName != "" { - // Check object-level workload identity feature gate. - if !auth.IsObjectLevelWorkloadIdentityEnabled() { - const gate = auth.FeatureGateObjectLevelWorkloadIdentity - const msgFmt = "to use spec.serviceAccountName for provider authentication please enable the %s feature gate in the controller" - err := fmt.Errorf(msgFmt, gate) - return sreconcile.ResultEmpty, serror.NewStalling(err, meta.FeatureGateDisabledReason) - } - // Set ServiceAccountName only if explicitly specified - opts = append(opts, auth.WithServiceAccountName(obj.Spec.ServiceAccountName)) + opts := auth.OptionsFromSpec(cryptoSpec.Auth) + opts = append(opts, + auth.WithClient(r.Client), + auth.WithObject(sourcev1.GroupVersion.Group, sourcev1.OCIRepositoryKind, obj)) + if sa != nil { + opts = append(opts, auth.WithServiceAccountName(sa.Name)) } if r.TokenCache != nil { - involvedObject := cache.InvolvedObject{ - Kind: sourcev1.OCIRepositoryKind, - Name: obj.GetName(), - Namespace: obj.GetNamespace(), - Operation: cache.OperationReconcile, - } - opts = append(opts, auth.WithCache(*r.TokenCache, involvedObject)) + opts = append(opts, + auth.WithCache(*r.TokenCache), + auth.WithCacheOperation(cache.OperationReconcile)) } if proxyURL != nil { opts = append(opts, auth.WithProxyURL(*proxyURL)) @@ -1079,8 +1136,11 @@ func convertOCIToSemVerTag(ociTag, mediaType string) string { // keychain generates the credential keychain based on the resource // configuration. If no auth is specified a default keychain with -// anonymous access is returned -func (r *OCIRepositoryReconciler) keychain(ctx context.Context, obj *sourcev1.OCIRepository) (authn.Keychain, error) { +// anonymous access is returned. +// +// The ServiceAccount is only used for its image pull secrets and must be +// resolved and validated by the caller beforehand. +func (r *OCIRepositoryReconciler) keychain(ctx context.Context, obj *sourcev1.OCIRepository, sa *corev1.ServiceAccount) (authn.Keychain, error) { var imagePullSecrets []corev1.Secret // lookup auth secret @@ -1096,10 +1156,9 @@ func (r *OCIRepositoryReconciler) keychain(ctx context.Context, obj *sourcev1.OC imagePullSecrets = append(imagePullSecrets, imagePullSecret) } - // lookup service account - if obj.Spec.ServiceAccountName != "" { - saRef := types.NamespacedName{Namespace: obj.Namespace, Name: obj.Spec.ServiceAccountName} - saSecrets, err := secrets.PullSecretsFromServiceAccountRef(ctx, r.Client, saRef) + // lookup service account image pull secrets + if sa != nil { + saSecrets, err := secrets.PullSecretsFromServiceAccount(ctx, r.Client, sa) if err != nil { return nil, err } diff --git a/internal/controller/ocirepository_controller_test.go b/internal/controller/ocirepository_controller_test.go index c3cfb2712..b8e51c8ca 100644 --- a/internal/controller/ocirepository_controller_test.go +++ b/internal/controller/ocirepository_controller_test.go @@ -64,7 +64,6 @@ import ( "github.com/fluxcd/pkg/apis/meta" intdigest "github.com/fluxcd/pkg/artifact/digest" "github.com/fluxcd/pkg/artifact/storage" - "github.com/fluxcd/pkg/auth" "github.com/fluxcd/pkg/git" "github.com/fluxcd/pkg/oci" "github.com/fluxcd/pkg/runtime/conditions" @@ -1681,7 +1680,7 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignatureNotation(t *testi obj.Spec.Reference.Digest = podinfoVersions[tt.reference.Tag].digest.String() } - keychain, err := r.keychain(ctx, obj) + keychain, err := r.keychain(ctx, obj, nil) if err != nil { g.Expect(err).ToNot(HaveOccurred()) } @@ -2028,7 +2027,7 @@ func TestOCIRepository_reconcileSource_verifyOCISourceTrustPolicyNotation(t *tes obj.Spec.Reference.Digest = podinfoVersions[tt.reference.Tag].digest.String() } - keychain, err := r.keychain(ctx, obj) + keychain, err := r.keychain(ctx, obj, nil) if err != nil { g.Expect(err).ToNot(HaveOccurred()) } @@ -2337,7 +2336,7 @@ func TestOCIRepository_reconcileSource_verifyOCISourceSignatureCosign(t *testing podinfoVersions, err := pushMultiplePodinfoImages(server.registryHost, tt.insecure, tt.reference.Tag) g.Expect(err).ToNot(HaveOccurred()) - keychain, err := r.keychain(ctx, obj) + keychain, err := r.keychain(ctx, obj, nil) if err != nil { g.Expect(err).ToNot(HaveOccurred()) } @@ -3231,8 +3230,8 @@ func TestOCIRepository_objectLevelWorkloadIdentityFeatureGate(t *testing.T) { g.Expect(stalledCondition.Reason).Should(Equal(meta.FeatureGateDisabledReason)) g.Expect(stalledCondition.Message).Should(Equal("to use spec.serviceAccountName for provider authentication please enable the ObjectLevelWorkloadIdentity feature gate in the controller")) - auth.EnableObjectLevelWorkloadIdentity() - t.Cleanup(auth.DisableObjectLevelWorkloadIdentity) + testOCIRepositoryReconciler.ObjectLevelWorkloadIdentity = true + t.Cleanup(func() { testOCIRepositoryReconciler.ObjectLevelWorkloadIdentity = false }) g.Eventually(func() bool { if err := testEnv.Get(ctx, key, resultobj); err != nil { diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 91d999c60..8023923fb 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -87,6 +87,8 @@ var ( testServer *testserver.ArtifactServer testMetricsH controller.Metrics ctx = ctrl.SetupSignalHandler() + + testOCIRepositoryReconciler *OCIRepositoryReconciler ) var ( @@ -345,12 +347,13 @@ func TestMain(m *testing.M) { testCache = cache.New(5, 1*time.Second) cacheRecorder := cache.MustMakeMetrics() - if err := (&OCIRepositoryReconciler{ + testOCIRepositoryReconciler = &OCIRepositoryReconciler{ Client: testEnv, Recorder: events.NewFakeRecorder(32, false), Metrics: testMetricsH, Storage: testStorage, - }).SetupWithManager(testEnv, OCIRepositoryReconcilerOptions{ + } + if err := testOCIRepositoryReconciler.SetupWithManager(testEnv, OCIRepositoryReconcilerOptions{ RateLimiter: controller.GetDefaultRateLimiter(), }); err != nil { panic(fmt.Sprintf("Failed to start OCIRepositoryReconciler: %v", err)) diff --git a/internal/features/features.go b/internal/features/features.go index edb9beb17..661d3eaa1 100644 --- a/internal/features/features.go +++ b/internal/features/features.go @@ -36,16 +36,20 @@ var features = map[string]bool{ // CacheSecretsAndConfigMaps // opt-in from v0.34 CacheSecretsAndConfigMaps: false, + + // ObjectLevelWorkloadIdentity + // opt-in from Flux v2.6. + auth.FeatureGateObjectLevelWorkloadIdentity: false, } -func init() { - auth.SetFeatureGates(features) +var retiredFeatures = map[string]string{ + // TODO: retire ObjectLevelWorkloadIdentity in Flux 2.11. } // FeatureGates contains a list of all supported feature gates and // their default values. -func FeatureGates() map[string]bool { - return features +func FeatureGates() (map[string]bool, map[string]string) { + return features, retiredFeatures } // Enabled verifies whether the feature is enabled or not. diff --git a/main.go b/main.go index 3fbef9181..8f5e76c46 100644 --- a/main.go +++ b/main.go @@ -17,6 +17,7 @@ limitations under the License. package main import ( + "errors" "fmt" "os" "time" @@ -159,25 +160,19 @@ func main() { logger.SetLogger(logger.NewLogger(logOptions)) - if defaultServiceAccount != "" { - auth.SetDefaultServiceAccount(defaultServiceAccount) - } - if err := featureGates.WithLogger(setupLog).SupportedFeatures(features.FeatureGates()); err != nil { setupLog.Error(err, "unable to load feature gates") os.Exit(1) } - switch enabled, err := features.Enabled(auth.FeatureGateObjectLevelWorkloadIdentity); { - case err != nil: + objectLevelWorkloadIdentity, err := features.Enabled(auth.FeatureGateObjectLevelWorkloadIdentity) + if err != nil { setupLog.Error(err, "unable to check feature gate "+auth.FeatureGateObjectLevelWorkloadIdentity) os.Exit(1) - case enabled: - auth.EnableObjectLevelWorkloadIdentity() } - - if auth.InconsistentObjectLevelConfiguration() { - setupLog.Error(auth.ErrInconsistentObjectLevelConfiguration, "invalid configuration") + if defaultServiceAccount != "" && !objectLevelWorkloadIdentity { + setupLog.Error(errors.New("cannot set default service accounts when the "+ + auth.FeatureGateObjectLevelWorkloadIdentity+" feature gate is not enabled"), "invalid configuration") os.Exit(1) } @@ -233,12 +228,14 @@ func main() { ctx := ctrl.SetupSignalHandler() if err := (&controller.GitRepositoryReconciler{ - Client: mgr.GetClient(), - Recorder: eventRecorder, - Metrics: metrics, - Storage: storage, - ControllerName: controllerName, - TokenCache: tokenCache, + Client: mgr.GetClient(), + Recorder: eventRecorder, + Metrics: metrics, + Storage: storage, + ControllerName: controllerName, + TokenCache: tokenCache, + DefaultServiceAccount: defaultServiceAccount, + ObjectLevelWorkloadIdentity: objectLevelWorkloadIdentity, }).SetupWithManager(mgr, controller.GitRepositoryReconcilerOptions{ DependencyRequeueInterval: requeueDependency, RateLimiter: helper.GetRateLimiter(rateLimiterOptions), @@ -283,12 +280,14 @@ func main() { } if err := (&controller.BucketReconciler{ - Client: mgr.GetClient(), - Recorder: eventRecorder, - Metrics: metrics, - Storage: storage, - ControllerName: controllerName, - TokenCache: tokenCache, + Client: mgr.GetClient(), + Recorder: eventRecorder, + Metrics: metrics, + Storage: storage, + ControllerName: controllerName, + TokenCache: tokenCache, + DefaultServiceAccount: defaultServiceAccount, + ObjectLevelWorkloadIdentity: objectLevelWorkloadIdentity, }).SetupWithManager(mgr, controller.BucketReconcilerOptions{ RateLimiter: helper.GetRateLimiter(rateLimiterOptions), }); err != nil { @@ -297,13 +296,15 @@ func main() { } if err := (&controller.OCIRepositoryReconciler{ - Client: mgr.GetClient(), - Storage: storage, - Recorder: eventRecorder, - ControllerName: controllerName, - TokenCache: tokenCache, - CosignVerifierFactory: CosignVerifierFactory, - Metrics: metrics, + Client: mgr.GetClient(), + Storage: storage, + Recorder: eventRecorder, + ControllerName: controllerName, + TokenCache: tokenCache, + CosignVerifierFactory: CosignVerifierFactory, + Metrics: metrics, + DefaultServiceAccount: defaultServiceAccount, + ObjectLevelWorkloadIdentity: objectLevelWorkloadIdentity, }).SetupWithManager(mgr, controller.OCIRepositoryReconcilerOptions{ RateLimiter: helper.GetRateLimiter(rateLimiterOptions), }); err != nil {