Skip to content

Commit 2f5901c

Browse files
author
Bogdan Stolojan
committed
Removing reliance on SDK
Turns out last SDK upgrade broke something it shouldn't and proved how brittle some parts of this code base are. One thing that's not brittle is the endpoint we make requests to. Endpoint seems to be quite easy to generate, thus we're moving away from using the SDK in a "nice" way and we're doing it for ourselves, since it's easier to understand what's happening (and it's 1 line of code).
1 parent 77dd103 commit 2f5901c

File tree

2 files changed

+39
-22
lines changed

2 files changed

+39
-22
lines changed

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/ArnResource.java

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,12 @@
2121
import javax.annotation.Nonnull;
2222

2323
import com.amazonaws.arn.Arn;
24-
import com.amazonaws.regions.RegionUtils;
2524

2625
/**
2726
* Represents an Arn Resource, this can be an accesspoint or bucket.
2827
*/
2928
public final class ArnResource {
29+
private final static String ACCESSPOINT_ENDPOINT_FORMAT = "s3-accesspoint.%s.amazonaws.com";
3030

3131
/**
3232
* Resource name.
@@ -106,11 +106,7 @@ public String getFullArn() {
106106
* @return resource endpoint.
107107
*/
108108
public String getEndpoint() {
109-
return RegionUtils.getRegion(accessPointRegionKey)
110-
.getServiceEndpoint("s3")
111-
// There's a slight issue with getServiceEndpoint which breaks an endpoint related to
112-
// access points, i.e. the correct one starts with "s3-accesspoint."
113-
.replace("s3.accesspoint-", "s3-accesspoint.");
109+
return String.format(ACCESSPOINT_ENDPOINT_FORMAT, region);
114110
}
115111

116112
/**

hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/TestArnResource.java

Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -39,39 +39,43 @@
3939
public class TestArnResource extends HadoopTestBase {
4040
private final static Logger LOG = LoggerFactory.getLogger(TestArnResource.class);
4141

42+
private final static String MOCK_ACCOUNT = "123456789101";
43+
4244
@Test
4345
public void parseAccessPointFromArn() throws IllegalArgumentException {
4446
describe("Parse AccessPoint ArnResource from arn string");
4547

4648
String accessPoint = "testAp";
47-
String accountId = "123456789101";
4849
String[][] regionPartitionEndpoints = new String[][] {
49-
{Regions.EU_WEST_1.getName(), "aws", "s3-accesspoint.eu-west-1.amazonaws.com"},
50-
{Regions.US_GOV_EAST_1.getName(), "aws-us-gov",
51-
"s3-accesspoint.us-gov-east-1.amazonaws.com"},
52-
{Regions.CN_NORTH_1.getName(), "aws-cn", "s3-accesspoint.cn-north-1.amazonaws.com"},
50+
{Regions.EU_WEST_1.getName(), "aws"},
51+
{Regions.US_GOV_EAST_1.getName(), "aws-us-gov"},
52+
{Regions.CN_NORTH_1.getName(), "aws-cn"},
5353
};
5454

5555
for (String[] testPair : regionPartitionEndpoints) {
5656
String region = testPair[0];
5757
String partition = testPair[1];
58-
String endpoint = testPair[2];
59-
60-
// arn:partition:service:region:account-id:resource-type/resource-id
61-
String arn = String.format("arn:%s:s3:%s:%s:accesspoint/%s", partition, region, accountId,
62-
accessPoint);
6358

64-
ArnResource resource = ArnResource.accessPointFromArn(arn);
65-
assertEquals("Arn does not match", arn, resource.getFullArn());
59+
ArnResource resource = getArnResourceFrom(partition, region, MOCK_ACCOUNT, accessPoint);
6660
assertEquals("Access Point name does not match", accessPoint, resource.getName());
67-
assertEquals("Account Id does not match", accountId, resource.getOwnerAccountId());
61+
assertEquals("Account Id does not match", MOCK_ACCOUNT, resource.getOwnerAccountId());
6862
assertEquals("Region does not match", region, resource.getRegion());
69-
Assertions.assertThat(resource.getEndpoint())
70-
.describedAs("Endpoint does not match")
71-
.contains(endpoint);
7263
}
7364
}
7465

66+
@Test
67+
public void makeSureEndpointHasTheCorrectFormat() {
68+
// Access point (AP) endpoints are different from S3 bucket endpoints, thus when using APs the
69+
// endpoints for the client are modified. This test makes sure endpoint is set up correctly.
70+
ArnResource accessPoint = getArnResourceFrom("aws", "eu-west-1", MOCK_ACCOUNT,
71+
"test");
72+
String expected = "s3-accesspoint.eu-west-1.amazonaws.com";
73+
74+
Assertions.assertThat(accessPoint.getEndpoint())
75+
.describedAs("Endpoint has invalid format. Access Point requests will not work")
76+
.isEqualTo(expected);
77+
}
78+
7579
@Test
7680
public void invalidARNsMustThrow() throws Exception {
7781
describe("Using an invalid ARN format must throw when initializing an ArnResource.");
@@ -80,6 +84,23 @@ public void invalidARNsMustThrow() throws Exception {
8084
ArnResource.accessPointFromArn("invalid:arn:resource"));
8185
}
8286

87+
/**
88+
* Create an {@link ArnResource} from string components
89+
* @param partition - partition for ARN
90+
* @param region - region for ARN
91+
* @param accountId - accountId for ARN
92+
* @param resourceName - ARN resource name
93+
* @return ArnResource described by its properties
94+
*/
95+
private ArnResource getArnResourceFrom(String partition, String region, String accountId,
96+
String resourceName) {
97+
// arn:partition:service:region:account-id:resource-type/resource-id
98+
String arn = String.format("arn:%s:s3:%s:%s:accesspoint/%s", partition, region, accountId,
99+
resourceName);
100+
101+
return ArnResource.accessPointFromArn(arn);
102+
}
103+
83104
private void describe(String message) {
84105
LOG.info(message);
85106
}

0 commit comments

Comments
 (0)