HDDS-10014. Fixed internal error on generating S3 secret

myskov · Dec 27, 2023 · b6536bc · b6536bc
1 parent fab58e3
commit b6536bc
Show file tree

Hide file tree

Showing 5 changed files with 67 additions and 41 deletions.
diff --git a/hadoop-ozone/dist/src/main/smoketest/commonlib.robot b/hadoop-ozone/dist/src/main/smoketest/commonlib.robot
@@ -51,3 +51,8 @@ Requires admin privilege
     Pass Execution If   '${SECURITY_ENABLED}' == 'false'    Skip privilege check in unsecure cluster
     Kinit test user     testuser2     testuser2.keytab
     Access should be denied    ${command}
+
+Revoke S3 secrets
+    Execute and Ignore Error             ozone s3 revokesecret -y
+    Execute and Ignore Error             ozone s3 revokesecret -y -u testuser
+    Execute and Ignore Error             ozone s3 revokesecret -y -u testuser2
diff --git a/hadoop-ozone/dist/src/main/smoketest/s3/secretgenerate.robot b/hadoop-ozone/dist/src/main/smoketest/s3/secretgenerate.robot
@@ -21,30 +21,31 @@ Library             String
 Resource            ../commonlib.robot
 Resource            ./commonawslib.robot
 Test Timeout        5 minutes
-Suite Setup         Setup s3 tests
 Default Tags        no-bucket-type
+Test Setup          Run Keywords       Kinit test user    testuser    testuser.keytab
+...                 AND                Revoke S3 secrets
+Test Teardown       Run Keyword        Revoke S3 secrets
 
 *** Variables ***
 ${ENDPOINT_URL}       http://s3g:9878
+${SECURITY_ENABLED}   true
 
 *** Test Cases ***
 
 S3 Gateway Generate Secret
-    Run Keyword if      '${SECURITY_ENABLED}' == 'true'     Kinit HTTP user
+    Pass Execution If   '${SECURITY_ENABLED}' == 'false'    Skipping this check as security is not enabled
     ${result} =         Execute                             curl -X PUT --negotiate -u : -v ${ENDPOINT_URL}/secret
-                        IF   '${SECURITY_ENABLED}' == 'true'
-                            Should contain          ${result}       HTTP/1.1 200 OK    ignore_case=True
-                            Should Match Regexp     ${result}       <awsAccessKey>.*</awsAccessKey><awsSecret>.*</awsSecret>
-                        ELSE
-                            Should contain          ${result}       S3 Secret endpoint is disabled.
-                        END
+                        Should contain          ${result}       HTTP/1.1 200 OK    ignore_case=True
+                        Should Match Regexp     ${result}       <awsAccessKey>.*</awsAccessKey><awsSecret>.*</awsSecret>
+
+S3 Gateway Secret Already Exists
+    Pass Execution If   '${SECURITY_ENABLED}' == 'false'    Skipping this check as security is not enabled
+    [Setup]             Setup v4 headers
+    ${result} =         Execute                             curl -X PUT --negotiate -u : -v ${ENDPOINT_URL}/secret
+                        Should contain          ${result}       HTTP/1.1 400 S3_SECRET_ALREADY_EXISTS    ignore_case=True
 
 S3 Gateway Generate Secret By Username
-    Run Keyword if      '${SECURITY_ENABLED}' == 'true'     Kinit test user     testuser     testuser.keytab
+    Pass Execution If   '${SECURITY_ENABLED}' == 'false'    Skipping this check as security is not enabled
     ${result} =         Execute                             curl -X PUT --negotiate -u : -v ${ENDPOINT_URL}/secret/testuser2
