Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public class ListSubcommand extends ScmCertSubcommand {
description = "Filter certificate by the type: valid or revoked",
defaultValue = "valid", showDefaultValue = Visibility.ALWAYS)
private String type;
private static final String OUTPUT_FORMAT = "%-17s %-30s %-30s %-110s";
private static final String OUTPUT_FORMAT = "%-17s %-30s %-30s %-110s %-110s";

private HddsProtos.NodeType parseCertRole(String r) {
if (r.equalsIgnoreCase("om")) {
Expand All @@ -79,17 +79,20 @@ private HddsProtos.NodeType parseCertRole(String r) {

private void printCert(X509Certificate cert) {
LOG.info(String.format(OUTPUT_FORMAT, cert.getSerialNumber(),
cert.getNotBefore(), cert.getNotAfter(), cert.getSubjectDN()));
cert.getNotBefore(), cert.getNotAfter(), cert.getSubjectDN(),
cert.getIssuerDN()));
}

@Override
protected void execute(SCMSecurityProtocol client) throws IOException {
boolean isRevoked = type.equalsIgnoreCase("revoked");
List<String> certPemList = client.listCertificate(
parseCertRole(role), startSerialId, count, isRevoked);
LOG.info("Total {} {} certificates: ", certPemList.size(), type);
HddsProtos.NodeType nodeType = parseCertRole(role);
List<String> certPemList = client.listCertificate(nodeType,
startSerialId, count, isRevoked);
LOG.info("Certificate list:(Type={}, BatchSize={}, CertCount={})",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: for clarity, CertCount can be reworked to TotalCertCount.

Changes look good. Thanks. +1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@neils-dev , thanks for the code review. TotalCertCount will be misleading. It will make user think there is only this amount of certs in the cluster. This case happened before.

type.toUpperCase(), count, certPemList.size());
LOG.info(String.format(OUTPUT_FORMAT, "SerialNumber", "Valid From",
"Expiry", "Subject"));
"Expiry", "Subject", "Issuer"));
for (String certPemStr : certPemList) {
try {
X509Certificate cert = CertificateCodec.getX509Certificate(certPemStr);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@ Setup Test
*** Test Cases ***
List valid certificates
${output} = Execute ozone admin cert list
Should Contain ${output} valid certificates
Should Contain ${output} Type=VALID

List revoked certificates
${output} = Execute ozone admin cert list -t revoked
Should Contain ${output} Total 0 revoked certificates
Should Contain ${output} Certificate list:(Type=REVOKED, BatchSize=20, CertCount=0)

Info of the cert
${output} = Execute for id in $(ozone admin cert list -c 1|grep UTC|awk '{print $1}'); do ozone admin cert info $id; done
Expand Down