[
https://issues.apache.org/jira/browse/HADOOP-17198?focusedWorklogId=634104&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-634104
]
ASF GitHub Bot logged work on HADOOP-17198:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 05/Aug/21 11:12
Start Date: 05/Aug/21 11:12
Worklog Time Spent: 10m
Work Description: steveloughran commented on a change in pull request
#3260:
URL: https://github.com/apache/hadoop/pull/3260#discussion_r682650167
##########
File path:
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/ArnResource.java
##########
@@ -0,0 +1,132 @@
+/*
+ * 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 javax.annotation.Nonnull;
+
+import com.amazonaws.arn.Arn;
+import com.amazonaws.regions.RegionUtils;
+
+/**
+ * Represents an Arn Resource, this can be an accesspoint or bucket.
+ */
+public class ArnResource {
+
+ /**
+ * Resource name.
+ */
+ private final String name;
+
+ /**
+ * Resource owner account id.
+ */
+ private final String ownerAccountId;
+
+ /**
+ * Resource region.
+ */
+ private final String region;
+
+ /**
+ * Full Arn for the resource.
+ */
+ private final String fullArn;
+
+ /**
+ * Partition for the resource. Allowed partitions: aws, aws-cn, aws-us-gov
+ */
+ private final String partition;
+
+ /**
+ * Because of the different ways an endpoint can be constructed depending on
partition we're
+ * relying on the AWS SDK to produce the endpoint. In this case we need a
region key of the form
+ * {@code String.format("accesspoint-%s", awsRegion)}
+ */
+ private final String accessPointRegionKey;
+
+ private ArnResource(String name, String owner, String region, String
partition, String fullArn) {
+ this.name = name;
+ this.ownerAccountId = owner;
+ this.region = region;
+ this.partition = partition;
+ this.fullArn = fullArn;
+ this.accessPointRegionKey = String.format("accesspoint-%s", region);
+ }
+
+ /**
+ * Resource name.
+ * @return resource name.
+ */
+ public String getName() {
+ return name;
+ }
+
+ /**
+ * Return owner's account id.
+ */
+ public String getOwnerAccountId() {
+ return ownerAccountId;
+ }
+
+ /**
+ * Resource region.
+ * @return resource region.
+ */
+ public String getRegion() {
+ return region;
+ }
+
+ /**
+ * Full arn for resource.
+ * @return arn for resource.
+ */
+ public String getFullArn() {
+ return fullArn;
+ }
+
+ /**
+ * Formatted endpoint for the resource.
+ * @return resource endpoint.
+ */
+ public String getEndpoint() {
+ return RegionUtils.getRegion(accessPointRegionKey)
+ .getServiceEndpoint("s3");
+ }
+
+ /**
+ * Parses the passed `arn` string into a full ArnResource
+ * @param arn - string representing an Arn resource
+ * @throws IllegalArgumentException - if the Arn is malformed or any of the
region, accountId and
Review comment:
nit: move below @return
##########
File path:
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
##########
@@ -400,6 +410,14 @@ public void initialize(URI name, Configuration
originalConf)
LOG.debug("Initializing S3AFileSystem for {}", bucket);
// clone the configuration into one with propagated bucket options
Configuration conf = propagateBucketOptions(originalConf, bucket);
+
+ String apArn = conf.getTrimmed(ACCESS_POINT_ARN, "");
+ if (!apArn.isEmpty()) {
+ accessPoint = ArnResource.accessPointFromArn(apArn);
+ LOG.info("Using AccessPoint ARN \"{}\" for bucket {}", apArn, bucket);
Review comment:
not sure about info vs debug; info is safest.
the .toString() value should also include it.
##########
File path:
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AConfiguration.java
##########
@@ -361,6 +362,14 @@ public void
shouldBeAbleToSwitchOnS3PathStyleAccessViaConfigProperty()
// isn't in the same region as the s3 client default. See
// http://docs.aws.amazon.com/AmazonS3/latest/dev/VirtualHosting.html
assertEquals(HttpStatus.SC_MOVED_PERMANENTLY, e.getStatusCode());
+ } catch (final IllegalArgumentException e) {
+ // Path style addressing does not work with AP ARNs
+ if (conf.get(ACCESS_POINT_ARN, "").isEmpty()) {
+ LOG.error("Caught unexpected exception: ", e);
+ throw e;
+ }
+
+ assertTrue(e.getMessage().contains("ARN of type accesspoint cannot be
passed as a bucket"));
Review comment:
there's a GenericTestUtils check which will rethrow the exception if it
doesn't contain the string; ensures the root exception isn't lost
##########
File path:
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestArnResource.java
##########
@@ -0,0 +1,73 @@
+/*
+ * 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 com.amazonaws.regions.Regions;
+import org.junit.Assert;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.apache.hadoop.test.LambdaTestUtils.intercept;
+
+public class TestArnResource extends Assert {
Review comment:
extend org.apache.hadoop.test.HadoopTestBase to get thread naming and
timeout
##########
File path:
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
##########
@@ -360,6 +365,11 @@
private AuditManagerS3A auditManager =
AuditIntegration.stubAuditManager();
+ /**
+ * Bucket AccessPoint
Review comment:
nit: add .
##########
File path:
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AContractGetFileStatusV1List.java
##########
@@ -54,6 +60,9 @@ protected Configuration createConfiguration() {
// Use v1 List Objects API
conf.setInt(LIST_VERSION, 1);
+ if (!conf.get(ACCESS_POINT_ARN, "").isEmpty()) {
Review comment:
`S3ATestUtils.assume("skipped due to ARN", <condition>`
##########
File path:
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
##########
@@ -1154,7 +1197,8 @@ public String getBucketLocation(String bucketName) throws
IOException {
final String region = trackDurationAndSpan(
STORE_EXISTS_PROBE, bucketName, null, () ->
invoker.retry("getBucketLocation()", bucketName, true, () ->
- s3.getBucketLocation(bucketName)));
+ // If accessPoint then region is known from Arn
+ accessPoint != null ? accessPoint.getRegion() :
s3.getBucketLocation(bucketName)));
Review comment:
nit: split on lines with ? and : at start, to make more obvious this is
a ?: sequence
##########
File path: hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/index.md
##########
@@ -1576,6 +1576,81 @@ Why explicitly declare a bucket bound to the central
endpoint? It ensures
that if the default endpoint is changed to a new region, data store in
US-east is still reachable.
+## <a name="accesspoints"></a>Configuring S3 AccessPoints usage with S3a
+S3a now supports [S3 Access
Point](https://aws.amazon.com/s3/features/access-points/) usage which
+improves VPC integration with S3 and simplifies your data's permission model
because different
+policies can be applied now on the Access Point level. For more information
about why to use them
+make sure to read the official documentation.
+
+Accessing data through an access point, is done by using its ARN, as opposed
to just the bucket name.
+You can set the Access Point ARN property using the following configuration
property:
+```xml
+<property>
+ <name>fs.s3a.accesspoint.arn</name>
+ <value> {ACCESSPOINT_ARN_HERE} </value>
+ <description>Configure S3a traffic to use this AccessPoint</description>
+</property>
+```
+
+Be mindful that this configures **all access** to S3a, and in turn S3, to go
through that ARN.
+So for example `s3a://yourbucket/key` will now use your configured ARN when
getting data from S3
+instead of your bucket. The flip side to this is that if you're working with
multiple buckets
Review comment:
presumably you'd always want to do a per-bucket mapping, as otherwise
you don't get 1 AP ARN.
Maybe we should mandate that it MUST always be on a per-bucket basis (looked
up via a conf.getTrimmedString("fs.s3a." + bucket + ".access-point.arn", "")
we would have no global ARN, so no bugreps about always getting the same
bucket
##########
File path:
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
##########
@@ -2570,6 +2614,11 @@ protected S3ListResult continueListObjects(S3ListRequest
request,
OBJECT_CONTINUE_LIST_REQUEST,
() -> {
if (useListV1) {
+ if (accessPoint != null) {
+ // AccessPoints are not compatible with V1List
+ throw new InvalidRequestException("ListV1 is not supported
by AccessPoints");
Review comment:
do you think maybe during initialize it should just downgrade?
##########
File path:
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java
##########
@@ -1098,4 +1098,11 @@ private Constants() {
*/
public static final String AWS_S3_CENTRAL_REGION = "us-east-1";
+ /**
+ * AccessPoint ARN for the bucket. When set globally, all requests to S3 are
going through
+ * the AccessPoint pointed to by the ARN. If set as a bucket override then
only the requests for
+ * that bucket will go through the AccessPoint. Default value " ".
Review comment:
default is "", not " " ?
Prefer a separate default const, and for this constant add a @value so any
javadocs show it
##########
File path:
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AFileSystem.java
##########
@@ -730,11 +749,24 @@ protected void verifyBucketExists()
*/
@Retries.RetryTranslated
protected void verifyBucketExistsV2()
- throws UnknownStoreException, IOException {
+ throws UnknownStoreException, IOException {
if (!invoker.retry("doesBucketExistV2", bucket, true,
trackDurationOfOperation(getDurationTrackerFactory(),
STORE_EXISTS_PROBE.getSymbol(),
- () -> s3.doesBucketExistV2(bucket)))) {
+ () -> {
+ // Bug in SDK always returns `true` for AccessPoint ARNs with
`doesBucketExistV2()`
+ // expanding implementation to use ARNs and buckets correctly
Review comment:
aah
--
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
Issue Time Tracking
-------------------
Worklog Id: (was: 634104)
Time Spent: 1h 50m (was: 1h 40m)
> Support S3 Access Points
> ------------------------
>
> Key: HADOOP-17198
> URL: https://issues.apache.org/jira/browse/HADOOP-17198
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: fs/s3
> Affects Versions: 3.3.0
> Reporter: Steve Loughran
> Assignee: Bogdan Stolojan
> Priority: Major
> Labels: pull-request-available
> Time Spent: 1h 50m
> Remaining Estimate: 0h
>
> Improve VPC integration by supporting access points for buckets
> https://docs.aws.amazon.com/AmazonS3/latest/dev/access-points.html
> Not sure how to do this *at all*;
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
