Merge branch '2018.3' into issue46909

doc/topics/tutorials/minionfs.rst

@@ -139,7 +139,7 @@ blacklist, can be found below:
       - web*
       - 'mail\d+\.domain\.tld'
 
-    minionfs_whitelist:
+    minionfs_blacklist:
       - web21
 
 Potential Concerns

salt/config/__init__.py

@@ -3675,6 +3675,8 @@ def apply_minion_config(overrides=None,
     '''
     if defaults is None:
         defaults = DEFAULT_MINION_OPTS
+    if overrides is None:
+        overrides = {}
 
     opts = defaults.copy()
     opts['__role'] = 'minion'
@@ -3683,7 +3685,7 @@ def apply_minion_config(overrides=None,
         opts.update(overrides)
 
     if 'environment' in opts:
-        if 'saltenv' in opts:
+        if opts['saltenv'] is not None:
             log.warning(
                 'The \'saltenv\' and \'environment\' minion config options '
                 'cannot both be used. Ignoring \'environment\' in favor of '
@@ -3783,7 +3785,7 @@ def apply_minion_config(overrides=None,
     if 'beacons' not in opts:
         opts['beacons'] = {}
 
-    if (overrides or {}).get('ipc_write_buffer', '') == 'dynamic':
+    if overrides.get('ipc_write_buffer', '') == 'dynamic':
         opts['ipc_write_buffer'] = _DFLT_IPC_WBUFFER
     if 'ipc_write_buffer' not in overrides:
         opts['ipc_write_buffer'] = 0
@@ -3872,6 +3874,8 @@ def apply_master_config(overrides=None, defaults=None):
     '''
     if defaults is None:
         defaults = DEFAULT_MASTER_OPTS
+    if overrides is None:
+        overrides = {}
 
     opts = defaults.copy()
     opts['__role'] = 'master'
@@ -3880,7 +3884,7 @@ def apply_master_config(overrides=None, defaults=None):
         opts.update(overrides)
 
     if 'environment' in opts:
-        if 'saltenv' in opts:
+        if opts['saltenv'] is not None:
             log.warning(
                 'The \'saltenv\' and \'environment\' master config options '
                 'cannot both be used. Ignoring \'environment\' in favor of '
@@ -3930,7 +3934,7 @@ def apply_master_config(overrides=None, defaults=None):
     # Insert all 'utils_dirs' directories to the system path
     insert_system_path(opts, opts['utils_dirs'])
 
-    if (overrides or {}).get('ipc_write_buffer', '') == 'dynamic':
+    if overrides.get('ipc_write_buffer', '') == 'dynamic':
         opts['ipc_write_buffer'] = _DFLT_IPC_WBUFFER
     if 'ipc_write_buffer' not in overrides:
         opts['ipc_write_buffer'] = 0

salt/crypt.py

@@ -28,7 +28,7 @@ from salt.ext.six.moves import zip  # pylint: disable=import-error,redefined-bui
 from salt.ext import six
 
 try:
-    from M2Crypto import RSA, EVP
+    from M2Crypto import RSA, EVP, BIO
     HAS_M2 = True
 except ImportError:
     HAS_M2 = False
@@ -206,7 +206,10 @@ def get_rsa_pub_key(path):
     '''
     log.debug('salt.crypt.get_rsa_pub_key: Loading public key')
     if HAS_M2:
-        key = RSA.load_pub_key(path)
+        with salt.utils.files.fopen(path) as f:
+            data = f.read().replace(b'RSA ', '')
+        bio = BIO.MemoryBuffer(data)
+        key = RSA.load_pub_key_bio(bio)
     else:
         with salt.utils.files.fopen(path) as f:
             key = RSA.importKey(f.read())

salt/engines/slack.py

@@ -33,26 +33,26 @@ In addition, other groups are being loaded from pillars.
             default:
               users:
                 - *
-            commands:
-              - test.ping
-              - cmd.run
-              - list_jobs
-              - list_commands
-            aliases:
-              list_jobs:
-                cmd: jobs.list_jobs
-              list_commands:
-                cmd: pillar.get salt:engines:slack:valid_commands target=saltmaster tgt_type=list
-            default_target:
-              target: saltmaster
-              tgt_type: glob
-            targets:
-              test.ping:
-                target: '*'
-                tgt_type: glob
-              cmd.run:
+              commands:
+                - test.ping
+                - cmd.run
+                - list_jobs
+                - list_commands
+              aliases:
+                list_jobs:
+                  cmd: jobs.list_jobs
+                list_commands:
+                  cmd: pillar.get salt:engines:slack:valid_commands target=saltmaster tgt_type=list
+              default_target:
                 target: saltmaster
-                tgt_type: list
+                tgt_type: glob
+              targets:
+                test.ping:
+                  target: '*'
+                  tgt_type: glob
+                cmd.run:
+                  target: saltmaster
+                  tgt_type: list
 
 :configuration: Example configuration using the 'default' group and a non-default group and a pillar that will be merged in
     If the user is '*' (without the quotes) then the group's users or commands will match all users as appropriate
@@ -219,7 +219,7 @@ class SlackClient(object):
                 ret_groups[name]['aliases'].update(config.get('aliases', {}))
                 ret_groups[name]['default_target'].update(config.get('default_target', {}))
                 ret_groups[name]['targets'].update(config.get('targets', {}))
-            except IndexError:
+            except (IndexError, AttributeError):
                 log.warn("Couldn't use group %s. Check that targets is a dict and not a list", name)
 
         log.debug('Got the groups: %s', ret_groups)
@@ -351,7 +351,7 @@ class SlackClient(object):
 
         # maybe there are aliases, so check on that
         if cmdline[0] in permitted_group[1].get('aliases', {}).keys():
-            use_cmdline = self.commandline_to_list(permitted_group[1]['aliases'][cmdline[0]], '')
+            use_cmdline = self.commandline_to_list(permitted_group[1]['aliases'][cmdline[0]].get('cmd', ''), '')
         else:
             use_cmdline = cmdline
         target = self.get_target(permitted_group, cmdline, use_cmdline)

salt/grains/core.py

@@ -2403,9 +2403,8 @@ def get_server_id():
     if py_ver >= (3, 3):
         # Python 3.3 enabled hash randomization, so we need to shell out to get
         # a reliable hash.
-        py_bin = 'python{0}.{1}'.format(*py_ver)
         id_hash = __salt__['cmd.run'](
-            [py_bin, '-c', 'print(hash("{0}"))'.format(id_)],
+            [sys.executable, '-c', 'print(hash("{0}"))'.format(id_)],
             env={'PYTHONHASHSEED': '0'}
         )
         try:

salt/modules/dockermod.py

@@ -6787,7 +6787,7 @@ def sls_build(repository,
             'The \'name\' argument to docker.sls_build has been deprecated, '
             'please use \'repository\' instead.'
         )
-        respository = name
+        repository = name
 
     create_kwargs = __utils__['args.clean_kwargs'](**copy.deepcopy(kwargs))
     for key in ('image', 'name', 'cmd', 'interactive', 'tty'):

salt/modules/file.py

@@ -4812,7 +4812,7 @@ def check_file_meta(
             if sfn:
                 try:
                     changes['diff'] = get_diff(
-                        sfn, name, template=True, show_filenames=False)
+                        name, sfn, template=True, show_filenames=False)
                 except CommandExecutionError as exc:
                     changes['diff'] = exc.strerror
             else:

salt/modules/ldap3.py

@@ -12,6 +12,8 @@ This is an alternative to the ``ldap`` interface provided by the
 '''
 
 from __future__ import absolute_import, print_function, unicode_literals
+import logging
+import sys
 
 available_backends = set()
 try:
@@ -22,9 +24,9 @@ try:
     available_backends.add('ldap')
 except ImportError:
     pass
-import logging
+
+import salt.utils.data
 from salt.ext import six
-import sys
 
 log = logging.getLogger(__name__)
 
@@ -407,7 +409,10 @@ def add(connect_spec, dn, attributes):
     if 'unicodePwd' in attributes:
         attributes['unicodePwd'] = [_format_unicode_password(x) for x in attributes['unicodePwd']]
 
-    modlist = ldap.modlist.addModlist(attributes)
+    modlist = salt.utils.data.decode(
+        ldap.modlist.addModlist(attributes),
+        to_str=True
+    )
     try:
         l.c.add_s(dn, modlist)
     except ldap.LDAPError as e:
@@ -507,6 +512,7 @@ def modify(connect_spec, dn, directives):
             modlist[idx] = (mod[0], mod[1],
                 [_format_unicode_password(x) for x in mod[2]])
 
+    modlist = salt.utils.data.decode(modlist, to_str=True)
     try:
         l.c.modify_s(dn, modlist)
     except ldap.LDAPError as e:
@@ -573,7 +579,10 @@ def change(connect_spec, dn, before, after):
     if 'unicodePwd' in after:
         after['unicodePwd'] = [_format_unicode_password(x) for x in after['unicodePwd']]
 
-    modlist = ldap.modlist.modifyModlist(before, after)
+    modlist = salt.utils.data.decode(
+        ldap.modlist.modifyModlist(before, after),
+        to_str=True
+    )
     try:
         l.c.modify_s(dn, modlist)
     except ldap.LDAPError as e:

salt/modules/ldapmod.py

@@ -46,6 +46,7 @@ import logging
 import time
 
 # Import Salt libs
+import salt.utils.data
 from salt.ext import six
 from salt.exceptions import CommandExecutionError
 
@@ -140,7 +141,7 @@ def search(filter,      # pylint: disable=C0103
     if attrs == '':  # Allow command line 'return all' attr override
         attrs = None
     elif attrs is None:
-        attrs = _config('attrs')
+        attrs = salt.utils.data.decode(_config('attrs'), to_str=True)
     _ldap = _connect(**kwargs)
     start = time.time()
     log.debug(

salt/modules/mac_assistive.py

@@ -31,7 +31,7 @@ def __virtual__():
     '''
     if not salt.utils.platform.is_darwin():
         return False, 'Must be run on macOS'
-    if not _LooseVersion(__grains__['osrelease']) >= salt.utils.stringutils.to_str('10.9'):
+    if _LooseVersion(__grains__['osrelease']) < salt.utils.stringutils.to_str('10.9'):
         return False, 'Must be run on macOS 10.9 or newer'
     return __virtualname__
 

salt/modules/mysql.py

@@ -1691,11 +1691,11 @@ def __grant_generate(grant,
     table = db_part[2]
 
     if escape:
-        if dbc is not '*':
+        if dbc != '*':
             # _ and % are authorized on GRANT queries and should get escaped
             # on the db name, but only if not requesting a table level grant
-            dbc = quote_identifier(dbc, for_grants=(table is '*'))
-        if table is not '*':
+            dbc = quote_identifier(dbc, for_grants=(table == '*'))
+        if table != '*':
             table = quote_identifier(table)
     # identifiers cannot be used as values, and same thing for grants
     qry = 'GRANT {0} ON {1}.{2} TO %(user)s@%(host)s'.format(grant, dbc, table)
@@ -1895,16 +1895,16 @@ def grant_revoke(grant,
     db_part = database.rpartition('.')
     dbc = db_part[0]
     table = db_part[2]
-    if dbc is not '*':
+    if dbc != '*':
         # _ and % are authorized on GRANT queries and should get escaped
         # on the db name, but only if not requesting a table level grant
-        s_database = quote_identifier(dbc, for_grants=(table is '*'))
-    if dbc is '*':
+        s_database = quote_identifier(dbc, for_grants=(table == '*'))
+    if dbc == '*':
         # add revoke for *.*
         # before the modification query send to mysql will looks like
         # REVOKE SELECT ON `*`.* FROM %(user)s@%(host)s
         s_database = dbc
-    if table is not '*':
+    if table != '*':
         table = quote_identifier(table)
     # identifiers cannot be used as values, same thing for grants
     qry = 'REVOKE {0} ON {1}.{2} FROM %(user)s@%(host)s;'.format(

salt/modules/pip.py

@@ -130,7 +130,8 @@ def _get_pip_bin(bin_env):
              'pip{0}'.format(sys.version_info[0]),
              'pip', 'pip-python']
         )
-        if salt.utils.platform.is_windows() and six.PY2:
+        if salt.utils.platform.is_windows() and six.PY2 \
+           and isinstance(which_result, str):
             which_result.encode('string-escape')
         if which_result is None:
             raise CommandNotFoundError('Could not find a `pip` binary')

salt/modules/puppet.py

@@ -372,7 +372,12 @@ def facts(puppet=False):
     '''
     ret = {}
     opt_puppet = '--puppet' if puppet else ''
-    output = __salt__['cmd.run']('facter {0}'.format(opt_puppet))
+    cmd_ret = __salt__['cmd.run_all']('facter {0}'.format(opt_puppet))
+
+    if cmd_ret['retcode'] != 0:
+        raise CommandExecutionError(cmd_ret['stderr'])
+
+    output = cmd_ret['stdout']
 
     # Loop over the facter output and  properly
     # parse it into a nice dictionary for using
@@ -398,9 +403,13 @@ def fact(name, puppet=False):
         salt '*' puppet.fact kernel
     '''
     opt_puppet = '--puppet' if puppet else ''
-    ret = __salt__['cmd.run'](
+    ret = __salt__['cmd.run_all'](
             'facter {0} {1}'.format(opt_puppet, name),
             python_shell=False)
-    if not ret:
+
+    if ret['retcode'] != 0:
+        raise CommandExecutionError(ret['stderr'])
+
+    if not ret['stdout']:
         return ''
-    return ret
+    return ret['stdout']

salt/modules/saltutil.py

@@ -1504,6 +1504,9 @@ def runner(name, arg=None, kwarg=None, full_return=False, saltenv='base', jid=No
         if 'saltenv' in aspec.args:
             kwarg['saltenv'] = saltenv
 
+    if name in ['state.orchestrate', 'state.orch', 'state.sls']:
+        kwarg['orchestration_jid'] = jid
+
     if jid:
         salt.utils.event.fire_args(
             __opts__,

salt/modules/win_lgpo.py

@@ -49,6 +49,7 @@ import time
 
 # Import Salt libs
 from salt.exceptions import CommandExecutionError, SaltInvocationError
+from salt.serializers.configparser import deserialize
 import salt.utils.dictupdate as dictupdate
 import salt.utils.files
 import salt.utils.path
@@ -4652,37 +4653,34 @@ def _writeAdminTemplateRegPolFile(admtemplate_data,
 def _getScriptSettingsFromIniFile(policy_info):
     '''
     helper function to parse/read a GPO Startup/Shutdown script file
+
+    psscript.ini and script.ini file definitions are here
+        https://msdn.microsoft.com/en-us/library/ff842529.aspx
+        https://msdn.microsoft.com/en-us/library/dd303238.aspx
     '''
-    _existingData = _read_regpol_file(policy_info['ScriptIni']['IniPath'])
-    if _existingData:
-        _existingData = _existingData.split('\r\n')
-        script_settings = {}
-        this_section = None
-        for eLine in _existingData:
-            if eLine.startswith('[') and eLine.endswith(']'):
-                this_section = eLine.replace('[', '').replace(']', '')
-                log.debug('adding section %s', this_section)
-                if this_section:
-                    script_settings[this_section] = {}
-            else:
-                if '=' in eLine:
-                    log.debug('working with config line %s', eLine)
-                    eLine = eLine.split('=')
-                    if this_section in script_settings:
-                        script_settings[this_section][eLine[0]] = eLine[1]
-        if 'SettingName' in policy_info['ScriptIni']:
-            log.debug('Setting Name is in policy_info')
-            if policy_info['ScriptIni']['SettingName'] in script_settings[policy_info['ScriptIni']['Section']]:
-                log.debug('the value is set in the file')
-                return script_settings[policy_info['ScriptIni']['Section']][policy_info['ScriptIni']['SettingName']]
+    _existingData = None
+    if os.path.isfile(policy_info['ScriptIni']['IniPath']):
+        with salt.utils.files.fopen(policy_info['ScriptIni']['IniPath'], 'rb') as fhr:
+            _existingData = fhr.read()
+        if _existingData:
+            try:
+                _existingData = deserialize(_existingData.decode('utf-16-le').lstrip('\ufeff'))
+                log.debug('Have deserialized data %s', _existingData)
+            except Exception as error:
+                log.error('An error occurred attempting to deserialize data for %s', policy_info['Policy'])
+                raise CommandExecutionError(error)
+            if 'Section' in policy_info['ScriptIni'] and policy_info['ScriptIni']['Section'].lower() in [z.lower() for z in _existingData.keys()]:
+                if 'SettingName' in policy_info['ScriptIni']:
+                    log.debug('Need to look for %s', policy_info['ScriptIni']['SettingName'])
+                    if policy_info['ScriptIni']['SettingName'].lower() in [z.lower() for z in _existingData[policy_info['ScriptIni']['Section']].keys()]:
+                        return _existingData[policy_info['ScriptIni']['Section']][policy_info['ScriptIni']['SettingName'].lower()]
+                    else:
+                        return None
+                else:
+                    return _existingData[policy_info['ScriptIni']['Section']]
             else:
                 return None
-        elif policy_info['ScriptIni']['Section'] in script_settings:
-            log.debug('no setting name')
-            return script_settings[policy_info['ScriptIni']['Section']]
-        else:
-            log.debug('broad else')
-            return None
+
     return None
 
 

salt/modules/win_ntp.py

@@ -48,9 +48,10 @@ def set_servers(*servers):
     update_cmd = ['W32tm', '/config', '/update']
 
     for cmd in server_cmd, reliable_cmd, update_cmd:
-        ret = __salt__['cmd.run'](cmd, python_shell=False)
-        if 'command completed successfully' not in ret:
-            return False
+        __salt__['cmd.run'](cmd, python_shell=False)
+
+    if not sorted(list(servers)) == get_servers():
+        return False
 
     __salt__['service.restart'](service_name)
     return True
@@ -71,7 +72,7 @@ def get_servers():
     for line in lines:
         try:
             if line.startswith('NtpServer:'):
-                _, ntpsvrs = line.rstrip(' (Local)').split(':', 1)
+                _, ntpsvrs = line.rsplit(' (', 1)[0].split(':', 1)
                 return sorted(ntpsvrs.split())
         except ValueError as e:
             return False

salt/netapi/rest_tornado/saltnado.py

@@ -213,6 +213,7 @@ ioloop.install()
 import salt.netapi
 import salt.utils.args
 import salt.utils.event
+import salt.utils.jid
 import salt.utils.json
 import salt.utils.yaml
 from salt.utils.event import tagify
@@ -383,7 +384,7 @@ class EventListener(object):
         for (tag, matcher), futures in six.iteritems(self.tag_map):
             try:
                 is_matched = matcher(mtag, tag)
-            except Exception as e:
+            except Exception:
                 log.error('Failed to run a matcher.', exc_info=True)
                 is_matched = False
 
@@ -928,7 +929,11 @@ class SaltAPIHandler(BaseSaltAPIHandler):  # pylint: disable=W0223
         '''
         Dispatch local client commands
         '''
-        chunk_ret = {}
+        # Generate jid before triggering a job to subscribe all returns from minions
+        chunk['jid'] = salt.utils.jid.gen_jid(self.application.opts)
+
+        # Subscribe returns from minions before firing a job
+        future_minion_map = self.subscribe_minion_returns(chunk['jid'], chunk['tgt'])
 
         f_call = self._format_call_run_job_async(chunk)
         # fire a job off
@@ -937,64 +942,71 @@ class SaltAPIHandler(BaseSaltAPIHandler):  # pylint: disable=W0223
         # if the job didn't publish, lets not wait around for nothing
         # TODO: set header??
         if 'jid' not in pub_data:
+            for future in future_minion_map:
+                try:
+                    future.set_result(None)
+                except Exception:
+                    pass
             raise tornado.gen.Return('No minions matched the target. No command was sent, no jid was assigned.')
 
-        # seed minions_remaining with the pub_data
-        minions_remaining = pub_data['minions']
-
         syndic_min_wait = None
         if self.application.opts['order_masters']:
             syndic_min_wait = tornado.gen.sleep(self.application.opts['syndic_wait'])
 
         # To ensure job_not_running and all_return are terminated by each other, communicate using a future
         is_finished = Future()
+
         job_not_running_future = self.job_not_running(pub_data['jid'],
                                                       chunk['tgt'],
                                                       f_call['kwargs']['tgt_type'],
-                                                      is_finished,
-                                                      minions_remaining=list(minions_remaining),
-                                                      )
+                                                      is_finished)
 
         # if we have a min_wait, do that
         if syndic_min_wait is not None:
             yield syndic_min_wait
 
-        all_return_future = self.all_returns(pub_data['jid'],
-                                             is_finished,
-                                             minions_remaining=list(minions_remaining),
-                                             )
+        minion_returns_future = self.sanitize_minion_returns(future_minion_map, pub_data['minions'], is_finished)
+
         yield job_not_running_future
-        raise tornado.gen.Return((yield all_return_future))
+        raise tornado.gen.Return((yield minion_returns_future))
+
+    def subscribe_minion_returns(self, jid, minions):
+        # Subscribe each minion event
+        future_minion_map = {}
+        for minion in minions:
+            tag = tagify([jid, 'ret', minion], 'job')
+            minion_future = self.application.event_listener.get_event(self,
+                                                                      tag=tag,
+                                                                      matcher=EventListener.exact_matcher,
+                                                                      timeout=self.application.opts['timeout'])
+            future_minion_map[minion_future] = minion
+        return future_minion_map
 
     @tornado.gen.coroutine
-    def all_returns(self,
-                    jid,
-                    is_finished,
-                    minions_remaining=None,
-                    ):
+    def sanitize_minion_returns(self, future_minion_map, minions, is_finished):
         '''
         Return a future which will complete once all returns are completed
-        (according to minions_remaining), or one of the passed in "is_finished" completes
+        (according to minions), or one of the passed in "finish_chunk_ret_future" completes
         '''
-        if minions_remaining is None:
-            minions_remaining = []
+        if minions is None:
+            minions = []
+
+        # Remove redundant minions
+        redundant_minion_futures = [future for future in future_minion_map.keys() if future_minion_map[future] not in minions]
+        for redundant_minion_future in redundant_minion_futures:
+            try:
+                redundant_minion_future.set_result(None)
+            except Exception:
+                pass
+            del future_minion_map[redundant_minion_future]
 
         chunk_ret = {}
-
-        minion_events = {}
-        for minion in minions_remaining:
-            tag = tagify([jid, 'ret', minion], 'job')
-            minion_event = self.application.event_listener.get_event(self,
-                                                                     tag=tag,
-                                                                     matcher=EventListener.exact_matcher,
-                                                                     timeout=self.application.opts['timeout'])
-            minion_events[minion_event] = minion
-
         while True:
-            f = yield Any(minion_events.keys() + [is_finished])
+            f = yield Any(future_minion_map.keys() + [is_finished])
             try:
+                # When finished entire routine, cleanup other futures and return result
                 if f is is_finished:
-                    for event in minion_events:
+                    for event in future_minion_map.keys():
                         if not event.done():
                             event.set_result(None)
                     raise tornado.gen.Return(chunk_ret)
@@ -1005,31 +1017,22 @@ class SaltAPIHandler(BaseSaltAPIHandler):  # pylint: disable=W0223
 
             # clear finished event future
             try:
-                minions_remaining.remove(minion_events[f])
-                del minion_events[f]
+                minions.remove(future_minion_map[f])
+                del future_minion_map[f]
             except ValueError:
                 pass
 
-            if len(minions_remaining) == 0:
+            if not minions:
                 if not is_finished.done():
                     is_finished.set_result(True)
                 raise tornado.gen.Return(chunk_ret)
 
     @tornado.gen.coroutine
-    def job_not_running(self,
-                        jid,
-                        tgt,
-                        tgt_type,
-                        is_finished,
-                        minions_remaining=None,
-                        ):
+    def job_not_running(self, jid, tgt, tgt_type, is_finished):
         '''
         Return a future which will complete once jid (passed in) is no longer
         running on tgt
         '''
-        if minions_remaining is None:
-            minions_remaining = []
-
         ping_pub_data = yield self.saltclients['local'](tgt,
                                                         'saltutil.find_job',
                                                         [jid],
@@ -1063,13 +1066,11 @@ class SaltAPIHandler(BaseSaltAPIHandler):  # pylint: disable=W0223
                     ping_tag = tagify([ping_pub_data['jid'], 'ret'], 'job')
                     minion_running = False
                     continue
+
             # Minions can return, we want to see if the job is running...
             if event['data'].get('return', {}) == {}:
                 continue
             minion_running = True
-            id_ = event['data']['id']
-            if id_ not in minions_remaining:
-                minions_remaining.append(event['data']['id'])
 
     @tornado.gen.coroutine
     def _disbatch_local_async(self, chunk):

salt/runner.py

@@ -232,7 +232,7 @@ class Runner(RunnerClient):
                 else:
                     user = salt.utils.user.get_specific_user()
 
-                if low['fun'] in ('state.orchestrate', 'state.orch'):
+                if low['fun'] in ['state.orchestrate', 'state.orch', 'state.sls']:
                     low['kwarg']['orchestration_jid'] = async_pub['jid']
 
                 # Run the runner!

salt/runners/state.py

@@ -103,6 +103,7 @@ def orchestrate(mods,
             saltenv=saltenv,
             pillarenv=pillarenv,
             pillar_enc=pillar_enc,
+            __pub_jid=orchestration_jid,
             orchestration_jid=orchestration_jid)
     ret = {'data': {minion.opts['id']: running}, 'outputter': 'highstate'}
     res = __utils__['state.check_result'](ret['data'])

salt/state.py

@@ -42,6 +42,7 @@ import salt.utils.platform
 import salt.utils.process
 import salt.utils.url
 import salt.syspaths as syspaths
+from salt.serializers.msgpack import serialize as msgpack_serialize, deserialize as msgpack_deserialize
 from salt.template import compile_template, compile_template_str
 from salt.exceptions import (
     SaltRenderError,
@@ -53,11 +54,11 @@ from salt.utils.locales import sdecode
 import salt.utils.yamlloader as yamlloader
 
 # Import third party libs
+import msgpack
 # pylint: disable=import-error,no-name-in-module,redefined-builtin
 from salt.ext import six
 from salt.ext.six.moves import map, range, reload_module
 # pylint: enable=import-error,no-name-in-module,redefined-builtin
-import msgpack
 
 log = logging.getLogger(__name__)
 
@@ -179,7 +180,7 @@ def _calculate_fake_duration():
     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
+    duration = (delta.seconds * 1000000 + delta.microseconds) / 1000.0
 
     return start_time, duration
 
@@ -1710,27 +1711,20 @@ class State(object):
         errors.extend(req_in_errors)
         return req_in_high, errors
 
-    def _call_parallel_target(self, cdata, low):
+    def _call_parallel_target(self, name, cdata, low):
         '''
         The target function to call that will create the parallel thread/process
         '''
+        # we need to re-record start/end duration here because it is impossible to
+        # correctly calculate further down the chain
+        utc_start_time = datetime.datetime.utcnow()
+
         tag = _gen_tag(low)
         try:
             ret = self.states[cdata['full']](*cdata['args'],
                                              **cdata['kwargs'])
         except Exception:
             trb = traceback.format_exc()
-            # There are a number of possibilities to not have the cdata
-            # populated with what we might have expected, so just be smart
-            # enough to not raise another KeyError as the name is easily
-            # guessable and fallback in all cases to present the real
-            # exception to the user
-            if len(cdata['args']) > 0:
-                name = cdata['args'][0]
-            elif 'name' in cdata['kwargs']:
-                name = cdata['kwargs']['name']
-            else:
-                name = low.get('name', low.get('__id__'))
             ret = {
                 'result': False,
                 'name': name,
@@ -1738,6 +1732,13 @@ class State(object):
                 'comment': 'An exception occurred in this state: {0}'.format(
                     trb)
             }
+
+        utc_finish_time = datetime.datetime.utcnow()
+        delta = (utc_finish_time - utc_start_time)
+        # duration in milliseconds.microseconds
+        duration = (delta.seconds * 1000000 + delta.microseconds) / 1000.0
+        ret['duration'] = duration
+
         troot = os.path.join(self.opts['cachedir'], self.jid)
         tfile = os.path.join(troot, _clean_tag(tag))
         if not os.path.isdir(troot):
@@ -1748,17 +1749,26 @@ class State(object):
                 # and the attempt, we are safe to pass
                 pass
         with salt.utils.files.fopen(tfile, 'wb+') as fp_:
-            fp_.write(msgpack.dumps(ret))
+            fp_.write(msgpack_serialize(ret))
 
     def call_parallel(self, cdata, low):
         '''
         Call the state defined in the given cdata in parallel
         '''
+        # There are a number of possibilities to not have the cdata
+        # populated with what we might have expected, so just be smart
+        # enough to not raise another KeyError as the name is easily
+        # guessable and fallback in all cases to present the real
+        # exception to the user
+        name = (cdata.get('args') or [None])[0] or cdata['kwargs'].get('name')
+        if not name:
+            name = low.get('name', low.get('__id__'))
+
         proc = salt.utils.process.MultiprocessingProcess(
                 target=self._call_parallel_target,
-                args=(cdata, low))
+                args=(name, cdata, low))
         proc.start()
-        ret = {'name': cdata['args'][0],
+        ret = {'name': name,
                 'result': None,
                 'changes': {},
                 'comment': 'Started in a separate process',
@@ -1903,12 +1913,10 @@ class State(object):
             # enough to not raise another KeyError as the name is easily
             # guessable and fallback in all cases to present the real
             # exception to the user
-            if len(cdata['args']) > 0:
-                name = cdata['args'][0]
-            elif 'name' in cdata['kwargs']:
-                name = cdata['kwargs']['name']
-            else:
+            name = (cdata.get('args') or [None])[0] or cdata['kwargs'].get('name')
+            if not name:
                 name = low.get('name', low.get('__id__'))
+
             ret = {
                 'result': False,
                 'name': name,
@@ -1949,7 +1957,7 @@ class State(object):
         ret['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
+        duration = (delta.seconds * 1000000 + delta.microseconds) / 1000.0
         ret['duration'] = duration
         ret['__id__'] = low['__id__']
         log.info(
@@ -2117,7 +2125,7 @@ class State(object):
         while True:
             if self.reconcile_procs(running):
                 break
-            time.sleep(0.01)
+            time.sleep(0.0001)
         ret = dict(list(disabled.items()) + list(running.items()))
         return ret
 
@@ -2149,7 +2157,7 @@ class State(object):
                     tries = 0
                     with salt.utils.files.fopen(pause_path, 'rb') as fp_:
                         try:
-                            pdat = msgpack.loads(fp_.read())
+                            pdat = msgpack_deserialize(fp_.read())
                         except msgpack.UnpackValueError:
                             # Reading race condition
                             if tries > 10:
@@ -2196,7 +2204,7 @@ class State(object):
                                'changes': {}}
                     try:
                         with salt.utils.files.fopen(ret_cache, 'rb') as fp_:
-                            ret = msgpack.loads(fp_.read())
+                            ret = msgpack_deserialize(fp_.read())
                     except (OSError, IOError):
                         ret = {'result': False,
                                'comment': 'Parallel cache failure',
@@ -2309,15 +2317,17 @@ class State(object):
                 run_dict = self.pre
             else:
                 run_dict = running
+
+            while True:
+                if self.reconcile_procs(run_dict):
+                    break
+                time.sleep(0.0001)
+
             for chunk in chunks:
                 tag = _gen_tag(chunk)
                 if tag not in run_dict:
                     req_stats.add('unmet')
                     continue
-                if run_dict[tag].get('proc'):
-                    # Run in parallel, first wait for a touch and then recheck
-                    time.sleep(0.01)
-                    return self.check_requisite(low, running, chunks, pre)
                 if r_state.startswith('onfail'):
                     if run_dict[tag]['result'] is True:
                         req_stats.add('onfail')  # At least one state is OK

salt/states/grafana4_datasource.py

@@ -163,7 +163,7 @@ def present(name,
             datasource[key] = None
 
     if data == datasource:
-        ret['changes'] = None
+        ret['changes'] = {}
         ret['comment'] = 'Data source {0} already up-to-date'.format(name)
         return ret
 

salt/states/grafana4_org.py

@@ -187,7 +187,7 @@ def present(name,
         if ret['changes']:
             ret['comment'] = 'Org {0} updated'.format(name)
         else:
-            ret['changes'] = None
+            ret['changes'] = {}
             ret['comment'] = 'Org {0} already up-to-date'.format(name)
 
     return ret

salt/states/grafana4_user.py

@@ -124,7 +124,7 @@ def present(name,
         if ret['changes']:
             ret['comment'] = 'User {0} updated'.format(name)
         else:
-            ret['changes'] = None
+            ret['changes'] = {}
             ret['comment'] = 'User {0} already up-to-date'.format(name)
 
     return ret

salt/utils/data.py

@@ -68,9 +68,12 @@ def compare_lists(old=None, new=None):
 
 
 def decode(data, encoding=None, errors='strict', keep=False,
-           normalize=False, preserve_dict_class=False, preserve_tuples=False):
+           normalize=False, preserve_dict_class=False, preserve_tuples=False,
+           to_str=False):
     '''
-    Generic function which will decode whichever type is passed, if necessary
+    Generic function which will decode whichever type is passed, if necessary.
+    Optionally use to_str=True to ensure strings are str types and not unicode
+    on Python 2.
 
     If `strict` is True, and `keep` is False, and we fail to decode, a
     UnicodeDecodeError will be raised. Passing `keep` as True allows for the
@@ -94,22 +97,24 @@ def decode(data, encoding=None, errors='strict', keep=False,
     for the base character, and one for the breve mark). Normalizing allows for
     a more reliable test case.
     '''
+    _decode_func = salt.utils.stringutils.to_unicode \
+        if not to_str \
+        else salt.utils.stringutils.to_str
     if isinstance(data, collections.Mapping):
         return decode_dict(data, encoding, errors, keep, normalize,
-                           preserve_dict_class, preserve_tuples)
+                           preserve_dict_class, preserve_tuples, to_str)
     elif isinstance(data, list):
         return decode_list(data, encoding, errors, keep, normalize,
-                           preserve_dict_class, preserve_tuples)
+                           preserve_dict_class, preserve_tuples, to_str)
     elif isinstance(data, tuple):
         return decode_tuple(data, encoding, errors, keep, normalize,
-                            preserve_dict_class) \
+                            preserve_dict_class, to_str) \
             if preserve_tuples \
             else decode_list(data, encoding, errors, keep, normalize,
-                             preserve_dict_class, preserve_tuples)
+                             preserve_dict_class, preserve_tuples, to_str)
     else:
         try:
-            data = salt.utils.stringutils.to_unicode(
-                data, encoding, errors, normalize)
+            data = _decode_func(data, encoding, errors, normalize)
         except TypeError:
             # to_unicode raises a TypeError when input is not a
             # string/bytestring/bytearray. This is expected and simply means we
@@ -123,23 +128,26 @@ def decode(data, encoding=None, errors='strict', keep=False,
 
 def decode_dict(data, encoding=None, errors='strict', keep=False,
                 normalize=False, preserve_dict_class=False,
-                preserve_tuples=False):
+                preserve_tuples=False, to_str=False):
     '''
-    Decode all string values to Unicode
+    Decode all string values to Unicode. Optionally use to_str=True to ensure
+    strings are str types and not unicode on Python 2.
     '''
+    _decode_func = salt.utils.stringutils.to_unicode \
+        if not to_str \
+        else salt.utils.stringutils.to_str
     # Make sure we preserve OrderedDicts
     rv = data.__class__() if preserve_dict_class else {}
     for key, value in six.iteritems(data):
         if isinstance(key, tuple):
             key = decode_tuple(key, encoding, errors, keep, normalize,
-                               preserve_dict_class) \
+                               preserve_dict_class, to_str) \
                 if preserve_tuples \
                 else decode_list(key, encoding, errors, keep, normalize,
-                                 preserve_dict_class, preserve_tuples)
+                                 preserve_dict_class, preserve_tuples, to_str)
         else:
             try:
-                key = salt.utils.stringutils.to_unicode(
-                    key, encoding, errors, normalize)
+                key = _decode_func(key, encoding, errors, normalize)
             except TypeError:
                 # to_unicode raises a TypeError when input is not a
                 # string/bytestring/bytearray. This is expected and simply
@@ -151,20 +159,19 @@ def decode_dict(data, encoding=None, errors='strict', keep=False,
 
         if isinstance(value, list):
             value = decode_list(value, encoding, errors, keep, normalize,
-                                preserve_dict_class, preserve_tuples)
+                                preserve_dict_class, preserve_tuples, to_str)
         elif isinstance(value, tuple):
             value = decode_tuple(value, encoding, errors, keep, normalize,
-                                 preserve_dict_class) \
+                                 preserve_dict_class, to_str) \
                 if preserve_tuples \
                 else decode_list(value, encoding, errors, keep, normalize,
-                                 preserve_dict_class, preserve_tuples)
+                                 preserve_dict_class, preserve_tuples, to_str)
         elif isinstance(value, collections.Mapping):
             value = decode_dict(value, encoding, errors, keep, normalize,
-                                preserve_dict_class, preserve_tuples)
+                                preserve_dict_class, preserve_tuples, to_str)
         else:
             try:
-                value = salt.utils.stringutils.to_unicode(
-                    value, encoding, errors, normalize)
+                value = _decode_func(value, encoding, errors, normalize)
             except TypeError:
                 # to_unicode raises a TypeError when input is not a
                 # string/bytestring/bytearray. This is expected and simply
@@ -180,28 +187,31 @@ def decode_dict(data, encoding=None, errors='strict', keep=False,
 
 def decode_list(data, encoding=None, errors='strict', keep=False,
                 normalize=False, preserve_dict_class=False,
-                preserve_tuples=False):
+                preserve_tuples=False, to_str=False):
     '''
-    Decode all string values to Unicode
+    Decode all string values to Unicode. Optionally use to_str=True to ensure
+    strings are str types and not unicode on Python 2.
     '''
+    _decode_func = salt.utils.stringutils.to_unicode \
+        if not to_str \
+        else salt.utils.stringutils.to_str
     rv = []
     for item in data:
         if isinstance(item, list):
             item = decode_list(item, encoding, errors, keep, normalize,
-                               preserve_dict_class, preserve_tuples)
+                               preserve_dict_class, preserve_tuples, to_str)
         elif isinstance(item, tuple):
             item = decode_tuple(item, encoding, errors, keep, normalize,
-                                preserve_dict_class) \
+                                preserve_dict_class, to_str) \
                 if preserve_tuples \
                 else decode_list(item, encoding, errors, keep, normalize,
-                                 preserve_dict_class, preserve_tuples)
+                                 preserve_dict_class, preserve_tuples, to_str)
         elif isinstance(item, collections.Mapping):
             item = decode_dict(item, encoding, errors, keep, normalize,
-                               preserve_dict_class, preserve_tuples)
+                               preserve_dict_class, preserve_tuples, to_str)
         else:
             try:
-                item = salt.utils.stringutils.to_unicode(
-                    item, encoding, errors, normalize)
+                item = _decode_func(item, encoding, errors, normalize)
             except TypeError:
                 # to_unicode raises a TypeError when input is not a
                 # string/bytestring/bytearray. This is expected and simply
@@ -216,13 +226,14 @@ def decode_list(data, encoding=None, errors='strict', keep=False,
 
 
 def decode_tuple(data, encoding=None, errors='strict', keep=False,
-                 normalize=False, preserve_dict_class=False):
+                 normalize=False, preserve_dict_class=False, to_str=False):
     '''
-    Decode all string values to Unicode
+    Decode all string values to Unicode. Optionally use to_str=True to ensure
+    strings are str types and not unicode on Python 2.
     '''
     return tuple(
         decode_list(data, encoding, errors, keep, normalize,
-                    preserve_dict_class, True)
+                    preserve_dict_class, True, to_str)
     )
 
 

salt/utils/files.py

@@ -16,7 +16,6 @@ import stat
 import subprocess
 import tempfile
 import time
-import urllib
 
 # Import Salt libs
 import salt.utils.path
@@ -29,6 +28,7 @@ from salt.utils.decorators.jinja import jinja_filter
 # Import 3rd-party libs
 from salt.ext import six
 from salt.ext.six.moves import range
+from salt.ext.six.moves.urllib.parse import quote  # pylint: disable=no-name-in-module
 try:
     import fcntl
     HAS_FCNTL = True
@@ -583,7 +583,7 @@ def safe_filename_leaf(file_basename):
     :codeauthor: Damon Atkins <https://github.com/damon-atkins>
     '''
     def _replace(re_obj):
-        return urllib.quote(re_obj.group(0), safe='')
+        return quote(re_obj.group(0), safe='')
     if not isinstance(file_basename, six.text_type):
         # the following string is not prefixed with u
         return re.sub('[\\\\:/*?"<>|]',

salt/utils/stringutils.py

@@ -51,39 +51,45 @@ def to_bytes(s, encoding=None, errors='strict'):
         return to_str(s, encoding, errors)
 
 
-def to_str(s, encoding=None, errors='strict'):
+def to_str(s, encoding=None, errors='strict', normalize=False):
     '''
     Given str, bytes, bytearray, or unicode (py2), return str
     '''
+    def _normalize(s):
+        try:
+            return unicodedata.normalize('NFC', s) if normalize else s
+        except TypeError:
+            return s
+
     # This shouldn't be six.string_types because if we're on PY2 and we already
     # have a string, we should just return it.
     if isinstance(s, str):
-        return s
+        return _normalize(s)
     if six.PY3:
         if isinstance(s, (bytes, bytearray)):
             if encoding:
-                return s.decode(encoding, errors)
+                return _normalize(s.decode(encoding, errors))
             else:
                 try:
                     # Try UTF-8 first
-                    return s.decode('utf-8', errors)
+                    return _normalize(s.decode('utf-8', errors))
                 except UnicodeDecodeError:
                     # Fall back to detected encoding
-                    return s.decode(__salt_system_encoding__, errors)
+                    return _normalize(s.decode(__salt_system_encoding__, errors))
         raise TypeError('expected str, bytes, or bytearray not {}'.format(type(s)))
     else:
         if isinstance(s, bytearray):
             return str(s)  # future lint: disable=blacklisted-function
         if isinstance(s, unicode):  # pylint: disable=incompatible-py3-code,undefined-variable
             if encoding:
-                return s.encode(encoding, errors)
+                return _normalize(s).encode(encoding, errors)
             else:
                 try:
                     # Try UTF-8 first
-                    return s.encode('utf-8', errors)
+                    return _normalize(s).encode('utf-8', errors)
                 except UnicodeEncodeError:
                     # Fall back to detected encoding
-                    return s.encode(__salt_system_encoding__, errors)
+                    return _normalize(s).encode(__salt_system_encoding__, errors)
         raise TypeError('expected str, bytearray, or unicode')
 
 

salt/utils/templates.py

@@ -205,7 +205,8 @@ def _get_jinja_error_slug(tb_data):
         return [
             x
             for x in tb_data if x[2] in ('top-level template code',
-                                         'template')
+                                         'template',
+                                         '<module>')
         ][-1]
     except IndexError:
         pass

tests/integration/modules/test_pip.py

@@ -48,7 +48,7 @@ class PipModuleTest(ModuleCase):
         '''
         return any(w in ret for w in ['URLError', 'Download error'])
 
-    def pip_successful_install(self, target, expect=('tox', 'pep8',)):
+    def pip_successful_install(self, target, expect=('irc3-plugins-test', 'pep8',)):
         '''
         isolate regex for extracting `successful install` message from pip
         '''
@@ -103,7 +103,7 @@ class PipModuleTest(ModuleCase):
         with salt.utils.files.fopen(req1_filename, 'w') as f:
             f.write('-r requirements1b.txt\n')
         with salt.utils.files.fopen(req1b_filename, 'w') as f:
-            f.write('tox\n')
+            f.write('irc3-plugins-test\n')
         with salt.utils.files.fopen(req2_filename, 'w') as f:
             f.write('-r requirements2b.txt\n')
         with salt.utils.files.fopen(req2b_filename, 'w') as f:
@@ -141,7 +141,7 @@ class PipModuleTest(ModuleCase):
         with salt.utils.files.fopen(req1_filename, 'w') as f:
             f.write('-r requirements1b.txt\n')
         with salt.utils.files.fopen(req1b_filename, 'w') as f:
-            f.write('tox\n')
+            f.write('irc3-plugins-test\n')
         with salt.utils.files.fopen(req2_filename, 'w') as f:
             f.write('-r requirements2b.txt\n')
         with salt.utils.files.fopen(req2b_filename, 'w') as f:
@@ -174,7 +174,7 @@ class PipModuleTest(ModuleCase):
         req2_filename = os.path.join(self.venv_dir, 'requirements2.txt')
 
         with salt.utils.files.fopen(req1_filename, 'w') as f:
-            f.write('tox\n')
+            f.write('irc3-plugins-test\n')
         with salt.utils.files.fopen(req2_filename, 'w') as f:
             f.write('pep8\n')
 
@@ -211,7 +211,7 @@ class PipModuleTest(ModuleCase):
         req2_filepath = os.path.join(req_cwd, req2_filename)
 
         with salt.utils.files.fopen(req1_filepath, 'w') as f:
-            f.write('tox\n')
+            f.write('irc3-plugins-test\n')
         with salt.utils.files.fopen(req2_filepath, 'w') as f:
             f.write('pep8\n')
 

tests/integration/runners/test_state.py

@@ -6,10 +6,12 @@ Tests for the state runner
 # Import Python Libs
 from __future__ import absolute_import, print_function, unicode_literals
 import errno
+import logging
 import os
 import shutil
 import signal
 import tempfile
+import time
 import textwrap
 import threading
 from salt.ext.six.moves import queue
@@ -31,6 +33,8 @@ import salt.utils.yaml
 # Import 3rd-party libs
 from salt.ext import six
 
+log = logging.getLogger(__name__)
+
 
 class StateRunnerTest(ShellCase):
     '''
@@ -364,3 +368,85 @@ class OrchEventTest(ShellCase):
         finally:
             del listener
             signal.alarm(0)
+
+    def test_parallel_orchestrations(self):
+        '''
+        Test to confirm that the parallel state requisite works in orch
+        we do this by running 10 test.sleep's of 10 seconds, and insure it only takes roughly 10s
+        '''
+        self.write_conf({
+            'fileserver_backend': ['roots'],
+            'file_roots': {
+                'base': [self.base_env],
+            },
+        })
+
+        orch_sls = os.path.join(self.base_env, 'test_par_orch.sls')
+
+        with salt.utils.files.fopen(orch_sls, 'w') as fp_:
+            fp_.write(textwrap.dedent('''
+                {% for count in range(1, 20) %}
+
+                sleep {{ count }}:
+                    module.run:
+                        - name: test.sleep
+                        - length: 10
+                        - parallel: True
+
+                {% endfor %}
+
+                sleep 21:
+                    module.run:
+                        - name: test.sleep
+                        - length: 10
+                        - parallel: True
+                        - require:
+                            - module: sleep 1
+            '''))
+
+        orch_sls = os.path.join(self.base_env, 'test_par_orch.sls')
+
+        listener = salt.utils.event.get_event(
+            'master',
+            sock_dir=self.master_opts['sock_dir'],
+            transport=self.master_opts['transport'],
+            opts=self.master_opts)
+
+        start_time = time.time()
+        jid = self.run_run_plus(
+            'state.orchestrate',
+            'test_par_orch',
+            __reload_config=True).get('jid')
+
+        if jid is None:
+            raise Exception('jid missing from run_run_plus output')
+
+        signal.signal(signal.SIGALRM, self.alarm_handler)
+        signal.alarm(self.timeout)
+        received = False
+        try:
+            while True:
+                event = listener.get_event(full=True)
+                if event is None:
+                    continue
+
+                # if we receive the ret for this job before self.timeout (60),
+                # the test is implicitly sucessful; if it were happening in serial it would be
+                # atleast 110 seconds.
+                if event['tag'] == 'salt/run/{0}/ret'.format(jid):
+                    received = True
+                    # Don't wrap this in a try/except. We want to know if the
+                    # data structure is different from what we expect!
+                    ret = event['data']['return']['data']['master']
+                    for state in ret:
+                        data = ret[state]
+                        # we expect each duration to be greater than 10s
+                        self.assertTrue(data['duration'] > 10000)
+                    break
+
+                # self confirm that the total runtime is roughly 30s (left 10s for buffer)
+                self.assertTrue((time.time() - start_time) < 40)
+        finally:
+            self.assertTrue(received)
+            del listener
+            signal.alarm(0)

tests/integration/states/test_pkgrepo.py

@@ -83,7 +83,7 @@ class PkgrepoTest(ModuleCase, SaltReturnAssertsMixin):
         '''
         os_family = grains['os_family'].lower()
 
-        if os_family in ('redhat', 'suse'):
+        if os_family in ('redhat',):
             kwargs = {
                 'name': 'examplerepo',
                 'baseurl': 'http://example.com/repo',

tests/unit/config/test_config.py

@@ -19,7 +19,7 @@ import textwrap
 from tests.support.mixins import AdaptedConfigurationTestCaseMixin
 from tests.support.paths import TMP
 from tests.support.unit import skipIf, TestCase
-from tests.support.mock import NO_MOCK, NO_MOCK_REASON, MagicMock, patch
+from tests.support.mock import NO_MOCK, NO_MOCK_REASON, Mock, MagicMock, patch
 
 # Import Salt libs
 import salt.minion
@@ -1318,3 +1318,92 @@ class ConfigTestCase(TestCase, AdaptedConfigurationTestCaseMixin):
                                            config_path,
                                            verbose=False,
                                            exit_on_config_errors=True)
+
+    @staticmethod
+    def _get_defaults(**kwargs):
+        ret = {
+            'saltenv': kwargs.pop('saltenv', None),
+            'id': 'test',
+            'cachedir': '/A',
+            'sock_dir': '/B',
+            'root_dir': '/C',
+            'fileserver_backend': 'roots',
+            'open_mode': False,
+            'auto_accept': False,
+            'file_roots': {},
+            'pillar_roots': {},
+            'file_ignore_glob': [],
+            'file_ignore_regex': [],
+            'worker_threads': 5,
+            'hash_type': 'sha256',
+            'log_file': 'foo.log',
+        }
+        ret.update(kwargs)
+        return ret
+
+    @skipIf(NO_MOCK, NO_MOCK_REASON)
+    def test_apply_config(self):
+        '''
+        Ensure that the environment and saltenv options work properly
+        '''
+        with patch.object(sconfig, '_adjust_log_file_override', Mock()), \
+                patch.object(sconfig, '_update_ssl_config', Mock()), \
+                patch.object(sconfig, '_update_discovery_config', Mock()):
+
+            # MASTER CONFIG
+
+            # Ensure that environment overrides saltenv when saltenv not
+            # explicitly passed.
+            defaults = self._get_defaults(environment='foo')
+            ret = sconfig.apply_master_config(defaults=defaults)
+            self.assertEqual(ret['environment'], 'foo')
+            self.assertEqual(ret['saltenv'], 'foo')
+
+            # Ensure that environment overrides saltenv when saltenv not
+            # explicitly passed.
+            defaults = self._get_defaults(environment='foo', saltenv='bar')
+            ret = sconfig.apply_master_config(defaults=defaults)
+            self.assertEqual(ret['environment'], 'bar')
+            self.assertEqual(ret['saltenv'], 'bar')
+
+            # If environment was not explicitly set, it should not be in the
+            # opts at all.
+            defaults = self._get_defaults()
+            ret = sconfig.apply_master_config(defaults=defaults)
+            self.assertNotIn('environment', ret)
+            self.assertEqual(ret['saltenv'], None)
+
+            # Same test as above but with saltenv explicitly set
+            defaults = self._get_defaults(saltenv='foo')
+            ret = sconfig.apply_master_config(defaults=defaults)
+            self.assertNotIn('environment', ret)
+            self.assertEqual(ret['saltenv'], 'foo')
+
+            # MINION CONFIG
+
+            # Ensure that environment overrides saltenv when saltenv not
+            # explicitly passed.
+            defaults = self._get_defaults(environment='foo')
+            ret = sconfig.apply_minion_config(defaults=defaults)
+            self.assertEqual(ret['environment'], 'foo')
+            self.assertEqual(ret['saltenv'], 'foo')
+
+            # Ensure that environment overrides saltenv when saltenv not
+            # explicitly passed.
+            defaults = self._get_defaults(environment='foo', saltenv='bar')
+            ret = sconfig.apply_minion_config(defaults=defaults)
+            self.assertEqual(ret['environment'], 'bar')
+            self.assertEqual(ret['saltenv'], 'bar')
+
+            # If environment was not explicitly set, it should not be in the
+            # opts at all.
+            defaults = self._get_defaults()
+            ret = sconfig.apply_minion_config(defaults=defaults)
+            self.assertNotIn('environment', ret)
+            self.assertEqual(ret['saltenv'], None)
+
+            # Same test as above but with saltenv explicitly set
+            defaults = self._get_defaults(saltenv='foo')
+            ret = sconfig.apply_minion_config(defaults=defaults)
+            self.assertNotIn('environment', ret)
+            self.assertEqual(ret['saltenv'], 'foo')

tests/unit/modules/test_puppet.py

@@ -160,22 +160,27 @@ class PuppetTestCase(TestCase, LoaderModuleMockMixin):
         '''
         Test to run facter and return the results
         '''
-        mock_lst = MagicMock(return_value=[])
-        with patch.dict(puppet.__salt__, {'cmd.run': mock_lst}):
-            mock_lst = MagicMock(return_value="True")
-            with patch.dict(puppet.__salt__, {'cmd.run': mock_lst}):
-                mock = MagicMock(return_value=["a", "b"])
-                with patch.object(puppet, '_format_fact', mock):
-                    self.assertDictEqual(puppet.facts(), {'a': 'b'})
+        mock = MagicMock(return_value={
+            'retcode': 0,
+            'stdout': "1\n2"
+        })
+        with patch.dict(puppet.__salt__, {'cmd.run_all': mock}):
+            mock = MagicMock(side_effect=[
+                ['a', 'b'],
+                ['c', 'd'],
+            ])
+            with patch.object(puppet, '_format_fact', mock):
+                self.assertDictEqual(puppet.facts(), {'a': 'b', 'c': 'd'})
 
     def test_fact(self):
         '''
         Test to run facter for a specific fact
         '''
-        mock_lst = MagicMock(return_value=[])
-        with patch.dict(puppet.__salt__, {'cmd.run': mock_lst}):
-            mock_lst = MagicMock(side_effect=[False, True])
-            with patch.dict(puppet.__salt__, {'cmd.run': mock_lst}):
-                self.assertEqual(puppet.fact("salt"), "")
+        mock = MagicMock(side_effect=[
+            {'retcode': 0, 'stdout': False},
+            {'retcode': 0, 'stdout': True},
+        ])
+        with patch.dict(puppet.__salt__, {'cmd.run_all': mock}):
+            self.assertEqual(puppet.fact('salt'), '')
 
-                self.assertTrue(puppet.fact("salt"))
+            self.assertTrue(puppet.fact('salt'))

tests/unit/modules/test_win_ntp.py

@@ -31,23 +31,25 @@ class WinNtpTestCase(TestCase, LoaderModuleMockMixin):
         '''
         Test if it set Windows to use a list of NTP servers
         '''
+        # Windows Time (W32Time) service is not started
+        # Windows Time (W32Time) service fails to start
         mock_service = MagicMock(return_value=False)
-        mock_cmd = MagicMock(return_value='Failure')
         with patch.dict(win_ntp.__salt__, {'service.status': mock_service,
-                                           'service.start': mock_service,
-                                           'cmd.run': mock_cmd}):
+                                           'service.start': mock_service}):
             self.assertFalse(win_ntp.set_servers('pool.ntp.org'))
 
+        # Windows Time service is running
+        # Fail to set NTP servers
         mock_service = MagicMock(return_value=True)
-        mock_cmd = MagicMock(return_value='Failure')
+        mock_cmd = MagicMock(side_effect=['Failure', 'Failure', 'Failure', 'NtpServer: time.windows.com,0x8'])
         with patch.dict(win_ntp.__salt__, {'service.status': mock_service,
-                                           'service.start': mock_service,
                                            'cmd.run': mock_cmd}):
             self.assertFalse(win_ntp.set_servers('pool.ntp.org'))
 
-        mock_cmd = MagicMock(return_value='command completed successfully')
+        # Windows Time service is running
+        # Successfully set NTP servers
+        mock_cmd = MagicMock(side_effect=['Success', 'Success', 'Success', 'NtpServer: pool.ntp.org'])
         with patch.dict(win_ntp.__salt__, {'service.status': mock_service,
-                                           'service.start': mock_service,
                                            'service.restart': mock_service,
                                            'cmd.run': mock_cmd}):
             self.assertTrue(win_ntp.set_servers('pool.ntp.org'))

tests/unit/test_crypt.py

@@ -3,6 +3,8 @@
 # python libs
 from __future__ import absolute_import
 import os
+import tempfile
+import shutil
 
 # salt testing libs
 from tests.support.unit import TestCase, skipIf
@@ -196,8 +198,7 @@ class M2CryptTestCase(TestCase):
             self.assertEqual(SIG, crypt.sign_message('/keydir/keyname.pem', MSG, passphrase='password'))
 
     def test_verify_signature(self):
-        key = M2Crypto.RSA.load_pub_key_bio(M2Crypto.BIO.MemoryBuffer(six.b(PUBKEY_DATA)))
-        with patch('M2Crypto.RSA.load_pub_key', return_value=key):
+        with patch('salt.utils.files.fopen', mock_open(read_data=PUBKEY_DATA)):
             self.assertTrue(crypt.verify_signature('/keydir/keyname.pub', MSG, SIG))
 
     def test_encrypt_decrypt_bin(self):
@@ -206,3 +207,46 @@ class M2CryptTestCase(TestCase):
         encrypted = salt.crypt.private_encrypt(priv_key, b'salt')
         decrypted = salt.crypt.public_decrypt(pub_key, encrypted)
         self.assertEqual(b'salt', decrypted)
+
+
+class TestBadCryptodomePubKey(TestCase):
+    '''
+    Test that we can load public keys exported by pycrpytodome<=3.4.6
+    '''
+
+    TEST_KEY = (
+        '-----BEGIN RSA PUBLIC KEY-----\n'
+        'MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAzLtFhsvfbFDFaUgulSEX\n'
+        'Gl12XriL1DT78Ef2/u8HHaSMmPie37BLWas/zaHwI6066bIyYQJ/nUCahTaoHM7L\n'
+        'GlWc0wOU6zyfpihCRQHil05Y6F+olFBoZuYbFPvtp7/hJx/D7I/0n2o/c7M5i3Y2\n'
+        '3sBxAYNooIQHXHUmPQW6C9iu95ylZDW8JQzYy/EI4vCC8yQMdTK8jK1FQV0Sbwny\n'
+        'qcMxSyAWDoFbnhh2P2TnO8HOWuUOaXR8ZHOJzVcDl+a6ew+medW090x3K5O1f80D\n'
+        '+WjgnG6b2HG7VQpOCfM2GALD/FrxicPilvZ38X1aLhJuwjmVE4LAAv8DVNJXohaO\n'
+        'WQIDAQAB\n'
+        '-----END RSA PUBLIC KEY-----\n'
+    )
+
+    def setUp(self):
+        self.test_dir = tempfile.mkdtemp()
+        self.key_path = os.path.join(self.test_dir, 'cryptodom-3.4.6.pub')
+        with salt.utils.files.fopen(self.key_path, 'wb') as fd:
+            fd.write(self.TEST_KEY.encode())
+
+    def tearDown(self):
+        shutil.rmtree(self.test_dir)
+
+    @skipIf(not HAS_M2, "Skip when m2crypto is not installed")
+    def test_m2_bad_key(self):
+        '''
+        Load public key with an invalid header using m2crypto and validate it
+        '''
+        key = salt.crypt.get_rsa_pub_key(self.key_path)
+        assert key.check_key() == 1
+
+    @skipIf(HAS_M2, "Skip when m2crypto is installed")
+    def test_crypto_bad_key(self):
+        '''
+        Load public key with an invalid header and validate it without m2crypto
+        '''
+        key = salt.crypt.get_rsa_pub_key(self.key_path)
+        assert key.can_encrypt()

tests/unit/utils/test_data.py

@@ -9,14 +9,16 @@ import logging
 
 # Import Salt libs
 import salt.utils.data
-import salt.utils.data
+import salt.utils.stringutils
 from salt.utils.odict import OrderedDict
 from tests.support.unit import TestCase, skipIf, LOREM_IPSUM
 from tests.support.mock import patch, NO_MOCK, NO_MOCK_REASON
 from salt.ext.six.moves import builtins  # pylint: disable=import-error,redefined-builtin
+from salt.ext import six
 
 log = logging.getLogger(__name__)
 _b = lambda x: x.encode('utf-8')
+_s = lambda x: salt.utils.stringutils.to_str(x, normalize=True)
 # Some randomized data that will not decode
 BYTES = b'\x9c\xb1\xf7\xa3'
 # This is an example of a unicode string with й constructed using two separate
@@ -213,6 +215,9 @@ class DataTestCase(TestCase):
 
     def test_decode(self):
         '''
+        Companion to test_decode_to_str, they should both be kept up-to-date
+        with one another.
+
         NOTE: This uses the lambda "_b" defined above in the global scope,
         which encodes a string to a bytestring, assuming utf-8.
         '''
@@ -291,6 +296,97 @@ class DataTestCase(TestCase):
             BYTES,
             keep=False)
 
+    def test_decode_to_str(self):
+        '''
+        Companion to test_decode, they should both be kept up-to-date with one
+        another.
+
+        NOTE: This uses the lambda "_s" defined above in the global scope,
+        which converts the string/bytestring to a str type.
+        '''
+        expected = [
+            _s('unicode_str'),
+            _s('питон'),
+            123,
+            456.789,
+            True,
+            False,
+            None,
+            _s('яйца'),
+            BYTES,
+            [123, 456.789, _s('спам'), True, False, None, _s('яйца'), BYTES],
+            (987, 654.321, _s('яйца'), _s('яйца'), None, (True, _s('яйца'), BYTES)),
+            {_s('str_key'): _s('str_val'),
+             None: True,
+             123: 456.789,
+             _s('яйца'): BYTES,
+             _s('subdict'): {
+                 _s('unicode_key'): _s('яйца'),
+                 _s('tuple'): (123, _s('hello'), _s('world'), True, _s('яйца'), BYTES),
+                 _s('list'): [456, _s('спам'), False, _s('яйца'), BYTES]}},
+            OrderedDict([(_s('foo'), _s('bar')), (123, 456), (_s('яйца'), BYTES)])
+        ]
+
+        ret = salt.utils.data.decode(
+            self.test_data,
+            keep=True,
+            normalize=True,
+            preserve_dict_class=True,
+            preserve_tuples=True,
+            to_str=True)
+        self.assertEqual(ret, expected)
+
+        if six.PY3:
+            # The binary data in the data structure should fail to decode, even
+            # using the fallback, and raise an exception.
+            self.assertRaises(
+                UnicodeDecodeError,
+                salt.utils.data.decode,
+                self.test_data,
+                keep=False,
+                normalize=True,
+                preserve_dict_class=True,
+                preserve_tuples=True,
+                to_str=True)
+
+        # Now munge the expected data so that we get what we would expect if we
+        # disable preservation of dict class and tuples
+        expected[10] = [987, 654.321, _s('яйца'), _s('яйца'), None, [True, _s('яйца'), BYTES]]
+        expected[11][_s('subdict')][_s('tuple')] = [123, _s('hello'), _s('world'), True, _s('яйца'), BYTES]
+        expected[12] = {_s('foo'): _s('bar'), 123: 456, _s('яйца'): BYTES}
+
+        ret = salt.utils.data.decode(
+            self.test_data,
+            keep=True,
+            normalize=True,
+            preserve_dict_class=False,
+            preserve_tuples=False,
+            to_str=True)
+        self.assertEqual(ret, expected)
+
+        # Now test single non-string, non-data-structure items, these should
+        # return the same value when passed to this function
+        for item in (123, 4.56, True, False, None):
+            log.debug('Testing decode of %s', item)
+            self.assertEqual(salt.utils.data.decode(item, to_str=True), item)
+
+        # Test single strings (not in a data structure)
+        self.assertEqual(salt.utils.data.decode('foo', to_str=True), _s('foo'))
+        self.assertEqual(salt.utils.data.decode(_b('bar'), to_str=True), _s('bar'))
+
+        # Test binary blob
+        self.assertEqual(
+            salt.utils.data.decode(BYTES, keep=True, to_str=True),
+            BYTES
+        )
+        if six.PY3:
+            self.assertRaises(
+                UnicodeDecodeError,
+                salt.utils.data.decode,
+                BYTES,
+                keep=False,
+                to_str=True)
+
     @skipIf(NO_MOCK, NO_MOCK_REASON)
     def test_decode_fallback(self):
         '''