Azure Bucket: anonymous access to public containers is unreachable since #1875

[Pranav-error]

Since #1875 (04ab27b42a4ab3bc989accdd8e958e189c60370c, merged 2025-09-02), a Bucket with provider: azure and no secretRef can no longer get an unauthenticated client. Both paths that would produce one are unreachable on main (99d894dd).

Why

chainCredentialWithSecret appends a credential that is never nil:

https://github.com/fluxcd/source-controller/blob/main/internal/bucket/azure/blob.go#L518

if token := azureauth.NewTokenCredential(ctx, opts...); token != nil {
    creds = append(creds, token)
}

auth/azure.NewTokenCredential returns &tokenCredential{ctx, opts} — a non-nil pointer in a non-nil interface, unconditionally. So the guard is always true, creds is never empty, len(creds) > 0 always holds, and the documented return nil, nil ("If no valid token is created, it returns nil") is dead code.

That propagates to the caller, where token != nil is then always true:

token, err = chainCredentialWithSecret(ctx, o.secret, o.authOpts...)
...
if token != nil {
    c.Client, err = azblob.NewClient(obj.Spec.Endpoint, token, clientOpts)
    return
}

// Fallback to simple client.
c.Client, err = azblob.NewClientWithNoCredential(obj.Spec.Endpoint, clientOpts)  // unreachable

The other unauthenticated path, the o.withoutCredentials early return, is gated on an unexported withoutCredentials() option that is only ever called from blob_test.go (4 call sites) and never from production code. So the anonymous behaviour that tests exercise is reached through a door the reconciler does not have, which is why the tests still pass.

Before #1875 this worked: creds only received NewEnvironmentCredential when that returned non-nil, so an empty chain — and therefore the nil, nil return and the no-credential fallback — was reachable.

Impact

A public/anonymously-readable Azure container configured without a secretRef gets an azblob client carrying a token credential instead of a no-credential client. At request time that calls auth.GetAccessToken, which fails when no Azure identity is configured, so the fetch fails rather than falling back to anonymous access.

I have verified the control flow and the regression point by reading the source; I have not reproduced it against a live public container, so please treat the runtime symptom as inferred from the code path rather than observed.

Found by

staticcheck ./internal/... flags it as SA4023, "this comparison is always true". The repo has no .golangci.yaml and CI runs make test only, so the check does not currently run anywhere.

Possible fixes

• Drop the always-true token != nil guard and decide the chain on whether any credential source is actually configured, so chainCredentialWithSecret can return nil, nil again as documented.
• Or make the intent explicit at the caller and have the reconciler pass the existing withoutCredentials() option when no secretRef is set, which would also give the anonymous path production coverage rather than test-only coverage.

Happy to open a PR for whichever direction you prefer.

[Pranav-error]

Correction to the report above: I attributed this to #1875 and that is wrong. The unreachability predates it.

The tail of the pre-#1875 chain was:

if token, _ := azidentity.NewManagedIdentityCredential(nil); token != nil {
    creds = append(creds, token)
}

I assumed that could yield nil off-Azure. It does not. Checked against the azidentity version currently vendored here, on a machine with no Azure environment:

NewEnvironmentCredential      -> nil=true   err=missing environment variable AZURE_TENANT_ID
NewManagedIdentityCredential(nil) -> nil=false  err=<nil>

newManagedIdentityClient resolves the source to IMDS by default and does not probe it at construction, so the credential is built successfully anywhere. That means creds was already non-empty before #1875, return nil, nil was already dead, and NewClientWithNoCredential was already unreachable. #1875 replaced one always-non-nil credential with another; it did not introduce the behaviour.

What still stands:

• Anonymous access to a public Azure container is unreachable on main. docs/spec/v1/buckets.md documents the opposite ("If no chain can be established, the bucket is assumed to be publicly reachable"), ships an azure-public example with no secretRef, and states that publicly accessible storage needs neither secretRef nor serviceAccountName.
• The o.withoutCredentials path is gated on an unexported option called only from blob_test.go, so the anonymous behaviour under test is not reachable from the reconciler.
• staticcheck SA4023 on the current guard is still valid, and still not run anywhere in CI.

What changes is that this is a long-standing gap between documented and actual behaviour rather than a recent regression, so the fix is a design decision rather than a revert. Given gcp.NewClient has no anonymous path at all and always uses a token source when no secret is present, it may be that "cloud providers always authenticate" is the intended direction and the Azure docs plus the dead fallback should be corrected instead. That is your call, and I would rather ask than guess — happy to write either PR.

Apologies for the incorrect attribution; I should have verified the old constructor before naming a commit.