Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow for changing the default merging strategy. #53

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,14 @@ The merging strategy can be set by including a dict in the form of:

__: <merging strategy>

If you want to use a different default merging strategy, you can do so in your
salt master config:

.. code:: yaml

pillarstack:
default_strategy: merge-first

as the first item of the dict or list.
This allows fine grained control over the merging process.

Expand Down
28 changes: 16 additions & 12 deletions stack.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,20 +36,21 @@ 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:
if not os.path.isfile(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)
'Ignoring pillar stack cfg "%s": file does not exist', cfg_path)
continue
stack = _process_stack_cfg(cfg, stack, minion_id, pillar)
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 _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])
Expand Down Expand Up @@ -85,7 +86,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


Expand All @@ -101,8 +102,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))
Expand All @@ -120,22 +121,25 @@ 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"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the purpose of the above lines?

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:
stack[k] = _cleanup(v)
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]
Expand Down
79 changes: 79 additions & 0 deletions test_stack.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()