adutra commented on code in PR #3750:
URL: https://github.com/apache/polaris/pull/3750#discussion_r2799623378
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalogHandler.java:
##########
@@ -1307,6 +1317,34 @@ private void checkAllowExternalCatalogCredentialVending(
}
}
+ /**
+ * Resolves the optimal access delegation mode from the set of requested
modes based on the
+ * catalog's capabilities.
+ *
+ * <p>When both VENDED_CREDENTIALS and REMOTE_SIGNING are requested, the
resolver checks:
+ *
+ * <ul>
+ * <li>STS availability from the catalog's storage configuration
+ * <li>Whether credential subscoping is skipped
+ * </ul>
+ *
+ * <p>This ensures the optimal mode is selected based on actual catalog
capabilities rather than
+ * simple heuristics.
+ *
+ * @param requestedModes The set of delegation modes requested by the client
+ * @return The resolved set of delegation modes (containing the single
optimal mode)
+ */
+ protected EnumSet<AccessDelegationMode> resolveAccessDelegationModes(
+ EnumSet<AccessDelegationMode> requestedModes) {
+ if (requestedModes.isEmpty()) {
+ return requestedModes;
+ }
+
+ AccessDelegationModeResolver resolver = new
AccessDelegationModeResolver(realmConfig());
Review Comment:
1. The resolver itself could be exposed as a method like this:
```java
@Value.Lazy
protected AccessDelegationModeResolver accessDelegationModeResolver() {
return new AccessDelegationModeResolver(realmConfig());
}
```
2. I would suggest to store the result of the access delegation mode
resolution as a field in this class for later retrieval – similar to what we do
with the resolution manifest. It's not necessary to pass the access mode around
as a parameter in each method.
--
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]