-                        IF   '${SECURITY_ENABLED}' == 'true'
-                            Should contain          ${result}       HTTP/1.1 200 OK    ignore_case=True
-                            Should Match Regexp     ${result}       <awsAccessKey>.*</awsAccessKey><awsSecret>.*</awsSecret>
-                        ELSE
-                            Should contain          ${result}       S3 Secret endpoint is disabled.
-                        END
+                        Should contain          ${result}       HTTP/1.1 200 OK    ignore_case=True
+                        Should Match Regexp     ${result}       <awsAccessKey>.*</awsAccessKey><awsSecret>.*</awsSecret>
diff --git a/hadoop-ozone/dist/src/main/smoketest/s3/secretrevoke.robot b/hadoop-ozone/dist/src/main/smoketest/s3/secretrevoke.robot
@@ -23,6 +23,8 @@ Resource            ./commonawslib.robot
 Test Timeout        5 minutes
 Suite Setup         Setup s3 tests
 Default Tags        no-bucket-type
+Test Setup          Run Keyword          Setup v4 headers
+
 
 *** Variables ***
 ${ENDPOINT_URL}       http://s3g:9878
@@ -31,19 +33,11 @@ ${SECURITY_ENABLED}   true
 *** Test Cases ***
 
 S3 Gateway Revoke Secret
-    Run Keyword if      '${SECURITY_ENABLED}' == 'true'     Kinit HTTP user
+    Pass Execution If   '${SECURITY_ENABLED}' == 'false'    Skipping this check as security is not enabled
     ${result} =         Execute                             curl -X DELETE --negotiate -u : -v ${ENDPOINT_URL}/secret
-                        IF   '${SECURITY_ENABLED}' == 'true'
-                            Should contain      ${result}       HTTP/1.1 200 OK    ignore_case=True
-                        ELSE
-                            Should contain      ${result}       S3 Secret endpoint is disabled.
-                        END
+                        Should contain      ${result}       HTTP/1.1 200 OK    ignore_case=True
 
 S3 Gateway Revoke Secret By Username
-    Run Keyword if      '${SECURITY_ENABLED}' == 'true'     Kinit test user     testuser     testuser.keytab
+    Pass Execution If   '${SECURITY_ENABLED}' == 'false'    Skipping this check as security is not enabled
     ${result} =         Execute                             curl -X DELETE --negotiate -u : -v ${ENDPOINT_URL}/secret/testuser2
-                        IF   '${SECURITY_ENABLED}' == 'true'
-                            Should contain      ${result}       HTTP/1.1 200 OK    ignore_case=True
-                        ELSE
-                            Should contain      ${result}       S3 Secret endpoint is disabled.
-                        END
+                        Should contain      ${result}       HTTP/1.1 200 OK    ignore_case=True
diff --git a/.../s3gateway/src/main/java/org/apache/hadoop/ozone/s3secret/S3SecretManagementEndpoint.java b/.../s3gateway/src/main/java/org/apache/hadoop/ozone/s3secret/S3SecretManagementEndpoint.java
@@ -32,6 +32,7 @@
 import javax.ws.rs.core.Response;
 import java.io.IOException;
 
