fix: secret masking not working as expected

davidgiga1993 · Oct 7, 2024 · e65d610 · e65d610
1 parent a3e68fc
commit e65d610
Show file tree

Hide file tree

Showing 2 changed files with 135 additions and 84 deletions.
diff --git a/octoploy/k8s/K8sObjectDiff.py b/octoploy/k8s/K8sObjectDiff.py
@@ -1,12 +1,31 @@
 import difflib
-from typing import Dict, List, Iterator, Set
+from typing import Dict, List, Iterator
 
 from octoploy.api.Kubectl import K8sApi
 from octoploy.k8s.BaseObj import BaseObj
 from octoploy.utils.Log import ColorFormatter
 from octoploy.utils.YmlWriter import YmlWriter
 
 
+class ValueMask:
+    fields: List[List[str]] = []
+    """
+    Holds a list of context paths that should be masked
+    """
+
+    def __init__(self):
+        self.fields = []
+
+    def should_mask_value(self, context: List[str]) -> bool:
+        for mask_path in self.fields:
+            if len(mask_path) > len(context):
+                continue
+            # Check if the context path starts with the mask path
+            if context[:len(mask_path)] == mask_path:
+                return True
+        return False
+
+
 class K8sObjectDiff:
     """
     Creates nice to look at diffs of two yml files.
@@ -22,20 +41,19 @@ def print(self, current: BaseObj, new: BaseObj):
         :param current: The current object in the cluster
         :param new: The new object
         """
-        mask_values: List[List[str]] = []
+        mask = ValueMask()
         if current.is_kind('secret') or new.is_kind('secret'):
-            mask_values.append(['spec', 'data'])
-            mask_values.append(['spec', 'stringData'])
-
+            mask.fields.append(['data'])
+            mask.fields.append(['stringData'])
         current_data = self._filter_injected(current.data)
 
         # Server side dry-run to get the same format / list sorting
         new = self._api.dry_run(YmlWriter.dump(new.data))
         new_data = self._filter_injected(new.data)
-        self._print_diff(current_data, new_data, [], mask_values)
+        self._print_diff(current_data, new_data, [], mask)
 
     def _print_diff(self, current_data: Dict[str, any], new_data: Dict[str, any],
-                    context: List[str], mask_values: List[List[str]]):
+                    context: List[str], value_mask: ValueMask):
         if current_data is None:
             current_data = {}
         if new_data is None:
