fix: Hierarchical overrides didn't handle removals

libsentrykube/cluster.py

@@ -15,7 +15,7 @@
 from libsentrykube.config import Config
 from libsentrykube.config import K8sConfig
 from libsentrykube.helm import HelmData
-from libsentrykube.utils import deep_merge_dict
+from libsentrykube.utils import deep_merge_dict, remove_none_values
 from libsentrykube.utils import die
 from libsentrykube.utils import workspace_root
 from yaml import safe_load
@@ -91,6 +91,8 @@ def load_cluster_configuration(config: K8sConfig, cluster_name: str) -> Cluster:
     helm_spec = data.pop("helm", {})
     helm_data = copy.deepcopy(data)
     deep_merge_dict(helm_data, helm_spec.get("values", {}))
+    # Remove keys marked for deletion (those set to None during merges)
+    remove_none_values(helm_data)
     helm_svclist = []
     helm_svcdata: dict[str, dict[str, Any]] = {}
     for svc in helm_spec.get("services", []):

libsentrykube/helm.py

@@ -14,7 +14,7 @@
 from yaml import safe_dump, safe_load_all
 
 from libsentrykube.loader import load_macros
-from libsentrykube.utils import deep_merge_dict, pretty
+from libsentrykube.utils import deep_merge_dict, pretty, remove_none_values
 
 
 @dataclass(frozen=True)
@@ -110,6 +110,8 @@ def service_names(self) -> list[str]:
     def service_data(self, service_name) -> dict[str, Any]:
         rv = copy.deepcopy(self.global_data)
         deep_merge_dict(rv, self.svc_data.get(service_name, {}))
+        # Remove keys marked for deletion (those set to None during merges)
+        remove_none_values(rv)
         return rv
 
 
@@ -204,6 +206,9 @@ def _consolidate_ctx(
     if src_files_prefix == "_values":
         ctx_region, _ = get_helm_service_data(region_name, service_name, cluster_name)
         deep_merge_dict(ctx, ctx_region)
+
+    # Remove keys marked for deletion (those set to None during merges)
+    remove_none_values(ctx)
     return ctx
 
 

libsentrykube/kube.py

@@ -50,6 +50,7 @@
     kube_convert_kind_to_func,
     kube_get_client,
     pretty,
+    remove_none_values,
     workspace_root,
 )
 
@@ -238,6 +239,9 @@ def _consolidate_variables(
     )
     deep_merge_dict(service_values, customer_values)
 
+    # Remove keys marked for deletion (those set to None during merges)
+    remove_none_values(service_values)
+
     return service_values
 
 

libsentrykube/tests/test_deep_merge.py

@@ -1,4 +1,4 @@
-from libsentrykube.utils import deep_merge_dict
+from libsentrykube.utils import deep_merge_dict, remove_none_values
 
 
 def test_basic_merge() -> None:
@@ -119,3 +119,172 @@ def test_deep_merge_double_dict_no_overwrite() -> None:
             "ceven": 7,
         },
     }
