From 41353b7ab3c1877f1b8802ed0c4efa317215a02e Mon Sep 17 00:00:00 2001 From: Sean Jenkins Date: Tue, 29 Dec 2015 10:22:54 -0700 Subject: [PATCH] Remove recurse_list from pillar_source_merging_strategy and add pillar_merge_list (bool) instead --- conf/master | 3 ++ doc/ref/configuration/master.rst | 44 ++++++++------------- salt/auth/__init__.py | 4 +- salt/config/__init__.py | 4 ++ salt/modules/config.py | 9 +++-- salt/pillar/__init__.py | 12 ++++-- salt/pillar/git_pillar.py | 7 +++- salt/utils/dictupdate.py | 16 ++++---- tests/unit/pillar_test.py | 59 ----------------------------- tests/unit/utils/dictupdate_test.py | 8 ++-- 10 files changed, 57 insertions(+), 109 deletions(-) diff --git a/conf/master b/conf/master index f355c9fcab1b..d831361e7f2e 100644 --- a/conf/master +++ b/conf/master @@ -618,6 +618,9 @@ # on the "renderer" setting and is the default value. #pillar_source_merging_strategy: smart +# Recursively merge lists by aggregating them instead of replacing them. +#pillar_merge_lists: True + ##### Syndic settings ##### ########################################## diff --git a/doc/ref/configuration/master.rst b/doc/ref/configuration/master.rst index 55d5d0fd2c75..aff0f0ceb5ce 100644 --- a/doc/ref/configuration/master.rst +++ b/doc/ref/configuration/master.rst @@ -2255,35 +2255,6 @@ strategy between different sources. It accepts 4 values: element2: True baz: quux -* recurse_list: - - it will merge recursively mapping of data similar to ``recurse`` but merge - lists by aggregating them instead of replacing them. - - .. code-block:: yaml - - foo: 43 - bar: - - 1 - - 2 - - .. code-block:: yaml - - bar: - - 3 - baz: quux - - will be merged as: - - .. code-block:: yaml - - foo: 42 - bar: - - 1 - - 2 - - 3 - baz: quux - * aggregate: instructs aggregation of elements between sources that use the #!yamlex renderer. @@ -2353,6 +2324,21 @@ strategy between different sources. It accepts 4 values: Guesses the best strategy based on the "renderer" setting. +``pillar_merge_lists`` +---------------------------------- + +.. versionadded:: 2015.8.0 + +Default: ``True`` + +Recursively merge lists by aggregating them instead of replacing them. + +.. code-block:: yaml + + pillar_merge_lists: True + +.. conf_master:: pillar_source_merging_strategy + Syndic Server Settings ====================== diff --git a/salt/auth/__init__.py b/salt/auth/__init__.py index 9e340df31abc..4e2a891a9b99 100644 --- a/salt/auth/__init__.py +++ b/salt/auth/__init__.py @@ -211,12 +211,14 @@ def auth_data(self): Gather and create the authorization data sets ''' auth_data = self.opts['external_auth'] + merge_lists = self.opts['pillar_merge_lists'] if 'django' in auth_data and '^model' in auth_data['django']: auth_from_django = salt.auth.django.retrieve_auth_entries() auth_data = salt.utils.dictupdate.merge(auth_data, auth_from_django, - strategy='list') + strategy='list', + merge_lists=merge_lists) #for auth_back in self.opts.get('external_auth_sources', []): # fstr = '{0}.perms'.format(auth_back) diff --git a/salt/config/__init__.py b/salt/config/__init__.py index 179203227d1d..3341a1085aa9 100644 --- a/salt/config/__init__.py +++ b/salt/config/__init__.py @@ -512,6 +512,9 @@ # encountering duplicate values 'pillar_source_merging_strategy': str, + # Recursively merge lists by aggregating them instead of replacing them. + 'pillar_merge_lists': bool, + # How to merge multiple top files from multiple salt environments # (saltenvs); can be 'merge' or 'same' 'top_file_merging_strategy': str, @@ -1067,6 +1070,7 @@ 'pillar_opts': False, 'pillar_safe_render_error': True, 'pillar_source_merging_strategy': 'smart', + 'pillar_merge_lists': True, 'ping_on_rotate': False, 'peer': {}, 'preserve_minion_cache': False, diff --git a/salt/modules/config.py b/salt/modules/config.py index 88bdba790985..1a36b2dd0df1 100644 --- a/salt/modules/config.py +++ b/salt/modules/config.py @@ -11,6 +11,7 @@ import logging # Import salt libs +import salt.config import salt.utils try: # Gated for salt-ssh (salt.utils.cloud imports msgpack) @@ -357,10 +358,12 @@ def get(key, default='', delimiter=':', merge=None): 'to \'recurse\'.'.format(merge)) merge = 'recurse' + merge_lists = salt.config.master_config('/etc/salt/master').get('pillar_merge_lists') + data = copy.copy(__pillar__.get('master', {})) - data = salt.utils.dictupdate.merge(data, __pillar__, strategy=merge) - data = salt.utils.dictupdate.merge(data, __grains__, strategy=merge) - data = salt.utils.dictupdate.merge(data, __opts__, strategy=merge) + data = salt.utils.dictupdate.merge(data, __pillar__, strategy=merge, merge_lists=merge_lists) + data = salt.utils.dictupdate.merge(data, __grains__, strategy=merge, merge_lists=merge_lists) + data = salt.utils.dictupdate.merge(data, __opts__, strategy=merge, merge_lists=merge_lists) ret = salt.utils.traverse_dict_and_list(data, key, '_|-', diff --git a/salt/pillar/__init__.py b/salt/pillar/__init__.py index 5f4d90936c5f..95dd159c1088 100644 --- a/salt/pillar/__init__.py +++ b/salt/pillar/__init__.py @@ -540,7 +540,8 @@ def render_pstate(self, sls, saltenv, mods, defaults=None): state, nstate, self.merge_strategy, - self.opts.get('renderer', 'yaml')) + self.opts.get('renderer', 'yaml'), + self.opts.get('pillar_merge_lists', 'True')) if err: errors += err @@ -579,7 +580,8 @@ def render_pillar(self, matches): pillar, pstate, self.merge_strategy, - self.opts.get('renderer', 'yaml')) + self.opts.get('renderer', 'yaml'), + self.opts.get('pillar_merge_lists', 'True')) return pillar, errors @@ -667,7 +669,8 @@ def ext_pillar(self, pillar, pillar_dirs): pillar, ext, self.merge_strategy, - self.opts.get('renderer', 'yaml')) + self.opts.get('renderer', 'yaml'), + self.opts.get('pillar_merge_lists', 'True')) ext = None return pillar @@ -684,7 +687,8 @@ def compile_pillar(self, ext=True, pillar_dirs=None): pillar = merge(pillar, self.opts['pillar'], self.merge_strategy, - self.opts.get('renderer', 'yaml')) + self.opts.get('renderer', 'yaml'), + self.opts.get('pillar_merge_lists', 'True')) else: matches = self.top_matches(top) pillar, errors = self.render_pillar(matches) diff --git a/salt/pillar/git_pillar.py b/salt/pillar/git_pillar.py index 4f28fcb03ef1..72ecaab210ee 100644 --- a/salt/pillar/git_pillar.py +++ b/salt/pillar/git_pillar.py @@ -261,6 +261,10 @@ def ext_pillar(minion_id, repo, pillar_dirs): 'pillar_source_merging_strategy', 'smart' ) + merge_lists = __opts__.get( + 'pillar_merge_lists', + 'True' + ) for pillar_dir, env in six.iteritems(pillar.pillar_dirs): log.debug( 'git_pillar is processing pillar SLS from {0} for pillar ' @@ -274,7 +278,8 @@ def ext_pillar(minion_id, repo, pillar_dirs): ret = salt.utils.dictupdate.merge( ret, local_pillar.compile_pillar(ext=False), - strategy=merge_strategy + strategy=merge_strategy, + merge_lists=merge_lists ) return ret diff --git a/salt/utils/dictupdate.py b/salt/utils/dictupdate.py index 033b22766cae..f89ba5e06b07 100644 --- a/salt/utils/dictupdate.py +++ b/salt/utils/dictupdate.py @@ -17,7 +17,7 @@ log = logging.getLogger(__name__) -def update(dest, upd, recursive_update=True, merge_lists=False): +def update(dest, upd, recursive_update=True, merge_lists=True): ''' Recursive version of the default dict.update @@ -76,7 +76,7 @@ def merge_list(obj_a, obj_b): return ret -def merge_recurse(obj_a, obj_b, merge_lists=False): +def merge_recurse(obj_a, obj_b, merge_lists=True): copied = copy.deepcopy(obj_a) return update(copied, obj_b, merge_lists=merge_lists) @@ -85,14 +85,14 @@ def merge_aggregate(obj_a, obj_b): return _yamlex_merge_recursive(obj_a, obj_b, level=1) -def merge_overwrite(obj_a, obj_b): +def merge_overwrite(obj_a, obj_b, merge_lists=True): for obj in obj_b: if obj in obj_a: obj_a[obj] = obj_b[obj] - return merge_recurse(obj_a, obj_b) + return merge_recurse(obj_a, obj_b, merge_lists=merge_lists) -def merge(obj_a, obj_b, strategy='smart', renderer='yaml'): +def merge(obj_a, obj_b, strategy='smart', renderer='yaml', merge_lists=True): if strategy == 'smart': if renderer == 'yamlex' or renderer.startswith('yamlex_'): strategy = 'aggregate' @@ -102,14 +102,12 @@ def merge(obj_a, obj_b, strategy='smart', renderer='yaml'): if strategy == 'list': merged = merge_list(obj_a, obj_b) elif strategy == 'recurse': - merged = merge_recurse(obj_a, obj_b) - elif strategy == 'recurse_list': - merged = merge_recurse(obj_a, obj_b, merge_lists=True) + merged = merge_recurse(obj_a, obj_b, merge_lists) elif strategy == 'aggregate': #: level = 1 merge at least root data merged = merge_aggregate(obj_a, obj_b) elif strategy == 'overwrite': - merged = merge_overwrite(obj_a, obj_b) + merged = merge_overwrite(obj_a, obj_b, merge_lists) else: log.warning('Unknown merging strategy \'{0}\', ' 'fallback to recurse'.format(strategy)) diff --git a/tests/unit/pillar_test.py b/tests/unit/pillar_test.py index e9aff93ffd40..14a253c02c76 100644 --- a/tests/unit/pillar_test.py +++ b/tests/unit/pillar_test.py @@ -130,38 +130,6 @@ def test_topfile_order(self, Matcher, get_file_client): pillar = salt.pillar.Pillar(opts, grains, 'mocked-minion', 'base') self.assertEqual(pillar.compile_pillar()['ssh'], 'foo') - @patch('salt.pillar.salt.fileclient.get_file_client', autospec=True) - @patch('salt.pillar.salt.minion.Matcher') # autospec=True disabled due to py3 mock bug - def test_pillar_multiple_matches(self, Matcher, get_file_client): - # Uses the ``recurse_list`` strategy. - opts = { - 'renderer': 'yaml', - 'state_top': '', - 'pillar_roots': [], - 'extension_modules': '', - 'environment': 'base', - 'file_roots': [], - 'pillar_source_merging_strategy': 'recurse_list', - } - grains = { - 'os': 'Ubuntu', - 'os_family': 'Debian', - 'oscodename': 'raring', - 'osfullname': 'Ubuntu', - 'osrelease': '13.04', - 'kernel': 'Linux' - } - self._setup_test_topfile_mocks(Matcher, get_file_client, 1, 2) - pillar = salt.pillar.Pillar(opts, grains, 'mocked-minion', 'base') - # Pillars should be merged, but only once per pillar file. - self.assertDictEqual(pillar.compile_pillar()['generic'], { - 'key1': ['value1', 'value2', 'value3'], - 'key2': { - 'sub_key1': [], - 'sub_key2': [], - } - }) - def _setup_test_topfile_mocks(self, Matcher, get_file_client, nodegroup_order, glob_order): # Write a simple topfile and two pillar state files @@ -232,33 +200,6 @@ def get_state(sls, env): client.get_state.side_effect = get_state - @patch('salt.pillar.salt.fileclient.get_file_client', autospec=True) - @patch('salt.pillar.salt.minion.Matcher') # autospec=True disabled due to py3 mock bug - def test_pillar_include(self, Matcher, get_file_client): - # Uses the ``recurse_list`` strategy. - opts = { - 'renderer': 'yaml', - 'state_top': '', - 'pillar_roots': [], - 'extension_modules': '', - 'environment': 'base', - 'file_roots': [], - 'pillar_source_merging_strategy': 'recurse_list', - } - grains = { - 'os': 'Ubuntu', - 'os_family': 'Debian', - 'oscodename': 'raring', - 'osfullname': 'Ubuntu', - 'osrelease': '13.04', - 'kernel': 'Linux' - } - self._setup_test_include_mocks(Matcher, get_file_client) - pillar = salt.pillar.Pillar(opts, grains, 'mocked-minion', 'base').compile_pillar() - # Both pillar modules should only be loaded once. - self.assertEqual(pillar['p1'], ['value1_3', 'value1_1', 'value1_2']) - self.assertEqual(pillar['p2'], ['value2_1', 'value2_2']) - def _setup_test_include_mocks(self, Matcher, get_file_client): self.top_file = top_file = tempfile.NamedTemporaryFile() top_file.write(b''' diff --git a/tests/unit/utils/dictupdate_test.py b/tests/unit/utils/dictupdate_test.py index bb3337c7614e..137dd98cc145 100644 --- a/tests/unit/utils/dictupdate_test.py +++ b/tests/unit/utils/dictupdate_test.py @@ -29,7 +29,8 @@ def test_update(self): # level 1 value changes (list replacement) mdict = copy.deepcopy(self.dict1) mdict['A'] = [1, 2] - res = dictupdate.update(copy.deepcopy(mdict), {'A': [2, 3]}) + res = dictupdate.update(copy.deepcopy(mdict), {'A': [2, 3]}, + merge_lists=False) mdict['A'] = [2, 3] self.assertEqual(res, mdict) @@ -50,7 +51,8 @@ def test_update(self): # level 2 value changes (list replacement) mdict = copy.deepcopy(self.dict1) mdict['C']['D'] = ['a', 'b'] - res = dictupdate.update(copy.deepcopy(mdict), {'C': {'D': ['c', 'd']}}) + res = dictupdate.update(copy.deepcopy(mdict), {'C': {'D': ['c', 'd']}}, + merge_lists=False) mdict['C']['D'] = ['c', 'd'] self.assertEqual(res, mdict) @@ -75,7 +77,7 @@ def test_update(self): mdict = copy.deepcopy(self.dict1) mdict['C']['F']['G'] = ['a', 'b'] res = dictupdate.update(copy.deepcopy(mdict), - {'C': {'F': {'G': ['c', 'd']}}}) + {'C': {'F': {'G': ['c', 'd']}}}, merge_lists=False) mdict['C']['F']['G'] = ['c', 'd'] self.assertEqual(res, mdict)