singhpk234 commented on code in PR #2672:
URL: https://github.com/apache/polaris/pull/2672#discussion_r2388448759
##########
polaris-core/src/main/java/org/apache/polaris/core/storage/aws/AwsCredentialsStorageIntegration.java:
##########
@@ -79,43 +79,46 @@ public AccessConfig getSubscopedCreds(
int storageCredentialDurationSeconds =
realmConfig.getConfig(STORAGE_CREDENTIAL_DURATION_SECONDS);
AwsStorageConfigurationInfo storageConfig = config();
- AssumeRoleRequest.Builder request =
- AssumeRoleRequest.builder()
- .externalId(storageConfig.getExternalId())
- .roleArn(storageConfig.getRoleARN())
- .roleSessionName("PolarisAwsCredentialsStorageIntegration")
- .policy(
- policyString(
- storageConfig.getAwsPartition(),
- allowListOperation,
- allowedReadLocations,
- allowedWriteLocations)
- .toJson())
- .durationSeconds(storageCredentialDurationSeconds);
- credentialsProvider.ifPresent(
- cp -> request.overrideConfiguration(b -> b.credentialsProvider(cp)));
-
String region = storageConfig.getRegion();
- @SuppressWarnings("resource")
- // Note: stsClientProvider returns "thin" clients that do not need closing
- StsClient stsClient =
-
stsClientProvider.stsClient(StsDestination.of(storageConfig.getStsEndpointUri(),
region));
-
- AssumeRoleResponse response = stsClient.assumeRole(request.build());
AccessConfig.Builder accessConfig = AccessConfig.builder();
- accessConfig.put(StorageAccessProperty.AWS_KEY_ID,
response.credentials().accessKeyId());
- accessConfig.put(
- StorageAccessProperty.AWS_SECRET_KEY,
response.credentials().secretAccessKey());
- accessConfig.put(StorageAccessProperty.AWS_TOKEN,
response.credentials().sessionToken());
- Optional.ofNullable(response.credentials().expiration())
- .ifPresent(
- i -> {
- accessConfig.put(
- StorageAccessProperty.EXPIRATION_TIME,
String.valueOf(i.toEpochMilli()));
- accessConfig.put(
- StorageAccessProperty.AWS_SESSION_TOKEN_EXPIRES_AT_MS,
- String.valueOf(i.toEpochMilli()));
- });
+
+ if (storageConfig.shouldUseSts()) {
Review Comment:
I see, thanks for the explanation, I totally missed taking `FILE` into
consideration ! IMHO its relatively easy to start with a stricter behaviour
first and relax it later rather doing the other way as now there might be
clients out there who would expect the behaviour to fail open and now expect
200, which we later plan to change to 400 anyways.
I would have appreciated to discussed this more !
--
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]
