HDDS-10469. Ozone Manager should continue to work when S3 secret stor…

…age is unavailable (#6339)
apache · Jan 5, 2025 · 71de2a2 · 71de2a2
1 parent f9bd055
commit 71de2a2
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 8 deletions.
diff --git a/...ager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3GetSecretRequest.java b/...ager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3GetSecretRequest.java
@@ -188,6 +188,14 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, TermIn
                   OMException.ResultCodes.ACCESS_ID_NOT_FOUND);
             }
 
+            if (assignS3SecretValue != null && !s3SecretManager.isBatchSupported()) {
+              // A storage that does not support batch writing is likely to be a
+              // third-party secret storage that might throw an exception on write.
+              // In the case of the exception the request will fail.
+              s3SecretManager.storeSecret(assignS3SecretValue.getKerberosID(),
+                                          assignS3SecretValue);
+            }
+
             // Compose response
             final GetS3SecretResponse.Builder getS3SecretResponse =
                 GetS3SecretResponse.newBuilder().setS3Secret(

diff --git a/...er/src/main/java/org/apache/hadoop/ozone/om/response/s3/security/S3GetSecretResponse.java b/...er/src/main/java/org/apache/hadoop/ozone/om/response/s3/security/S3GetSecretResponse.java
@@ -59,12 +59,9 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
         = getOMResponse().getStatus() == OzoneManagerProtocolProtos.Status.OK;
     if (s3SecretValue != null && isOk) {
       if (s3SecretManager.isBatchSupported()) {
-        s3SecretManager.batcher().addWithBatch(batchOperation,
-            s3SecretValue.getKerberosID(), s3SecretValue);
-      } else {
-        s3SecretManager.storeSecret(s3SecretValue.getKerberosID(),
-            s3SecretValue);
-      }
+        s3SecretManager.batcher()
+            .addWithBatch(batchOperation, s3SecretValue.getKerberosID(), s3SecretValue);
+      } // else - the secret has already been stored in S3GetSecretRequest.
     }
   }
 

diff --git a/.../src/test/java/org/apache/hadoop/ozone/om/request/s3/security/TestS3GetSecretRequest.java b/.../src/test/java/org/apache/hadoop/ozone/om/request/s3/security/TestS3GetSecretRequest.java
@@ -27,9 +27,11 @@
 import org.apache.hadoop.ozone.om.OMConfigKeys;
 import org.apache.hadoop.ozone.om.OmMetadataManagerImpl;
 import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.S3SecretFunction;
 import org.apache.hadoop.ozone.om.S3SecretLockedManager;
 import org.apache.hadoop.ozone.om.OMMetrics;
 import org.apache.hadoop.ozone.om.OMMultiTenantManager;
+import org.apache.hadoop.ozone.om.S3SecretManager;
 import org.apache.hadoop.ozone.om.S3SecretManagerImpl;
 import org.apache.hadoop.ozone.om.TenantOp;
 import org.apache.hadoop.ozone.om.S3SecretCache;
@@ -76,10 +78,12 @@
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 import static org.mockito.Mockito.any;
 import static org.mockito.Mockito.doNothing;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.framework;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
@@ -319,6 +323,30 @@ public void testGetSecretOfAnotherUserAsAdmin() throws IOException {
     processSuccessSecretRequest(USER_CAROL, 1, true);
   }
 
+  @Test
+  public void testFailSecretManagerOnGetSecret() throws IOException {
+
+    // This effectively makes alice an S3 admin.
+    when(ozoneManager.isS3Admin(ugiAlice)).thenReturn(true);
+
+    S3SecretManager failingS3Secret = mock(S3SecretManager.class);
+    doThrow(new IOException("Test Exception: Failed to store secret"))
+        .when(failingS3Secret).storeSecret(any(), any());
+    when(failingS3Secret.doUnderLock(any(), any()))
+        .thenAnswer(invocationOnMock -> {
+          S3SecretFunction<Boolean> action =
+              invocationOnMock.getArgument(1, S3SecretFunction.class);
+
+          return action.accept(failingS3Secret);
+        });
+
+    when(ozoneManager.getS3SecretManager()).thenReturn(failingS3Secret);
+
+    assertThrows(Exception.class, () ->
+        processSuccessSecretRequest(USER_ALICE, 1, true)
+    );
+  }
+
   @Test
   public void testGetOwnSecretAsNonAdmin() throws IOException {
 

diff --git a/...ecret-store/src/main/java/org/apache/hadoop/ozone/s3/remote/vault/VaultS3SecretStore.java b/...ecret-store/src/main/java/org/apache/hadoop/ozone/s3/remote/vault/VaultS3SecretStore.java
@@ -72,15 +72,19 @@ public VaultS3SecretStore(String vaultAddress,
           .nameSpace(nameSpace)
           .sslConfig(sslConfig)
           .build();
-
       this.auth = auth;
-      vault = auth.auth(config);
       this.secretPath = secretPath.endsWith("/")
           ? secretPath.substring(0, secretPath.length() - 1)
           : secretPath;
     } catch (VaultException e) {
       throw new IOException("Failed to initialize remote secret store", e);
     }
+
+    try {
+      auth();
+    } catch (VaultException e) {
+      LOG.error("Failed to authenticate with remote secret store", e);
+    }
   }
 
   private void auth() throws VaultException {