Merge pull request #45922 from rallytime/merge-oxygen

[oxygen] Merge forward from 2017.7 to oxygen

doc/conf.py

@@ -46,6 +46,8 @@ class Mock(object):
             data = self.__mapping.get(name)
         elif name in ('__file__', '__path__'):
             data = '/dev/null'
+        elif name == '__qualname__':
+            raise AttributeError("'Mock' object has no attribute '__qualname__'")
         else:
             data = Mock(mapping=self.__mapping)
         return data
@@ -248,8 +250,8 @@ on_saltstack = 'SALT_ON_SALTSTACK' in os.environ
 project = 'Salt'
 
 version = salt.version.__version__
-latest_release = '2017.7.2'  # latest release
-previous_release = '2016.11.8'  # latest release from previous branch
+latest_release = '2017.7.3'  # latest release
+previous_release = '2016.11.9'  # latest release from previous branch
 previous_release_dir = '2016.11'  # path on web server for previous branch
 next_release = ''  # next release
 next_release_dir = ''  # path on web server for next release branch

doc/topics/releases/2016.11.9.rst

@@ -2,7 +2,7 @@
 Salt 2016.11.9 Release Notes
 ============================
 
-Version 2016.11.9 is a bugfix release for :ref:`2016.11.0 <release-2016-11-0>`.]
+Version 2016.11.9 is a bugfix release for :ref:`2016.11.0 <release-2016-11-0>`.
 
 Changes for v2016.11.8..v2016.11.9
 ----------------------------------------------------------------

doc/topics/releases/2017.7.4.rst

@@ -0,0 +1,5 @@
+===========================
+Salt 2017.7.4 Release Notes
+===========================
+
+Version 2017.7.4 is a bugfix release for :ref:`2017.7.0 <release-2017-7-0>`.

salt/cache/localfs.py

@@ -14,6 +14,7 @@ from __future__ import absolute_import, print_function, unicode_literals
 import logging
 import os
 import os.path
+import errno
 import shutil
 import tempfile
 
@@ -45,13 +46,14 @@ def store(bank, key, data, cachedir):
     Store information in a file.
     '''
     base = os.path.join(cachedir, os.path.normpath(bank))
-    if not os.path.isdir(base):
-        try:
-            os.makedirs(base)
-        except OSError as exc:
+    try:
+        os.makedirs(base)
+    except OSError as exc:
+        if exc.errno != errno.EEXIST:
             raise SaltCacheError(
-                'The cache directory, {0}, does not exist and could not be '
-                'created: {1}'.format(base, exc)
+                'The cache directory, {0}, could not be created: {1}'.format(
+                    base, exc
+                )
             )
 
     outfile = os.path.join(base, '{0}.p'.format(key))

salt/client/ssh/__init__.py

@@ -161,17 +161,17 @@ do
         py_cmd_path=`"$py_cmd" -c \
                    'from __future__ import print_function;
                    import sys; print(sys.executable);'`
-        cmdpath=$(command -v $py_cmd 2>/dev/null || which $py_cmd 2>/dev/null)
+        cmdpath=`command -v $py_cmd 2>/dev/null || which $py_cmd 2>/dev/null`
         if file $cmdpath | grep "shell script" > /dev/null
         then
             ex_vars="'PATH', 'LD_LIBRARY_PATH', 'MANPATH', \
                    'XDG_DATA_DIRS', 'PKG_CONFIG_PATH'"
-            export $($py_cmd -c \
+            export `$py_cmd -c \
                   "from __future__ import print_function;
                   import sys;
                   import os;
                   map(sys.stdout.write, ['{{{{0}}}}={{{{1}}}} ' \
-                  .format(x, os.environ[x]) for x in [$ex_vars]])")
+                  .format(x, os.environ[x]) for x in [$ex_vars]])"`
             exec $SUDO PATH=$PATH LD_LIBRARY_PATH=$LD_LIBRARY_PATH \
                      MANPATH=$MANPATH XDG_DATA_DIRS=$XDG_DATA_DIRS \
                      PKG_CONFIG_PATH=$PKG_CONFIG_PATH \

salt/modules/cmdmod.py

@@ -42,7 +42,6 @@ from salt.exceptions import CommandExecutionError, TimedProcTimeoutError, \
     SaltInvocationError
 from salt.log import LOG_LEVELS
 from salt.ext.six.moves import range, zip
-from salt.ext.six.moves import shlex_quote as _cmd_quote
 
 # Only available on POSIX systems, nonfatal on windows
 try:
@@ -52,8 +51,10 @@ except ImportError:
 
 if salt.utils.platform.is_windows():
     from salt.utils.win_runas import runas as win_runas
+    from salt.utils.win_functions import escape_argument as _cmd_quote
     HAS_WIN_RUNAS = True
 else:
+    from salt.ext.six.moves import shlex_quote as _cmd_quote
     HAS_WIN_RUNAS = False
 
 __proxyenabled__ = ['*']

salt/modules/localemod.py

@@ -67,7 +67,7 @@ def _localectl_status():
     Parse localectl status into a dict.
     :return: dict
     '''
-    if salt.utils.which('localectl') is None:
+    if salt.utils.path.which('localectl') is None:
         raise CommandExecutionError('Unable to find "localectl"')
 
     ret = {}

salt/modules/x509.py

@@ -508,7 +508,7 @@ def get_pem_entries(glob_path):
 
     .. code-block:: bash
 