+
+
+def test_none_marks_key_for_deletion() -> None:
+    """When a key has None value in other, it should be set to None in into."""
+    into = {
+        "foo": "bar",
+        "baz": "qux",
+    }
+
+    other = {
+        "foo": None,
+    }
+
+    deep_merge_dict(into=into, other=other)
+    assert into == {
+        "foo": None,
+        "baz": "qux",
+    }
+
+
+def test_none_added_when_not_in_into() -> None:
+    """When a key has None value in other and doesn't exist in into, it should be added as None."""
+    into = {
+        "baz": "qux",
+    }
+
+    other = {
+        "foo": None,
+    }
+
+    deep_merge_dict(into=into, other=other)
+    assert into == {
+        "foo": None,
+        "baz": "qux",
+    }
+
+
+def test_none_propagates_through_multiple_merges() -> None:
+    """None values should propagate through multiple merge layers to enable deletion from base."""
+    # This simulates the hierarchical merge scenario:
+    # 1. Base service values
+    base = {
+        "feature": {"enabled": True, "config": "value"},
+        "other": "keep",
+    }
+
+    # 2. Hierarchical override that wants to remove "feature"
+    hierarchical = {
+        "feature": None,
+        "extra": "added",
+    }
+
+    # First merge: hierarchical into base
+    deep_merge_dict(into=base, other=hierarchical)
+
+    # feature should be None (marked for deletion), not removed yet
+    assert base == {
+        "feature": None,
+        "other": "keep",
+        "extra": "added",
+    }
+
+    # After cleanup, feature should be removed
+    remove_none_values(base)
+    assert base == {
+        "other": "keep",
+        "extra": "added",
+    }
+
+
+def test_remove_none_values_basic() -> None:
+    """remove_none_values should remove all keys with None values."""
+    d = {
+        "foo": None,
+        "bar": "keep",
+        "baz": None,
+    }
+
+    remove_none_values(d)
+    assert d == {"bar": "keep"}
+
+
+def test_remove_none_values_nested() -> None:
+    """remove_none_values should recursively remove None values in nested dicts."""
+    d = {
+        "top": None,
+        "nested": {
+            "inner": None,
+            "keep": "value",
+        },
+        "keep_top": "value",
+    }
+
+    remove_none_values(d)
+    assert d == {
+        "nested": {
+            "keep": "value",
+        },
+        "keep_top": "value",
+    }
+
+
+def test_remove_none_values_empty_dict() -> None:
+    """remove_none_values should handle empty dicts."""
+    d: dict = {}
+    remove_none_values(d)
+    assert d == {}
+
+
+def test_multi_layer_merge_with_cleanup() -> None:
+    """
+    Simulates the real-world scenario where:
+    1. Base service config has a feature
+    2. Group _values.yaml sets it to None
+    3. Region override also sets it to None
+    4. Final cleanup removes it
+    """
+    # Base service _values.yaml
+    service_values = {
+        "gcp_secret_keys": {
+            "objectstore": {"key": "value"},
+        },
+        "config": {"setting": True},
+    }
+
+    # Group _values.yaml (single-tenant/_values.yaml)
+    group_values = {
+        "gcp_secret_keys": None,
+        "single_tenant": True,
+    }
+
+    # Region override (disney/default.yaml)
+    region_values = {
+        "gcp_secret_keys": None,
+        "region_specific": "disney",
+    }
+
+    # Simulate hierarchical merge (what get_hierarchical_value_overrides does)
+    hierarchical_base: dict[str, object] = {}
+    deep_merge_dict(hierarchical_base, group_values)
+    deep_merge_dict(hierarchical_base, region_values)
+
+    # hierarchical_base should still have gcp_secret_keys: None (not removed)
+    assert hierarchical_base == {
+        "gcp_secret_keys": None,
+        "single_tenant": True,
+        "region_specific": "disney",
+    }
+
+    # Now merge hierarchical into service (what _consolidate_variables does)
+    deep_merge_dict(service_values, hierarchical_base)
+
+    # service_values should have gcp_secret_keys: None
+    assert service_values == {
+        "gcp_secret_keys": None,
+        "config": {"setting": True},
+        "single_tenant": True,
+        "region_specific": "disney",
+    }
+
+    # Final cleanup
+    remove_none_values(service_values)
+
+    # gcp_secret_keys should be gone
+    assert service_values == {
+        "config": {"setting": True},
+        "single_tenant": True,
+        "region_specific": "disney",
+    }

libsentrykube/utils.py

@@ -361,12 +361,16 @@ def deep_merge_dict(
 
     overwrite: By default, if a key exists in both dicts, the value from `other` will overwrite the value in `into`.
     You can set `overwrite=False` if you want to retain the existing value in `into`.
+
+    None values in `other` mark keys for deletion. The key is set to None in the result,
+    allowing the deletion intent to propagate through multiple merge layers.
+    Use `remove_none_values()` on the final result to actually remove these keys.
     """
 
     for k, v in other.items():
         if v is None:
-            if k in into:
-                into.pop(k)
+            # Mark key for deletion by setting to None (preserves deletion intent through merges)
+            into[k] = None
         elif k in into and isinstance(v, dict) and isinstance(into[k], dict):
             deep_merge_dict(into=into[k], other=v, overwrite=overwrite)
         elif k in into:
@@ -376,6 +380,20 @@ def deep_merge_dict(
             into[k] = copy.deepcopy(v)
 
 
+def remove_none_values(d: dict[Any, Any]) -> None:
+    """
+    Recursively removes all keys with None values from a dict.
+    This should be called after all merges are complete to finalize deletions.
+    """
+    keys_to_remove = [k for k, v in d.items() if v is None]
+    for k in keys_to_remove:
+        del d[k]
+
+    for v in d.values():
+        if isinstance(v, dict):
+            remove_none_values(v)
+
+
 def macos_notify(title: str, text: str) -> None:
     if ENABLE_NOTIFICATIONS and platform.system() == "Darwin":
         cmd = """