[
https://issues.apache.org/jira/browse/HADOOP-18980?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17811044#comment-17811044
]
ASF GitHub Bot commented on HADOOP-18980:
-----------------------------------------
steveloughran commented on code in PR #6406:
URL: https://github.com/apache/hadoop/pull/6406#discussion_r1467034744
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java:
##########
@@ -68,6 +68,10 @@ private Constants() {
public static final String AWS_CREDENTIALS_PROVIDER =
"fs.s3a.aws.credentials.provider";
+ // aws credentials providers mapping with key/value pairs
Review Comment:
nit: javadocs with @value
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AAWSCredentialsProvider.java:
##########
@@ -206,6 +210,52 @@ public void testFallbackToDefaults() throws Throwable {
assertTrue("empty credentials", credentials.size() > 0);
}
+ @Test
+ public void testAssumedRoleWithRemap() throws Throwable {
+ Configuration conf = new Configuration(false);
+ conf.set(ASSUMED_ROLE_CREDENTIALS_PROVIDER,
+
"custom.assume.role.key1,custom.assume.role.key2,custom.assume.role.key3");
+ conf.set(AWS_CREDENTIALS_PROVIDER_MAPPING,
+ "custom.assume.role.key1="
+ + CredentialProviderListFactory.ENVIRONMENT_CREDENTIALS_V2
+ + " ,custom.assume.role.key2 ="
+ + CountInvocationsProvider.NAME
+ + ", custom.assume.role.key3= "
+ + CredentialProviderListFactory.PROFILE_CREDENTIALS_V1);
+ final AWSCredentialProviderList credentials =
+ buildAWSProviderList(
+ new URI("s3a://bucket1"),
+ conf,
+ ASSUMED_ROLE_CREDENTIALS_PROVIDER,
+ new ArrayList<>(),
+ new HashSet<>());
+ assertEquals("Credentials not matching", 3, credentials.size());
Review Comment:
assertJ size assert
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/auth/CredentialProviderListFactory.java:
##########
@@ -233,6 +236,11 @@ public static AWSCredentialProviderList
buildAWSProviderList(
key, className, mapped);
className = mapped;
}
+ if (awsCredsMappedClasses != null &&
awsCredsMappedClasses.containsKey(className)) {
Review Comment:
make an `else` unless we really want to support remapping of the standard
map to different values. Which I suppose we might...
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AAWSCredentialsProvider.java:
##########
@@ -206,6 +210,52 @@ public void testFallbackToDefaults() throws Throwable {
assertTrue("empty credentials", credentials.size() > 0);
}
+ @Test
+ public void testAssumedRoleWithRemap() throws Throwable {
+ Configuration conf = new Configuration(false);
+ conf.set(ASSUMED_ROLE_CREDENTIALS_PROVIDER,
+
"custom.assume.role.key1,custom.assume.role.key2,custom.assume.role.key3");
+ conf.set(AWS_CREDENTIALS_PROVIDER_MAPPING,
+ "custom.assume.role.key1="
+ + CredentialProviderListFactory.ENVIRONMENT_CREDENTIALS_V2
+ + " ,custom.assume.role.key2 ="
+ + CountInvocationsProvider.NAME
+ + ", custom.assume.role.key3= "
+ + CredentialProviderListFactory.PROFILE_CREDENTIALS_V1);
+ final AWSCredentialProviderList credentials =
+ buildAWSProviderList(
+ new URI("s3a://bucket1"),
+ conf,
+ ASSUMED_ROLE_CREDENTIALS_PROVIDER,
+ new ArrayList<>(),
+ new HashSet<>());
+ assertEquals("Credentials not matching", 3, credentials.size());
+ }
+
+ @Test
+ public void testAwsCredentialProvidersWithRemap() throws Throwable {
+ Configuration conf = new Configuration(false);
+ conf.set(AWS_CREDENTIALS_PROVIDER,
+
"custom.aws.creds.key1,custom.aws.creds.key2,custom.aws.creds.key3,custom.aws.creds.key4");
+ conf.set(AWS_CREDENTIALS_PROVIDER_MAPPING,
+ "custom.aws.creds.key1="
+ + CredentialProviderListFactory.ENVIRONMENT_CREDENTIALS_V2
+ + " ,\ncustom.aws.creds.key2="
+ + CountInvocationsProvider.NAME
+ + "\n, custom.aws.creds.key3="
+ + CredentialProviderListFactory.PROFILE_CREDENTIALS_V1
+ + ",custom.aws.creds.key4 = "
+ + CredentialProviderListFactory.PROFILE_CREDENTIALS_V2);
+ final AWSCredentialProviderList credentials =
+ buildAWSProviderList(
+ new URI("s3a://bucket1"),
+ conf,
+ AWS_CREDENTIALS_PROVIDER,
+ new ArrayList<>(),
+ new HashSet<>());
+ assertEquals("Credentials not matching", 4, credentials.size());
Review Comment:
assertJ. It's predictable I'll expect these, save time by embracing the api
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/auth/CredentialProviderListFactory.java:
##########
@@ -233,6 +236,11 @@ public static AWSCredentialProviderList
buildAWSProviderList(
key, className, mapped);
className = mapped;
}
+ if (awsCredsMappedClasses != null &&
awsCredsMappedClasses.containsKey(className)) {
+ final String mapped = awsCredsMappedClasses.get(className);
+ LOG_REMAPPED_ENTRY.info("Credential entry {} is mapped to {}",
className, mapped);
Review Comment:
debug
> S3A credential provider remapping: make extensible
> --------------------------------------------------
>
> Key: HADOOP-18980
> URL: https://issues.apache.org/jira/browse/HADOOP-18980
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/s3
> Affects Versions: 3.4.0
> Reporter: Steve Loughran
> Assignee: Viraj Jasani
> Priority: Minor
> Labels: pull-request-available
>
> s3afs will now remap the common com.amazonaws credential providers to
> equivalents in the v2 sdk or in hadoop-aws
> We could do the same for third party credential providers by taking a
> key=value list in a configuration property and adding to the map.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