-        salt '*' x509.read_pem_entries "/etc/pki/*.crt"
+        salt '*' x509.get_pem_entries "/etc/pki/*.crt"
     '''
     ret = {}
 

salt/state.py

@@ -167,6 +167,23 @@ def _l_tag(name, id_):
     return _gen_tag(low)
 
 
+def _calculate_fake_duration():
+    '''
+    Generate a NULL duration for when states do not run
+    but we want the results to be consistent.
+    '''
+    utc_start_time = datetime.datetime.utcnow()
+    local_start_time = utc_start_time - \
+        (datetime.datetime.utcnow() - datetime.datetime.now())
+    utc_finish_time = datetime.datetime.utcnow()
+    start_time = local_start_time.time().isoformat()
+    delta = (utc_finish_time - utc_start_time)
+    # duration in milliseconds.microseconds
+    duration = (delta.seconds * 1000000 + delta.microseconds)/1000.0
+
+    return start_time, duration
+
+
 def get_accumulator_dir(cachedir):
     '''
     Return the directory that accumulator data is stored in, creating it if it
@@ -2475,8 +2492,11 @@ class State(object):
                     run_dict = self.pre
                 else:
                     run_dict = running
+                start_time, duration = _calculate_fake_duration()
                 run_dict[tag] = {'changes': {},
                                  'result': False,
+                                 'duration': duration,
+                                 'start_time': start_time,
                                  'comment': comment,
                                  '__run_num__': self.__run_num,
                                  '__sls__': low['__sls__']}
@@ -2559,9 +2579,12 @@ class State(object):
                 _cmt = 'One or more requisite failed: {0}'.format(
                     ', '.join(six.text_type(i) for i in failed_requisites)
                 )
