You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "adoroszlai (via GitHub)" <gi...@apache.org> on 2025/11/07 09:17:14 UTC
Re: [PR] HDDS-13781. Certificate expiry date should consider time zone daylight saving impact [ozone]
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org