Skip to content

Commit 058c655

Browse files
authored
HDDS-13706. Limit max-uploads at 1000 (apache#9060)
1 parent 86ee18f commit 058c655

File tree

3 files changed

+36
-23
lines changed

3 files changed

+36
-23
lines changed

hadoop-ozone/integration-test-s3/src/test/java/org/apache/hadoop/ozone/s3/awssdk/v1/AbstractS3SDKV1Tests.java

Lines changed: 31 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@
9797
import java.util.Map;
9898
import java.util.Set;
9999
import java.util.stream.Collectors;
100+
import java.util.stream.IntStream;
100101
import javax.xml.bind.DatatypeConverter;
101102
import org.apache.commons.io.IOUtils;
102103
import org.apache.commons.lang3.RandomStringUtils;
@@ -128,6 +129,8 @@
128129
import org.junit.jupiter.api.TestInstance;
129130
import org.junit.jupiter.api.TestMethodOrder;
130131
import org.junit.jupiter.api.io.TempDir;
132+
import org.junit.jupiter.params.ParameterizedTest;
133+
import org.junit.jupiter.params.provider.ValueSource;
131134

132135
/**
133136
* This is an abstract class to test the AWS Java S3 SDK operations.
@@ -141,6 +144,9 @@
141144
@TestMethodOrder(MethodOrderer.MethodName.class)
142145
public abstract class AbstractS3SDKV1Tests extends OzoneTestBase {
143146

147+
// server-side limitation
148+
private static final int MAX_UPLOADS_LIMIT = 1000;
149+
144150
/**
145151
* There are still some unsupported S3 operations.
146152
* Current unsupported S3 operations (non-exhaustive):
@@ -758,6 +764,7 @@ public void testListMultipartUploads() {
758764
uploadIds.add(uploadId3);
759765

760766
ListMultipartUploadsRequest listMultipartUploadsRequest = new ListMultipartUploadsRequest(bucketName);
767+
listMultipartUploadsRequest.setMaxUploads(5000);
761768

762769
MultipartUploadListing result = s3Client.listMultipartUploads(listMultipartUploadsRequest);
763770

@@ -768,26 +775,31 @@ public void testListMultipartUploads() {
768775
assertEquals(uploadIds, listUploadIds);
769776
}
770777

771-
@Test
772-
public void testListMultipartUploadsPagination() {
773-
final String bucketName = getBucketName();
778+
@ParameterizedTest
779+
@ValueSource(ints = {10, 5000})
780+
public void testListMultipartUploadsPagination(int requestedMaxUploads) {
781+
final String bucketName = getBucketName() + "-" + requestedMaxUploads;
774782
final String multipartKeyPrefix = getKeyName("multipart");
775783

776784
s3Client.createBucket(bucketName);
777785

778-
// Create 25 multipart uploads to test pagination
786+
// Create multipart uploads to test pagination
779787
List<String> allKeys = new ArrayList<>();
780788
Map<String, String> keyToUploadId = new HashMap<>();
781789

782-
for (int i = 0; i < 25; i++) {
783-
String key = String.format("%s-%03d", multipartKeyPrefix, i);
790+
final int effectiveMaxUploads = Math.min(requestedMaxUploads, MAX_UPLOADS_LIMIT);
791+
final int uploadsCreated = 2 * effectiveMaxUploads + 5;
792+
final int expectedPages = uploadsCreated / effectiveMaxUploads + 1;
793+
794+
for (int i = 0; i < uploadsCreated; i++) {
795+
String key = String.format("%s-%04d", multipartKeyPrefix, i);
784796
allKeys.add(key);
785797
String uploadId = initiateMultipartUpload(bucketName, key, null, null, null);
786798
keyToUploadId.put(key, uploadId);
787799
}
788800
Collections.sort(allKeys);
789801

790-
// Test pagination with maxUploads=10
802+
// Test pagination
791803
Set<String> retrievedKeys = new HashSet<>();
792804
String keyMarker = null;
793805
String uploadIdMarker = null;
@@ -796,18 +808,19 @@ public void testListMultipartUploadsPagination() {
796808

797809
do {
798810
ListMultipartUploadsRequest request = new ListMultipartUploadsRequest(bucketName)
799-
.withMaxUploads(10)
811+
.withMaxUploads(requestedMaxUploads)
800812
.withKeyMarker(keyMarker)
801813
.withUploadIdMarker(uploadIdMarker);
802814

803815
MultipartUploadListing result = s3Client.listMultipartUploads(request);
816+
pageCount++;
804817

805818
// Verify page size
806-
if (pageCount < 2) {
807-
assertEquals(10, result.getMultipartUploads().size());
819+
if (pageCount < expectedPages) {
820+
assertEquals(effectiveMaxUploads, result.getMultipartUploads().size());
808821
assertTrue(result.isTruncated());
809822
} else {
810-
assertEquals(5, result.getMultipartUploads().size());
823+
assertEquals(uploadsCreated % effectiveMaxUploads, result.getMultipartUploads().size());
811824
assertFalse(result.isTruncated());
812825
}
813826

@@ -822,7 +835,7 @@ public void testListMultipartUploadsPagination() {
822835
assertNull(result.getPrefix());
823836
assertEquals(result.getUploadIdMarker(), uploadIdMarker);
824837
assertEquals(result.getKeyMarker(), keyMarker);
825-
assertEquals(result.getMaxUploads(), 10);
838+
assertEquals(effectiveMaxUploads, result.getMaxUploads());
826839

827840
// Verify next markers content
828841
if (result.isTruncated()) {
@@ -840,32 +853,29 @@ public void testListMultipartUploadsPagination() {
840853
uploadIdMarker = result.getNextUploadIdMarker();
841854

842855
truncated = result.isTruncated();
843-
pageCount++;
844856

845857
} while (truncated);
846858

847859
// Verify pagination results
848-
assertEquals(3, pageCount, "Should have exactly 3 pages");
849-
assertEquals(25, retrievedKeys.size(), "Should retrieve all uploads");
860+
assertEquals(expectedPages, pageCount);
861+
assertEquals(uploadsCreated, retrievedKeys.size(), "Should retrieve all uploads");
850862
assertEquals(
851863
allKeys,
852864
retrievedKeys.stream().sorted().collect(Collectors.toList()),
853865
"Retrieved keys should match expected keys in order");
854866

855867
// Test with prefix
856-
String prefix = multipartKeyPrefix + "-01";
868+
String prefix = multipartKeyPrefix + "-001";
857869
ListMultipartUploadsRequest prefixRequest = new ListMultipartUploadsRequest(bucketName)
858870
.withPrefix(prefix);
859871

860872
MultipartUploadListing prefixResult = s3Client.listMultipartUploads(prefixRequest);
861873

862874
assertEquals(prefix, prefixResult.getPrefix());
863875
assertEquals(
864-
Arrays.asList(multipartKeyPrefix + "-010", multipartKeyPrefix + "-011",
865-
multipartKeyPrefix + "-012", multipartKeyPrefix + "-013",
866-
multipartKeyPrefix + "-014", multipartKeyPrefix + "-015",
867-
multipartKeyPrefix + "-016", multipartKeyPrefix + "-017",
868-
multipartKeyPrefix + "-018", multipartKeyPrefix + "-019"),
876+
IntStream.rangeClosed(0, 9)
877+
.mapToObj(i -> prefix + i)
878+
.collect(Collectors.toList()),
869879
prefixResult.getMultipartUploads().stream()
870880
.map(MultipartUpload::getKey)
871881
.collect(Collectors.toList()));

hadoop-ozone/integration-test-s3/src/test/java/org/apache/hadoop/ozone/s3/awssdk/v2/AbstractS3SDKV2Tests.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1228,6 +1228,7 @@ public void testListMultipartUploads() {
12281228
ListMultipartUploadsRequest correctRequest = ListMultipartUploadsRequest.builder()
12291229
.bucket(DEFAULT_BUCKET_NAME)
12301230
.expectedBucketOwner(correctOwner)
1231+
.maxUploads(5000)
12311232
.build();
12321233
verifyPassBucketOwnershipVerification(() -> s3Client.listMultipartUploads(correctRequest));
12331234

hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/BucketEndpoint.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -355,9 +355,11 @@ public Response listMultipartUploads(
355355
int maxUploads)
356356
throws OS3Exception, IOException {
357357

358-
if (maxUploads < 1 || maxUploads > 1000) {
358+
if (maxUploads < 1) {
359359
throw newError(S3ErrorTable.INVALID_ARGUMENT, "max-uploads",
360-
new Exception("max-uploads must be between 1 and 1000"));
360+
new Exception("max-uploads must be positive"));
361+
} else {
362+
maxUploads = Math.min(maxUploads, 1000);
361363
}
362364

363365
long startNanos = Time.monotonicNowNanos();

0 commit comments

Comments
 (0)