jackye1995 commented on code in PR #5684:
URL: https://github.com/apache/iceberg/pull/5684#discussion_r963119759
##########
aws/src/main/java/org/apache/iceberg/aws/AssumeRoleAwsClientFactory.java:
##########
@@ -91,37 +93,26 @@ public void initialize(Map<String, String> properties) {
Preconditions.checkNotNull(
region, "Cannot initialize AssumeRoleClientConfigFactory with null
region");
- this.s3Endpoint = properties.get(AwsProperties.S3FILEIO_ENDPOINT);
this.tags = toTags(properties);
- this.s3UseArnRegionEnabled =
- PropertyUtil.propertyAsBoolean(
- properties,
- AwsProperties.S3_USE_ARN_REGION_ENABLED,
- AwsProperties.S3_USE_ARN_REGION_ENABLED_DEFAULT);
- this.dynamoDbEndpoint = properties.get(AwsProperties.DYNAMODB_ENDPOINT);
- this.httpClientType =
- PropertyUtil.propertyAsString(
- properties, AwsProperties.HTTP_CLIENT_TYPE,
AwsProperties.HTTP_CLIENT_TYPE_DEFAULT);
- }
-
- protected <T extends AwsClientBuilder & AwsSyncClientBuilder> T configure(T
clientBuilder) {
- AssumeRoleRequest request =
+ this.assumeRoleRequest =
AssumeRoleRequest.builder()
.roleArn(roleArn)
.roleSessionName(genSessionName())
Review Comment:
yes I agree that session name should be configurable in 2 ways:
1. the session name itself
2. a boolean flag indicating if session name should be random, which is done
by appending a random UUID suffix
However, I think the session names in the request used by the
`StsAssumeRoleCredentialsProvider ` should be consistent and not changing for
each request. Considering from the audit log perspective, we should expect the
same session name to show even if there are multiple assume role requests sent
to refresh the credential, because they all come from the same program from
start to end. So I think `roleSessionName` should be an instance variable with
a fixed name.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
