Improved refactor of hide_fields to better cope

Needed to cope with multiple levels of bracketed keys e.g. spec.template.spec[securityContext][runAsUser] and implementing that lead to a much simpler version that finds the first key and the rest, and deals with just those two. Added more tests to cover some of the new supported cases.
ansible-collections · Sep 5, 2023 · 22f928e · 22f928e
1 parent 80d9316
commit 22f928e
Show file tree

Hide file tree

Showing 2 changed files with 206 additions and 95 deletions.
diff --git a/plugins/module_utils/k8s/service.py b/plugins/module_utils/k8s/service.py
@@ -544,9 +544,6 @@ def hide_fields(definition: dict, hidden_fields: Optional[list]) -> dict:
 # spec.template.spec.containers[0].env[3].value or
 # metadata.annotations[kubectl.kubernetes.io/last-applied-configuration]
 def hide_field(definition: dict, hidden_field: str) -> dict:
-    lbracket = hidden_field.find("[")
-    dot = hidden_field.find(".")
-
     def dict_contains_key(field: dict, key: str) -> bool:
         return key in field
 
@@ -555,30 +552,48 @@ def list_contains_key(field: list, key: str) -> bool:
 
     field_contains_key = dict_contains_key
 
-    if lbracket != -1 and (dot == -1 or lbracket < dot):
-        # handle lists and dicts
-        rbracket = hidden_field.find("]")
-        key = hidden_field[lbracket + 1:rbracket]
-        field = hidden_field[:lbracket]
-        # skip past right bracket and any following dot
-        rest = hidden_field[rbracket + 2:]
-
-        if key.isdecimal():
-            key = int(key)
-            field_contains_key = list_contains_key
-        if field in definition and field_contains_key(definition[field], key):
-            if rest:
-                definition[field][key] = hide_field(definition[field][key], rest)
-            else:
-                del definition[field][key]
-                if not definition[field]:
-                    del definition[field]
-    else:
-        # handle standard fields
-        split = hidden_field.split(".", 1)
-        if split[0] in definition:
-            if len(split) == 2:
-                definition[split[0]] = hide_field(definition[split[0]], split[1])
-            else:
-                del definition[split[0]]
+    (key, rest) = hide_field_split2(hidden_field)
+
+    if key.isdecimal():
+        key = int(key)
+        field_contains_key = list_contains_key
+    if field_contains_key(definition, key):
+        if rest:
+            definition[key] = hide_field(definition[key], rest)
+            # remove empty dicts and lists from the result
+            if definition[key] == dict() or definition[key] == list():
+                del definition[key]
+        else:
+            del definition[key]
     return definition
+
+
+# hide_field_split2 returns the first key in hidden_field and the rest of the hidden_field
+# We expect the first key to either be in brackets, to be terminated by the start of a left
+# bracket, or to be terminated by a dot.
+
+# examples would be:
+# field.another.next -> (field, another.next)
+# field[key].value -> (field, [key].value)
+# [key].value -> (key, value)
+# [one][two] -> (one, [two])
+
+def hide_field_split2(hidden_field: str) -> (str, str):
+    lbracket = hidden_field.find("[")
+    rbracket = hidden_field.find("]")
+    dot = hidden_field.find(".")
+
+    if lbracket == 0:
+        # skip past right bracket and any following dot
+        rest = hidden_field[rbracket + 1:]
+        if rest and rest[0] == '.':
+            rest = rest[1:]
+        return (hidden_field[lbracket + 1:rbracket], rest)
+
+    if lbracket != -1 and (dot == -1 or lbracket < dot):
+        return (hidden_field[:lbracket], hidden_field[lbracket:])
+
+    split = hidden_field.split(".", 1)
+    if len(split) == 1:
+        return split[0], ""
+    return split
diff --git a/tests/unit/module_utils/test_hide_fields.py b/tests/unit/module_utils/test_hide_fields.py
@@ -2,75 +2,171 @@
     hide_fields,
 )
 
-tests = [
-    dict(
-        output=dict(
-            kind="ConfigMap", metadata=dict(name="foo"), data=dict(one="1", two="2")
+
+def test_hiding_missing_field_does_nothing():
+    output=dict(
+        kind="ConfigMap", metadata=dict(name="foo"), data=dict(one="1", two="2")
+    )
+    hidden_fields=["doesnotexist"]
+    assert(hide_fields(output, hidden_fields) == output)
+
+
+def test_hiding_simple_field():
+    output=dict(
+        kind="ConfigMap", metadata=dict(name="foo"), data=dict(one="1", two="2")
+    )
+    hidden_fields=["metadata"]
+    expected=dict(kind="ConfigMap", data=dict(one="1", two="2"))
+    assert(hide_fields(output, hidden_fields) == expected)
+
+
+def test_hiding_only_key_in_dict_removes_dict():
+    output=dict(
+        kind="ConfigMap", metadata=dict(name="foo"), data=dict(one="1")
+    )
+    hidden_fields=["data.one"]
+    expected=dict(kind="ConfigMap", metadata=dict(name="foo"))
+    assert(hide_fields(output, hidden_fields) == expected)
+
+
+def test_hiding_all_keys_in_dict_removes_dict():
+    output=dict(
+        kind="ConfigMap", metadata=dict(name="foo"), data=dict(one="1", two="2")
+    )
+    hidden_fields=["data.one", "data.two"]
+    expected=dict(kind="ConfigMap", metadata=dict(name="foo"))
+    assert(hide_fields(output, hidden_fields) == expected)
+
+
+def test_hiding_multiple_fields():
+    output=dict(
+        kind="ConfigMap", metadata=dict(name="foo"), data=dict(one="1", two="2")
+    )
+    hidden_fields=["metadata", "data.one"]
+    expected=dict(kind="ConfigMap", data=dict(two="2"))
+    assert(hide_fields(output, hidden_fields) == expected)
+
+
+def test_hiding_dict_key():
+    output=dict(
+        kind="ConfigMap",
+        metadata=dict(
+            name="foo",
+            annotations={
+                "kubectl.kubernetes.io/last-applied-configuration": '{"testvalue"}'
+            },
         ),
-        hide_fields=["metadata"],
-        expected=dict(kind="ConfigMap", data=dict(one="1", two="2")),
-    ),
-    dict(
-        output=dict(
-            kind="ConfigMap",
-            metadata=dict(
-                name="foo",
-                annotations={
-                    "kubectl.kubernetes.io/last-applied-configuration": '{"testvalue"}'
-                },
-            ),
-            data=dict(one="1", two="2"),
+        data=dict(one="1", two="2"),
+    )
+    hidden_fields=[
+        "metadata.annotations[kubectl.kubernetes.io/last-applied-configuration]",
+    ]
+    expected=dict(kind="ConfigMap", metadata=dict(name="foo"), data=dict(one="1", two="2"))
+    assert(hide_fields(output, hidden_fields) == expected)
+
+
+def test_hiding_list_value_key():
+    output=dict(
+        kind="Pod",
+        metadata=dict(name="foo"),
+        spec=dict(
+            containers=[
+                dict(
+                    name="containers",
+                    image="busybox",
+                    env=[
+                        dict(name="ENV1", value="env1"),
+                        dict(name="ENV2", value="env2"),
+                        dict(name="ENV3", value="env3"),
+                    ],
+                )
+            ]
         ),
-        hide_fields=[
-            "metadata.annotations[kubectl.kubernetes.io/last-applied-configuration]",
-            "data.one",
-        ],
-        expected=dict(kind="ConfigMap", metadata=dict(name="foo"), data=dict(two="2")),
-    ),
-    dict(
-        output=dict(
-            kind="Pod",
-            metadata=dict(name="foo"),
-            spec=dict(
-                containers=[
-                    dict(
-                        name="containers",
-                        image="busybox",
-                        env=[
-                            dict(name="ENV1", value="env1"),
-                            dict(name="ENV2", value="env2"),
-                            dict(name="ENV3", value="env3"),
-                        ],
-                    )
-                ]
-            ),
+    )
+    hidden_fields=["spec.containers[0].env[1].value"]
+    expected=dict(
+        kind="Pod",
+        metadata=dict(name="foo"),
+        spec=dict(
+            containers=[
+                dict(
+                    name="containers",
+                    image="busybox",
+                    env=[
+                        dict(name="ENV1", value="env1"),
+                        dict(name="ENV2"),
+                        dict(name="ENV3", value="env3"),
+                    ],
+                )
+            ]
         ),
-        hide_fields=["spec.containers[0].env[1].value"],
-        expected=dict(
-            kind="Pod",
-            metadata=dict(name="foo"),
-            spec=dict(
-                containers=[
-                    dict(
-                        name="containers",
-                        image="busybox",
-                        env=[
-                            dict(name="ENV1", value="env1"),
-                            dict(name="ENV2"),
-                            dict(name="ENV3", value="env3"),
-                        ],
-                    )
-                ]
-            ),
+    )
+    assert(hide_fields(output, hidden_fields) == expected)
+
+def test_hiding_last_list_item():
+    output=dict(
+        kind="Pod",
+        metadata=dict(name="foo"),
+        spec=dict(
+            containers=[
+                dict(
+                    name="containers",
+                    image="busybox",
+                    env=[
+                        dict(name="ENV1", value="env1"),
+                    ],
+                )
+            ]
+        ),
+    )
+    hidden_fields=["spec.containers[0].env[0]"]
+    expected=dict(
+        kind="Pod",
+        metadata=dict(name="foo"),
+        spec=dict(
+            containers=[
+                dict(
+                    name="containers",
+                    image="busybox",
+                )
+            ]
         ),
-    ),
-]
+    )
+    assert(hide_fields(output, hidden_fields) == expected)
 
 
-def test_hide_fields():
-    for test in tests:
-        if hide_fields(test["output"], test["hide_fields"]) != test["expected"]:
-            print(test["output"])
-            print(hide_fields(test["output"], test["hide_fields"]))
-            print(test["expected"])
-        assert hide_fields(test["output"], test["hide_fields"]) == test["expected"]
+
+
+def test_hiding_nested_dicts_using_brackets():
+    output=dict(
+        kind="Pod",
+        metadata=dict(name="foo"),
+        spec=dict(
+            containers=[
+                dict(
+                    name="containers",
+                    image="busybox",
+                    securityContext=dict(
+                        runAsUser=101
+                    )
+                )
+            ]
+        ),
+    )
+    hidden_fields=["spec.containers[0][securityContext][runAsUser]"]
+    expected=dict(
+        kind="Pod",
+        metadata=dict(name="foo"),
+        spec=dict(
+            containers=[
+                dict(
+                    name="containers",
+                    image="busybox",
+                )
+            ]
+        ),
+    )
+    if hide_fields(output, hidden_fields) != expected:
+        print(output)
+        print(expected)
+    assert(hide_fields(output, hidden_fields) == expected)