Merge pull request #42599 from rallytime/merge-develop

[develop] Merge forward from 2017.7 to develop

pkg/osx/build.sh

@@ -86,9 +86,9 @@ sudo $PKGRESOURCES/build_env.sh $PYVER
 # Install Salt
 ############################################################################
 echo -n -e "\033]0;Build: Install Salt\007"
-sudo rm -rm $SRCDIR/build
-sudo rm -rm $SRCDIR/dist
-sudo $PYTHON $SRCDIR/setup.py install
+sudo rm -rf $SRCDIR/build
+sudo rm -rf $SRCDIR/dist
+sudo $PYTHON $SRCDIR/setup.py build -e "$PYTHON -E -s" install
 
 ############################################################################
 # Build Package

salt/ext/win_inet_pton.py

@@ -9,6 +9,7 @@ from __future__ import absolute_import
 import socket
 import ctypes
 import os
+import ipaddress
 
 
 class sockaddr(ctypes.Structure):
@@ -31,6 +32,24 @@ else:
 
 
 def inet_pton(address_family, ip_string):
+    # Verify IP Address
+    # This will catch IP Addresses such as 10.1.2
+    if address_family == socket.AF_INET:
+        try:
+            ipaddress.ip_address(ip_string.decode())
+        except ValueError:
+            raise socket.error('illegal IP address string passed to inet_pton')
+        return socket.inet_aton(ip_string)
+
+    # Verify IP Address
+    # The `WSAStringToAddressA` function handles notations used by Berkeley
+    # software which includes 3 part IP Addresses such as `10.1.2`. That's why
+    # the above check is needed to enforce more strict IP Address validation as
+    # used by the `inet_pton` function in Unix.
+    # See the following:
+    # https://stackoverflow.com/a/29286098
+    # Docs for the `inet_addr` function on MSDN
+    # https://msdn.microsoft.com/en-us/library/windows/desktop/ms738563.aspx
     addr = sockaddr()
     addr.sa_family = address_family
     addr_size = ctypes.c_int(ctypes.sizeof(addr))

salt/loader.py

@@ -1100,7 +1100,8 @@ class LazyLoader(salt.utils.lazy.LazyDict):
             virtual_funcs = []
         self.virtual_funcs = virtual_funcs
 
-        self.disabled = set(self.opts.get('disable_{0}s'.format(self.tag), []))
+        self.disabled = set(self.opts.get('disable_{0}{1}'.format(
+            self.tag, '' if self.tag[-1] == 's' else 's'), []))
 
         self.refresh_file_mapping()
 

salt/modules/win_system.py

@@ -37,6 +37,7 @@ except ImportError:
 import salt.utils
 import salt.utils.locales
 import salt.ext.six as six
+from salt.exceptions import CommandExecutionError
 
 # Set up logging
 log = logging.getLogger(__name__)
