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
Resolves: rhbz#2404347

Signed-off-by: Redouane Kachach <rkachach@ibm.com>
(cherry picked from commit d06d06fda84363a57fe0c66339fcc2ebd254f4b1)

diff --git a/src/pybind/mgr/cephadm/tests/test_certmgr.py b/src/pybind/mgr/cephadm/tests/test_certmgr.py
index 0aa39a04de51c4c9ddf4990b2b41890250fb9b4d..9aa3e56a4b9b81305cdd48f3d3dad3dbdd915df8 100644 (file)
--- 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 1b5141b6eeb1fe0a722820708feb872fb5f0a932..2d196558fa4348165a515ce679f7c8693b104d18 100644 (file)
--- 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 b1c601cafe06b96e63859945a476f76072eca07a..7aa0cb9fdb131a1da6236810126cd8abc2542b9d 100644 (file)
--- 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)