From: Redouane Kachach Date: Wed, 5 Mar 2025 12:48:08 +0000 (+0100) Subject: mgr/cephadm: addressing reviewer comments X-Git-Tag: v20.3.0~386^2~1 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=ec2b3415b9514184c1e4a6454dea98bbe61da480;p=ceph.git mgr/cephadm: addressing reviewer comments Signed-off-by: Redouane Kachach --- diff --git a/src/pybind/mgr/cephadm/cert_mgr.py b/src/pybind/mgr/cephadm/cert_mgr.py index 2977916568e2e..aabc587db175f 100644 --- a/src/pybind/mgr/cephadm/cert_mgr.py +++ b/src/pybind/mgr/cephadm/cert_mgr.py @@ -42,7 +42,7 @@ class CertInfo: 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 @@ class CertMgr: 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,7 +434,7 @@ class CertMgr: 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) @@ -442,7 +442,7 @@ class CertMgr: 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 @@ class CertMgr: 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) diff --git a/src/pybind/mgr/cephadm/module.py b/src/pybind/mgr/cephadm/module.py index 2822697dd59d1..d3b027ce1fd50 100644 --- a/src/pybind/mgr/cephadm/module.py +++ b/src/pybind/mgr/cephadm/module.py @@ -55,6 +55,7 @@ from mgr_module import ( 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.', diff --git a/src/pybind/mgr/cephadm/services/mgmt_gateway.py b/src/pybind/mgr/cephadm/services/mgmt_gateway.py index d21478333ded5..1ca7bb7485578 100644 --- a/src/pybind/mgr/cephadm/services/mgmt_gateway.py +++ b/src/pybind/mgr/cephadm/services/mgmt_gateway.py @@ -162,7 +162,7 @@ class MgmtGatewayService(CephadmService): 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. """ diff --git a/src/pybind/mgr/cephadm/services/oauth2_proxy.py b/src/pybind/mgr/cephadm/services/oauth2_proxy.py index 372383bdb6a81..1b77d0faafd4b 100644 --- a/src/pybind/mgr/cephadm/services/oauth2_proxy.py +++ b/src/pybind/mgr/cephadm/services/oauth2_proxy.py @@ -89,7 +89,7 @@ class OAuth2ProxyService(CephadmService): 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. """ diff --git a/src/pybind/mgr/cephadm/tests/test_certmgr.py b/src/pybind/mgr/cephadm/tests/test_certmgr.py index 6d20791a48820..38accc16a7d53 100644 --- a/src/pybind/mgr/cephadm/tests/test_certmgr.py +++ b/src/pybind/mgr/cephadm/tests/test_certmgr.py @@ -715,7 +715,7 @@ class TestCertMgr(object): @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 @@ class TestCertMgr(object): '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 @@ class TestCertMgr(object): '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): diff --git a/src/pybind/mgr/cephadm/tlsobject_store.py b/src/pybind/mgr/cephadm/tlsobject_store.py index 52f26f7a648dc..83ff9e14a9cf6 100644 --- a/src/pybind/mgr/cephadm/tlsobject_store.py +++ b/src/pybind/mgr/cephadm/tlsobject_store.py @@ -143,7 +143,7 @@ class TLSObjectStore(): 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: