From 80d6d15c4f0bb205f72a5e0470dafeadf2c18f75 Mon Sep 17 00:00:00 2001 From: "martin f. krafft" Date: Thu, 5 Mar 2015 20:37:23 +0100 Subject: [PATCH 1/2] Fix propagation of negations While _negations in the Applications type was being properly initialised, it wasn't copied on merges. This simple fix merely extends the list, which should work just fine i.e. I don't think it makes sense to ensure uniqueness here. Closes: https://github.com/madduck/reclass/issues/44 Signed-off-by: martin f. krafft --- reclass/datatypes/applications.py | 1 + 1 file changed, 1 insertion(+) diff --git a/reclass/datatypes/applications.py b/reclass/datatypes/applications.py index 4f6ee10b..c7f00e41 100644 --- a/reclass/datatypes/applications.py +++ b/reclass/datatypes/applications.py @@ -54,6 +54,7 @@ def merge_unique(self, iterable): self._items.remove(negation) except ValueError: pass + self._negations.extend(iterable._negations) iterable = iterable.as_list() for i in iterable: self.append_if_new(i) From 44065c890ded9d222bac0f3294a3f32120d516ee Mon Sep 17 00:00:00 2001 From: Simon Gerber Date: Tue, 26 Jul 2022 11:53:10 +0200 Subject: [PATCH 2/2] Don't insert previously negated items in `Applications.append_if_new()` We add logic in `Applications.append_if_new()` to only insert non-negated items if they aren't already on our internal negation list. Additionally, we drop items from the negation list, once we've applied the negation once either in `append_if_new()` or in `merge_unique()` so that patterns like adding, then removing and then adding an item again have the expected result of the item being present in the final list. This fixes the issue where it wasn't possible to preemptively remove an entry from the applications list in a multi-dimensional hierarchy, e.g. in a [Commodore] global defaults repository where we may want to exclude applications for a certain Kubernetes distribution regardless of the cloud on which a cluster with that distribution is running. [Commodore]: https://syn.tools/commodore --- reclass/datatypes/applications.py | 27 +++++++++++++++++++--- reclass/tests/data/07/classes/cls1.yml | 2 ++ reclass/tests/data/07/classes/cls2.yml | 2 ++ reclass/tests/data/07/classes/global.yml | 3 +++ reclass/tests/data/07/classes/override.yml | 2 ++ reclass/tests/data/07/nodes/A.yml | 10 ++++++++ reclass/tests/data/07/nodes/B.yml | 6 +++++ reclass/tests/data/07/nodes/C.yml | 10 ++++++++ reclass/tests/data/07/nodes/D.yml | 7 ++++++ reclass/tests/test_core.py | 13 +++++++++++ 10 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 reclass/tests/data/07/classes/cls1.yml create mode 100644 reclass/tests/data/07/classes/cls2.yml create mode 100644 reclass/tests/data/07/classes/global.yml create mode 100644 reclass/tests/data/07/classes/override.yml create mode 100644 reclass/tests/data/07/nodes/A.yml create mode 100644 reclass/tests/data/07/nodes/B.yml create mode 100644 reclass/tests/data/07/nodes/C.yml create mode 100644 reclass/tests/data/07/nodes/D.yml diff --git a/reclass/datatypes/applications.py b/reclass/datatypes/applications.py index c7f00e41..8ada6ded 100644 --- a/reclass/datatypes/applications.py +++ b/reclass/datatypes/applications.py @@ -23,6 +23,21 @@ class Applications(Classes): a reference of the negation is kept, in case the instance is later used to extend another instance, in which case the negations should apply to the instance to be extended. + + Negations are dropped after they've been applied once, so that patterns like + + ``` + applications: + - item + + applications: + - ~item + + applications: + - item + ``` + + result in `item` being present in the final list. ''' DEFAULT_NEGATION_PREFIX = '~' @@ -37,12 +52,17 @@ def append_if_new(self, item): self._assert_is_string(item) if item.startswith(self.negation_prefix): item = item[self._offset:] - self._negations.append(item) try: self._items.remove(item) except ValueError: - pass + # Only keep the negation if we couldn't remove the indicated item. + self._negations.append(item) + elif item in self._negations: + # Remove previously negated items from the negations list instead of + # inserting them into the list. + self._negations.remove(item) else: + # Insert non-negated items if they're not in our negations list already. super(Applications, self)._append_if_new(item) def merge_unique(self, iterable): @@ -53,8 +73,9 @@ def merge_unique(self, iterable): try: self._items.remove(negation) except ValueError: + # only remember negations which we didn't process during the merge + self._negations.append(negation) pass - self._negations.extend(iterable._negations) iterable = iterable.as_list() for i in iterable: self.append_if_new(i) diff --git a/reclass/tests/data/07/classes/cls1.yml b/reclass/tests/data/07/classes/cls1.yml new file mode 100644 index 00000000..bfd40770 --- /dev/null +++ b/reclass/tests/data/07/classes/cls1.yml @@ -0,0 +1,2 @@ +applications: + - ~foo diff --git a/reclass/tests/data/07/classes/cls2.yml b/reclass/tests/data/07/classes/cls2.yml new file mode 100644 index 00000000..834b6a1f --- /dev/null +++ b/reclass/tests/data/07/classes/cls2.yml @@ -0,0 +1,2 @@ +applications: + - foo diff --git a/reclass/tests/data/07/classes/global.yml b/reclass/tests/data/07/classes/global.yml new file mode 100644 index 00000000..6969f908 --- /dev/null +++ b/reclass/tests/data/07/classes/global.yml @@ -0,0 +1,3 @@ +classes: + - cls2 + - cls1 diff --git a/reclass/tests/data/07/classes/override.yml b/reclass/tests/data/07/classes/override.yml new file mode 100644 index 00000000..834b6a1f --- /dev/null +++ b/reclass/tests/data/07/classes/override.yml @@ -0,0 +1,2 @@ +applications: + - foo diff --git a/reclass/tests/data/07/nodes/A.yml b/reclass/tests/data/07/nodes/A.yml new file mode 100644 index 00000000..37c0274a --- /dev/null +++ b/reclass/tests/data/07/nodes/A.yml @@ -0,0 +1,10 @@ +classes: + - cls1 + - cls2 + +applications: + - foo + +parameters: + expected_apps: + - foo diff --git a/reclass/tests/data/07/nodes/B.yml b/reclass/tests/data/07/nodes/B.yml new file mode 100644 index 00000000..49a596cf --- /dev/null +++ b/reclass/tests/data/07/nodes/B.yml @@ -0,0 +1,6 @@ +classes: + - cls2 + - cls1 + +parameters: + expected_apps: [] diff --git a/reclass/tests/data/07/nodes/C.yml b/reclass/tests/data/07/nodes/C.yml new file mode 100644 index 00000000..9a0050b6 --- /dev/null +++ b/reclass/tests/data/07/nodes/C.yml @@ -0,0 +1,10 @@ +classes: + - cls2 + - cls1 + +applications: + - foo + +parameters: + expected_apps: + - foo diff --git a/reclass/tests/data/07/nodes/D.yml b/reclass/tests/data/07/nodes/D.yml new file mode 100644 index 00000000..cf089bff --- /dev/null +++ b/reclass/tests/data/07/nodes/D.yml @@ -0,0 +1,7 @@ +classes: + - global + - override + +parameters: + expected_apps: + - foo diff --git a/reclass/tests/test_core.py b/reclass/tests/test_core.py index dd51f3a0..eca29c04 100644 --- a/reclass/tests/test_core.py +++ b/reclass/tests/test_core.py @@ -123,5 +123,18 @@ def test_application_removal(self): self.assertEqual(A_node['applications'], A_node['parameters']['expected_apps']) self.assertEqual(B_node['applications'], B_node['parameters']['expected_apps']) + def test_application_removal_before_insertion(self): + reclass = self._core('07') + + A_node = reclass.nodeinfo('A') + B_node = reclass.nodeinfo('B') + C_node = reclass.nodeinfo('C') + D_node = reclass.nodeinfo('D') + + self.assertEqual(A_node['applications'], A_node['parameters']['expected_apps']) + self.assertEqual(B_node['applications'], B_node['parameters']['expected_apps']) + self.assertEqual(C_node['applications'], C_node['parameters']['expected_apps']) + self.assertEqual(D_node['applications'], D_node['parameters']['expected_apps']) + if __name__ == '__main__': unittest.main()