-
Notifications
You must be signed in to change notification settings - Fork 9.2k
HADOOP-19044. AWS SDK V2 - Update S3A region logic #6479
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 18 commits
872dcac
d8c9793
cefb30b
81d345b
05225bb
2c653da
99cf2d4
12ff2f1
fcf56cf
6197723
adb10d3
54e5c66
e65a51a
2b89855
5fdb23d
b1f4df9
44760ca
12e503e
09ff933
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -228,6 +228,8 @@ If you do any of these: change your credentials immediately! | |
|
|
||
| See [Connecting to an Amazon S3 Bucket through the S3A Connector](connecting.md). | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that should end with .html (I know, not your change. mine)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
|
|
||
| Also, please check [S3 endpoint and region settings in detail](connecting.html#s3_endpoint_region_details). | ||
|
|
||
| ## <a name="authenticating"></a> Authenticating with S3 | ||
|
|
||
| Except when interacting with public S3 buckets, the S3A client | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,13 +38,20 @@ | |
| import software.amazon.awssdk.services.s3.model.HeadBucketResponse; | ||
|
|
||
| import org.apache.hadoop.conf.Configuration; | ||
| import org.apache.hadoop.fs.FSDataInputStream; | ||
| import org.apache.hadoop.fs.FSDataOutputStream; | ||
| import org.apache.hadoop.fs.Path; | ||
| import org.apache.hadoop.fs.s3a.statistics.impl.EmptyS3AStatisticsContext; | ||
| import org.apache.hadoop.fs.s3a.test.PublicDatasetTestUtils; | ||
|
|
||
| import static org.apache.hadoop.fs.s3a.Constants.ALLOW_REQUESTER_PAYS; | ||
| import static org.apache.hadoop.fs.s3a.Constants.AWS_REGION; | ||
| import static org.apache.hadoop.fs.s3a.Constants.CENTRAL_ENDPOINT; | ||
| import static org.apache.hadoop.fs.s3a.Constants.ENDPOINT; | ||
| import static org.apache.hadoop.fs.s3a.Constants.PATH_STYLE_ACCESS; | ||
| import static org.apache.hadoop.fs.s3a.DefaultS3ClientFactory.ERROR_ENDPOINT_WITH_FIPS; | ||
| import static org.apache.hadoop.fs.s3a.S3ATestUtils.removeBaseAndBucketOverrides; | ||
| import static org.apache.hadoop.fs.s3a.test.PublicDatasetTestUtils.DEFAULT_REQUESTER_PAYS_BUCKET_NAME; | ||
| import static org.apache.hadoop.io.IOUtils.closeStream; | ||
| import static org.apache.hadoop.test.LambdaTestUtils.intercept; | ||
|
|
||
|
|
@@ -146,11 +153,28 @@ public void testCentralEndpoint() throws Throwable { | |
| describe("Create a client with the central endpoint"); | ||
| Configuration conf = getConfiguration(); | ||
|
|
||
| S3Client client = createS3Client(conf, CENTRAL_ENDPOINT, null, US_EAST_1, false); | ||
| S3Client client = createS3Client(conf, CENTRAL_ENDPOINT, null, US_EAST_2, false); | ||
|
|
||
| expectInterceptorException(client); | ||
| } | ||
|
|
||
| @Test | ||
| public void testCentralEndpointWithRegion() throws Throwable { | ||
| describe("Create a client with the central endpoint but also specify region"); | ||
| Configuration conf = getConfiguration(); | ||
|
|
||
| S3Client client = createS3Client(conf, CENTRAL_ENDPOINT, US_WEST_2, | ||
| US_WEST_2, false); | ||
|
|
||
| expectInterceptorException(client); | ||
|
|
||
| client = createS3Client(conf, CENTRAL_ENDPOINT, US_EAST_1, | ||
| US_EAST_1, false); | ||
|
|
||
| expectInterceptorException(client); | ||
|
|
||
| } | ||
|
|
||
| @Test | ||
| public void testWithRegionConfig() throws Throwable { | ||
| describe("Create a client with a configured region"); | ||
|
|
@@ -257,6 +281,144 @@ public void testWithVPCE() throws Throwable { | |
| expectInterceptorException(client); | ||
| } | ||
|
|
||
| @Test | ||
|
ahmarsuhail marked this conversation as resolved.
|
||
| public void testCentralEndpointAndDifferentRegionThanBucket() throws Throwable { | ||
| describe("Access public bucket using central endpoint and region " | ||
| + "different than that of the public bucket"); | ||
| final Configuration conf = getConfiguration(); | ||
| final Configuration newConf = new Configuration(conf); | ||
|
|
||
| removeBaseAndBucketOverrides( | ||
| newConf, | ||
| ENDPOINT, | ||
| AWS_REGION, | ||
| ALLOW_REQUESTER_PAYS, | ||
| KEY_REQUESTER_PAYS_FILE); | ||
|
|
||
| removeBaseAndBucketOverrides( | ||
| DEFAULT_REQUESTER_PAYS_BUCKET_NAME, | ||
| newConf, | ||
| ENDPOINT, | ||
| AWS_REGION, | ||
| ALLOW_REQUESTER_PAYS, | ||
| KEY_REQUESTER_PAYS_FILE); | ||
|
|
||
| newConf.set(ENDPOINT, CENTRAL_ENDPOINT); | ||
| newConf.set(AWS_REGION, EU_WEST_1); | ||
| newConf.setBoolean(ALLOW_REQUESTER_PAYS, true); | ||
|
|
||
| Path filePath = new Path(PublicDatasetTestUtils | ||
| .getRequesterPaysObject(newConf)); | ||
| newFS = (S3AFileSystem) filePath.getFileSystem(newConf); | ||
|
|
||
| Assertions | ||
| .assertThat(newFS.exists(filePath)) | ||
| .describedAs("Existence of path: " + filePath) | ||
| .isTrue(); | ||
| } | ||
|
|
||
| @Test | ||
| public void testCentralEndpointAndSameRegionAsBucket() throws Throwable { | ||
| describe("Access public bucket using central endpoint and region " | ||
| + "same as that of the public bucket"); | ||
| final Configuration conf = getConfiguration(); | ||
| final Configuration newConf = new Configuration(conf); | ||
|
|
||
| removeBaseAndBucketOverrides( | ||
| newConf, | ||
| ENDPOINT, | ||
| AWS_REGION, | ||
| ALLOW_REQUESTER_PAYS, | ||
| KEY_REQUESTER_PAYS_FILE); | ||
|
|
||
| removeBaseAndBucketOverrides( | ||
| DEFAULT_REQUESTER_PAYS_BUCKET_NAME, | ||
| newConf, | ||
| ENDPOINT, | ||
| AWS_REGION, | ||
| ALLOW_REQUESTER_PAYS, | ||
| KEY_REQUESTER_PAYS_FILE); | ||
|
|
||
| newConf.set(ENDPOINT, CENTRAL_ENDPOINT); | ||
| newConf.set(AWS_REGION, US_WEST_2); | ||
| newConf.setBoolean(ALLOW_REQUESTER_PAYS, true); | ||
|
|
||
| Path filePath = new Path(PublicDatasetTestUtils | ||
| .getRequesterPaysObject(newConf)); | ||
| newFS = (S3AFileSystem) filePath.getFileSystem(newConf); | ||
|
|
||
| Assertions | ||
| .assertThat(newFS.exists(filePath)) | ||
| .describedAs("Existence of path: " + filePath) | ||
| .isTrue(); | ||
| } | ||
|
|
||
| @Test | ||
| public void testCentralEndpointAndNullRegionWithCRUD() throws Throwable { | ||
| describe("Access the test bucket using central endpoint and" | ||
| + " null region, perform file system CRUD operations"); | ||
| final Configuration conf = getConfiguration(); | ||
|
|
||
| final Configuration newConf = new Configuration(conf); | ||
|
|
||
| removeBaseAndBucketOverrides( | ||
| newConf, | ||
| ENDPOINT, | ||
| AWS_REGION); | ||
|
|
||
| newConf.set(ENDPOINT, CENTRAL_ENDPOINT); | ||
|
|
||
| newFS = new S3AFileSystem(); | ||
| newFS.initialize(getFileSystem().getUri(), newConf); | ||
|
|
||
| assertOpsUsingNewFs(); | ||
| } | ||
|
|
||
| private void assertOpsUsingNewFs() throws IOException { | ||
| final String file = getMethodName(); | ||
|
ahmarsuhail marked this conversation as resolved.
|
||
| final Path basePath = methodPath(); | ||
| final Path srcDir = new Path(basePath, "srcdir"); | ||
| newFS.mkdirs(srcDir); | ||
| Path srcFilePath = new Path(srcDir, file); | ||
|
|
||
| try (FSDataOutputStream out = newFS.create(srcFilePath)) { | ||
| out.write(new byte[] {1, 2, 3}); | ||
| } | ||
|
|
||
| Assertions | ||
| .assertThat(newFS.exists(srcFilePath)) | ||
| .describedAs("Existence of file: " + srcFilePath) | ||
| .isTrue(); | ||
| Assertions | ||
| .assertThat(getFileSystem().exists(srcFilePath)) | ||
| .describedAs("Existence of file: " + srcFilePath) | ||
| .isTrue(); | ||
|
|
||
| byte[] buffer = new byte[3]; | ||
|
|
||
| try (FSDataInputStream in = newFS.open(srcFilePath)) { | ||
| Assertions | ||
| .assertThat(in.read(buffer, 0, 3)) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use readFully() and skip the assert. read() can return 0-3 so brittle on slow networks
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
| .describedAs("Total bytes read from " + srcFilePath) | ||
| .isEqualTo(3); | ||
| Assertions | ||
| .assertThat(buffer) | ||
| .describedAs("Contents read from " + srcFilePath) | ||
| .containsExactly(1, 2, 3); | ||
| } | ||
|
|
||
| newFS.delete(srcDir, true); | ||
|
|
||
| Assertions | ||
| .assertThat(newFS.exists(srcFilePath)) | ||
| .describedAs("Existence of file: " + srcFilePath) | ||
| .isFalse(); | ||
| Assertions | ||
| .assertThat(getFileSystem().exists(srcFilePath)) | ||
| .describedAs("Existence of file: " + srcFilePath) | ||
| .isFalse(); | ||
| } | ||
|
|
||
| private final class RegionInterceptor implements ExecutionInterceptor { | ||
| private final String endpoint; | ||
| private final String region; | ||
|
|
@@ -272,7 +434,7 @@ private final class RegionInterceptor implements ExecutionInterceptor { | |
| public void beforeExecution(Context.BeforeExecution context, | ||
| ExecutionAttributes executionAttributes) { | ||
|
|
||
| if (endpoint != null) { | ||
| if (endpoint != null && !endpoint.endsWith(CENTRAL_ENDPOINT)) { | ||
| Assertions.assertThat( | ||
| executionAttributes.getAttribute(AwsExecutionAttribute.ENDPOINT_OVERRIDDEN)) | ||
| .describedAs("Endpoint not overridden").isTrue(); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.