fix: append to list or add new key with merge in loop #66 #107

tackle/parser.py

@@ -163,8 +163,13 @@ def evaluate_for(hook_dict: dict, Hook: ModelMetaclass, context: 'Context'):
     hook_dict.pop('for')
 
     # Need add an empty list in the value so we have something to append to
-    target_context, key_path = get_target_and_key(context)
-    nested_set(target_context, key_path, [])
+    if 'merge' not in hook_dict:
+        target_context, key_path = get_target_and_key(context)
+        nested_set(target_context, key_path, [])
+    elif not hook_dict['merge']:
+        target_context, key_path = get_target_and_key(context)
+        # If we are merging into a list / dict, we don't want init a list
+        nested_set(target_context, key_path, [])
 
     # Account for nested contexts and justify the new keys based on the key path within
     #  blocks by trimming the key_path_block from the key_path.
@@ -254,25 +259,34 @@ def merge_output(
     append_hook_value: bool = False,
 ):
     """Merge the contents into it's top level set of keys."""
-    if append_hook_value:
-        raise exceptions.AppendMergeException(
-            "Can't merge from for loop.", context=context
-        ) from None
-
     if context.key_path[-1] in ('->', '_>'):
         # Expanded key - Remove parent key from key path
         key_path = context.key_path[:-2] + [context.key_path[-1]]
     else:
         # Compact key
         key_path = context.key_path[:-1] + [context.key_path[-1][-2:]]
 
+    if append_hook_value:
+        if isinstance(key_path[-3], bytes):
+            set_key(context=context, value=hook_output_value, key_path=key_path[:-1])
+        elif isinstance(hook_output_value, (str, int, float, bool)):
+            raise exceptions.AppendMergeException(
+                "Can't merge str/int/float/bool into dict from for loop.",
+                context=context,
+            ) from None
+        else:
+            # TODO: This needs fixing for merging from loop into dict
+            #  https://github.com/robcxyz/tackle/issues/107
+            tmp_key_path = key_path[:-3] + [key_path[-2]]
+            set_key(context=context, value=hook_output_value, key_path=tmp_key_path)
+        return
+
     # Can't merge into top level keys without merging k/v individually
     if len(key_path) == 1:
         # This is only valid for dict output
         if isinstance(hook_output_value, (dict, OrderedDict)):
             for k, v in hook_output_value.items():
                 set_key(context=context, value=v, key_path=[k] + key_path)
-            return
         else:
             raise exceptions.TopLevelMergeException(
                 "Can't merge non maps into top level keys.", context=context
@@ -281,7 +295,6 @@ def merge_output(
         if isinstance(hook_output_value, dict):
             for k, v in hook_output_value.items():
                 set_key(context=context, value=v, key_path=key_path + [k])
-            return
         else:
             set_key(context=context, value=hook_output_value, key_path=key_path)
 
@@ -359,9 +372,7 @@ def render_hook_vars(hook_dict: dict, Hook: ModelMetaclass, context: 'Context'):
             hook_dict[key] = render_variable(context, value)
 
 
-def parse_sub_context(
-    context: 'Context', hook_dict: dict, target: str, append_hook_value: bool = None
-):
+def parse_sub_context(context: 'Context', hook_dict: dict, target: str):
     """
     Reparse a subcontext as in the case with `else` and `except` where you have to
     handle the negative side of the either `if` or `try`. Works on both looped and
@@ -373,14 +384,12 @@ def parse_sub_context(
         set_key(
             context=context,
             value=render_variable(context, hook_dict[target]),
-            append_hook_value=append_hook_value,
         )
         return
     elif isinstance(hook_target, (bool, int, float)):
         set_key(
             context=context,
             value=hook_target,
-            append_hook_value=append_hook_value,
         )
         return
 
@@ -518,7 +527,6 @@ def parse_hook(
                 set_key(
                     context=context,
                     value=hook_output_value,
-                    append_hook_value=append_hook_value,
                 )
 
     elif 'else' in hook_dict:

tackle/utils/dicts.py

@@ -216,7 +216,6 @@ def set_key(
     context: 'Context',
     value: Any,
     key_path: list = None,
-    append_hook_value: bool = False,
 ):
     """
     Wrap nested_set to set keys for both public and private hook calls.

tests/parser/fixtures/merge-dict-loop-dict.yaml

@@ -0,0 +1,4 @@
+resources:
+  name: operator
+  type: helm
+  do some merge->: literal {'foo-{{index}}':'foo-{{index}}'} --for [1,2,3] --merge

tests/parser/fixtures/merge-dict-loop-exception.yaml

@@ -0,0 +1,4 @@
+resources:
+  name: operator
+  type: helm
+  do some merge->: literal foo-{{index}} --for [1,2,3] --merge

tests/parser/fixtures/merge-list-loop.yaml

@@ -0,0 +1,6 @@
+resources:
+  - name: operator
+    type: helm
+  - name: Cluster role
+    type: manifest
+  - ->: literal foo-{{index}} --for [1,2,3] --merge

tests/parser/fixtures/merge-list-value.yaml

@@ -0,0 +1,6 @@
+resources:
+  - name: operator
+    type: helm
+  - name: Cluster role
+    type: manifest
+  - ->: literal foo --merge

tests/parser/test_parser_methods.py

@@ -1,8 +1,11 @@
+import pytest
 from ruamel.yaml import YAML
 from tackle import tackle
+from tackle import exceptions
 
 
 def test_parser_methods_merge(change_curdir_fixtures):
+    """Verify we can run tackle against a fixture and merge it up to equal the same."""
     yaml = YAML()
     with open('petstore.yaml') as f:
         expected_output = yaml.load(f)
@@ -11,6 +14,42 @@ def test_parser_methods_merge(change_curdir_fixtures):
     assert output == expected_output
 
 
+def test_parser_methods_merge_list_value(change_curdir_fixtures):
+    """Validate that when in a list, running a hook with a merge overwrites the list."""
+    # Note: this is kind of strong... But what else is it supposed to mean? Don't use
+    # merge for values...
+    output = tackle('merge-list-value.yaml')
+    assert output['resources'] == 'foo'
+
+
+def test_parser_methods_merge_list_loop(change_curdir_fixtures):
+    """
+    Validate that when in a list, running a hook in a for loop with a merge appends to
+     the list.
+    """
+    output = tackle('merge-list-loop.yaml')
+    assert len(output['resources']) == 5
+
+
+def test_parser_methods_merge_dict_loop_dict(change_curdir_fixtures):
+    """
+    Validate that when in a dict, running a hook in a for loop with a merge adds new
+     keys to the output.
+    """
+    # TODO: Associated with https://github.com/robcxyz/tackle/issues/107
+    output = tackle('merge-dict-loop-dict.yaml')
+    assert output['resources']['foo-2']
+
+
+def test_parser_methods_merge_dict_loop_exception(change_curdir_fixtures):
+    """
+    Validate exception that when in a dict, running a hook in a for loop with a merge
+     with the hook output being a value.
+    """
+    with pytest.raises(exceptions.AppendMergeException):
+        tackle('merge-dict-loop-exception.yaml')
+
+
 def test_parser_methods_try(chdir):
     """Use try which should not have any output"""
     chdir("method-fixtures")