[GitHub] [ozone] neils-dev commented on a diff in pull request #4442: HDDS-8178. CertificateClient and KeyStoresFactory support multiple Sub-CA certificates in the trust chain

["neils-dev (via GitHub)" <gi...@apache.org>]

neils-dev commented on code in PR #4442:
URL: https://github.com/apache/ozone/pull/4442#discussion_r1150003228


##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/DefaultCertificateClient.java:
##########
@@ -331,6 +332,38 @@ public synchronized X509Certificate getCACertificate() {
     return firstCertificateFrom(caCertPath);
   }
 
+  /**
+   * Return all certificates in this component's trust chain,
+   * the last one is the root CA certificate.
+   */
+  public synchronized List<X509Certificate> getTrustChain() {

Review Comment:
   add Override annotation for implementation of interface



##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/ssl/ReloadingX509KeyManager.java:
##########
@@ -64,7 +65,7 @@ public class ReloadingX509KeyManager extends X509ExtendedKeyManager {
    * materials are changed.
    */
   private PrivateKey currentPrivateKey;
-  private String currentCertId;
+  private List<String> currentCertList = new ArrayList<>();

Review Comment:
   Thanks @ChenSammi for the patch with changes to the keystore and truststore.
   _Minor_: may read better if `currentCertList` _renamed_ to `currentCertIdsList` or similar to differentiate and emphasize list of serialIds from list of certificate objects. 



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org