obelix74 commented on code in PR #4707:
URL: https://github.com/apache/polaris/pull/4707#discussion_r3425648913


##########
polaris-core/src/main/java/org/apache/polaris/core/storage/gcp/GcpCredentialsStorageIntegration.java:
##########
@@ -176,17 +178,57 @@ private GcpStorageCredentialCacheKey buildCacheKey(
       @NonNull Set<String> writeLocations,
       @NonNull Optional<String> refreshEndpoint,
       @NonNull CredentialVendingContext context) {
+    // Principal attribution makes the vended token per-principal, so the 
principal must
+    // participate in cache identity; otherwise it is left empty to preserve 
cross-principal cache
+    // reuse. Attribution requires a service account to impersonate and a 
principal to attribute.
+    String principalName = "";
+    Optional<GcpAttributionParams> attributionParams = Optional.empty();
+    if (principalAttributionConfigured(realmConfig())
+        && storageConfig().getGcpServiceAccount() != null) {
+      principalName = context.principalName().orElse("");
+      if (!principalName.isEmpty()) {
+        attributionParams =
+            Optional.of(
+                GcpAttributionParams.of(
+                    realmConfig()
+                        
.getConfig(FeatureConfiguration.GCS_PRINCIPAL_ATTRIBUTION_TOKEN_ISSUER),
+                    realmConfig()
+                        
.getConfig(FeatureConfiguration.GCS_PRINCIPAL_ATTRIBUTION_WIF_AUDIENCE),
+                    realmConfig()
+                        
.getConfig(FeatureConfiguration.GCS_PRINCIPAL_ATTRIBUTION_SIGNING_KEY_FILE),
+                    realmConfig()
+                        
.getConfig(FeatureConfiguration.GCS_PRINCIPAL_ATTRIBUTION_SIGNING_KEY_ID)));
+      }
+    }
     return GcpStorageCredentialCacheKey.of(
         context.realm().orElse(""),
         storageConfig(),
         readLocations,
         listLocations,
         writeLocations,
         refreshEndpoint,
+        principalName,
         sourceCredentials,
         transportFactory,
         realmConfig(),
-        credentialOps);
+        credentialOps,
+        attributionParams);
+  }
+
+  /**
+   * Returns true when GCS principal attribution is fully configured (WIF 
audience, token issuer,
+   * and signing key file all set). There is intentionally no separate on/off 
flag.
+   */
+  private static boolean principalAttributionConfigured(RealmConfig 
realmConfig) {

Review Comment:
   Good point — done. Added an explicit `GCS_PRINCIPAL_ATTRIBUTION_ENABLED` 
boolean flag (default `false`). When `true`, `principalAttributionConfigured()` 
validates that `WIF_AUDIENCE`, `TOKEN_ISSUER`, and `SIGNING_KEY_FILE` are all 
set and throws `IllegalStateException` naming the missing values if not — so a 
misconfigured deployment fails fast on the first credential-vending attempt 
rather than silently producing unattributed tokens. Config docs regenerated. 
Pushed in 6906ebc8e.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to