Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]
singhpk234 commented on PR #6544: URL: https://github.com/apache/hadoop/pull/6544#issuecomment-2257342895 @steveloughran thanks for bringing this up, Thanks @adnanhemani for connecting us. For S3Access Grants we need SDK version 2.25.70 atleast for this as called out here (they added Copy obj support ...etc): https://github.com/apache/hadoop/pull/6900#issuecomment-2189403812 -- 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]
singhpk234 commented on PR #6544: URL: https://github.com/apache/hadoop/pull/6544#issuecomment-2257341062 @steveloughran thanks for bringing this up, Thanks @adnanhemani for connecting us. For S3Access Grants we need SDK version 2.25.70 atleast for this as called out here (they added Copy obj support ...etc): https://github.com/apache/hadoop/pull/6900#issuecomment-2189403812 Let me get back with precise confirmation if that's all, we need to verify internally as well everything works its pending testing internally which is stalled due to internal re:orgs -- 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]
adnanhemani commented on PR #6544: URL: https://github.com/apache/hadoop/pull/6544#issuecomment-2257332416 Unfortunately, I think there are further changes that are still missing from the AWS SDK to unblock all code paths. But I've moved off of this work recently - I'll let the new owner comment @singhpk234. -- 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]
steveloughran commented on PR #6544: URL: https://github.com/apache/hadoop/pull/6544#issuecomment-2242904433 @adnanhemani hadoop branch-3.4 AWS SDK is now at 2.25.53. That has everything you need, doesn't it? If you can do a request of this patch back part branch 3.4 I will merge it and we can hopefully get it into 3.4.1 release -- 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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
Re: [PR] HADOOP-19050. Add S3 Access Grants Support in S3A [hadoop]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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