From: Redouane Kachach Date: Thu, 23 Oct 2025 11:06:08 +0000 (+0200) Subject: mgr/cephadm: fix objects_by_names initialization + some improvements X-Git-Tag: testing/wip-vshankar-testing-20260219.125903~6^2~4^2~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=d06d06fda84363a57fe0c66339fcc2ebd254f4b1;p=ceph-ci.git mgr/cephadm: fix objects_by_names initialization + some improvements This commit include the following changes: 1. Fix objects_by_names initialization as we was using a dict for all the case including global scoped objects which is not correct. For those cases an instance of an empty TLSObject must be used. 2. Add sanity checks to the load() method to avoid loading incorrect and malformed entries. 3. Add some helper functions to avoid code repetition Fixes: https://tracker.ceph.com/issues/73625 Signed-off-by: Redouane Kachach --- diff --git a/src/pybind/mgr/cephadm/tests/test_certmgr.py b/src/pybind/mgr/cephadm/tests/test_certmgr.py index 0aa39a04de5..9aa3e56a4b9 100644 --- a/src/pybind/mgr/cephadm/tests/test_certmgr.py +++ b/src/pybind/mgr/cephadm/tests/test_certmgr.py @@ -773,13 +773,12 @@ class TestCertMgr(object): assert 'iscsi_ssl_key' in key_store assert isinstance(key_store['iscsi_ssl_key'], dict) and len(key_store['iscsi_ssl_key']) == 0 - # Global-scoped key with bad JSON should also not be instantiated (no PrivKey object) - # Depending on how you seed known names, it might be absent OR present but falsy. - # Accept either: absent OR not a PrivKey instance. - if 'mgmt_gateway_ssl_key' in key_store: - assert not isinstance(key_store['mgmt_gateway_ssl_key'], PrivKey) - else: - assert 'mgmt_gateway_ssl_key' not in key_store + # Global-scoped key with bad JSON should NOT be hydrated + assert 'mgmt_gateway_ssl_key' in key_store + obj = key_store.get('mgmt_gateway_ssl_key') + # It should be a PrivKey (because globals are pre-seeded) but remain empty/falsy + assert isinstance(obj, PrivKey) + assert not bool(obj) # still the tombstone, not parsed content messages = [r.getMessage() for r in caplog.records if r.levelno >= logging.WARNING and r.name == "cephadm.tlsobject_store"] diff --git a/src/pybind/mgr/cephadm/tlsobject_store.py b/src/pybind/mgr/cephadm/tlsobject_store.py index 1b5141b6eeb..2d196558fa4 100644 --- a/src/pybind/mgr/cephadm/tlsobject_store.py +++ b/src/pybind/mgr/cephadm/tlsobject_store.py @@ -1,6 +1,5 @@ from typing import (Any, Dict, - Union, List, Tuple, Optional, @@ -33,12 +32,22 @@ class TLSObjectStore(): self.mgr: CephadmOrchestrator = mgr self.cephadm_signed_object_checker = cephadm_signed_obj_checker self.tlsobject_class = tlsobject_class - all_known_objects_names = [item for sublist in known_objects_names.values() for item in sublist] - self.objects_by_name: Dict[str, Any] = {key: {} for key in all_known_objects_names} self.service_scoped_objects = known_objects_names[TLSObjectScope.SERVICE] self.host_scoped_objects = known_objects_names[TLSObjectScope.HOST] self.global_scoped_objects = known_objects_names[TLSObjectScope.GLOBAL] self.store_prefix = f'{TLSOBJECT_STORE_PREFIX}{tlsobject_class.STORAGE_PREFIX}.' + # initialize objects by name for the different scopes + self.objects_by_name: Dict[str, Any] = {} + for n in self.service_scoped_objects + self.host_scoped_objects: + self.objects_by_name[n] = {} + for n in self.global_scoped_objects: + self.objects_by_name[n] = self.tlsobject_class() + + def _kv_key(self, obj_name: str) -> str: + return self.store_prefix + obj_name + + def _set_store(self, obj_name: str, payload: Any) -> None: + self.mgr.set_store(self._kv_key(obj_name), json.dumps(payload)) def register_object_name(self, obj_name: str, scope: TLSObjectScope) -> None: """ @@ -105,18 +114,16 @@ class TLSObjectStore(): self._validate_tlsobject_name(obj_name, service_name, host) tlsobject = self.tlsobject_class(tlsobject, user_made, editable) scope, target = self.get_tlsobject_scope_and_target(obj_name, service_name, host) - j: Union[str, Dict[Any, Any], None] = None if scope in (TLSObjectScope.SERVICE, TLSObjectScope.HOST): self.objects_by_name[obj_name][target] = tlsobject - j = { + serialized_targets = { key: self.tlsobject_class.to_json(self.objects_by_name[obj_name][key]) for key in self.objects_by_name[obj_name] } - self.mgr.set_store(self.store_prefix + obj_name, json.dumps(j)) + self._set_store(obj_name, serialized_targets) elif scope == TLSObjectScope.GLOBAL: self.objects_by_name[obj_name] = tlsobject - j = self.tlsobject_class.to_json(tlsobject) - self.mgr.set_store(self.store_prefix + obj_name, json.dumps(j)) + self._set_store(obj_name, self.tlsobject_class.to_json(tlsobject)) else: logger.error(f'Trying to save TLS object name {obj_name} with a not-supported/unknown TLSObjectScope scope {scope.value}') @@ -155,20 +162,19 @@ class TLSObjectStore(): """ self._validate_tlsobject_name(obj_name, service_name, host) scope, target = self.get_tlsobject_scope_and_target(obj_name, service_name, host) - j: Union[str, Dict[Any, Any], None] = None if scope in (TLSObjectScope.SERVICE, TLSObjectScope.HOST): if obj_name in self.objects_by_name and target in self.objects_by_name[obj_name]: del self.objects_by_name[obj_name][target] - j = { + serialized_targets = { key: self.tlsobject_class.to_json(self.objects_by_name[obj_name][key]) for key in self.objects_by_name[obj_name] } - self.mgr.set_store(self.store_prefix + obj_name, json.dumps(j)) + self._set_store(obj_name, serialized_targets) return True elif scope == TLSObjectScope.GLOBAL: self.objects_by_name[obj_name] = self.tlsobject_class() - j = self.tlsobject_class.to_json(self.objects_by_name[obj_name]) - self.mgr.set_store(self.store_prefix + obj_name, json.dumps(j)) + serialized_obj = self.tlsobject_class.to_json(self.objects_by_name[obj_name]) + self._set_store(obj_name, serialized_obj) return True else: raise TLSObjectException(f'Attempted to remove {self.tlsobject_class.__name__.lower()} for unknown obj_name {obj_name}') @@ -176,7 +182,7 @@ class TLSObjectStore(): def _validate_tlsobject_name(self, obj_name: str, service_name: Optional[str] = None, host: Optional[str] = None) -> None: cred_type = self.tlsobject_class.__name__.lower() - if obj_name not in self.objects_by_name.keys(): + if obj_name not in self.objects_by_name: raise TLSObjectException(f'Attempted to access {cred_type} for unknown TLS object name {obj_name}') if obj_name in self.host_scoped_objects and not host: raise TLSObjectException(f'Need host to access {cred_type} for TLS object {obj_name}') @@ -205,25 +211,42 @@ class TLSObjectStore(): return tlsobjects def load(self) -> None: + + def _kv_preview(key: str, raw: str, n: int = 20) -> Tuple[str, int, str]: + # Safe, short, escaped preview for logs + return key, (len(raw) if raw else 0), (raw[:n] if raw else "") + for k, v in self.mgr.get_store_prefix(self.store_prefix).items(): obj_name = k[len(self.store_prefix):] is_cephadm_signed_object = self.cephadm_signed_object_checker(obj_name) if not is_cephadm_signed_object and obj_name not in self.objects_by_name: - logger.warning(f"TLSObjectStore: Discarding unknown obj_name '{obj_name}'") + logger.warning("TLSObjectStore: Discarding unknown obj_name %r", obj_name) continue try: tls_object_targets = json.loads(v) + if not isinstance(tls_object_targets, dict): + key_preview, vlen, vstart = _kv_preview(k, v) + logger.error( + "TLSObjectStore: Invalid data structure for object %r. " + "Expected dict but got %s. key=%r, len=%d, startswith=%r", + obj_name, type(tls_object_targets).__name__, + key_preview, vlen, vstart, + ) + continue except json.JSONDecodeError as e: + key_preview, vlen, vstart = _kv_preview(k, v) logger.warning( - f"TLSObjectStore: Cannot parse JSON for '{obj_name}': " - f"key={k}, len={len(v) if v else 0}, startswith={v[:20]!r}, error={e}" + "TLSObjectStore: Cannot parse JSON for %r: key=%r, len=%d, startswith=%r, error=%r", + obj_name, key_preview, vlen, vstart, e, ) continue except Exception as e: + key_preview, vlen, vstart = _kv_preview(k, v) logger.error( - f"TLSObjectStore: Unexpected error happened while trying to parse JSON for '{obj_name}': " - f"key={k}, len={len(v) if v else 0}, startswith={v[:20]!r}, error={e}" + "TLSObjectStore: Unexpected error while parsing %r: key=%r, len=%d, startswith=%r, error=%r", + obj_name, key_preview, vlen, vstart, e, + exc_info=True, # include traceback for unexpected errors ) continue @@ -231,13 +254,29 @@ class TLSObjectStore(): if is_cephadm_signed_object and obj_name not in self.host_scoped_objects: self.host_scoped_objects.append(obj_name) self.objects_by_name[obj_name] = {} - for target in tls_object_targets: - tlsobject = self.tlsobject_class.from_json(tls_object_targets[target]) - if tlsobject: - self.objects_by_name[obj_name][target] = tlsobject + for target, payload in tls_object_targets.items(): + try: + tlsobject = self.tlsobject_class.from_json(payload) + if tlsobject: # skip tombstones + self.objects_by_name[obj_name][target] = tlsobject + else: + logger.debug("TLSObjectStore: Skipping tombstone for %r (target %r)", obj_name, target) + except Exception as e: + key_preview, vlen, vstart = _kv_preview(k, str(payload)) + logger.warning( + "TLSObjectStore: Failed to decode scoped TLS object %r (target %r): %r. " + "key=%r, len=%d, startswith=%r", + obj_name, target, e, key_preview, vlen, vstart + ) elif obj_name in self.global_scoped_objects: - tlsobject = self.tlsobject_class.from_json(tls_object_targets) - if tlsobject: - self.objects_by_name[obj_name] = tlsobject + try: + self.objects_by_name[obj_name] = self.tlsobject_class.from_json(tls_object_targets) + except Exception as e: + key_preview, vlen, vstart = _kv_preview(k, v) + logger.warning( + "TLSObjectStore: Failed to decode global TLS object %r: %r. " + "key=%r, len=%d, startswith=%r", + obj_name, e, key_preview, vlen, vstart + ) else: - logger.error(f"TLSObjectStore: Found a known TLS object name {obj_name} with unknown scope!") + logger.error("TLSObjectStore: Found a known TLS object name %r with unknown scope!", obj_name) diff --git a/src/pybind/mgr/cephadm/tlsobject_types.py b/src/pybind/mgr/cephadm/tlsobject_types.py index b1c601cafe0..7aa0cb9fdb1 100644 --- a/src/pybind/mgr/cephadm/tlsobject_types.py +++ b/src/pybind/mgr/cephadm/tlsobject_types.py @@ -156,6 +156,6 @@ class PrivKey(TLSObjectProtocol): user_made = parse_bool(data.get('user_made', False)) editable = parse_bool(data.get('editable', False)) except ValueError as e: - raise TLSObjectException(f'Expected field in Cert object to be bool but got another type: {e}') + raise TLSObjectException(f'Expected field in PrivKey object to be bool but got another type: {e}') return cls(key=key, user_made=user_made, editable=editable)