@@ -47,11 +65,10 @@ def _print_diff(self, current_data: Dict[str, any], new_data: Dict[str, any],
         for key in all_keys:
             current_entry = current_data.get(key)
             new_entry = new_data.get(key)
-            self._print_value_diff(current_entry, new_entry, context + [key], mask_values)
-
-    def _print_value_diff(self, current_entry, new_entry, context: List[str], mask_values: List[List[str]]):
-        mask_value: bool = context in mask_values
+            self._print_value_diff(current_entry, new_entry, context + [key], value_mask)
 
+    def _print_value_diff(self, current_entry, new_entry, context: List[str], value_mask: ValueMask):
+        mask_value: bool = value_mask.should_mask_value(context)
         if isinstance(current_entry, list) or isinstance(new_entry, list):
             if current_entry is None:
                 current_entry = []
@@ -66,11 +83,11 @@ def _print_value_diff(self, current_entry, new_entry, context: List[str], mask_v
                     current_val = current_entry[i]
                 if i < len(new_entry):
                     new_val = new_entry[i]
-                self._print_value_diff(current_val, new_val, context + [f'[{i}]'], mask_values)
+                self._print_value_diff(current_val, new_val, context + [f'[{i}]'], value_mask)
             return
 
         if isinstance(current_entry, dict) or isinstance(new_entry, dict):
-            self._print_diff(current_entry, new_entry, context, mask_values)
+            self._print_diff(current_entry, new_entry, context, value_mask)
             return
         if current_entry == new_entry:
             return

diff --git a/tests/K8sObjectDiffTest.py b/tests/K8sObjectDiffTest.py
@@ -15,97 +15,131 @@ def test_secret_masking(self, mock_print):
         a = BaseObj({
             'kind': 'Secret',
             'apiVersion': 'v1',
-            'spec': {
-                'data': '1',
+            'data': {
+                'a': '1'
             }
         })
         b = BaseObj({
             'kind': 'Secret',
             'apiVersion': 'v1',
-            'spec': {
-                'data': '2',
-                'stringData': 'newField',
+            'data': {
+                'a': '2'
+            },
+            'stringData': {
+                'b': 'newField'
             }
         })
 
         K8sObjectDiff(api).print(a, b)
 
         stdout_lines = ''
         for args in mock_print.call_args_list:
-            stdout_lines += ColorFormatter.decolorize(str(args.args[0]))+'\n'
+            stdout_lines += ColorFormatter.decolorize(str(args.args[0])) + '\n'
 
         self.assertNotIn('1', stdout_lines)
         self.assertNotIn('2', stdout_lines)
-        self.assertIn('data', stdout_lines)
-        self.assertIn('stringData', stdout_lines)
+        self.assertIn('~ data.a = *** -> ***', stdout_lines)
+        self.assertIn('+ stringData.b = ***', stdout_lines)
         self.assertNotIn('newField', stdout_lines)
 
+    @patch('builtins.print')
+    def test_list_diff(self, mock_print):
+        api = DummyK8sApi()
+        a = BaseObj({
+            'kind': 'ConfingMap',
+            'apiVersion': 'v1',
+            'spec': {
+                'add': ['a', 'b'],
+                'change': ['a', 'b', 'c'],
+                'remove': ['a', 'b', 'c'],
+            }
+        })
+        b = BaseObj({
+            'kind': 'ConfingMap',
+            'apiVersion': 'v1',
+            'spec': {
+                'add': ['a', 'b', 'c'],
+                'change': ['a', 'c', 'd'],
+                'remove': ['a', 'c'],
+            }
+        })
 
-def test_diff(self):
-    api = DummyK8sApi()
-    a = BaseObj({
-        'kind': 'a',
-        'apiVersion': 'v1',
-        'metadata': {
-            'annotations': {
-                'kubectl.kubernetes.io/last-applied-configuration': 'ignore',
+        K8sObjectDiff(api).print(a, b)
+
+        stdout_lines = ''
+        for args in mock_print.call_args_list:
+            stdout_lines += ColorFormatter.decolorize(str(args.args[0])) + '\n'
+
+        self.assertIn('+ spec.add.[2] = c', stdout_lines)
+        self.assertIn('~ spec.change.[1] = b -> c', stdout_lines)
+        self.assertIn('~ spec.change.[2] = c -> d', stdout_lines)
+        self.assertIn('~ spec.remove.[1] = b -> c', stdout_lines)
+        self.assertIn('- spec.remove.[2] = c', stdout_lines)
+
+    def test_diff(self):
+        api = DummyK8sApi()
+        a = BaseObj({
+            'kind': 'a',
+            'apiVersion': 'v1',
+            'metadata': {
+                'annotations': {
+                    'kubectl.kubernetes.io/last-applied-configuration': 'ignore',
+                },
+                'uid': '',
+                'resourceVersion': '123',
             },
-            'uid': '',
-            'resourceVersion': '123',
-        },
-        'spec': {
-            'field': 'value',
-            'removed': 'value',
-            'listItemAdd': ['a'],
-            'listRemoved': ['a', 'b'],
-            'listItemRemoved': ['a', 'b'],
-            'listItemChanged': ['a', 'b'],
-            'removedDict': {'a': 'hello'}
-        }
-    })
-    b = BaseObj({
-        'kind': 'a',
-        'apiVersion': 'v1',
-        'metadata': {
-            'annotations': {
-                'newAnnotation': 'hello',
+            'spec': {
+                'field': 'value',
+                'removed': 'value',
+                'listItemAdd': ['a'],
+                'listRemoved': ['a', 'b'],
+                'listItemRemoved': ['a', 'b'],
+                'listItemChanged': ['a', 'b'],
+                'removedDict': {'a': 'hello'}
+            }
+        })
+        b = BaseObj({
+            'kind': 'a',
+            'apiVersion': 'v1',
+            'metadata': {
+                'annotations': {
+                    'newAnnotation': 'hello',
+                },
             },
-        },
-        'spec': {
-            'field': 'otherValue',
-            'listItemAdd': ['a', 'b'],
-            'listAdded': ['a', 'b'],
-            'listItemRemoved': ['a'],
-            'listItemChanged': ['a', 'other'],
-            'addedDict': {'a': 'hello'}
-        }
-    })
-
-    K8sObjectDiff(api).print(a, b)
+            'spec': {
+                'field': 'otherValue',
+                'listItemAdd': ['a', 'b'],
+                'listAdded': ['a', 'b'],
+                'listItemRemoved': ['a'],
+                'listItemChanged': ['a', 'other'],
+                'addedDict': {'a': 'hello'}
+            }
+        })
 
+        K8sObjectDiff(api).print(a, b)
 
-def test_multiline_str(self):
-    api = DummyK8sApi()
-    a = BaseObj({
-        'kind': 'a',
-        'apiVersion': 'v1',
-        'spec': {
-            'field': '''
-                This is a multiline string
-                It has multiple lines
-                Yay''',
-        }
-    })
-    b = BaseObj({
-        'kind': 'a',
-        'apiVersion': 'v1',
-        'spec': {
-            'field': '''
-                This is a multiline string
-                It has multiple lines, but some have been changed
-                or have been added
-                Yay''',
-        }
-    })
+    def test_multiline_str(self):
+        api = DummyK8sApi()
+        a = BaseObj({
+            'kind': 'a',
+            'apiVersion': 'v1',
+            'spec': {
+                'field': '''
+                    This is a multiline string
+                    It has multiple lines
+                    Yay''',
+            }
+        })
+        b = BaseObj({
+            'kind': 'a',
+            'apiVersion': 'v1',
+            'spec': {
+                'field': '''
+                    This is a multiline string
+                    It has multiple lines, but some have been changed
+                    or have been added
+                    Yay''',
+            }
+        })
 
-    K8sObjectDiff(api).print(a, b)
+        K8sObjectDiff(api).print(a, b)