From 71de2a200aaa1f2d8885745312e5bbf8503bfcb7 Mon Sep 17 00:00:00 2001 From: Cyrill Date: Sun, 5 Jan 2025 15:37:20 +0300 Subject: [PATCH] HDDS-10469. Ozone Manager should continue to work when S3 secret storage is unavailable (#6339) --- .../s3/security/S3GetSecretRequest.java | 8 ++++++ .../s3/security/S3GetSecretResponse.java | 9 ++---- .../s3/security/TestS3GetSecretRequest.java | 28 +++++++++++++++++++ .../s3/remote/vault/VaultS3SecretStore.java | 8 ++++-- 4 files changed, 45 insertions(+), 8 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3GetSecretRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3GetSecretRequest.java index bb57a58dd00..31df897513e 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/security/S3GetSecretRequest.java +++ b/hadoop-ozone/ozone-manager/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/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/security/S3GetSecretResponse.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/security/S3GetSecretResponse.java index df55af31fd6..6e30f755f68 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/security/S3GetSecretResponse.java +++ b/hadoop-ozone/ozone-manager/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/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/s3/security/TestS3GetSecretRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/s3/security/TestS3GetSecretRequest.java index 47cc293e280..9c80a05c127 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/s3/security/TestS3GetSecretRequest.java +++ b/hadoop-ozone/ozone-manager/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 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/hadoop-ozone/s3-secret-store/src/main/java/org/apache/hadoop/ozone/s3/remote/vault/VaultS3SecretStore.java b/hadoop-ozone/s3-secret-store/src/main/java/org/apache/hadoop/ozone/s3/remote/vault/VaultS3SecretStore.java index c9bb4d6435e..a96b888994f 100644 --- a/hadoop-ozone/s3-secret-store/src/main/java/org/apache/hadoop/ozone/s3/remote/vault/VaultS3SecretStore.java +++ b/hadoop-ozone/s3-secret-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 {