From 84f39ecdc97899f1280af3a0131bec414fa0d9c7 Mon Sep 17 00:00:00 2001 From: Sammi Chen Date: Tue, 5 Nov 2024 17:24:48 +0800 Subject: [PATCH] fix failed UT --- .../hadoop/hdds/scm/TestSecretKeysApi.java | 20 +++--------- .../hadoop/ozone/TestSecureOzoneCluster.java | 31 +++++++++---------- .../om/TestOzoneManagerListVolumesSecure.java | 2 ++ .../apache/hadoop/ozone/om/OzoneManager.java | 7 ++++- .../OzoneDelegationTokenSecretManager.java | 10 +++++- ...TestOzoneDelegationTokenSecretManager.java | 2 -- 6 files changed, 36 insertions(+), 36 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/TestSecretKeysApi.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/TestSecretKeysApi.java index fb92d91ee711..45458182344e 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/TestSecretKeysApi.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/TestSecretKeysApi.java @@ -23,7 +23,6 @@ import org.apache.hadoop.hdds.protocol.SecretKeyProtocol; import org.apache.hadoop.hdds.scm.server.SCMHTTPServerConfig; import org.apache.hadoop.hdds.scm.server.StorageContainerManager; -import org.apache.hadoop.hdds.security.exception.SCMSecretKeyException; import org.apache.hadoop.hdds.security.symmetric.ManagedSecretKey; import org.apache.hadoop.ipc.RemoteException; import org.apache.hadoop.minikdc.MiniKdc; @@ -67,7 +66,6 @@ import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_CLIENT_ADDRESS_KEY; import static org.apache.hadoop.hdds.scm.server.SCMHTTPServerConfig.ConfigStrings.HDDS_SCM_HTTP_KERBEROS_KEYTAB_FILE_KEY; import static org.apache.hadoop.hdds.scm.server.SCMHTTPServerConfig.ConfigStrings.HDDS_SCM_HTTP_KERBEROS_PRINCIPAL_KEY; -import static org.apache.hadoop.hdds.security.exception.SCMSecretKeyException.ErrorCode.SECRET_KEY_NOT_ENABLED; import static org.apache.hadoop.hdds.utils.HddsServerUtil.getSecretKeyClientForDatanode; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ADMINISTRATORS; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_SECURITY_ENABLED_KEY; @@ -245,24 +243,14 @@ public void testSecretKeyApiSuccess() throws Exception { } /** - * Verify API behavior when block token is not enable. + * Verify API behavior. */ @Test - public void testSecretKeyApiNotEnabled() throws Exception { + public void testSecretKeyApi() throws Exception { startCluster(1); SecretKeyProtocol secretKeyProtocol = getSecretKeyProtocol(); - - SCMSecretKeyException ex = assertThrows(SCMSecretKeyException.class, - secretKeyProtocol::getCurrentSecretKey); - assertEquals(SECRET_KEY_NOT_ENABLED, ex.getErrorCode()); - - ex = assertThrows(SCMSecretKeyException.class, - () -> secretKeyProtocol.getSecretKey(UUID.randomUUID())); - assertEquals(SECRET_KEY_NOT_ENABLED, ex.getErrorCode()); - - ex = assertThrows(SCMSecretKeyException.class, - secretKeyProtocol::getAllSecretKeys); - assertEquals(SECRET_KEY_NOT_ENABLED, ex.getErrorCode()); + assertNull(secretKeyProtocol.getSecretKey(UUID.randomUUID())); + assertEquals(1, secretKeyProtocol.getAllSecretKeys().size()); } /** diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestSecureOzoneCluster.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestSecureOzoneCluster.java index 4f41d516153f..8f0c6d8966f8 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestSecureOzoneCluster.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestSecureOzoneCluster.java @@ -64,6 +64,7 @@ import org.apache.hadoop.hdds.scm.server.StorageContainerManager; import org.apache.hadoop.hdds.security.exception.SCMSecurityException; import org.apache.hadoop.hdds.security.SecurityConfig; +import org.apache.hadoop.hdds.security.symmetric.ManagedSecretKey; import org.apache.hadoop.hdds.security.symmetric.SecretKeyManager; import org.apache.hadoop.hdds.security.x509.certificate.authority.CAType; import org.apache.hadoop.hdds.security.x509.certificate.authority.DefaultApprover; @@ -100,6 +101,7 @@ import org.apache.hadoop.ozone.om.protocolPB.OzoneManagerProtocolClientSideTranslatorPB; import org.apache.hadoop.ozone.security.OMCertificateClient; import org.apache.hadoop.ozone.security.OzoneTokenIdentifier; +import org.apache.hadoop.ozone.security.SecretKeyTestClient; import org.apache.hadoop.security.KerberosAuthException; import org.apache.hadoop.security.SaslRpcServer.AuthMethod; import org.apache.hadoop.security.SecurityUtil; @@ -152,7 +154,6 @@ import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.USER_MISMATCH; import static org.apache.hadoop.security.UserGroupInformation.AuthenticationMethod.KERBEROS; -import org.apache.ozone.test.LambdaTestUtils; import org.apache.ozone.test.tag.Flaky; import org.apache.ozone.test.tag.Unhealthy; import org.apache.ratis.protocol.ClientId; @@ -1182,10 +1183,10 @@ public String renewAndStoreKeyAndCertificate(boolean force) throws CertificateEx } /** - * Tests delegation token renewal after a certificate renew. + * Tests delegation token renewal after a secret key rotation. */ @Test - void testDelegationTokenRenewCrossCertificateRenew() throws Exception { + void testDelegationTokenRenewCrossSecretKeyRotation() throws Exception { initSCM(); try { scm = HddsTestUtils.getScmSimple(conf); @@ -1207,10 +1208,12 @@ void testDelegationTokenRenewCrossCertificateRenew() throws Exception { CertificateClientTestImpl certClient = new CertificateClientTestImpl(newConf, true); X509Certificate omCert = certClient.getCertificate(); - String omCertId1 = omCert.getSerialNumber().toString(); // Start OM om.setCertClient(certClient); om.setScmTopologyClient(new ScmTopologyClient(scmBlockClient)); + SecretKeyTestClient secretKeyClient = new SecretKeyTestClient(); + ManagedSecretKey secretKey1 = secretKeyClient.getCurrentSecretKey(); + om.setSecretKeyClient(secretKeyClient); om.start(); GenericTestUtils.waitFor(() -> om.isLeaderReady(), 100, 10000); @@ -1231,30 +1234,26 @@ void testDelegationTokenRenewCrossCertificateRenew() throws Exception { assertEquals(SecurityUtil.buildTokenService( om.getNodeDetails().getRpcAddress()).toString(), token1.getService().toString()); - assertEquals(omCertId1, token1.decodeIdentifier().getOmCertSerialId()); + assertEquals(secretKey1.getId().toString(), token1.decodeIdentifier().getSecretKeyId()); // Renew delegation token long expiryTime = omClient.renewDelegationToken(token1); assertThat(expiryTime).isGreaterThan(0); - // Wait for OM certificate to renew - LambdaTestUtils.await(certLifetime, 100, () -> - !StringUtils.equals(token1.decodeIdentifier().getOmCertSerialId(), - omClient.getDelegationToken(new Text("om")) - .decodeIdentifier().getOmCertSerialId())); - String omCertId2 = - certClient.getCertificate().getSerialNumber().toString(); - assertNotEquals(omCertId1, omCertId2); + // Rotate secret key + secretKeyClient.rotate(); + ManagedSecretKey secretKey2 = secretKeyClient.getCurrentSecretKey(); + assertNotEquals(secretKey1.getId(), secretKey2.getId()); // Get a new delegation token Token token2 = omClient.getDelegationToken( new Text("om")); - assertEquals(omCertId2, token2.decodeIdentifier().getOmCertSerialId()); + assertEquals(secretKey2.getId().toString(), token2.decodeIdentifier().getSecretKeyId()); - // Because old certificate is still valid, so renew old token will succeed + // Because old secret key is still valid, so renew old token will succeed expiryTime = omClient.renewDelegationToken(token1); assertThat(expiryTime) .isGreaterThan(0) - .isLessThan(omCert.getNotAfter().getTime()); + .isLessThan(secretKey2.getExpiryTime().toEpochMilli()); } finally { if (scm != null) { scm.stop(); diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumesSecure.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumesSecure.java index 72f1c3374b28..6c7cd89109e3 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumesSecure.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumesSecure.java @@ -50,6 +50,7 @@ import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClientTestImpl; import org.apache.hadoop.minikdc.MiniKdc; import org.apache.hadoop.ozone.OzoneAcl; +import org.apache.hadoop.ozone.client.SecretKeyTestClient; import org.apache.hadoop.ozone.om.exceptions.OMException; import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs; import org.apache.hadoop.ozone.om.protocolPB.OmTransportFactory; @@ -201,6 +202,7 @@ private void setupEnvironment(boolean aclEnabled, om.setScmTopologyClient(new ScmTopologyClient( new ScmBlockLocationTestingClient(null, null, 0))); om.setCertClient(new CertificateClientTestImpl(conf)); + om.setSecretKeyClient(new SecretKeyTestClient()); om.start(); // Get OM client diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index d85622b677d8..87e3670b1c92 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -1183,7 +1183,12 @@ public NetworkTopology getClusterMap() { @VisibleForTesting public void setSecretKeyClient(SecretKeyClient secretKeyClient) { this.secretKeyClient = secretKeyClient; - blockTokenMgr.setSecretKeyClient(secretKeyClient); + if (blockTokenMgr != null) { + blockTokenMgr.setSecretKeyClient(secretKeyClient); + } + if (delegationTokenMgr != null) { + delegationTokenMgr.setSecretKeyClient(secretKeyClient); + } } /** diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/OzoneDelegationTokenSecretManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/OzoneDelegationTokenSecretManager.java index bf67e3d74dc0..baa2b665cbb5 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/OzoneDelegationTokenSecretManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/OzoneDelegationTokenSecretManager.java @@ -28,6 +28,8 @@ import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; import org.apache.commons.lang3.StringUtils; import org.apache.hadoop.hdds.annotation.InterfaceAudience; import org.apache.hadoop.hdds.annotation.InterfaceStability; @@ -37,6 +39,7 @@ import org.apache.hadoop.hdds.security.exception.SCMSecurityException; import org.apache.hadoop.hdds.security.symmetric.ManagedSecretKey; import org.apache.hadoop.hdds.security.symmetric.SecretKeyClient; +import org.apache.hadoop.hdds.security.symmetric.SecretKeySignerClient; import org.apache.hadoop.hdds.security.x509.certificate.client.CertificateClient; import org.apache.hadoop.hdds.security.x509.exception.CertificateException; import org.apache.hadoop.io.Text; @@ -78,7 +81,7 @@ public class OzoneDelegationTokenSecretManager private final long tokenRemoverScanInterval; private final String omServiceId; private final OzoneManager ozoneManager; - private final SecretKeyClient secretKeyClient; + private SecretKeyClient secretKeyClient; /** * If the delegation token update thread holds this lock, it will not get @@ -623,6 +626,11 @@ public void stop() throws IOException { } } + @VisibleForTesting + public void setSecretKeyClient(SecretKeyClient client) { + this.secretKeyClient = client; + } + /** * Remove expired delegation tokens from cache and persisted store. */ diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestOzoneDelegationTokenSecretManager.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestOzoneDelegationTokenSecretManager.java index ca63be4413eb..23f2b926f6db 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestOzoneDelegationTokenSecretManager.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/security/TestOzoneDelegationTokenSecretManager.java @@ -373,8 +373,6 @@ public void testVerifySignatureSuccess() throws Exception { expiryTime, TOKEN_REMOVER_SCAN_INTERVAL); secretManager.start(certificateClient); OzoneTokenIdentifier id = new OzoneTokenIdentifier(); - id.setOmCertSerialId(certificateClient.getCertificate() - .getSerialNumber().toString()); id.setMaxDate(Time.now() + 60 * 60 * 24); id.setOwner(new Text("test")); id.setSecretKeyId(secretKeyClient.getCurrentSecretKey().getId().toString());