adoroszlai commented on code in PR #9209:
URL: https://github.com/apache/ozone/pull/9209#discussion_r2502253747
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/authority/DefaultCAServer.java:
##########
@@ -240,18 +242,21 @@ public Future<CertPath> requestCertificate(
return certPathPromise;
}
- private LocalDateTime expiryFor(LocalDateTime beginDate, NodeType role) {
+ private Duration getDuration(LocalDateTime beginDate, NodeType role) {
// When issuing certificates for sub-ca use the max certificate duration
similar to self-signed root certificate.
if (role == NodeType.SCM) {
- return beginDate.plus(config.getMaxCertificateDuration());
+ return config.getMaxCertificateDuration();
}
- return beginDate.plus(config.getDefaultCertDuration());
+ return config.getDefaultCertDuration();
}
private X509Certificate signAndStoreCertificate(
- LocalDateTime beginDate, LocalDateTime endDate,
PKCS10CertificationRequest csr, NodeType role, String certSerialId
+ LocalDateTime beginDate, Duration duration, PKCS10CertificationRequest
csr, NodeType role, String certSerialId
) throws IOException, OperatorCreationException, CertificateException {
+ ZoneId zoneId = ZoneId.systemDefault();
+ ZonedDateTime endDate = beginDate.atZone(zoneId).plus(duration);
Review Comment:
Since `beginDate` is no longer needed in the `signAndStoreCertificate`'s
caller, and it's always `now()`, we can simplify.
```suggestion
Duration duration, PKCS10CertificationRequest csr, NodeType role,
String certSerialId
) throws IOException, OperatorCreationException, CertificateException {
ZonedDateTime beginDate = ZonedDateTime.now();
ZonedDateTime endDate = beginDate.plus(duration);
```
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/authority/DefaultCAServer.java:
##########
@@ -487,14 +492,14 @@ private void generateRootCertificate(
throws IOException, SCMSecurityException {
Preconditions.checkNotNull(this.config);
LocalDateTime beginDate = LocalDateTime.now();
- LocalDateTime endDate =
- beginDate.plus(securityConfig.getMaxCertificateDuration());
+ ZoneId zoneId = ZoneId.systemDefault();
+ ZonedDateTime endDate =
beginDate.atZone(zoneId).plus(securityConfig.getMaxCertificateDuration());
Review Comment:
Similarly, here:
```java
ZonedDateTime beginDate = ZonedDateTime.now();
ZonedDateTime endDate =
beginDate.plus(securityConfig.getMaxCertificateDuration());
```
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/authority/DefaultCAServer.java:
##########
@@ -240,18 +242,21 @@ public Future<CertPath> requestCertificate(
return certPathPromise;
}
- private LocalDateTime expiryFor(LocalDateTime beginDate, NodeType role) {
+ private Duration getDuration(LocalDateTime beginDate, NodeType role) {
Review Comment:
nit: `beginDate` is no longer used, can be removed.
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/authority/DefaultCAServer.java:
##########
@@ -487,14 +492,14 @@ private void generateRootCertificate(
throws IOException, SCMSecurityException {
Preconditions.checkNotNull(this.config);
LocalDateTime beginDate = LocalDateTime.now();
- LocalDateTime endDate =
- beginDate.plus(securityConfig.getMaxCertificateDuration());
+ ZoneId zoneId = ZoneId.systemDefault();
+ ZonedDateTime endDate =
beginDate.atZone(zoneId).plus(securityConfig.getMaxCertificateDuration());
SelfSignedCertificate.Builder builder = SelfSignedCertificate.newBuilder()
.setSubject(this.subject)
.setScmID(this.scmID)
.setClusterID(this.clusterID)
.setBeginDate(beginDate)
Review Comment:
```suggestion
.setBeginDate(beginDate.toLocalDateTime())
```
##########
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/authority/DefaultCAServer.java:
##########
@@ -260,7 +265,7 @@ private X509Certificate signAndStoreCertificate(
getPrivateKey(),
getCACertificate(),
Date.from(beginDate.atZone(ZoneId.systemDefault()).toInstant()),
Review Comment:
```suggestion
Date.from(beginDate.toInstant()),
```
--
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]