+                start_time, duration = _calculate_fake_duration()
                 running[tag] = {
                     'changes': {},
                     'result': False,
+                    'duration': duration,
+                    'start_time': start_time,
                     'comment': _cmt,
                     '__run_num__': self.__run_num,
                     '__sls__': low['__sls__']
@@ -2577,8 +2600,11 @@ class State(object):
                 ret = self.call(low, chunks, running)
             running[tag] = ret
         elif status == 'pre':
+            start_time, duration = _calculate_fake_duration()
             pre_ret = {'changes': {},
                        'result': True,
+                       'duration': duration,
+                       'start_time': start_time,
                        'comment': 'No changes detected',
                        '__run_num__': self.__run_num,
                        '__sls__': low['__sls__']}
@@ -2586,15 +2612,21 @@ class State(object):
             self.pre[tag] = pre_ret
             self.__run_num += 1
         elif status == 'onfail':
+            start_time, duration = _calculate_fake_duration()
             running[tag] = {'changes': {},
                             'result': True,
+                            'duration': duration,
+                            'start_time': start_time,
                             'comment': 'State was not run because onfail req did not change',
                             '__run_num__': self.__run_num,
                             '__sls__': low['__sls__']}
             self.__run_num += 1
         elif status == 'onchanges':
+            start_time, duration = _calculate_fake_duration()
             running[tag] = {'changes': {},
                             'result': True,
+                            'duration': duration,
+                            'start_time': start_time,
                             'comment': 'State was not run because none of the onchanges reqs changed',
                             '__run_num__': self.__run_num,
                             '__sls__': low['__sls__']}

salt/states/archive.py

@@ -966,7 +966,7 @@ def extracted(name,
 
         if result['result']:
             # Get the path of the file in the minion cache
-            cached = __salt__['cp.is_cached'](source_match)
+            cached = __salt__['cp.is_cached'](source_match, saltenv=__env__)
         else:
             log.debug(
                 'failed to download %s',

salt/states/file.py

@@ -6628,7 +6628,7 @@ def cached(name,
 
     .. code-block:: python
 
-        cached = __salt__['cp.is_cached'](source_match)
+        cached = __salt__['cp.is_cached'](source_match, saltenv=__env__)
 
     This function will return the cached path of the file, or an empty string
     if the file is not present in the minion cache.

salt/states/pkgrepo.py

@@ -417,6 +417,9 @@ def managed(name, ppa=None, **kwargs):
         repo = salt.utils.pkg.deb.strip_uri(repo)
 
     if pre:
+        #22412: Remove file attribute in case same repo is set up multiple times but with different files
+        pre.pop('file', None)
+        sanitizedkwargs.pop('file', None)
         for kwarg in sanitizedkwargs:
             if kwarg not in pre:
                 if kwarg == 'enabled':

salt/utils/gitfs.py

@@ -2749,15 +2749,20 @@ class GitFS(GitBase):
                 return fnd
 
             salt.fileserver.wait_lock(lk_fn, dest)
-            if os.path.isfile(blobshadest) and os.path.isfile(dest):
+            try:
                 with salt.utils.files.fopen(blobshadest, 'r') as fp_:
                     sha = salt.utils.stringutils.to_unicode(fp_.read())
                     if sha == blob_hexsha:
                         fnd['rel'] = path
                         fnd['path'] = dest
                         return _add_file_stat(fnd, blob_mode)
+            except IOError as exc:
+                if exc.errno != errno.ENOENT:
+                    raise exc
+
             with salt.utils.files.fopen(lk_fn, 'w'):
                 pass
+
             for filename in glob.glob(hashes_glob):
                 try:
                     os.remove(filename)
@@ -2829,17 +2834,24 @@ class GitFS(GitBase):
                                         load['saltenv'],
                                         '{0}.hash.{1}'.format(relpath,
                                                               self.opts['hash_type']))
-        if not os.path.isfile(hashdest):
-            if not os.path.exists(os.path.dirname(hashdest)):
-                os.makedirs(os.path.dirname(hashdest))
-            ret['hsum'] = salt.utils.hashutils.get_hash(path, self.opts['hash_type'])
-            with salt.utils.files.fopen(hashdest, 'w+') as fp_:
-                fp_.write(ret['hsum'])
-            return ret
-        else:
+        try:
             with salt.utils.files.fopen(hashdest, 'rb') as fp_:
                 ret['hsum'] = fp_.read()
             return ret
+        except IOError as exc:
+            if exc.errno != errno.ENOENT:
+                raise exc
+
+        try:
+            os.makedirs(os.path.dirname(hashdest))
+        except OSError as exc:
+            if exc.errno != errno.EEXIST:
+                raise exc
+
+        ret['hsum'] = salt.utils.hashutils.get_hash(path, self.opts['hash_type'])
+        with salt.utils.files.fopen(hashdest, 'w+') as fp_:
+            fp_.write(ret['hsum'])
+        return ret
 
     def _file_lists(self, load, form):
         '''

salt/utils/http.py

@@ -567,7 +567,7 @@ def query(url,
                     not isinstance(result_text, six.text_type):
                 result_text = result_text.decode(res_params['charset'])
         ret['body'] = result_text
-        if 'Set-Cookie' in result_headers.keys() and cookies is not None:
+        if 'Set-Cookie' in result_headers and cookies is not None:
             result_cookies = parse_cookie_header(result_headers['Set-Cookie'])
             for item in result_cookies:
                 sess_cookies.set_cookie(item)
@@ -897,12 +897,10 @@ def parse_cookie_header(header):
     for cookie in cookies:
         name = None
         value = None
-        for item in cookie:
+        for item in list(cookie):
             if item in attribs:
                 continue
-            name = item
-            value = cookie[item]
-            del cookie[name]
+            value = cookie.pop(item)
 
         # cookielib.Cookie() requires an epoch
         if 'expires' in cookie:
@@ -910,7 +908,7 @@ def parse_cookie_header(header):
 
         # Fill in missing required fields
         for req in reqd:
-            if req not in cookie.keys():
+            if req not in cookie:
                 cookie[req] = ''
         if cookie['version'] == '':
             cookie['version'] = 0

salt/utils/win_functions.py

@@ -5,6 +5,7 @@ missing functions in other modules
 '''
 from __future__ import absolute_import, print_function, unicode_literals
 import platform
+import re
 
 # Import Salt Libs
 from salt.exceptions import CommandExecutionError
@@ -168,3 +169,53 @@ def enable_ctrl_logoff_handler():
             lambda event: True if event == ctrl_logoff_event else False,
             1
         )
+
+
+def escape_argument(arg):
+    '''
+    Escape the argument for the cmd.exe shell.
+    See http://blogs.msdn.com/b/twistylittlepassagesallalike/archive/2011/04/23/everyone-quotes-arguments-the-wrong-way.aspx
+
+    First we escape the quote chars to produce a argument suitable for
+    CommandLineToArgvW. We don't need to do this for simple arguments.
+
+    Args:
+        arg (str): a single command line argument to escape for the cmd.exe shell
+
+    Returns:
+        str: an escaped string suitable to be passed as a program argument to the cmd.exe shell
+    '''
+    if not arg or re.search(r'(["\s])', arg):
+        arg = '"' + arg.replace('"', r'\"') + '"'
+
+    return escape_for_cmd_exe(arg)
+
+
+def escape_for_cmd_exe(arg):
+    '''
+    Escape an argument string to be suitable to be passed to
+    cmd.exe on Windows
+
+    This method takes an argument that is expected to already be properly
+    escaped for the receiving program to be properly parsed. This argument
+    will be further escaped to pass the interpolation performed by cmd.exe
+    unchanged.
+
+    Any meta-characters will be escaped, removing the ability to e.g. use
+    redirects or variables.
+
+    Args:
+        arg (str): a single command line argument to escape for cmd.exe
+
+    Returns:
+        str: an escaped string suitable to be passed as a program argument to cmd.exe
+    '''
+    meta_chars = '()%!^"<>&|'
+    meta_re = re.compile('(' + '|'.join(re.escape(char) for char in list(meta_chars)) + ')')
+    meta_map = {char: "^{0}".format(char) for char in meta_chars}
+
+    def escape_meta_chars(m):
+        char = m.group(1)
+        return meta_map[char]
+
+    return meta_re.sub(escape_meta_chars, arg)

salt/utils/yamlloader.py

@@ -30,7 +30,7 @@ warnings.simplefilter('always', category=DuplicateKeyWarning)
 
 
 # with code integrated from https://gist.github.com/844388
-class SaltYamlSafeLoader(yaml.SafeLoader, object):
+class SaltYamlSafeLoader(yaml.SafeLoader):
     '''
     Create a custom YAML loader that uses the custom constructor. This allows
     for the YAML loading defaults to be manipulated based on needs within salt

tests/integration/files/file/prod/issue45893/init.sls

@@ -0,0 +1,5 @@
+test_non_base_env:
+  archive.extracted:
+    - name: {{ pillar['issue45893.name'] }}
+    - source: salt://issue45893/custom.tar.gz
+    - keep: False

tests/integration/modules/test_state.py

@@ -1367,6 +1367,20 @@ class StateModuleTest(ModuleCase, SaltReturnAssertsMixin):
         expected_result = 'Command "echo "Success!"" run'
         self.assertIn(expected_result, test_data)
 
+    def test_onchanges_requisite_with_duration(self):
+        '''
+        Tests a simple state using the onchanges requisite
+        the state will not run but results will include duration
+        '''
+
+        # Only run the state once and keep the return data
+        state_run = self.run_function('state.sls', mods='requisites.onchanges_simple')
+
+        # Then, test the result of the state run when changes are not expected to happen
+        # and ensure duration is included in the results
+        test_data = state_run['cmd_|-test_non_changing_state_|-echo "Should not run"_|-run']
+        self.assertIn('duration', test_data)
+
     # onfail tests
 
     def test_onfail_requisite(self):
@@ -1438,6 +1452,18 @@ class StateModuleTest(ModuleCase, SaltReturnAssertsMixin):
         expected_result = 'State was not run because onfail req did not change'
         self.assertIn(expected_result, test_data)
 
+    def test_onfail_requisite_with_duration(self):
+        '''
+        Tests a simple state using the onfail requisite
+        '''
+
+        # Only run the state once and keep the return data
+        state_run = self.run_function('state.sls', mods='requisites.onfail_simple')
+
+        # Then, test the result of the state run when a failure is not expected to happen
+        test_data = state_run['cmd_|-test_non_failing_state_|-echo "Should not run"_|-run']
+        self.assertIn('duration', test_data)
+
     # listen tests
 
     def test_listen_requisite(self):

tests/integration/states/test_archive.py

@@ -68,6 +68,16 @@ class ArchiveTest(ModuleCase, SaltReturnAssertsMixin):
         log.debug('Checking for extracted file: %s', path)
         self.assertTrue(os.path.isfile(path))
 
+    def run_function(self, *args, **kwargs):
+        ret = super(ArchiveTest, self).run_function(*args, **kwargs)
+        log.debug('ret = %s', ret)
+        return ret
+
+    def run_state(self, *args, **kwargs):
+        ret = super(ArchiveTest, self).run_state(*args, **kwargs)
+        log.debug('ret = %s', ret)
+        return ret
+
     def test_archive_extracted_skip_verify(self):
         '''
         test archive.extracted with skip_verify
@@ -75,7 +85,6 @@ class ArchiveTest(ModuleCase, SaltReturnAssertsMixin):
         ret = self.run_state('archive.extracted', name=ARCHIVE_DIR,
                              source=self.archive_tar_source, archive_format='tar',
                              skip_verify=True)
-        log.debug('ret = %s', ret)
         if 'Timeout' in ret:
             self.skipTest('Timeout talking to local tornado server.')
         self.assertSaltTrueReturn(ret)
@@ -91,7 +100,6 @@ class ArchiveTest(ModuleCase, SaltReturnAssertsMixin):
         ret = self.run_state('archive.extracted', name=ARCHIVE_DIR,
                              source=self.archive_tar_source, archive_format='tar',
                              source_hash=ARCHIVE_TAR_HASH)
-        log.debug('ret = %s', ret)
         if 'Timeout' in ret:
             self.skipTest('Timeout talking to local tornado server.')
 
@@ -111,7 +119,6 @@ class ArchiveTest(ModuleCase, SaltReturnAssertsMixin):
                              source=self.archive_tar_source, archive_format='tar',
                              source_hash=ARCHIVE_TAR_HASH,
                              user='root', group=r_group)
-        log.debug('ret = %s', ret)
         if 'Timeout' in ret:
             self.skipTest('Timeout talking to local tornado server.')
 
@@ -128,7 +135,6 @@ class ArchiveTest(ModuleCase, SaltReturnAssertsMixin):
                              source_hash=ARCHIVE_TAR_HASH,
                              options='--strip=1',
                              enforce_toplevel=False)
