You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by "jxhan3 (via GitHub)" <gi...@apache.org> on 2024/01/27 22:14:23 UTC
[PR] HADOOP-17351, Add Support for AWS S3 Access Grants [hadoop]
jxhan3 opened a new pull request, #6507:
URL: https://github.com/apache/hadoop/pull/6507
<!--
Thanks for sending a pull request!
1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute
2. Make sure your PR title starts with JIRA issue id, e.g., 'HADOOP-17799. Your PR title ...'.
-->
### Description of PR
Add support for AWS S3 Access Grants(https://aws.amazon.com/s3/features/access-grants/)
### How was this patch tested?
Run all integration tests with scale, assume role and KMS.
### For code changes:
- [Y] Does the title or this PR starts with the corresponding JIRA issue id (e.g. 'HADOOP-17799. Your PR title ...')?
- [Y] Object storage: have the integration tests been executed and the endpoint declared according to the connector-specific documentation?
- [Y] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [Y] If applicable, have you updated the `LICENSE`, `LICENSE-binary`, `NOTICE-binary` files?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#issuecomment-1919823492
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 31s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 1s | | detect-secrets was not available. |
| +0 :ok: | xmllint | 0m 1s | | xmllint was not available. |
| +0 :ok: | shelldocs | 0m 1s | | Shelldocs was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 15m 5s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 30m 55s | | trunk passed |
| +1 :green_heart: | compile | 16m 15s | | trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | compile | 14m 52s | | trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +1 :green_heart: | checkstyle | 4m 13s | | trunk passed |
| +1 :green_heart: | mvnsite | 18m 22s | | trunk passed |
| +1 :green_heart: | javadoc | 8m 29s | | trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 7m 29s | | trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +0 :ok: | spotbugs | 0m 19s | | branch/hadoop-project no spotbugs output file (spotbugsXml.xml) |
| +1 :green_heart: | shadedclient | 60m 46s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 1m 35s | | Maven dependency ordering for patch |
| -1 :x: | mvninstall | 0m 20s | [/patch-mvninstall-hadoop-tools_hadoop-aws.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/3/artifact/out/patch-mvninstall-hadoop-tools_hadoop-aws.txt) | hadoop-aws in the patch failed. |
| -1 :x: | mvninstall | 28m 12s | [/patch-mvninstall-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/3/artifact/out/patch-mvninstall-root.txt) | root in the patch failed. |
| -1 :x: | compile | 15m 13s | [/patch-compile-root-jdkUbuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/3/artifact/out/patch-compile-root-jdkUbuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04.txt) | root in the patch failed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04. |
| -1 :x: | javac | 15m 13s | [/patch-compile-root-jdkUbuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/3/artifact/out/patch-compile-root-jdkUbuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04.txt) | root in the patch failed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04. |
| -1 :x: | compile | 14m 31s | [/patch-compile-root-jdkPrivateBuild-1.8.0_392-8u392-ga-1~20.04-b08.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/3/artifact/out/patch-compile-root-jdkPrivateBuild-1.8.0_392-8u392-ga-1~20.04-b08.txt) | root in the patch failed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08. |
| -1 :x: | javac | 14m 31s | [/patch-compile-root-jdkPrivateBuild-1.8.0_392-8u392-ga-1~20.04-b08.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/3/artifact/out/patch-compile-root-jdkPrivateBuild-1.8.0_392-8u392-ga-1~20.04-b08.txt) | root in the patch failed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08. |
| +1 :green_heart: | blanks | 0m 1s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 4m 6s | | the patch passed |
| -1 :x: | mvnsite | 4m 36s | [/patch-mvnsite-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/3/artifact/out/patch-mvnsite-root.txt) | root in the patch failed. |
| +1 :green_heart: | shellcheck | 0m 0s | | No new issues. |
| +1 :green_heart: | javadoc | 8m 30s | | the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 7m 29s | | the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +0 :ok: | spotbugs | 0m 19s | | hadoop-project has no data from spotbugs |
| -1 :x: | spotbugs | 0m 24s | [/patch-spotbugs-hadoop-tools_hadoop-aws.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/3/artifact/out/patch-spotbugs-hadoop-tools_hadoop-aws.txt) | hadoop-aws in the patch failed. |
| -1 :x: | spotbugs | 26m 34s | [/patch-spotbugs-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/3/artifact/out/patch-spotbugs-root.txt) | root in the patch failed. |
| +1 :green_heart: | shadedclient | 31m 43s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 740m 5s | [/patch-unit-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/3/artifact/out/patch-unit-root.txt) | root in the patch failed. |
| +1 :green_heart: | asflicense | 1m 32s | | The patch does not generate ASF License warnings. |
| | | 1071m 24s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/3/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/6507 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient codespell detsecrets xmllint spotbugs checkstyle shellcheck shelldocs |
| uname | Linux 5c8a8be24ec9 5.15.0-91-generic #101-Ubuntu SMP Tue Nov 14 13:30:08 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / a35a0f8aa2bf4d7553f40606daba4830de8244a9 |
| Default Java | Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/3/testReport/ |
| Max. process+thread count | 4504 (vs. ulimit of 5500) |
| modules | C: hadoop-project hadoop-tools/hadoop-aws . U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/3/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 shellcheck=0.7.0 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "adnanhemani (via GitHub)" <gi...@apache.org>.
adnanhemani commented on code in PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#discussion_r1473617958
##########
hadoop-tools/hadoop-aws/pom.xml:
##########
@@ -508,6 +508,29 @@
<artifactId>bundle</artifactId>
<scope>compile</scope>
</dependency>
+ <dependency>
+ <groupId>software.amazon.s3.accessgrants</groupId>
Review Comment:
This new JAR should be optional - and as per the original instructions the AWS S3 Access Grants team was given by the AWS Java SDK team, this plugin should not be part of the AWS Java SDK nor the SDK bundle. The reasoning behind this was that Java SDKv2 Plugins should be considered as "open source" for the most part as they are only interfaces that anyone can implement and then use wherever they'd like. In other words, the S3 Access Grants plugin should be, in theory, treated as any other open source dependency that we would be utilizing if a customer explicitly enables this in S3A.
So, to further answer the question, we need to find a way to optionally load these classes if a user specifies that they'd like to use the plugin AND provides the JAR on the classpath. That is missing from this PR as of now and @jxhan3 and I will work on it. I think @ahmarsuhail's link above has a good call pattern for doing this - we'll follow this unless you have any other suggestion: https://github.com/apache/hadoop/pull/6164/files#diff-c76d380f28cd282404a2b7110a6ea76bf2edd7277ed09639a2af594171b07efaR53
One interesting thing to note - I've seen the `S3ExpressPlugin` being merged into the AWS Java SDK (which was explicitly not the recommended option by the AWS Java SDK team, per my understanding). I've started further inquiries to find why that's the case - and how this is different than S3 Access Grants. Will report my findings here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "ahmarsuhail (via GitHub)" <gi...@apache.org>.
ahmarsuhail commented on code in PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#discussion_r1478502544
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/DefaultS3ClientFactory.java:
##########
@@ -401,4 +409,32 @@ private static Region getS3RegionFromEndpoint(final String endpoint,
return Region.of(AWS_S3_DEFAULT_REGION);
}
+ public static <BuilderT extends S3BaseClientBuilder<BuilderT, ClientT>, ClientT> void
Review Comment:
method can be private?
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/tools/S3AccessGrantsUtil.java:
##########
@@ -0,0 +1,60 @@
+package org.apache.hadoop.fs.s3a.tools;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.s3a.DefaultS3ClientFactory;
+import org.apache.hadoop.fs.store.LogExactlyOnce;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import software.amazon.awssdk.s3accessgrants.plugin.S3AccessGrantsPlugin;
+import software.amazon.awssdk.services.s3.S3BaseClientBuilder;
+
+import static org.apache.hadoop.fs.s3a.Constants.AWS_S3_ACCESS_GRANTS_FALLBACK_TO_IAM_ENABLED;
+
+public class S3AccessGrantsUtil {
+
+ protected static final Logger LOG =
+ LoggerFactory.getLogger(S3AccessGrantsUtil.class);
+
+ private static final LogExactlyOnce LOG_EXACTLY_ONCE = new LogExactlyOnce(LOG);
Review Comment:
rename from `LOG_EXACTLY_ONCE` to what this log is actually for, eg: `IAM_FALLBACK_WARN`. look at `WARN_OF_DEFAULT_REGION_CHAIN` in DefaultS3ClientFactory as an example.
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/tools/S3AccessGrantsUtil.java:
##########
@@ -0,0 +1,60 @@
+package org.apache.hadoop.fs.s3a.tools;
+
+import org.apache.hadoop.conf.Configuration;
Review Comment:
add apache license to the top of this class (copy it over from any other class)
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/tools/S3AccessGrantsUtil.java:
##########
@@ -0,0 +1,60 @@
+package org.apache.hadoop.fs.s3a.tools;
+
Review Comment:
wrong package for this class, move to the impl package.
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AccessGrantConfiguration.java:
##########
@@ -0,0 +1,89 @@
+/*
+ * 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.Test;
+
+import software.amazon.awssdk.services.s3.S3AsyncClient;
+import software.amazon.awssdk.services.s3.S3BaseClientBuilder;
+import software.amazon.awssdk.services.s3.S3Client;
+
+import static org.apache.hadoop.fs.s3a.Constants.AWS_S3_ACCESS_GRANTS_ENABLED;
+import static org.junit.Assert.assertEquals;
+
+
+/**
+ * Test S3 Access Grants configurations.
+ */
+public class TestS3AccessGrantConfiguration extends AbstractHadoopTestBase {
+
+ @Test
+ public void testS3AccessGrantsEnabled() {
+ applyVerifyS3AGPlugin(S3Client.builder(), false, true);
+ }
+
+ @Test
+ public void testS3AccessGrantsEnabledAsync() {
+ applyVerifyS3AGPlugin(S3AsyncClient.builder(), false, true);
+ }
+
+ @Test
+ public void testS3AccessGrantsDisabled() {
+ applyVerifyS3AGPlugin(S3Client.builder(), false, false);
+ }
+
+ @Test
+ public void testS3AccessGrantsDisabledByDefault() {
+ applyVerifyS3AGPlugin(S3Client.builder(), true, false);
+ }
+
+ @Test
+ public void testS3AccessGrantsDisabledAsync() {
+ applyVerifyS3AGPlugin(S3AsyncClient.builder(), false, false);
+ }
+
+ @Test
+ public void testS3AccessGrantsDisabledByDefaultAsync() {
+ applyVerifyS3AGPlugin(S3AsyncClient.builder(), true, false);
+ }
+
+ private Configuration createConfig(boolean isDefault, boolean s3agEnabled) {
+ Configuration conf = new Configuration();
+ if (!isDefault){
+ conf.setBoolean(AWS_S3_ACCESS_GRANTS_ENABLED, s3agEnabled);
+ }
+ return conf;
+ }
+
+ private <BuilderT extends S3BaseClientBuilder<BuilderT, ClientT>, ClientT> void
+ applyVerifyS3AGPlugin(BuilderT builder, boolean isDefault, boolean enabled) {
+ DefaultS3ClientFactory.applyS3AccessGrantsConfigurations(builder, createConfig(isDefault, enabled));
+ if (enabled){
+ assertEquals(1, builder.plugins().size());
+ assertEquals("software.amazon.awssdk.s3accessgrants.plugin.S3AccessGrantsPlugin",
+ builder.plugins().get(0).getClass().getName()
+ );
+ }
+ else {
+ assertEquals(builder.plugins().size(), 0);
+ }
+ }
+}
Review Comment:
add new line after class
##########
hadoop-tools/hadoop-aws/pom.xml:
##########
@@ -508,6 +508,29 @@
<artifactId>bundle</artifactId>
<scope>compile</scope>
</dependency>
+ <dependency>
Review Comment:
these should all be moved into the pom.xml in hadoop-project as that where we define dependencies. look at how we import the sdk dependency in this pom.xml for example.
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/DefaultS3ClientFactory.java:
##########
@@ -401,4 +409,32 @@ private static Region getS3RegionFromEndpoint(final String endpoint,
return Region.of(AWS_S3_DEFAULT_REGION);
}
+ public static <BuilderT extends S3BaseClientBuilder<BuilderT, ClientT>, ClientT> void
+ applyS3AccessGrantsConfigurations(BuilderT builder, Configuration conf) {
+ boolean s3agEnabled = conf.getBoolean(AWS_S3_ACCESS_GRANTS_ENABLED, false);
+ if (!s3agEnabled){
+ LOG_EXACTLY_ONCE.debug("s3ag plugin is not enabled.");
Review Comment:
this won't work, so basically it will log `s3ag plugin is not enabled.` once, but then your `LOG_EXACTLY_ONCE.debug(
"Class {} is not found exception: {}."` and wherever else you use it will never log. If you want to log those they will need their own instances of log exactly once.
But also these are debug logs, you don't need to use log exactly once. We do that more when we need to make user logs clearer when logging at `warn` or `info`. You can just use a regular logger here in my opinion.
I also don't think you need this `s3ag plugin is not enabled.` log specifically. it's getting logged for any who is not using S3AG, so not adding any value. instead maybe add a log in the try block `Configuring S3 access grants plugin`
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/tools/S3AccessGrantsUtil.java:
##########
@@ -0,0 +1,60 @@
+package org.apache.hadoop.fs.s3a.tools;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.s3a.DefaultS3ClientFactory;
+import org.apache.hadoop.fs.store.LogExactlyOnce;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import software.amazon.awssdk.s3accessgrants.plugin.S3AccessGrantsPlugin;
+import software.amazon.awssdk.services.s3.S3BaseClientBuilder;
+
+import static org.apache.hadoop.fs.s3a.Constants.AWS_S3_ACCESS_GRANTS_FALLBACK_TO_IAM_ENABLED;
+
+public class S3AccessGrantsUtil {
+
+ protected static final Logger LOG =
+ LoggerFactory.getLogger(S3AccessGrantsUtil.class);
+
+ private static final LogExactlyOnce LOG_EXACTLY_ONCE = new LogExactlyOnce(LOG);
+ private static final String S3AG_PLUGIN_CLASSNAME =
+ "software.amazon.awssdk.s3accessgrants.plugin.S3AccessGrantsPlugin";
+
+ /**
+ * S3 Access Grants plugin availability.
+ */
+ private static final boolean S3AG_PLUGIN_FOUND = checkForS3AGPlugin();
+
+ private static boolean checkForS3AGPlugin() {
+ try {
+ ClassLoader cl = DefaultS3ClientFactory.class.getClassLoader();
+ cl.loadClass(S3AG_PLUGIN_CLASSNAME);
+ LOG.debug("S3AG plugin class {} found", S3AG_PLUGIN_CLASSNAME);
+ return true;
+ } catch (Exception e) {
+ LOG.debug("S3AG plugin class {} not found", S3AG_PLUGIN_CLASSNAME, e);
+ return false;
+ }
+ }
+
+ /**
+ * Is the S3AG plugin available?
+ * @return true if it was found in the classloader
+ */
+ private static synchronized boolean isS3AGPluginAvailable() {
+ return S3AG_PLUGIN_FOUND;
+ }
+
+ public static <BuilderT extends S3BaseClientBuilder<BuilderT, ClientT>, ClientT> void
+ applyS3AccessGrantsConfigurations(BuilderT builder, Configuration conf) {
+ if (isS3AGPluginAvailable()) {
+ boolean s3agFallbackEnabled = conf.getBoolean(
+ AWS_S3_ACCESS_GRANTS_FALLBACK_TO_IAM_ENABLED, false);
+ S3AccessGrantsPlugin accessGrantsPlugin =
+ S3AccessGrantsPlugin.builder().enableFallback(s3agFallbackEnabled).build();
+ builder.addPlugin(accessGrantsPlugin);
+ LOG_EXACTLY_ONCE.info("s3ag plugin is added to s3 client with fallback: {}", s3agFallbackEnabled);
Review Comment:
and +1 on @adnanhemani 's comment, let's make these logs uniform so they begin with "S3 access grant ... "
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/tools/S3AccessGrantsUtil.java:
##########
@@ -0,0 +1,60 @@
+package org.apache.hadoop.fs.s3a.tools;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.s3a.DefaultS3ClientFactory;
+import org.apache.hadoop.fs.store.LogExactlyOnce;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import software.amazon.awssdk.s3accessgrants.plugin.S3AccessGrantsPlugin;
+import software.amazon.awssdk.services.s3.S3BaseClientBuilder;
+
+import static org.apache.hadoop.fs.s3a.Constants.AWS_S3_ACCESS_GRANTS_FALLBACK_TO_IAM_ENABLED;
+
+public class S3AccessGrantsUtil {
+
+ protected static final Logger LOG =
+ LoggerFactory.getLogger(S3AccessGrantsUtil.class);
+
+ private static final LogExactlyOnce LOG_EXACTLY_ONCE = new LogExactlyOnce(LOG);
+ private static final String S3AG_PLUGIN_CLASSNAME =
+ "software.amazon.awssdk.s3accessgrants.plugin.S3AccessGrantsPlugin";
+
+ /**
+ * S3 Access Grants plugin availability.
+ */
+ private static final boolean S3AG_PLUGIN_FOUND = checkForS3AGPlugin();
+
+ private static boolean checkForS3AGPlugin() {
+ try {
+ ClassLoader cl = DefaultS3ClientFactory.class.getClassLoader();
+ cl.loadClass(S3AG_PLUGIN_CLASSNAME);
+ LOG.debug("S3AG plugin class {} found", S3AG_PLUGIN_CLASSNAME);
+ return true;
+ } catch (Exception e) {
+ LOG.debug("S3AG plugin class {} not found", S3AG_PLUGIN_CLASSNAME, e);
+ return false;
+ }
+ }
+
+ /**
+ * Is the S3AG plugin available?
+ * @return true if it was found in the classloader
+ */
+ private static synchronized boolean isS3AGPluginAvailable() {
+ return S3AG_PLUGIN_FOUND;
+ }
+
+ public static <BuilderT extends S3BaseClientBuilder<BuilderT, ClientT>, ClientT> void
+ applyS3AccessGrantsConfigurations(BuilderT builder, Configuration conf) {
+ if (isS3AGPluginAvailable()) {
+ boolean s3agFallbackEnabled = conf.getBoolean(
+ AWS_S3_ACCESS_GRANTS_FALLBACK_TO_IAM_ENABLED, false);
+ S3AccessGrantsPlugin accessGrantsPlugin =
+ S3AccessGrantsPlugin.builder().enableFallback(s3agFallbackEnabled).build();
+ builder.addPlugin(accessGrantsPlugin);
+ LOG_EXACTLY_ONCE.info("s3ag plugin is added to s3 client with fallback: {}", s3agFallbackEnabled);
Review Comment:
Have two different Loggers that log exactly once for these two different statements.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "jxhan3 (via GitHub)" <gi...@apache.org>.
jxhan3 commented on PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#issuecomment-1922460753
Local verification is running, update soon.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-17351, Add Support for AWS S3 Access Grants [hadoop]
Posted by "jxhan3 (via GitHub)" <gi...@apache.org>.
jxhan3 commented on PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#issuecomment-1915850470
The failed junit test is not related to this change. (hadoop.yarn.server.timelineservice.security.TestTimelineAuthFilterForV2)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "adnanhemani (via GitHub)" <gi...@apache.org>.
adnanhemani commented on code in PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#discussion_r1473617958
##########
hadoop-tools/hadoop-aws/pom.xml:
##########
@@ -508,6 +508,29 @@
<artifactId>bundle</artifactId>
<scope>compile</scope>
</dependency>
+ <dependency>
+ <groupId>software.amazon.s3.accessgrants</groupId>
Review Comment:
This new JAR should be optional - and as per the original instructions the AWS S3 Access Grants team was given by the AWS Java SDK team, this plugin should not be part of the AWS Java SDK nor the SDK bundle. The reasoning behind this was that Java SDKv2 Plugins should be considered as "open source" for the most part as they are only interfaces that anyone can implement and then use wherever they'd like. In other words, the S3 Access Grants plugin should be, in theory, treated as any other open source dependency that we would be utilizing if a customer explicitly enables this in S3A.
So, to further answer the question, we need to find a way to optionally load these classes if a user specifies that they'd like to use the plugin AND provides the JAR on the classpath. That is missing from this PR as of now and @jxhan3 and I will work on it. I think @ahmarsuhail's [link above](https://github.com/apache/hadoop/pull/6507#discussion_r1472930404) has a good call pattern for doing this - we'll follow this unless you have any other suggestion.
One interesting thing to note - I've seen the `S3ExpressPlugin` being merged into the AWS Java SDK (which was explicitly not the recommended option by the AWS Java SDK team, per my understanding). I've started further inquiries to find why that's the case - and how this is different than S3 Access Grants. Will report my findings here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "adnanhemani (via GitHub)" <gi...@apache.org>.
adnanhemani commented on PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#issuecomment-1935463310
This has been evolved into #6544. This PR will no longer be used now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#issuecomment-1928684496
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 30s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +0 :ok: | xmllint | 0m 0s | | xmllint was not available. |
| +0 :ok: | markdownlint | 0m 0s | | markdownlint was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 14m 26s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 30m 37s | | trunk passed |
| +1 :green_heart: | compile | 16m 16s | | trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | compile | 14m 41s | | trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +1 :green_heart: | checkstyle | 4m 41s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 32s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 12s | | trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 1m 17s | | trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +0 :ok: | spotbugs | 0m 41s | | branch/hadoop-project no spotbugs output file (spotbugsXml.xml) |
| +1 :green_heart: | shadedclient | 32m 58s | | branch has no errors when building and testing our client artifacts. |
| -0 :warning: | patch | 33m 22s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 34s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 43s | | the patch passed |
| +1 :green_heart: | compile | 15m 33s | | the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javac | 15m 33s | | the patch passed |
| +1 :green_heart: | compile | 15m 13s | | the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +1 :green_heart: | javac | 15m 13s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 4m 4s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/10/artifact/out/results-checkstyle-root.txt) | root: The patch generated 7 new + 2 unchanged - 0 fixed = 9 total (was 2) |
| +1 :green_heart: | mvnsite | 1m 25s | | the patch passed |
| +1 :green_heart: | javadoc | 1m 8s | | the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 1m 9s | | the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +0 :ok: | spotbugs | 0m 34s | | hadoop-project has no data from spotbugs |
| +1 :green_heart: | shadedclient | 34m 27s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 32s | | hadoop-project in the patch passed. |
| +1 :green_heart: | unit | 3m 10s | | hadoop-aws in the patch passed. |
| +1 :green_heart: | asflicense | 0m 56s | | The patch does not generate ASF License warnings. |
| | | 207m 7s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/10/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/6507 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient codespell detsecrets xmllint spotbugs checkstyle markdownlint |
| uname | Linux e6a0ac0030c9 5.15.0-88-generic #98-Ubuntu SMP Mon Oct 2 15:18:56 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / dcbab4bc5973ef4886ebe3c350160fcb7f9df0f6 |
| Default Java | Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/10/testReport/ |
| Max. process+thread count | 550 (vs. ulimit of 5500) |
| modules | C: hadoop-project hadoop-tools/hadoop-aws U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/10/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#issuecomment-1928393471
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 30s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +0 :ok: | xmllint | 0m 0s | | xmllint was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 14m 34s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 30m 49s | | trunk passed |
| +1 :green_heart: | compile | 16m 23s | | trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | compile | 15m 4s | | trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +1 :green_heart: | checkstyle | 4m 12s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 31s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 15s | | trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 1m 20s | | trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +0 :ok: | spotbugs | 0m 42s | | branch/hadoop-project no spotbugs output file (spotbugsXml.xml) |
| +1 :green_heart: | shadedclient | 32m 57s | | branch has no errors when building and testing our client artifacts. |
| -0 :warning: | patch | 33m 21s | | Used diff version of patch file. Binary files and potentially other changes not applied. Please rebase and squash commits if necessary. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 33s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 44s | | the patch passed |
| +1 :green_heart: | compile | 15m 37s | | the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javac | 15m 37s | | the patch passed |
| +1 :green_heart: | compile | 14m 50s | | the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +1 :green_heart: | javac | 14m 50s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 4m 2s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/9/artifact/out/results-checkstyle-root.txt) | root: The patch generated 7 new + 2 unchanged - 0 fixed = 9 total (was 2) |
| +1 :green_heart: | mvnsite | 1m 28s | | the patch passed |
| +1 :green_heart: | javadoc | 1m 12s | | the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 1m 20s | | the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +0 :ok: | spotbugs | 0m 33s | | hadoop-project has no data from spotbugs |
| +1 :green_heart: | shadedclient | 33m 14s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 34s | | hadoop-project in the patch passed. |
| +1 :green_heart: | unit | 3m 12s | | hadoop-aws in the patch passed. |
| +1 :green_heart: | asflicense | 0m 57s | | The patch does not generate ASF License warnings. |
| | | 206m 37s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/9/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/6507 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient codespell detsecrets xmllint spotbugs checkstyle |
| uname | Linux e44bc4938765 5.15.0-91-generic #101-Ubuntu SMP Tue Nov 14 13:30:08 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 2bc7c41b6f41fe5ba3ab29e52c58ab538d1bcfa1 |
| Default Java | Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/9/testReport/ |
| Max. process+thread count | 625 (vs. ulimit of 5500) |
| modules | C: hadoop-project hadoop-tools/hadoop-aws U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/9/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050. SDK Add Support for AWS S3 Access Grants [hadoop]
Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran closed pull request #6507: HADOOP-19050. SDK Add Support for AWS S3 Access Grants
URL: https://github.com/apache/hadoop/pull/6507
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "adnanhemani (via GitHub)" <gi...@apache.org>.
adnanhemani commented on PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#issuecomment-1926257843
Hi @steveloughran and @ahmarsuhail - I think this code is in a much more ready state than before and we've attempted to answer the questions you had earlier. Please let us know what other thoughts you have on this.
(Also, I think the Unit Tests are a bit flaky here - not sure what to do about those)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "jxhan3 (via GitHub)" <gi...@apache.org>.
jxhan3 commented on code in PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#discussion_r1478681708
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/DefaultS3ClientFactory.java:
##########
@@ -401,4 +409,32 @@ private static Region getS3RegionFromEndpoint(final String endpoint,
return Region.of(AWS_S3_DEFAULT_REGION);
}
+ public static <BuilderT extends S3BaseClientBuilder<BuilderT, ClientT>, ClientT> void
Review Comment:
This is for testing purpose, otherwise we may need to use reflection to test private method. Please share your thoughts on this. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#issuecomment-1924323299
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 35s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +0 :ok: | xmllint | 0m 0s | | xmllint was not available. |
| +0 :ok: | shelldocs | 0m 0s | | Shelldocs was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 14m 32s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 30m 58s | | trunk passed |
| +1 :green_heart: | compile | 16m 15s | | trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | compile | 14m 50s | | trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +1 :green_heart: | checkstyle | 4m 45s | | trunk passed |
| +1 :green_heart: | mvnsite | 18m 39s | | trunk passed |
| +1 :green_heart: | javadoc | 8m 27s | | trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 7m 29s | | trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +0 :ok: | spotbugs | 0m 19s | | branch/hadoop-project no spotbugs output file (spotbugsXml.xml) |
| +1 :green_heart: | shadedclient | 60m 30s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 34s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 28m 43s | | the patch passed |
| +1 :green_heart: | compile | 15m 39s | | the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javac | 15m 39s | | the patch passed |
| +1 :green_heart: | compile | 14m 51s | | the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +1 :green_heart: | javac | 14m 51s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 4m 3s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/5/artifact/out/results-checkstyle-root.txt) | root: The patch generated 5 new + 2 unchanged - 0 fixed = 7 total (was 2) |
| +1 :green_heart: | mvnsite | 13m 33s | | the patch passed |
| +1 :green_heart: | shellcheck | 0m 0s | | No new issues. |
| +1 :green_heart: | javadoc | 8m 24s | | the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 7m 32s | | the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +0 :ok: | spotbugs | 0m 19s | | hadoop-project has no data from spotbugs |
| +1 :green_heart: | shadedclient | 61m 1s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 746m 38s | [/patch-unit-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/5/artifact/out/patch-unit-root.txt) | root in the patch passed. |
| +1 :green_heart: | asflicense | 1m 35s | | The patch does not generate ASF License warnings. |
| | | 1090m 33s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hdfs.server.datanode.TestDirectoryScanner |
| | hadoop.hdfs.TestRollingUpgrade |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/5/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/6507 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient codespell detsecrets xmllint spotbugs checkstyle shellcheck shelldocs |
| uname | Linux 2741712011d2 5.15.0-91-generic #101-Ubuntu SMP Tue Nov 14 13:30:08 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 167fecd265d127c1618f0a887f9de348599c632f |
| Default Java | Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/5/testReport/ |
| Max. process+thread count | 4193 (vs. ulimit of 5500) |
| modules | C: hadoop-project hadoop-tools/hadoop-aws . U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/5/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 shellcheck=0.7.0 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "jxhan3 (via GitHub)" <gi...@apache.org>.
jxhan3 commented on PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#issuecomment-1925038842
Test optional plugin:
1. if plugin is in class path, same result
2. if plugin is not in class path:
```
hadoop@ip-172-31-35-222 ~]$ hadoop fs -ls s3a://s3ag-pentests-us-east-1/s3ag-test/read-only/sample-pyspark.py
2024-02-03 03:16:01,189 INFO impl.MetricsConfig: Loaded properties from hadoop-metrics2.properties
2024-02-03 03:16:01,270 INFO impl.MetricsSystemImpl: Scheduled Metric snapshot period at 300 second(s).
2024-02-03 03:16:01,270 INFO impl.MetricsSystemImpl: s3a-file-system metrics system started
2024-02-03 03:16:01,348 WARN impl.ConfigurationHelper: Option fs.s3a.connection.establish.timeout is too low (5,000 ms). Setting to 15,000 ms instead
2024-02-03 03:16:01,510 WARN tools.S3AccessGrantsUtil: s3ag plugin is not available.
2024-02-03 03:16:01,975 WARN tools.S3AccessGrantsUtil: s3ag plugin is not available.
ls: s3a://s3ag-pentests-us-east-1/s3ag-test/read-only/sample-pyspark.py: getFileStatus on s3a://s3ag-pentests-us-east-1/s3ag-test/read-only/sample-pyspark.py: software.amazon.awssdk.services.s3.model.S3Exception: null (Service: S3, Status Code: 403, Request ID: 4Y02S3N6HM5A1NBQ, Extended Request ID: Kish6CPduYPD6y/2+iSeW8ZlcPJGLJ/Ef0Q9lhN7eVX+OG8TkvpMpgaXSy27tg5pJEnJIiyuugA=):null
2024-02-03 03:16:04,593 INFO impl.MetricsSystemImpl: Stopping s3a-file-system metrics system...
2024-02-03 03:16:04,593 INFO impl.MetricsSystemImpl: s3a-file-system metrics system stopped.
2024-02-03 03:16:04,594 INFO impl.MetricsSystemImpl: s3a-file-system metrics system shutdown complete.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "ahmarsuhail (via GitHub)" <gi...@apache.org>.
ahmarsuhail commented on code in PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#discussion_r1472858212
##########
hadoop-project/pom.xml:
##########
@@ -187,7 +187,7 @@
<make-maven-plugin.version>1.0-beta-1</make-maven-plugin.version>
<surefire.fork.timeout>900</surefire.fork.timeout>
<aws-java-sdk.version>1.12.599</aws-java-sdk.version>
- <aws-java-sdk-v2.version>2.23.5</aws-java-sdk-v2.version>
Review Comment:
cut, SDK upgrade needs to happen separately
##########
hadoop-tools/hadoop-aws/pom.xml:
##########
@@ -508,6 +508,29 @@
<artifactId>bundle</artifactId>
<scope>compile</scope>
</dependency>
+ <dependency>
Review Comment:
@steveloughran do you have any advice here? I think we should do what we did for Client Side Encryption, have this S3 access grants jar as optional, and create a new client factory which will add the S3 access grants plugin.
If there are other plugins that we want to add in the future, this new client factory can be generalised for that.
##########
hadoop-tools/hadoop-aws/pom.xml:
##########
@@ -508,6 +508,29 @@
<artifactId>bundle</artifactId>
<scope>compile</scope>
</dependency>
+ <dependency>
Review Comment:
this should go in hadoop-project, and then set the dependency here. We should probably also use `provided` scope, so the jar is optional.
See this PR, which added the client side encryption for example https://github.com/apache/hadoop/pull/6164 and [this](https://github.com/apache/hadoop/pull/6164/files#diff-c76d380f28cd282404a2b7110a6ea76bf2edd7277ed09639a2af594171b07efaR53) class which checks if the class exists
##########
LICENSE-binary:
##########
@@ -363,7 +363,7 @@ org.objenesis:objenesis:2.6
org.xerial.snappy:snappy-java:1.1.10.4
org.yaml:snakeyaml:2.0
org.wildfly.openssl:wildfly-openssl:1.1.3.Final
-software.amazon.awssdk:bundle:jar:2.23.5
Review Comment:
this shouldn't be here. it's already part of your SDK upgrade PR so cut from here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-17351, Add Support for AWS S3 Access Grants [hadoop]
Posted by "jxhan3 (via GitHub)" <gi...@apache.org>.
jxhan3 commented on PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#issuecomment-1915909095
Trunk compile tests all passed, but some of the patch compile tests failed. Could someone help to give some pointer how to fix this? Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#issuecomment-1925460913
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 34s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +0 :ok: | xmllint | 0m 0s | | xmllint was not available. |
| +0 :ok: | shelldocs | 0m 0s | | Shelldocs was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 14m 31s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 30m 32s | | trunk passed |
| +1 :green_heart: | compile | 16m 20s | | trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | compile | 14m 53s | | trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +1 :green_heart: | checkstyle | 4m 12s | | trunk passed |
| +1 :green_heart: | mvnsite | 18m 14s | | trunk passed |
| +1 :green_heart: | javadoc | 8m 30s | | trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 7m 34s | | trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +0 :ok: | spotbugs | 0m 19s | | branch/hadoop-project no spotbugs output file (spotbugsXml.xml) |
| +1 :green_heart: | shadedclient | 60m 19s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 33s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 29m 2s | | the patch passed |
| +1 :green_heart: | compile | 15m 55s | | the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javac | 15m 55s | | the patch passed |
| +1 :green_heart: | compile | 14m 46s | | the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +1 :green_heart: | javac | 14m 46s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 4m 14s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/8/artifact/out/results-checkstyle-root.txt) | root: The patch generated 7 new + 2 unchanged - 0 fixed = 9 total (was 2) |
| +1 :green_heart: | mvnsite | 13m 51s | | the patch passed |
| +1 :green_heart: | shellcheck | 0m 0s | | No new issues. |
| +1 :green_heart: | javadoc | 8m 21s | | the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 7m 34s | | the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +0 :ok: | spotbugs | 0m 19s | | hadoop-project has no data from spotbugs |
| +1 :green_heart: | shadedclient | 62m 14s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 742m 0s | [/patch-unit-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/8/artifact/out/patch-unit-root.txt) | root in the patch passed. |
| -1 :x: | asflicense | 1m 35s | [/results-asflicense.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/8/artifact/out/results-asflicense.txt) | The patch generated 1 ASF License warnings. |
| | | 1086m 27s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hdfs.server.datanode.TestDirectoryScanner |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/8/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/6507 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient codespell detsecrets xmllint spotbugs checkstyle shellcheck shelldocs |
| uname | Linux be58e8be39f5 5.15.0-88-generic #98-Ubuntu SMP Mon Oct 2 15:18:56 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / b404c2db253f9adc3de578ba63c1af2be13865d2 |
| Default Java | Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/8/testReport/ |
| Max. process+thread count | 4013 (vs. ulimit of 5500) |
| modules | C: hadoop-project hadoop-tools/hadoop-aws . U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/8/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 shellcheck=0.7.0 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "jxhan3 (via GitHub)" <gi...@apache.org>.
jxhan3 commented on code in PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#discussion_r1475271643
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/DefaultS3ClientFactory.java:
##########
@@ -370,4 +375,18 @@ private static Region getS3RegionFromEndpoint(String endpoint) {
return Region.US_EAST_1;
}
+ public static <BuilderT extends S3BaseClientBuilder<BuilderT, ClientT>, ClientT> void
+ applyS3AccessGrantsConfigurations(BuilderT builder, Configuration conf) {
+ boolean s3agEnabled = conf.getBoolean(AWS_S3_ACCESS_GRANTS_ENABLED, false);
+ if (s3agEnabled) {
+ boolean s3agFallbackEnabled = conf.getBoolean(
+ AWS_S3_ACCESS_GRANTS_FALLBACK_TO_IAM_ENABLED, false);
+ S3AccessGrantsPlugin accessGrantsPlugin =
+ S3AccessGrantsPlugin.builder().enableFallback(s3agFallbackEnabled).build();
+ builder.addPlugin(accessGrantsPlugin);
+ LOG.info("s3ag plugin is added to s3 client with fallback: {}", s3agFallbackEnabled);
Review Comment:
changed in next version
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#issuecomment-1936006860
@adnanhemani thanks; without that change we'd have problems with the PR, as in "you get to support it all through reflection" the way we have to do with wildfly/openssl binding (NetworkBinding) and more.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "jxhan3 (via GitHub)" <gi...@apache.org>.
jxhan3 commented on code in PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#discussion_r1475155705
##########
LICENSE-binary:
##########
@@ -363,7 +363,7 @@ org.objenesis:objenesis:2.6
org.xerial.snappy:snappy-java:1.1.10.4
org.yaml:snakeyaml:2.0
org.wildfly.openssl:wildfly-openssl:1.1.3.Final
-software.amazon.awssdk:bundle:jar:2.23.5
Review Comment:
reverted in the next version
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "jxhan3 (via GitHub)" <gi...@apache.org>.
jxhan3 commented on code in PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#discussion_r1475274368
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AccessGrantConfiguration.java:
##########
@@ -0,0 +1,102 @@
+/*
+ * 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.junit.Test;
+
+import software.amazon.awssdk.services.s3.S3AsyncClientBuilder;
+import software.amazon.awssdk.services.s3.S3AsyncClient;
+import software.amazon.awssdk.services.s3.S3BaseClientBuilder;
+import software.amazon.awssdk.services.s3.S3ClientBuilder;
+import software.amazon.awssdk.services.s3.S3Client;
+
+import static org.apache.hadoop.fs.s3a.Constants.AWS_S3_ACCESS_GRANTS_ENABLED;
+import static org.junit.Assert.assertEquals;
+
+
+/**
+ * Test S3 Access Grants configurations.
+ */
+public class TestS3AccessGrantConfiguration {
+
+ @Test
+ public void testS3AccessGrantsEnabled() {
+ Configuration conf = new Configuration();
+ conf.set(AWS_S3_ACCESS_GRANTS_ENABLED, "true");
Review Comment:
changed in next version
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "adnanhemani (via GitHub)" <gi...@apache.org>.
adnanhemani commented on PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#issuecomment-1920244731
Hi @steveloughran - thanks for your time on this. We appreciate it a lot. I've responded to your comments inline below:
> We cannot have any more unshaded aws sdk jars as required on our classpath; removing s3 select in https://github.com/apache/hadoop/pull/6144 has simplified our life by removing another optional one.
>Do you have a timetable for incorporating this plugging into bundle.jar?
>Otherwise, it is critical that if the jar is not on the cross path normal S3 clients can be constructed and used.
Totally agreed with you on this. @jxhan3 and I will work to make sure this is optional as we do not have any timeline (or known plans) for getting the plugin incorporated into the AWS Java SDKv2 bundle.jar. If this changes in the future, we'd be glad to reverse any classloading code we may need for now.
>This will need documentation. Either in connecting.md or a new file in the same directory src/site/markdown/tools/hadoop-aws
Noted. @jxhan3 will work on this in the next PR revision.
>I do not see any integration tests. What is the story here? Is it possible to run the whole mvn verify test run with access grants? if so, adding a paragraph in testing.md would be good, and particular: how to set it up. I am particularly curious about how well the delegation tokens worked...are session credentials supported?
The story currently is, if we treat the plugin as yet another third-party (and optional) dependency, then this PR is only going to be providing the bare minimum code for users to be able to enable the plugin if they explicitly choose to do so. Any issues with the actual functionality of the plugin should be addressed by the plugin itself at their [open source GitHub](https://github.com/aws/aws-s3-accessgrants-plugin-java-v2). So then, the only testing that we'd require would be to ensure that if users are explicitly enabling this feature, that S3A is ensuring its S3 clients have the plugin attached. Other open-source contributions (e.g. Iceberg) have accepted this testing model - and I'd also recommend it to reduce the need for redundant test coverage between S3A and the S3 Access Grants plugin.
If you don't agree with this testing model, we can surely try to add additional ITest cases that will both setup and tear down the S3 Access Grants instance, locations, and required grants (to be noted: S3 Access Grants APIs are _not_ free) - and then test both when users should and should not be able to receive access. However, running all existing test cases under this model will be a heavy task that will likely require lots of test case refactoring, as Access Grants are defined on a location-by-location basis. In order to test both when users should and should not have access for each test case will require both additional setup and test code to ensure that those situations can be adequately tested with multiple data locations. I'm not sure that the ROI on making such a large change will be there. Please let me know your thoughts on this.
As for how the feature works, S3 Access Grants will authenticate the credentials to find the IAM user associated with it - then use that identity for the authorization before returning a new set of scoped credentials to actually access the data (in other words, the credentials that are inputted to the S3 client will not be the credentials used to actually access the data). The S3 Access Grants plugin is the mechanism that will do the entire credential vending process and using the vended credentials properly in any calls made from the attached S3 client. As such, session credentials and delegation tokens will work given that the credentials that are passed to the S3 client (using any mechanism) are valid and can be authenticated properly.
>The feature probably also needs an extra line in the "qualifying an SDK" section.
Noted. @jxhan3 will work on this in the next PR revision.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on code in PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#discussion_r1512593751
##########
hadoop-project/pom.xml:
##########
@@ -1976,6 +1976,30 @@
<artifactId>log4j-web</artifactId>
<version>${log4j2.version}</version>
</dependency>
+ <dependency>
+ <groupId>software.amazon.s3.accessgrants</groupId>
+ <artifactId>aws-s3-accessgrants-java-plugin</artifactId>
+ <version>2.0.0</version>
+ <scope>provided</scope>
+ <exclusions>
+ <exclusion>
+ <groupId>software.amazon.awssdk</groupId>
+ <artifactId>*</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>com.github.ben-manes.caffeine</groupId>
+ <artifactId>*</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.apache.logging.log4j</groupId>
+ <artifactId>*</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>org.assertj</groupId>
Review Comment:
this should really be scoped as testing in the module itself...
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/DefaultS3ClientFactory.java:
##########
@@ -401,4 +409,33 @@ private static Region getS3RegionFromEndpoint(final String endpoint,
return Region.of(AWS_S3_DEFAULT_REGION);
}
+ public static <BuilderT extends S3BaseClientBuilder<BuilderT, ClientT>, ClientT> void
+ applyS3AccessGrantsConfigurations(BuilderT builder, Configuration conf) {
+ boolean s3agEnabled = conf.getBoolean(AWS_S3_ACCESS_GRANTS_ENABLED, false);
+ if (!s3agEnabled){
+ LOG.debug("S3 Access Grants plugin is not enabled.");
+ return;
+ }
+ try {
+ LOG_S3AG_ENABLED.info("S3 Access Grants plugin is enabled.");
+ Class s3agUtil = Class.forName(S3AG_UTIL_CLASSNAME);
+ Method applyS3agConfig =
+ s3agUtil.getMethod("applyS3AccessGrantsConfigurations", S3BaseClientBuilder.class, Configuration.class);
+ applyS3agConfig.invoke(null, builder, conf);
+ } catch (ClassNotFoundException e) {
+ LOG.debug(
+ "Class {} is not found exception: {}.",
+ S3AG_UTIL_CLASSNAME,
+ e.getStackTrace()
Review Comment:
1. pass in e and let the logger handle the rest.
2. use multiple classes in the catch statement to avoid duplication/maintenance costs
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/S3AccessGrantsUtil.java:
##########
@@ -0,0 +1,77 @@
+/*
+ * 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.impl;
+
+import org.apache.hadoop.conf.Configuration;
Review Comment:
nit: import ordering. I will keep complaining about this on new classes, so learn them and we will both save time. There's a style file for intellij somewhere to help
##########
hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/s3_access_grants.md:
##########
@@ -0,0 +1,61 @@
+<!---
+ Licensed 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. See accompanying LICENSE file.
+-->
+
+# S3 Access Grants
+
+<!-- MACRO{toc|fromDepth=0|toDepth=5} -->
+
+S3 Access Grants is a credential vending service for S3 data. More information:
+* https://aws.amazon.com/s3/features/access-grants/
Review Comment:
use bracketed links with [title] as well as (url)
##########
hadoop-project/pom.xml:
##########
@@ -1976,6 +1976,30 @@
<artifactId>log4j-web</artifactId>
<version>${log4j2.version}</version>
</dependency>
+ <dependency>
+ <groupId>software.amazon.s3.accessgrants</groupId>
+ <artifactId>aws-s3-accessgrants-java-plugin</artifactId>
+ <version>2.0.0</version>
Review Comment:
can you move this version flag into a property next to the other aws s3 options, at least until this module is merged into bundle.jar. this makes it overrideable
##########
hadoop-project/pom.xml:
##########
@@ -1976,6 +1976,30 @@
<artifactId>log4j-web</artifactId>
<version>${log4j2.version}</version>
</dependency>
+ <dependency>
+ <groupId>software.amazon.s3.accessgrants</groupId>
+ <artifactId>aws-s3-accessgrants-java-plugin</artifactId>
+ <version>2.0.0</version>
+ <scope>provided</scope>
+ <exclusions>
+ <exclusion>
+ <groupId>software.amazon.awssdk</groupId>
+ <artifactId>*</artifactId>
+ </exclusion>
+ <exclusion>
+ <groupId>com.github.ben-manes.caffeine</groupId>
Review Comment:
are you confident this isn't used?
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AccessGrantConfiguration.java:
##########
@@ -0,0 +1,90 @@
+/*
+ * 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.Test;
+
+import software.amazon.awssdk.services.s3.S3AsyncClient;
+import software.amazon.awssdk.services.s3.S3BaseClientBuilder;
+import software.amazon.awssdk.services.s3.S3Client;
+
+import static org.apache.hadoop.fs.s3a.Constants.AWS_S3_ACCESS_GRANTS_ENABLED;
+import static org.junit.Assert.assertEquals;
+
+
+/**
+ * Test S3 Access Grants configurations.
+ */
+public class TestS3AccessGrantConfiguration extends AbstractHadoopTestBase {
+
+ @Test
+ public void testS3AccessGrantsEnabled() {
+ applyVerifyS3AGPlugin(S3Client.builder(), false, true);
+ }
+
+ @Test
+ public void testS3AccessGrantsEnabledAsync() {
+ applyVerifyS3AGPlugin(S3AsyncClient.builder(), false, true);
+ }
+
+ @Test
+ public void testS3AccessGrantsDisabled() {
+ applyVerifyS3AGPlugin(S3Client.builder(), false, false);
+ }
+
+ @Test
+ public void testS3AccessGrantsDisabledByDefault() {
+ applyVerifyS3AGPlugin(S3Client.builder(), true, false);
+ }
+
+ @Test
+ public void testS3AccessGrantsDisabledAsync() {
+ applyVerifyS3AGPlugin(S3AsyncClient.builder(), false, false);
+ }
+
+ @Test
+ public void testS3AccessGrantsDisabledByDefaultAsync() {
+ applyVerifyS3AGPlugin(S3AsyncClient.builder(), true, false);
+ }
+
+ private Configuration createConfig(boolean isDefault, boolean s3agEnabled) {
Review Comment:
add javadoc
##########
hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/s3_access_grants.md:
##########
@@ -0,0 +1,61 @@
+<!---
+ Licensed 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. See accompanying LICENSE file.
+-->
+
+# S3 Access Grants
+
+<!-- MACRO{toc|fromDepth=0|toDepth=5} -->
+
+S3 Access Grants is a credential vending service for S3 data. More information:
+* https://aws.amazon.com/s3/features/access-grants/
+
+In S3A, S3 Access Grants Plugin is used to support S3 Access Grants. More information:
+* https://github.com/aws/aws-s3-accessgrants-plugin-java-v2/
+
+
+
+## How to enable S3 Access Grants in S3A
+
+1. Add the `hadoop-aws` JAR on your classpath.
+
+1. Add the `aws-java-sdk-bundle.jar` JAR to your classpath, the minimum version is v2.23.7.
+
+2. Add the `aws-s3-accessgrants-java-plugin-2.0.0.jar` JAR to your classpath.
+3. Add the `caffeine.jar` JAR to your classpath.
Review Comment:
ok, if it is compulsory then leave it as an access grant dependency.
##########
hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/s3_access_grants.md:
##########
@@ -0,0 +1,61 @@
+<!---
+ Licensed 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. See accompanying LICENSE file.
+-->
+
+# S3 Access Grants
+
+<!-- MACRO{toc|fromDepth=0|toDepth=5} -->
+
+S3 Access Grants is a credential vending service for S3 data. More information:
+* https://aws.amazon.com/s3/features/access-grants/
+
+In S3A, S3 Access Grants Plugin is used to support S3 Access Grants. More information:
+* https://github.com/aws/aws-s3-accessgrants-plugin-java-v2/
+
+
+
+## How to enable S3 Access Grants in S3A
+
+1. Add the `hadoop-aws` JAR on your classpath.
+
+1. Add the `aws-java-sdk-bundle.jar` JAR to your classpath, the minimum version is v2.23.7.
Review Comment:
cut this. name is wrong and version is already out of date
##########
hadoop-tools/hadoop-aws/pom.xml:
##########
@@ -508,6 +508,29 @@
<artifactId>bundle</artifactId>
<scope>compile</scope>
</dependency>
+ <dependency>
+ <groupId>software.amazon.s3.accessgrants</groupId>
Review Comment:
@ahmarsuhail put in the work to get s3 express in
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AccessGrantConfiguration.java:
##########
@@ -0,0 +1,90 @@
+/*
+ * 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.Test;
+
+import software.amazon.awssdk.services.s3.S3AsyncClient;
+import software.amazon.awssdk.services.s3.S3BaseClientBuilder;
+import software.amazon.awssdk.services.s3.S3Client;
+
+import static org.apache.hadoop.fs.s3a.Constants.AWS_S3_ACCESS_GRANTS_ENABLED;
+import static org.junit.Assert.assertEquals;
+
+
+/**
+ * Test S3 Access Grants configurations.
+ */
+public class TestS3AccessGrantConfiguration extends AbstractHadoopTestBase {
+
+ @Test
+ public void testS3AccessGrantsEnabled() {
+ applyVerifyS3AGPlugin(S3Client.builder(), false, true);
+ }
+
+ @Test
+ public void testS3AccessGrantsEnabledAsync() {
+ applyVerifyS3AGPlugin(S3AsyncClient.builder(), false, true);
+ }
+
+ @Test
+ public void testS3AccessGrantsDisabled() {
+ applyVerifyS3AGPlugin(S3Client.builder(), false, false);
+ }
+
+ @Test
+ public void testS3AccessGrantsDisabledByDefault() {
+ applyVerifyS3AGPlugin(S3Client.builder(), true, false);
+ }
+
+ @Test
+ public void testS3AccessGrantsDisabledAsync() {
+ applyVerifyS3AGPlugin(S3AsyncClient.builder(), false, false);
+ }
+
+ @Test
+ public void testS3AccessGrantsDisabledByDefaultAsync() {
+ applyVerifyS3AGPlugin(S3AsyncClient.builder(), true, false);
+ }
+
+ private Configuration createConfig(boolean isDefault, boolean s3agEnabled) {
+ Configuration conf = new Configuration();
+ if (!isDefault){
+ conf.setBoolean(AWS_S3_ACCESS_GRANTS_ENABLED, s3agEnabled);
+ }
+ return conf;
+ }
+
+ private <BuilderT extends S3BaseClientBuilder<BuilderT, ClientT>, ClientT> void
+ applyVerifyS3AGPlugin(BuilderT builder, boolean isDefault, boolean enabled) {
+ DefaultS3ClientFactory.applyS3AccessGrantsConfigurations(builder, createConfig(isDefault, enabled));
+ if (enabled){
+ assertEquals(1, builder.plugins().size());
+ assertEquals("software.amazon.awssdk.s3accessgrants.plugin.S3AccessGrantsPlugin",
Review Comment:
use assertj asserts here
##########
hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/s3_access_grants.md:
##########
@@ -0,0 +1,61 @@
+<!---
+ Licensed 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. See accompanying LICENSE file.
+-->
+
+# S3 Access Grants
+
+<!-- MACRO{toc|fromDepth=0|toDepth=5} -->
+
+S3 Access Grants is a credential vending service for S3 data. More information:
+* https://aws.amazon.com/s3/features/access-grants/
+
+In S3A, S3 Access Grants Plugin is used to support S3 Access Grants. More information:
+* https://github.com/aws/aws-s3-accessgrants-plugin-java-v2/
+
+
+
+## How to enable S3 Access Grants in S3A
+
+1. Add the `hadoop-aws` JAR on your classpath.
Review Comment:
just say "hadoop-aws JAR of the hadoop release and the normal dependencies, including bundle.jar
(gives us scope to add more mandatory stuff; i think wildfly is there already)
##########
hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/s3_access_grants.md:
##########
@@ -0,0 +1,61 @@
+<!---
+ Licensed 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. See accompanying LICENSE file.
+-->
+
+# S3 Access Grants
+
+<!-- MACRO{toc|fromDepth=0|toDepth=5} -->
+
+S3 Access Grants is a credential vending service for S3 data. More information:
+* https://aws.amazon.com/s3/features/access-grants/
+
+In S3A, S3 Access Grants Plugin is used to support S3 Access Grants. More information:
+* https://github.com/aws/aws-s3-accessgrants-plugin-java-v2/
+
+
+
+## How to enable S3 Access Grants in S3A
+
+1. Add the `hadoop-aws` JAR on your classpath.
+
+1. Add the `aws-java-sdk-bundle.jar` JAR to your classpath, the minimum version is v2.23.7.
+
+2. Add the `aws-s3-accessgrants-java-plugin-2.0.0.jar` JAR to your classpath.
+3. Add the `caffeine.jar` JAR to your classpath.
+
+1. Add configurations to enable S3 Access Grants in `core-site.xml`
+
+
+
+Example:
+
+```xml
+<configuration>
+...
+ <property>
Review Comment:
nit: use two chars for indentation
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AccessGrantConfiguration.java:
##########
@@ -0,0 +1,90 @@
+/*
+ * 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.Test;
+
+import software.amazon.awssdk.services.s3.S3AsyncClient;
+import software.amazon.awssdk.services.s3.S3BaseClientBuilder;
+import software.amazon.awssdk.services.s3.S3Client;
+
+import static org.apache.hadoop.fs.s3a.Constants.AWS_S3_ACCESS_GRANTS_ENABLED;
+import static org.junit.Assert.assertEquals;
+
+
+/**
+ * Test S3 Access Grants configurations.
+ */
+public class TestS3AccessGrantConfiguration extends AbstractHadoopTestBase {
+
+ @Test
+ public void testS3AccessGrantsEnabled() {
+ applyVerifyS3AGPlugin(S3Client.builder(), false, true);
+ }
+
+ @Test
+ public void testS3AccessGrantsEnabledAsync() {
+ applyVerifyS3AGPlugin(S3AsyncClient.builder(), false, true);
+ }
+
+ @Test
+ public void testS3AccessGrantsDisabled() {
+ applyVerifyS3AGPlugin(S3Client.builder(), false, false);
+ }
+
+ @Test
+ public void testS3AccessGrantsDisabledByDefault() {
+ applyVerifyS3AGPlugin(S3Client.builder(), true, false);
+ }
+
+ @Test
+ public void testS3AccessGrantsDisabledAsync() {
+ applyVerifyS3AGPlugin(S3AsyncClient.builder(), false, false);
+ }
+
+ @Test
+ public void testS3AccessGrantsDisabledByDefaultAsync() {
+ applyVerifyS3AGPlugin(S3AsyncClient.builder(), true, false);
+ }
+
+ private Configuration createConfig(boolean isDefault, boolean s3agEnabled) {
+ Configuration conf = new Configuration();
+ if (!isDefault){
+ conf.setBoolean(AWS_S3_ACCESS_GRANTS_ENABLED, s3agEnabled);
+ }
+ return conf;
+ }
+
+ private <BuilderT extends S3BaseClientBuilder<BuilderT, ClientT>, ClientT> void
+ applyVerifyS3AGPlugin(BuilderT builder, boolean isDefault, boolean enabled) {
+ DefaultS3ClientFactory.applyS3AccessGrantsConfigurations(builder, createConfig(isDefault, enabled));
+ if (enabled){
+ assertEquals(1, builder.plugins().size());
Review Comment:
assert j assert on .hasSize() for list
##########
hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/s3_access_grants.md:
##########
@@ -0,0 +1,61 @@
+<!---
+ Licensed 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. See accompanying LICENSE file.
+-->
+
+# S3 Access Grants
+
+<!-- MACRO{toc|fromDepth=0|toDepth=5} -->
+
+S3 Access Grants is a credential vending service for S3 data. More information:
+* https://aws.amazon.com/s3/features/access-grants/
+
+In S3A, S3 Access Grants Plugin is used to support S3 Access Grants. More information:
+* https://github.com/aws/aws-s3-accessgrants-plugin-java-v2/
+
+
+
+## How to enable S3 Access Grants in S3A
+
+1. Add the `hadoop-aws` JAR on your classpath.
+
+1. Add the `aws-java-sdk-bundle.jar` JAR to your classpath, the minimum version is v2.23.7.
+
+2. Add the `aws-s3-accessgrants-java-plugin-2.0.0.jar` JAR to your classpath.
Review Comment:
don't state the version; adds more work and will go out of date fast.
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AccessGrantConfiguration.java:
##########
@@ -0,0 +1,90 @@
+/*
+ * 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.Test;
+
+import software.amazon.awssdk.services.s3.S3AsyncClient;
+import software.amazon.awssdk.services.s3.S3BaseClientBuilder;
+import software.amazon.awssdk.services.s3.S3Client;
+
+import static org.apache.hadoop.fs.s3a.Constants.AWS_S3_ACCESS_GRANTS_ENABLED;
+import static org.junit.Assert.assertEquals;
+
+
+/**
+ * Test S3 Access Grants configurations.
+ */
+public class TestS3AccessGrantConfiguration extends AbstractHadoopTestBase {
+
+ @Test
+ public void testS3AccessGrantsEnabled() {
+ applyVerifyS3AGPlugin(S3Client.builder(), false, true);
+ }
+
+ @Test
+ public void testS3AccessGrantsEnabledAsync() {
+ applyVerifyS3AGPlugin(S3AsyncClient.builder(), false, true);
+ }
+
+ @Test
+ public void testS3AccessGrantsDisabled() {
+ applyVerifyS3AGPlugin(S3Client.builder(), false, false);
+ }
+
+ @Test
+ public void testS3AccessGrantsDisabledByDefault() {
+ applyVerifyS3AGPlugin(S3Client.builder(), true, false);
+ }
+
+ @Test
+ public void testS3AccessGrantsDisabledAsync() {
+ applyVerifyS3AGPlugin(S3AsyncClient.builder(), false, false);
+ }
+
+ @Test
+ public void testS3AccessGrantsDisabledByDefaultAsync() {
+ applyVerifyS3AGPlugin(S3AsyncClient.builder(), true, false);
+ }
+
+ private Configuration createConfig(boolean isDefault, boolean s3agEnabled) {
+ Configuration conf = new Configuration();
+ if (!isDefault){
+ conf.setBoolean(AWS_S3_ACCESS_GRANTS_ENABLED, s3agEnabled);
+ }
+ return conf;
+ }
+
+ private <BuilderT extends S3BaseClientBuilder<BuilderT, ClientT>, ClientT> void
+ applyVerifyS3AGPlugin(BuilderT builder, boolean isDefault, boolean enabled) {
+ DefaultS3ClientFactory.applyS3AccessGrantsConfigurations(builder, createConfig(isDefault, enabled));
+ if (enabled){
+ assertEquals(1, builder.plugins().size());
+ assertEquals("software.amazon.awssdk.s3accessgrants.plugin.S3AccessGrantsPlugin",
+ builder.plugins().get(0).getClass().getName()
+ );
+ }
+ else {
+ assertEquals(builder.plugins().size(), 0);
Review Comment:
assertj
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/S3AccessGrantsUtil.java:
##########
@@ -0,0 +1,77 @@
+/*
+ * 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.impl;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.s3a.DefaultS3ClientFactory;
+import org.apache.hadoop.fs.store.LogExactlyOnce;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import software.amazon.awssdk.s3accessgrants.plugin.S3AccessGrantsPlugin;
+import software.amazon.awssdk.services.s3.S3BaseClientBuilder;
+
+import static org.apache.hadoop.fs.s3a.Constants.AWS_S3_ACCESS_GRANTS_FALLBACK_TO_IAM_ENABLED;
+
+public class S3AccessGrantsUtil {
Review Comment:
1. javadocs here and on methods below.
2. mark as final and give a private constructor
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on code in PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#discussion_r1473066304
##########
hadoop-tools/hadoop-aws/pom.xml:
##########
@@ -508,6 +508,29 @@
<artifactId>bundle</artifactId>
<scope>compile</scope>
</dependency>
+ <dependency>
Review Comment:
the plugin should go in bundle.jar. it's meant to be a bundle.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-17351, Add Support for AWS S3 Access Grants [hadoop]
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#issuecomment-1913651462
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 32s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +0 :ok: | xmllint | 0m 0s | | xmllint was not available. |
| +0 :ok: | shelldocs | 0m 0s | | Shelldocs was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 14m 34s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 30m 46s | | trunk passed |
| +1 :green_heart: | compile | 16m 17s | | trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | compile | 14m 54s | | trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +1 :green_heart: | checkstyle | 4m 13s | | trunk passed |
| +1 :green_heart: | mvnsite | 18m 58s | | trunk passed |
| +1 :green_heart: | javadoc | 8m 59s | | trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 7m 28s | | trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +0 :ok: | spotbugs | 0m 19s | | branch/hadoop-project no spotbugs output file (spotbugsXml.xml) |
| +1 :green_heart: | shadedclient | 60m 57s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 1m 33s | | Maven dependency ordering for patch |
| -1 :x: | mvninstall | 0m 20s | [/patch-mvninstall-hadoop-tools_hadoop-aws.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/1/artifact/out/patch-mvninstall-hadoop-tools_hadoop-aws.txt) | hadoop-aws in the patch failed. |
| -1 :x: | mvninstall | 28m 39s | [/patch-mvninstall-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/1/artifact/out/patch-mvninstall-root.txt) | root in the patch failed. |
| -1 :x: | compile | 15m 23s | [/patch-compile-root-jdkUbuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/1/artifact/out/patch-compile-root-jdkUbuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04.txt) | root in the patch failed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04. |
| -1 :x: | javac | 15m 23s | [/patch-compile-root-jdkUbuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/1/artifact/out/patch-compile-root-jdkUbuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04.txt) | root in the patch failed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04. |
| -1 :x: | compile | 14m 45s | [/patch-compile-root-jdkPrivateBuild-1.8.0_392-8u392-ga-1~20.04-b08.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/1/artifact/out/patch-compile-root-jdkPrivateBuild-1.8.0_392-8u392-ga-1~20.04-b08.txt) | root in the patch failed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08. |
| -1 :x: | javac | 14m 45s | [/patch-compile-root-jdkPrivateBuild-1.8.0_392-8u392-ga-1~20.04-b08.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/1/artifact/out/patch-compile-root-jdkPrivateBuild-1.8.0_392-8u392-ga-1~20.04-b08.txt) | root in the patch failed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08. |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 4m 58s | | the patch passed |
| -1 :x: | mvnsite | 4m 50s | [/patch-mvnsite-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/1/artifact/out/patch-mvnsite-root.txt) | root in the patch failed. |
| +1 :green_heart: | shellcheck | 0m 0s | | No new issues. |
| +1 :green_heart: | javadoc | 8m 38s | | the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 7m 51s | | the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +0 :ok: | spotbugs | 0m 18s | | hadoop-project has no data from spotbugs |
| -1 :x: | spotbugs | 0m 23s | [/patch-spotbugs-hadoop-tools_hadoop-aws.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/1/artifact/out/patch-spotbugs-hadoop-tools_hadoop-aws.txt) | hadoop-aws in the patch failed. |
| -1 :x: | spotbugs | 29m 17s | [/patch-spotbugs-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/1/artifact/out/patch-spotbugs-root.txt) | root in the patch failed. |
| +1 :green_heart: | shadedclient | 35m 46s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 749m 22s | [/patch-unit-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/1/artifact/out/patch-unit-root.txt) | root in the patch failed. |
| +1 :green_heart: | asflicense | 1m 28s | | The patch does not generate ASF License warnings. |
| | | 1090m 19s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.yarn.server.timelineservice.security.TestTimelineAuthFilterForV2 |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/6507 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient codespell detsecrets xmllint spotbugs checkstyle shellcheck shelldocs |
| uname | Linux 4f63f73bb90a 5.15.0-88-generic #98-Ubuntu SMP Mon Oct 2 15:18:56 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 2152287eef9b873b9360424c5d56617c49bdc633 |
| Default Java | Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/1/testReport/ |
| Max. process+thread count | 3991 (vs. ulimit of 5500) |
| modules | C: hadoop-project hadoop-tools/hadoop-aws . U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/1/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 shellcheck=0.7.0 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "jxhan3 (via GitHub)" <gi...@apache.org>.
jxhan3 commented on code in PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#discussion_r1475155889
##########
hadoop-project/pom.xml:
##########
@@ -187,7 +187,7 @@
<make-maven-plugin.version>1.0-beta-1</make-maven-plugin.version>
<surefire.fork.timeout>900</surefire.fork.timeout>
<aws-java-sdk.version>1.12.599</aws-java-sdk.version>
- <aws-java-sdk-v2.version>2.23.5</aws-java-sdk-v2.version>
Review Comment:
revert this as well
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#issuecomment-1925326604
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 11m 19s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +0 :ok: | xmllint | 0m 0s | | xmllint was not available. |
| +0 :ok: | shelldocs | 0m 0s | | Shelldocs was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 15m 10s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 31m 19s | | trunk passed |
| +1 :green_heart: | compile | 16m 9s | | trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | compile | 14m 44s | | trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +1 :green_heart: | checkstyle | 4m 14s | | trunk passed |
| +1 :green_heart: | mvnsite | 19m 1s | | trunk passed |
| +1 :green_heart: | javadoc | 8m 21s | | trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 7m 31s | | trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +0 :ok: | spotbugs | 0m 19s | | branch/hadoop-project no spotbugs output file (spotbugsXml.xml) |
| +1 :green_heart: | shadedclient | 60m 32s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 34s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 29m 3s | | the patch passed |
| +1 :green_heart: | compile | 15m 47s | | the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javac | 15m 47s | | the patch passed |
| +1 :green_heart: | compile | 14m 58s | | the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +1 :green_heart: | javac | 14m 58s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 4m 3s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/6/artifact/out/results-checkstyle-root.txt) | root: The patch generated 8 new + 2 unchanged - 0 fixed = 10 total (was 2) |
| +1 :green_heart: | mvnsite | 13m 33s | | the patch passed |
| +1 :green_heart: | shellcheck | 0m 0s | | No new issues. |
| +1 :green_heart: | javadoc | 8m 56s | | the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 7m 32s | | the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +0 :ok: | spotbugs | 0m 19s | | hadoop-project has no data from spotbugs |
| +1 :green_heart: | shadedclient | 61m 1s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 746m 36s | [/patch-unit-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/6/artifact/out/patch-unit-root.txt) | root in the patch passed. |
| +1 :green_heart: | asflicense | 1m 32s | | The patch does not generate ASF License warnings. |
| | | 1102m 26s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.yarn.webapp.TestWebApp |
| | hadoop.hdfs.TestRollingUpgrade |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/6/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/6507 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient codespell detsecrets xmllint spotbugs checkstyle shellcheck shelldocs |
| uname | Linux 8d079798773f 5.15.0-91-generic #101-Ubuntu SMP Tue Nov 14 13:30:08 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 8382af88b7c6ad40b92a0d67dfbdb9f988490c3c |
| Default Java | Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/6/testReport/ |
| Max. process+thread count | 3517 (vs. ulimit of 5500) |
| modules | C: hadoop-project hadoop-tools/hadoop-aws . U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/6/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 shellcheck=0.7.0 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#issuecomment-1925476864
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 48s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +0 :ok: | xmllint | 0m 0s | | xmllint was not available. |
| +0 :ok: | shelldocs | 0m 0s | | Shelldocs was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 14m 28s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 34m 54s | | trunk passed |
| +1 :green_heart: | compile | 18m 16s | | trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | compile | 16m 52s | | trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +1 :green_heart: | checkstyle | 4m 44s | | trunk passed |
| +1 :green_heart: | mvnsite | 19m 23s | | trunk passed |
| +1 :green_heart: | javadoc | 8m 47s | | trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 7m 31s | | trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +0 :ok: | spotbugs | 0m 18s | | branch/hadoop-project no spotbugs output file (spotbugsXml.xml) |
| +1 :green_heart: | shadedclient | 67m 25s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 32s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 33m 47s | | the patch passed |
| +1 :green_heart: | compile | 17m 53s | | the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javac | 17m 53s | | the patch passed |
| +1 :green_heart: | compile | 17m 3s | | the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +1 :green_heart: | javac | 17m 3s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 4m 35s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/7/artifact/out/results-checkstyle-root.txt) | root: The patch generated 7 new + 2 unchanged - 0 fixed = 9 total (was 2) |
| +1 :green_heart: | mvnsite | 14m 40s | | the patch passed |
| +1 :green_heart: | shellcheck | 0m 0s | | No new issues. |
| +1 :green_heart: | javadoc | 8m 37s | | the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 7m 29s | | the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +0 :ok: | spotbugs | 0m 18s | | hadoop-project has no data from spotbugs |
| -1 :x: | spotbugs | 1m 14s | [/new-spotbugs-hadoop-tools_hadoop-aws.html](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/7/artifact/out/new-spotbugs-hadoop-tools_hadoop-aws.html) | hadoop-tools/hadoop-aws generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| -1 :x: | spotbugs | 31m 30s | [/new-spotbugs-root.html](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/7/artifact/out/new-spotbugs-root.html) | root generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +1 :green_heart: | shadedclient | 68m 35s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 792m 8s | [/patch-unit-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/7/artifact/out/patch-unit-root.txt) | root in the patch passed. |
| -1 :x: | asflicense | 1m 31s | [/results-asflicense.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/7/artifact/out/results-asflicense.txt) | The patch generated 1 ASF License warnings. |
| | | 1171m 11s | | |
| Reason | Tests |
|-------:|:------|
| SpotBugs | module:hadoop-tools/hadoop-aws |
| | Useless object stored in variable argTypes of method org.apache.hadoop.fs.s3a.DefaultS3ClientFactory.applyS3AccessGrantsConfigurations(S3BaseClientBuilder, Configuration) At DefaultS3ClientFactory.java:argTypes of method org.apache.hadoop.fs.s3a.DefaultS3ClientFactory.applyS3AccessGrantsConfigurations(S3BaseClientBuilder, Configuration) At DefaultS3ClientFactory.java:[line 421] |
| SpotBugs | module:root |
| | Useless object stored in variable argTypes of method org.apache.hadoop.fs.s3a.DefaultS3ClientFactory.applyS3AccessGrantsConfigurations(S3BaseClientBuilder, Configuration) At DefaultS3ClientFactory.java:argTypes of method org.apache.hadoop.fs.s3a.DefaultS3ClientFactory.applyS3AccessGrantsConfigurations(S3BaseClientBuilder, Configuration) At DefaultS3ClientFactory.java:[line 421] |
| Failed junit tests | hadoop.hdfs.server.datanode.TestDirectoryScanner |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/7/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/6507 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient codespell detsecrets xmllint spotbugs checkstyle shellcheck shelldocs |
| uname | Linux 686a24e04556 5.15.0-88-generic #98-Ubuntu SMP Mon Oct 2 15:18:56 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 347cc9f4b52d1e5fb804c0ad7eb683cbd89989c5 |
| Default Java | Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/7/testReport/ |
| Max. process+thread count | 3705 (vs. ulimit of 5500) |
| modules | C: hadoop-project hadoop-tools/hadoop-aws . U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/7/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 shellcheck=0.7.0 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#issuecomment-1920154192
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 45s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available. |
| +0 :ok: | xmllint | 0m 0s | | xmllint was not available. |
| +0 :ok: | shelldocs | 0m 0s | | Shelldocs was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 14m 31s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 35m 9s | | trunk passed |
| +1 :green_heart: | compile | 18m 9s | | trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | compile | 16m 28s | | trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +1 :green_heart: | checkstyle | 4m 38s | | trunk passed |
| +1 :green_heart: | mvnsite | 19m 28s | | trunk passed |
| +1 :green_heart: | javadoc | 8m 46s | | trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 7m 31s | | trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +0 :ok: | spotbugs | 0m 18s | | branch/hadoop-project no spotbugs output file (spotbugsXml.xml) |
| +1 :green_heart: | shadedclient | 68m 9s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 51s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 34m 53s | | the patch passed |
| +1 :green_heart: | compile | 23m 16s | | the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javac | 23m 16s | | the patch passed |
| +1 :green_heart: | compile | 17m 32s | | the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +1 :green_heart: | javac | 17m 32s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 4m 39s | | the patch passed |
| +1 :green_heart: | mvnsite | 14m 54s | | the patch passed |
| +1 :green_heart: | shellcheck | 0m 1s | | No new issues. |
| +1 :green_heart: | javadoc | 8m 50s | | the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 |
| +1 :green_heart: | javadoc | 7m 58s | | the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| +0 :ok: | spotbugs | 0m 17s | | hadoop-project has no data from spotbugs |
| +1 :green_heart: | shadedclient | 73m 39s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| -1 :x: | unit | 784m 55s | [/patch-unit-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/4/artifact/out/patch-unit-root.txt) | root in the patch passed. |
| +1 :green_heart: | asflicense | 1m 28s | | The patch does not generate ASF License warnings. |
| | | 1177m 11s | | |
| Reason | Tests |
|-------:|:------|
| Failed junit tests | hadoop.hdfs.server.datanode.TestDirectoryScanner |
| | hadoop.yarn.webapp.TestWebApp |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.44 ServerAPI=1.44 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/4/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/6507 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient codespell detsecrets xmllint spotbugs checkstyle shellcheck shelldocs |
| uname | Linux 143e26d5ad39 5.15.0-88-generic #98-Ubuntu SMP Mon Oct 2 15:18:56 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / eaa7edc3a44126abe854d30f41a508036eef7eeb |
| Default Java | Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_392-8u392-ga-1~20.04-b08 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/4/testReport/ |
| Max. process+thread count | 3961 (vs. ulimit of 5500) |
| modules | C: hadoop-project hadoop-tools/hadoop-aws . U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6507/4/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 shellcheck=0.7.0 |
| Powered by | Apache Yetus 0.14.0 https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050. SDK Add Support for AWS S3 Access Grants [hadoop]
Posted by "adnanhemani (via GitHub)" <gi...@apache.org>.
adnanhemani commented on PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#issuecomment-1978496258
Hi @steveloughran, as noted in this comment, I’ve made the changes you requested (including getting the S3 Access Grants plugin added to the bundle JAR) in a new PR: https://github.com/apache/hadoop/pull/6507#issuecomment-1935463310
#6544 has those changes. Unless I’m misunderstanding what you are commenting about (from what I got, you are advocating for the plugin to be part of the bundle JAR - which is now the case), please close this PR. I’m glad to continue our discussion on the new PR (#6544)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on code in PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#discussion_r1472896913
##########
hadoop-tools/hadoop-aws/pom.xml:
##########
@@ -508,6 +508,29 @@
<artifactId>bundle</artifactId>
<scope>compile</scope>
</dependency>
+ <dependency>
+ <groupId>software.amazon.s3.accessgrants</groupId>
Review Comment:
why isn't this in bundle.jar?
* if it is: it's not needed.
* If it isn't, why not?
is this new jar going to be mandatory, or optional?
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java:
##########
@@ -1600,4 +1600,20 @@ private Constants() {
*/
public static final boolean CHECKSUM_VALIDATION_DEFAULT = false;
+
+ /**
+ * Flag to enable S3 Access Grants to control authorization to S3 data. More information:
+ * https://aws.amazon.com/s3/features/access-grants/
+ * and
+ * https://github.com/aws/aws-s3-accessgrants-plugin-java-v2/
+ */
+ public static final String AWS_S3_ACCESS_GRANTS_ENABLED = "fs.s3a.access-grants.enabled";
+
+ /**
+ * Flag to enable jobs fall back to the Job Execution IAM role in
+ * case they get Access Denied from the S3 Access Grants call. More information:
+ * https://github.com/aws/aws-s3-accessgrants-plugin-java-v2/
+ */
+ public static final String AWS_S3_ACCESS_GRANTS_FALLBACK_TO_IAM_ENABLED =
+ "fs.s3a.access-grants.fallback-to-iam";
Review Comment:
nit; can you use "." over "-"
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/DefaultS3ClientFactory.java:
##########
@@ -370,4 +375,18 @@ private static Region getS3RegionFromEndpoint(String endpoint) {
return Region.US_EAST_1;
}
+ public static <BuilderT extends S3BaseClientBuilder<BuilderT, ClientT>, ClientT> void
+ applyS3AccessGrantsConfigurations(BuilderT builder, Configuration conf) {
+ boolean s3agEnabled = conf.getBoolean(AWS_S3_ACCESS_GRANTS_ENABLED, false);
+ if (s3agEnabled) {
+ boolean s3agFallbackEnabled = conf.getBoolean(
+ AWS_S3_ACCESS_GRANTS_FALLBACK_TO_IAM_ENABLED, false);
+ S3AccessGrantsPlugin accessGrantsPlugin =
+ S3AccessGrantsPlugin.builder().enableFallback(s3agFallbackEnabled).build();
+ builder.addPlugin(accessGrantsPlugin);
+ LOG.info("s3ag plugin is added to s3 client with fallback: {}", s3agFallbackEnabled);
Review Comment:
use a LogExactlyOnce. this will get oververbose in processes which create/destroy fs instances
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AccessGrantConfiguration.java:
##########
@@ -0,0 +1,102 @@
+/*
+ * 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.junit.Test;
+
+import software.amazon.awssdk.services.s3.S3AsyncClientBuilder;
+import software.amazon.awssdk.services.s3.S3AsyncClient;
+import software.amazon.awssdk.services.s3.S3BaseClientBuilder;
+import software.amazon.awssdk.services.s3.S3ClientBuilder;
+import software.amazon.awssdk.services.s3.S3Client;
+
+import static org.apache.hadoop.fs.s3a.Constants.AWS_S3_ACCESS_GRANTS_ENABLED;
+import static org.junit.Assert.assertEquals;
+
+
+/**
+ * Test S3 Access Grants configurations.
+ */
+public class TestS3AccessGrantConfiguration {
Review Comment:
`extends AbstractHadoopTestBase`
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AccessGrantConfiguration.java:
##########
@@ -0,0 +1,102 @@
+/*
+ * 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.junit.Test;
+
+import software.amazon.awssdk.services.s3.S3AsyncClientBuilder;
+import software.amazon.awssdk.services.s3.S3AsyncClient;
+import software.amazon.awssdk.services.s3.S3BaseClientBuilder;
+import software.amazon.awssdk.services.s3.S3ClientBuilder;
+import software.amazon.awssdk.services.s3.S3Client;
+
+import static org.apache.hadoop.fs.s3a.Constants.AWS_S3_ACCESS_GRANTS_ENABLED;
+import static org.junit.Assert.assertEquals;
+
+
+/**
+ * Test S3 Access Grants configurations.
+ */
+public class TestS3AccessGrantConfiguration {
+
+ @Test
+ public void testS3AccessGrantsEnabled() {
+ Configuration conf = new Configuration();
+ conf.set(AWS_S3_ACCESS_GRANTS_ENABLED, "true");
Review Comment:
`setBoolean(.., true)`
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AccessGrantConfiguration.java:
##########
@@ -0,0 +1,102 @@
+/*
+ * 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.junit.Test;
+
+import software.amazon.awssdk.services.s3.S3AsyncClientBuilder;
+import software.amazon.awssdk.services.s3.S3AsyncClient;
+import software.amazon.awssdk.services.s3.S3BaseClientBuilder;
+import software.amazon.awssdk.services.s3.S3ClientBuilder;
+import software.amazon.awssdk.services.s3.S3Client;
+
+import static org.apache.hadoop.fs.s3a.Constants.AWS_S3_ACCESS_GRANTS_ENABLED;
+import static org.junit.Assert.assertEquals;
+
+
+/**
+ * Test S3 Access Grants configurations.
+ */
+public class TestS3AccessGrantConfiguration {
+
+ @Test
+ public void testS3AccessGrantsEnabled() {
+ Configuration conf = new Configuration();
Review Comment:
use new Configuration(false) to avoid loading any default values -this avoids integration test settings breaking unit tests. even better: make it a final field
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AccessGrantConfiguration.java:
##########
@@ -0,0 +1,102 @@
+/*
+ * 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.junit.Test;
+
+import software.amazon.awssdk.services.s3.S3AsyncClientBuilder;
+import software.amazon.awssdk.services.s3.S3AsyncClient;
+import software.amazon.awssdk.services.s3.S3BaseClientBuilder;
+import software.amazon.awssdk.services.s3.S3ClientBuilder;
+import software.amazon.awssdk.services.s3.S3Client;
+
+import static org.apache.hadoop.fs.s3a.Constants.AWS_S3_ACCESS_GRANTS_ENABLED;
+import static org.junit.Assert.assertEquals;
+
+
+/**
+ * Test S3 Access Grants configurations.
+ */
+public class TestS3AccessGrantConfiguration {
+
+ @Test
+ public void testS3AccessGrantsEnabled() {
+ Configuration conf = new Configuration();
+ conf.set(AWS_S3_ACCESS_GRANTS_ENABLED, "true");
+ S3ClientBuilder builder = S3Client.builder();
+ DefaultS3ClientFactory.applyS3AccessGrantsConfigurations(builder, conf);
+ verifyS3AGPluginEnabled(builder);
+ }
+
+ @Test
+ public void testS3AccessGrantsEnabledAsync() {
+ Configuration conf = new Configuration();
+ conf.set(AWS_S3_ACCESS_GRANTS_ENABLED, "true");
+ S3AsyncClientBuilder builder = S3AsyncClient.builder();
+ DefaultS3ClientFactory.applyS3AccessGrantsConfigurations(builder, conf);
+ verifyS3AGPluginEnabled(builder);
+ }
+
+ @Test
+ public void testS3AccessGrantsDisabled() {
+ Configuration conf = new Configuration();
+ conf.set(AWS_S3_ACCESS_GRANTS_ENABLED, "false");
+ S3ClientBuilder builder = S3Client.builder();
+ DefaultS3ClientFactory.applyS3AccessGrantsConfigurations(builder, conf);
+ verifyS3AGPluginDisabled(builder);
+ }
+
+ @Test
+ public void testS3AccessGrantsDisabledByDefault() {
+ Configuration conf = new Configuration();
+ S3ClientBuilder builder = S3Client.builder();
+ DefaultS3ClientFactory.applyS3AccessGrantsConfigurations(builder, conf);
+ verifyS3AGPluginDisabled(builder);
+ }
+
+ @Test
+ public void testS3AccessGrantsDisabledAsync() {
+ Configuration conf = new Configuration();
+ conf.set(AWS_S3_ACCESS_GRANTS_ENABLED, "false");
+ S3AsyncClientBuilder builder = S3AsyncClient.builder();
+ DefaultS3ClientFactory.applyS3AccessGrantsConfigurations(builder, conf);
+ verifyS3AGPluginDisabled(builder);
+ }
+
+ @Test
+ public void testS3AccessGrantsDisabledByDefaultAsync() {
+ Configuration conf = new Configuration();
+ S3AsyncClientBuilder builder = S3AsyncClient.builder();
+ DefaultS3ClientFactory.applyS3AccessGrantsConfigurations(builder, conf);
+ verifyS3AGPluginDisabled(builder);
+ }
+
+ private <BuilderT extends S3BaseClientBuilder<BuilderT, ClientT>, ClientT> void
+ verifyS3AGPluginEnabled(BuilderT builder) {
+ assertEquals(builder.plugins().size(), 1);
Review Comment:
use assertJ assertions on list sizes here and below
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AccessGrantConfiguration.java:
##########
@@ -0,0 +1,102 @@
+/*
+ * 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;
Review Comment:
nit: review import ordering
##########
hadoop-project/pom.xml:
##########
@@ -187,7 +187,7 @@
<make-maven-plugin.version>1.0-beta-1</make-maven-plugin.version>
<surefire.fork.timeout>900</surefire.fork.timeout>
<aws-java-sdk.version>1.12.599</aws-java-sdk.version>
- <aws-java-sdk-v2.version>2.23.5</aws-java-sdk-v2.version>
+ <aws-java-sdk-v2.version>2.23.7</aws-java-sdk-v2.version>
Review Comment:
SDK updates need to be self contained PRs for isolated cherrypick and revert. section in testing.md on the process. Yes, you do have to follow it, including looking at accidental dependency exports and new error messages in the text output.
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java:
##########
@@ -1600,4 +1600,20 @@ private Constants() {
*/
public static final boolean CHECKSUM_VALIDATION_DEFAULT = false;
+
+ /**
+ * Flag to enable S3 Access Grants to control authorization to S3 data. More information:
+ * https://aws.amazon.com/s3/features/access-grants/
+ * and
+ * https://github.com/aws/aws-s3-accessgrants-plugin-java-v2/
Review Comment:
use {@value} in the comments so ides will show the value
##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AccessGrantConfiguration.java:
##########
@@ -0,0 +1,102 @@
+/*
+ * 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.junit.Test;
+
+import software.amazon.awssdk.services.s3.S3AsyncClientBuilder;
+import software.amazon.awssdk.services.s3.S3AsyncClient;
+import software.amazon.awssdk.services.s3.S3BaseClientBuilder;
+import software.amazon.awssdk.services.s3.S3ClientBuilder;
+import software.amazon.awssdk.services.s3.S3Client;
+
+import static org.apache.hadoop.fs.s3a.Constants.AWS_S3_ACCESS_GRANTS_ENABLED;
+import static org.junit.Assert.assertEquals;
+
+
+/**
+ * Test S3 Access Grants configurations.
+ */
+public class TestS3AccessGrantConfiguration {
+
+ @Test
+ public void testS3AccessGrantsEnabled() {
Review Comment:
There is a lot of duplication in these tests: you should factor it out as much as possible to keep test maintenance down.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "jxhan3 (via GitHub)" <gi...@apache.org>.
jxhan3 commented on code in PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#discussion_r1475168380
##########
hadoop-project/pom.xml:
##########
@@ -187,7 +187,7 @@
<make-maven-plugin.version>1.0-beta-1</make-maven-plugin.version>
<surefire.fork.timeout>900</surefire.fork.timeout>
<aws-java-sdk.version>1.12.599</aws-java-sdk.version>
- <aws-java-sdk-v2.version>2.23.5</aws-java-sdk-v2.version>
+ <aws-java-sdk-v2.version>2.23.7</aws-java-sdk-v2.version>
Review Comment:
removed in next version
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "jxhan3 (via GitHub)" <gi...@apache.org>.
jxhan3 commented on PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#issuecomment-1922460413
Some tests done in EMR cluster: (all successful with expected behavior)
1. set up AWS IAM roles with S3 Access Grants: read prefix, write prefix, read/write prefix
2. test on us-east-1 bucket/prefix
3. with and without fallback options
4. hadoop CLI tests: ls, put, get, rm
5. Spark tests: create table, insert data, select, drop table
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "adnanhemani (via GitHub)" <gi...@apache.org>.
adnanhemani commented on PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#issuecomment-1922468809
@jxhan3 can you please put the output from your console testing onto the comments here? No need to paste the actual grants but just say which ones have grants and should be successful or not. It would be nice to have the actual output noted on this CR itself as proof.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "adnanhemani (via GitHub)" <gi...@apache.org>.
adnanhemani commented on PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#issuecomment-1933245232
Update: I've successfully influenced the S3 Access Grants team and the AWS Java SDK team to include the S3 Access Grants Plugin within the AWS Java SDK bundle. For that, we will require an SDK version upgrade. Will work on that first to ensure we don't require the reflection logic here.
In the meantime, any review on the latest version of this code would be appreciated - it will help me make the logic here solid while we work on the AWS SDK upgrade in parallel.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] HADOOP-19050, Add Support for AWS S3 Access Grants [hadoop]
Posted by "adnanhemani (via GitHub)" <gi...@apache.org>.
adnanhemani commented on code in PR #6507:
URL: https://github.com/apache/hadoop/pull/6507#discussion_r1477676885
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/DefaultS3ClientFactory.java:
##########
@@ -401,4 +409,32 @@ private static Region getS3RegionFromEndpoint(final String endpoint,
return Region.of(AWS_S3_DEFAULT_REGION);
}
+ public static <BuilderT extends S3BaseClientBuilder<BuilderT, ClientT>, ClientT> void
+ applyS3AccessGrantsConfigurations(BuilderT builder, Configuration conf) {
+ boolean s3agEnabled = conf.getBoolean(AWS_S3_ACCESS_GRANTS_ENABLED, false);
+ if (!s3agEnabled){
+ LOG_EXACTLY_ONCE.debug("s3ag plugin is not enabled.");
Review Comment:
On all logs, can we use the full name: "S3 Access Grants..." to make it clear for users looking through the logs?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org