[
https://issues.apache.org/jira/browse/HDDS-102?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16723393#comment-16723393
]
Xiaoyu Yao commented on HDDS-102:
---------------------------------
Thanks [~anu] for the patch. It looks good to me overall. Here are a few minor
comments:
PKIProfile.java
Line 57-67: I think we could move getGeneralNames() and
isSupportedGeneralName() into the implementation.
And validateGeneralName() is a good candidate for the interface method.
Also, getGeneralNameTypes() and isSupportedGeneralNameType() will be more
accurate.
Line 84: NIT: getExtensions() -> getSupportedExtensions()
Line 91: NIT: the name could be simpified into isSupported(Extension
extension), also could be potentially consolidated into
implementation detail as shown in validateExtension() to simplify the Profile
interface.
Line 105: can validateExtendedKeyUsage covered by validateExtension()?
Line 125/133: can you elaboration on the difference between isValidRDN and
validateRND, from the declaration both accept a RDN name and returns boolean?
BaseApprover.java
Line 93: need to check null for returned array from
attribute.getAttributeValues() to avoid NPE.
Line 102: NIT: the comment is incomplete.
Line 107: NIT: maybe rename it to getExtensionList to be consistent with
getExtensionsList.
Line 110: same as Line 93
CertificateServer.java
Line 107: can we move ApprovalType enum to CertificateApprover interface as
CertificateApprover#type?
CertificateSignRequest.java
Line 111: this can be moved within try-with-resource and 113 can be removed.
DefaultCAServer.java
Line 206: do we need to reread CA public/private key from file for each CSR?
This may slow down the perf of the CA server.
Line 207/208: can we use DateTime#toDate() instead of java.sql.Date.valueOf?
SecurityConfig.java
Line 176: {color:#1948a6}HDDS_X509_DEFAULT_DURATION is confusing with
HDDS_X509_CERT_DURATION_DEFAULT.{color}
Maybe we can just name them as HDDS_X509_CERT_VALID_DURATION AND
HDDS_X509_CERT_VALID_DURATION_DEFAULT
> SCM CA: SCM CA server signs certificate for approved CSR
> --------------------------------------------------------
>
> Key: HDDS-102
> URL: https://issues.apache.org/jira/browse/HDDS-102
> Project: Hadoop Distributed Data Store
> Issue Type: Sub-task
> Reporter: Xiaoyu Yao
> Assignee: Anu Engineer
> Priority: Major
> Attachments: HDDS-102-HDDS-4.001.patch, HDDS-102-HDDS-4.001.patch,
> HDDS-102-HDDS-4.002.patch
>
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