-        log.debug('ret = %s', ret)
         if 'Timeout' in ret:
             self.skipTest('Timeout talking to local tornado server.')
 
@@ -145,7 +151,6 @@ class ArchiveTest(ModuleCase, SaltReturnAssertsMixin):
                              source_hash=ARCHIVE_TAR_HASH,
                              options='--strip-components=1',
                              enforce_toplevel=False)
-        log.debug('ret = %s', ret)
         if 'Timeout' in ret:
             self.skipTest('Timeout talking to local tornado server.')
 
@@ -160,7 +165,6 @@ class ArchiveTest(ModuleCase, SaltReturnAssertsMixin):
         ret = self.run_state('archive.extracted', name=ARCHIVE_DIR,
                              source=self.archive_tar_source,
                              source_hash=ARCHIVE_TAR_HASH)
-        log.debug('ret = %s', ret)
         if 'Timeout' in ret:
             self.skipTest('Timeout talking to local tornado server.')
         self.assertSaltTrueReturn(ret)
@@ -177,7 +181,6 @@ class ArchiveTest(ModuleCase, SaltReturnAssertsMixin):
                              source_hash=ARCHIVE_TAR_HASH,
                              use_cmd_unzip=False,
                              archive_format='tar')
-        log.debug('ret = %s', ret)
         if 'Timeout' in ret:
             self.skipTest('Timeout talking to local tornado server.')
         self.assertSaltTrueReturn(ret)
