From 079f09798555d9eb565d244fc1a652eff8dcba09 Mon Sep 17 00:00:00 2001 From: Jason Unovitch Date: Mon, 7 Aug 2017 14:47:28 -0400 Subject: [PATCH 01/24] Fix 'preserve_minion_cache: True' functionality (fixes #35840) - Drop preserve_minions as a a condition used in the check_minion_cache() logic test. This should be more in line with the intent of the "Optionally, pass in a list of minions which should have their caches preserved. To preserve all caches, set __opts__['preserve_minion_cache']" comment documented in key.py. - Add --preserve_minions as an optional CLI option to salt-key. This will allow the user to optionally preserve caches independently of the preserve_minion_cache option. Option does not override config file. - This effectively reverts commit 661f5686bf1090554d571a6ed23f09d511f5a15a which introduced the regression with preserve_minion_cache set to True to fix a regression when preserve_minion_cache is set to False. Prior to that commit, the preserve_minion_cache option was completely ignored when set to True and when set to False cache directories were still preserved. - Functional testing (three minions 'minion1', 'oldminion', and 'minion2') /etc/salt/master - preserve_minion_cache: False # salt-key -d minion1 PASS: deletes 'minion1', deletes stale 'oldminion', preserve active 'minion2' # salt-key -d minion1 --preserve-minions=true PASS: preserves minion1 as requested, deletes oldminion as it was not in the match from the delete_key() comment "To preserve the master caches of minions who are matched", preserves active minion2 /etc/salt/master - preserve_minion_cache: True # salt-key -d minion1 PASS: no directories deleted per config option # salt-key -d minion1 --preserve-minions=false PASS: no directories deleted per config option, does not override config --- salt/key.py | 37 ++++++++++++++++++++----------------- salt/utils/parsers.py | 17 +++++++++++++++++ 2 files changed, 37 insertions(+), 17 deletions(-) diff --git a/salt/key.py b/salt/key.py index 51d7be6a89..f1dd28319f 100644 --- a/salt/key.py +++ b/salt/key.py @@ -489,7 +489,7 @@ class Key(object): minions = [] for key, val in six.iteritems(keys): minions.extend(val) - if not self.opts.get('preserve_minion_cache', False) or not preserve_minions: + if not self.opts.get('preserve_minion_cache', False): m_cache = os.path.join(self.opts['cachedir'], self.ACC) if os.path.isdir(m_cache): for minion in os.listdir(m_cache): @@ -736,7 +736,7 @@ class Key(object): def delete_key(self, match=None, match_dict=None, - preserve_minions=False, + preserve_minions=None, revoke_auth=False): ''' Delete public keys. If "match" is passed, it is evaluated as a glob. @@ -774,11 +774,10 @@ class Key(object): salt.utils.event.tagify(prefix='key')) except (OSError, IOError): pass - if preserve_minions: - preserve_minions_list = matches.get('minions', []) + if self.opts.get('preserve_minions') is True: + self.check_minion_cache(preserve_minions=matches.get('minions', [])) else: - preserve_minions_list = [] - self.check_minion_cache(preserve_minions=preserve_minions_list) + self.check_minion_cache() if self.opts.get('rotate_aes_key'): salt.crypt.dropfile(self.opts['cachedir'], self.opts['user']) return ( @@ -969,16 +968,17 @@ class RaetKey(Key): minions.extend(val) m_cache = os.path.join(self.opts['cachedir'], 'minions') - if os.path.isdir(m_cache): - for minion in os.listdir(m_cache): - if minion not in minions: - shutil.rmtree(os.path.join(m_cache, minion)) - cache = salt.cache.factory(self.opts) - clist = cache.ls(self.ACC) - if clist: - for minion in clist: + if not self.opts.get('preserve_minion_cache', False): + if os.path.isdir(m_cache): + for minion in os.listdir(m_cache): if minion not in minions and minion not in preserve_minions: - cache.flush('{0}/{1}'.format(self.ACC, minion)) + shutil.rmtree(os.path.join(m_cache, minion)) + cache = salt.cache.factory(self.opts) + clist = cache.ls(self.ACC) + if clist: + for minion in clist: + if minion not in minions and minion not in preserve_minions: + cache.flush('{0}/{1}'.format(self.ACC, minion)) kind = self.opts.get('__role', '') # application kind if kind not in salt.utils.kinds.APPL_KINDS: @@ -1220,7 +1220,7 @@ class RaetKey(Key): def delete_key(self, match=None, match_dict=None, - preserve_minions=False, + preserve_minions=None, revoke_auth=False): ''' Delete public keys. If "match" is passed, it is evaluated as a glob. @@ -1251,7 +1251,10 @@ class RaetKey(Key): os.remove(os.path.join(self.opts['pki_dir'], status, key)) except (OSError, IOError): pass - self.check_minion_cache(preserve_minions=matches.get('minions', [])) + if self.opts.get('preserve_minions') is True: + self.check_minion_cache(preserve_minions=matches.get('minions', [])) + else: + self.check_minion_cache() return ( self.name_match(match) if match is not None else self.dict_match(matches) diff --git a/salt/utils/parsers.py b/salt/utils/parsers.py index 34565da0d6..fe8bacaf15 100644 --- a/salt/utils/parsers.py +++ b/salt/utils/parsers.py @@ -2306,6 +2306,16 @@ class SaltKeyOptionParser(six.with_metaclass(OptionParserMeta, 'Default: %default.') ) + self.add_option( + '--preserve-minions', + default=False, + help=('Setting this to True prevents the master from deleting ' + 'the minion cache when keys are deleted, this may have ' + 'security implications if compromised minions auth with ' + 'a previous deleted minion ID. ' + 'Default: %default.') + ) + key_options_group = optparse.OptionGroup( self, 'Key Generation Options' ) @@ -2405,6 +2415,13 @@ class SaltKeyOptionParser(six.with_metaclass(OptionParserMeta, elif self.options.rotate_aes_key.lower() == 'false': self.options.rotate_aes_key = False + def process_preserve_minions(self): + if hasattr(self.options, 'preserve_minions') and isinstance(self.options.preserve_minions, str): + if self.options.preserve_minions.lower() == 'true': + self.options.preserve_minions = True + elif self.options.preserve_minions.lower() == 'false': + self.options.preserve_minions = False + def process_list(self): # Filter accepted list arguments as soon as possible if not self.options.list: From f7c945f6e44c15c0a62572a73fb8eb101420527f Mon Sep 17 00:00:00 2001 From: Paul Miller Date: Sun, 20 Aug 2017 09:06:39 -0400 Subject: [PATCH 02/24] Prevent spurious "Template does not exist" error This was merged previously (though slightly differently) in #39516 Took me a second to track it down and then realized that I fixed this in 2016.x --- salt/pillar/__init__.py | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/salt/pillar/__init__.py b/salt/pillar/__init__.py index a62e11dc77..8d5eb7e998 100644 --- a/salt/pillar/__init__.py +++ b/salt/pillar/__init__.py @@ -405,20 +405,19 @@ class Pillar(object): self.opts['pillarenv'], ', '.join(self.opts['file_roots']) ) else: - tops[self.opts['pillarenv']] = [ - compile_template( - self.client.cache_file( - self.opts['state_top'], - self.opts['pillarenv'] - ), - self.rend, - self.opts['renderer'], - self.opts['renderer_blacklist'], - self.opts['renderer_whitelist'], - self.opts['pillarenv'], - _pillar_rend=True, - ) - ] + top = self.client.cache_file(self.opts['state_top'], self.opts['pillarenv']) + if top: + tops[self.opts['pillarenv']] = [ + compile_template( + top, + self.rend, + self.opts['renderer'], + self.opts['renderer_blacklist'], + self.opts['renderer_whitelist'], + self.opts['pillarenv'], + _pillar_rend=True, + ) + ] else: for saltenv in self._get_envs(): if self.opts.get('pillar_source_merging_strategy', None) == "none": From 0d5a46dbaa8815636bd1e78b582ccc9c4b91ad19 Mon Sep 17 00:00:00 2001 From: rallytime Date: Tue, 29 Aug 2017 09:38:11 -0400 Subject: [PATCH 03/24] Update release branch section with a few more details --- doc/topics/development/contributing.rst | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/doc/topics/development/contributing.rst b/doc/topics/development/contributing.rst index fd21d86a23..7941397682 100644 --- a/doc/topics/development/contributing.rst +++ b/doc/topics/development/contributing.rst @@ -263,9 +263,17 @@ against that branch. Release Branches ---------------- -For each release a branch will be created when we are ready to tag. The branch will be the same name as the tag minus the v. For example, the v2017.7.1 release was created from the 2017.7.1 branch. This branching strategy will allow for more stability when there is a need for a re-tag during the testing phase of our releases. +For each release, a branch will be created when the SaltStack release team is +ready to tag. The release branch is created from the parent branch and will be +the same name as the tag minus the ``v``. For example, the ``2017.7.1`` release +branch was created from the ``2017.7`` parent branch and the ``v2017.7.1`` +release was tagged at the ``HEAD`` of the ``2017.7.1`` branch. This branching +strategy will allow for more stability when there is a need for a re-tag during +the testing phase of the release process. -Once the branch is created, the fixes required for a given release, as determined by the SaltStack release team, will be added to this branch. All commits in this branch will be merged forward into the parent branch as well. +Once the release branch is created, the fixes required for a given release, as +determined by the SaltStack release team, will be added to this branch. All +commits in this branch will be merged forward into the parent branch as well. Keeping Salt Forks in Sync ========================== From bad8f56969401040d5f249b8c1cf7e36f5e933bc Mon Sep 17 00:00:00 2001 From: rallytime Date: Mon, 14 Aug 2017 16:39:20 -0400 Subject: [PATCH 04/24] Always notify ryan-lane when changes occur on boto files --- .mention-bot | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.mention-bot b/.mention-bot index 56be9ab9e6..b6ed4f2b24 100644 --- a/.mention-bot +++ b/.mention-bot @@ -1,4 +1,11 @@ { + "alwaysNotifyForPaths": [ + { + "name": "ryan-lane", + "files": ["salt/**/*boto*.py"], + "skipTeamPrs": false + } + ], "skipTitle": "Merge forward", "userBlacklist": ["cvrebert", "markusgattol", "olliewalsh"] } From 40b5a29f90378e15e75c152d5c42e04f0654ad0d Mon Sep 17 00:00:00 2001 From: Colton Myers Date: Thu, 24 Aug 2017 15:55:30 -0600 Subject: [PATCH 05/24] Add basepi to userBlacklist for mention bot --- .mention-bot | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.mention-bot b/.mention-bot index b6ed4f2b24..86890cfde0 100644 --- a/.mention-bot +++ b/.mention-bot @@ -7,6 +7,6 @@ } ], "skipTitle": "Merge forward", - "userBlacklist": ["cvrebert", "markusgattol", "olliewalsh"] + "userBlacklist": ["cvrebert", "markusgattol", "olliewalsh", "basepi"] } From 2b85757d733d5a82307391f77c15dda2de5a0dc8 Mon Sep 17 00:00:00 2001 From: rallytime Date: Fri, 25 Aug 2017 19:34:29 -0400 Subject: [PATCH 06/24] Always notify tkwilliams when changes occur on boto files --- .mention-bot | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.mention-bot b/.mention-bot index 86890cfde0..c07f85b9fc 100644 --- a/.mention-bot +++ b/.mention-bot @@ -4,6 +4,11 @@ "name": "ryan-lane", "files": ["salt/**/*boto*.py"], "skipTeamPrs": false + }, + { + "name": "tkwilliams", + "files": ["salt/**/*boto*.py"], + "skipTeamPrs": false } ], "skipTitle": "Merge forward", From f36efbd6a7bed64c28b7c525ef382e219621e2af Mon Sep 17 00:00:00 2001 From: twangboy Date: Thu, 31 Aug 2017 17:10:38 -0600 Subject: [PATCH 07/24] Fix `unit.test_spm` for Windows This only fixes the test... I don't think it fixes SPM on Windows --- salt/spm/__init__.py | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/salt/spm/__init__.py b/salt/spm/__init__.py index 2bd2646b61..dc7e1009d5 100644 --- a/salt/spm/__init__.py +++ b/salt/spm/__init__.py @@ -14,9 +14,12 @@ import shutil import msgpack import hashlib import logging -import pwd -import grp import sys +try: + import pwd + import grp +except ImportError: + pass # Import Salt libs import salt.client @@ -491,10 +494,20 @@ class SPMClient(object): # No defaults for this in config.py; default to the current running # user and group - uid = self.opts.get('spm_uid', os.getuid()) - gid = self.opts.get('spm_gid', os.getgid()) - uname = pwd.getpwuid(uid)[0] - gname = grp.getgrgid(gid)[0] + import salt.utils + if salt.utils.is_windows(): + import salt.utils.win_functions + cur_user = salt.utils.win_functions.get_current_user() + cur_user_sid = salt.utils.win_functions.get_sid_from_name(cur_user) + uid = self.opts.get('spm_uid', cur_user_sid) + gid = self.opts.get('spm_gid', cur_user_sid) + uname = cur_user + gname = cur_user + else: + uid = self.opts.get('spm_uid', os.getuid()) + gid = self.opts.get('spm_gid', os.getgid()) + uname = pwd.getpwuid(uid)[0] + gname = grp.getgrgid(gid)[0] # Second pass: install the files for member in pkg_files: @@ -710,7 +723,7 @@ class SPMClient(object): raise SPMInvocationError('A path to a directory must be specified') if args[1] == '.': - repo_path = os.environ['PWD'] + repo_path = os.getcwd() else: repo_path = args[1] From b8da04c04da6bb26dbbe905cdf26cf5961f22fb6 Mon Sep 17 00:00:00 2001 From: twangboy Date: Fri, 1 Sep 2017 11:27:58 -0600 Subject: [PATCH 08/24] Add Mike's changes Remove unnecessary assignment Use getcwdu() --- salt/spm/__init__.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/salt/spm/__init__.py b/salt/spm/__init__.py index dc7e1009d5..47b4defab8 100644 --- a/salt/spm/__init__.py +++ b/salt/spm/__init__.py @@ -497,12 +497,10 @@ class SPMClient(object): import salt.utils if salt.utils.is_windows(): import salt.utils.win_functions - cur_user = salt.utils.win_functions.get_current_user() - cur_user_sid = salt.utils.win_functions.get_sid_from_name(cur_user) - uid = self.opts.get('spm_uid', cur_user_sid) - gid = self.opts.get('spm_gid', cur_user_sid) - uname = cur_user - gname = cur_user + uname = gname = salt.utils.win_functions.get_current_user() + uname_sid = salt.utils.win_functions.get_sid_from_name(uname) + uid = self.opts.get('spm_uid', uname_sid) + gid = self.opts.get('spm_gid', uname_sid) else: uid = self.opts.get('spm_uid', os.getuid()) gid = self.opts.get('spm_gid', os.getgid()) @@ -723,7 +721,7 @@ class SPMClient(object): raise SPMInvocationError('A path to a directory must be specified') if args[1] == '.': - repo_path = os.getcwd() + repo_path = os.getcwdu() else: repo_path = args[1] From 23d9abb5606d770ad1b3a663f7d38b93aff1a771 Mon Sep 17 00:00:00 2001 From: Daniel Wallace Date: Tue, 5 Sep 2017 11:20:47 -0600 Subject: [PATCH 09/24] ipaddr_start ipaddr_end for el7 --- salt/templates/rh_ip/rh7_eth.jinja | 2 ++ 1 file changed, 2 insertions(+) diff --git a/salt/templates/rh_ip/rh7_eth.jinja b/salt/templates/rh_ip/rh7_eth.jinja index 62c871ae63..d60932dd20 100644 --- a/salt/templates/rh_ip/rh7_eth.jinja +++ b/salt/templates/rh_ip/rh7_eth.jinja @@ -15,6 +15,8 @@ DEVICE="{{name}}" {%endif%}{% if onparent %}ONPARENT={{onparent}} {%endif%}{% if ipv4_failure_fatal %}IPV4_FAILURE_FATAL="{{ipv4_failure_fatal}}" {%endif%}{% if ipaddr %}IPADDR="{{ipaddr}}" +{%endif%}{% if ipaddr_start %}IPADDR_START="{{ipaddr_start}}" +{%endif%}{% if ipaddr_end %}IPADDR_END="{{ipaddr_end}}" {%endif%}{% if netmask %}NETMASK="{{netmask}}" {%endif%}{% if prefix %}PREFIX="{{prefix}}" {%endif%}{% if gateway %}GATEWAY="{{gateway}}" From f6c16935d897085203d17e3d144ac258ef1c2491 Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Tue, 5 Sep 2017 15:10:05 -0500 Subject: [PATCH 10/24] Move --showduplicates before repository-packages As of Fedora 26, dnf interprets arguments after "repository-packages" as arguments to the repository-package subcommand, which breaks pkg.list_repo_pkgs on that Fedora release. Moving this argument earlier in the command allows pkg.list_repo_pkgs to work. --- salt/modules/yumpkg.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/salt/modules/yumpkg.py b/salt/modules/yumpkg.py index 14cdf0d899..c675a56608 100644 --- a/salt/modules/yumpkg.py +++ b/salt/modules/yumpkg.py @@ -764,8 +764,8 @@ def list_repo_pkgs(*args, **kwargs): _parse_output(out['stdout'], strict=True) else: for repo in repos: - cmd = [_yum(), '--quiet', 'repository-packages', repo, - 'list', '--showduplicates'] + cmd = [_yum(), '--quiet', '--showduplicates', + 'repository-packages', repo, 'list'] # Can't concatenate because args is a tuple, using list.extend() cmd.extend(args) From 433bca14b17d94501a4d94d0a318eeb22a24e63a Mon Sep 17 00:00:00 2001 From: Erik Johnson Date: Tue, 5 Sep 2017 15:13:22 -0500 Subject: [PATCH 11/24] Fix KeyError in yumpkg configparser code on Python 3 It looks like the configparser in Python 3 does not insert the `__name__` key in each section. Popping it without a default causes a KeyError on Python 3. This commit fixes that KeyError. --- salt/modules/yumpkg.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/modules/yumpkg.py b/salt/modules/yumpkg.py index c675a56608..9b2c541953 100644 --- a/salt/modules/yumpkg.py +++ b/salt/modules/yumpkg.py @@ -2539,7 +2539,7 @@ def _parse_repo_file(filename): for section in parsed._sections: section_dict = dict(parsed._sections[section]) - section_dict.pop('__name__') + section_dict.pop('__name__', None) config[section] = section_dict # Try to extract leading comments From b09e5b43796caff82d03d53470b35c857ce7be59 Mon Sep 17 00:00:00 2001 From: John Jawed Date: Fri, 1 Sep 2017 18:33:28 -0700 Subject: [PATCH 12/24] Fix #43295, better handling of consul initialization issues --- salt/cache/consul.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/salt/cache/consul.py b/salt/cache/consul.py index b545c96ead..0148c73bf0 100644 --- a/salt/cache/consul.py +++ b/salt/cache/consul.py @@ -4,6 +4,8 @@ Minion data cache plugin for Consul key/value data store. .. versionadded:: 2016.11.2 +:depends: python-consul >= 0.2.0 + It is up to the system administrator to set up and configure the Consul infrastructure. All is needed for this plugin is a working Consul agent with a read-write access to the key-value store. @@ -81,8 +83,11 @@ def __virtual__(): 'verify': __opts__.get('consul.verify', True), } - global api - api = consul.Consul(**consul_kwargs) + try: + global api + api = consul.Consul(**consul_kwargs) + except AttributeError: + return (False, "Failed to invoke consul.Consul, please make sure you have python-consul >= 0.2.0 installed") return __virtualname__ From 281e47185398b781cf63d192e25ab989d2b8ee4a Mon Sep 17 00:00:00 2001 From: Sergey Kizunov Date: Wed, 6 Sep 2017 11:25:25 -0500 Subject: [PATCH 13/24] Fix system.set_system_time when no hw clock is present If a hardware clock is not present, then the `cmd.run_all` call in `has_settable_hwclock` will return a non-zero retcode. If `ignore_retcode` is not set to True in that call, then it will set `__context__['retcode']` to that code which will cause the job to appear as if it failed (due to non-zero retcode) even though it didn't really fail. Signed-off-by: Sergey Kizunov --- salt/modules/system.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/salt/modules/system.py b/salt/modules/system.py index c397ad2f27..fe8fc0c699 100644 --- a/salt/modules/system.py +++ b/salt/modules/system.py @@ -168,7 +168,10 @@ def has_settable_hwclock(): salt '*' system.has_settable_hwclock ''' if salt.utils.which_bin(['hwclock']) is not None: - res = __salt__['cmd.run_all'](['hwclock', '--test', '--systohc'], python_shell=False) + res = __salt__['cmd.run_all']( + ['hwclock', '--test', '--systohc'], python_shell=False, + output_loglevel='quiet', ignore_retcode=True + ) return res['retcode'] == 0 return False From 4a8d7e522ce94961cb08140cdb50b2d30404d276 Mon Sep 17 00:00:00 2001 From: twangboy Date: Thu, 7 Sep 2017 14:41:56 -0600 Subject: [PATCH 14/24] Fix tests, Use full path to salt.utils.which --- salt/modules/mount.py | 5 ++--- tests/unit/modules/test_mount.py | 25 ++++++++++++++++++------- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/salt/modules/mount.py b/salt/modules/mount.py index 1418bc7673..1749c82ccd 100644 --- a/salt/modules/mount.py +++ b/salt/modules/mount.py @@ -11,7 +11,6 @@ import logging # Import salt libs import salt.utils -from salt.utils import which as _which from salt.exceptions import CommandNotFoundError, CommandExecutionError # Import 3rd-party libs @@ -1114,12 +1113,12 @@ def is_fuse_exec(cmd): salt '*' mount.is_fuse_exec sshfs ''' - cmd_path = _which(cmd) + cmd_path = salt.utils.which(cmd) # No point in running ldd on a command that doesn't exist if not cmd_path: return False - elif not _which('ldd'): + elif not salt.utils.which('ldd'): raise CommandNotFoundError('ldd') out = __salt__['cmd.run']('ldd {0}'.format(cmd_path), python_shell=False) diff --git a/tests/unit/modules/test_mount.py b/tests/unit/modules/test_mount.py index 4f72f28a1a..1e42048d14 100644 --- a/tests/unit/modules/test_mount.py +++ b/tests/unit/modules/test_mount.py @@ -19,7 +19,7 @@ from tests.support.mock import ( # Import Salt Libs import salt.utils -from salt.exceptions import CommandExecutionError +from salt.exceptions import CommandExecutionError, CommandNotFoundError import salt.modules.mount as mount MOCK_SHELL_FILE = 'A B C D F G\n' @@ -242,15 +242,26 @@ class MountTestCase(TestCase, LoaderModuleMockMixin): ''' Returns true if the command passed is a fuse mountable application ''' - with patch.object(salt.utils, 'which', return_value=None): + # Return False if fuse doesn't exist + with patch('salt.utils.which', return_value=None): self.assertFalse(mount.is_fuse_exec('cmd')) - with patch.object(salt.utils, 'which', return_value=True): - self.assertFalse(mount.is_fuse_exec('cmd')) + # Return CommandNotFoundError if fuse exists, but ldd doesn't exist + with patch('salt.utils.which', side_effect=[True, False]): + self.assertRaises(CommandNotFoundError, mount.is_fuse_exec, 'cmd') - mock = MagicMock(side_effect=[1, 0]) - with patch.object(salt.utils, 'which', mock): - self.assertFalse(mount.is_fuse_exec('cmd')) + # Return False if fuse exists, ldd exists, but libfuse is not in the + # return + with patch('salt.utils.which', side_effect=[True, True]): + mock = MagicMock(return_value='not correct') + with patch.dict(mount.__salt__, {'cmd.run': mock}): + self.assertFalse(mount.is_fuse_exec('cmd')) + + # Return True if fuse exists, ldd exists, and libfuse is in the return + with patch('salt.utils.which', side_effect=[True, True]): + mock = MagicMock(return_value='contains libfuse') + with patch.dict(mount.__salt__, {'cmd.run': mock}): + self.assertTrue(mount.is_fuse_exec('cmd')) def test_swaps(self): ''' From 6257aa964ad5f2831fffa4799ef2789c7b9fc783 Mon Sep 17 00:00:00 2001 From: twangboy Date: Thu, 7 Sep 2017 15:18:14 -0600 Subject: [PATCH 15/24] Fix `unit.modules.test_pam` for Windows Mocks os.path.exists --- tests/unit/modules/test_pam.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/unit/modules/test_pam.py b/tests/unit/modules/test_pam.py index 9cb2bc4577..492aabdd90 100644 --- a/tests/unit/modules/test_pam.py +++ b/tests/unit/modules/test_pam.py @@ -34,7 +34,8 @@ class PamTestCase(TestCase): ''' Test if the parsing function works ''' - with patch('salt.utils.fopen', mock_open(read_data=MOCK_FILE)): + with patch('os.path.exists', return_value=True), \ + patch('salt.utils.fopen', mock_open(read_data=MOCK_FILE)): self.assertListEqual(pam.read_file('/etc/pam.d/login'), [{'arguments': [], 'control_flag': 'ok', 'interface': 'ok', 'module': 'ignore'}]) From 8e3e897ee26ab3af3efaf9f8ac9b55db6227aad9 Mon Sep 17 00:00:00 2001 From: twangboy Date: Thu, 7 Sep 2017 15:27:11 -0600 Subject: [PATCH 16/24] Fix `unit.modules.test_parted` for Windows Mock salt.utils.is_windows to be False so that the __virtual__ tests will run --- tests/unit/modules/test_parted.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/unit/modules/test_parted.py b/tests/unit/modules/test_parted.py index 991c6787a2..abd0b5d91f 100644 --- a/tests/unit/modules/test_parted.py +++ b/tests/unit/modules/test_parted.py @@ -49,21 +49,24 @@ class PartedTestCase(TestCase, LoaderModuleMockMixin): def test_virtual_bails_without_parted(self): '''If parted not in PATH, __virtual__ shouldn't register module''' - with patch('salt.utils.which', lambda exe: not exe == "parted"): + with patch('salt.utils.which', lambda exe: not exe == "parted"),\ + patch('salt.utils.is_windows', return_value=False): ret = parted.__virtual__() err = (False, 'The parted execution module failed to load parted binary is not in the path.') self.assertEqual(err, ret) def test_virtual_bails_without_lsblk(self): '''If lsblk not in PATH, __virtual__ shouldn't register module''' - with patch('salt.utils.which', lambda exe: not exe == "lsblk"): + with patch('salt.utils.which', lambda exe: not exe == "lsblk"),\ + patch('salt.utils.is_windows', return_value=False): ret = parted.__virtual__() err = (False, 'The parted execution module failed to load lsblk binary is not in the path.') self.assertEqual(err, ret) def test_virtual_bails_without_partprobe(self): '''If partprobe not in PATH, __virtual__ shouldn't register module''' - with patch('salt.utils.which', lambda exe: not exe == "partprobe"): + with patch('salt.utils.which', lambda exe: not exe == "partprobe"),\ + patch('salt.utils.is_windows', return_value=False): ret = parted.__virtual__() err = (False, 'The parted execution module failed to load partprobe binary is not in the path.') self.assertEqual(err, ret) From 78e39a1b9dba1cda3a5c829787fdafd4767fbe93 Mon Sep 17 00:00:00 2001 From: twangboy Date: Thu, 7 Sep 2017 15:34:57 -0600 Subject: [PATCH 17/24] Fix `unit.modules.test_pw_group` for Windows Skip `test_info` and `test_getent` because they require grp which is not available on Windows --- tests/unit/modules/test_pw_group.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/unit/modules/test_pw_group.py b/tests/unit/modules/test_pw_group.py index 3d21bbd43c..07854e0aea 100644 --- a/tests/unit/modules/test_pw_group.py +++ b/tests/unit/modules/test_pw_group.py @@ -18,6 +18,7 @@ from tests.support.mock import ( # Import Salt Libs import salt.modules.pw_group as pw_group +import salt.utils @skipIf(NO_MOCK, NO_MOCK_REASON) @@ -44,6 +45,7 @@ class PwGroupTestCase(TestCase, LoaderModuleMockMixin): with patch.dict(pw_group.__salt__, {'cmd.run_all': mock}): self.assertTrue(pw_group.delete('a')) + @skipIf(salt.utils.is_windows(), 'grp not available on Windows') def test_info(self): ''' Tests to return information about a group @@ -57,6 +59,7 @@ class PwGroupTestCase(TestCase, LoaderModuleMockMixin): with patch.dict(pw_group.grinfo, mock): self.assertDictEqual(pw_group.info('name'), {}) + @skipIf(salt.utils.is_windows(), 'grp not available on Windows') def test_getent(self): ''' Tests for return info on all groups From 531ce8022b1aa05ec31a1420554f3d5812caef58 Mon Sep 17 00:00:00 2001 From: twangboy Date: Thu, 7 Sep 2017 15:44:35 -0600 Subject: [PATCH 18/24] Fix `unit.modules.test_qemu_nbd` for Windows Use os.sep --- tests/unit/modules/test_qemu_nbd.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/tests/unit/modules/test_qemu_nbd.py b/tests/unit/modules/test_qemu_nbd.py index ec6ec84587..59361c0050 100644 --- a/tests/unit/modules/test_qemu_nbd.py +++ b/tests/unit/modules/test_qemu_nbd.py @@ -80,15 +80,14 @@ class QemuNbdTestCase(TestCase, LoaderModuleMockMixin): with patch.dict(qemu_nbd.__salt__, {'cmd.run': mock}): self.assertEqual(qemu_nbd.init('/srv/image.qcow2'), '') - with patch.object(os.path, 'isfile', mock): - with patch.object(glob, 'glob', - MagicMock(return_value=['/dev/nbd0'])): - with patch.dict(qemu_nbd.__salt__, - {'cmd.run': mock, - 'mount.mount': mock, - 'cmd.retcode': MagicMock(side_effect=[1, 0])}): - self.assertDictEqual(qemu_nbd.init('/srv/image.qcow2'), - {'{0}/nbd/nbd0/nbd0'.format(tempfile.gettempdir()): '/dev/nbd0'}) + with patch.object(os.path, 'isfile', mock),\ + patch.object(glob, 'glob', MagicMock(return_value=['/dev/nbd0'])),\ + patch.dict(qemu_nbd.__salt__, + {'cmd.run': mock, + 'mount.mount': mock, + 'cmd.retcode': MagicMock(side_effect=[1, 0])}): + expected = {os.sep.join([tempfile.gettempdir(), 'nbd', 'nbd0', 'nbd0']): '/dev/nbd0'} + self.assertDictEqual(qemu_nbd.init('/srv/image.qcow2'), expected) # 'clear' function tests: 1 From 6ceb895a843e1188eff4dafe26a3abbd0c3550f3 Mon Sep 17 00:00:00 2001 From: twangboy Date: Thu, 7 Sep 2017 16:51:10 -0600 Subject: [PATCH 19/24] Use os.path.join for paths --- tests/unit/modules/test_seed.py | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/tests/unit/modules/test_seed.py b/tests/unit/modules/test_seed.py index 39be4a47cb..ea00a25d90 100644 --- a/tests/unit/modules/test_seed.py +++ b/tests/unit/modules/test_seed.py @@ -47,14 +47,19 @@ class SeedTestCase(TestCase, LoaderModuleMockMixin): ''' Test to update and get the random script to a random place ''' - with patch.dict(seed.__salt__, - {'config.gather_bootstrap_script': MagicMock(return_value='BS_PATH/BS')}): - with patch.object(uuid, 'uuid4', return_value='UUID'): - with patch.object(os.path, 'exists', return_value=True): - with patch.object(os, 'chmod', return_value=None): - with patch.object(shutil, 'copy', return_value=None): - self.assertEqual(seed.prep_bootstrap('MPT'), ('MPT/tmp/UUID/BS', '/tmp/UUID')) - self.assertEqual(seed.prep_bootstrap('/MPT'), ('/MPT/tmp/UUID/BS', '/tmp/UUID')) + with patch.dict(seed.__salt__, {'config.gather_bootstrap_script': MagicMock(return_value=os.path.join('BS_PATH', 'BS'))}),\ + patch.object(uuid, 'uuid4', return_value='UUID'),\ + patch.object(os.path, 'exists', return_value=True),\ + patch.object(os, 'chmod', return_value=None),\ + patch.object(shutil, 'copy', return_value=None): + + expect = (os.path.join('MPT', 'tmp', 'UUID', 'BS'), + os.sep + os.path.join('tmp', 'UUID')) + self.assertEqual(seed.prep_bootstrap('MPT'), expect) + + expect = (os.sep + os.path.join('MPT', 'tmp', 'UUID', 'BS'), + os.sep + os.path.join('tmp', 'UUID')) + self.assertEqual(seed.prep_bootstrap(os.sep + 'MPT'), expect) def test_apply_(self): ''' From e93a962980e64afa6ba5486e86d1df577d60de3d Mon Sep 17 00:00:00 2001 From: matt Date: Fri, 8 Sep 2017 17:10:07 +0200 Subject: [PATCH 20/24] Fix env_order in state.py Fixes #42165 --- salt/state.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/salt/state.py b/salt/state.py index 44fb73b97d..0f625c8fe2 100644 --- a/salt/state.py +++ b/salt/state.py @@ -3100,7 +3100,7 @@ class BaseHighState(object): Returns: {'saltenv': ['state1', 'state2', ...]} ''' - matches = {} + matches = DefaultOrderedDict(OrderedDict) # pylint: disable=cell-var-from-loop for saltenv, body in six.iteritems(top): if self.opts['environment']: From 58378866e53bac9379a66ad1f485d5e6739f3374 Mon Sep 17 00:00:00 2001 From: Daniel Wallace Date: Fri, 8 Sep 2017 14:10:46 -0600 Subject: [PATCH 21/24] make cache dirs when spm starts --- salt/cli/spm.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/salt/cli/spm.py b/salt/cli/spm.py index 3d347c80a8..303e5ce65f 100644 --- a/salt/cli/spm.py +++ b/salt/cli/spm.py @@ -14,7 +14,7 @@ from __future__ import absolute_import # Import Salt libs import salt.spm import salt.utils.parsers as parsers -from salt.utils.verify import verify_log +from salt.utils.verify import verify_log, verify_env class SPM(parsers.SPMParser): @@ -29,6 +29,10 @@ class SPM(parsers.SPMParser): ui = salt.spm.SPMCmdlineInterface() self.parse_args() self.setup_logfile_logger() + v_dirs = [ + self.config['cachedir'], + ] + verify_env(v_dirs, self.config['user'],) verify_log(self.config) client = salt.spm.SPMClient(ui, self.config) client.run(self.args) From 137962733477d7d71f0ee5d10d6edce2bf57f6b4 Mon Sep 17 00:00:00 2001 From: twangboy Date: Fri, 8 Sep 2017 15:36:52 -0600 Subject: [PATCH 22/24] Fix `unit.cloud.clouds.test_ec2` for Windows Mock instead of create tempfile --- tests/unit/cloud/clouds/test_ec2.py | 41 +++++++++++++---------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/tests/unit/cloud/clouds/test_ec2.py b/tests/unit/cloud/clouds/test_ec2.py index 9ffd74d47b..4f77b14a1b 100644 --- a/tests/unit/cloud/clouds/test_ec2.py +++ b/tests/unit/cloud/clouds/test_ec2.py @@ -2,42 +2,39 @@ # Import Python libs from __future__ import absolute_import -import os -import tempfile # Import Salt Libs from salt.cloud.clouds import ec2 from salt.exceptions import SaltCloudSystemExit # Import Salt Testing Libs -from tests.support.mixins import LoaderModuleMockMixin from tests.support.unit import TestCase, skipIf -from tests.support.mock import NO_MOCK, NO_MOCK_REASON +from tests.support.mock import NO_MOCK, NO_MOCK_REASON, patch, PropertyMock @skipIf(NO_MOCK, NO_MOCK_REASON) -class EC2TestCase(TestCase, LoaderModuleMockMixin): +class EC2TestCase(TestCase): ''' Unit TestCase for salt.cloud.clouds.ec2 module. ''' - def setup_loader_modules(self): - return {ec2: {}} - def test__validate_key_path_and_mode(self): - with tempfile.NamedTemporaryFile() as f: - key_file = f.name - os.chmod(key_file, 0o644) - self.assertRaises(SaltCloudSystemExit, - ec2._validate_key_path_and_mode, - key_file) - os.chmod(key_file, 0o600) - self.assertTrue(ec2._validate_key_path_and_mode(key_file)) - os.chmod(key_file, 0o400) - self.assertTrue(ec2._validate_key_path_and_mode(key_file)) + # Key file exists + with patch('os.path.exists', return_value=True): + with patch('os.stat') as patched_stat: - # tmp file removed - self.assertRaises(SaltCloudSystemExit, - ec2._validate_key_path_and_mode, - key_file) + type(patched_stat.return_value).st_mode = PropertyMock(return_value=0o644) + self.assertRaises( + SaltCloudSystemExit, ec2._validate_key_path_and_mode, 'key_file') + + type(patched_stat.return_value).st_mode = PropertyMock(return_value=0o600) + self.assertTrue(ec2._validate_key_path_and_mode('key_file')) + + type(patched_stat.return_value).st_mode = PropertyMock(return_value=0o400) + self.assertTrue(ec2._validate_key_path_and_mode('key_file')) + + # Key file does not exist + with patch('os.path.exists', return_value=False): + self.assertRaises( + SaltCloudSystemExit, ec2._validate_key_path_and_mode, 'key_file') From b2cea18d1368cb1df1e53b033f61396209cd6f0f Mon Sep 17 00:00:00 2001 From: twangboy Date: Fri, 8 Sep 2017 15:53:07 -0600 Subject: [PATCH 23/24] Fix `unit.modules.test_gem` for Windows Mock `salt.utils.is_windows` to return False so the test will run on Windows --- tests/unit/modules/test_gem.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/unit/modules/test_gem.py b/tests/unit/modules/test_gem.py index 14e38da893..b1ff893f7e 100644 --- a/tests/unit/modules/test_gem.py +++ b/tests/unit/modules/test_gem.py @@ -65,7 +65,8 @@ class TestGemModule(TestCase, LoaderModuleMockMixin): with patch.dict(gem.__salt__, {'rvm.is_installed': MagicMock(return_value=False), 'rbenv.is_installed': MagicMock(return_value=True), - 'rbenv.do': mock}): + 'rbenv.do': mock}),\ + patch('salt.utils.is_windows', return_value=False): gem._gem(['install', 'rails']) mock.assert_called_once_with( ['gem', 'install', 'rails'], From 5413a5337e4533a06efa9618862d9f07344dc06a Mon Sep 17 00:00:00 2001 From: rallytime Date: Mon, 11 Sep 2017 14:34:20 -0400 Subject: [PATCH 24/24] Lint: remove unused import --- tests/unit/modules/test_mount.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/modules/test_mount.py b/tests/unit/modules/test_mount.py index 1850faa212..8d8dcb6067 100644 --- a/tests/unit/modules/test_mount.py +++ b/tests/unit/modules/test_mount.py @@ -22,7 +22,7 @@ from tests.support.mock import ( import salt.utils.files import salt.utils.path import salt.modules.mount as mount -from salt.exceptions import CommandExecutionError, CommandNotFoundError +from salt.exceptions import CommandExecutionError MOCK_SHELL_FILE = 'A B C D F G\n'