Merge branch '2016.3' into '2016.11'

Conflicts:
  - salt/utils/__init__.py
  - salt/utils/gitfs.py
  - tests/unit/modules/ssh_test.py

salt/utils/__init__.py

@@ -18,6 +18,7 @@ import json
 import logging
 import numbers
 import os
+import posixpath
 import random
 import re
 import shlex
@@ -40,7 +41,6 @@ from salt.ext.six.moves.urllib.parse import urlparse  # pylint: disable=no-name-
 # pylint: disable=redefined-builtin
 from salt.ext.six.moves import range
 from salt.ext.six.moves import zip
-from salt.ext.six.moves import map
 from stat import S_IMODE
 # pylint: enable=import-error,redefined-builtin
 
@@ -899,21 +899,32 @@ def backup_minion(path, bkroot):
         os.chmod(bkpath, fstat.st_mode)
 
 
-def path_join(*parts):
+def path_join(*parts, **kwargs):
     '''
     This functions tries to solve some issues when joining multiple absolute
     paths on both *nix and windows platforms.
 
     See tests/unit/utils/path_join_test.py for some examples on what's being
     talked about here.
+
+    The "use_posixpath" kwarg can be be used to force joining using poxixpath,
+    which is useful for Salt fileserver paths on Windows masters.
     '''
     if six.PY3:
         new_parts = []
         for part in parts:
             new_parts.append(to_str(part))
         parts = new_parts
+
+    kwargs = salt.utils.clean_kwargs(**kwargs)
+    use_posixpath = kwargs.pop('use_posixpath', False)
+    if kwargs:
+        invalid_kwargs(kwargs)
+
+    pathlib = posixpath if use_posixpath else os.path
+
     # Normalize path converting any os.sep as needed
-    parts = [os.path.normpath(p) for p in parts]
+    parts = [pathlib.normpath(p) for p in parts]
 
     try:
         root = parts.pop(0)
@@ -924,14 +935,9 @@ def path_join(*parts):
     if not parts:
         ret = root
     else:
-        if is_windows():
-            if len(root) == 1:
-                root += ':'
-            root = root.rstrip(os.sep) + os.sep
-
         stripped = [p.lstrip(os.sep) for p in parts]
         try:
-            ret = os.path.join(root, *stripped)
+            ret = pathlib.join(root, *stripped)
         except UnicodeDecodeError:
             # This is probably Python 2 and one of the parts contains unicode
             # characters in a bytestring. First try to decode to the system
@@ -941,13 +947,13 @@ def path_join(*parts):
             except NameError:
                 enc = sys.stdin.encoding or sys.getdefaultencoding()
             try:
