[RFC-0010] Add multi-tenant workload identity support for GCP Bucket

[cappyzawa]

Part of: fluxcd/flux2#5022

[matheuscscp]

Looking good! 😁

[matheuscscp]

Workload identity should always be attempted if .spec.secretRef is not set (and .spec.provider is neither "" nor generic, which is the case inside gcp.go)

[matheuscscp]

So, after the analysis around the TestNewClientWithProxyErr test, I'd apply the following diff to this PR:

diff --git a/pkg/gcp/gcp.go b/pkg/gcp/gcp.go
index 67fdf22..d5061fa 100644
--- a/pkg/gcp/gcp.go
+++ b/pkg/gcp/gcp.go
@@ -114,23 +114,15 @@ func NewClient(ctx context.Context, bucket *sourcev1.Bucket, opts ...Option) (*G
 	switch {
 	case o.secret != nil && o.proxyURL == nil:
 		clientOpts = append(clientOpts, option.WithCredentialsJSON(o.secret.Data["serviceaccount"]))
-	case o.secret != nil && o.proxyURL != nil:
+	case o.secret == nil && o.proxyURL == nil:
+		tokenSource := gcpauth.NewTokenSource(ctx, o.authOpts...)
+		clientOpts = append(clientOpts, option.WithTokenSource(tokenSource))
+	default: // o.proxyURL != nil:
 		httpClient, err := o.newCustomHTTPClient(ctx, o)
 		if err != nil {
 			return nil, err
 		}
 		clientOpts = append(clientOpts, option.WithHTTPClient(httpClient))
-	default:
-		// Workload identity: Create TokenSource using auth options
-		tokenSource := gcpauth.NewTokenSource(ctx, o.authOpts...)
-		clientOpts = append(clientOpts, option.WithTokenSource(tokenSource))
-		if o.proxyURL != nil {
-			httpClient, err := o.newCustomHTTPClient(ctx, o)
-			if err != nil {
-				return nil, err
-			}
-			clientOpts = append(clientOpts, option.WithHTTPClient(httpClient))
-		}
 	}
 
 	client, err := gcpstorage.NewClient(ctx, clientOpts...)
@@ -160,8 +152,9 @@ func newHTTPClient(ctx context.Context, o *options) (*http.Client, error) {
 			return nil, fmt.Errorf("failed to create Google credentials from secret: %w", err)
 		}
 		opts = append(opts, option.WithCredentials(creds))
-	} else if len(o.authOpts) > 0 {
+	} else { // Workload Identity.
		tokenSource := gcpauth.NewTokenSource(ctx, o.authOpts...)
 		opts = append(opts, option.WithTokenSource(tokenSource))
 	}
 
diff --git a/pkg/gcp/gcp_test.go b/pkg/gcp/gcp_test.go
index 7d9add4..8a1899f 100644
--- a/pkg/gcp/gcp_test.go
+++ b/pkg/gcp/gcp_test.go
@@ -176,38 +176,29 @@ func TestNewClientWithProxyErr(t *testing.T) {
 	assert.Assert(t, !envADCIsSet)
 	assert.Assert(t, !metadata.OnGCE())
 
-	tests := []struct {
-		name string
-		opts []Option
-		err  string
-	}{
-		{
-			name: "invalid secret",
-			opts: []Option{WithSecret(secret.DeepCopy())},
-			err:  "failed to create Google credentials from secret: invalid character 'e' looking for beginning of value",
-		},
-		{
-			name: "proxy only - fallback to Controller-Level Workload Identity",
-			// Behavior change: previously failed, now falls back to Controller-Level Workload Identity
-		},
-	}
+	t.Run("with secret", func(t *testing.T) {
+		g := NewWithT(t)
+		bucket := createTestBucket()
+		gcpClient, err := NewClient(context.Background(), bucket,
+			WithProxyURL(&url.URL{}),
+			WithSecret(secret.DeepCopy()))
+		g.Expect(err).To(HaveOccurred())
+		g.Expect(gcpClient).To(BeNil())
+		g.Expect(err.Error()).To(Equal("failed to create Google credentials from secret: invalid character 'e' looking for beginning of value"))
+	})
 
-	for _, tt := range tests {
-		tt := tt
-		t.Run(tt.name, func(t *testing.T) {
-			bucket := createTestBucket()
-			opts := append([]Option{WithProxyURL(&url.URL{})}, tt.opts...)
-			gcpClient, err := NewClient(context.Background(), bucket, opts...)
-			if tt.err != "" {
-				assert.Error(t, err, tt.err)
-				assert.Assert(t, gcpClient == nil)
-			} else {
-				// For proxy-only case, it should succeed with Controller-Level Workload Identity
-				assert.NilError(t, err)
-				assert.Assert(t, gcpClient != nil)
-			}
-		})
-	}
+	t.Run("without secret", func(t *testing.T) {
+		g := NewWithT(t)
+		bucket := createTestBucket()
+		gcpClient, err := NewClient(context.Background(), bucket,
+			WithProxyURL(&url.URL{}))
+		g.Expect(err).NotTo(HaveOccurred())
+		g.Expect(gcpClient).NotTo(BeNil())
+		bucketAttrs, err := gcpClient.Client.Bucket("some-bucket").Attrs(context.Background())
+		g.Expect(err).To(HaveOccurred())
+		g.Expect(bucketAttrs).To(BeNil())
+		g.Expect(err.Error()).To(ContainSubstring("could not find default credentials"))
+	})
 }
 
 func TestProxy(t *testing.T) {

Edit: I remembered we're passing auth.WithProxyURL() in the controller and removed it from this diff 👍

[matheuscscp]

@cappyzawa Thanks very much for all the work here!

I was able to test this e2e for both controller-level and object-level workload identity 🚀

Please rebase, squash and amend the commit message to match the PR title 🙏