@@ -190,7 +193,6 @@ class ArchiveTest(ModuleCase, SaltReturnAssertsMixin):
         '''
         ret = self.run_state('archive.extracted', name=ARCHIVE_DIR,
                              source=ARCHIVE_LOCAL_TAR_SOURCE, archive_format='tar')
-        log.debug('ret = %s', ret)
 
         self.assertSaltTrueReturn(ret)
 
@@ -203,7 +205,6 @@ class ArchiveTest(ModuleCase, SaltReturnAssertsMixin):
         ret = self.run_state('archive.extracted', name=ARCHIVE_DIR,
                              source=ARCHIVE_LOCAL_TAR_SOURCE, archive_format='tar',
                              source_hash=ARCHIVE_TAR_BAD_HASH, skip_verify=True)
-        log.debug('ret = %s', ret)
 
         self.assertSaltTrueReturn(ret)
 
@@ -216,7 +217,6 @@ class ArchiveTest(ModuleCase, SaltReturnAssertsMixin):
         ret = self.run_state('archive.extracted', name=ARCHIVE_DIR,
                              source=ARCHIVE_LOCAL_TAR_SOURCE, archive_format='tar',
                              source_hash=ARCHIVE_TAR_HASH)
-        log.debug('ret = %s', ret)
 
         self.assertSaltTrueReturn(ret)
 
@@ -229,6 +229,17 @@ class ArchiveTest(ModuleCase, SaltReturnAssertsMixin):
         ret = self.run_state('archive.extracted', name=ARCHIVE_DIR,
                              source=ARCHIVE_LOCAL_TAR_SOURCE, archive_format='tar',
                              source_hash=ARCHIVE_TAR_BAD_HASH)
-        log.debug('ret = %s', ret)
 
         self.assertSaltFalseReturn(ret)
+
+    def test_archive_extracted_with_non_base_saltenv(self):
+        '''
+        test archive.extracted with a saltenv other than `base`
+        '''
+        ret = self.run_function(
+            'state.sls',
+            ['issue45893'],
+            pillar={'issue45893.name': ARCHIVE_DIR},
+            saltenv='prod')
+        self.assertSaltTrueReturn(ret)
+        self._check_extracted(os.path.join(ARCHIVE_DIR, UNTAR_FILE))

tests/unit/cache/test_localfs.py

@@ -5,6 +5,7 @@ unit tests for the localfs cache
 
 # Import Python libs
 from __future__ import absolute_import, print_function, unicode_literals
+import os
 import shutil
 import tempfile
 
@@ -45,16 +46,24 @@ class LocalFSTest(TestCase, LoaderModuleMockMixin):
             with patch.dict(localfs.__context__, {'serial': serializer}):
                 localfs.store(bank='bank', key='key', data='payload data', cachedir=tmp_dir)
 
-    # 'store' function tests: 4
+    # 'store' function tests: 5
 
-    def test_store_no_base_cache_dir(self):
+    def test_handled_exception_cache_dir(self):
         '''
         Tests that a SaltCacheError is raised when the base directory doesn't exist and
         cannot be created.
         '''
-        with patch('os.path.isdir', MagicMock(return_value=None)):
-            with patch('os.makedirs', MagicMock(side_effect=OSError)):
-                self.assertRaises(SaltCacheError, localfs.store, bank='', key='', data='', cachedir='')
+        with patch('os.makedirs', MagicMock(side_effect=OSError(os.errno.EEXIST, ''))):
+            with patch('tempfile.mkstemp', MagicMock(side_effect=Exception)):
+                self.assertRaises(Exception, localfs.store, bank='', key='', data='', cachedir='')
+
+    def test_unhandled_exception_cache_dir(self):
+        '''
+        Tests that a SaltCacheError is raised when the base directory doesn't exist and
+        cannot be created.
+        '''
+        with patch('os.makedirs', MagicMock(side_effect=OSError(1, ''))):
+            self.assertRaises(SaltCacheError, localfs.store, bank='', key='', data='', cachedir='')
 
     def test_store_close_mkstemp_file_handle(self):
         '''
@@ -64,7 +73,7 @@ class LocalFSTest(TestCase, LoaderModuleMockMixin):
         This test mocks the call to mkstemp, but forces an OSError to be raised when the
         close() function is called on a file descriptor that doesn't exist.
         '''
-        with patch('os.path.isdir', MagicMock(return_value=True)):
+        with patch('os.makedirs', MagicMock(side_effect=OSError(os.errno.EEXIST, ''))):
             with patch('tempfile.mkstemp', MagicMock(return_value=(12345, 'foo'))):
                 self.assertRaises(OSError, localfs.store, bank='', key='', data='', cachedir='')
 
@@ -73,7 +82,7 @@ class LocalFSTest(TestCase, LoaderModuleMockMixin):
         Tests that a SaltCacheError is raised when there is a problem writing to the
         cache file.
         '''
-        with patch('os.path.isdir', MagicMock(return_value=True)):
+        with patch('os.makedirs', MagicMock(side_effect=OSError(os.errno.EEXIST, ''))):
             with patch('tempfile.mkstemp', MagicMock(return_value=('one', 'two'))):
                 with patch('os.close', MagicMock(return_value=None)):
                     with patch('salt.utils.files.fopen', MagicMock(side_effect=IOError)):

tests/unit/modules/test_localemod.py

@@ -64,7 +64,7 @@ class LocalemodTestCase(TestCase, LoaderModuleMockMixin):
                         {'cmd.run': MagicMock(return_value='A\nB')}):
             assert localemod.list_avail() == ['A', 'B']
 
