dombizita commented on code in PR #6802:
URL: https://github.com/apache/ozone/pull/6802#discussion_r1637106565
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/utils/SelfSignedCertificate.java:
##########
@@ -322,7 +325,7 @@ public X509CertificateHolder build()
new SelfSignedCertificate(this);
try {
return rootCertificate.generateCertificate(caCertSerialId);
- } catch (OperatorCreationException | CertIOException e) {
+ } catch (OperatorCreationException | CertIOException e) {
Review Comment:
nit
```suggestion
} catch (OperatorCreationException | CertIOException e) {
```
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HddsServerUtil.java:
##########
@@ -451,6 +452,7 @@ public static String
getDatanodeIdFilePath(ConfigurationSource conf) {
*/
public static SCMSecurityProtocolClientSideTranslatorPB getScmSecurityClient(
ConfigurationSource conf) throws IOException {
+ new SecurityConfig(conf);
Review Comment:
Are these needed for the same reason as here?
https://github.com/apache/ozone/blob/217f37bca42403f38847bfdd4bd110083949dc0c/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneSecurityUtil.java#L65-L67
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/authority/DefaultCAServer.java:
##########
@@ -223,70 +219,50 @@ public Future<CertPath> requestCertificate(
CertificateApprover.ApprovalType approverType, NodeType role,
String certSerialId) {
LocalDateTime beginDate = LocalDateTime.now();
- LocalDateTime endDate;
- // When issuing certificates for sub-ca use the max certificate duration
- // similar to self signed root certificate.
- if (role == NodeType.SCM) {
- endDate = beginDate.plus(config.getMaxCertificateDuration());
- } else {
- endDate = beginDate.plus(config.getDefaultCertDuration());
- }
-
- CompletableFuture<X509CertificateHolder> xcertHolder =
- approver.inspectCSR(csr);
+ LocalDateTime endDate = expiryFor(beginDate, role);
- CompletableFuture<CertPath> xCertHolders
- = new CompletableFuture<>();
-
- if (xcertHolder.isCompletedExceptionally()) {
- // This means that approver told us there are things which it disagrees
- // with in this Certificate Request. Since the first set of sanity
- // checks failed, we just return the future object right here.
Review Comment:
NIT: maybe we could keep this comment
--
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]
