Skip to content

Commit e219660

Browse files
alextwoodsalextwoods
authored
Support MRAP buckets in S3 TransferManager (#6916)
* First pass * Update tests + add changelog
1 parent ae7047c commit e219660

6 files changed

Lines changed: 91 additions & 69 deletions

File tree

‎.changes/next-release/feature-S3TransferManager-9973f30.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"type": "feature",
3+
"category": "S3TransferManager",
4+
"contributor": "",
5+
"description": "Support MRAP buckets in S3 TransferManager."
6+
}

‎services-custom/s3-transfer-manager/pom.xml‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,6 @@
8282
<!-- Only required for CRT-based TM -->
8383
<optional>true</optional>
8484
</dependency>
85-
<dependency>
86-
<groupId>software.amazon.awssdk</groupId>
87-
<artifactId>arns</artifactId>
88-
<version>${awsjavasdk.version}</version>
89-
</dependency>
9085
<dependency>
9186
<groupId>software.amazon.awssdk</groupId>
9287
<artifactId>aws-core</artifactId>

‎services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/GenericS3TransferManager.java‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import java.util.function.Consumer;
3030
import software.amazon.awssdk.annotations.SdkInternalApi;
3131
import software.amazon.awssdk.annotations.SdkTestInternalApi;
32-
import software.amazon.awssdk.arns.Arn;
3332
import software.amazon.awssdk.awscore.AwsRequestOverrideConfiguration;
3433
import software.amazon.awssdk.core.FileTransformerConfiguration;
3534
import software.amazon.awssdk.core.async.AsyncRequestBody;
@@ -40,9 +39,6 @@
4039
import software.amazon.awssdk.services.s3.S3AsyncClient;
4140
import software.amazon.awssdk.services.s3.internal.multipart.MultipartDownloadResumeContext;
4241
import software.amazon.awssdk.services.s3.internal.multipart.MultipartS3AsyncClient;
43-
import software.amazon.awssdk.services.s3.internal.resource.S3AccessPointResource;
44-
import software.amazon.awssdk.services.s3.internal.resource.S3ArnConverter;
45-
import software.amazon.awssdk.services.s3.internal.resource.S3Resource;
4642
import software.amazon.awssdk.services.s3.model.CopyObjectRequest;
4743
import software.amazon.awssdk.services.s3.model.CopyObjectResponse;
4844
import software.amazon.awssdk.services.s3.model.GetObjectRequest;
@@ -622,27 +618,9 @@ protected static void assertNotUnsupportedArn(String bucket, String operation) {
622618
String error = String.format("%s does not support S3 Object Lambda resources", operation);
623619
throw new IllegalArgumentException(error);
624620
}
625-
626-
Arn arn = Arn.fromString(bucket);
627-
628-
if (isMrapArn(arn)) {
629-
String error = String.format("%s does not support S3 multi-region access point ARN", operation);
630-
throw new IllegalArgumentException(error);
631-
}
632621
}
633622

634623
private static boolean isObjectLambdaArn(String arn) {
635624
return arn.contains(":s3-object-lambda");
636625
}
637-
638-
private static boolean isMrapArn(Arn arn) {
639-
S3Resource s3Resource = S3ArnConverter.create().convertArn(arn);
640-
641-
S3AccessPointResource s3EndpointResource =
642-
Validate.isInstanceOf(S3AccessPointResource.class, s3Resource,
643-
"An ARN was passed as a bucket parameter to an S3 operation, however it does not "
644-
+ "appear to be a valid S3 access point ARN.");
645-
646-
return !s3EndpointResource.region().isPresent();
647-
}
648626
}

‎services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/S3TransferManagerTest.java‎

Lines changed: 85 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -289,55 +289,105 @@ void objectLambdaArnBucketProvided_shouldThrowException() {
289289
}
290290

291291
@Test
292-
void mrapArnProvided_shouldThrowException() {
292+
void uploadFile_mrapArn_returnsResponse() {
293293
String mrapArn = "arn:aws:s3::123456789012:accesspoint:mfzwi23gnjvgw.mrap";
294+
PutObjectResponse response = PutObjectResponse.builder().build();
295+
when(mockS3Crt.putObject(any(PutObjectRequest.class), any(AsyncRequestBody.class)))
296+
.thenReturn(CompletableFuture.completedFuture(response));
294297

295-
assertThatThrownBy(() -> tm.uploadFile(b -> b.putObjectRequest(p -> p.bucket(mrapArn).key("key"))
296-
.source(Paths.get(".")))
297-
.completionFuture().join())
298-
.hasMessageContaining("multi-region access point ARN").hasCauseInstanceOf(IllegalArgumentException.class);
298+
CompletedFileUpload completedFileUpload = tm.uploadFile(u -> u.putObjectRequest(p -> p.bucket(mrapArn).key("key"))
299+
.source(Paths.get(".")))
300+
.completionFuture().join();
299301

300-
assertThatThrownBy(() -> tm.upload(b -> b.putObjectRequest(p -> p.bucket(mrapArn).key("key"))
301-
.requestBody(AsyncRequestBody.fromString("foo")))
302-
.completionFuture().join())
303-
.hasMessageContaining("multi-region access point ARN").hasCauseInstanceOf(IllegalArgumentException.class);
302+
assertThat(completedFileUpload.response()).isEqualTo(response);
303+
}
304304

305-
assertThatThrownBy(() -> tm.downloadFile(b -> b.getObjectRequest(p -> p.bucket(mrapArn).key("key"))
306-
.destination(Paths.get(".")))
307-
.completionFuture().join())
308-
.hasMessageContaining("multi-region access point ARN").hasCauseInstanceOf(IllegalArgumentException.class);
305+
@Test
306+
void upload_mrapArn_returnsResponse() {
307+
String mrapArn = "arn:aws:s3::123456789012:accesspoint:mfzwi23gnjvgw.mrap";
308+
PutObjectResponse response = PutObjectResponse.builder().build();
309+
when(mockS3Crt.putObject(any(PutObjectRequest.class), any(AsyncRequestBody.class)))
310+
.thenReturn(CompletableFuture.completedFuture(response));
311+
312+
CompletedUpload completedUpload = tm.upload(u -> u.putObjectRequest(p -> p.bucket(mrapArn).key("key"))
313+
.requestBody(AsyncRequestBody.fromString("foo")))
314+
.completionFuture().join();
315+
316+
assertThat(completedUpload.response()).isEqualTo(response);
317+
}
318+
319+
@Test
320+
void downloadFile_mrapArn_returnsResponse() {
321+
String mrapArn = "arn:aws:s3::123456789012:accesspoint:mfzwi23gnjvgw.mrap";
322+
GetObjectResponse response = GetObjectResponse.builder().build();
323+
when(mockS3Crt.getObject(any(GetObjectRequest.class), any(AsyncResponseTransformer.class)))
324+
.thenReturn(CompletableFuture.completedFuture(response));
325+
326+
CompletedFileDownload completedFileDownload = tm.downloadFile(d -> d.getObjectRequest(g -> g.bucket(mrapArn).key("key"))
327+
.destination(Paths.get(".")))
328+
.completionFuture().join();
329+
330+
assertThat(completedFileDownload.response()).isEqualTo(response);
331+
}
332+
333+
@Test
334+
void download_mrapArn_returnsResponse() {
335+
String mrapArn = "arn:aws:s3::123456789012:accesspoint:mfzwi23gnjvgw.mrap";
336+
GetObjectResponse response = GetObjectResponse.builder().build();
337+
when(mockS3Crt.getObject(any(GetObjectRequest.class), any(AsyncResponseTransformer.class)))
338+
.thenReturn(CompletableFuture.completedFuture(response));
309339

310340
DownloadRequest<ResponseBytes<GetObjectResponse>> downloadRequest =
311341
DownloadRequest.builder()
312342
.getObjectRequest(g -> g.bucket(mrapArn).key("key"))
313343
.responseTransformer(AsyncResponseTransformer.toBytes()).build();
314344

315-
assertThatThrownBy(() -> tm.download(downloadRequest).completionFuture().join())
316-
.hasMessageContaining("multi-region access point ARN").hasCauseInstanceOf(IllegalArgumentException.class);
345+
CompletedDownload<ResponseBytes<GetObjectResponse>> completedDownload =
346+
tm.download(downloadRequest).completionFuture().join();
317347

318-
assertThatThrownBy(() -> tm.uploadDirectory(b -> b.bucket(mrapArn)
319-
.source(Paths.get(".")))
320-
.completionFuture().join())
321-
.hasMessageContaining("multi-region access point ARN").hasCauseInstanceOf(IllegalArgumentException.class);
348+
assertThat(completedDownload).isNotNull();
349+
}
322350

323-
assertThatThrownBy(() -> tm.downloadDirectory(b -> b.bucket(mrapArn)
324-
.destination(Paths.get(".")))
325-
.completionFuture().join())
326-
.hasMessageContaining("multi-region access point ARN").hasCauseInstanceOf(IllegalArgumentException.class);
351+
@Test
352+
void copy_mrapArn_returnsResponse() {
353+
String mrapArn = "arn:aws:s3::123456789012:accesspoint:mfzwi23gnjvgw.mrap";
354+
CopyObjectResponse response = CopyObjectResponse.builder().build();
355+
when(mockS3Crt.copyObject(any(CopyObjectRequest.class)))
356+
.thenReturn(CompletableFuture.completedFuture(response));
327357

328-
assertThatThrownBy(() -> tm.copy(b -> b.copyObjectRequest(p -> p.sourceBucket(mrapArn)
329-
.sourceKey("sourceKey")
330-
.destinationBucket("bucket")
331-
.destinationKey("destKey")))
332-
.completionFuture().join())
333-
.hasMessageContaining("multi-region access point ARN").hasCauseInstanceOf(IllegalArgumentException.class);
358+
CompletedCopy completedCopy = tm.copy(u -> u.copyObjectRequest(p -> p.sourceBucket(mrapArn)
359+
.sourceKey("sourceKey")
360+
.destinationBucket("bucket")
361+
.destinationKey("destKey")))
362+
.completionFuture().join();
334363

335-
assertThatThrownBy(() -> tm.copy(b -> b.copyObjectRequest(p -> p.sourceBucket("bucket")
336-
.sourceKey("sourceKey")
337-
.destinationBucket(mrapArn)
338-
.destinationKey("destKey")))
339-
.completionFuture().join())
340-
.hasMessageContaining("multi-region access point ARN").hasCauseInstanceOf(IllegalArgumentException.class);
364+
assertThat(completedCopy.response()).isEqualTo(response);
365+
366+
completedCopy = tm.copy(u -> u.copyObjectRequest(p -> p.sourceBucket("bucket")
367+
.sourceKey("sourceKey")
368+
.destinationBucket(mrapArn)
369+
.destinationKey("destKey")))
370+
.completionFuture().join();
371+
372+
assertThat(completedCopy.response()).isEqualTo(response);
373+
}
374+
375+
@Test
376+
void uploadDirectory_mrapArn_returnsResponse() {
377+
String mrapArn = "arn:aws:s3::123456789012:accesspoint:mfzwi23gnjvgw.mrap";
378+
379+
tm.uploadDirectory(u -> u.source(Paths.get(".")).bucket(mrapArn));
380+
381+
verify(uploadDirectoryHelper).uploadDirectory(any(UploadDirectoryRequest.class));
382+
}
383+
384+
@Test
385+
void downloadDirectory_mrapArn_returnsResponse() {
386+
String mrapArn = "arn:aws:s3::123456789012:accesspoint:mfzwi23gnjvgw.mrap";
387+
388+
tm.downloadDirectory(d -> d.destination(Paths.get(".")).bucket(mrapArn));
389+
390+
verify(downloadDirectoryHelper).downloadDirectory(any(DownloadDirectoryRequest.class));
341391
}
342392

343393
@Test

‎test/s3-tests/src/it/java/software/amazon/awssdk/services/s3/regression/S3ChecksumsTestUtils.java‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -203,11 +203,6 @@ public static void assumeNotAccessPointWithPathStyle(UploadConfig config) {
203203
"Path style doesn't work with ARN type buckets");
204204
}
205205

206-
public static void assumeNotMRAP(UploadConfig config) {
207-
Assumptions.assumeFalse(config.getBucketType().equals(BucketType.MRAP),
208-
"MRAP buckets are not supported by TransferManager.");
209-
}
210-
211206
public static String crc32(String s) {
212207
return crc32(s.getBytes(StandardCharsets.UTF_8));
213208
}

‎test/s3-tests/src/it/java/software/amazon/awssdk/services/s3/regression/upload/UploadTransferManagerRegressionTesting.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
package software.amazon.awssdk.services.s3.regression.upload;
1717

1818
import static software.amazon.awssdk.services.s3.regression.S3ChecksumsTestUtils.assumeNotAccessPointWithPathStyle;
19-
import static software.amazon.awssdk.services.s3.regression.S3ChecksumsTestUtils.assumeNotMRAP;
2019
import static software.amazon.awssdk.services.s3.regression.S3ClientFlavor.MULTIPART_ENABLED;
2120

2221
import java.time.Duration;
@@ -49,7 +48,6 @@ public static List<UploadConfig> testConfigs() {
4948
void putObject(UploadConfig config) throws Exception {
5049

5150
assumeNotAccessPointWithPathStyle(config);
52-
assumeNotMRAP(config);
5351

5452
// For testing purposes, ContentProvider is Publisher<ByteBuffer> for async clients
5553
// There is no way to create AsyncRequestBody with a Publisher<ByteBuffer> and also provide the content length

0 commit comments

Comments
 (0)