mgr/cephadm: addressing reviewer comments

Signed-off-by: Redouane Kachach <[email protected]>

Author: rkachach

src/pybind/mgr/cephadm/cert_mgr.py

@@ -42,7 +42,7 @@ def is_operationally_valid(self) -> bool:
         return self.is_valid and not self.is_close_to_expiration
 
     def get_status_description(self) -> str:
-        cert_source = 'user-made' if self.user_made else 'self-signed'
+        cert_source = 'user-made' if self.user_made else 'cephadm-signed'
         cert_target = f' ({self.target})' if self.target else ''
         cert_details = f"'{self.cert_name}{cert_target}' ({cert_source})"
         if not self.is_valid:
@@ -66,7 +66,7 @@ class CertMgr:
     It tracks known certificates and private keys, associates them with services, and ensures
     their validity. If certificates are close to expiration or invalid, depending on the configuration
     (governed by the mgr/cephadm/certificate_automated_rotation_enabled parameter), CertMgr generates
-    warnings or attempts renewal for self-signed certificates.
+    warnings or attempts renewal for cephadm-signed certificates.
 
     Additionally, CertMgr provides methods for certificate management, including retrieving, saving,
     and removing certificates and keys, as well as reporting certificate health status in case of issues.
@@ -385,8 +385,8 @@ def prepare_certificate(self,
                                f'error: {cert_info.error_info}')
 
         # Reaching this point means either certificates are not present or they are
-        # invalid self-signed certificates. Either way, we will just generate new ones.
-        logger.info(f'Generating cephadm self-signed certificates for {cert_name}/{key_name}')
+        # invalid cephadm-signed certificates. Either way, we will just generate new ones.
+        logger.info(f'Generating cephadm-signed certificates for {cert_name}/{key_name}')
         cert, pkey = self.generate_cert(host_fqdns, host_ips)
         self.mgr.cert_mgr.save_cert(cert_name, cert, host=target_host, service_name=target_service)
         self.mgr.cert_mgr.save_key(key_name, pkey, host=target_host, service_name=target_service)
@@ -434,15 +434,15 @@ def get_key(cert_name: str, key_name: str, target: Optional[str]) -> Optional[Pr
 
     def _renew_self_signed_certificate(self, cert_info: CertInfo, cert_obj: Cert) -> bool:
         try:
-            logger.info(f'Renewing self-signed certificate for {cert_info.cert_name}')
+            logger.info(f'Renewing cephadm-signed certificate for {cert_info.cert_name}')
             new_cert, new_key = self.ssl_certs.renew_cert(cert_obj.cert, self.mgr.certificate_duration_days)
             service_name, host = self.cert_store.determine_tlsobject_target(cert_info.cert_name, cert_info.target)
             self.cert_store.save_tlsobject(cert_info.cert_name, new_cert, service_name=service_name, host=host)
             key_name = cert_info.cert_name.replace('_cert', '_key')
             self.key_store.save_tlsobject(key_name, new_key, service_name=service_name, host=host)
             return True
         except SSLConfigException as e:
-            logger.error(f'Error while trying to renew self-signed certificate for {cert_info.cert_name}: {e}')
+            logger.error(f'Error while trying to renew cephadm-signed certificate for {cert_info.cert_name}: {e}')
             return False
 
     def check_services_certificates(self, fix_issues: bool = False) -> Tuple[List[str], List[CertInfo]]:
@@ -466,7 +466,7 @@ def trigger_auto_fix(cert_info: CertInfo, cert_obj: Cert) -> bool:
             if not self.mgr.certificate_automated_rotation_enabled or cert_obj.user_made:
                 return False
 
-            # This is a self-signed certificate, let's try to fix it
+            # This is a cephadm-signed certificate, let's try to fix it
             if not cert_info.is_valid:
                 # Remove the invalid certificate to force regeneration
                 service_name, host = self.cert_store.determine_tlsobject_target(cert_info.cert_name, cert_info.target)

src/pybind/mgr/cephadm/module.py

@@ -55,6 +55,7 @@
     MgrModule,
     HandleCommandResult,
     Option,
+    OptionLevel,
     NotifyType,
     MonCommandFailed,
 )
