You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ozone.apache.org by ad...@apache.org on 2026/09/30 11:36:14 UTC
(ozone) branch master updated: HDDS-16637. Remove redundant InfoBucket RPC from S3 ListMultipartUploads (#11359)
This is an automated email from the ASF dual-hosted git repository.
adoroszlai pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new a0119aaebd2 HDDS-16637. Remove redundant InfoBucket RPC from S3 ListMultipartUploads (#11359)
a0119aaebd2 is described below
commit a0119aaebd23dc1b5e75e613605257a0dc9a0f16
Author: KUAN-HAO HUANG <10...@users.noreply.github.com>
AuthorDate: Wed Sep 30 19:36:05 2026 +0800
HDDS-16637. Remove redundant InfoBucket RPC from S3 ListMultipartUploads (#11359)
---
.../apache/hadoop/ozone/client/rpc/RpcClient.java | 2 +
.../hadoop/ozone/client/rpc/TestRpcClient.java | 17 +++
.../s3/endpoint/ListMultipartUploadsHandler.java | 10 +-
.../s3/endpoint/TestListMultipartUploads.java | 166 +++++++++++++++++++++
.../ozone/s3/endpoint/TestPermissionCheck.java | 3 +-
5 files changed, 192 insertions(+), 6 deletions(-)
diff --git a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
index 5b3c4f9e63c..2eeb72c3e88 100644
--- a/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
+++ b/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
@@ -2382,6 +2382,8 @@ public OzoneMultipartUploadPartListParts listParts(String volumeName,
@Override
public OzoneMultipartUploadList listMultipartUploads(String volumeName,
String bucketName, String prefix, String keyMarker, String uploadIdMarker, int maxUploads) throws IOException {
+ verifyVolumeName(volumeName);
+ verifyBucketName(bucketName);
OmMultipartUploadList omMultipartUploadList;
if (omVersion.compareTo(OzoneManagerVersion.S3_LIST_MULTIPART_UPLOADS_PAGINATION) >= 0) {
diff --git a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/rpc/TestRpcClient.java b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/rpc/TestRpcClient.java
index 999b892ff7b..c7c4d343f6c 100644
--- a/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/rpc/TestRpcClient.java
+++ b/hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/rpc/TestRpcClient.java
@@ -32,6 +32,8 @@
import org.apache.hadoop.ozone.OzoneManagerVersion;
import org.apache.hadoop.ozone.client.MockOmTransport;
import org.apache.hadoop.ozone.client.MockXceiverClientFactory;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes;
import org.apache.hadoop.ozone.om.helpers.ServiceInfo;
import org.apache.hadoop.ozone.om.helpers.ServiceInfoEx;
import org.apache.hadoop.ozone.om.protocolPB.OmTransport;
@@ -39,6 +41,7 @@
import org.apache.ozone.test.GenericTestUtils.LogCapturer;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
import org.junit.jupiter.params.provider.EnumSource;
import org.slf4j.event.Level;
@@ -249,6 +252,20 @@ public void testCloseTwiceDoesNotWarn() throws IOException {
}
}
+ @ParameterizedTest
+ @CsvSource({"invalid/volume, bucket1, INVALID_VOLUME_NAME", "volume1, invalid/bucket, INVALID_BUCKET_NAME"})
+ public void testListMultipartUploadsValidatesNames(String volume, String bucket, ResultCodes expected)
+ throws Exception {
+ RpcClient client = createRpcClient();
+ try {
+ OMException error = assertThrows(OMException.class,
+ () -> client.listMultipartUploads(volume, bucket, null, null, null, 10));
+ assertThat(error.getResult()).isEqualTo(expected);
+ } finally {
+ client.close();
+ }
+ }
+
private static RpcClient createRpcClient() throws IOException {
OzoneConfiguration config = new OzoneConfiguration();
return new RpcClient(config, null) {
diff --git a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ListMultipartUploadsHandler.java b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ListMultipartUploadsHandler.java
index 7f8efb3678d..f574417d79e 100644
--- a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ListMultipartUploadsHandler.java
+++ b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/ListMultipartUploadsHandler.java
@@ -57,12 +57,14 @@ Response handleGetRequest(S3RequestContext context, String bucketName)
long startNanos = context.getStartNanos();
- OzoneBucket bucket = context.getVolume().getBucket(bucketName);
-
try {
- S3Owner.verifyBucketOwnerCondition(getHeaders(), bucketName, bucket.getOwner());
+ if (S3Owner.hasBucketOwnershipVerificationConditions(getHeaders())) {
+ OzoneBucket bucket = context.getVolume().getBucket(bucketName);
+ S3Owner.verifyBucketOwnerCondition(getHeaders(), bucketName, bucket.getOwner());
+ }
OzoneMultipartUploadList ozoneMultipartUploadList =
- bucket.listMultipartUploads(prefix, keyMarker, uploadIdMarker, maxUploads);
+ getClientProtocol().listMultipartUploads(context.getVolume().getName(), bucketName, prefix,
+ keyMarker, uploadIdMarker, maxUploads);
ListMultipartUploadsResult result = new ListMultipartUploadsResult();
result.setBucket(bucketName);
diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestListMultipartUploads.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestListMultipartUploads.java
new file mode 100644
index 00000000000..12842e72132
--- /dev/null
+++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestListMultipartUploads.java
@@ -0,0 +1,166 @@
+/*
+ * 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.ozone.s3.endpoint;
+
+import static org.apache.hadoop.ozone.s3.endpoint.EndpointTestUtils.assertErrorResponse;
+import static org.apache.hadoop.ozone.s3.util.S3Consts.EXPECTED_BUCKET_OWNER_HEADER;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.time.Instant;
+import java.util.Collections;
+import javax.ws.rs.core.HttpHeaders;
+import javax.ws.rs.core.Response;
+import org.apache.hadoop.hdds.client.RatisReplicationConfig;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.ozone.client.ObjectStore;
+import org.apache.hadoop.ozone.client.OzoneBucket;
+import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.client.OzoneMultipartUpload;
+import org.apache.hadoop.ozone.client.OzoneMultipartUploadList;
+import org.apache.hadoop.ozone.client.OzoneVolume;
+import org.apache.hadoop.ozone.client.protocol.ClientProtocol;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
+import org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes;
+import org.apache.hadoop.ozone.s3.exception.S3ErrorTable;
+import org.apache.hadoop.ozone.s3.metrics.S3GatewayMetrics;
+import org.apache.hadoop.ozone.s3.util.S3Consts.QueryParams;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+import org.junit.jupiter.params.provider.ValueSource;
+
+/** Tests listing multipart uploads through the bucket endpoint. */
+public class TestListMultipartUploads {
+ private ClientProtocol proxy;
+ private HttpHeaders headers;
+ private BucketEndpoint endpoint;
+
+ @BeforeEach
+ public void setup() throws Exception {
+ OzoneConfiguration conf = new OzoneConfiguration();
+ S3GatewayMetrics.create(conf);
+ proxy = mock(ClientProtocol.class);
+ headers = mock(HttpHeaders.class);
+ OzoneVolume volume = OzoneVolume.newBuilder(conf, proxy)
+ .setName("volume1").setAcls(Collections.emptyList()).build();
+ OzoneBucket bucket = OzoneBucket.newBuilder(conf, proxy)
+ .setVolumeName("volume1").setName("bucket1").setOwner("owner").build();
+ when(proxy.getBucketDetails("volume1", "bucket1")).thenReturn(bucket);
+ ObjectStore objectStore = mock(ObjectStore.class);
+ when(objectStore.getS3Volume()).thenReturn(volume);
+ when(objectStore.getClientProxy()).thenReturn(proxy);
+ OzoneClient client = mock(OzoneClient.class);
+ when(client.getObjectStore()).thenReturn(objectStore);
+ when(client.getProxy()).thenReturn(proxy);
+ endpoint = EndpointBuilder.newBucketEndpointBuilder().setClient(client).setHeaders(headers).build();
+ endpoint.queryParamsForTest().set(QueryParams.UPLOADS, "");
+ endpoint.queryParamsForTest().set(QueryParams.PREFIX, "prefix/");
+ endpoint.queryParamsForTest().set(QueryParams.KEY_MARKER, "prefix/previous");
+ endpoint.queryParamsForTest().set(QueryParams.UPLOAD_ID_MARKER, "previous-upload");
+ endpoint.queryParamsForTest().set(QueryParams.MAX_UPLOADS, "2");
+ }
+
+ @ParameterizedTest
+ @CsvSource({", 0", "'', 0", "owner, 1"})
+ public void testOnlyLooksUpBucketForOwner(String expectedOwner, int bucketLookups) throws Exception {
+ when(headers.getHeaderString(EXPECTED_BUCKET_OWNER_HEADER)).thenReturn(expectedOwner);
+ OzoneMultipartUpload upload = new OzoneMultipartUpload("volume1", "bucket1", "prefix/key", "upload1",
+ Instant.EPOCH, RatisReplicationConfig.getInstance(HddsProtos.ReplicationFactor.THREE));
+ when(proxy.listMultipartUploads("volume1", "bucket1", "prefix/", "prefix/previous", "previous-upload", 2))
+ .thenReturn(new OzoneMultipartUploadList(Collections.singletonList(upload), "prefix/key", "upload1", true));
+
+ try (Response response = endpoint.get("bucket1")) {
+ assertThat(response.getStatus()).isEqualTo(200);
+ ListMultipartUploadsResult result = (ListMultipartUploadsResult) response.getEntity();
+ assertThat(result.getBucket()).isEqualTo("bucket1");
+ assertThat(result.getPrefix()).isEqualTo("prefix/");
+ assertThat(result.getKeyMarker()).isEqualTo("prefix/previous");
+ assertThat(result.getUploadIdMarker()).isEqualTo("previous-upload");
+ assertThat(result.getNextKeyMarker()).isEqualTo("prefix/key");
+ assertThat(result.getNextUploadIdMarker()).isEqualTo("upload1");
+ assertThat(result.getMaxUploads()).isEqualTo(2);
+ assertThat(result.isTruncated()).isTrue();
+ assertThat(result.getUploads()).hasSize(1);
+ assertThat(result.getUploads().get(0).getKey()).isEqualTo("prefix/key");
+ }
+ verify(proxy, times(bucketLookups)).getBucketDetails("volume1", "bucket1");
+ verify(proxy).listMultipartUploads("volume1", "bucket1", "prefix/", "prefix/previous", "previous-upload", 2);
+ }
+
+ @Test
+ public void testRejectsIncorrectOwner() throws Exception {
+ when(headers.getHeaderString(EXPECTED_BUCKET_OWNER_HEADER)).thenReturn("other-owner");
+
+ assertErrorResponse(S3ErrorTable.BUCKET_OWNER_MISMATCH, () -> endpoint.get("bucket1"));
+
+ verify(proxy).getBucketDetails("volume1", "bucket1");
+ verify(proxy, never()).listMultipartUploads("volume1", "bucket1", "prefix/", "prefix/previous",
+ "previous-upload", 2);
+ }
+
+ @ParameterizedTest
+ @CsvSource({
+ "BUCKET_NOT_FOUND, NO_SUCH_BUCKET", "VOLUME_NOT_FOUND, NO_SUCH_BUCKET", "PERMISSION_DENIED, ACCESS_DENIED"
+ })
+ public void testProtocolErrors(ResultCodes resultCode, S3ErrorTable expectedError) throws Exception {
+ when(proxy.listMultipartUploads("volume1", "bucket1", "prefix/", "prefix/previous", "previous-upload", 2))
+ .thenThrow(new OMException("List uploads failed", resultCode));
+
+ assertErrorResponse(expectedError, () -> endpoint.get("bucket1"));
+
+ verify(proxy, never()).getBucketDetails("volume1", "bucket1");
+ verify(proxy).listMultipartUploads("volume1", "bucket1", "prefix/", "prefix/previous", "previous-upload", 2);
+ }
+
+ @ParameterizedTest
+ @CsvSource({"1, 1", "1000, 1000", "1001, 1000"})
+ public void testMaxUploadsAndEmptyResult(int requested, int forwarded) throws Exception {
+ endpoint.queryParamsForTest().set(QueryParams.MAX_UPLOADS, Integer.toString(requested));
+ when(proxy.listMultipartUploads("volume1", "bucket1", "prefix/", "prefix/previous", "previous-upload", forwarded))
+ .thenReturn(new OzoneMultipartUploadList(Collections.emptyList(), null, null, false));
+
+ try (Response response = endpoint.get("bucket1")) {
+ ListMultipartUploadsResult result = (ListMultipartUploadsResult) response.getEntity();
+ assertThat(result.getMaxUploads()).isEqualTo(forwarded);
+ assertThat(result.getUploads()).isEmpty();
+ assertThat(result.isTruncated()).isFalse();
+ }
+ verify(proxy, never()).getBucketDetails("volume1", "bucket1");
+ verify(proxy).listMultipartUploads("volume1", "bucket1", "prefix/", "prefix/previous",
+ "previous-upload", forwarded);
+ }
+
+ @ParameterizedTest
+ @ValueSource(ints = {0, -1})
+ public void testRejectsInvalidMaxUploads(int maxUploads) throws Exception {
+ endpoint.queryParamsForTest().set(QueryParams.MAX_UPLOADS, Integer.toString(maxUploads));
+
+ assertErrorResponse(S3ErrorTable.INVALID_ARGUMENT, () -> endpoint.get("bucket1"));
+
+ verify(proxy, never()).getBucketDetails("volume1", "bucket1");
+ verify(proxy, never()).listMultipartUploads("volume1", "bucket1", "prefix/", "prefix/previous", "previous-upload",
+ maxUploads);
+ }
+}
diff --git a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPermissionCheck.java b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPermissionCheck.java
index 6fff7466d5d..f766803c688 100644
--- a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPermissionCheck.java
+++ b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPermissionCheck.java
@@ -162,8 +162,7 @@ public void testDeleteBucket() throws IOException {
@Test
public void testListMultiUpload() throws IOException {
when(objectStore.getS3Volume()).thenReturn(volume);
- when(volume.getBucket(anyString())).thenReturn(bucket);
- doThrow(exception).when(bucket).listMultipartUploads(any(), any(), any(), anyInt());
+ doThrow(exception).when(clientProtocol).listMultipartUploads(any(), any(), any(), any(), any(), anyInt());
BucketEndpoint bucketEndpoint = EndpointBuilder.newBucketEndpointBuilder()
.setClient(client)
.build();
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@ozone.apache.org
For additional commands, e-mail: commits-help@ozone.apache.org