fapifta commented on code in PR #6802:
URL: https://github.com/apache/ozone/pull/6802#discussion_r1637294330
##########
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:
I don't really a fan of this comment, as it does not really tell anything
new or useful. If you look at what the code does before (if something completed
exceptionally we complete exceptionally), and look at this description than it
is easier to figure out what failure causes us to fail than just from the code,
however after renaming the variables, it is fairly visible that if the
inspection of the CSR is not successful, we complete exceptionally here.
On the other hand, we swallow a useful exception here... I will take a look
tomorrow and I will provide a change here that propagates the currently
swallowed exception together with that (I don't know how I did that) extra
space removal.
--
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]
