Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HADOOP-18980. S3A credential provider remapping: make extensible #6406

Merged
merged 7 commits into from
Feb 2, 2024
Merged

HADOOP-18980. S3A credential provider remapping: make extensible #6406

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4964b5a
HADOOP-18980. S3A credential provider remapping: make extensible
virajjasani Jan 4, 2024
5e2fcb5
addendum - checkstyle
virajjasani Jan 8, 2024
576e80b
addendum - addressing comments
virajjasani Jan 26, 2024
c261fcd
addendum - checkstyle
virajjasani Jan 26, 2024
cff702f
move utility usage from config class to s3a util class
virajjasani Jan 31, 2024
f918d23
test + doc changes
virajjasani Feb 1, 2024
7428c13
minor doc changes
virajjasani Feb 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions ...-common-project/hadoop-common/src/test/java/org/apache/hadoop/conf/TestConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2716,4 +2716,72 @@ public void testConcurrentModificationDuringIteration() throws InterruptedExcept

assertFalse("ConcurrentModificationException occurred", exceptionOccurred.get());
}

@Test
public void testStringCollectionSplitByEquals() {
Configuration conf = new Configuration();
conf.set("custom_key", "");
Map<String, String> splitMap =
conf.getTrimmedStringCollectionSplitByEquals("custom_key");
Assertions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use

assertThat(splitMap).hasSize(0)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, btw TestConfiguration changes are removed after reverting Configuration class changes as per your previous feedback.

.assertThat(splitMap.size())
.describedAs("Map of key value pairs derived from config, split by equals(=) and comma(,)")
.isEqualTo(0);

splitMap = conf.getTrimmedStringCollectionSplitByEquals("not_present");
Assertions
.assertThat(splitMap.size())
.describedAs("Map of key value pairs derived from config, split by equals(=) and comma(,)")
.isEqualTo(0);

conf.set("custom_key", "element.first.key1 = element.first.val1");
splitMap = conf.getTrimmedStringCollectionSplitByEquals(
"custom_key");
Assertions
.assertThat(splitMap.size())
.describedAs("Map of key value pairs derived from config, split by equals(=) and comma(,)")
.isEqualTo(1);
Assertions
.assertThat(splitMap)
.describedAs("Map of key value pairs derived from config, split by equals(=) and comma(,)")
.containsEntry("element.first.key1", "element.first.val1");

conf.set("custom_key",
"element.xyz.key1 =element.abc.val1 , element.xyz.key2= element.abc.val2");
splitMap = conf.getTrimmedStringCollectionSplitByEquals(
"custom_key");
Assertions
.assertThat(splitMap.size())
.describedAs("Map of key value pairs derived from config, split by equals(=) and comma(,)")
.isEqualTo(2);
Assertions
.assertThat(splitMap)
.describedAs("Map of key value pairs derived from config, split by equals(=) and comma(,)")
.containsEntry("element.xyz.key1", "element.abc.val1")
.containsEntry("element.xyz.key2", "element.abc.val2");

conf.set("custom_key",
"\nelement.xyz.key1 =element.abc.val1 \n"
+ ", element.xyz.key2=element.abc.val2,element.xyz.key3=element.abc.val3"
+ " , element.xyz.key4 =element.abc.val4,element.xyz.key5= "
+ "element.abc.val5 ,\n \n \n "
+ " element.xyz.key6 = element.abc.val6 \n , \n"
+ "element.xyz.key7=element.abc.val7,\n");
splitMap = conf.getTrimmedStringCollectionSplitByEquals(
"custom_key");
Assertions
.assertThat(splitMap.size())
.describedAs("Map of key value pairs derived from config, split by equals(=) and comma(,)")
.isEqualTo(7);
Assertions
.assertThat(splitMap)
.describedAs("Map of key value pairs derived from config, split by equals(=) and comma(,)")
.containsEntry("element.xyz.key1", "element.abc.val1")
.containsEntry("element.xyz.key2", "element.abc.val2")
.containsEntry("element.xyz.key3", "element.abc.val3")
.containsEntry("element.xyz.key4", "element.abc.val4")
.containsEntry("element.xyz.key5", "element.abc.val5")
.containsEntry("element.xyz.key6", "element.abc.val6")
.containsEntry("element.xyz.key7", "element.abc.val7");
}
}
63 changes: 63 additions & 0 deletions ...op-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestStringUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@
import org.apache.commons.lang3.time.FastDateFormat;
import org.apache.hadoop.test.UnitTestcaseTimeLimit;
import org.apache.hadoop.util.StringUtils.TraditionalBinaryPrefix;

import org.assertj.core.api.Assertions;
import org.junit.Test;

