fapifta commented on code in PR #5404:
URL: https://github.com/apache/ozone/pull/5404#discussion_r1366825064
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/security/x509/certificate/client/CertificateClient.java:
##########
@@ -250,6 +250,11 @@ void registerRootCARotationListener(
* */
InitResponse init() throws CertificateException;
+ /**
+ * Recover the state if needed.
+ * */
+ void recoverStateIfNeeded(InitResponse state) throws IOException;
Review Comment:
I think at the end of the day we do not need this method to be public.
We can call this method during handleCase() within the
DefaultCertificateClient, and based on the result, we can provide a success or
failure as InitResponse.
SCMCertificateClient would require some work, as there a GETCERT response is
either handled locally or by sending a CSR to an other SCM. But I think as the
two methods that are called are static, we can move it to the
SCMCertificateClient class instead of HASecurityUtil.
OzoneManager has also a static method within which we save the certificate
serial id to the OmStorage instance, for this I believe we can provide a
certIdSaveCallback properly.
With that we potentially do not need to expose InitResponse, and we can
think about changing init() to be void, and just throw an exception on failure.
Also if we do all this, I believe then changing from generating a new CSR to
request the cert based on a known ID at failure is a localized change that we
may or may not do.
##########
hadoop-ozone/dist/src/main/compose/ozonesecure/test-public-key-cert-recovery.sh:
##########
@@ -0,0 +1,55 @@
+#!/usr/bin/env bash
+# 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.
+
+#suite:cert-rotation
+
+COMPOSE_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
+export COMPOSE_DIR
+
+# shellcheck source=/dev/null
+source "$COMPOSE_DIR/../testlib.sh"
+
+export SECURITY_ENABLED=true
+
+: ${OZONE_BUCKET_KEY_NAME:=key1}
+
+start_docker_env
+
+#test recon
+wait_for_port recon 9888 120
+wait_for_execute_command recon 60 "find /data/metadata/recon/keys/public.pem
-delete && find /data/metadata/recon/certs/*.crt -delete"
+docker-compose stop recon
+docker-compose start recon
+wait_for_port recon 9888 120
+wait_for_execute_command recon 60 "find /data/metadata/recon/keys/public.pem
&& find /data/metadata/recon/certs/ROOTCA*.crt"
Review Comment:
Shouldn't we check here for the certificate of the component (in this case
Recon) instead of checking for the rootCA cert?
--
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]
