This is an automated email from the ASF dual-hosted git repository.
sodonnell 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 16e7581967 HDDS-9314. create-bucket on an existing bucket should fail
(#5319)
16e7581967 is described below
commit 16e758196783f0b5c4beac3de3fa39085e3efd7c
Author: Tejaskriya <[email protected]>
AuthorDate: Wed Sep 20 22:08:14 2023 +0530
HDDS-9314. create-bucket on an existing bucket should fail (#5319)
---
.../dist/src/main/smoketest/s3/bucketcreate.robot | 8 +++---
.../dist/src/main/smoketest/upgrade/generate.robot | 5 ++--
.../hadoop/ozone/s3/endpoint/EndpointBase.java | 6 ++---
.../hadoop/ozone/s3/exception/S3ErrorTable.java | 4 +++
.../hadoop/ozone/s3/endpoint/TestBucketPut.java | 9 +++++++
.../ozone/s3/metrics/TestS3GatewayMetrics.java | 30 ++++++++++++++++------
6 files changed, 46 insertions(+), 16 deletions(-)
diff --git a/hadoop-ozone/dist/src/main/smoketest/s3/bucketcreate.robot
b/hadoop-ozone/dist/src/main/smoketest/s3/bucketcreate.robot
index aaf0da4b77..76e0cadf37 100644
--- a/hadoop-ozone/dist/src/main/smoketest/s3/bucketcreate.robot
+++ b/hadoop-ozone/dist/src/main/smoketest/s3/bucketcreate.robot
@@ -33,13 +33,15 @@ Create new bucket
Create bucket
Create bucket which already exists
- ${bucket} = Create bucket
- Create bucket with name ${bucket}
+ ${bucket} = Create bucket
+ ${result} = Execute AWSS3APICli and checkrc create-bucket
--bucket ${bucket} 255
+ Should contain ${result}
BucketAlreadyExists
Create bucket with invalid bucket name
${randStr} = Generate Ozone String
${result} = Execute AWSS3APICli and checkrc create-bucket
--bucket invalid_bucket_${randStr} 255
- Should contain ${result}
InvalidBucketName
+ Should contain ${result}
InvalidBucketName
+
Create new bucket and check no group ACL
${bucket} = Create bucket
${acl} = Execute ozone sh bucket getacl s3v/${bucket}
diff --git a/hadoop-ozone/dist/src/main/smoketest/upgrade/generate.robot
b/hadoop-ozone/dist/src/main/smoketest/upgrade/generate.robot
index acecbab5f8..2bfde82b04 100644
--- a/hadoop-ozone/dist/src/main/smoketest/upgrade/generate.robot
+++ b/hadoop-ozone/dist/src/main/smoketest/upgrade/generate.robot
@@ -54,8 +54,9 @@ Setup credentials for S3
Run Keyword Setup dummy credentials for S3
Try to create a bucket using S3 API
- # Note: S3 API does not return error if the bucket already exists
- ${output} = Create bucket with name ${PREFIX}-bucket
+ # Note: S3 API returns error if the bucket already exists
+ ${random} = Generate Ozone String
+ ${output} = Create bucket with name ${PREFIX}-bucket-${random}
Should Be Equal ${output} ${None}
Create key using S3 API
diff --git
a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/EndpointBase.java
b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/EndpointBase.java
index b04bfaa4de..66fb7df190 100644
---
a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/EndpointBase.java
+++
b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/endpoint/EndpointBase.java
@@ -173,9 +173,9 @@ public abstract class EndpointBase implements Auditor {
} else if (ex.getResult() == ResultCodes.TIMEOUT ||
ex.getResult() == ResultCodes.INTERNAL_ERROR) {
throw newError(S3ErrorTable.INTERNAL_ERROR, bucketName, ex);
- } else if (ex.getResult() != ResultCodes.BUCKET_ALREADY_EXISTS) {
- // S3 does not return error for bucket already exists, it just
- // returns the location.
+ } else if (ex.getResult() == ResultCodes.BUCKET_ALREADY_EXISTS) {
+ throw newError(S3ErrorTable.BUCKET_ALREADY_EXISTS, bucketName, ex);
+ } else {
throw ex;
}
}
diff --git
a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/exception/S3ErrorTable.java
b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/exception/S3ErrorTable.java
index 776f1c7654..a526404ff1 100644
---
a/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/exception/S3ErrorTable.java
+++
b/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/exception/S3ErrorTable.java
@@ -132,6 +132,10 @@ public final class S3ErrorTable {
"exceeds the maximum allowed metadata size of " +
S3_REQUEST_HEADER_METADATA_SIZE_LIMIT_KB + "KB", HTTP_BAD_REQUEST);
+ public static final OS3Exception BUCKET_ALREADY_EXISTS = new OS3Exception(
+ "BucketAlreadyExists", "The requested bucket name is not available" +
+ " as it already exists.", HTTP_CONFLICT);
+
public static OS3Exception newError(OS3Exception e, String resource) {
return newError(e, resource, null);
}
diff --git
a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketPut.java
b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketPut.java
index dd155186e4..8d09da98ab 100644
---
a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketPut.java
+++
b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestBucketPut.java
@@ -28,7 +28,9 @@ import org.apache.hadoop.ozone.client.OzoneClient;
import org.apache.hadoop.ozone.client.OzoneClientStub;
import org.apache.hadoop.ozone.s3.exception.OS3Exception;
+import static java.net.HttpURLConnection.HTTP_CONFLICT;
import static java.net.HttpURLConnection.HTTP_NOT_FOUND;
+import static
org.apache.hadoop.ozone.s3.exception.S3ErrorTable.BUCKET_ALREADY_EXISTS;
import static
org.apache.hadoop.ozone.s3.exception.S3ErrorTable.MALFORMED_HEADER;
import static
org.apache.hadoop.ozone.s3.signature.SignatureProcessor.DATE_FORMATTER;
import org.junit.Assert;
@@ -73,6 +75,13 @@ public class TestBucketPut {
Response response = bucketEndpoint.put(bucketName, null, null, null);
assertEquals(200, response.getStatus());
assertNotNull(response.getLocation());
+ try {
+ // Create-bucket on an existing bucket fails
+ bucketEndpoint.put(bucketName, null, null, null);
+ } catch (OS3Exception ex) {
+ Assert.assertEquals(HTTP_CONFLICT, ex.getHttpCode());
+ Assert.assertEquals(BUCKET_ALREADY_EXISTS.getCode(), ex.getCode());
+ }
}
@Test
diff --git
a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/metrics/TestS3GatewayMetrics.java
b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/metrics/TestS3GatewayMetrics.java
index bb1764ac2d..9dacc7a7db 100644
---
a/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/metrics/TestS3GatewayMetrics.java
+++
b/hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/metrics/TestS3GatewayMetrics.java
@@ -50,8 +50,10 @@ import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
+import static java.net.HttpURLConnection.HTTP_CONFLICT;
import static java.net.HttpURLConnection.HTTP_OK;
import static java.nio.charset.StandardCharsets.UTF_8;
+import static
org.apache.hadoop.ozone.s3.exception.S3ErrorTable.BUCKET_ALREADY_EXISTS;
import static org.apache.hadoop.ozone.s3.util.S3Consts.COPY_SOURCE_HEADER;
import static org.apache.hadoop.ozone.s3.util.S3Consts.STORAGE_CLASS_HEADER;
import static org.apache.hadoop.ozone.s3.util.S3Utils.urlEncode;
@@ -174,22 +176,34 @@ public class TestS3GatewayMetrics {
long oriMetric = metrics.getCreateBucketSuccess();
- bucketEndpoint.put(bucketName, null,
- null, null);
- long curMetric = metrics.getCreateBucketSuccess();
+ try {
+ bucketEndpoint.put("newBucket", null, null, null);
+
+ long curMetric = metrics.getCreateBucketSuccess();
+ assertEquals(1L, curMetric - oriMetric);
+
+ } catch (OS3Exception ex) {
+ fail(); // Should not throw error for new bucket
+ }
- assertEquals(1L, curMetric - oriMetric);
}
@Test
public void testCreateBucketFailure() throws Exception {
- // Creating an error by trying to create a bucket that already exists
long oriMetric = metrics.getCreateBucketFailure();
- bucketEndpoint.put(bucketName, null, null, null);
+ try {
+ // Creating an error by trying to create a bucket that already exists
+ bucketEndpoint.put(bucketName, null, null, null);
+ fail();
- long curMetric = metrics.getCreateBucketFailure();
- assertEquals(1L, curMetric - oriMetric);
+ } catch (OS3Exception ex) {
+ Assert.assertEquals(HTTP_CONFLICT, ex.getHttpCode());
+ Assert.assertEquals(BUCKET_ALREADY_EXISTS.getCode(), ex.getCode());
+
+ long curMetric = metrics.getCreateBucketFailure();
+ assertEquals(1L, curMetric - oriMetric);
+ }
}
@Test
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