-    @patch('salt.utils.which', MagicMock(return_value="/usr/bin/localctl"))
+    @patch('salt.utils.path.which', MagicMock(return_value="/usr/bin/localctl"))
     @patch('salt.modules.localemod.__salt__', {'cmd.run': MagicMock(return_value=locale_ctl_out)})
     def test_localectl_status_parser(self):
         '''
@@ -119,7 +119,7 @@ class LocalemodTestCase(TestCase, LoaderModuleMockMixin):
             assert msg == ('Odd locale parameter "Fatal error right in front of screen" detected in dbus locale output.'
                            ' This should not happen. You should probably investigate what caused this.')
 
-    @patch('salt.utils.which', MagicMock(return_value=None))
+    @patch('salt.utils.path.which', MagicMock(return_value=None))
     @patch('salt.modules.localemod.log', MagicMock())
     def test_localectl_status_parser_no_systemd(self):
         '''
@@ -131,21 +131,21 @@ class LocalemodTestCase(TestCase, LoaderModuleMockMixin):
         assert 'Unable to find "localectl"' in six.text_type(err)
         assert not localemod.log.debug.called
 
-    @patch('salt.utils.which', MagicMock(return_value="/usr/bin/localctl"))
+    @patch('salt.utils.path.which', MagicMock(return_value="/usr/bin/localctl"))
     @patch('salt.modules.localemod.__salt__', {'cmd.run': MagicMock(return_value=locale_ctl_out_empty)})
     def test_localectl_status_parser_empty(self):
         with pytest.raises(CommandExecutionError) as err:
             localemod._localectl_status()
         assert 'Unable to parse result of "localectl"' in six.text_type(err)
 
-    @patch('salt.utils.which', MagicMock(return_value="/usr/bin/localctl"))
+    @patch('salt.utils.path.which', MagicMock(return_value="/usr/bin/localctl"))
     @patch('salt.modules.localemod.__salt__', {'cmd.run': MagicMock(return_value=locale_ctl_out_broken)})
     def test_localectl_status_parser_broken(self):
         with pytest.raises(CommandExecutionError) as err:
             localemod._localectl_status()
         assert 'Unable to parse result of "localectl"' in six.text_type(err)
 
-    @patch('salt.utils.which', MagicMock(return_value="/usr/bin/localctl"))
+    @patch('salt.utils.path.which', MagicMock(return_value="/usr/bin/localctl"))
     @patch('salt.modules.localemod.__salt__', {'cmd.run': MagicMock(return_value=locale_ctl_out_structure)})
     def test_localectl_status_parser_structure(self):
         out = localemod._localectl_status()
@@ -156,7 +156,7 @@ class LocalemodTestCase(TestCase, LoaderModuleMockMixin):
                 assert isinstance(out[key][in_key], six.text_type)
         assert isinstance(out['reason'], six.text_type)
 
-    @patch('salt.utils.which', MagicMock(return_value="/usr/bin/localctl"))
+    @patch('salt.utils.path.which', MagicMock(return_value="/usr/bin/localctl"))
     @patch('salt.modules.localemod.__grains__', {'os_family': 'Ubuntu', 'osmajorrelease': 42})
     @patch('salt.modules.localemod.dbus', None)
     @patch('salt.modules.localemod._parse_dbus_locale', MagicMock(return_value={'LANG': 'en_US.utf8'}))
@@ -169,7 +169,7 @@ class LocalemodTestCase(TestCase, LoaderModuleMockMixin):
         '''
         assert localemod.get_locale() == 'de_DE.utf8'
 
-    @patch('salt.utils.which', MagicMock(return_value="/usr/bin/localctl"))
+    @patch('salt.utils.path.which', MagicMock(return_value="/usr/bin/localctl"))
     @patch('salt.modules.localemod.__grains__', {'os_family': 'Ubuntu', 'osmajorrelease': 42})
     @patch('salt.modules.localemod.dbus', True)
     @patch('salt.modules.localemod._parse_dbus_locale', MagicMock(return_value={'LANG': 'en_US.utf8'}))
@@ -182,7 +182,7 @@ class LocalemodTestCase(TestCase, LoaderModuleMockMixin):
         '''
         assert localemod.get_locale() == 'en_US.utf8'
 
-    @patch('salt.utils.which', MagicMock(return_value="/usr/bin/localctl"))
+    @patch('salt.utils.path.which', MagicMock(return_value="/usr/bin/localctl"))
     @patch('salt.modules.localemod.__grains__', {'os_family': 'Suse', 'osmajorrelease': 12})
     @patch('salt.modules.localemod.dbus', True)
     @patch('salt.modules.localemod._parse_dbus_locale', MagicMock(return_value={'LANG': 'en_US.utf8'}))
@@ -197,7 +197,7 @@ class LocalemodTestCase(TestCase, LoaderModuleMockMixin):
         localemod.get_locale()
         assert localemod.__salt__['cmd.run'].call_args[0][0] == 'grep "^RC_LANG" /etc/sysconfig/language'
 
-    @patch('salt.utils.which', MagicMock(return_value=None))
+    @patch('salt.utils.path.which', MagicMock(return_value=None))
     @patch('salt.modules.localemod.__grains__', {'os_family': 'RedHat', 'osmajorrelease': 12})
     @patch('salt.modules.localemod.dbus', None)
     @patch('salt.modules.localemod.__salt__', {'cmd.run': MagicMock()})
@@ -210,7 +210,7 @@ class LocalemodTestCase(TestCase, LoaderModuleMockMixin):
         localemod.get_locale()
         assert localemod.__salt__['cmd.run'].call_args[0][0] == 'grep "^LANG=" /etc/sysconfig/i18n'
 
