dimas-b commented on code in PR #211:
URL: https://github.com/apache/polaris/pull/211#discussion_r1731929861
##########
polaris-service/src/main/java/org/apache/polaris/service/catalog/IcebergCatalogAdapter.java:
##########
@@ -171,28 +173,39 @@ public Response updateProperties(
.build();
}
+ private EnumSet<AccessDelegationMode> parseAccessDelegationModes(String
accessDelegationMode) {
+ EnumSet<AccessDelegationMode> delegationModes =
+ AccessDelegationMode.fromProtocolValuesList(accessDelegationMode);
+ if (!delegationModes.isEmpty() &&
!delegationModes.contains(VENDED_CREDENTIALS)) {
Review Comment:
> The server may choose to supply access via any or none of the requested
mechanisms.
My reading of this is that if the client specifies some access modes via the
header, the server must use zero, one or more of them. The server may not use a
delegation mode that the client did not request (for the client may not be able
to handle it).
In this PR, if the client does not request any delegation mode, the server
will _not_ use "vended credentials" (same as before). If the client sends
"vended credentials" and something else, the server will use "vended
credentials" (same as before).
If the client requests `true` - that will lead to an error response (change
in behaviour).
If the client requests only `remote signing` - that will lead to an error
response (change in behaviour). Previously, the server would still use "vended
credentials", so this is a bug fix, IMHO.
I'm not against adding config for more lenient processing, but given that
there's no released Polaris version that supported old values, I think we can
use simpler and stricter validation logic. It is easier to relax conditions
after a release than tighten them if we have to.
--
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]
