[
https://issues.apache.org/jira/browse/HDDS-696?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16701045#comment-16701045
]
Anu Engineer commented on HDDS-696:
-----------------------------------
[~xyao] Thanks for the review and comments. Patch v4 fixes those issues. Please
see more detailed comment below.
{quote}BlockTokenException.java#Line 26: NIT: accidental change can be removed.
{quote}
Fixed.
{quote}CertificateCodec.java - Files.setPosixFilePermissions already have it
coverred.
{quote}
You are absolutely right. Thanks for pointing this out. Removed this code. In
the KeyCodec, this function is used in test cases. I did not repeat the same
test in certificates, even though it was the idea.
{quote}static JcaX509CertificateConverter, This will be useful for CA. Also, we
need to call setProvider() to honor the "BC"
{quote}
Fixed , For the provider we want to use the default JAVA class here. When we
use the BC provider we get a parse error. I can investigate this more.
{quote}Line 201: basePath is not hornored in the code. (Same on Line 248)
{quote}
Fixed.
{quote}Line 255: need to use the getInstance with provider name parameter to
honor "BC" provider from security config.
{quote}
I am sorry, did you mean for the CertificateHolder?, that is a BC class not
from the JCA.
{quote}CertificateServer.java#Line 56: SCMSecurityException can be removed.
{quote}
Fixed.
{quote}CertificateSignRequest.java. The file location does not match the
package declaration
{quote}
Moved all files to certificates.utils.
{quote}DefaultCAServer.java# Line 63: NIT: can we start a new line for "1.
Success…", Line 84: NIT: typo: "success"
{quote}
Fixed.
{quote}Line 227/245: should we remove the securityConfig parameter and use the
member variable config instead if we could
{quote}
Fixed.
{quote}it has been initialized outside the DefaultCAServer anyway?
{quote}
The init call does that. Do you want this to be passed via ctor?
{quote}Line 65-68: NIT: let's be consistent with the order of "final static"
{quote}
Fixed.
{quote}Line 324 will throw if it is not posix, do we still need a separate
check here?
{quote}
I use this in tests to simulate failure as if the file system is not posix.
{quote}SelfSignedCertificate.java# Line 20: file need to be moved under
certificate.utils with the package name change.
{quote}
Fixed.
{quote}I think we should simply use endDate.atTime(LocalTime.MAX) to indicate
proper end time or
{quote}
Thanks, I converted both begin and endDate to use LocalTime.MIN and
LocalTime.MAX respectively.
{quote}Line 216: do we need to +1 considering we allow the certificate to be
valid from the begin
{quote}
Fixed.
> 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, HDDS-696-HDDS-4.004.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
(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
