From b49dbcfce8ff7d7392c97fc15e55c339f3887bfd Mon Sep 17 00:00:00 2001 From: sai pranav Date: Fri, 21 Aug 2026 17:12:29 +0530 Subject: [PATCH] fix: treat Azure buckets without configured identity as public azureauth.NewTokenCredential never returns nil, so the token != nil guard in chainCredentialWithSecret was always true, the chain was never empty, and the documented nil return was dead. At the caller that made azblob.NewClientWithNoCredential unreachable, so a Bucket with provider azure and no secretRef could not read a public container. docs/spec/v1/buckets.md states that when no chain can be established the bucket is assumed to be publicly reachable, and ships an azure-public example with no secretRef. Add the token credential only when an identity was actually requested: per-object via .spec.serviceAccountName, or controller-wide via AZURE_CLIENT_ID / AZURE_FEDERATED_TOKEN_FILE. Otherwise the chain stays empty and the caller builds an unauthenticated client. The existing chain test asserted the buggy behaviour, expecting a credential for a nil secret. It now covers all three cases: no identity, object-level, and controller-level. Co-Authored-By: Claude Opus 5 Signed-off-by: sai pranav --- internal/bucket/azure/blob.go | 36 +++++++++++++++++++++--- internal/bucket/azure/blob_test.go | 31 +++++++++++++++++--- internal/controller/bucket_controller.go | 3 ++ 3 files changed, 62 insertions(+), 8 deletions(-) diff --git a/internal/bucket/azure/blob.go b/internal/bucket/azure/blob.go index 94489add9..6b3f7001c 100644 --- a/internal/bucket/azure/blob.go +++ b/internal/bucket/azure/blob.go @@ -88,6 +88,7 @@ type options struct { secret *corev1.Secret proxyURL *url.URL withoutCredentials bool + objectLevelIdentity bool withoutRetries bool authOpts []auth.Option } @@ -96,6 +97,16 @@ type options struct { // This is a test-only option useful for testing the client with HTTP // endpoints (without TLS) alongside all the other options unrelated to // credentials. +// WithObjectLevelIdentity signals that the Bucket requests object-level workload +// identity, i.e. it sets .spec.serviceAccountName. Without this signal, and without +// controller-level identity configured in the environment, no token credential is +// added to the chain and the bucket is treated as publicly reachable. +func WithObjectLevelIdentity() Option { + return func(o *options) { + o.objectLevelIdentity = true + } +} + func withoutCredentials() Option { return func(o *options) { o.withoutCredentials = true @@ -202,7 +213,7 @@ func NewClient(ctx context.Context, obj *sourcev1.Bucket, opts ...Option) (c *Bl // Compose token chain based on environment. // This functions as a replacement for azidentity.NewDefaultAzureCredential // to not shell out. - token, err = chainCredentialWithSecret(ctx, o.secret, o.authOpts...) + token, err = chainCredentialWithSecret(ctx, o.secret, o.objectLevelIdentity, o.authOpts...) if err != nil { err = fmt.Errorf("failed to create environment credential chain: %w", err) return nil, err @@ -502,7 +513,7 @@ func sasTokenFromSecret(ep string, secret *corev1.Secret) (string, error) { // - azidentity.ManagedIdentityCredential with defaults. // // If no valid token is created, it returns nil. -func chainCredentialWithSecret(ctx context.Context, secret *corev1.Secret, opts ...auth.Option) (azcore.TokenCredential, error) { +func chainCredentialWithSecret(ctx context.Context, secret *corev1.Secret, objectLevelIdentity bool, opts ...auth.Option) (azcore.TokenCredential, error) { var creds []azcore.TokenCredential credOpts := &azidentity.EnvironmentCredentialOptions{} @@ -515,8 +526,13 @@ func chainCredentialWithSecret(ctx context.Context, secret *corev1.Secret, opts if token, _ := azidentity.NewEnvironmentCredential(credOpts); token != nil { creds = append(creds, token) } - if token := azureauth.NewTokenCredential(ctx, opts...); token != nil { - creds = append(creds, token) + // azureauth.NewTokenCredential never returns nil, so its presence in the chain + // cannot signal that any identity is actually configured. Add it only when an + // identity has been requested, either per-object via .spec.serviceAccountName or + // controller-wide via the environment. Otherwise the chain stays empty and the + // caller falls back to an unauthenticated client, as documented for public buckets. + if objectLevelIdentity || hasControllerLevelIdentity() { + creds = append(creds, azureauth.NewTokenCredential(ctx, opts...)) } if len(creds) > 0 { @@ -526,6 +542,18 @@ func chainCredentialWithSecret(ctx context.Context, secret *corev1.Secret, opts return nil, nil } +// hasControllerLevelIdentity reports whether the controller's environment carries an +// Azure identity. These are the variables the workload and managed identity flows in +// fluxcd/pkg/auth read; if none is set there is nothing for a token credential to use. +func hasControllerLevelIdentity() bool { + for _, env := range []string{"AZURE_CLIENT_ID", "AZURE_FEDERATED_TOKEN_FILE"} { + if os.Getenv(env) != "" { + return true + } + } + return false +} + // extractAccountNameFromEndpoint extracts the Azure account name from the // provided endpoint URL. It parses the endpoint as a URL, and returns the // first subdomain as the assumed account name. diff --git a/internal/bucket/azure/blob_test.go b/internal/bucket/azure/blob_test.go index 433f0f14c..50d043c40 100644 --- a/internal/bucket/azure/blob_test.go +++ b/internal/bucket/azure/blob_test.go @@ -657,11 +657,34 @@ func TestBlobClient_VisitObjects_MissingFields(t *testing.T) { } func Test_chainCredentialWithSecret(t *testing.T) { - g := NewWithT(t) + t.Run("no secret and no identity yields no credential", func(t *testing.T) { + g := NewWithT(t) - got, err := chainCredentialWithSecret(t.Context(), nil) - g.Expect(err).ToNot(HaveOccurred()) - g.Expect(got).To(BeAssignableToTypeOf(&azidentity.ChainedTokenCredential{})) + // Documented behaviour: "If no chain can be established, the bucket is + // assumed to be publicly reachable." The caller relies on a nil credential + // to build an unauthenticated client. + got, err := chainCredentialWithSecret(t.Context(), nil, false) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).To(BeNil()) + }) + + t.Run("object-level identity yields a credential", func(t *testing.T) { + g := NewWithT(t) + + got, err := chainCredentialWithSecret(t.Context(), nil, true) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).To(BeAssignableToTypeOf(&azidentity.ChainedTokenCredential{})) + }) + + t.Run("controller-level identity yields a credential", func(t *testing.T) { + g := NewWithT(t) + + t.Setenv("AZURE_CLIENT_ID", "00000000-0000-0000-0000-000000000000") + + got, err := chainCredentialWithSecret(t.Context(), nil, false) + g.Expect(err).ToNot(HaveOccurred()) + g.Expect(got).To(BeAssignableToTypeOf(&azidentity.ChainedTokenCredential{})) + }) } func Test_extractAccountNameFromEndpoint1(t *testing.T) { diff --git a/internal/controller/bucket_controller.go b/internal/controller/bucket_controller.go index 797b48709..2d7c96ba4 100644 --- a/internal/controller/bucket_controller.go +++ b/internal/controller/bucket_controller.go @@ -925,6 +925,9 @@ func (r *BucketReconciler) createBucketProvider(ctx context.Context, obj *source if creds.proxyURL != nil { opts = append(opts, azure.WithProxyURL(creds.proxyURL)) } + if obj.Spec.ServiceAccountName != "" { + opts = append(opts, azure.WithObjectLevelIdentity()) + } opts = append(opts, azure.WithAuth(authOpts...)) return azure.NewClient(ctx, obj, opts...)