hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Xiaoyu Yao (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDDS-696) Bootstrap genesis SCM(CA) with self-signed certificate.
Date Tue, 27 Nov 2018 07:04:00 GMT

    [ https://issues.apache.org/jira/browse/HDDS-696?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16699998#comment-16699998

Xiaoyu Yao commented on HDDS-696:

Thanks [~anu] for the patch. It looks pretty good to me. Here are a few minor comments:



Line 26: NIT: accidental change can be removed.



Line 103-106: can we put isPosix() into a util class so that the same code

can be shared between CertificateCodec and KeyCodec.

After second check, I think we don't need this as it is not being used in the code.  Line
221 calls Files.setPosixFilePermissions already have it coverred.


Line 117-118: should we have a static JcaX509CertificateConverter so that we don't have to
create each time.

This will be useful for CA. Also, we need to call setProvider() to honor the "BC" as provider
from the



Line 201: basePath is not hornored in the code. (Same on Line 248)

Line 203: SCMSecurityException is not needed to be declared here as it is a subclass of IOException.



Line 255: need to use the getInstance with provider name parameter to honor "BC" provider 
from security config.




Line 56: SCMSecurityException can be removed.




The file location does not match the package declaration.



Line 63: NIT: can we start a new line for "1. Success…"


Line 84: NIT: typo: "success"


Line 227/245: should we remove the securityConfig parameter and use the member variable config
instead if we could

let SecurityConfig passed into DefaultCAServer contstructor (like other class such as KeyCodec/HDDSKeyGenerator)
and it has been initialized outside the DefaultCAServer anyway?



Line 65-68: NIT: let's be consistent with the order of "final static"

Line 315-319: Line 324 will throw if it is not posix, do we still need a separate check here?




Line 160: NIT: empty line change can be removed.



Line 22: the package for

main/…/x509/certificates should not change its package name to main/…/x509.certificate.utils

test/…/x509/certificates should not change its package name to test/…/x509.certificate.utils

If they are moved under utils, we might be able to remove these files.



Line 20: file need to be moved under certificate.utils with the package name change.


Line 132-133: I think we should simply use endDate.atTime(LocalTime.MAX) to indicate proper
end time or

 a slightly complex one like endDate.atStartOfDay().plusDays(1).minusSeconds(1).toInstant(zoneOffset);


Line 216: do we need to +1 considering we allow the certificate to be valid from the begin
of the beginDate to the end of the endDate.


Line 219: this should be > 0, i.e., when certDuration > maxDuration we throw.


> Bootstrap genesis SCM(CA) with self-signed certificate.
> -------------------------------------------------------
>                 Key: HDDS-696
>                 URL: https://issues.apache.org/jira/browse/HDDS-696
>             Project: Hadoop Distributed Data Store
>          Issue Type: Sub-task
>            Reporter: Xiaoyu Yao
>            Assignee: Anu Engineer
>            Priority: Major
>         Attachments: HDDS-696-HDDS-4.001.patch, HDDS-696-HDDS-4.002.patch, HDDS-696-HDDS-4.003.patch
> If security is enabled, SCM will generate the CA certs and bootstrap a CA. If it is already
 bootstrapped it the keys and root certificates are read from the secure store, if not, they
are generated.

This message was sent by Atlassian JIRA

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

View raw message