-    @patch('salt.utils.which', MagicMock(return_value=None))
+    @patch('salt.utils.path.which', MagicMock(return_value=None))
     @patch('salt.modules.localemod.__grains__', {'os_family': 'Debian', 'osmajorrelease': 12})
     @patch('salt.modules.localemod.dbus', None)
     @patch('salt.modules.localemod.__salt__', {'cmd.run': MagicMock()})
@@ -223,7 +223,7 @@ class LocalemodTestCase(TestCase, LoaderModuleMockMixin):
         localemod.get_locale()
         assert localemod.__salt__['cmd.run'].call_args[0][0] == 'grep "^LANG=" /etc/default/locale'
 
-    @patch('salt.utils.which', MagicMock(return_value=None))
+    @patch('salt.utils.path.which', MagicMock(return_value=None))
     @patch('salt.modules.localemod.__grains__', {'os_family': 'Gentoo', 'osmajorrelease': 12})
     @patch('salt.modules.localemod.dbus', None)
     @patch('salt.modules.localemod.__salt__', {'cmd.run': MagicMock()})
@@ -236,7 +236,7 @@ class LocalemodTestCase(TestCase, LoaderModuleMockMixin):
         localemod.get_locale()
         assert localemod.__salt__['cmd.run'].call_args[0][0] == 'eselect --brief locale show'
 
-    @patch('salt.utils.which', MagicMock(return_value=None))
+    @patch('salt.utils.path.which', MagicMock(return_value=None))
     @patch('salt.modules.localemod.__grains__', {'os_family': 'Solaris', 'osmajorrelease': 12})
     @patch('salt.modules.localemod.dbus', None)
     @patch('salt.modules.localemod.__salt__', {'cmd.run': MagicMock()})
@@ -249,7 +249,7 @@ class LocalemodTestCase(TestCase, LoaderModuleMockMixin):
         localemod.get_locale()
         assert localemod.__salt__['cmd.run'].call_args[0][0] == 'grep "^LANG=" /etc/default/init'
 
-    @patch('salt.utils.which', MagicMock(return_value=None))
+    @patch('salt.utils.path.which', MagicMock(return_value=None))
     @patch('salt.modules.localemod.__grains__', {'os_family': 'BSD', 'osmajorrelease': 8, 'oscodename': 'DrunkDragon'})
     @patch('salt.modules.localemod.dbus', None)
     @patch('salt.modules.localemod.__salt__', {'cmd.run': MagicMock()})
@@ -263,7 +263,7 @@ class LocalemodTestCase(TestCase, LoaderModuleMockMixin):
             localemod.get_locale()
         assert '"DrunkDragon" is unsupported' in six.text_type(err)
 
-    @patch('salt.utils.which', MagicMock(return_value="/usr/bin/localctl"))
+    @patch('salt.utils.path.which', MagicMock(return_value="/usr/bin/localctl"))
     @patch('salt.modules.localemod.__grains__', {'os_family': 'Ubuntu', 'osmajorrelease': 42})
     @patch('salt.modules.localemod.dbus', None)
     @patch('salt.utils.systemd.booted', MagicMock(return_value=True))
@@ -277,7 +277,7 @@ class LocalemodTestCase(TestCase, LoaderModuleMockMixin):
         localemod.set_locale(loc)
         assert localemod._localectl_set.call_args[0][0] == 'de_DE.utf8'
 
-    @patch('salt.utils.which', MagicMock(return_value="/usr/bin/localctl"))
+    @patch('salt.utils.path.which', MagicMock(return_value="/usr/bin/localctl"))
     @patch('salt.modules.localemod.__grains__', {'os_family': 'Ubuntu', 'osmajorrelease': 42})
     @patch('salt.modules.localemod.dbus', True)
     @patch('salt.utils.systemd.booted', MagicMock(return_value=True))
@@ -291,7 +291,7 @@ class LocalemodTestCase(TestCase, LoaderModuleMockMixin):
         localemod.set_locale(loc)
         assert localemod._localectl_set.call_args[0][0] == 'de_DE.utf8'
 
-    @patch('salt.utils.which', MagicMock(return_value="/usr/bin/localctl"))
+    @patch('salt.utils.path.which', MagicMock(return_value="/usr/bin/localctl"))
     @patch('salt.modules.localemod.__grains__', {'os_family': 'Suse', 'osmajorrelease': 12})
     @patch('salt.modules.localemod.dbus', True)
     @patch('salt.modules.localemod.__salt__', MagicMock())
@@ -310,7 +310,7 @@ class LocalemodTestCase(TestCase, LoaderModuleMockMixin):
         assert localemod.__salt__['file.replace'].call_args[0][1] == '^RC_LANG=.*'
         assert localemod.__salt__['file.replace'].call_args[0][2] == 'RC_LANG="{}"'.format(loc)
 
-    @patch('salt.utils.which', MagicMock(return_value=None))
+    @patch('salt.utils.path.which', MagicMock(return_value=None))
     @patch('salt.modules.localemod.__grains__', {'os_family': 'RedHat', 'osmajorrelease': 42})
     @patch('salt.modules.localemod.dbus', None)
     @patch('salt.modules.localemod.__salt__', MagicMock())
@@ -329,7 +329,6 @@ class LocalemodTestCase(TestCase, LoaderModuleMockMixin):
         assert localemod.__salt__['file.replace'].call_args[0][1] == '^LANG=.*'
         assert localemod.__salt__['file.replace'].call_args[0][2] == 'LANG="{}"'.format(loc)
 
