From 3e8612ca4543686b9795991945a2c521f3dc93cf Mon Sep 17 00:00:00 2001 From: Didip Kerabat Date: Wed, 16 Mar 2022 17:08:28 -0700 Subject: [PATCH 1/9] Make AWS WebIdentityToken actually working and usable from inside EKS. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Original PR: https://github.com/apache/druid/pull/10541 doesn’t work. This PR addresses 2 of its issues: 1. Missing aws-java-sdk-sts.jar 2. applyAssumeRole function interferes with default s3Builder client behavior. --- cloud/aws-common/pom.xml | 4 ++ extensions-core/s3-extensions/pom.xml | 2 +- .../druid/data/input/s3/S3InputSource.java | 47 ++++++++++++------- .../data/input/s3/S3InputSourceConfig.java | 21 +++++++++ 4 files changed, 57 insertions(+), 17 deletions(-) diff --git a/cloud/aws-common/pom.xml b/cloud/aws-common/pom.xml index ddedf0b9bb73..c0f45fdd4aca 100644 --- a/cloud/aws-common/pom.xml +++ b/cloud/aws-common/pom.xml @@ -46,6 +46,10 @@ com.amazonaws aws-java-sdk-s3 + + com.amazonaws + aws-java-sdk-sts + org.checkerframework checker-qual diff --git a/extensions-core/s3-extensions/pom.xml b/extensions-core/s3-extensions/pom.xml index 48e5b3d223d1..9fe84ae6ad08 100644 --- a/extensions-core/s3-extensions/pom.xml +++ b/extensions-core/s3-extensions/pom.xml @@ -114,7 +114,7 @@ com.amazonaws aws-java-sdk-sts - ${aws.sdk.version} + provided diff --git a/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSource.java b/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSource.java index 20688ab07204..5a7aff0cc3ae 100644 --- a/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSource.java +++ b/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSource.java @@ -105,22 +105,31 @@ public S3InputSource( this.s3ClientSupplier = Suppliers.memoize( () -> { if (s3ClientBuilder != null && s3InputSourceConfig != null) { - if (s3InputSourceConfig.isCredentialsConfigured()) { - if (s3InputSourceConfig.getAssumeRoleArn() == null) { - s3ClientBuilder - .getAmazonS3ClientBuilder() - .withCredentials(createStaticCredentialsProvider(s3InputSourceConfig)); - } else { - applyAssumeRole( - s3ClientBuilder, - s3InputSourceConfig, - createStaticCredentialsProvider(s3InputSourceConfig) - ); - } - } else { + // Each of these if statements will manipulate s3ClientBuilder if the condition is fulfilled. + + // If both static key-pair and assume role ARN are defined, use the static key-pair to assume role. + if (s3InputSourceConfig.isCredentialsConfigured() && s3InputSourceConfig.isAssumeRoleArnConfigured()) { + applyAssumeRole( + s3ClientBuilder, + s3InputSourceConfig, + createStaticCredentialsProvider(s3InputSourceConfig) + ); + + // If only static key-pair is defined, build the S3 client with the static key-pair + } else if (s3InputSourceConfig.isCredentialsConfigured() && !s3InputSourceConfig.isAssumeRoleArnConfigured()) { + s3ClientBuilder.getAmazonS3ClientBuilder().withCredentials(createStaticCredentialsProvider(s3InputSourceConfig)); + + // If assume role ARN is defined, static key-pair is undefined, and WebIdentityToken file from the environment variable is undefined. + } else if (s3InputSourceConfig.isAssumeRoleArnConfigured() && !s3InputSourceConfig.isCredentialsConfigured() && !s3InputSourceConfig.isWebIdentityTokenEnvConfigured()) { applyAssumeRole(s3ClientBuilder, s3InputSourceConfig, awsCredentialsProvider); } + + // Actually build the ServerSideEncryptingAmazonS3 object. + return s3ClientBuilder.build(); + + } else if (s3ClientBuilder != null) { return s3ClientBuilder.build(); + } else { return s3Client; } @@ -166,15 +175,21 @@ private void applyAssumeRole( AWSCredentialsProvider awsCredentialsProvider ) { - String assumeRoleArn = s3InputSourceConfig.getAssumeRoleArn(); - if (assumeRoleArn != null) { + // Do not run if WebIdentityToken file and assumeRole ARN are detected from the environment variable, + // we want the default s3ClientBuilder behavior for ServiceAccount + eks.amazonaws.com/role-arn annotation to work. + if (s3InputSourceConfig.isWebIdentityTokenEnvConfigured() && s3InputSourceConfig.isAssumeRoleArnEnvConfigured()) { + return; + } + + if (s3InputSourceConfig.isAssumeRoleArnConfigured()) { String roleSessionName = StringUtils.format("druid-s3-input-source-%s", UUID.randomUUID().toString()); AWSSecurityTokenService securityTokenService = AWSSecurityTokenServiceClientBuilder.standard() .withCredentials(awsCredentialsProvider) .build(); + STSAssumeRoleSessionCredentialsProvider.Builder roleCredentialsProviderBuilder; roleCredentialsProviderBuilder = new STSAssumeRoleSessionCredentialsProvider - .Builder(assumeRoleArn, roleSessionName).withStsClient(securityTokenService); + .Builder(s3InputSourceConfig.getAssumeRoleArn(), roleSessionName).withStsClient(securityTokenService); if (s3InputSourceConfig.getAssumeRoleExternalId() != null) { roleCredentialsProviderBuilder.withExternalId(s3InputSourceConfig.getAssumeRoleExternalId()); diff --git a/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSourceConfig.java b/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSourceConfig.java index 6b837e703a07..852463a0363b 100644 --- a/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSourceConfig.java +++ b/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSourceConfig.java @@ -28,6 +28,9 @@ import javax.annotation.Nullable; import java.util.Objects; +import static com.amazonaws.SDKGlobalConfiguration.AWS_ROLE_ARN_ENV_VAR; +import static com.amazonaws.SDKGlobalConfiguration.AWS_WEB_IDENTITY_ENV_VAR; + /** * Contains properties for s3 input source. * Properties can be specified by ingestionSpec which will override system default. @@ -90,6 +93,24 @@ public boolean isCredentialsConfigured() secretAccessKey != null; } + @JsonIgnore + public boolean isAssumeRoleArnConfigured() + { + return assumeRoleArn != null && !assumeRoleArn.trim().isEmpty(); + } + + @JsonIgnore + public boolean isAssumeRoleArnEnvConfigured() + { + return !System.getenv(AWS_ROLE_ARN_ENV_VAR).trim().isEmpty(); + } + + @JsonIgnore + public boolean isWebIdentityTokenEnvConfigured() + { + return !System.getenv(AWS_WEB_IDENTITY_ENV_VAR).trim().isEmpty(); + } + @Override public String toString() { From 26faaf362b5b74b074b403dfa5d56e9735a3ee27 Mon Sep 17 00:00:00 2001 From: Didip Kerabat Date: Thu, 17 Mar 2022 15:42:31 -0700 Subject: [PATCH 2/9] Fix apache-rat:check error. --- pom.xml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pom.xml b/pom.xml index 598a16a0d1b6..7646a8e377e0 100644 --- a/pom.xml +++ b/pom.xml @@ -1835,6 +1835,8 @@ .editorconfig **/hadoop.indexer.libs.version **/codegen/** + website/target/** + website/node_modules/** From 307ba49b6768b951fc49b7bc6e0d1598adaaba46 Mon Sep 17 00:00:00 2001 From: Didip Kerabat Date: Thu, 17 Mar 2022 18:04:46 -0700 Subject: [PATCH 3/9] Fix stylistic issues. --- .../java/org/apache/druid/data/input/s3/S3InputSource.java | 6 +++--- .../org/apache/druid/data/input/s3/S3InputSourceConfig.java | 5 ++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSource.java b/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSource.java index 5a7aff0cc3ae..5989565f21d6 100644 --- a/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSource.java +++ b/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSource.java @@ -110,9 +110,9 @@ public S3InputSource( // If both static key-pair and assume role ARN are defined, use the static key-pair to assume role. if (s3InputSourceConfig.isCredentialsConfigured() && s3InputSourceConfig.isAssumeRoleArnConfigured()) { applyAssumeRole( - s3ClientBuilder, - s3InputSourceConfig, - createStaticCredentialsProvider(s3InputSourceConfig) + s3ClientBuilder, + s3InputSourceConfig, + createStaticCredentialsProvider(s3InputSourceConfig) ); // If only static key-pair is defined, build the S3 client with the static key-pair diff --git a/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSourceConfig.java b/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSourceConfig.java index 852463a0363b..0521f5ad92f0 100644 --- a/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSourceConfig.java +++ b/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSourceConfig.java @@ -19,6 +19,8 @@ package org.apache.druid.data.input.s3; +import com.amazonaws.SDKGlobalConfiguration.AWS_ROLE_ARN_ENV_VAR; +import com.amazonaws.SDKGlobalConfiguration.AWS_WEB_IDENTITY_ENV_VAR; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; @@ -28,9 +30,6 @@ import javax.annotation.Nullable; import java.util.Objects; -import static com.amazonaws.SDKGlobalConfiguration.AWS_ROLE_ARN_ENV_VAR; -import static com.amazonaws.SDKGlobalConfiguration.AWS_WEB_IDENTITY_ENV_VAR; - /** * Contains properties for s3 input source. * Properties can be specified by ingestionSpec which will override system default. From 7f15e88661d449f0947f8d353593a5d9af28bc92 Mon Sep 17 00:00:00 2001 From: Didip Kerabat Date: Thu, 17 Mar 2022 18:25:30 -0700 Subject: [PATCH 4/9] Fix the import bug I introduced due to removing static import. --- .../apache/druid/data/input/s3/S3InputSourceConfig.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSourceConfig.java b/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSourceConfig.java index 0521f5ad92f0..57a0c376afa6 100644 --- a/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSourceConfig.java +++ b/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSourceConfig.java @@ -19,8 +19,7 @@ package org.apache.druid.data.input.s3; -import com.amazonaws.SDKGlobalConfiguration.AWS_ROLE_ARN_ENV_VAR; -import com.amazonaws.SDKGlobalConfiguration.AWS_WEB_IDENTITY_ENV_VAR; +import com.amazonaws.SDKGlobalConfiguration; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; @@ -101,13 +100,13 @@ public boolean isAssumeRoleArnConfigured() @JsonIgnore public boolean isAssumeRoleArnEnvConfigured() { - return !System.getenv(AWS_ROLE_ARN_ENV_VAR).trim().isEmpty(); + return !System.getenv(SDKGlobalConfiguration.AWS_ROLE_ARN_ENV_VAR).trim().isEmpty(); } @JsonIgnore public boolean isWebIdentityTokenEnvConfigured() { - return !System.getenv(AWS_WEB_IDENTITY_ENV_VAR).trim().isEmpty(); + return !System.getenv(SDKGlobalConfiguration.AWS_WEB_IDENTITY_ENV_VAR).trim().isEmpty(); } @Override From e0d5c643ddb6451b59c9f4c9436378a36aae9067 Mon Sep 17 00:00:00 2001 From: Didip Kerabat Date: Thu, 17 Mar 2022 18:26:11 -0700 Subject: [PATCH 5/9] Attempt to satisfy code coverage. --- .../data/input/s3/S3InputSourceTest.java | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/extensions-core/s3-extensions/src/test/java/org/apache/druid/data/input/s3/S3InputSourceTest.java b/extensions-core/s3-extensions/src/test/java/org/apache/druid/data/input/s3/S3InputSourceTest.java index ff8f9682ff28..cee04527753e 100644 --- a/extensions-core/s3-extensions/src/test/java/org/apache/druid/data/input/s3/S3InputSourceTest.java +++ b/extensions-core/s3-extensions/src/test/java/org/apache/druid/data/input/s3/S3InputSourceTest.java @@ -21,6 +21,7 @@ import com.amazonaws.SdkClientException; import com.amazonaws.auth.AWSCredentialsProvider; +import com.amazonaws.SDKGlobalConfiguration; import com.amazonaws.services.s3.AmazonS3; import com.amazonaws.services.s3.AmazonS3Client; import com.amazonaws.services.s3.AmazonS3ClientBuilder; @@ -223,6 +224,61 @@ public void testSerdeWithCloudConfigPropertiesWithKeyAndSecret() throws Exceptio EasyMock.verify(SERVER_SIDE_ENCRYPTING_AMAZON_S3_BUILDER); } + @Test + public void testSerdeWithCloudConfigPropertiesWithIdentityFileConfigured() throws Exception + { + EasyMock.reset(SERVER_SIDE_ENCRYPTING_AMAZON_S3_BUILDER); + EasyMock.expect(SERVER_SIDE_ENCRYPTING_AMAZON_S3_BUILDER.getAmazonS3ClientBuilder()) + .andStubReturn(AMAZON_S3_CLIENT_BUILDER); + EasyMock.expect(SERVER_SIDE_ENCRYPTING_AMAZON_S3_BUILDER.build()) + .andReturn(SERVICE); + EasyMock.replay(SERVER_SIDE_ENCRYPTING_AMAZON_S3_BUILDER); + + // Mock setting the AWS's environment variables. + String oldRoleARN = System.getProperty(SDKGlobalConfiguration.AWS_ROLE_ARN_ENV_VAR); + if(oldRoleARN == "") { + System.setProperty(SDKGlobalConfiguration.AWS_ROLE_ARN_ENV_VAR, "mockROLEARN"); + } + + String oldIdentityFile = System.getProperty(SDKGlobalConfiguration.AWS_WEB_IDENTITY_ENV_VAR); + if(oldIdentityFile == "") { + System.setProperty(SDKGlobalConfiguration.AWS_WEB_IDENTITY_ENV_VAR, "mockIdentityFile"); + } + + S3InputSourceConfig cloudConfigProperties = new S3InputSourceConfig( + null, + null, + null, + null + ); + + final S3InputSource withPrefixes = new S3InputSource( + SERVICE, + SERVER_SIDE_ENCRYPTING_AMAZON_S3_BUILDER, + INPUT_DATA_CONFIG, + null, + null, + EXPECTED_LOCATION, + cloudConfigProperties + ); + final S3InputSource serdeWithPrefixes = + MAPPER.readValue(MAPPER.writeValueAsString(withPrefixes), S3InputSource.class); + + // This is to force the s3ClientSupplier to initialize the ServerSideEncryptingAmazonS3 + serdeWithPrefixes.createEntity(new CloudObjectLocation("bucket", "path")); + + Assert.assertEquals(withPrefixes, serdeWithPrefixes); + + if(oldRoleARN == "") { + System.clearProperty(SDKGlobalConfiguration.AWS_ROLE_ARN_ENV_VAR); + } + if(oldIdentityFile == "") { + System.clearProperty(SDKGlobalConfiguration.AWS_WEB_IDENTITY_ENV_VAR); + } + + EasyMock.verify(SERVER_SIDE_ENCRYPTING_AMAZON_S3_BUILDER); + } + @Test public void testS3InputSourceUseDefaultPasswordWhenCloudConfigPropertiesWithoutCrediential() { From f81f85a66c25221aff3f7e0bc1dc45b61e16fb50 Mon Sep 17 00:00:00 2001 From: Didip Kerabat Date: Fri, 18 Mar 2022 10:25:38 -0700 Subject: [PATCH 6/9] Make style checker happy. --- .../org/apache/druid/data/input/s3/S3InputSourceTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/extensions-core/s3-extensions/src/test/java/org/apache/druid/data/input/s3/S3InputSourceTest.java b/extensions-core/s3-extensions/src/test/java/org/apache/druid/data/input/s3/S3InputSourceTest.java index cee04527753e..3aef08e70d93 100644 --- a/extensions-core/s3-extensions/src/test/java/org/apache/druid/data/input/s3/S3InputSourceTest.java +++ b/extensions-core/s3-extensions/src/test/java/org/apache/druid/data/input/s3/S3InputSourceTest.java @@ -236,12 +236,12 @@ public void testSerdeWithCloudConfigPropertiesWithIdentityFileConfigured() throw // Mock setting the AWS's environment variables. String oldRoleARN = System.getProperty(SDKGlobalConfiguration.AWS_ROLE_ARN_ENV_VAR); - if(oldRoleARN == "") { + if(oldRoleARN.equals("")) { System.setProperty(SDKGlobalConfiguration.AWS_ROLE_ARN_ENV_VAR, "mockROLEARN"); } String oldIdentityFile = System.getProperty(SDKGlobalConfiguration.AWS_WEB_IDENTITY_ENV_VAR); - if(oldIdentityFile == "") { + if(oldIdentityFile.equals("")) { System.setProperty(SDKGlobalConfiguration.AWS_WEB_IDENTITY_ENV_VAR, "mockIdentityFile"); } @@ -269,10 +269,10 @@ public void testSerdeWithCloudConfigPropertiesWithIdentityFileConfigured() throw Assert.assertEquals(withPrefixes, serdeWithPrefixes); - if(oldRoleARN == "") { + if(oldRoleARN.equals("")) { System.clearProperty(SDKGlobalConfiguration.AWS_ROLE_ARN_ENV_VAR); } - if(oldIdentityFile == "") { + if(oldIdentityFile.equals("")) { System.clearProperty(SDKGlobalConfiguration.AWS_WEB_IDENTITY_ENV_VAR); } From d4753d8d77805ac280fe43b8df34d2ddfaeb2f78 Mon Sep 17 00:00:00 2001 From: Didip Kerabat Date: Sun, 3 Apr 2022 10:28:45 -0700 Subject: [PATCH 7/9] Revert the exclusion of apache-rat-plugin. --- pom.xml | 2 -- 1 file changed, 2 deletions(-) diff --git a/pom.xml b/pom.xml index 7646a8e377e0..598a16a0d1b6 100644 --- a/pom.xml +++ b/pom.xml @@ -1835,8 +1835,6 @@ .editorconfig **/hadoop.indexer.libs.version **/codegen/** - website/target/** - website/node_modules/** From b9b02da9072b3761789e54cdc5631bfb985861a8 Mon Sep 17 00:00:00 2001 From: Didip Kerabat Date: Sun, 3 Apr 2022 22:01:51 -0700 Subject: [PATCH 8/9] Remove trim(). --- .../org/apache/druid/data/input/s3/S3InputSourceConfig.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSourceConfig.java b/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSourceConfig.java index 57a0c376afa6..0ed8019c3090 100644 --- a/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSourceConfig.java +++ b/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSourceConfig.java @@ -94,19 +94,19 @@ public boolean isCredentialsConfigured() @JsonIgnore public boolean isAssumeRoleArnConfigured() { - return assumeRoleArn != null && !assumeRoleArn.trim().isEmpty(); + return assumeRoleArn != null && !assumeRoleArn.isEmpty(); } @JsonIgnore public boolean isAssumeRoleArnEnvConfigured() { - return !System.getenv(SDKGlobalConfiguration.AWS_ROLE_ARN_ENV_VAR).trim().isEmpty(); + return !System.getenv(SDKGlobalConfiguration.AWS_ROLE_ARN_ENV_VAR).isEmpty(); } @JsonIgnore public boolean isWebIdentityTokenEnvConfigured() { - return !System.getenv(SDKGlobalConfiguration.AWS_WEB_IDENTITY_ENV_VAR).trim().isEmpty(); + return !System.getenv(SDKGlobalConfiguration.AWS_WEB_IDENTITY_ENV_VAR).isEmpty(); } @Override From 6b2897d920faaafbfbe7beac099fb9e25fcc4165 Mon Sep 17 00:00:00 2001 From: Didip Kerabat Date: Tue, 5 Apr 2022 21:34:33 -0700 Subject: [PATCH 9/9] Prevents NPE. --- .../apache/druid/data/input/s3/S3InputSourceConfig.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSourceConfig.java b/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSourceConfig.java index 0ed8019c3090..94aed87ba199 100644 --- a/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSourceConfig.java +++ b/extensions-core/s3-extensions/src/main/java/org/apache/druid/data/input/s3/S3InputSourceConfig.java @@ -100,13 +100,17 @@ public boolean isAssumeRoleArnConfigured() @JsonIgnore public boolean isAssumeRoleArnEnvConfigured() { - return !System.getenv(SDKGlobalConfiguration.AWS_ROLE_ARN_ENV_VAR).isEmpty(); + String conf = null; + conf = System.getenv(SDKGlobalConfiguration.AWS_ROLE_ARN_ENV_VAR); + return conf != null && !conf.isEmpty(); } @JsonIgnore public boolean isWebIdentityTokenEnvConfigured() { - return !System.getenv(SDKGlobalConfiguration.AWS_WEB_IDENTITY_ENV_VAR).isEmpty(); + String conf = null; + conf = System.getenv(SDKGlobalConfiguration.AWS_WEB_IDENTITY_ENV_VAR); + return conf != null && !conf.isEmpty(); } @Override