snazy commented on code in PR #3496:
URL: https://github.com/apache/polaris/pull/3496#discussion_r2711388701
##########
polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsStorageConfigurationInfo.java:
##########
@@ -124,6 +124,13 @@ public URI getStsEndpointUri() {
return getStsEndpoint() == null ? getInternalEndpointUri() :
URI.create(getStsEndpoint());
}
+ @JsonIgnore
+ public boolean isAwsS3() {
+ String endpoint = getEndpoint();
+ // AWS S3 if no endpoint is specified or if it uses an amazonaws.com
endpoint
+ return endpoint == null || endpoint.contains(".amazonaws.com");
Review Comment:
Checking for `.amazonaws.com` works for the current public regions and
zones. Technically, it would be better to check for the string only in the host
part of the endpoint URI though. I do not know the endpoints of the relatively
new AWS local regions/zones. Whether those are underneath the `amazonaws.com`
domain. Amazon promised a "EU sovereign cloud", and I think to make it
completely sovereign, the endpoints would be in a different DNS domain.
OTOH this check wouldn't work for localstack, which supports KMS, or any
other KMS implementation.
I think, it would be better to gate this check on a different
condition/setting rather than looking for `.amazonaws.com` in the whole
endpoint URI.
--
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]