-                ret = os.path.join(root.decode(enc),
+                ret = pathlib.join(root.decode(enc),
                                    *[x.decode(enc) for x in stripped])
             except UnicodeDecodeError:
                 # Last resort, try UTF-8
-                ret = os.path.join(root.decode('UTF-8'),
+                ret = pathlib.join(root.decode('UTF-8'),
                                    *[x.decode('UTF-8') for x in stripped])
-    return os.path.normpath(ret)
+    return pathlib.normpath(ret)
 
 
 def pem_finger(path=None, key=None, sum_type='sha256'):

salt/utils/gitfs.py

@@ -863,7 +863,8 @@ class GitPython(GitProvider):
             relpath = lambda path: os.path.relpath(path, self.root(tgt_env))
         else:
             relpath = lambda path: path
-        add_mountpoint = lambda path: salt.utils.path_join(self.mountpoint(tgt_env), path)
+        add_mountpoint = lambda path: salt.utils.path_join(
+            self.mountpoint(tgt_env), path, use_posixpath=True)
         for blob in tree.traverse():
             if isinstance(blob, git.Tree):
                 ret.add(add_mountpoint(relpath(blob.path)))
@@ -939,7 +940,8 @@ class GitPython(GitProvider):
             relpath = lambda path: os.path.relpath(path, self.root(tgt_env))
         else:
             relpath = lambda path: path
-        add_mountpoint = lambda path: salt.utils.path_join(self.mountpoint(tgt_env), path)
+        add_mountpoint = lambda path: salt.utils.path_join(
+            self.mountpoint(tgt_env), path, use_posixpath=True)
         for file_blob in tree.traverse():
             if not isinstance(file_blob, git.Blob):
                 continue
@@ -981,7 +983,8 @@ class GitPython(GitProvider):
                     stream.seek(0)
                     link_tgt = stream.read()
                     stream.close()
-                    path = salt.utils.path_join(os.path.dirname(path), link_tgt)
+                    path = salt.utils.path_join(
+                        os.path.dirname(path), link_tgt, use_posixpath=True)
                 else:
                     blob = file_blob
                     if isinstance(blob, git.Tree):
@@ -1355,9 +1358,14 @@ class Pygit2(GitProvider):
                 blob = self.repo[entry.oid]
                 if not isinstance(blob, pygit2.Tree):
                     continue
-                blobs.append(salt.utils.path_join(prefix, entry.name))
+                blobs.append(
+                    salt.utils.path_join(prefix, entry.name, use_posixpath=True)
+                )
                 if len(blob):
-                    _traverse(blob, blobs, salt.utils.path_join(prefix, entry.name))
+                    _traverse(
+                        blob, blobs, salt.utils.path_join(
+                            prefix, entry.name, use_posixpath=True)
+                    )
 
         ret = set()
         tree = self.get_tree(tgt_env)
@@ -1377,7 +1385,8 @@ class Pygit2(GitProvider):
         blobs = []
         if len(tree):
             _traverse(tree, blobs, self.root(tgt_env))
-        add_mountpoint = lambda path: salt.utils.path_join(self.mountpoint(tgt_env), path)
+        add_mountpoint = lambda path: salt.utils.path_join(
+            self.mountpoint(tgt_env), path, use_posixpath=True)
         for blob in blobs:
             ret.add(add_mountpoint(relpath(blob)))
         if self.mountpoint(tgt_env):
@@ -1467,13 +1476,17 @@ class Pygit2(GitProvider):
                     continue
                 obj = self.repo[entry.oid]
                 if isinstance(obj, pygit2.Blob):
-                    repo_path = salt.utils.path_join(prefix, entry.name)
+                    repo_path = salt.utils.path_join(
+                        prefix, entry.name, use_posixpath=True)
                     blobs.setdefault('files', []).append(repo_path)
                     if stat.S_ISLNK(tree[entry.name].filemode):
                         link_tgt = self.repo[tree[entry.name].oid].data
                         blobs.setdefault('symlinks', {})[repo_path] = link_tgt
                 elif isinstance(obj, pygit2.Tree):
-                    _traverse(obj, blobs, salt.utils.path_join(prefix, entry.name))
+                    _traverse(
+                        obj, blobs, salt.utils.path_join(
+                            prefix, entry.name, use_posixpath=True)
+                    )
 
         files = set()
         symlinks = {}
@@ -1497,7 +1510,8 @@ class Pygit2(GitProvider):
         blobs = {}
         if len(tree):
             _traverse(tree, blobs, self.root(tgt_env))
-        add_mountpoint = lambda path: salt.utils.path_join(self.mountpoint(tgt_env), path)
+        add_mountpoint = lambda path: salt.utils.path_join(
+            self.mountpoint(tgt_env), path, use_posixpath=True)
         for repo_path in blobs.get('files', []):
             files.add(add_mountpoint(relpath(repo_path)))
         for repo_path, link_tgt in six.iteritems(blobs.get('symlinks', {})):
@@ -1529,7 +1543,8 @@ class Pygit2(GitProvider):
                     # the symlink and set path to the location indicated
                     # in the blob data.
                     link_tgt = self.repo[entry.oid].data
-                    path = salt.utils.path_join(os.path.dirname(path), link_tgt)
+                    path = salt.utils.path_join(
+                        os.path.dirname(path), link_tgt, use_posixpath=True)
                 else:
                     blob = self.repo[entry.oid]
                     if isinstance(blob, pygit2.Tree):
@@ -1738,9 +1753,14 @@ class Dulwich(GitProvider):  # pylint: disable=abstract-method
                     continue
                 if not isinstance(obj, dulwich.objects.Tree):
                     continue
-                blobs.append(salt.utils.path_join(prefix, item.path))
+                blobs.append(
+                    salt.utils.path_join(prefix, item.path, use_posixpath=True)
+                )
                 if len(self.repo.get_object(item.sha)):
-                    _traverse(obj, blobs, salt.utils.path_join(prefix, item.path))
+                    _traverse(
+                        obj, blobs, salt.utils.path_join(
+                            prefix, item.path, use_posixpath=True)
+                    )
 
         ret = set()
         tree = self.get_tree(tgt_env)
@@ -1754,7 +1774,8 @@ class Dulwich(GitProvider):  # pylint: disable=abstract-method
             relpath = lambda path: os.path.relpath(path, self.root(tgt_env))
         else:
             relpath = lambda path: path
-        add_mountpoint = lambda path: salt.utils.path_join(self.mountpoint(tgt_env), path)
+        add_mountpoint = lambda path: salt.utils.path_join(
+            self.mountpoint(tgt_env), path, use_posixpath=True)
         for blob in blobs:
             ret.add(add_mountpoint(relpath(blob)))
         if self.mountpoint(tgt_env):
@@ -1854,14 +1875,18 @@ class Dulwich(GitProvider):  # pylint: disable=abstract-method
                     # Entry is a submodule, skip it
                     continue
                 if isinstance(obj, dulwich.objects.Blob):
-                    repo_path = salt.utils.path_join(prefix, item.path)
+                    repo_path = salt.utils.path_join(
+                        prefix, item.path, use_posixpath=True)
                     blobs.setdefault('files', []).append(repo_path)
                     mode, oid = tree[item.path]
                     if stat.S_ISLNK(mode):
                         link_tgt = self.repo.get_object(oid).as_raw_string()
                         blobs.setdefault('symlinks', {})[repo_path] = link_tgt
                 elif isinstance(obj, dulwich.objects.Tree):
-                    _traverse(obj, blobs, salt.utils.path_join(prefix, item.path))
+                    _traverse(
+                        obj, blobs, salt.utils.path_join(
+                            prefix, item.path, use_posixpath=True)
+                    )
 
         files = set()
         symlinks = {}
@@ -1876,7 +1901,8 @@ class Dulwich(GitProvider):  # pylint: disable=abstract-method
             relpath = lambda path: os.path.relpath(path, self.root(tgt_env))
         else:
             relpath = lambda path: path
-        add_mountpoint = lambda path: salt.utils.path_join(self.mountpoint(tgt_env), path)
+        add_mountpoint = lambda path: salt.utils.path_join(
+            self.mountpoint(tgt_env), path, use_posixpath=True)
         for repo_path in blobs.get('files', []):
             files.add(add_mountpoint(relpath(repo_path)))
         for repo_path, link_tgt in six.iteritems(blobs.get('symlinks', {})):
@@ -1912,7 +1938,8 @@ class Dulwich(GitProvider):  # pylint: disable=abstract-method
                     # symlink. Follow the symlink and set path to the
                     # location indicated in the blob data.
                     link_tgt = self.repo.get_object(oid).as_raw_string()
-                    path = salt.utils.path_join(os.path.dirname(path), link_tgt)
+                    path = salt.utils.path_join(
+                        os.path.dirname(path), link_tgt, use_posixpath=True)
                 else:
                     blob = self.repo.get_object(oid)
                     if isinstance(blob, dulwich.objects.Tree):
@@ -2110,10 +2137,10 @@ class GitBase(object):
         if cache_root is not None:
             self.cache_root = cache_root
         else:
-            self.cache_root = salt.utils.path_join(self.opts['cachedir'], self.role)
+            self.cache_root = salt.utils.path_join(
+                self.opts['cachedir'], self.role)
         self.env_cache = salt.utils.path_join(self.cache_root, 'envs.p')
-        self.hash_cachedir = salt.utils.path_join(
+        self.hash_cachedir = salt.utils.path_join(self.cache_root, 'hash')
-            self.cache_root, 'hash')
         self.file_list_cachedir = salt.utils.path_join(
             self.opts['cachedir'], 'file_lists', self.role)
 

tests/unit/modules/ssh_test.py

@@ -2,19 +2,25 @@
 
 # import Python Libs
 from __future__ import absolute_import
+import tempfile
 
 # Import Salt Testing Libs
 from salttesting import skipIf, TestCase
 from salttesting.helpers import ensure_in_syspath
 from salttesting.mock import (
+    MagicMock,
     NO_MOCK,
-    NO_MOCK_REASON
+    NO_MOCK_REASON,
+    patch
 )
 
 # Import Salt Libs
 ensure_in_syspath('../../')
 from salt.modules import ssh
 from salt.exceptions import CommandExecutionError
+import salt.utils
+
+ssh.__salt__ = {}
 
 
 @skipIf(NO_MOCK, NO_MOCK_REASON)
@@ -61,6 +67,64 @@ class SSHAuthKeyTestCase(TestCase):
         invalid_key = 'AAAAB3NzaC1kc3MAAACBAL0sQ9fJ5bYTEyY'  # missing padding
         self.assertEqual(ssh.set_auth_key('user', invalid_key), 'Invalid public key')
 
+    def test_replace_auth_key(self):
+        '''
+        Test the _replace_auth_key with some different authorized_keys examples
+        '''
+        # First test a known working example, gathered from the authorized_keys file
+        # in the integration test files.
+        enc = 'ssh-rsa'
+        key = 'AAAAB3NzaC1yc2EAAAABIwAAAQEAq2A7hRGmdnm9tUDbO9IDSwBK6TbQa+' \
+              'PXYPCPy6rbTrTtw7PHkccKrpp0yVhp5HdEIcKr6pLlVDBfOLX9QUsyCOV0wzfjIJNl' \
+              'GEYsdlLJizHhbn2mUjvSAHQqZETYP81eFzLQNnPHt4EVVUh7VfDESU84KezmD5QlWp' \
+              'XLmvU31/yMf+Se8xhHTvKSCZIFImWwoG6mbUoWf9nzpIoaSjB+weqqUUmpaaasXVal' \
+              '72J+UX2B+2RPW3RcT0eOzQgqlJL3RKrTJvdsjE3JEAvGq3lGHSZXy28G3skua2SmVi' \
+              '/w4yCE6gbODqnTWlg7+wC604ydGXA8VJiS5ap43JXiUFFAaQ=='
+        options = 'command="/usr/local/lib/ssh-helper"'
+        email = 'github.com'
+
+        # Write out the authorized key to a temporary file
+        temp_file = tempfile.NamedTemporaryFile(delete=False, mode='w+')
+        temp_file.write('{0} {1} {2} {3}'.format(options, enc, key, email))
+        temp_file.close()
+
+        with patch.dict(ssh.__salt__, {'user.info': MagicMock(return_value={})}):
+            with patch('salt.modules.ssh._get_config_file', MagicMock(return_value=temp_file.name)):
+                ssh._replace_auth_key('foo', key, config=temp_file.name)
+
+        # The previous authorized key should have been replaced by the simpler one
+        with salt.utils.fopen(temp_file.name) as _fh:
+            file_txt = _fh.read()
+            self.assertIn(enc, file_txt)
+            self.assertIn(key, file_txt)
+            self.assertNotIn(options, file_txt)
+            self.assertNotIn(email, file_txt)
+
+        # Now test a very simple key using ecdsa instead of ssh-rsa and with multiple options
+        enc = 'ecdsa-sha2-nistp256'
+        key = 'abcxyz'
+
+        with salt.utils.fopen(temp_file.name, 'a') as _fh:
+            _fh.write('{0} {1}'.format(enc, key))
+
+        # Replace the simple key from before with the more complicated options + new email
+        # Option example is taken from Pull Request #39855
+        options = ['no-port-forwarding', 'no-agent-forwarding', 'no-X11-forwarding',
+                   'command="echo \'Please login as the user \"ubuntu\" rather than the user \"root\".\'']
+        email = 'foo@example.com'
+
+        with patch.dict(ssh.__salt__, {'user.info': MagicMock(return_value={})}):
+            with patch('salt.modules.ssh._get_config_file', MagicMock(return_value=temp_file.name)):
+                ssh._replace_auth_key('foo', key, enc=enc, comment=email, options=options, config=temp_file.name)
+
+        # Assert that the new line was added as-is to the file
+        with salt.utils.fopen(temp_file.name) as _fh:
+            file_txt = _fh.read()
+            self.assertIn(enc, file_txt)
+            self.assertIn(key, file_txt)
+            self.assertIn('{0} '.format(','.join(options)), file_txt)
+            self.assertIn(email, file_txt)
+
 
 if __name__ == '__main__':
     from integration import run_tests

tests/unit/utils/path_join_test.py

@@ -48,7 +48,6 @@ class PathJoinTestCase(TestCase):
         ((r'c:\\', r'\temp', r'\foo'), 'c:\\temp\\foo'),
         (('c:', r'\temp', r'\foo', 'bar'), 'c:\\temp\\foo\\bar'),
         (('c:', r'\temp', r'\foo\bar'), 'c:\\temp\\foo\\bar'),
-        (('c', r'\temp', r'\foo\bar'), 'c:\\temp\\foo\\bar')
     )
 
     @skipIf(True, 'Skipped until properly mocked')