+import static javax.ws.rs.core.Response.Status.BAD_REQUEST;
 import static javax.ws.rs.core.Response.Status.NOT_FOUND;
 
 /**
@@ -44,26 +45,33 @@ public class S3SecretManagementEndpoint extends S3SecretEndpointBase {
       LoggerFactory.getLogger(S3SecretManagementEndpoint.class);
 
   @PUT
-  public Response generate() throws IOException {
+  public Response generate() {
     return generateInternal(null);
   }
 
   @PUT
   @Path("/{username}")
-  public Response generate(@PathParam("username") String username)
-      throws IOException {
+  public Response generate(@PathParam("username") String username) {
     return generateInternal(username);
   }
 
-  private Response generateInternal(@Nullable String username)
-      throws IOException {
-    S3SecretResponse s3SecretResponse = new S3SecretResponse();
-    S3SecretValue s3SecretValue = generateS3Secret(username);
-    s3SecretResponse.setAwsSecret(s3SecretValue.getAwsSecret());
-    s3SecretResponse.setAwsAccessKey(s3SecretValue.getAwsAccessKey());
-    AUDIT.logReadSuccess(buildAuditMessageForSuccess(
-        S3GAction.GENERATE_SECRET, getAuditParameters()));
-    return Response.ok(s3SecretResponse).build();
+  private Response generateInternal(@Nullable String username) {
+    try {
+      S3SecretValue s3SecretValue = generateS3Secret(username);
+
+      S3SecretResponse s3SecretResponse = new S3SecretResponse();
+      s3SecretResponse.setAwsSecret(s3SecretValue.getAwsSecret());
+      s3SecretResponse.setAwsAccessKey(s3SecretValue.getAwsAccessKey());
+      AUDIT.logReadSuccess(buildAuditMessageForSuccess(
+          S3GAction.GENERATE_SECRET, getAuditParameters()));
+      return Response.ok(s3SecretResponse).build();
+    } catch (IOException e) {
+      AUDIT.logReadFailure(buildAuditMessageForFailure(
+          S3GAction.GENERATE_SECRET, getAuditParameters(), e));
+      return Response.status(BAD_REQUEST.getStatusCode(),
+              OMException.ResultCodes.S3_SECRET_ALREADY_EXISTS.toString())
+          .build();
+    }
   }
 
   private S3SecretValue generateS3Secret(@Nullable String username)

diff --git a/...op-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3secret/TestSecretGenerate.java b/...op-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3secret/TestSecretGenerate.java
@@ -22,6 +22,7 @@
 import java.security.Principal;
 import javax.ws.rs.container.ContainerRequestContext;
 import javax.ws.rs.core.MultivaluedHashMap;
+import javax.ws.rs.core.Response;
 import javax.ws.rs.core.SecurityContext;
 import javax.ws.rs.core.UriInfo;
 
@@ -30,6 +31,7 @@
 import org.apache.hadoop.ozone.client.OzoneClient;
 import org.apache.hadoop.ozone.client.OzoneClientStub;
 import org.apache.hadoop.ozone.client.protocol.ClientProtocol;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
 import org.apache.hadoop.ozone.om.helpers.S3SecretValue;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -38,6 +40,7 @@
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.junit.jupiter.MockitoExtension;
 
+import static javax.ws.rs.core.Response.Status.BAD_REQUEST;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.when;
@@ -85,7 +88,7 @@ void setUp() throws IOException {
   }
 
   @Test
-  void testSecretGenerate() throws IOException {
+  void testSecretGenerate() {
     when(principal.getName()).thenReturn(USER_NAME);
     when(securityContext.getUserPrincipal()).thenReturn(principal);
     when(context.getSecurityContext()).thenReturn(securityContext);
@@ -97,7 +100,22 @@ void testSecretGenerate() throws IOException {
   }
 
   @Test
-  void testSecretGenerateWithUsername() throws IOException {
+  void testIfSecretAlreadyExists() throws IOException {
+    when(principal.getName()).thenReturn(USER_NAME);
+    when(securityContext.getUserPrincipal()).thenReturn(principal);
+    when(context.getSecurityContext()).thenReturn(securityContext);
+    when(proxy.getS3Secret(any())).thenThrow(new OMException("Secret already exists",
+        OMException.ResultCodes.S3_SECRET_ALREADY_EXISTS));
+
+    Response response = endpoint.generate();
+
+    assertEquals(BAD_REQUEST.getStatusCode(), response.getStatus());
+    assertEquals(OMException.ResultCodes.S3_SECRET_ALREADY_EXISTS.toString(),
+        response.getStatusInfo().getReasonPhrase());
+  }
+
+  @Test
+  void testSecretGenerateWithUsername() {
     S3SecretResponse response =
             (S3SecretResponse) endpoint.generate(OTHER_USER_NAME).getEntity();
     assertEquals(USER_SECRET, response.getAwsSecret());