[
https://issues.apache.org/jira/browse/HADOOP-19050?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17816031#comment-17816031
]
ASF GitHub Bot commented on HADOOP-19050:
-----------------------------------------
ahmarsuhail commented on code in PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#discussion_r1484099683
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/DefaultS3ClientFactory.java:
##########
@@ -401,4 +411,19 @@ private static Region getS3RegionFromEndpoint(final String
endpoint,
return Region.of(AWS_S3_DEFAULT_REGION);
}
+ private static <BuilderT extends S3BaseClientBuilder<BuilderT, ClientT>,
ClientT> void
+ applyS3AccessGrantsConfigurations(BuilderT builder, Configuration conf) {
+ if (!conf.getBoolean(AWS_S3_ACCESS_GRANTS_ENABLED, false)){
+ LOG_S3AG_ENABLED.debug("S3 Access Grants plugin is not enabled.");
+ return;
+ }
+
+ LOG_S3AG_ENABLED.info("S3 Access Grants plugin is enabled.");
+ boolean isFallbackEnabled =
conf.getBoolean(AWS_S3_ACCESS_GRANTS_FALLBACK_TO_IAM_ENABLED, false);
+ S3AccessGrantsPlugin accessGrantsPlugin =
+
S3AccessGrantsPlugin.builder().enableFallback(isFallbackEnabled).build();
+ builder.addPlugin(accessGrantsPlugin);
+ LOG_S3AG_ENABLED.info("S3 Access Grants plugin is added to S3 client with
fallback: {}", isFallbackEnabled);
Review Comment:
this won't log, cause you have already used the only once on line 421. Cut
the log on 421, and just keep this one.
Update text to "S3Access Grants plugin is enabled with IAM fallback set to
{} "
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AccessGrantConfiguration.java:
##########
@@ -0,0 +1,108 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.s3a;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.test.AbstractHadoopTestBase;
+import org.junit.Assert;
+import org.junit.Test;
+
+import software.amazon.awssdk.awscore.AwsClient;
+import
software.amazon.awssdk.s3accessgrants.plugin.S3AccessGrantsIdentityProvider;
+import software.amazon.awssdk.services.s3.S3BaseClientBuilder;
+
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+
+import static org.apache.hadoop.fs.s3a.Constants.AWS_S3_ACCESS_GRANTS_ENABLED;
+
+
+/**
+ * Test S3 Access Grants configurations.
+ */
+public class TestS3AccessGrantConfiguration extends AbstractHadoopTestBase {
+ /**
+ * This credential provider will be attached to any client
+ * that has been configured with the S3 Access Grants plugin.
+ * {@link
software.amazon.awssdk.s3accessgrants.plugin.S3AccessGrantsPlugin}.
+ */
+ public static final String
S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS =
+ S3AccessGrantsIdentityProvider.class.getName();
+
+ @Test
+ public void testS3AccessGrantsEnabled() throws IOException,
URISyntaxException {
+ // Feature is explicitly enabled
+ AwsClient s3AsyncClient = getAwsClient(createConfig(true), true);
+ Assert.assertEquals(
+ S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS,
+ getCredentialProviderName(s3AsyncClient));
+
+ AwsClient s3Client = getAwsClient(createConfig(true), false);
+ Assert.assertEquals(
+ S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS,
+ getCredentialProviderName(s3Client));
+ }
+
+ @Test
+ public void testS3AccessGrantsDisabled() throws IOException,
URISyntaxException {
+ // Disabled by default
+ AwsClient s3AsyncDefaultClient = getAwsClient(new Configuration(),
true);
+ Assert.assertNotEquals(
+ S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS,
+ getCredentialProviderName(s3AsyncDefaultClient));
+
+ AwsClient s3DefaultClient = getAwsClient(new Configuration(), true);
+ Assert.assertNotEquals(
+ S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS,
+ getCredentialProviderName(s3DefaultClient));
+
+ // Disabled if explicitly set
+ AwsClient s3AsyncExplicitlyDisabledClient =
getAwsClient(createConfig(false), true);
+ Assert.assertNotEquals(
+ S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS,
+ getCredentialProviderName(s3AsyncExplicitlyDisabledClient));
+
+ AwsClient s3ExplicitlyDisabledClient =
getAwsClient(createConfig(false), true);
+ Assert.assertNotEquals(
+ S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS,
+ getCredentialProviderName(s3ExplicitlyDisabledClient));
+ }
+
+ private Configuration createConfig(boolean s3agEnabled) {
+ Configuration conf = new Configuration();
+ conf.setBoolean(AWS_S3_ACCESS_GRANTS_ENABLED, s3agEnabled);
+ return conf;
+ }
+
+ private String getCredentialProviderName(AwsClient awsClient) {
+ return
awsClient.serviceClientConfiguration().credentialsProvider().getClass().getName();
+ }
+
+ private <BuilderT extends S3BaseClientBuilder<BuilderT, ClientT>, ClientT>
AwsClient
Review Comment:
since you're returning an instance of `AwsClient` , i don't think you need
generics here?
##########
hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/index.md:
##########
@@ -614,6 +614,38 @@ If the following property is not set or set to `true`, the
following exception w
java.io.IOException: From option fs.s3a.aws.credentials.provider
java.lang.ClassNotFoundException: Class CustomCredentialsProvider not found
```
+## S3 Authorization Using S3 Access Grants
+
+[S3 Access Grants](https://aws.amazon.com/s3/features/access-grants/) can be
used to grant accesses to S3 data using IAM Principals.
+In order to enable S3 Access Grants to work with S3A, we enable the
Review Comment:
nit: Cut the "we" and "you", so just "In order to enable S3 Access Grants,
S3A uses the Access grant pugin" .. applicable to the rest of the section as
well
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AccessGrantConfiguration.java:
##########
@@ -0,0 +1,108 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.s3a;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.test.AbstractHadoopTestBase;
+import org.junit.Assert;
+import org.junit.Test;
+
+import software.amazon.awssdk.awscore.AwsClient;
+import
software.amazon.awssdk.s3accessgrants.plugin.S3AccessGrantsIdentityProvider;
+import software.amazon.awssdk.services.s3.S3BaseClientBuilder;
+
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+
+import static org.apache.hadoop.fs.s3a.Constants.AWS_S3_ACCESS_GRANTS_ENABLED;
+
+
+/**
+ * Test S3 Access Grants configurations.
+ */
+public class TestS3AccessGrantConfiguration extends AbstractHadoopTestBase {
+ /**
+ * This credential provider will be attached to any client
+ * that has been configured with the S3 Access Grants plugin.
+ * {@link
software.amazon.awssdk.s3accessgrants.plugin.S3AccessGrantsPlugin}.
+ */
+ public static final String
S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS =
+ S3AccessGrantsIdentityProvider.class.getName();
+
+ @Test
+ public void testS3AccessGrantsEnabled() throws IOException,
URISyntaxException {
+ // Feature is explicitly enabled
+ AwsClient s3AsyncClient = getAwsClient(createConfig(true), true);
+ Assert.assertEquals(
+ S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS,
+ getCredentialProviderName(s3AsyncClient));
+
+ AwsClient s3Client = getAwsClient(createConfig(true), false);
+ Assert.assertEquals(
+ S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS,
+ getCredentialProviderName(s3Client));
+ }
+
+ @Test
+ public void testS3AccessGrantsDisabled() throws IOException,
URISyntaxException {
+ // Disabled by default
+ AwsClient s3AsyncDefaultClient = getAwsClient(new Configuration(),
true);
+ Assert.assertNotEquals(
+ S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS,
+ getCredentialProviderName(s3AsyncDefaultClient));
+
+ AwsClient s3DefaultClient = getAwsClient(new Configuration(), true);
+ Assert.assertNotEquals(
+ S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS,
+ getCredentialProviderName(s3DefaultClient));
+
+ // Disabled if explicitly set
+ AwsClient s3AsyncExplicitlyDisabledClient =
getAwsClient(createConfig(false), true);
+ Assert.assertNotEquals(
+ S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS,
+ getCredentialProviderName(s3AsyncExplicitlyDisabledClient));
+
+ AwsClient s3ExplicitlyDisabledClient =
getAwsClient(createConfig(false), true);
+ Assert.assertNotEquals(
+ S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS,
+ getCredentialProviderName(s3ExplicitlyDisabledClient));
+ }
+
+ private Configuration createConfig(boolean s3agEnabled) {
Review Comment:
I think you'll need to do `removeBaseAndBucketOverrides` here before setting
the value.
and is there a way to check for if the IAM fallback is set on the client?
##########
hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/index.md:
##########
@@ -614,6 +614,38 @@ If the following property is not set or set to `true`, the
following exception w
java.io.IOException: From option fs.s3a.aws.credentials.provider
java.lang.ClassNotFoundException: Class CustomCredentialsProvider not found
```
+## S3 Authorization Using S3 Access Grants
Review Comment:
few checkstyle errors in this section
> Add S3 Access Grants Support in S3A
> -----------------------------------
>
> Key: HADOOP-19050
> URL: https://issues.apache.org/jira/browse/HADOOP-19050
> Project: Hadoop Common
> Issue Type: New Feature
> Components: fs/s3
> Affects Versions: 3.4.0
> Reporter: Jason Han
> Assignee: Jason Han
> Priority: Minor
> Labels: pull-request-available
>
> Add support for S3 Access Grants
> (https://aws.amazon.com/s3/features/access-grants/) in S3A.
--
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
