ss77892 commented on code in PR #10916:
URL: https://github.com/apache/ozone/pull/10916#discussion_r3709291939
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMSecurityProtocolServer.java:
##########
@@ -326,7 +334,29 @@ public String getSCMCertificate(ScmNodeDetailsProto
scmNodeDetails,
LOGGER.info("Processing CSR for scm {}, nodeId: {}",
scmNodeDetails.getHostName(), scmNodeDetails.getScmNodeId());
- return getEncodedCertToString(certSignReq, NodeType.SCM);
+ boolean leaderless = !storageContainerManager.checkLeader()
+ && isLeaderlessPrimaryScmSigner(storageContainerManager,
+
storageContainerManager.getScmHAManager().getRatisServer().triggerNotLeaderException(),
+ isRenew);
+
+ return getEncodedCertToString(certSignReq, NodeType.SCM, leaderless,
scmNodeDetails.getScmNodeId());
+ }
+
+ /**
+ * Single source of truth for whether this SCM should sign its own
leaderless bootstrap SCM
+ * certificate: no Ratis leader is known cluster-wide (not merely that this
node isn't leader),
+ * this is not a renewal, and this SCM hosts the primary root CA.
+ *
+ * @param scm - the serving StorageContainerManager.
+ * @param nle - the NotLeaderException produced by the local Ratis
server, or null.
+ * @param isRenew - whether this request is a certificate renewal.
+ * @return true iff the leaderless SCM-certificate signing path should be
used.
+ */
+ @VisibleForTesting
+ public static boolean isLeaderlessPrimaryScmSigner(StorageContainerManager
scm, NotLeaderException nle,
+ boolean isRenew) {
+ return !isRenew && nle != null && nle.getSuggestedLeader() == null
Review Comment:
Good question. Well, it might definitely happen that during the network
split, a new SCM is bootstrapping (we skip the renewal process) and requesting
a new certificate. That might create some mess. I think we might make the check
stricter. We should also check that the request came from the ratis member,
that it has never had a certificate, and that this node is the primary SCM.
There is still a case with an unsecured 5-node cluster that is being secured,
bootstrapped, and started. 4 nodes are started; a network split occurs for the
primary SCM and the 5th node only, and the 5th node is bootstrapped. Not sure
whether this case justifies significantly more complicated logic (disjoint the
ranges of IDs for leaderless and ratis as an option)
--
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]