@@ -622,7 +623,11 @@ def join_domain(domain,
             .. versionadded:: 2015.8.2/2015.5.7
 
     Returns:
-        dict: Dictionary if successful, otherwise False
+        dict: Dictionary if successful
+
+    Raises:
+        CommandExecutionError: Raises an error if _join_domain returns anything
+            other than 0
 
     CLI Example:
 
@@ -655,6 +660,56 @@ def join_domain(domain,
         account_ou = account_ou.split('\\')
         account_ou = ''.join(account_ou)
 
+    err = _join_domain(domain=domain, username=username, password=password,
+                       account_ou=account_ou, account_exists=account_exists)
+
+    if not err:
+        ret = {'Domain': domain,
+               'Restart': False}
+        if restart:
+            ret['Restart'] = reboot()
+        return ret
+
+    raise CommandExecutionError(win32api.FormatMessage(err).rstrip())
+
+
+def _join_domain(domain,
+                 username=None,
+                 password=None,
+                 account_ou=None,
+                 account_exists=False):
+    '''
+    Helper function to join the domain.
+
+    Args:
+        domain (str): The domain to which the computer should be joined, e.g.
+            ``example.com``
+
+        username (str): Username of an account which is authorized to join
+            computers to the specified domain. Need to be either fully qualified
+            like ``user@domain.tld`` or simply ``user``
+
+        password (str): Password of the specified user
+
+        account_ou (str): The DN of the OU below which the account for this
+            computer should be created when joining the domain, e.g.
+            ``ou=computers,ou=departm_432,dc=my-company,dc=com``
+
+        account_exists (bool): If set to ``True`` the computer will only join
+            the domain if the account already exists. If set to ``False`` the
+            computer account will be created if it does not exist, otherwise it
+            will use the existing account. Default is False.
+
+    Returns:
+        int:
+
+    :param domain:
+    :param username:
+    :param password:
+    :param account_ou:
+    :param account_exists:
+    :return:
+    '''
     NETSETUP_JOIN_DOMAIN = 0x1  # pylint: disable=invalid-name
     NETSETUP_ACCOUNT_CREATE = 0x2  # pylint: disable=invalid-name
     NETSETUP_DOMAIN_JOIN_IF_JOINED = 0x20  # pylint: disable=invalid-name
@@ -670,23 +725,13 @@ def join_domain(domain,
     pythoncom.CoInitialize()
     conn = wmi.WMI()
     comp = conn.Win32_ComputerSystem()[0]
-    err = comp.JoinDomainOrWorkgroup(Name=domain,
-                                     Password=password,
-                                     UserName=username,
-                                     AccountOU=account_ou,
-                                     FJoinOptions=join_options)
 
-    # you have to do this because JoinDomainOrWorkgroup returns a strangely
-    # formatted value that looks like (0,)
-    if not err[0]:
-        ret = {'Domain': domain,
-               'Restart': False}
-        if restart:
-            ret['Restart'] = reboot()
-        return ret
-
-    log.error(win32api.FormatMessage(err[0]).rstrip())
-    return False
+    # Return the results of the command as an error
+    # JoinDomainOrWorkgroup returns a strangely formatted value that looks like
+    # (0,) so return the first item
+    return comp.JoinDomainOrWorkgroup(
+        Name=domain, Password=password, UserName=username, AccountOU=account_ou,
+        FJoinOptions=join_options)[0]
 
 
 def unjoin_domain(username=None,
@@ -919,7 +964,11 @@ def set_system_date_time(years=None,
         seconds (int): Seconds digit: 0 - 59
 
     Returns:
-        bool: True if successful, otherwise False.
+        bool: True if successful
+
+    Raises:
+        CommandExecutionError: Raises an error if ``SetLocalTime`` function
+            fails
 
     CLI Example:
 
@@ -972,12 +1021,15 @@ def set_system_date_time(years=None,
         system_time.wSecond = int(seconds)
         system_time_ptr = ctypes.pointer(system_time)
         succeeded = ctypes.windll.kernel32.SetLocalTime(system_time_ptr)
-        return succeeded is not 0
-    except OSError:
+        if succeeded is not 0:
+            return True
+        else:
+            log.error('Failed to set local time')
+            raise CommandExecutionError(
+                win32api.FormatMessage(succeeded).rstrip())
+    except OSError as err:
         log.error('Failed to set local time')
-        return False
-
-    return True
+        raise CommandExecutionError(err)
 
 
 def get_system_date():

salt/output/__init__.py

@@ -169,6 +169,13 @@ def get_printout(out, opts=None, **kwargs):
             opts['color'] = False
         else:
             opts['color'] = True
+    else:
+        if opts.get('force_color', False):
+            opts['color'] = True
+        elif opts.get('no_color', False) or salt.utils.is_windows():
+            opts['color'] = False
+        else:
+            pass
 
     outputters = salt.loader.outputters(opts)
     if out not in outputters:

salt/states/chocolatey.py

@@ -92,7 +92,7 @@ def installed(name, version=None, source=None, force=False, pre_versions=False,
 
     # Determine action
     # Package not installed
-    if name not in [package.split('|')[0].lower() for package in pre_install.splitlines()]:
+    if name.lower() not in [package.lower() for package in pre_install.keys()]:
         if version:
             ret['changes'] = {name: 'Version {0} will be installed'
                                     ''.format(version)}
@@ -193,9 +193,13 @@ def uninstalled(name, version=None, uninstall_args=None, override_args=False):
     pre_uninstall = __salt__['chocolatey.list'](local_only=True)
 
     # Determine if package is installed
-    if name in [package.split('|')[0].lower() for package in pre_uninstall.splitlines()]:
-        ret['changes'] = {name: '{0} version {1} will be removed'
-                                ''.format(name, pre_uninstall[name][0])}
+    if name.lower() in [package.lower() for package in pre_uninstall.keys()]:
+        try:
+            ret['changes'] = {name: '{0} version {1} will be removed'
+                                    ''.format(name, pre_uninstall[name][0])}
+        except KeyError:
+            ret['changes'] = {name: '{0} will be removed'
+                                    ''.format(name)}
     else:
         ret['comment'] = 'The package {0} is not installed'.format(name)
         return ret

salt/states/pkg.py

@@ -105,6 +105,7 @@ if salt.utils.is_windows():
     import datetime
     import errno
     import time
+    from functools import cmp_to_key
     # pylint: disable=import-error
     # pylint: enable=unused-import
     from salt.modules.win_pkg import _get_package_info

tests/unit/modules/test_win_system.py

@@ -67,30 +67,31 @@ class WinSystemTestCase(TestCase, LoaderModuleMockMixin):
         '''
             Test to reboot the system
         '''
-        mock = MagicMock(return_value='salt')
-        with patch.dict(win_system.__salt__, {'cmd.run': mock}):
-            self.assertEqual(win_system.reboot(), 'salt')
-            mock.assert_called_once_with(['shutdown', '/r', '/t', '300'], python_shell=False)
+        with patch('salt.modules.win_system.shutdown',
+                   MagicMock(return_value=True)) as shutdown:
+            self.assertEqual(win_system.reboot(), True)
 
     @skipIf(not win_system.HAS_WIN32NET_MODS, 'this test needs the w32net library')
     def test_reboot_with_timeout_in_minutes(self):
         '''
             Test to reboot the system with a timeout
         '''
-        mock = MagicMock(return_value='salt')
-        with patch.dict(win_system.__salt__, {'cmd.run': mock}):
-            self.assertEqual(win_system.reboot(5, in_seconds=False), 'salt')
-            mock.assert_called_once_with(['shutdown', '/r', '/t', '300'], python_shell=False)
+        with patch('salt.modules.win_system.shutdown',
+                   MagicMock(return_value=True)) as shutdown:
+            self.assertEqual(win_system.reboot(5, in_seconds=False), True)
+            shutdown.assert_called_with(timeout=5, in_seconds=False, reboot=True,
+                                        only_on_pending_reboot=False)
 
     @skipIf(not win_system.HAS_WIN32NET_MODS, 'this test needs the w32net library')
     def test_reboot_with_timeout_in_seconds(self):
         '''
             Test to reboot the system with a timeout
         '''
-        mock = MagicMock(return_value='salt')
-        with patch.dict(win_system.__salt__, {'cmd.run': mock}):
-            self.assertEqual(win_system.reboot(5, in_seconds=True), 'salt')
-            mock.assert_called_once_with(['shutdown', '/r', '/t', '5'], python_shell=False)
+        with patch('salt.modules.win_system.shutdown',
+                   MagicMock(return_value=True)) as shutdown:
+            self.assertEqual(win_system.reboot(5, in_seconds=True), True)
+            shutdown.assert_called_with(timeout=5, in_seconds=True, reboot=True,
+                                        only_on_pending_reboot=False)
 
     @skipIf(not win_system.HAS_WIN32NET_MODS, 'this test needs the w32net library')
     def test_reboot_with_wait(self):
@@ -98,50 +99,49 @@ class WinSystemTestCase(TestCase, LoaderModuleMockMixin):
             Test to reboot the system with a timeout and
             wait for it to finish
         '''
-        mock = MagicMock(return_value='salt')
-        sleep_mock = MagicMock(return_value='salt')
-        with patch.dict(win_system.__salt__, {'cmd.run': mock}):
-            with patch('time.sleep', sleep_mock):
-                self.assertEqual(win_system.reboot(wait_for_reboot=True), 'salt')
-                mock.assert_called_once_with(['shutdown', '/r', '/t', '300'], python_shell=False)
-                sleep_mock.assert_called_once_with(330)
+        with patch('salt.modules.win_system.shutdown',
+                   MagicMock(return_value=True)), \
+                patch('salt.modules.win_system.time.sleep',
+                      MagicMock()) as time:
+            self.assertEqual(win_system.reboot(wait_for_reboot=True), True)
+            time.assert_called_with(330)
 
     @skipIf(not win_system.HAS_WIN32NET_MODS, 'this test needs the w32net library')
     def test_shutdown(self):
         '''
             Test to shutdown a running system
         '''
-        mock = MagicMock(return_value='salt')
-        with patch.dict(win_system.__salt__, {'cmd.run': mock}):
-            self.assertEqual(win_system.shutdown(), 'salt')
+        with patch('salt.modules.win_system.win32api.InitiateSystemShutdown',
+                   MagicMock()):
+            self.assertEqual(win_system.shutdown(), True)
 
     @skipIf(not win_system.HAS_WIN32NET_MODS, 'this test needs the w32net library')
     def test_shutdown_hard(self):
         '''
             Test to shutdown a running system with no timeout or warning
         '''
-        mock = MagicMock(return_value='salt')
-        with patch.dict(win_system.__salt__, {'cmd.run': mock}):
-            self.assertEqual(win_system.shutdown_hard(), 'salt')
+        with patch('salt.modules.win_system.shutdown',
+                   MagicMock(return_value=True)) as shutdown:
+            self.assertEqual(win_system.shutdown_hard(), True)
+            shutdown.assert_called_with(timeout=0)
 
     @skipIf(not win_system.HAS_WIN32NET_MODS, 'this test needs the w32net library')
     def test_set_computer_name(self):
         '''
             Test to set the Windows computer name
         '''
-        mock = MagicMock(side_effect=[{'Computer Name': {'Current': ""},
-                                       'ReturnValue = 0;': True},
-                                      {'Computer Name': {'Current': 'salt'}}])
-        with patch.dict(win_system.__salt__, {'cmd.run': mock}):
-            mock = MagicMock(return_value='salt')
-            with patch.object(win_system, 'get_computer_name', mock):
-                mock = MagicMock(return_value=True)
-                with patch.object(win_system,
-                                  'get_pending_computer_name', mock):
-                    self.assertDictEqual(win_system.set_computer_name("salt"),
+        with patch('salt.modules.win_system.windll.kernel32.SetComputerNameExW',
+                   MagicMock(return_value=True)):
+            with patch.object(win_system, 'get_computer_name',
+                              MagicMock(return_value='salt')):
+                with patch.object(win_system, 'get_pending_computer_name',
+                                  MagicMock(return_value='salt_new')):
+                    self.assertDictEqual(win_system.set_computer_name("salt_new"),
                                          {'Computer Name': {'Current': 'salt',
-                                                            'Pending': True}})
-
+                                                            'Pending': 'salt_new'}})
+        # Test set_computer_name failure
+        with patch('salt.modules.win_system.windll.kernel32.SetComputerNameExW',
+                   MagicMock(return_value=False)):
             self.assertFalse(win_system.set_computer_name("salt"))
 
     @skipIf(not win_system.HAS_WIN32NET_MODS, 'this test needs the w32net library')
@@ -149,25 +149,25 @@ class WinSystemTestCase(TestCase, LoaderModuleMockMixin):
         '''
             Test to get a pending computer name.
         '''
-        mock = MagicMock(return_value='salt')
-        with patch.object(win_system, 'get_computer_name', mock):
-            mock = MagicMock(side_effect=['salt0',
-                                          'ComputerName REG_SZ (salt)'])
-            with patch.dict(win_system.__salt__, {'cmd.run': mock}):
+        with patch.object(win_system, 'get_computer_name',
+                          MagicMock(return_value='salt')):
+            reg_mock = MagicMock(return_value={'vdata': 'salt'})
+            with patch.dict(win_system.__salt__, {'reg.read_value': reg_mock}):
                 self.assertFalse(win_system.get_pending_computer_name())
 
+            reg_mock = MagicMock(return_value={'vdata': 'salt_pending'})
+            with patch.dict(win_system.__salt__, {'reg.read_value': reg_mock}):
                 self.assertEqual(win_system.get_pending_computer_name(),
-                                 '(salt)')
+                                 'salt_pending')
 
     @skipIf(not win_system.HAS_WIN32NET_MODS, 'this test needs the w32net library')
     def test_get_computer_name(self):
         '''
             Test to get the Windows computer name
         '''
-        mock = MagicMock(side_effect=['Server Name Salt', 'Salt'])
-        with patch.dict(win_system.__salt__, {'cmd.run': mock}):
-            self.assertEqual(win_system.get_computer_name(), 'Salt')
-
+        with patch('salt.modules.win_system.win32api.GetComputerNameEx',
+                   MagicMock(side_effect=['computer name', ''])):
+            self.assertEqual(win_system.get_computer_name(), 'computer name')
             self.assertFalse(win_system.get_computer_name())
 
     @skipIf(not win_system.HAS_WIN32NET_MODS, 'this test needs the w32net library')
@@ -189,10 +189,10 @@ class WinSystemTestCase(TestCase, LoaderModuleMockMixin):
         '''
             Test to get the Windows computer description
         '''
-        mock = MagicMock(side_effect=['Server Comment Salt', 'Salt'])
-        with patch.dict(win_system.__salt__, {'cmd.run': mock}):
-            self.assertEqual(win_system.get_computer_desc(), 'Salt')
-
+        with patch('salt.modules.win_system.get_system_info',
+                   MagicMock(side_effect=[{'description': 'salt description'},
+                                          {'description': None}])):
+            self.assertEqual(win_system.get_computer_desc(), 'salt description')
             self.assertFalse(win_system.get_computer_desc())
 
     @skipIf(not win_system.HAS_WIN32NET_MODS, 'this test needs w32net and other windows libraries')
@@ -200,17 +200,20 @@ class WinSystemTestCase(TestCase, LoaderModuleMockMixin):
         '''
             Test to join a computer to an Active Directory domain
         '''
-        mock = MagicMock(side_effect=[{'ReturnValue = 0;': True},
-                                      {'Salt': True}])
-        with patch.dict(win_system.__salt__, {'cmd.run': mock}):
-            self.assertDictEqual(win_system.join_domain("saltstack",
-                                                        "salt",
-                                                        "salt@123"),
-                                 {'Domain': 'saltstack'})
+        with patch('salt.modules.win_system._join_domain',
+                   MagicMock(return_value=0)):
+            with patch('salt.modules.win_system.get_domain_workgroup',
+                       MagicMock(return_value={'Workgroup': 'Workgroup'})):
+                self.assertDictEqual(
+                    win_system.join_domain(
+                        "saltstack", "salt", "salt@123"),
+                        {'Domain': 'saltstack', 'Restart': False})
 
-            self.assertFalse(win_system.join_domain("saltstack",
-                                                    "salt",
-                                                    "salt@123"))
+            with patch('salt.modules.win_system.get_domain_workgroup',
+                       MagicMock(return_value={'Domain': 'saltstack'})):
+                self.assertEqual(
+                    win_system.join_domain("saltstack", "salt", "salt@123"),
+                    'Already joined to saltstack')
 
     def test_get_system_time(self):
         '''
@@ -230,13 +233,10 @@ class WinSystemTestCase(TestCase, LoaderModuleMockMixin):
         '''
             Test to set system time
         '''
-        mock = MagicMock(side_effect=[False, True])
-        with patch.object(win_system, '_validate_time', mock):
+        with patch('salt.modules.win_system.set_system_date_time',
+                   MagicMock(side_effect=[False, True])):
             self.assertFalse(win_system.set_system_time("11:31:15 AM"))
-
-            mock = MagicMock(return_value=True)
-            with patch.dict(win_system.__salt__, {'cmd.retcode': mock}):
-                self.assertFalse(win_system.set_system_time("11:31:15 AM"))
+            self.assertTrue(win_system.set_system_time("11:31:15 AM"))
 
     def test_get_system_date(self):
         '''
@@ -250,13 +250,10 @@ class WinSystemTestCase(TestCase, LoaderModuleMockMixin):
         '''
             Test to set system date
         '''
-        mock = MagicMock(side_effect=[False, True])
-        with patch.object(win_system, '_validate_date', mock):
+        with patch('salt.modules.win_system.set_system_date_time',
+                   MagicMock(side_effect=[False, True])):
             self.assertFalse(win_system.set_system_date("03-28-13"))
-
-            mock = MagicMock(return_value=True)
-            with patch.dict(win_system.__salt__, {'cmd.retcode': mock}):
-                self.assertFalse(win_system.set_system_date("03-28-13"))
+            self.assertTrue(win_system.set_system_date("03-28-13"))
 
     def test_start_time_service(self):
         '''