fix failed UT

ChenSammi · Nov 5, 2024 · 84f39ec · 84f39ec
1 parent 74a8432
commit 84f39ec
Show file tree

Hide file tree

Showing 6 changed files with 36 additions and 36 deletions.
diff --git a/...op-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/scm/TestSecretKeysApi.java b/...op-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/...-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestSecureOzoneCluster.java b/...-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<OzoneTokenIdentifier> 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/...tion-test/src/test/java/org/apache/hadoop/ozone/om/TestOzoneManagerListVolumesSecure.java b/...tion-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
@@ -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/...ger/src/main/java/org/apache/hadoop/ozone/security/OzoneDelegationTokenSecretManager.java b/...ger/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/...src/test/java/org/apache/hadoop/ozone/security/TestOzoneDelegationTokenSecretManager.java b/...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());