From 64480bc0b11871bf4fd3972949f7e3ada1021101 Mon Sep 17 00:00:00 2001 From: robcxyz Date: Wed, 30 Nov 2022 20:21:58 +0530 Subject: [PATCH] fix: append to list or add new key with merge in loop #66 #107 --- tackle/parser.py | 38 +++++++++++------- tackle/utils/dicts.py | 1 - .../parser/fixtures/merge-dict-loop-dict.yaml | 4 ++ .../fixtures/merge-dict-loop-exception.yaml | 4 ++ tests/parser/fixtures/merge-list-loop.yaml | 6 +++ tests/parser/fixtures/merge-list-value.yaml | 6 +++ tests/parser/test_parser_methods.py | 39 +++++++++++++++++++ 7 files changed, 82 insertions(+), 16 deletions(-) create mode 100644 tests/parser/fixtures/merge-dict-loop-dict.yaml create mode 100644 tests/parser/fixtures/merge-dict-loop-exception.yaml create mode 100644 tests/parser/fixtures/merge-list-loop.yaml create mode 100644 tests/parser/fixtures/merge-list-value.yaml diff --git a/tackle/parser.py b/tackle/parser.py index 0774125c8..518437e69 100644 --- a/tackle/parser.py +++ b/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,11 +259,6 @@ 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]] @@ -266,13 +266,27 @@ def merge_output( # 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: diff --git a/tackle/utils/dicts.py b/tackle/utils/dicts.py index 8cca6b78a..6ac455487 100644 --- a/tackle/utils/dicts.py +++ b/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. diff --git a/tests/parser/fixtures/merge-dict-loop-dict.yaml b/tests/parser/fixtures/merge-dict-loop-dict.yaml new file mode 100644 index 000000000..fbf653cf9 --- /dev/null +++ b/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 diff --git a/tests/parser/fixtures/merge-dict-loop-exception.yaml b/tests/parser/fixtures/merge-dict-loop-exception.yaml new file mode 100644 index 000000000..88a3b50be --- /dev/null +++ b/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 diff --git a/tests/parser/fixtures/merge-list-loop.yaml b/tests/parser/fixtures/merge-list-loop.yaml new file mode 100644 index 000000000..e239576d3 --- /dev/null +++ b/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 diff --git a/tests/parser/fixtures/merge-list-value.yaml b/tests/parser/fixtures/merge-list-value.yaml new file mode 100644 index 000000000..38493a67a --- /dev/null +++ b/tests/parser/fixtures/merge-list-value.yaml @@ -0,0 +1,6 @@ +resources: + - name: operator + type: helm + - name: Cluster role + type: manifest + - ->: literal foo --merge diff --git a/tests/parser/test_parser_methods.py b/tests/parser/test_parser_methods.py index 8a2f4ab93..7ef8f6d96 100644 --- a/tests/parser/test_parser_methods.py +++ b/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")