public class TestStringUtils extends UnitTestcaseTimeLimit {
Expand Down Expand Up @@ -512,6 +514,67 @@ public void testCreateStartupShutdownMessage() {
assertTrue(msg.startsWith("STARTUP_MSG:"));
}

@Test
public void testStringCollectionSplitByEquals() {
Map<String, String> splitMap =
StringUtils.getTrimmedStringCollectionSplitByEquals("");
Assertions
.assertThat(splitMap.size())
.describedAs("Map of key value pairs split by equals(=) and comma(,)")
.isEqualTo(0);

splitMap = StringUtils.getTrimmedStringCollectionSplitByEquals(null);
Assertions
.assertThat(splitMap.size())
.describedAs("Map of key value pairs split by equals(=) and comma(,)")
.isEqualTo(0);

splitMap = StringUtils.getTrimmedStringCollectionSplitByEquals(
"element.first.key1 = element.first.val1");
Assertions
.assertThat(splitMap.size())
.describedAs("Map of key value pairs split by equals(=) and comma(,)")
.isEqualTo(1);
Assertions
.assertThat(splitMap)
.describedAs("Map of key value pairs split by equals(=) and comma(,)")
.containsEntry("element.first.key1", "element.first.val1");

splitMap = StringUtils.getTrimmedStringCollectionSplitByEquals(
"element.xyz.key1 =element.abc.val1 , element.xyz.key2= element.abc.val2");
Assertions
.assertThat(splitMap.size())
.describedAs("Map of key value pairs split by equals(=) and comma(,)")
.isEqualTo(2);
Assertions
.assertThat(splitMap)
.describedAs("Map of key value pairs split by equals(=) and comma(,)")
.containsEntry("element.xyz.key1", "element.abc.val1")
.containsEntry("element.xyz.key2", "element.abc.val2");

splitMap = StringUtils.getTrimmedStringCollectionSplitByEquals(
"\nelement.xyz.key1 =element.abc.val1 \n"
+ ", element.xyz.key2=element.abc.val2,element.xyz.key3=element.abc.val3"
+ " , element.xyz.key4 =element.abc.val4,element.xyz.key5= "
+ "element.abc.val5 ,\n \n \n "
+ " element.xyz.key6 = element.abc.val6 \n , \n"
+ "element.xyz.key7=element.abc.val7,\n");
Assertions
.assertThat(splitMap.size())
.describedAs("Map of key value pairs split by equals(=) and comma(,)")
.isEqualTo(7);
Assertions
.assertThat(splitMap)
.describedAs("Map of key value pairs split by equals(=) and comma(,)")
.containsEntry("element.xyz.key1", "element.abc.val1")
.containsEntry("element.xyz.key2", "element.abc.val2")
.containsEntry("element.xyz.key3", "element.abc.val3")
.containsEntry("element.xyz.key4", "element.abc.val4")
.containsEntry("element.xyz.key5", "element.abc.val5")
.containsEntry("element.xyz.key6", "element.abc.val6")
.containsEntry("element.xyz.key7", "element.abc.val7");
}

// Benchmark for StringUtils split
public static void main(String []args) {
final String TO_SPLIT = "foo,bar,baz,blah,blah";
Expand Down
5 changes: 4 additions & 1 deletion hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,10 @@ private Constants() {
public static final String AWS_CREDENTIALS_PROVIDER =
"fs.s3a.aws.credentials.provider";

// aws credentials providers mapping with key/value pairs
/**
* AWS credentials providers mapping with key/value pairs.
* Value = {@value}
*/
public static final String AWS_CREDENTIALS_PROVIDER_MAPPING =
"fs.s3a.aws.credentials.provider.mapping";

Expand Down
5 changes: 2 additions & 3 deletions ...hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/auth/CredentialProviderListFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,10 +235,9 @@ public static AWSCredentialProviderList buildAWSProviderList(
LOG_REMAPPED_ENTRY.warn("Credentials option {} contains AWS v1 SDK entry {}; mapping to {}",
key, className, mapped);
className = mapped;
}
if (awsCredsMappedClasses != null && awsCredsMappedClasses.containsKey(className)) {
} else if (awsCredsMappedClasses != null && awsCredsMappedClasses.containsKey(className)) {
final String mapped = awsCredsMappedClasses.get(className);
LOG_REMAPPED_ENTRY.info("Credential entry {} is mapped to {}", className, mapped);
LOG_REMAPPED_ENTRY.debug("Credential entry {} is mapped to {}", className, mapped);
className = mapped;
}
// now scan the forbidden list. doing this after any mappings ensures the v1 names
Expand Down
10 changes: 8 additions & 2 deletions ...ools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AAWSCredentialsProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,10 @@ public void testAssumedRoleWithRemap() throws Throwable {
ASSUMED_ROLE_CREDENTIALS_PROVIDER,
new ArrayList<>(),
new HashSet<>());
assertEquals("Credentials not matching", 3, credentials.size());
Assertions
.assertThat(credentials.size())
.describedAs("List of Credentials providers")
.isEqualTo(3);
}

@Test
Expand All @@ -253,7 +256,10 @@ public void testAwsCredentialProvidersWithRemap() throws Throwable {
AWS_CREDENTIALS_PROVIDER,
new ArrayList<>(),
new HashSet<>());
assertEquals("Credentials not matching", 4, credentials.size());
Assertions
.assertThat(credentials.size())
.describedAs("List of Credentials providers")
.isEqualTo(4);
}

@Test
Expand Down
Loading