adoroszlai commented on code in PR #6630:
URL: https://github.com/apache/ozone/pull/6630#discussion_r1590233790
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/OzoneDelegationTokenSecretManager.java:
##########
@@ -441,10 +441,12 @@ public boolean verifySignature(OzoneTokenIdentifier
identifier,
signerCert = getCertClient().getCertificate(
identifier.getOmCertSerialId());
} catch (CertificateException e) {
+ LOG.error("getCertificate with identifier {} failed", identifier, e);
return false;
}
if (signerCert == null) {
+ LOG.error("signerCert is null");
Review Comment:
Should this message also indicate which certificate was not found?
```suggestion
LOG.error("signerCert is null for serialId {}",
identifier.getOmCertSerialId());
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/security/OzoneDelegationTokenSecretManager.java:
##########
@@ -441,10 +441,12 @@ public boolean verifySignature(OzoneTokenIdentifier
identifier,
signerCert = getCertClient().getCertificate(
identifier.getOmCertSerialId());
} catch (CertificateException e) {
+ LOG.error("getCertificate with identifier {} failed", identifier, e);
Review Comment:
I think logging the complete token identifier is unnecessary, since only the
certificate serial ID is used for the lookup.
Example from unit test:
```
getCertificate with identifier OzoneToken owner=test, renewer=, realUser=,
issueDate=1970-01-01T00:00:00Z, maxDate=2024-05-05T07:12:26.229Z,
sequenceNumber=0, masterKeyId=0, strToSign=null, signature=null,
awsAccessKeyId=null, omServiceId=null, omCertSerialId=1927393 failed
```
```suggestion
LOG.error("getCertificate failed for serialId {}",
identifier.getOmCertSerialId(), e);
```
On the other hand, it may be useful to add some common information from the
token identifier to all four (2 new, 2 existing) messages.
--
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]