-    @patch('salt.utils.which', MagicMock(return_value=None))
     @patch('salt.utils.path.which', MagicMock(return_value='/usr/sbin/update-locale'))
     @patch('salt.modules.localemod.__grains__', {'os_family': 'Debian', 'osmajorrelease': 42})
     @patch('salt.modules.localemod.dbus', None)
@@ -349,7 +348,7 @@ class LocalemodTestCase(TestCase, LoaderModuleMockMixin):
         assert localemod.__salt__['file.replace'].call_args[0][1] == '^LANG=.*'
         assert localemod.__salt__['file.replace'].call_args[0][2] == 'LANG="{}"'.format(loc)
 
-    @patch('salt.utils.which', MagicMock(return_value=None))
+    @patch('salt.utils.path.which', MagicMock(return_value=None))
     @patch('salt.utils.path.which', MagicMock(return_value=None))
     @patch('salt.modules.localemod.__grains__', {'os_family': 'Debian', 'osmajorrelease': 42})
     @patch('salt.modules.localemod.dbus', None)
@@ -367,7 +366,7 @@ class LocalemodTestCase(TestCase, LoaderModuleMockMixin):
         assert not localemod._localectl_set.called
         assert 'Cannot set locale: "update-locale" was not found.' in six.text_type(err)
 
-    @patch('salt.utils.which', MagicMock(return_value=None))
+    @patch('salt.utils.path.which', MagicMock(return_value=None))
     @patch('salt.modules.localemod.__grains__', {'os_family': 'Gentoo', 'osmajorrelease': 42})
     @patch('salt.modules.localemod.dbus', None)
     @patch('salt.modules.localemod.__salt__', MagicMock())
@@ -383,7 +382,7 @@ class LocalemodTestCase(TestCase, LoaderModuleMockMixin):
         assert not localemod._localectl_set.called
         assert localemod.__salt__['cmd.retcode'].call_args[0][0] == 'eselect --brief locale set de_DE.utf8'
 
-    @patch('salt.utils.which', MagicMock(return_value=None))
+    @patch('salt.utils.path.which', MagicMock(return_value=None))
     @patch('salt.modules.localemod.__grains__', {'os_family': 'Solaris', 'osmajorrelease': 42})
     @patch('salt.modules.localemod.dbus', None)
     @patch('salt.modules.localemod.__salt__', {'locale.list_avail': MagicMock(return_value=['de_DE.utf8']),
@@ -404,7 +403,7 @@ class LocalemodTestCase(TestCase, LoaderModuleMockMixin):
         assert localemod.__salt__['file.replace'].call_args[0][1] == '^LANG=.*'
         assert localemod.__salt__['file.replace'].call_args[0][2] == 'LANG="{}"'.format(loc)
 
-    @patch('salt.utils.which', MagicMock(return_value=None))
+    @patch('salt.utils.path.which', MagicMock(return_value=None))
     @patch('salt.modules.localemod.__grains__', {'os_family': 'Solaris', 'osmajorrelease': 42})
     @patch('salt.modules.localemod.dbus', None)
     @patch('salt.modules.localemod.__salt__', {'locale.list_avail': MagicMock(return_value=['en_GB.utf8']),
@@ -422,7 +421,7 @@ class LocalemodTestCase(TestCase, LoaderModuleMockMixin):
         assert not localemod._localectl_set.called
         assert not localemod.__salt__['file.replace'].called
 
-    @patch('salt.utils.which', MagicMock(return_value=None))
+    @patch('salt.utils.path.which', MagicMock(return_value=None))
     @patch('salt.modules.localemod.__grains__', {'os_family': 'BSD', 'osmajorrelease': 42})
     @patch('salt.modules.localemod.dbus', None)
     @patch('salt.modules.localemod.__salt__', {'locale.list_avail': MagicMock(return_value=['en_GB.utf8']),

tests/unit/utils/test_win_functions.py

@@ -0,0 +1,53 @@
+# -*- coding: utf-8 -*-
+
+# Import Python Libs
+from __future__ import absolute_import, unicode_literals, print_function
+
+# Import Salt Testing Libs
+from tests.support.unit import TestCase, skipIf
+from tests.support.mock import (
+    NO_MOCK,
+    NO_MOCK_REASON
+)
+
+# Import Salt Libs
+import salt.utils.win_functions as win_functions
+
+
+@skipIf(NO_MOCK, NO_MOCK_REASON)
+class WinFunctionsTestCase(TestCase):
+    '''
+    Test cases for salt.utils.win_functions
+    '''
+
+    def test_escape_argument_simple(self):
+        '''
+        Test to make sure we encode simple arguments correctly
+        '''
+        encoded = win_functions.escape_argument('simple')
+
+        self.assertEqual(encoded, 'simple')
+
+    def test_escape_argument_with_space(self):
+        '''
+        Test to make sure we encode arguments containing spaces correctly
+        '''
+        encoded = win_functions.escape_argument('with space')
+
+        self.assertEqual(encoded, '^"with space^"')
+
+    def test_escape_argument_simple_path(self):
+        '''
+        Test to make sure we encode simple path arguments correctly
+        '''
+        encoded = win_functions.escape_argument('C:\\some\\path')
+
+        self.assertEqual(encoded, 'C:\\some\\path')
+
+    def test_escape_argument_path_with_space(self):
+        '''
+        Test to make sure we encode path arguments containing spaces correctly
+        '''
+        encoded = win_functions.escape_argument('C:\\Some Path\\With Spaces')
+
+        self.assertEqual(encoded, '^"C:\\Some Path\\With Spaces^"')