adutra commented on code in PR #12582:
URL: https://github.com/apache/iceberg/pull/12582#discussion_r2007075056
##########
aws/src/main/java/org/apache/iceberg/aws/RESTSigV4AuthManager.java:
##########
@@ -61,20 +61,29 @@ public RESTSigV4AuthSession catalogSession(
}
@Override
- public AuthSession contextualSession(SessionCatalog.SessionContext context,
AuthSession parent) {
+ public RESTSigV4AuthSession contextualSession(
+ SessionCatalog.SessionContext context, AuthSession parent) {
AwsProperties contextProperties =
- new AwsProperties(RESTUtil.merge(catalogProperties,
context.properties()));
+ new AwsProperties(
+ RESTUtil.merge(
+ catalogProperties,
Review Comment:
This morning I tried to look at this problem from a different angle, and
looked at how the old SigV4 code used to create the `AwsProperties` instance:
https://github.com/apache/iceberg/blob/50863d7c028ea36c4cfb5d857f1324f7f978e579/aws/src/main/java/org/apache/iceberg/aws/RESTSigV4Signer.java#L66-L72
The `AwsProperties` was created with the _catalog_ properties, and was never
updated after that. IOW any context or table properties were effectively
ignored for the purposes of request signing.
So, if we want to have the same thing now, we'd need to reuse the catalog
`AwsProperties` without merging in the context or table properties.
But is that what we want?
I think this was the case before only because SigV4 signing was done via an
`HttpRequestInterceptor`, so it was impossible to apply context or table
properties.
But now this _is_ possible.
If we don't take them into account, we'd lose, for example, the ability to
leverage the context or the table config to provide user-specific AWS access
and secret keys, via dedicated properties like `rest.access-key-id` and
`rest.secret-access-key`.
I'm personally fine wither way 😄
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
