Galsza commented on code in PR #5163:
URL: https://github.com/apache/ozone/pull/5163#discussion_r1295606753


##########
hadoop-ozone/dist/src/main/compose/ozonesecure-ha/test-root-ca-rotation.sh:
##########
@@ -64,11 +64,14 @@ wait_for_execute_command scm4.org 120 "ozone admin scm 
roles | grep scm4.org"
 wait_for_execute_command scm4.org 30 "ozone admin cert list --role=scm | grep 
scm4.org"
 
 # wait for next root CA rotation
-wait_for_execute_command scm4.org 240 "ozone admin cert info 4"
-
-wait_for_execute_command om1 30 "find /data/metadata/om/certs/ROOTCA-4.crt"
-wait_for_execute_command om2 30 "find /data/metadata/om/certs/ROOTCA-4.crt"
-wait_for_execute_command om3 30 "find /data/metadata/om/certs/ROOTCA-4.crt"
+wait_for_root_certificate scm4.org 240 4
+
+execute_robot_test om1 kinit.robot
+execute_robot_test om2 kinit.robot
+execute_robot_test om3 kinit.robot
+wait_for_execute_command om1 30 "ozone admin cert list --role=scm | grep -v 
'scm-sub' | grep 'scm'  | cut -d ' ' -f 1 | sort | tail -n 1 | xargs -I {} echo 
/data/metadata/om/certs/ROOTCA-{}.crt | xargs find"

Review Comment:
   Could you extract this to a separate command/string to avoid repeating it?



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SequenceIdGenerator.java:
##########
@@ -416,17 +436,13 @@ public static void upgradeToSequenceId(SCMMetadataStore 
scmMetadataStore)
         while (iterator.hasNext()) {
           X509Certificate cert =
               iterator.next().getValue().getX509Certificate();
-          if (HASecurityUtils.isSelfSignedCertificate(cert) &&
-              HASecurityUtils.isCACertificate(cert)) {
-            largestRootCertId =
-                Long.max(cert.getSerialNumber().longValueExact(),
-                    largestRootCertId);
-          }
+          largestCertId = Long.max(

Review Comment:
   Is it possible here that we have an older certificate that still has an id 
out of the long range here?



##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SequenceIdGenerator.java:
##########
@@ -393,20 +398,35 @@ public static void upgradeToSequenceId(SCMMetadataStore 
scmMetadataStore)
           CONTAINER_ID, sequenceIdTable.get(CONTAINER_ID));
     }
 
-    // upgrade root certificate ID
-    if (sequenceIdTable.get(ROOT_CERTIFICATE_ID) == null) {
-      long largestRootCertId = BigInteger.ONE.longValueExact();
+    upgradeToCertificateSequenceId(scmMetadataStore, false);
+  }
+
+  public static void upgradeToCertificateSequenceId(
+      SCMMetadataStore scmMetadataStore, boolean force) throws IOException {
+    Table<String, Long> sequenceIdTable = 
scmMetadataStore.getSequenceIdTable();
+
+    // upgrade certificate ID table
+    if (sequenceIdTable.get(CERTIFICATE_ID) == null || force) {
+      // Start from ID 2.
+      // ID 1 - root certificate, ID 2 - first SCM certificate.
+      long largestCertId = BigInteger.ONE.add(BigInteger.ONE).longValueExact();
       try (TableIterator<BigInteger,
           ? extends KeyValue<BigInteger, X509Certificate>> iterator =
                scmMetadataStore.getValidSCMCertsTable().iterator()) {
         while (iterator.hasNext()) {
           X509Certificate cert = iterator.next().getValue();
-          if (HASecurityUtils.isSelfSignedCertificate(cert) &&
-              HASecurityUtils.isCACertificate(cert)) {
-            largestRootCertId =
-                Long.max(cert.getSerialNumber().longValueExact(),
-                    largestRootCertId);
-          }
+          largestCertId = Long.max(cert.getSerialNumber().longValueExact(),
+              largestCertId);
+        }
+      }
+
+      try (TableIterator<BigInteger,
+          ? extends KeyValue<BigInteger, X509Certificate>> iterator =
+               scmMetadataStore.getValidCertsTable().iterator()) {
+        while (iterator.hasNext()) {
+          X509Certificate cert = iterator.next().getValue();
+          largestCertId = Long.max(

Review Comment:
   Is it possible here that we have an older certificate that still has an id 
out of the long range here?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to