You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "Tejaskriya (via GitHub)" <gi...@apache.org> on 2023/07/17 06:36:46 UTC
[GitHub] [ozone] Tejaskriya opened a new pull request, #5078: HDDS-8256. ozone admin cert list should provide JSON output option
Tejaskriya opened a new pull request, #5078:
URL: https://github.com/apache/ozone/pull/5078
## What changes were proposed in this pull request?
Add the option for the command `ozone admin cert list` to output as JSON
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-8256
## How was this patch tested?
Added new Robot test. Also verified the JSON out using a docker-compose cluster:
```
bash-4.2$ ozone admin cert list --json | more
Certificate list:(Type=VALID, BatchSize=20, CertCount=8)
[ {
"serialNumber" : 554359750626,
"validFrom" : "Thu Jul 13 07:32:58 UTC 2023",
"expiry" : "Sun Aug 20 07:32:58 UTC 2028",
"subjectDN" : "CN=scm-sub-554314287126@scm,OU=899b84a5-3c9a-4245-a2c4-f3b74158c512,O=CID-94fd40c6-5fe3-466c-8b72-06e8f93b6e55",
"issuerDN" : "CN=scm-1@scm,OU=899b84a5-3c9a-4245-a2c4-f3b74158c512,O=CID-94fd40c6-5fe3-466c-8b72-06e8f93b6e55"
}, {
"serialNumber" : 571775840717,
"validFrom" : "Thu Jul 13 07:33:15 UTC 2023",
"expiry" : "Fri Jul 12 07:33:15 UTC 2024",
"subjectDN" : "CN=dn@4c415ef3e295,OU=899b84a5-3c9a-4245-a2c4-f3b74158c512,O=CID-94fd40c6-5fe3-466c-8b72-06e8f93b6e55",
"issuerDN" : "CN=scm-sub-554314287126@scm,OU=899b84a5-3c9a-4245-a2c4-f3b74158c512,O=CID-94fd40c6-5fe3-466c-8b72-06e8f93b6e55"
}, {
"serialNumber" : 571816886884,
"validFrom" : "Thu Jul 13 07:33:15 UTC 2023",
"expiry" : "Fri Jul 12 07:33:15 UTC 2024",
"subjectDN" : "CN=recon@recon,OU=899b84a5-3c9a-4245-a2c4-f3b74158c512,O=CID-94fd40c6-5fe3-466c-8b72-06e8f93b6e55",
"issuerDN" : "CN=scm-sub-554314287126@scm,OU=899b84a5-3c9a-4245-a2c4-f3b74158c512,O=CID-94fd40c6-5fe3-466c-8b72-06e8f93b6e55"
}, {
--More--
```
--
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
[GitHub] [ozone] adoroszlai merged pull request #5078: HDDS-8256. ozone admin cert list should provide JSON output option
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #5078:
URL: https://github.com/apache/ozone/pull/5078
--
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
[GitHub] [ozone] adoroszlai commented on a diff in pull request #5078: HDDS-8256. ozone admin cert list should provide JSON output option
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #5078:
URL: https://github.com/apache/ozone/pull/5078#discussion_r1266550824
##########
hadoop-ozone/dist/src/main/smoketest/admincli/cert.robot:
##########
@@ -0,0 +1,39 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+*** Settings ***
+Documentation Test ozone admin cert command
+Resource ../commonlib.robot
+Test Timeout 5 minutes
+
+*** Keywords ***
+Setup Test
+ Run Keyword if '${SECURITY_ENABLED}' == 'true' Kinit test user testuser testuser.keytab
+
+*** Test Cases ***
+Run cert list
+ Pass Execution If '${SECURITY_ENABLED}' == 'false' N/A
+ ${output} = Execute ozone admin cert list
+ Should Contain ${output} Certificate list:(Type=
+
+cert list as JSON
Review Comment:
I'd suggest `List certificates` and `List certificates as JSON` (similar test case names are used in other `admincli` tests).
--
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
[GitHub] [ozone] ashishkumar50 commented on a diff in pull request #5078: HDDS-8256. ozone admin cert list should provide JSON output option
Posted by "ashishkumar50 (via GitHub)" <gi...@apache.org>.
ashishkumar50 commented on code in PR #5078:
URL: https://github.com/apache/ozone/pull/5078#discussion_r1266532941
##########
hadoop-ozone/dist/src/main/smoketest/admincli/cert.robot:
##########
@@ -0,0 +1,39 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+*** Settings ***
+Documentation Test ozone admin cert command
+Resource ../commonlib.robot
+Test Timeout 5 minutes
+
+*** Keywords ***
+Setup Test
+ Run Keyword if '${SECURITY_ENABLED}' == 'true' Kinit test user testuser testuser.keytab
+
+*** Test Cases ***
+Run cert list
+ Pass Execution If '${SECURITY_ENABLED}' == 'false' N/A
+ ${output} = Execute ozone admin cert list
+ Should Contain ${output} Certificate list:(Type=
+
+cert list as JSON
Review Comment:
```suggestion
Cert list as JSON
```
--
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
[GitHub] [ozone] ashishkumar50 commented on a diff in pull request #5078: HDDS-8256. ozone admin cert list should provide JSON output option
Posted by "ashishkumar50 (via GitHub)" <gi...@apache.org>.
ashishkumar50 commented on code in PR #5078:
URL: https://github.com/apache/ozone/pull/5078#discussion_r1265019475
##########
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/cert/ListSubcommand.java:
##########
@@ -89,12 +104,31 @@ protected void execute(SCMSecurityProtocol client) throws IOException {
HddsProtos.NodeType nodeType = parseCertRole(role);
List<String> certPemList = client.listCertificate(nodeType,
startSerialId, count, isRevoked);
- LOG.info("Certificate list:(Type={}, BatchSize={}, CertCount={})",
- type.toUpperCase(), count, certPemList.size());
if (count == certPemList.size()) {
LOG.info("The certificate list could be longer than the batch size: {}." +
Review Comment:
Please output this to err, else json parse will fail if this prints along with json.
--
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
[GitHub] [ozone] Tejaskriya commented on pull request #5078: HDDS-8256. ozone admin cert list should provide JSON output option
Posted by "Tejaskriya (via GitHub)" <gi...@apache.org>.
Tejaskriya commented on PR #5078:
URL: https://github.com/apache/ozone/pull/5078#issuecomment-1637958984
> Can we change below :
>
> ```
> "subjectDN" : "CN=scm-sub-554314287126@scm,OU=899b84a5-3c9a-4245-a2c4-f3b74158c512,O=CID-94fd40c6-5fe3-466c-8b72-06e8f93b6e55",
> "issuerDN" : "CN=scm-1@scm,OU=899b84a5-3c9a-4245-a2c4-f3b74158c512,O=CID-94fd40c6-5fe3-466c-8b72-06e8f93b6e55"
> ```
>
> to :
>
> ```
> "subjectDN" : {"CN":"scm-sub-554314287126@scm","OU":"899b84a5-3c9a-4245-a2c4-f3b74158c512","O":"CID-94fd40c6-5fe3-466c-8b72-06e8f93b6e55"},
> "issuerDN" : {"CN":"scm-1@scm","OU":"899b84a5-3c9a-4245-a2c4-f3b74158c512","O":"CID-94fd40c6-5fe3-466c-8b72-06e8f93b6e55"}
> ```
>
> This will help for a better retrieval
I have made the changes for this in my latest commit, please review the new implementation. Thank you!
--
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
[GitHub] [ozone] Tejaskriya commented on a diff in pull request #5078: HDDS-8256. ozone admin cert list should provide JSON output option
Posted by "Tejaskriya (via GitHub)" <gi...@apache.org>.
Tejaskriya commented on code in PR #5078:
URL: https://github.com/apache/ozone/pull/5078#discussion_r1266377417
##########
hadoop-ozone/dist/src/main/smoketest/admincli/cert.robot:
##########
@@ -0,0 +1,37 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+*** Settings ***
+Documentation Test ozone admin cert command
+Resource ../commonlib.robot
+Test Timeout 5 minutes
+
+*** Keywords ***
+Setup Test
+ Run Keyword if '${SECURITY_ENABLED}' == 'true' Kinit test user testuser testuser.keytab
+
+*** Test Cases ***
+Run cert list
+ Pass Execution If '${SECURITY_ENABLED}' == 'false' N/A
+ ${output} = Execute ozone admin cert list
+ Should Contain ${output} Certificate list:(Type=
+
+cert list as JSON
+ Pass Execution If '${SECURITY_ENABLED}' == 'false' N/A
+ ${output} = Execute ozone admin cert list --json 2>> certInfo
+ ${errOutput} = Execute cat certInfo
+ Execute echo '${output}' | jq -r '.'
Review Comment:
Thank you for pointing that out, I have fixed it now.
--
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
[GitHub] [ozone] Tejaskriya commented on a diff in pull request #5078: HDDS-8256. ozone admin cert list should provide JSON output option
Posted by "Tejaskriya (via GitHub)" <gi...@apache.org>.
Tejaskriya commented on code in PR #5078:
URL: https://github.com/apache/ozone/pull/5078#discussion_r1266597755
##########
hadoop-ozone/dist/src/main/smoketest/admincli/cert.robot:
##########
@@ -0,0 +1,39 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+*** Settings ***
+Documentation Test ozone admin cert command
+Resource ../commonlib.robot
+Test Timeout 5 minutes
+
+*** Keywords ***
+Setup Test
+ Run Keyword if '${SECURITY_ENABLED}' == 'true' Kinit test user testuser testuser.keytab
+
+*** Test Cases ***
+Run cert list
+ Pass Execution If '${SECURITY_ENABLED}' == 'false' N/A
+ ${output} = Execute ozone admin cert list
+ Should Contain ${output} Certificate list:(Type=
+
+cert list as JSON
Review Comment:
I have updated it
--
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
[GitHub] [ozone] Tejaskriya commented on a diff in pull request #5078: HDDS-8256. ozone admin cert list should provide JSON output option
Posted by "Tejaskriya (via GitHub)" <gi...@apache.org>.
Tejaskriya commented on code in PR #5078:
URL: https://github.com/apache/ozone/pull/5078#discussion_r1266597755
##########
hadoop-ozone/dist/src/main/smoketest/admincli/cert.robot:
##########
@@ -0,0 +1,39 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+*** Settings ***
+Documentation Test ozone admin cert command
+Resource ../commonlib.robot
+Test Timeout 5 minutes
+
+*** Keywords ***
+Setup Test
+ Run Keyword if '${SECURITY_ENABLED}' == 'true' Kinit test user testuser testuser.keytab
+
+*** Test Cases ***
+Run cert list
+ Pass Execution If '${SECURITY_ENABLED}' == 'false' N/A
+ ${output} = Execute ozone admin cert list
+ Should Contain ${output} Certificate list:(Type=
+
+cert list as JSON
Review Comment:
I have updated it with your suggestion
--
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
[GitHub] [ozone] adoroszlai commented on pull request #5078: HDDS-8256. ozone admin cert list should provide JSON output option
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #5078:
URL: https://github.com/apache/ozone/pull/5078#issuecomment-1640898803
Thanks @Tejaskriya for the patch, @ashishkumar50, @ssulav for the review.
--
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
[GitHub] [ozone] adoroszlai commented on a diff in pull request #5078: HDDS-8256. ozone admin cert list should provide JSON output option
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #5078:
URL: https://github.com/apache/ozone/pull/5078#discussion_r1264996419
##########
hadoop-ozone/dist/src/main/smoketest/admincli/cert.robot:
##########
@@ -0,0 +1,35 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+*** Settings ***
+Documentation Smoketest ozone cluster startup
+Library OperatingSystem
+Library BuiltIn
+Resource ../commonlib.robot
+Test Timeout 5 minutes
+
+*** Variables ***
+
+*** Test Cases ***
+Run cert list
+ ${output} = Execute ozone admin cert list
+ Should Contain ${output} Certificate list:(Type=
+
+cert list as JSON
+ ${output} = Execute ozone admin cert list --json 2>> certInfo
+ ${errOutput} = Execute cat certInfo
+ Execute echo '${output}' | jq -r '.'
+ Should Contain ${errOutput} Certificate list:(Type=
+ Execute rm certInfo
Review Comment:
I think this works only if security is enabled, so we need something like this:
https://github.com/apache/ozone/blob/f182b81751532ed5fa3b15aac7e6061a9f7f73d0/hadoop-ozone/dist/src/main/smoketest/basic/ozone-shell-lib.robot#L213
as the test is [failing](https://github.com/Tejaskriya/ozone/actions/runs/5572469036/jobs/10179226421#step:5:679) in unsecure:
```
Run cert list | FAIL |
255 != 0
------------------------------------------------------------------------------
cert list as JSON | FAIL |
255 != 0
```
--
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
[GitHub] [ozone] Tejaskriya commented on a diff in pull request #5078: HDDS-8256. ozone admin cert list should provide JSON output option
Posted by "Tejaskriya (via GitHub)" <gi...@apache.org>.
Tejaskriya commented on code in PR #5078:
URL: https://github.com/apache/ozone/pull/5078#discussion_r1266233002
##########
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/cert/ListSubcommand.java:
##########
@@ -154,15 +157,41 @@ private static class Certificate {
private BigInteger serialNumber;
private String validFrom;
private String expiry;
- private String subjectDN;
- private String issuerDN;
+ private Map<String, String> subjectDN = new HashMap<>();
+ private Map<String, String> issuerDN = new HashMap<>();;
Certificate(X509Certificate cert) {
serialNumber = cert.getSerialNumber();
validFrom = cert.getNotBefore().toString();
expiry = cert.getNotAfter().toString();
- subjectDN = cert.getSubjectDN().toString();
- issuerDN = cert.getIssuerDN().toString();
+
+ String[] subject = cert.getSubjectDN().getName().split(",");
+ if(subject.length == 0) {
+ err.println("Invalid format of subjectDN name");
+ } else {
+ for(String elem : subject) {
+ String[] subjectComponents = elem.split("=");
+ if(subjectComponents.length == 2) {
+ subjectDN.put(subjectComponents[0], subjectComponents[1]);
+ } else {
+ err.println("Invalid format of subjectDN name");
+ }
+ }
+ }
Review Comment:
I have changed it now, Thank you
--
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
[GitHub] [ozone] ashishkumar50 commented on a diff in pull request #5078: HDDS-8256. ozone admin cert list should provide JSON output option
Posted by "ashishkumar50 (via GitHub)" <gi...@apache.org>.
ashishkumar50 commented on code in PR #5078:
URL: https://github.com/apache/ozone/pull/5078#discussion_r1266291217
##########
hadoop-ozone/dist/src/main/smoketest/admincli/cert.robot:
##########
@@ -0,0 +1,37 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+*** Settings ***
+Documentation Test ozone admin cert command
+Resource ../commonlib.robot
+Test Timeout 5 minutes
+
+*** Keywords ***
+Setup Test
+ Run Keyword if '${SECURITY_ENABLED}' == 'true' Kinit test user testuser testuser.keytab
+
+*** Test Cases ***
+Run cert list
+ Pass Execution If '${SECURITY_ENABLED}' == 'false' N/A
+ ${output} = Execute ozone admin cert list
+ Should Contain ${output} Certificate list:(Type=
+
+cert list as JSON
+ Pass Execution If '${SECURITY_ENABLED}' == 'false' N/A
+ ${output} = Execute ozone admin cert list --json 2>> certInfo
+ ${errOutput} = Execute cat certInfo
+ Execute echo '${output}' | jq -r '.'
Review Comment:
I think this may not be working, output contains only err stream which is not in json. We should redirect out and verify json element.
--
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
[GitHub] [ozone] Tejaskriya commented on a diff in pull request #5078: HDDS-8256. ozone admin cert list should provide JSON output option
Posted by "Tejaskriya (via GitHub)" <gi...@apache.org>.
Tejaskriya commented on code in PR #5078:
URL: https://github.com/apache/ozone/pull/5078#discussion_r1265229367
##########
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/cert/ListSubcommand.java:
##########
@@ -89,12 +104,31 @@ protected void execute(SCMSecurityProtocol client) throws IOException {
HddsProtos.NodeType nodeType = parseCertRole(role);
List<String> certPemList = client.listCertificate(nodeType,
startSerialId, count, isRevoked);
- LOG.info("Certificate list:(Type={}, BatchSize={}, CertCount={})",
- type.toUpperCase(), count, certPemList.size());
if (count == certPemList.size()) {
LOG.info("The certificate list could be longer than the batch size: {}." +
Review Comment:
Thank you, I have fixed it now
--
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
[GitHub] [ozone] adoroszlai commented on a diff in pull request #5078: HDDS-8256. ozone admin cert list should provide JSON output option
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #5078:
URL: https://github.com/apache/ozone/pull/5078#discussion_r1265775308
##########
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/cert/ListSubcommand.java:
##########
@@ -154,15 +157,41 @@ private static class Certificate {
private BigInteger serialNumber;
private String validFrom;
private String expiry;
- private String subjectDN;
- private String issuerDN;
+ private Map<String, String> subjectDN = new HashMap<>();
+ private Map<String, String> issuerDN = new HashMap<>();;
Certificate(X509Certificate cert) {
serialNumber = cert.getSerialNumber();
validFrom = cert.getNotBefore().toString();
expiry = cert.getNotAfter().toString();
- subjectDN = cert.getSubjectDN().toString();
- issuerDN = cert.getIssuerDN().toString();
+
+ String[] subject = cert.getSubjectDN().getName().split(",");
+ if(subject.length == 0) {
+ err.println("Invalid format of subjectDN name");
+ } else {
+ for(String elem : subject) {
+ String[] subjectComponents = elem.split("=");
+ if(subjectComponents.length == 2) {
+ subjectDN.put(subjectComponents[0], subjectComponents[1]);
+ } else {
+ err.println("Invalid format of subjectDN name");
+ }
+ }
+ }
Review Comment:
Please extract this logic to avoid duplication (with `issuerDN`).
##########
hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/cert/ListSubcommand.java:
##########
@@ -154,15 +157,41 @@ private static class Certificate {
private BigInteger serialNumber;
private String validFrom;
private String expiry;
- private String subjectDN;
- private String issuerDN;
+ private Map<String, String> subjectDN = new HashMap<>();
Review Comment:
Please use `LinkedHashMap` for deterministic order.
--
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