From 83b165ad1f3a79e2bcd60a90ea74a22a4a73450a Mon Sep 17 00:00:00 2001
From: Sammi Chen <sammichen@apache.org>
Date: Mon, 24 Jul 2023 16:17:24 +0800
Subject: [PATCH] HDDS-9068. rootCA configs should not be checked when RootCA
 is disabled.

---
 .../hadoop/hdds/security/SecurityConfig.java  | 44 ++++++++++---------
 .../security/TestRootCARotationManager.java   | 11 +++++
 2 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/SecurityConfig.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/SecurityConfig.java
index 94fc692157b4..3dd0e2e9bcd3 100644
--- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/SecurityConfig.java
+++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/SecurityConfig.java
@@ -310,30 +310,32 @@ private void validateCertificateValidityConfig() {
       throw new IllegalArgumentException(msg);
     }
 
-    if (caCheckInterval.isNegative() || caCheckInterval.isZero()) {
-      String msg = "Property " + HDDS_X509_CA_ROTATION_CHECK_INTERNAL +
-          " should not be zero or negative";
-      LOG.error(msg);
-      throw new IllegalArgumentException(msg);
-    }
+    if (autoCARotationEnabled) {
+      if (caCheckInterval.isNegative() || caCheckInterval.isZero()) {
+        String msg = "Property " + HDDS_X509_CA_ROTATION_CHECK_INTERNAL +
+            " should not be zero or negative";
+        LOG.error(msg);
+        throw new IllegalArgumentException(msg);
+      }
 
-    if (caCheckInterval.compareTo(renewalGracePeriod) >= 0) {
-      throw new IllegalArgumentException("Property value of " +
-          HDDS_X509_CA_ROTATION_CHECK_INTERNAL +
-          " should be smaller than " + HDDS_X509_RENEW_GRACE_DURATION);
-    }
+      if (caCheckInterval.compareTo(renewalGracePeriod) >= 0) {
+        throw new IllegalArgumentException("Property value of " +
+            HDDS_X509_CA_ROTATION_CHECK_INTERNAL +
+            " should be smaller than " + HDDS_X509_RENEW_GRACE_DURATION);
+      }
 
-    if (caAckTimeout.isNegative() || caAckTimeout.isZero()) {
-      String msg = "Property " + HDDS_X509_CA_ROTATION_ACK_TIMEOUT +
-          " should not be zero or negative";
-      LOG.error(msg);
-      throw new IllegalArgumentException(msg);
-    }
+      if (caAckTimeout.isNegative() || caAckTimeout.isZero()) {
+        String msg = "Property " + HDDS_X509_CA_ROTATION_ACK_TIMEOUT +
+            " should not be zero or negative";
+        LOG.error(msg);
+        throw new IllegalArgumentException(msg);
+      }
 
-    if (caAckTimeout.compareTo(renewalGracePeriod) >= 0) {
-      throw new IllegalArgumentException("Property value of " +
-          HDDS_X509_CA_ROTATION_ACK_TIMEOUT +
-          " should be smaller than " + HDDS_X509_RENEW_GRACE_DURATION);
+      if (caAckTimeout.compareTo(renewalGracePeriod) >= 0) {
+        throw new IllegalArgumentException("Property value of " +
+            HDDS_X509_CA_ROTATION_ACK_TIMEOUT +
+            " should be smaller than " + HDDS_X509_RENEW_GRACE_DURATION);
+      }
     }
 
     if (tokenSanityChecksEnabled
diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/security/TestRootCARotationManager.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/security/TestRootCARotationManager.java
index ed3ce75874c3..de1d13a5fdfb 100644
--- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/security/TestRootCARotationManager.java
+++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/security/TestRootCARotationManager.java
@@ -57,6 +57,7 @@
 
 import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_ACK_TIMEOUT;
 import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_CHECK_INTERNAL;
+import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_ENABLED;
 import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_CA_ROTATION_TIME_OF_DAY;
 import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_GRACE_DURATION_TOKEN_CHECKS_ENABLED;
 import static org.apache.hadoop.hdds.HddsConfigKeys.HDDS_X509_RENEW_GRACE_DURATION;
@@ -97,6 +98,7 @@ public void init() throws IOException, TimeoutException,
         .set(HddsConfigKeys.OZONE_METADATA_DIRS, testDir.getAbsolutePath());
     ozoneConfig
         .setBoolean(HDDS_X509_GRACE_DURATION_TOKEN_CHECKS_ENABLED, false);
+    ozoneConfig.setBoolean(HDDS_X509_CA_ROTATION_ENABLED, true);
     scm = Mockito.mock(StorageContainerManager.class);
     securityConfig = new SecurityConfig(ozoneConfig);
     scmCertClient = new SCMCertificateClient(securityConfig, null, scmID, cID,
@@ -178,6 +180,15 @@ public void testProperties() {
     } catch (Exception e) {
       fail("Should succeed");
     }
+
+    // invalid property value is ignored when auto rotation is disabled.
+    ozoneConfig.setBoolean(HDDS_X509_CA_ROTATION_ENABLED, false);
+    ozoneConfig.set(HDDS_X509_CA_ROTATION_CHECK_INTERNAL, "P28D");
+    try {
+      rootCARotationManager = new RootCARotationManager(scm);
+    } catch (Exception e) {
+      fail("Should succeed");
+    }
   }
 
   @Test