[ 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: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org