HDDS-10014. Fixed internal error on generating S3 secret via HTTP (#5887

)
apache · Jan 21, 2024 · 27c1f9c · 27c1f9c
1 parent 7d2864c
commit 27c1f9c
Show file tree

Hide file tree

Showing 6 changed files with 83 additions and 37 deletions.
diff --git a/hadoop-ozone/dist/src/main/smoketest/commonlib.robot b/hadoop-ozone/dist/src/main/smoketest/commonlib.robot
@@ -32,10 +32,12 @@ Get test user principal
     [return]            ${user}/${instance}@EXAMPLE.COM
 
 Kinit HTTP user
+    Pass Execution If   '${SECURITY_ENABLED}' == 'false'    Skip in unsecure cluster
     ${principal} =      Get test user principal    HTTP
     Wait Until Keyword Succeeds      2min       10sec      Execute            kinit -k -t /etc/security/keytabs/HTTP.keytab ${principal}
 
 Kinit test user
+    Pass Execution If   '${SECURITY_ENABLED}' == 'false'    Skip in unsecure cluster
     [arguments]                      ${user}       ${keytab}
     ${TEST_USER} =      Get test user principal    ${user}
     Set Suite Variable  ${TEST_USER}

diff --git a/hadoop-ozone/dist/src/main/smoketest/s3/commonawslib.robot b/hadoop-ozone/dist/src/main/smoketest/s3/commonawslib.robot
@@ -207,3 +207,9 @@ Verify Multipart Upload
     ${tmp} =    Catenate    @{files}
     Execute    cat ${tmp} > /tmp/original${random}
     Compare files    /tmp/original${random}    /tmp/verify${random}
+
+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,37 @@ 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
+                        Execute                             ozone s3 getsecret ${OM_HA_PARAM}
+    ${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/testuser
+                        Should contain          ${result}       HTTP/1.1 200 OK    ignore_case=True
+                        Should Match Regexp     ${result}       <awsAccessKey>.*</awsAccessKey><awsSecret>.*</awsSecret>
+
+S3 Gateway Generate Secret By Username For Other 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/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
@@ -21,8 +21,9 @@ 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
 
 *** Variables ***
 ${ENDPOINT_URL}       http://s3g:9878
@@ -31,19 +32,19 @@ ${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
+                        Execute                             ozone s3 getsecret ${OM_HA_PARAM}
     ${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
+                        Execute                             ozone s3 getsecret -u testuser ${OM_HA_PARAM}
+    ${result} =         Execute                             curl -X DELETE --negotiate -u : -v ${ENDPOINT_URL}/secret/testuser
+                        Should contain      ${result}       HTTP/1.1 200 OK    ignore_case=True
+
+S3 Gateway Revoke Secret By Username For Other User
+    Pass Execution If   '${SECURITY_ENABLED}' == 'false'    Skipping this check as security is not enabled
+                        Execute                             ozone s3 getsecret -u testuser2 ${OM_HA_PARAM}
     ${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;
 
 /**
@@ -55,15 +56,26 @@ 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) throws IOException {
+    try {
+      S3SecretValue s3SecretValue = generateS3Secret(username);
+
+      S3SecretResponse s3SecretResponse = new S3SecretResponse();
+      s3SecretResponse.setAwsSecret(s3SecretValue.getAwsSecret());
+      s3SecretResponse.setAwsAccessKey(s3SecretValue.getAwsAccessKey());
+      AUDIT.logWriteSuccess(buildAuditMessageForSuccess(
+          S3GAction.GENERATE_SECRET, getAuditParameters()));
+      return Response.ok(s3SecretResponse).build();
+    } catch (OMException e) {
+      AUDIT.logWriteFailure(buildAuditMessageForFailure(
+          S3GAction.GENERATE_SECRET, getAuditParameters(), e));
+      if (e.getResult() == OMException.ResultCodes.S3_SECRET_ALREADY_EXISTS) {
+        return Response.status(BAD_REQUEST.getStatusCode(), e.getResult().toString()).build();
+      } else {
+        LOG.error("Can't execute get secret request: ", e);
+        return Response.serverError().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;
@@ -96,6 +99,21 @@ void testSecretGenerate() throws IOException {
     assertEquals(USER_NAME, response.getAwsAccessKey());
   }
 
+  @Test
+  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() throws IOException {
     S3SecretResponse response =