From 3c14fc963bdd9044d1f5f1395e78e8451870b15a Mon Sep 17 00:00:00 2001 From: Michael Mulqueen Date: Wed, 8 May 2019 16:19:28 +0100 Subject: [PATCH 1/2] Allow for changing the default merging strategy. --- README.rst | 8 ++++++ stack.py | 39 ++++++++++++++++++------- test_stack.py | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 11 deletions(-) diff --git a/README.rst b/README.rst index 5fd8007..3e72479 100644 --- a/README.rst +++ b/README.rst @@ -231,6 +231,14 @@ The merging strategy can be set by including a dict in the form of: __: +If you want to use a different default merging strategy, you can do so in your +salt master config by adding a parameter: + +.. code:: yaml + + ext_pillar: + - stack: /path/to/stack.cfg?default_strategy=merge-first + as the first item of the dict or list. This allows fine grained control over the merging process. diff --git a/stack.py b/stack.py index 314e002..1ddb49f 100644 --- a/stack.py +++ b/stack.py @@ -37,19 +37,33 @@ def ext_pillar(minion_id, pillar, *args, **kwargs): cfgs = [cfgs] stack_config_files += cfgs for cfg in stack_config_files: - if not os.path.isfile(cfg): + cfg_path, params = _extract_cfg(cfg) + if not os.path.isfile(cfg_path): log.info( - 'Ignoring pillar stack cfg "%s": file does not exist', cfg) + 'Ignoring pillar stack cfg "%s": file does not exist', cfg_path) continue - stack = _process_stack_cfg(cfg, stack, minion_id, pillar) + default_strategy = params.get("default_strategy", "merge-last") + stack = _process_stack_cfg(cfg_path, stack, minion_id, pillar, default_strategy) return stack +def _extract_cfg(cfg): + # Get the path of the config and any parameters. + if "?" not in cfg: + return cfg, {} + path, raw_params = cfg.split("?", 1) + params = {} + for param in raw_params.split("&"): + key, val = param.split("=") + params[key] = val + return path, params + + def _to_unix_slashes(path): return posixpath.join(*path.split(os.sep)) -def _process_stack_cfg(cfg, stack, minion_id, pillar): +def _process_stack_cfg(cfg, stack, minion_id, pillar, default_strategy): log.debug('Config: %s', cfg) basedir, filename = os.path.split(cfg) jenv = Environment(loader=FileSystemLoader(basedir), extensions=['jinja2.ext.do', salt.utils.jinja.SerializerExtension]) @@ -85,7 +99,7 @@ def _process_stack_cfg(cfg, stack, minion_id, pillar): log.info('Ignoring pillar stack template "%s": Can\'t parse ' 'as a valid yaml dictionary', path) continue - stack = _merge_dict(stack, obj) + stack = _merge_dict(stack, obj, default_strategy) return stack @@ -101,8 +115,8 @@ def _cleanup(obj): return obj -def _merge_dict(stack, obj): - strategy = obj.pop('__', 'merge-last') +def _merge_dict(stack, obj, default_strategy="merge-last"): + strategy = obj.pop('__', default_strategy) if strategy not in strategies: raise Exception('Unknown strategy "{0}", should be one of {1}'.format( strategy, strategies)) @@ -120,13 +134,16 @@ def _merge_dict(stack, obj): stack_k = stack[k] stack[k] = _cleanup(v) v = stack_k + # Stop it getting double inverted later on. + if default_strategy == "merge-first": + default_strategy = "merge-last" if type(stack[k]) != type(v): log.debug('Force overwrite, types differ: \'%s\' != \'%s\'', stack[k], v) stack[k] = _cleanup(v) elif isinstance(v, dict): - stack[k] = _merge_dict(stack[k], v) + stack[k] = _merge_dict(stack[k], v, default_strategy) elif isinstance(v, list): - stack[k] = _merge_list(stack[k], v) + stack[k] = _merge_list(stack[k], v, default_strategy) else: stack[k] = v else: @@ -134,8 +151,8 @@ def _merge_dict(stack, obj): return stack -def _merge_list(stack, obj): - strategy = 'merge-last' +def _merge_list(stack, obj, default_strategy="merge-last"): + strategy = default_strategy if obj and isinstance(obj[0], dict) and '__' in obj[0]: strategy = obj[0]['__'] del obj[0] diff --git a/test_stack.py b/test_stack.py index d2d5368..fbe7626 100644 --- a/test_stack.py +++ b/test_stack.py @@ -39,6 +39,85 @@ def test_merge(self): 'INSTEAD OF =>', after_yml, ])) + def test_merge_first_explicit_top_level(self): + # This checks __ behaviour. + cur_stack = { + "flat": "First value", + "nested": { + "val": "First value nested", + "nested_again": { + "val": 123 + } + } + } + obj = { + "__": "merge-first", + "flat": "Shouldn't overwrite", + "flat_newval": "OK", + "nested": { + "val": "Shouldn't overwrite", + "newval": "OK", + "nested_again": { + "val": 456, + "newval": 678 + } + } + } + expected = { + "flat": "First value", + "flat_newval": "OK", + "nested": { + "val": "First value nested", + "newval": "OK", + "nested_again": { + "val": 123, + "newval": 678 + } + } + } + new_stack = stack._merge_dict(cur_stack, obj) + self.assertDictEqual(new_stack, expected) + + def test_merge_first_with_default(self): + # This makes sure that we get the same effect as + # test_merge_first_explicit_top_level, but using + # default_strategy instead. + cur_stack = { + "flat": "First value", + "nested": { + "val": "First value nested", + "nested_again": { + "val": 123 + } + } + } + obj = { + "flat": "Shouldn't overwrite", + "flat_newval": "OK", + "nested": { + "val": "Shouldn't overwrite", + "newval": "OK", + "nested_again": { + "val": 456, + "newval": 678 + } + } + } + expected = { + "flat": "First value", + "flat_newval": "OK", + "nested": { + "val": "First value nested", + "newval": "OK", + "nested_again": { + "val": 123, + "newval": 678 + } + } + } + new_stack = stack._merge_dict(cur_stack, obj, default_strategy="merge-first") + self.assertDictEqual(new_stack, expected) + if __name__ == '__main__': unittest.main() From 4ea2bf6757b2990bcd7d6ae30edfb2e9eb063dcd Mon Sep 17 00:00:00 2001 From: Michael Mulqueen Date: Fri, 21 Jun 2019 16:25:39 +0100 Subject: [PATCH 2/2] Configure with __opts__ based on feedback on PR. https://github.com/bbinet/pillarstack/pull/53 --- README.rst | 6 +++--- stack.py | 17 ++--------------- 2 files changed, 5 insertions(+), 18 deletions(-) diff --git a/README.rst b/README.rst index 3e72479..ffd6e2b 100644 --- a/README.rst +++ b/README.rst @@ -232,12 +232,12 @@ The merging strategy can be set by including a dict in the form of: __: If you want to use a different default merging strategy, you can do so in your -salt master config by adding a parameter: +salt master config: .. code:: yaml - ext_pillar: - - stack: /path/to/stack.cfg?default_strategy=merge-first + pillarstack: + default_strategy: merge-first as the first item of the dict or list. This allows fine grained control over the merging process. diff --git a/stack.py b/stack.py index 1ddb49f..72ad821 100644 --- a/stack.py +++ b/stack.py @@ -36,29 +36,16 @@ def ext_pillar(minion_id, pillar, *args, **kwargs): if not isinstance(cfgs, list): cfgs = [cfgs] stack_config_files += cfgs - for cfg in stack_config_files: - cfg_path, params = _extract_cfg(cfg) + for cfg_path in stack_config_files: if not os.path.isfile(cfg_path): log.info( 'Ignoring pillar stack cfg "%s": file does not exist', cfg_path) continue - default_strategy = params.get("default_strategy", "merge-last") + default_strategy = __opts__.get("pillarstack", {}).get("default_strategy", "merge-last") stack = _process_stack_cfg(cfg_path, stack, minion_id, pillar, default_strategy) return stack -def _extract_cfg(cfg): - # Get the path of the config and any parameters. - if "?" not in cfg: - return cfg, {} - path, raw_params = cfg.split("?", 1) - params = {} - for param in raw_params.split("&"): - key, val = param.split("=") - params[key] = val - return path, params - - def _to_unix_slashes(path): return posixpath.join(*path.split(os.sep))