@@ -412,6 +413,7 @@ class CephadmOrchestrator(orchestrator.Orchestrator, MgrModule,
         ),
         Option(
             'certificate_check_debug_mode',
+            level=OptionLevel.DEV,
             type='bool',
             default=False,
             desc='FOR TESTING ONLY: This flag forces the certificate check instead of waiting for certificate_check_period.',

src/pybind/mgr/cephadm/services/mgmt_gateway.py

@@ -162,7 +162,7 @@ def generate_config(self, daemon_spec: CephadmDaemonDeploySpec) -> Tuple[Dict[st
 
         return daemon_config, sorted(MgmtGatewayService.get_dependencies(self.mgr))
 
-    def pre_remove(self, daemon: DaemonDescription) -> None:
+    def post_remove(self, daemon: DaemonDescription, is_failed_deploy: bool) -> None:
         """
         Called before mgmt-gateway daemon is removed.
         """

src/pybind/mgr/cephadm/services/oauth2_proxy.py

@@ -89,7 +89,7 @@ def generate_config(self, daemon_spec: CephadmDaemonDeploySpec) -> Tuple[Dict[st
 
         return daemon_config, []
 
-    def pre_remove(self, daemon: DaemonDescription) -> None:
+    def post_remove(self, daemon: DaemonDescription, is_failed_deploy: bool) -> None:
         """
         Called before mgmt-gateway daemon is removed.
         """

src/pybind/mgr/cephadm/tests/test_certmgr.py

@@ -715,7 +715,7 @@ def test_non_matching_key(self, _set_store, cephadm_module: CephadmOrchestrator)
 
     @mock.patch("cephadm.module.CephadmOrchestrator.set_store")
     def test_certificate_renewal_for_self_signed(self, _set_store, cephadm_module: CephadmOrchestrator):
-        """ Test that self-signed certificates close to expiration are renewed """
+        """ Test that cephadm-signed certificates close to expiration are renewed """
         cert_mgr = cephadm_module.cert_mgr
 
         # for services with host scope
@@ -758,7 +758,7 @@ def test_health_errors_appending(self, _set_store, cephadm_module: CephadmOrches
                                            'Detected 2 cephadm certificate(s) issues: 1 invalid, 1 expired',
                                            2,
                                            ["Certificate 'test_service_1 (target_1)' (user-made) has expired",
-                                            "Certificate 'test_service_2 (target_2)' (self-signed) is not valid (error: invalid format)"])
+                                            "Certificate 'test_service_2 (target_2)' (cephadm-signed) is not valid (error: invalid format)"])
 
         # Test in case of appending new errors we also report previous ones
         problematic_certs = [
@@ -770,8 +770,8 @@ def test_health_errors_appending(self, _set_store, cephadm_module: CephadmOrches
                                            'Detected 3 cephadm certificate(s) issues: 1 invalid, 1 expired, 1 expiring',
                                            3,
                                            ["Certificate 'test_service_1 (target_1)' (user-made) has expired",
-                                            "Certificate 'test_service_2 (target_2)' (self-signed) is not valid (error: invalid format)",
-                                            "Certificate 'test_service_3 (target_3)' (self-signed) is about to expire (remaining days: 0)"])
+                                            "Certificate 'test_service_2 (target_2)' (cephadm-signed) is not valid (error: invalid format)",
+                                            "Certificate 'test_service_3 (target_3)' (cephadm-signed) is about to expire (remaining days: 0)"])
 
     @mock.patch("cephadm.module.CephadmOrchestrator.set_store")
     def test_health_warning_on_bad_certificates(self, _set_store, cephadm_module: CephadmOrchestrator):

src/pybind/mgr/cephadm/tlsobject_store.py

@@ -143,7 +143,7 @@ def load(self) -> None:
         for k, v in self.mgr.get_store_prefix(self.store_prefix).items():
             entity = k[len(self.store_prefix):]
             if entity not in self.known_entities:
-                logger.warning(f"TLSObjectStore: Discarding unkown entity '{entity}'")
+                logger.warning(f"TLSObjectStore: Discarding unknown entity '{entity}'")
                 continue
             entity_targets = json.loads(v)
             if entity in self.per_service_name_tlsobjects or entity in self.per_host_tlsobjects: