Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-03-19 Thread via GitHub


steveloughran merged PR #6544:
URL: https://github.com/apache/hadoop/pull/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-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-03-19 Thread via GitHub


adnanhemani commented on PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#issuecomment-2007791751

   Yup, this makes sense. I will ensure that Hadoop gets the SDK changes as 
soon as the SDK updates are complete. Thank you again for all your time 
reviewing this!


-- 
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-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-03-19 Thread via GitHub


steveloughran commented on PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#issuecomment-2007781626

   hey, if yetus is unhappy, rebase is the right thing to do, so don't worry 
too much. just trying to remember where i was, that's all


-- 
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-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-03-19 Thread via GitHub


adnanhemani commented on PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#issuecomment-2007775835

   Sorry! Yetus was complaining that it could not apply the changes on top of 
`trunk` so I instinctively rebased and force pushed - will keep this in mind!


-- 
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-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-03-19 Thread via GitHub


steveloughran commented on PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#issuecomment-2007753756

   please, please please, no force push once reviews have started unless 
there's merge problems or its been neglected for too long...makes it harder to 
seee what's changed between reviews


-- 
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-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-03-13 Thread via GitHub


hadoop-yetus commented on PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#issuecomment-1995867156

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |::|--:|:|::|:---:|
   | +0 :ok: |  reexec  |   0m 46s |  |  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: |  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 _ |
   | +1 :green_heart: |  mvninstall  |  50m  6s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 42s |  |  trunk passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1  |
   | +1 :green_heart: |  compile  |   0m 34s |  |  trunk passed with JDK 
Private Build-1.8.0_392-8u392-ga-1~20.04-b08  |
   | +1 :green_heart: |  checkstyle  |   0m 30s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 40s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   0m 24s |  |  trunk passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 33s |  |  trunk passed with JDK 
Private Build-1.8.0_392-8u392-ga-1~20.04-b08  |
   | +1 :green_heart: |  spotbugs  |   1m  7s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  37m 32s |  |  branch has no errors 
when building and testing our client artifacts.  |
    _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 30s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 35s |  |  the patch passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1  |
   | +1 :green_heart: |  javac  |   0m 35s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 25s |  |  the patch passed with JDK 
Private Build-1.8.0_392-8u392-ga-1~20.04-b08  |
   | +1 :green_heart: |  javac  |   0m 25s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks 
issues.  |
   | -0 :warning: |  checkstyle  |   0m 19s | 
[/results-checkstyle-hadoop-tools_hadoop-aws.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6544/5/artifact/out/results-checkstyle-hadoop-tools_hadoop-aws.txt)
 |  hadoop-tools/hadoop-aws: The patch generated 1 new + 5 unchanged - 0 fixed 
= 6 total (was 5)  |
   | +1 :green_heart: |  mvnsite  |   0m 31s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   0m 15s |  |  the patch passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 25s |  |  the patch passed with JDK 
Private Build-1.8.0_392-8u392-ga-1~20.04-b08  |
   | +1 :green_heart: |  spotbugs  |   1m  6s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  38m 12s |  |  patch has no errors 
when building and testing our client artifacts.  |
    _ Other Tests _ |
   | +1 :green_heart: |  unit  |   2m 45s |  |  hadoop-aws in the patch passed. 
 |
   | +1 :green_heart: |  asflicense  |   0m 35s |  |  The patch does not 
generate ASF License warnings.  |
   |  |   | 143m  0s |  |  |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.44 ServerAPI=1.44 base: 
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6544/5/artifact/out/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/hadoop/pull/6544 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall 
mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets markdownlint 
|
   | uname | Linux bf2867ac1970 5.15.0-94-generic #104-Ubuntu SMP Tue Jan 9 
15:25:40 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / 3cfb18e9d733ddabd41e882193de4190dd6620d2 |
   | 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.22+7-post-Ubuntu-0ubuntu220.04.1 
/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-6544/5/testReport/ |
   | Max. process+thread count | 603 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws |
   | Console output | 
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6544/5/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.
   
   

Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-03-13 Thread via GitHub


adnanhemani commented on PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#issuecomment-1995387593

   > You're right there is no rename; copy is all there is. So that is not 
available yet? Hmmm. This isn't ready for production yet is it? Let us keep it 
in trunk for now. The other strategy would be to do a feature branch for it, 
which has mixed benefits. Good: isolated from other work. Bad: isolated from 
other work. So far the changes are minimal enough it is not a problem.
   
   Yeah :/ I believe there are still some committer types that are not renaming 
on every write and so some simple queries will actually succeed. But in 
general, the read queries are succeeding, which is why this is still going to 
be useful in the meantime to the community. When the new functionality arrives, 
it will only require a bump on the SDK version - so I don't think this is a 
blocker for `trunk`, while I can understand it being a blocker for a Hadoop 
release.
   
   > Regarding testing, when you think it is ready for others to play with a 
section in testing.md on how to get set up for this would be good. Well I don't 
personally have plans for that, maybe I could persuade colleagues. I tried to 
test a lot of the other corner cases.
   
   I believe the AWS Documentation and supporting blog posts have the right 
amount of instructions on how to have a sample setup. Then the Hadoop community 
members can test this code against that setup. Is there a specific reason to 
include it as part of this repo then? If you think we really should have a 
section on this, would it be okay to just add a link to the relevant blog posts 
and documentation in `testing.md`?


-- 
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-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-03-13 Thread via GitHub


hadoop-yetus commented on PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#issuecomment-1995387553

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |::|--:|:|::|:---:|
   | +0 :ok: |  reexec  |   0m  0s |  |  Docker mode activated.  |
   | -1 :x: |  patch  |   0m 21s |  |  
https://github.com/apache/hadoop/pull/6544 does not apply to trunk. Rebase 
required? Wrong Branch? See 
https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute for help.  
|
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | GITHUB PR | https://github.com/apache/hadoop/pull/6544 |
   | Console output | 
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6544/4/console |
   | versions | git=2.34.1 |
   | 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-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-03-13 Thread via GitHub


adnanhemani commented on code in PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#discussion_r1523757683


##
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AccessGrantConfiguration.java:
##
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.s3a;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.test.AbstractHadoopTestBase;
+import org.junit.Assert;
+import org.junit.Test;
+
+import software.amazon.awssdk.awscore.AwsClient;
+import 
software.amazon.awssdk.s3accessgrants.plugin.S3AccessGrantsIdentityProvider;

Review Comment:
   I'm not sure how this would function? Since we don't actually control the 
usage of this class - it's in the internals of the Access Grants plugin itself. 
In other words, if there is a version upgrade on the S3 Access Grants plugin 
and that include a change to this class name/structure, we will have to update 
this to reflect those changes - it's not in our control which class is used?



-- 
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-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-03-13 Thread via GitHub


adnanhemani commented on code in PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#discussion_r1523715465


##
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/DefaultS3ClientFactory.java:
##
@@ -401,4 +406,22 @@ private static Region getS3RegionFromEndpoint(final String 
endpoint,
 return Region.of(AWS_S3_DEFAULT_REGION);
   }
 
+  private static , 
ClientT> void
+  applyS3AccessGrantsConfigurations(BuilderT builder, Configuration conf) {
+boolean isS3AccessGrantsEnabled = 
conf.getBoolean(AWS_S3_ACCESS_GRANTS_ENABLED, false);
+if (!isS3AccessGrantsEnabled){
+  LOG.debug("S3 Access Grants plugin is not enabled.");
+  return;
+}
+
+boolean isFallbackEnabled =
+conf.getBoolean(AWS_S3_ACCESS_GRANTS_FALLBACK_TO_IAM_ENABLED, false);
+S3AccessGrantsPlugin accessGrantsPlugin =
+S3AccessGrantsPlugin.builder()
+.enableFallback(isFallbackEnabled)
+.build();
+builder.addPlugin(accessGrantsPlugin);
+LOG.info("S3 Access Grants plugin is enabled with IAM fallback set to {}", 
isFallbackEnabled);

Review Comment:
   Yeah, that's why we had added the log once earlier but I removed it in the 
last revision based on your comments (maybe I misunderstood...?)
   
   Personally, I agree with making it a log once so I'll change it back.



-- 
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-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-03-12 Thread via GitHub


steveloughran commented on code in PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#discussion_r1522069895


##
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/DefaultS3ClientFactory.java:
##
@@ -401,4 +406,22 @@ private static Region getS3RegionFromEndpoint(final String 
endpoint,
 return Region.of(AWS_S3_DEFAULT_REGION);
   }
 
+  private static , 
ClientT> void
+  applyS3AccessGrantsConfigurations(BuilderT builder, Configuration conf) {
+boolean isS3AccessGrantsEnabled = 
conf.getBoolean(AWS_S3_ACCESS_GRANTS_ENABLED, false);
+if (!isS3AccessGrantsEnabled){
+  LOG.debug("S3 Access Grants plugin is not enabled.");
+  return;
+}
+
+boolean isFallbackEnabled =
+conf.getBoolean(AWS_S3_ACCESS_GRANTS_FALLBACK_TO_IAM_ENABLED, false);
+S3AccessGrantsPlugin accessGrantsPlugin =
+S3AccessGrantsPlugin.builder()
+.enableFallback(isFallbackEnabled)
+.build();
+builder.addPlugin(accessGrantsPlugin);
+LOG.info("S3 Access Grants plugin is enabled with IAM fallback set to {}", 
isFallbackEnabled);

Review Comment:
   might be good to add once so that on a large system the logs don't get full 
of noise. tricky choice



##
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AccessGrantConfiguration.java:
##
@@ -0,0 +1,126 @@
+/*
+ * 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 java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+
+import org.assertj.core.api.AbstractStringAssert;
+import org.assertj.core.api.Assertions;
+import org.junit.Test;
+import software.amazon.awssdk.awscore.AwsClient;
+import 
software.amazon.awssdk.s3accessgrants.plugin.S3AccessGrantsIdentityProvider;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.test.AbstractHadoopTestBase;
+
+import static org.apache.hadoop.fs.s3a.Constants.AWS_S3_ACCESS_GRANTS_ENABLED;
+
+
+/**
+ * Test S3 Access Grants configurations.
+ */
+public class TestS3AccessGrantConfiguration extends AbstractHadoopTestBase {
+  /**
+   * This credential provider will be attached to any client
+   * that has been configured with the S3 Access Grants plugin.
+   * {@link software.amazon.awssdk.s3accessgrants.plugin.S3AccessGrantsPlugin}.

Review Comment:
   I don't think javadoc will resolve that; just use {@code}



##
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/DefaultS3ClientFactory.java:
##
@@ -178,6 +181,8 @@ private , ClientT> Build
 
 configureEndpointAndRegion(builder, parameters, conf);
 
+applyS3AccessGrantsConfigurations(builder, conf);

Review Comment:
   rename maybeApply... to make clear it isn't guaranteed to happen



##
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java:
##
@@ -5516,6 +5522,10 @@ public boolean hasPathCapability(final Path path, final 
String capability)
 case FIPS_ENDPOINT:
   return fipsEnabled;
 
+// is S3 Access Grants enabled
+case AWS_S3_ACCESS_GRANTS_ENABLED:

Review Comment:
   our bucket-info command has a list of capabilities to probe for -add this to 
the list



##
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java:
##
@@ -494,6 +494,11 @@ public class S3AFileSystem extends FileSystem implements 
StreamCapabilities,
*/
   private String configuredRegion;
 
+  /**
+   * Is a S3 Access Grants Enabled?

Review Comment:
   nit: "are S3 Access Grants Enabled?"



##
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AccessGrantConfiguration.java:
##
@@ -0,0 +1,107 @@
+/*
+ * 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

Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-03-12 Thread via GitHub


steveloughran commented on PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#issuecomment-1992515204

   You're right there is no rename; copy is all there is. So that is not 
available yet? Hmmm. This isn't ready for production yet is it? Let us keep it 
in trunk for now. The other strategy would be to do a feature branch for it, 
which has mixed benefits. Good: isolated from other work. Bad: isolated from 
other work. So far the changes are minimal enough it is not a problem.
   
   Now that I am working on a bulk delete API targeting Iceberg and similar 
where the caller congener write a bulk delete call across the bucket; currently 
in S3AFS we only do bulk deletes down a "directory tree" either in delete or 
incrementally during rename(). In both of these cases there is already no 
guarantees that your system will be left in a nice state if you don't have the 
permissions to do things. 
   
   Regarding testing, when you think it is ready for others to play with a 
section in testing.md on how to get set up for this would be good. Well I don't 
personally have plans for that, maybe I could persuade colleagues. I tried to 
test a lot of the other corner cases.


-- 
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-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-03-08 Thread via GitHub


hadoop-yetus commented on PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#issuecomment-1985634498

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |::|--:|:|::|:---:|
   | +0 :ok: |  reexec  |   0m 55s |  |  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: |  markdownlint  |   0m  1s |  |  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 _ |
   | +1 :green_heart: |  mvninstall  |  48m  8s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 42s |  |  trunk passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1  |
   | +1 :green_heart: |  compile  |   0m 34s |  |  trunk passed with JDK 
Private Build-1.8.0_392-8u392-ga-1~20.04-b08  |
   | +1 :green_heart: |  checkstyle  |   0m 31s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 41s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   0m 26s |  |  trunk passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 32s |  |  trunk passed with JDK 
Private Build-1.8.0_392-8u392-ga-1~20.04-b08  |
   | +1 :green_heart: |  spotbugs  |   1m  7s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  37m 35s |  |  branch has no errors 
when building and testing our client artifacts.  |
    _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 29s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 35s |  |  the patch passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1  |
   | +1 :green_heart: |  javac  |   0m 35s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 26s |  |  the patch passed with JDK 
Private Build-1.8.0_392-8u392-ga-1~20.04-b08  |
   | +1 :green_heart: |  javac  |   0m 26s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks 
issues.  |
   | +1 :green_heart: |  checkstyle  |   0m 19s |  |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |   0m 31s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   0m 14s |  |  the patch passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu220.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 24s |  |  the patch passed with JDK 
Private Build-1.8.0_392-8u392-ga-1~20.04-b08  |
   | +1 :green_heart: |  spotbugs  |   1m  5s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  37m 51s |  |  patch has no errors 
when building and testing our client artifacts.  |
    _ Other Tests _ |
   | +1 :green_heart: |  unit  |   2m 46s |  |  hadoop-aws in the patch passed. 
 |
   | +1 :green_heart: |  asflicense  |   0m 36s |  |  The patch does not 
generate ASF License warnings.  |
   |  |   | 140m 51s |  |  |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.44 ServerAPI=1.44 base: 
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6544/3/artifact/out/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/hadoop/pull/6544 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall 
mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets markdownlint 
|
   | uname | Linux 799115379a47 5.15.0-94-generic #104-Ubuntu SMP Tue Jan 9 
15:25:40 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / 4247a5e4b9e6923b9b98922decfd5c7904a2aa28 |
   | 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.22+7-post-Ubuntu-0ubuntu220.04.1 
/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-6544/3/testReport/ |
   | Max. process+thread count | 615 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws |
   | Console output | 
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6544/3/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-unsubscr...@hadoop.apache.org

For queries 

Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-03-08 Thread via GitHub


Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-03-08 Thread via GitHub


adnanhemani commented on PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#issuecomment-1985378218

   Hi Steve, thanks for reviewing this as you finished up with the Hadoop 
release work, I really appreciate you taking the time to look at this. I'm 
re-running the integration and unit tests after I made the changes you 
suggested above - I will ping this thread when the code is pushed and ready for 
a re-review. Responding to your earlier questions:
   
   > I'm curious about whether it is possible to test this with an ITest 
everywhere -and what happens? would it be possible to write a test for this?
   
   While it should be technically possible to do so, I'm not sure there's an 
easy (or acceptable) way to do so. In order to test this functionality with an 
ITest, we would need to set up an Access Grants instance, locations, and access 
grants themselves prior to actually running the client-side code, which we are 
integrating. Creating and tearing down those resources would need a good deal 
of effort imo - and do not come for free cost-wise either. Going back to the 
note we've made explicitly in the documentation, if there is a problem with 
functionality, this would be a general problem with the S3 Access Grants plugin 
- and not with S3A, as our unit test is capturing all code used for enabling 
this plugin. I believe the ITests on the S3 Access Grants plugin should then be 
sufficient for covering the full testing coverage.
   
   > now, how does this integrate with the usual auth mechanism? the standard 
credentials passed in are used to auth with (something? what?) to get the 
session credentials.
   
   Good question - when using S3 Access Grants, the flow would be to use your 
standard credentials to authenticate yourself to the S3 Access Grants server. 
Which would then hand you back a new set of credentials once you are authorized 
- this set of credentials should have access to the S3 objects you are looking 
to access. The Access Grants on the S3 Access Grants server are set by your 
account/organization's data admin, who is in charge of determining what sets of 
permissions each user should be able to receive. (I'm not sure if I'm answering 
the right question here - please feel free to elaborate on the question if I 
didn't answer it properly).
   
   > How are complex ops like rename() coped with?
   
   By `rename`, I'm assuming you mean the mechanism where we copy the object to 
the new name and then deleting the original object? If so, CopyObject is not 
supported at this moment but the S3 Access Grants team is working on a new 
revision of the plugin where they are able to support this in more constrained 
situations - such as when the object source and destination fall under the same 
S3 Access Grants prefix. Similar situation for DeleteObjects calls.
   
   I don't believe there's a S3 API specifically for `rename` - please correct 
me if I'm wrong.
   
   If there are any other complex/not-so-popular S3 actions that the user 
attempts to do, the S3 Access Grants plugin will not attempt to do credential 
vending and instead always instruct the S3 Client to fall back to using the 
user's provided authentication credentials.
   
   > When do things fail and how? specifically, are new errors likely to be 
raised when S3 requests are made, and if so is their translation valid?
   
   So given that, functionally, the only thing we are changing is the 
credentials which the user is using to access the S3 data, there should not be 
any changes to the set of errors that can be returned from S3 while attempting 
to access the data itself. However, there could be an error thrown while the 
user is attempting to authorize against the S3 Access Grants server. In this 
case, I believe the only exception the S3 Access Grants plugin is allowed to 
throw is a `SdkServiceException`. But per my understanding, this is simply 
passed onto the S3 client itself, which is fully in control of how to handle 
this error - and will likely send this error back to the user. Exactly which 
exception the user would receive back, I'm not completely sure but per my 
testing, it is (correctly) not causing the user to retry the credential vending 
process. I can research which error specifically this may be.


-- 
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-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-03-08 Thread via GitHub


adnanhemani commented on code in PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#discussion_r1517420760


##
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AccessGrantConfiguration.java:
##
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.s3a;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.test.AbstractHadoopTestBase;
+import org.junit.Assert;
+import org.junit.Test;
+
+import software.amazon.awssdk.awscore.AwsClient;
+import 
software.amazon.awssdk.s3accessgrants.plugin.S3AccessGrantsIdentityProvider;

Review Comment:
   Not really sure, honestly since all the code we are looking to test doesn't 
actually reside in that module. Or am I misunderstanding what you meant by 
"this" meaning the test class?



-- 
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-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-03-08 Thread via GitHub


adnanhemani commented on code in PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#discussion_r1517361600


##
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AccessGrantConfiguration.java:
##
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.s3a;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.test.AbstractHadoopTestBase;
+import org.junit.Assert;
+import org.junit.Test;
+
+import software.amazon.awssdk.awscore.AwsClient;
+import 
software.amazon.awssdk.s3accessgrants.plugin.S3AccessGrantsIdentityProvider;

Review Comment:
   I'm not too sure, given that all the code we are testing does not belong as 
part of the `fs.s3a.auth` module. Or did I misunderstand that by "this" you are 
not referring to the testing class itself?



-- 
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-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-03-07 Thread via GitHub


adnanhemani commented on code in PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#discussion_r1517130806


##
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java:
##
@@ -1624,4 +1624,21 @@ private Constants() {
* Value: {@value}.
*/
   public static final boolean DEFAULT_AWS_S3_CLASSLOADER_ISOLATION = true;
+
+  /**
+   * Flag {@value}
+   * 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.s3accessgrants.enabled";

Review Comment:
   Done.



##
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java:
##
@@ -1624,4 +1624,21 @@ private Constants() {
* Value: {@value}.
*/
   public static final boolean DEFAULT_AWS_S3_CLASSLOADER_ISOLATION = true;
+
+  /**
+   * Flag {@value}
+   * 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.s3accessgrants.enabled";

Review Comment:
   Done.



-- 
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-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-03-07 Thread via GitHub


steveloughran commented on code in PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#discussion_r1516477473


##
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java:
##
@@ -1624,4 +1624,21 @@ private Constants() {
* Value: {@value}.
*/
   public static final boolean DEFAULT_AWS_S3_CLASSLOADER_ISOLATION = true;
+
+  /**
+   * Flag {@value}
+   * 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.s3accessgrants.enabled";

Review Comment:
   1. can you use "fs.s3a.access.grants" as the prefix here and below
   2. It'd be good have s3afs .hasPathCapability() return the enabled flag for 
ease of testing



##
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AccessGrantConfiguration.java:
##
@@ -0,0 +1,107 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.s3a;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.test.AbstractHadoopTestBase;
+import org.junit.Assert;
+import org.junit.Test;
+
+import software.amazon.awssdk.awscore.AwsClient;
+import 
software.amazon.awssdk.s3accessgrants.plugin.S3AccessGrantsIdentityProvider;
+
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+
+import static org.apache.hadoop.fs.s3a.Constants.AWS_S3_ACCESS_GRANTS_ENABLED;
+
+
+/**
+ * Test S3 Access Grants configurations.
+ */
+public class TestS3AccessGrantConfiguration extends AbstractHadoopTestBase {
+  /**
+   * This credential provider will be attached to any client
+   * that has been configured with the S3 Access Grants plugin.
+   * {@link software.amazon.awssdk.s3accessgrants.plugin.S3AccessGrantsPlugin}.
+   */
+  public static final String 
S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS =
+  S3AccessGrantsIdentityProvider.class.getName();
+
+  @Test
+  public void testS3AccessGrantsEnabled() throws IOException, 
URISyntaxException {
+// Feature is explicitly enabled
+AwsClient s3AsyncClient = getAwsClient(createConfig(true), true);
+Assert.assertEquals(

Review Comment:
   1. I prefer AssertJ asserts with useful .description() values in new test 
suites. AssertEquals is not as bad as the others: it does generate a message, 
but more details are good.
   
   2. the same assert and operation is being used everywhere. Factor it out 
into a method and call it where needed.
   



##
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/DefaultS3ClientFactory.java:
##
@@ -401,4 +411,20 @@ private static Region getS3RegionFromEndpoint(final String 
endpoint,
 return Region.of(AWS_S3_DEFAULT_REGION);
   }
 
+  private static , 
ClientT> void
+  applyS3AccessGrantsConfigurations(BuilderT builder, Configuration conf) {
+if (!conf.getBoolean(AWS_S3_ACCESS_GRANTS_ENABLED, false)){

Review Comment:
   define and use a constant `AWS_S3_ACCESS_GRANTS_ENABLED` here.
   
   makes it easier to see/change what the default is in future.



##
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AccessGrantConfiguration.java:
##
@@ -0,0 +1,107 @@
+/*
+ * 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 

Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-03-07 Thread via GitHub


steveloughran commented on PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#issuecomment-1983988376

   Test results. thanks for these. you only need failures. (we don't care about 
successes unless something has got very slow)
   
   * rebase on trunk; you need "HADOOP-19057. S3A: Landsat bucket used in tests 
no longer accessible"
   * A lot of the tests are cases you can turn off, as is done for third-party 
stores. Look at testing.md and in S3ATestUtils to see how things are skipped
   
   For example, for ITestS3ATemporaryCredentials and delegation; set 
test.fs.s3a.sts.enabled false
   There's something similar for ACLs.
   
   


-- 
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-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-02-21 Thread via GitHub


steveloughran commented on PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#issuecomment-1957375191

   really focused purely on 3.4.0 shipping right now, not looking at stuff it 
doesn't need. sorry


-- 
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-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-02-15 Thread via GitHub


adnanhemani commented on PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#issuecomment-1947538335

   Hi @steveloughran, any thoughts on this?


-- 
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-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-02-09 Thread via GitHub


adnanhemani commented on PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#issuecomment-1936795463

   Hi @ahmarsuhail, I've made all changes requested. Please take a look when 
you can.


-- 
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-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-02-09 Thread via GitHub


hadoop-yetus commented on PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#issuecomment-1936789090

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |::|--:|:|::|:---:|
   | +0 :ok: |  reexec  |   0m 51s |  |  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: |  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 _ |
   | +1 :green_heart: |  mvninstall  |  48m 30s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 45s |  |  trunk passed with JDK 
Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04  |
   | +1 :green_heart: |  compile  |   0m 34s |  |  trunk passed with JDK 
Private Build-1.8.0_392-8u392-ga-1~20.04-b08  |
   | +1 :green_heart: |  checkstyle  |   0m 31s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 39s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   0m 26s |  |  trunk passed with JDK 
Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04  |
   | +1 :green_heart: |  javadoc  |   0m 32s |  |  trunk passed with JDK 
Private Build-1.8.0_392-8u392-ga-1~20.04-b08  |
   | +1 :green_heart: |  spotbugs  |   1m  6s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  37m 58s |  |  branch has no errors 
when building and testing our client artifacts.  |
    _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 43s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 35s |  |  the patch passed with JDK 
Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04  |
   | +1 :green_heart: |  javac  |   0m 35s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 26s |  |  the patch passed with JDK 
Private Build-1.8.0_392-8u392-ga-1~20.04-b08  |
   | +1 :green_heart: |  javac  |   0m 26s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks 
issues.  |
   | +1 :green_heart: |  checkstyle  |   0m 21s |  |  the patch passed  |
   | +1 :green_heart: |  mvnsite  |   0m 31s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   0m 15s |  |  the patch passed with JDK 
Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04  |
   | +1 :green_heart: |  javadoc  |   0m 24s |  |  the patch passed with JDK 
Private Build-1.8.0_392-8u392-ga-1~20.04-b08  |
   | +1 :green_heart: |  spotbugs  |   1m  6s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  37m 41s |  |  patch has no errors 
when building and testing our client artifacts.  |
    _ Other Tests _ |
   | +1 :green_heart: |  unit  |   2m 57s |  |  hadoop-aws in the patch passed. 
 |
   | +1 :green_heart: |  asflicense  |   0m 35s |  |  The patch does not 
generate ASF License warnings.  |
   |  |   | 141m 23s |  |  |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.44 ServerAPI=1.44 base: 
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6544/2/artifact/out/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/hadoop/pull/6544 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall 
mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets markdownlint 
|
   | uname | Linux 21c7753b925c 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 / 3e151bda68b2c6bee7f089f96431b576a6cf9143 |
   | 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-6544/2/testReport/ |
   | Max. process+thread count | 612 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws |
   | Console output | 
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6544/2/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-unsubscr...@hadoop.apache.org

For queries about this 

Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-02-09 Thread via GitHub


adnanhemani commented on code in PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#discussion_r1484871939


##
hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/index.md:
##
@@ -614,6 +614,38 @@ If the following property is not set or set to `true`, the 
following exception w
 java.io.IOException: From option fs.s3a.aws.credentials.provider 
java.lang.ClassNotFoundException: Class CustomCredentialsProvider not found
 ```
 
+## S3 Authorization Using S3 Access Grants

Review Comment:
   Think I fixed these. Will recheck the updated Yetus run.



-- 
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-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-02-09 Thread via GitHub


adnanhemani commented on code in PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#discussion_r1484857819


##
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AccessGrantConfiguration.java:
##
@@ -0,0 +1,108 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.s3a;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.test.AbstractHadoopTestBase;
+import org.junit.Assert;
+import org.junit.Test;
+
+import software.amazon.awssdk.awscore.AwsClient;
+import 
software.amazon.awssdk.s3accessgrants.plugin.S3AccessGrantsIdentityProvider;
+import software.amazon.awssdk.services.s3.S3BaseClientBuilder;
+
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+
+import static org.apache.hadoop.fs.s3a.Constants.AWS_S3_ACCESS_GRANTS_ENABLED;
+
+
+/**
+ * Test S3 Access Grants configurations.
+ */
+public class TestS3AccessGrantConfiguration extends AbstractHadoopTestBase {
+/**
+ * This credential provider will be attached to any client
+ * that has been configured with the S3 Access Grants plugin.
+ * {@link 
software.amazon.awssdk.s3accessgrants.plugin.S3AccessGrantsPlugin}.
+ */
+public static final String 
S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS =
+S3AccessGrantsIdentityProvider.class.getName();
+
+@Test
+public void testS3AccessGrantsEnabled() throws IOException, 
URISyntaxException {
+// Feature is explicitly enabled
+AwsClient s3AsyncClient = getAwsClient(createConfig(true), true);
+Assert.assertEquals(
+S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS,
+getCredentialProviderName(s3AsyncClient));
+
+AwsClient s3Client = getAwsClient(createConfig(true), false);
+Assert.assertEquals(
+S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS,
+getCredentialProviderName(s3Client));
+}
+
+@Test
+public void testS3AccessGrantsDisabled() throws IOException, 
URISyntaxException {
+// Disabled by default
+AwsClient s3AsyncDefaultClient = getAwsClient(new Configuration(), 
true);
+Assert.assertNotEquals(
+S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS,
+getCredentialProviderName(s3AsyncDefaultClient));
+
+AwsClient s3DefaultClient = getAwsClient(new Configuration(), true);
+Assert.assertNotEquals(
+S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS,
+getCredentialProviderName(s3DefaultClient));
+
+// Disabled if explicitly set
+AwsClient s3AsyncExplicitlyDisabledClient = 
getAwsClient(createConfig(false), true);
+Assert.assertNotEquals(
+S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS,
+getCredentialProviderName(s3AsyncExplicitlyDisabledClient));
+
+AwsClient s3ExplicitlyDisabledClient = 
getAwsClient(createConfig(false), true);
+Assert.assertNotEquals(
+S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS,
+getCredentialProviderName(s3ExplicitlyDisabledClient));
+}
+
+private Configuration createConfig(boolean s3agEnabled) {

Review Comment:
   > I think you'll need to do removeBaseAndBucketOverrides here before setting 
the value.
   
   I'm not sure about this because I'm starting a new Hadoop Configuration 
object itself rather than the `createConfiguration` methods that we use from 
the S3ATestUtils. In the end, I don't think it matters - because as long as we 
set the S3 Access Grants properties, that's all that matters to us for the 
purpose of this test, no?
   
   > and is there a way to check for if the IAM fallback is set on the client?
   Unfortunately not :( Did a lot of digging but in short, the plugins are 
"applied" to the client. When we apply the S3 Access Grants plugin on the S3 
clients, we get the following identity provider set as the Credential Provider 
for this client: `S3AccessGrantsIdentityProvider`. And in the case of the 
fallback, the fallback flag is only set on the `S3AccessGrantsIdentityProvider` 
class but as a 

Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-02-09 Thread via GitHub


adnanhemani commented on code in PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#discussion_r1484852608


##
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AccessGrantConfiguration.java:
##
@@ -0,0 +1,108 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.s3a;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.test.AbstractHadoopTestBase;
+import org.junit.Assert;
+import org.junit.Test;
+
+import software.amazon.awssdk.awscore.AwsClient;
+import 
software.amazon.awssdk.s3accessgrants.plugin.S3AccessGrantsIdentityProvider;
+import software.amazon.awssdk.services.s3.S3BaseClientBuilder;
+
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+
+import static org.apache.hadoop.fs.s3a.Constants.AWS_S3_ACCESS_GRANTS_ENABLED;
+
+
+/**
+ * Test S3 Access Grants configurations.
+ */
+public class TestS3AccessGrantConfiguration extends AbstractHadoopTestBase {
+/**
+ * This credential provider will be attached to any client
+ * that has been configured with the S3 Access Grants plugin.
+ * {@link 
software.amazon.awssdk.s3accessgrants.plugin.S3AccessGrantsPlugin}.
+ */
+public static final String 
S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS =
+S3AccessGrantsIdentityProvider.class.getName();
+
+@Test
+public void testS3AccessGrantsEnabled() throws IOException, 
URISyntaxException {
+// Feature is explicitly enabled
+AwsClient s3AsyncClient = getAwsClient(createConfig(true), true);
+Assert.assertEquals(
+S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS,
+getCredentialProviderName(s3AsyncClient));
+
+AwsClient s3Client = getAwsClient(createConfig(true), false);
+Assert.assertEquals(
+S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS,
+getCredentialProviderName(s3Client));
+}
+
+@Test
+public void testS3AccessGrantsDisabled() throws IOException, 
URISyntaxException {
+// Disabled by default
+AwsClient s3AsyncDefaultClient = getAwsClient(new Configuration(), 
true);
+Assert.assertNotEquals(
+S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS,
+getCredentialProviderName(s3AsyncDefaultClient));
+
+AwsClient s3DefaultClient = getAwsClient(new Configuration(), true);
+Assert.assertNotEquals(
+S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS,
+getCredentialProviderName(s3DefaultClient));
+
+// Disabled if explicitly set
+AwsClient s3AsyncExplicitlyDisabledClient = 
getAwsClient(createConfig(false), true);
+Assert.assertNotEquals(
+S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS,
+getCredentialProviderName(s3AsyncExplicitlyDisabledClient));
+
+AwsClient s3ExplicitlyDisabledClient = 
getAwsClient(createConfig(false), true);
+Assert.assertNotEquals(
+S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS,
+getCredentialProviderName(s3ExplicitlyDisabledClient));
+}
+
+private Configuration createConfig(boolean s3agEnabled) {
+Configuration conf = new Configuration();
+conf.setBoolean(AWS_S3_ACCESS_GRANTS_ENABLED, s3agEnabled);
+return conf;
+}
+
+private String getCredentialProviderName(AwsClient awsClient) {
+return 
awsClient.serviceClientConfiguration().credentialsProvider().getClass().getName();
+}
+
+private , ClientT> 
AwsClient

Review Comment:
   Yup, changed.



-- 
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-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-02-09 Thread via GitHub


adnanhemani commented on code in PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#discussion_r1484851522


##
hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/index.md:
##
@@ -614,6 +614,38 @@ If the following property is not set or set to `true`, the 
following exception w
 java.io.IOException: From option fs.s3a.aws.credentials.provider 
java.lang.ClassNotFoundException: Class CustomCredentialsProvider not found
 ```
 
+## S3 Authorization Using S3 Access Grants
+
+[S3 Access Grants](https://aws.amazon.com/s3/features/access-grants/) can be 
used to grant accesses to S3 data using IAM Principals.
+In order to enable S3 Access Grants to work with S3A, we enable the 

Review Comment:
   Good call, done!



-- 
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-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-02-09 Thread via GitHub


adnanhemani commented on code in PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#discussion_r1484848907


##
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/DefaultS3ClientFactory.java:
##
@@ -401,4 +411,19 @@ private static Region getS3RegionFromEndpoint(final String 
endpoint,
 return Region.of(AWS_S3_DEFAULT_REGION);
   }
 
+  private static , 
ClientT> void
+  applyS3AccessGrantsConfigurations(BuilderT builder, Configuration conf) {
+if (!conf.getBoolean(AWS_S3_ACCESS_GRANTS_ENABLED, false)){
+  LOG_S3AG_ENABLED.debug("S3 Access Grants plugin is not enabled.");
+  return;
+}
+
+LOG_S3AG_ENABLED.info("S3 Access Grants plugin is enabled.");
+boolean isFallbackEnabled = 
conf.getBoolean(AWS_S3_ACCESS_GRANTS_FALLBACK_TO_IAM_ENABLED, false);
+S3AccessGrantsPlugin accessGrantsPlugin =
+
S3AccessGrantsPlugin.builder().enableFallback(isFallbackEnabled).build();
+builder.addPlugin(accessGrantsPlugin);
+LOG_S3AG_ENABLED.info("S3 Access Grants plugin is added to S3 client with 
fallback: {}", isFallbackEnabled);

Review Comment:
   Good catch, change.



##
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/DefaultS3ClientFactory.java:
##
@@ -401,4 +411,19 @@ private static Region getS3RegionFromEndpoint(final String 
endpoint,
 return Region.of(AWS_S3_DEFAULT_REGION);
   }
 
+  private static , 
ClientT> void
+  applyS3AccessGrantsConfigurations(BuilderT builder, Configuration conf) {
+if (!conf.getBoolean(AWS_S3_ACCESS_GRANTS_ENABLED, false)){
+  LOG_S3AG_ENABLED.debug("S3 Access Grants plugin is not enabled.");
+  return;
+}
+
+LOG_S3AG_ENABLED.info("S3 Access Grants plugin is enabled.");
+boolean isFallbackEnabled = 
conf.getBoolean(AWS_S3_ACCESS_GRANTS_FALLBACK_TO_IAM_ENABLED, false);
+S3AccessGrantsPlugin accessGrantsPlugin =
+
S3AccessGrantsPlugin.builder().enableFallback(isFallbackEnabled).build();
+builder.addPlugin(accessGrantsPlugin);
+LOG_S3AG_ENABLED.info("S3 Access Grants plugin is added to S3 client with 
fallback: {}", isFallbackEnabled);

Review Comment:
   Good catch, changed.



-- 
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-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-02-09 Thread via GitHub


ahmarsuhail commented on PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#issuecomment-1936082879

   @adnanhemani re test failures, just updating the core-site won't be enough 
for some of them, you'll also need the code changes in Steve's PR 
https://github.com/apache/hadoop/pull/6515 , that should get merged soon so 
then you can rebase and retest.


-- 
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-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-02-09 Thread via GitHub


ahmarsuhail commented on code in PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#discussion_r1484099683


##
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/DefaultS3ClientFactory.java:
##
@@ -401,4 +411,19 @@ private static Region getS3RegionFromEndpoint(final String 
endpoint,
 return Region.of(AWS_S3_DEFAULT_REGION);
   }
 
+  private static , 
ClientT> void
+  applyS3AccessGrantsConfigurations(BuilderT builder, Configuration conf) {
+if (!conf.getBoolean(AWS_S3_ACCESS_GRANTS_ENABLED, false)){
+  LOG_S3AG_ENABLED.debug("S3 Access Grants plugin is not enabled.");
+  return;
+}
+
+LOG_S3AG_ENABLED.info("S3 Access Grants plugin is enabled.");
+boolean isFallbackEnabled = 
conf.getBoolean(AWS_S3_ACCESS_GRANTS_FALLBACK_TO_IAM_ENABLED, false);
+S3AccessGrantsPlugin accessGrantsPlugin =
+
S3AccessGrantsPlugin.builder().enableFallback(isFallbackEnabled).build();
+builder.addPlugin(accessGrantsPlugin);
+LOG_S3AG_ENABLED.info("S3 Access Grants plugin is added to S3 client with 
fallback: {}", isFallbackEnabled);

Review Comment:
   this won't log, cause you have already used the only once on line 421. Cut 
the log on 421, and just keep this one. 
   
   Update text to "S3Access Grants plugin is enabled with IAM fallback set to 
{} "



##
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestS3AccessGrantConfiguration.java:
##
@@ -0,0 +1,108 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.fs.s3a;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.test.AbstractHadoopTestBase;
+import org.junit.Assert;
+import org.junit.Test;
+
+import software.amazon.awssdk.awscore.AwsClient;
+import 
software.amazon.awssdk.s3accessgrants.plugin.S3AccessGrantsIdentityProvider;
+import software.amazon.awssdk.services.s3.S3BaseClientBuilder;
+
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+
+import static org.apache.hadoop.fs.s3a.Constants.AWS_S3_ACCESS_GRANTS_ENABLED;
+
+
+/**
+ * Test S3 Access Grants configurations.
+ */
+public class TestS3AccessGrantConfiguration extends AbstractHadoopTestBase {
+/**
+ * This credential provider will be attached to any client
+ * that has been configured with the S3 Access Grants plugin.
+ * {@link 
software.amazon.awssdk.s3accessgrants.plugin.S3AccessGrantsPlugin}.
+ */
+public static final String 
S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS =
+S3AccessGrantsIdentityProvider.class.getName();
+
+@Test
+public void testS3AccessGrantsEnabled() throws IOException, 
URISyntaxException {
+// Feature is explicitly enabled
+AwsClient s3AsyncClient = getAwsClient(createConfig(true), true);
+Assert.assertEquals(
+S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS,
+getCredentialProviderName(s3AsyncClient));
+
+AwsClient s3Client = getAwsClient(createConfig(true), false);
+Assert.assertEquals(
+S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS,
+getCredentialProviderName(s3Client));
+}
+
+@Test
+public void testS3AccessGrantsDisabled() throws IOException, 
URISyntaxException {
+// Disabled by default
+AwsClient s3AsyncDefaultClient = getAwsClient(new Configuration(), 
true);
+Assert.assertNotEquals(
+S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS,
+getCredentialProviderName(s3AsyncDefaultClient));
+
+AwsClient s3DefaultClient = getAwsClient(new Configuration(), true);
+Assert.assertNotEquals(
+S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS,
+getCredentialProviderName(s3DefaultClient));
+
+// Disabled if explicitly set
+AwsClient s3AsyncExplicitlyDisabledClient = 
getAwsClient(createConfig(false), true);
+Assert.assertNotEquals(
+S3_ACCESS_GRANTS_EXPECTED_CREDENTIAL_PROVIDER_CLASS,
+getCredentialProviderName(s3AsyncExplicitlyDisabledClient));
+
+AwsClient s3ExplicitlyDisabledClient 

Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-02-09 Thread via GitHub


hadoop-yetus commented on PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#issuecomment-1935575917

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |::|--:|:|::|:---:|
   | +0 :ok: |  reexec  |   0m 52s |  |  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: |  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 _ |
   | +1 :green_heart: |  mvninstall  |  49m 45s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |   0m 42s |  |  trunk passed with JDK 
Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04  |
   | +1 :green_heart: |  compile  |   0m 34s |  |  trunk passed with JDK 
Private Build-1.8.0_392-8u392-ga-1~20.04-b08  |
   | +1 :green_heart: |  checkstyle  |   0m 31s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   0m 40s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   0m 26s |  |  trunk passed with JDK 
Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04  |
   | +1 :green_heart: |  javadoc  |   0m 33s |  |  trunk passed with JDK 
Private Build-1.8.0_392-8u392-ga-1~20.04-b08  |
   | +1 :green_heart: |  spotbugs  |   1m  6s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  38m  4s |  |  branch has no errors 
when building and testing our client artifacts.  |
    _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 28s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 35s |  |  the patch passed with JDK 
Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04  |
   | +1 :green_heart: |  javac  |   0m 35s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 26s |  |  the patch passed with JDK 
Private Build-1.8.0_392-8u392-ga-1~20.04-b08  |
   | +1 :green_heart: |  javac  |   0m 26s |  |  the patch passed  |
   | -1 :x: |  blanks  |   0m  0s | 
[/blanks-eol.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6544/1/artifact/out/blanks-eol.txt)
 |  The patch has 7 line(s) that end in blanks. Use git apply --whitespace=fix 
<>. Refer https://git-scm.com/docs/git-apply  |
   | -0 :warning: |  checkstyle  |   0m 20s | 
[/results-checkstyle-hadoop-tools_hadoop-aws.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6544/1/artifact/out/results-checkstyle-hadoop-tools_hadoop-aws.txt)
 |  hadoop-tools/hadoop-aws: The patch generated 36 new + 2 unchanged - 0 fixed 
= 38 total (was 2)  |
   | +1 :green_heart: |  mvnsite  |   0m 33s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   0m 15s |  |  the patch passed with JDK 
Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04  |
   | +1 :green_heart: |  javadoc  |   0m 24s |  |  the patch passed with JDK 
Private Build-1.8.0_392-8u392-ga-1~20.04-b08  |
   | +1 :green_heart: |  spotbugs  |   1m 11s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  38m  8s |  |  patch has no errors 
when building and testing our client artifacts.  |
    _ Other Tests _ |
   | +1 :green_heart: |  unit  |   3m  0s |  |  hadoop-aws in the patch passed. 
 |
   | +1 :green_heart: |  asflicense  |   0m 35s |  |  The patch does not 
generate ASF License warnings.  |
   |  |   | 143m  8s |  |  |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.44 ServerAPI=1.44 base: 
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6544/1/artifact/out/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/hadoop/pull/6544 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall 
mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets markdownlint 
|
   | uname | Linux 07da37719acc 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 / 33b3350e7bbc1fd328ea085991bb51c3af36f3ca |
   | 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-6544/1/testReport/ |
   | Max. process+thread count | 529 (vs. ulimit of 5500) |
   | modules | C: hadoop-tools/hadoop-aws U: hadoop-tools/hadoop-aws |
   | Console output | 
https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6544/1/console 

Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-02-08 Thread via GitHub


adnanhemani commented on PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#issuecomment-1935468938

   WRT ITest failures, this is the `auth-keys.xml` file I'm using as per the 
suggestions on #6515. But I'm still getting the above failures - should this 
not be the case?
   
   Tested on PDX, `us-west-2`.
   
   ```
   
   
   
   test.fs.s3a.name
   
   
   
   
   fs.contract.test.fs.s3a
   ${test.fs.s3a.name}
   
   
   
   fs.s3a.access.key
   AWS access key ID. Omit for IAM role-based 
authentication.
   
   
   
   
   fs.s3a.secret.key
   AWS secret key. Omit for IAM role-based 
authentication.
   
   
   
   
   fs.s3a.session.token
   AWS secret key. Omit for IAM role-based 
authentication.
  
   
   
   
   test.sts.endpoint
   Specific endpoint to use for STS requests.
   sts.amazonaws.com
   
   
   
   fs.s3a.endpoint.region
   us-west-2
   
   
   
   fs.s3a.scale.test.csvfile
   s3a://noaa-cors-pds/raw/2024/001/akse/AKSE001x.24_.gz
   file used in scale tests
   
   
   fs.s3a.bucket.noaa-cors-pds.endpoint.region
   us-east-1
   
   
   fs.s3a.bucket.noaa-isd-pds.multipart.purge
   false
   Don't try to purge uploads in the read-only bucket, as
   it will only create log noise.
   
   
   fs.s3a.bucket.noaa-isd-pds.probe
   0
   Let's postpone existence checks to the first IO 
operation 
   
   
   fs.s3a.bucket.noaa-isd-pds.audit.add.referrer.header
   false
   Do not add the referrer header
   
   
   fs.s3a.bucket.noaa-isd-pds.prefetch.block.size
   128k
   Use a small prefetch size so tests fetch multiple 
blocks
   
   
   
   ```


-- 
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-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-02-08 Thread via GitHub


adnanhemani commented on PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#issuecomment-1935462396

   Hi @ahmarsuhail and @steveloughran - I've opened this with the new code that 
has the S3 Access Grants plugin as part of the AWS Java SDK bundle (this is an 
evolution of #6507). We should consider #6507 deprecated as of now, I believe I 
have addressed all feedback from that PR on here. Please let me know your 
thoughts!


-- 
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-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org



Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-02-08 Thread via GitHub


adnanhemani commented on PR #6544:
URL: https://github.com/apache/hadoop/pull/6544#issuecomment-1935460439

   Unit tests and Tests results below, per my understanding all failures are 
due to various other known reasons:
   ```
   ^C88665a0afd7e:hadoop-aws ahemani$ mvn verify -Dparallel-tests -Dprefetch 
-DtestsThreadCount=8
   Picked up JAVA_TOOL_OPTIONS: -Dlog4j2.formatMsgNoLookups=true
   [INFO] Scanning for projects...
   [INFO] 

   [INFO] Detecting the operating system and CPU architecture
   [INFO] 

   [INFO] os.detected.name: osx
   [INFO] os.detected.arch: x86_64
   [INFO] os.detected.bitness: 64
   [INFO] os.detected.version: 14.2
   [INFO] os.detected.version.major: 14
   [INFO] os.detected.version.minor: 2
   [INFO] os.detected.classifier: osx-x86_64
   [INFO] 
   [INFO] < org.apache.hadoop:hadoop-aws 
>
   [INFO] Building Apache Hadoop Amazon Web Services support 3.5.0-SNAPSHOT
   [INFO] [ jar 
]-
   ...
   [INFO] ---
   [INFO]  T E S T S
   [INFO] ---
   [INFO] Running 
org.apache.hadoop.fs.s3a.commit.staging.TestStagingPartitionedTaskCommit
   [INFO] Running 
org.apache.hadoop.fs.s3a.commit.staging.TestDirectoryCommitterScale
   [INFO] Running 
org.apache.hadoop.fs.s3a.commit.staging.TestStagingDirectoryOutputCommitter
   [INFO] Running 
org.apache.hadoop.fs.s3a.commit.staging.TestStagingPartitionedJobCommit
   [INFO] Running org.apache.hadoop.fs.s3a.commit.staging.TestStagingCommitter
   [INFO] Running org.apache.hadoop.fs.s3a.commit.TestMagicCommitPaths
   [INFO] Running 
org.apache.hadoop.fs.s3a.commit.staging.TestStagingPartitionedFileListing
   [INFO] Running org.apache.hadoop.fs.s3a.commit.staging.TestPaths
   [INFO] Tests run: 28, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
0.387 s - in org.apache.hadoop.fs.s3a.commit.TestMagicCommitPaths
   [INFO] Tests run: 14, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
1.246 s - in org.apache.hadoop.fs.s3a.commit.staging.TestPaths
   [INFO] Running org.apache.hadoop.fs.s3a.impl.TestAwsClientConfig
   [INFO] Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.694 
s - in org.apache.hadoop.fs.s3a.impl.TestAwsClientConfig
   [INFO] Running org.apache.hadoop.fs.s3a.impl.TestHeaderProcessing
   [INFO] Running org.apache.hadoop.fs.s3a.impl.TestCreateFileBuilder
   [INFO] Tests run: 10, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
1.715 s - in org.apache.hadoop.fs.s3a.impl.TestHeaderProcessing
   [INFO] Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.679 
s - in org.apache.hadoop.fs.s3a.impl.TestCreateFileBuilder
   [INFO] Running org.apache.hadoop.fs.s3a.impl.TestErrorTranslation
   [INFO] Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.475 
s - in org.apache.hadoop.fs.s3a.impl.TestErrorTranslation
   [INFO] Running org.apache.hadoop.fs.s3a.impl.TestS3AMultipartUploaderSupport
   [INFO] Tests run: 8, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 7.71 
s - in 
org.apache.hadoop.fs.s3a.commit.staging.TestStagingDirectoryOutputCommitter
   [INFO] Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.408 
s - in org.apache.hadoop.fs.s3a.impl.TestS3AMultipartUploaderSupport
   [INFO] Running org.apache.hadoop.fs.s3a.impl.TestRequestFactory
   [INFO] Running org.apache.hadoop.fs.s3a.impl.TestDirectoryMarkerPolicy
   [INFO] Running org.apache.hadoop.fs.s3a.impl.TestOpenFileSupport
   [INFO] Tests run: 9, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.633 
s - in org.apache.hadoop.fs.s3a.impl.TestDirectoryMarkerPolicy
   [INFO] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 1.485 
s - in org.apache.hadoop.fs.s3a.impl.TestRequestFactory
   [INFO] Tests run: 19, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
0.785 s - in org.apache.hadoop.fs.s3a.impl.TestOpenFileSupport
   [INFO] Running org.apache.hadoop.fs.s3a.impl.TestSDKStreamDrainer
   [INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 11.23 
s - in org.apache.hadoop.fs.s3a.commit.staging.TestStagingPartitionedFileListing
   [INFO] Running org.apache.hadoop.fs.s3a.impl.TestNetworkBinding
   [INFO] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
11.444 s - in 
org.apache.hadoop.fs.s3a.commit.staging.TestStagingPartitionedJobCommit
   [INFO] Tests run: 10, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 
0.667 s - in org.apache.hadoop.fs.s3a.impl.TestSDKStreamDrainer
   [INFO] Tests run: 4, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.461 
s - in org.apache.hadoop.fs.s3a.impl.TestNetworkBinding
   [INFO] Running org.apache.hadoop.fs.s3a.impl.TestS3ExpressStorage
   [INFO] Tests run: 3, Failures: 0, 

[PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]

2024-02-08 Thread via GitHub


adnanhemani opened a new pull request, #6544:
URL: https://github.com/apache/hadoop/pull/6544

   
   
   ### Description of PR
   This adds S3 Access Grants support in S3A using the S3 Access Grants plugin, 
which is a part of the AWS Java SDK bundle starting in v2.23.19. As part of 
this PR, we are adding new configuration flags that the user can use to enable 
the usage of the S3 Access Grants plugin on the S3 clients that S3A starts up.
   
   ### How was this patch tested?
   New unit tests written. Unit testing and integration testing patches 
incoming momentarily.
   
   ### For code changes:
   
   - [X] Does the title or this PR starts with the corresponding JIRA issue id 
(e.g. 'HADOOP-17799. Your PR title ...')?
   - [X] Object storage: have the integration tests been executed and the 
endpoint declared according to the connector-specific documentation?
   - [N/A] 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)?
   - [N/A] 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-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org