From deacaffeec68253a6989760f6a53c641b2a7a182 Mon Sep 17 00:00:00 2001 From: andyhuangdev Date: Wed, 28 Oct 2026 13:45:04 +0800 Subject: [PATCH] HDDS-17200. Treat deleted S3 secret cache entries as absent Use getSecret when reading the secret string so revoked cache entries cannot return their old secret before flush. Add a regression test with the persisted secret still present. Tests: 75 related unit tests pass; full checkstyle and git diff --check pass. --- .../hadoop/ozone/om/S3SecretManagerImpl.java | 7 +---- .../s3/security/TestS3GetSecretRequest.java | 30 +++++++++++++++++++ 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/S3SecretManagerImpl.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/S3SecretManagerImpl.java index 5baff853f0d4..f72931c527bd 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/S3SecretManagerImpl.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/S3SecretManagerImpl.java @@ -76,16 +76,11 @@ public String getSecretString(String awsAccessKey) "awsAccessKeyId cannot be null or empty."); LOG.trace("Get secret for awsAccessKey:{}", awsAccessKey); - S3SecretValue cacheValue = s3SecretCache.get(awsAccessKey); - if (cacheValue != null) { - return cacheValue.getAwsSecret(); - } - S3SecretValue s3Secret = s3SecretStore.getSecret(awsAccessKey); + S3SecretValue s3Secret = getSecret(awsAccessKey); if (s3Secret == null) { throw new OzoneSecurityException("S3 secret not found for " + "awsAccessKeyId " + awsAccessKey, S3_SECRET_NOT_FOUND); } - updateCache(awsAccessKey, s3Secret); return s3Secret.getAwsSecret(); } 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 bcbd26bbe6b9..0aac3fd6745d 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 @@ -17,6 +17,7 @@ package org.apache.hadoop.ozone.om.request.s3.security; +import static org.apache.hadoop.hdds.security.exception.OzoneSecurityException.ResultCodes.S3_SECRET_NOT_FOUND; import static org.apache.hadoop.security.authentication.util.KerberosName.DEFAULT_MECHANISM; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertInstanceOf; @@ -40,6 +41,7 @@ import java.util.Optional; import java.util.UUID; import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.hdds.security.exception.OzoneSecurityException; import org.apache.hadoop.ipc_.RPC; import org.apache.hadoop.ipc_.Server; import org.apache.hadoop.ipc_.Server.Call; @@ -313,6 +315,34 @@ public void testFetchSecretForRevokedUser() throws IOException { s3GetSecretResponse.getS3SecretValue().getAwsSecret()); } + @Test + public void testGetSecretStringOfRevokedSecret() throws IOException { + when(ozoneManager.isS3Admin(ugiAlice)).thenReturn(true); + S3Secret secret = processSuccessSecretRequest(USER_ALICE, 1, true); + S3SecretManager secretManager = ozoneManager.getS3SecretManager(); + assertEquals(secret.getAwsSecret(), secretManager.getSecretString(USER_ALICE)); + // Keep the persisted secret until the revoke is flushed. + omMetadataManager.storeSecret(USER_ALICE, S3SecretValue.of(USER_ALICE, secret.getAwsSecret())); + + OMRequest revokeRequest = OMRequest.newBuilder() + .setClientId(UUID.randomUUID().toString()) + .setCmdType(Type.RevokeS3Secret) + .setRevokeS3SecretRequest(OzoneManagerProtocolProtos.RevokeS3SecretRequest.newBuilder() + .setKerberosID(USER_ALICE)) + .build(); + OMClientRequest request = new S3RevokeSecretRequest(revokeRequest); + request = new S3RevokeSecretRequest(request.preExecute(ozoneManager)); + OMClientResponse response = request.validateAndUpdateCache(ozoneManager, 2); + assertEquals(OzoneManagerProtocolProtos.Status.OK, response.getOMResponse().getStatus()); + + assertTrue(secretManager.cache().get(USER_ALICE).isDeleted()); + assertNotNull(omMetadataManager.getSecret(USER_ALICE)); + assertNull(secretManager.getSecret(USER_ALICE)); + OzoneSecurityException exception = assertThrows(OzoneSecurityException.class, + () -> secretManager.getSecretString(USER_ALICE)); + assertEquals(S3_SECRET_NOT_FOUND, exception.getResult()); + } + @Test public void testGetSecretOfAnotherUserAsAdmin() throws IOException {