XJDKC commented on code in PR #1506:
URL: https://github.com/apache/polaris/pull/1506#discussion_r2070956330
##########
spec/polaris-management-service.yml:
##########
@@ -938,6 +940,34 @@ components:
format: password
description: Bearer token (input-only)
+ SigV4AuthenticationParameters:
+ type: object
+ description: AWS Signature Version 4 authentication
+ allOf:
+ - $ref: '#/components/schemas/AuthenticationParameters'
+ properties:
+ roleArn:
+ type: string
+ description: The aws IAM role arn assume when signing requests
Review Comment:
I think we should keep it consistent with the Iceberg SDK:
[AuthProperties.java#30](https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/rest/auth/AuthProperties.java#L30)
Also, getting the tmp AWS credential from STS is just the first step. The
actual operation involves signing the request using the SigV4 protocol. Given
that, I'd prefer we stick with using SIGV4 as the type name, it better reflects
the authentication mechanism being used.
But I agree that Apache Polaris might be deployed in a controlled
environment where users own both the AWS storage and the Polaris servers. In
that setup, using an IAM role isn't strictly necessary. Long-lived credentials
are often read from environment variables or command-line configs.
That said, we shouldn't recommend storing these long-lived credentials
directly in the catalog entity and persisting them in the metastore. It’s a
security risk, and it's generally best practice to keep credentials out of
persistent metadata wherever possible. (Maybe we can leverage the
`UserSecretsManager` added by @dennishuo and use a `UserSecretReference` to
refer to that long-lived credential).
WDYT?
--
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]
