Skip to content

Commit

Permalink
Don't insert previously negated items in Applications.append_if_new()
Browse files Browse the repository at this point in the history
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
  • Loading branch information
simu committed Oct 18, 2022
1 parent 80d6d15 commit 44065c8
Show file tree
Hide file tree
Showing 10 changed files with 79 additions and 3 deletions.
27 changes: 24 additions & 3 deletions reclass/datatypes/applications.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = '~'

Expand All @@ -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):
Expand All @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions reclass/tests/data/07/classes/cls1.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
applications:
- ~foo
2 changes: 2 additions & 0 deletions reclass/tests/data/07/classes/cls2.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
applications:
- foo
3 changes: 3 additions & 0 deletions reclass/tests/data/07/classes/global.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
classes:
- cls2
- cls1
2 changes: 2 additions & 0 deletions reclass/tests/data/07/classes/override.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
applications:
- foo
10 changes: 10 additions & 0 deletions reclass/tests/data/07/nodes/A.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
classes:
- cls1
- cls2

applications:
- foo

parameters:
expected_apps:
- foo
6 changes: 6 additions & 0 deletions reclass/tests/data/07/nodes/B.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
classes:
- cls2
- cls1

parameters:
expected_apps: []
10 changes: 10 additions & 0 deletions reclass/tests/data/07/nodes/C.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
classes:
- cls2
- cls1

applications:
- foo

parameters:
expected_apps:
- foo
7 changes: 7 additions & 0 deletions reclass/tests/data/07/nodes/D.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
classes:
- global
- override

parameters:
expected_apps:
- foo
13 changes: 13 additions & 0 deletions reclass/tests/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

0 comments on commit 44065c8

Please sign in to comment.