Merge branch '2017.7' into '2018.3'

Conflicts:
  - requirements/opt.txt
  - requirements/tests.txt
  - salt/modules/win_service.py
  - tests/integration/states/test_service.py

.github/CODEOWNERS

@@ -12,9 +12,10 @@
 # This file uses an fnmatch-style matching pattern.
 
 # Team Boto
-salt/**/*boto*                      @saltstack/team-boto
+salt/*/*boto*                       @saltstack/team-boto
 
 # Team Core
+requirements/*                      @saltstack/team-core
 salt/auth/*                         @saltstack/team-core
 salt/cache/*                        @saltstack/team-core
 salt/cli/*                          @saltstack/team-core
@@ -24,14 +25,16 @@ salt/daemons/*                      @saltstack/team-core
 salt/pillar/*                       @saltstack/team-core
 salt/loader.py                      @saltstack/team-core
 salt/payload.py                     @saltstack/team-core
-salt/**/master*                     @saltstack/team-core
-salt/**/minion*                     @saltstack/team-core
+salt/master.py                      @saltstack/team-core
+salt/*/master*                      @saltstack/team-core
+salt/minion.py                      @saltstack/team-core
+salt/*/minion*                      @saltstack/team-core
 
 # Team Cloud
 salt/cloud/*                        @saltstack/team-cloud
 salt/utils/openstack/*              @saltstack/team-cloud
 salt/utils/aws.py                   @saltstack/team-cloud
-salt/**/*cloud*                     @saltstack/team-cloud
+salt/*/*cloud*                      @saltstack/team-cloud
 
 # Team NetAPI
 salt/cli/api.py                     @saltstack/team-netapi
@@ -50,18 +53,18 @@ salt/cli/ssh.py                     @saltstack/team-ssh
 salt/client/ssh/*                   @saltstack/team-ssh
 salt/roster/*                       @saltstack/team-ssh
 salt/runners/ssh.py                 @saltstack/team-ssh
-salt/**/thin.py                     @saltstack/team-ssh
+salt/*/thin.py                      @saltstack/team-ssh
 
 # Team State
 salt/state.py                       @saltstack/team-state
 
 # Team SUSE
-salt/**/*btrfs*                     @saltstack/team-suse
-salt/**/*kubernetes*                @saltstack/team-suse
-salt/**/*pkg*                       @saltstack/team-suse
-salt/**/*snapper*                   @saltstack/team-suse
-salt/**/*xfs*                       @saltstack/team-suse
-salt/**/*zypper*                    @saltstack/team-suse
+salt/*/*btrfs*                      @saltstack/team-suse
+salt/*/*kubernetes*                 @saltstack/team-suse
+salt/*/*pkg*                        @saltstack/team-suse
+salt/*/*snapper*                    @saltstack/team-suse
+salt/*/*xfs*                        @saltstack/team-suse
+salt/*/*zypper*                     @saltstack/team-suse
 
 # Team Transport
 salt/transport/*                    @saltstack/team-transport

salt/fileserver/__init__.py

@@ -878,8 +878,6 @@ class FSChan(object):
         cmd = load['cmd'].lstrip('_')
         if cmd in self.cmd_stub:
             return self.cmd_stub[cmd]
-        if cmd == 'file_envs':
-            return self.fs.envs()
         if not hasattr(self.fs, cmd):
             log.error('Malformed request, invalid cmd: %s', load)
             return {}

salt/modules/win_service.py

@@ -147,6 +147,30 @@ def _status_wait(service_name, end_time, service_states):
     return info_results
 
 
+def _cmd_quote(cmd):
+    r'''
+    Helper function to properly format the path to the binary for the service
+    Must be wrapped in double quotes to account for paths that have spaces. For
+    example:
+
+    ``"C:\Program Files\Path\to\bin.exe"``
+
+    Args:
+        cmd (str): Full path to the binary
+
+    Returns:
+        str: Properly quoted path to the binary
+    '''
+    # Remove all single and double quotes from the beginning and the end
+    pattern = re.compile('^(\\"|\').*|.*(\\"|\')$')
+    while pattern.match(cmd) is not None:
+        cmd = cmd.strip('"').strip('\'')
+    # Ensure the path to the binary is wrapped in double quotes to account for
+    # spaces in the path
+    cmd = '"{0}"'.format(cmd)
+    return cmd
+
+
 def get_enabled():
     '''
     Return a list of enabled services. Enabled is defined as a service that is
@@ -334,7 +358,7 @@ def info(name):
             None, None, win32service.SC_MANAGER_CONNECT)
     except pywintypes.error as exc:
         raise CommandExecutionError(
-            'Failed to connect to the SCM: {0}'.format(exc[2]))
+            'Failed to connect to the SCM: {0}'.format(exc.strerror))
 
     try:
         handle_svc = win32service.OpenService(
@@ -345,7 +369,7 @@ def info(name):
             win32service.SERVICE_QUERY_STATUS)
     except pywintypes.error as exc:
         raise CommandExecutionError(
-            'Failed To Open {0}: {1}'.format(name, exc[2]))
+            'Failed To Open {0}: {1}'.format(name, exc.strerror))
 
     try:
         config_info = win32service.QueryServiceConfig(handle_svc)
@@ -439,7 +463,8 @@ def start(name, timeout=90):
             .. versionadded:: 2017.7.9, 2018.3.4
 
     Returns:
-        bool: ``True`` if successful, otherwise ``False``
+        bool: ``True`` if successful, otherwise ``False``. Also returns ``True``
+            if the service is already started
 
     CLI Example:
 
@@ -447,9 +472,6 @@ def start(name, timeout=90):
 
         salt '*' service.start <service name>
     '''
-    if status(name):
-        return True
-
     # Set the service to manual if disabled
     if disabled(name):
         modify(name, start_type='Manual')
@@ -457,8 +479,10 @@ def start(name, timeout=90):
     try:
         win32serviceutil.StartService(name)
     except pywintypes.error as exc:
-        raise CommandExecutionError(
-            'Failed To Start {0}: {1}'.format(name, exc[2]))
+        if exc.winerror != 1056:
+            raise CommandExecutionError(
+                'Failed To Start {0}: {1}'.format(name, exc.strerror))
+        log.debug('Service "{0}" is running'.format(name))
 
     srv_status = _status_wait(service_name=name,
                               end_time=time.time() + int(timeout),
@@ -481,7 +505,8 @@ def stop(name, timeout=90):
             .. versionadded:: 2017.7.9, 2018.3.4
 
     Returns:
-        bool: ``True`` if successful, otherwise ``False``
+        bool: ``True`` if successful, otherwise ``False``. Also returns ``True``
+            if the service is already stopped
 
     CLI Example:
 
@@ -492,9 +517,10 @@ def stop(name, timeout=90):
     try:
         win32serviceutil.StopService(name)
     except pywintypes.error as exc:
-        if exc[0] != 1062:
+        if exc.winerror != 1062:
             raise CommandExecutionError(
-                'Failed To Stop {0}: {1}'.format(name, exc[2]))
+                'Failed To Stop {0}: {1}'.format(name, exc.strerror))
+        log.debug('Service "{0}" is not running'.format(name))
 
     srv_status = _status_wait(service_name=name,
                               end_time=time.time() + int(timeout),
@@ -769,7 +795,7 @@ def modify(name,
             win32service.SERVICE_QUERY_CONFIG)
     except pywintypes.error as exc:
         raise CommandExecutionError(
-            'Failed To Open {0}: {1}'.format(name, exc))
+            'Failed To Open {0}: {1}'.format(name, exc.strerror))
 
     config_info = win32service.QueryServiceConfig(handle_svc)
 
@@ -777,7 +803,8 @@ def modify(name,
 
     # Input Validation
     if bin_path is not None:
-        bin_path = bin_path.strip('"')
+        # shlex.quote the path to the binary
+        bin_path = _cmd_quote(bin_path)
         if exe_args is not None:
             bin_path = '{0} {1}'.format(bin_path, exe_args)
         changes['BinaryPath'] = bin_path
@@ -1099,8 +1126,8 @@ def create(name,
     if name in get_all():
         raise CommandExecutionError('Service Already Exists: {0}'.format(name))
 
-    # Input validation
-    bin_path = bin_path.strip('"')
+    # shlex.quote the path to the binary
+    bin_path = _cmd_quote(bin_path)
     if exe_args is not None:
         bin_path = '{0} {1}'.format(bin_path, exe_args)
 
@@ -1188,7 +1215,8 @@ def delete(name, timeout=90):
             .. versionadded:: 2017.7.9, 2018.3.4
 
     Returns:
-        bool: ``True`` if successful, otherwise ``False``
+        bool: ``True`` if successful, otherwise ``False``. Also returns ``True``
+            if the service is not present
 
     CLI Example:
 
@@ -1203,8 +1231,12 @@ def delete(name, timeout=90):
         handle_svc = win32service.OpenService(
             handle_scm, name, win32service.SERVICE_ALL_ACCESS)
     except pywintypes.error as exc:
-        raise CommandExecutionError(
-            'Failed to open {0}. {1}'.format(name, exc.strerror))
+        win32service.CloseServiceHandle(handle_scm)
+        if exc.winerror != 1060:
+            raise CommandExecutionError(
+                'Failed to open {0}. {1}'.format(name, exc.strerror))
+        log.debug('Service "{0}" is not present'.format(name))
+        return True
 
     try:
         win32service.DeleteService(handle_svc)

tests/integration/modules/test_service.py

@@ -5,7 +5,7 @@ from __future__ import absolute_import, print_function, unicode_literals
 
 # Import Salt Testing libs
 from tests.support.case import ModuleCase
-from tests.support.helpers import destructiveTest
+from tests.support.helpers import destructiveTest, flaky
 from tests.support.unit import skipIf
 
 # Import Salt libs
@@ -59,6 +59,7 @@ class ServiceModuleTest(ModuleCase):
                 self.run_function('service.disable', [self.service_name])
         del self.service_name
 
+    @flaky
     def test_service_status_running(self):
         '''
         test service.status execution module

tests/integration/states/test_service.py

@@ -41,6 +41,8 @@ class ServiceTest(ModuleCase, SaltReturnAssertsMixin):
                 self.service_name = 'com.apple.AirPlayXPCHelper'
             self.stopped = ''
             self.running = '[0-9]'
+        elif os_family == 'Windows':
+            self.service_name = 'Spooler'
 
         self.pre_srv_enabled = True if self.service_name in self.run_function('service.get_enabled') else False
         self.post_srv_disable = False
@@ -48,7 +50,7 @@ class ServiceTest(ModuleCase, SaltReturnAssertsMixin):
             self.run_function('service.enable', name=self.service_name)
             self.post_srv_disable = True
 
-        if salt.utils.path.which(cmd_name) is None:
+        if os_family != 'Windows' and salt.utils.path.which(cmd_name) is None:
             self.skipTest('{0} is not installed'.format(cmd_name))
 
     def tearDown(self):

tests/unit/modules/test_win_service.py

@@ -23,6 +23,7 @@ import salt.modules.win_service as win_service
 try:
     WINAPI = True
     import win32serviceutil
+    import pywintypes
 except ImportError:
     WINAPI = False
 
@@ -119,18 +120,38 @@ class WinServiceTestCase(TestCase, LoaderModuleMockMixin):
         '''
         mock_true = MagicMock(return_value=True)
         mock_false = MagicMock(return_value=False)
-        mock_info = MagicMock(side_effect=[{'Status': 'Stopped'},
-                                           {'Status': 'Start Pending'},
-                                           {'Status': 'Running'}])
+        mock_info = MagicMock(side_effect=[{'Status': 'Running'}])
 
-        with patch.object(win_service, 'status', mock_true):
+        with patch.object(win32serviceutil, 'StartService', mock_true), \
+                patch.object(win_service, 'disabled', mock_false), \
+                patch.object(win_service, 'info', mock_info):
             self.assertTrue(win_service.start('spongebob'))
 
-        with patch.object(win_service, 'status', mock_false):
-            with patch.object(win32serviceutil, 'StartService', mock_true):
-                with patch.object(win_service, 'info', mock_info):
-                    with patch.object(win_service, 'status', mock_true):
-                        self.assertTrue(win_service.start('spongebob'))
+        mock_info = MagicMock(side_effect=[{'Status': 'Stopped', 'Status_WaitHint': 0},
+                                           {'Status': 'Start Pending', 'Status_WaitHint': 0},
+                                           {'Status': 'Running'}])
+
+        with patch.object(win32serviceutil, 'StartService', mock_true), \
+                patch.object(win_service, 'disabled', mock_false), \
+                patch.object(win_service, 'info', mock_info), \
+                patch.object(win_service, 'status', mock_true):
+            self.assertTrue(win_service.start('spongebob'))
+
+    @skipIf(not WINAPI, 'pywintypes not available')
+    def test_start_already_running(self):
+        '''
+        Test starting a service that is already running
+        '''
+        mock_false = MagicMock(return_value=False)
+        mock_error = MagicMock(
+            side_effect=pywintypes.error(1056,
+                                         'StartService',
+                                         'Service is running'))
+        mock_info = MagicMock(side_effect=[{'Status': 'Running'}])
+        with patch.object(win32serviceutil, 'StartService', mock_error), \
+                 patch.object(win_service, 'disabled', mock_false), \
+                 patch.object(win_service, '_status_wait', mock_info):
+            self.assertTrue(win_service.start('spongebob'))
 
     @skipIf(not WINAPI, 'win32serviceutil not available')
     def test_stop(self):
@@ -141,8 +162,7 @@ class WinServiceTestCase(TestCase, LoaderModuleMockMixin):
         mock_false = MagicMock(return_value=False)
         mock_info = MagicMock(side_effect=[{'Status': 'Stopped'}])
 
-        with patch.dict(win_service.__salt__, {'cmd.run': MagicMock(return_value="service was stopped")}), \
-                patch.object(win32serviceutil, 'StopService', mock_true), \
+        with patch.object(win32serviceutil, 'StopService', mock_true), \
                 patch.object(win_service, '_status_wait', mock_info):
             self.assertTrue(win_service.stop('spongebob'))
 
@@ -150,12 +170,25 @@ class WinServiceTestCase(TestCase, LoaderModuleMockMixin):
                                            {'Status': 'Stop Pending', 'Status_WaitHint': 0},
                                            {'Status': 'Stopped'}])
 
-        with patch.dict(win_service.__salt__, {'cmd.run': MagicMock(return_value="service was stopped")}), \
-                patch.object(win32serviceutil, 'StopService', mock_true), \
+        with patch.object(win32serviceutil, 'StopService', mock_true), \
                 patch.object(win_service, 'info', mock_info), \
                 patch.object(win_service, 'status', mock_false):
             self.assertTrue(win_service.stop('spongebob'))
 
+    @skipIf(not WINAPI, 'pywintypes not available')
+    def test_stop_not_running(self):
+        '''
+        Test stopping a service that is already stopped
+        '''
+        mock_error = MagicMock(
+            side_effect=pywintypes.error(1062,
+                                         'StopService',
+                                         'Service is not running'))
+        mock_info = MagicMock(side_effect=[{'Status': 'Stopped'}])
+        with patch.object(win32serviceutil, 'StopService', mock_error), \
+                patch.object(win_service, '_status_wait', mock_info):
+            self.assertTrue(win_service.stop('spongebob'))
+
     def test_restart(self):
         '''
             Test to restart the named service
@@ -266,3 +299,26 @@ class WinServiceTestCase(TestCase, LoaderModuleMockMixin):
         with patch.object(win_service, 'enabled', mock):
             self.assertTrue(win_service.disabled('spongebob'))
             self.assertFalse(win_service.disabled('squarepants'))
+
+    def test_cmd_quote(self):
+        '''
+        Make sure the command gets quoted correctly
+        '''
+        # Should always return command wrapped in double quotes
+        expected = r'"C:\Program Files\salt\test.exe"'
+
+        # test no quotes
+        bin_path = r'C:\Program Files\salt\test.exe'
+        self.assertEqual(win_service._cmd_quote(bin_path), expected)
+
+        # test single quotes
+        bin_path = r"'C:\Program Files\salt\test.exe'"
+        self.assertEqual(win_service._cmd_quote(bin_path), expected)
+
+        # test double quoted single quotes
+        bin_path = '"\'C:\\Program Files\\salt\\test.exe\'"'
+        self.assertEqual(win_service._cmd_quote(bin_path), expected)
+
+        # test single quoted, double quoted, single quotes
+        bin_path = "\'\"\'C:\\Program Files\\salt\\test.exe\'\"\'"
+        self.assertEqual(win_service._cmd_quote(bin_path), expected)

tests/whitelist.txt

@@ -56,6 +56,7 @@ integration.states.test_pkg
 integration.states.test_reg
 integration.states.test_renderers
 integration.states.test_file
+integration.states.test_service
 integration.states.test_user
 integration.utils.testprogram
 integration.wheel.test_client