Merge branch 'master' into its

doc/ref/modules/all/index.rst

@@ -186,6 +186,7 @@ execution modules
     haproxyconn
     hashutil
     heat
+    helm
     hg
     highstate_doc
     hosts

doc/ref/modules/all/salt.modules.helm.rst

@@ -0,0 +1,6 @@
+salt.modules.helm module
+========================
+
+.. automodule:: salt.modules.helm
+    :members:
+    :undoc-members:

doc/ref/returners/all/index.rst

@@ -10,6 +10,7 @@ returner modules
     :toctree:
     :template: autosummary.rst.tmpl
 
+    appoptics_return
     carbon_return
     cassandra_cql_return
     cassandra_return

doc/ref/returners/all/salt.returners.appoptics_return.rst

@@ -0,0 +1,6 @@
+salt.returners.appoptics_return module
+======================================
+
+.. automodule:: salt.returners.appoptics_return
+    :members:
+    :undoc-members:

doc/ref/states/all/index.rst

@@ -124,6 +124,7 @@ state modules
     grains
     group
     heat
+    helm
     hg
     highstate_doc
     host

doc/ref/states/all/salt.states.helm.rst

@@ -0,0 +1,6 @@
+salt.states.helm module
+=======================
+
+.. automodule:: salt.states.helm
+    :members:
+    :undoc-members:

doc/topics/development/pull_requests.rst

@@ -143,3 +143,45 @@ but these are things we consider when reviewing a pull request.
 
 * Should this be split into multiple PRs to make them easier to test and reason
   about?
+
+Pull Request Requirements
+=========================
+
+The following outlines what is required before a pull request can be merged into
+the salt project. For each of these requirements, an exception can be made
+that requires 3 approvals before merge. The exceptions are detailed more below.
+
+All PR requirements
+-------------------
+  * Approval Required: approval review from core team member OR 1 approval review
+    from captain of working group
+  * Cannot merge your own PR until 1 reviewer approves from defined list above that
+    is not the author.
+  * All Tests Pass
+
+Bug Fix PR requirements
+-----------------------
+  * Test Coverage: regression test written to cover bug fix. Contributers only need
+    to write test coverage for their specific changes.
+  * Point to the issue the PR is resolving. If there is not an issue one will need
+    to be created.
+
+Feature PR requirements
+-----------------------
+  * Test Coverage: tests written to cover new feature. Contributers only need to write
+    test coverage for their specific changes.
+  * Release Notes: Add note in release notes of new feature for relative release.
+  * Add .. versionadded:: <release> to module's documentation. If you are not certain
+    which release your fix will be included in you can include TBD and the PR reviewer
+    will let you know the correct name of the release you need to update to the versionadded.
+
+Exceptions to all requirements
+------------------------------
+As previously stated, all of the above requirements can be bypassed with 3 approvals.
+PR's that do not require tests include:
+
+  * documentation
+  * cosmetic changes (for example changing from log.debug to log.trace)
+  * fixing tests
+  * pylint
+  * changes outside of the salt directory

doc/topics/troubleshooting/yaml_idiosyncrasies.rst

@@ -379,8 +379,9 @@ string with quotes:
 Keys Limited to 1024 Characters
 ===============================
 
-Simple keys are limited to a single line and cannot be longer that 1024 characters.
-This is a limitation from PyYaml, as seen in a comment in `PyYAML's code`_, and
-applies to anything parsed by YAML in Salt.
+Simple keys are limited by the `YAML Spec`_ to a single line, and cannot be
+longer that 1024 characters. PyYAML enforces these limitations (see here__),
+and therefore anything parsed as YAML in Salt is subject to them.
 
-.. _PyYAML's code: http://pyyaml.org/browser/pyyaml/trunk/lib/yaml/scanner.py#L91
+.. _`YAML Spec`: https://yaml.org/spec/1.2/spec.html#id2792424
+.. __: https://github.com/yaml/pyyaml/blob/eb459f8/lib/yaml/scanner.py#L279-L293

salt/grains/core.py

@@ -1063,6 +1063,12 @@ def _virtual(osdata):
                         grains["virtual"] = "gce"
                     elif "BHYVE" in output:
                         grains["virtual"] = "bhyve"
+            except UnicodeDecodeError:
+                # Some firmwares provide non-valid 'product_name'
+                # files, ignore them
+                log.debug(
+                    "The content in /sys/devices/virtual/dmi/id/product_name is not valid"
+                )
             except IOError:
                 pass
     elif osdata["kernel"] == "FreeBSD":
@@ -2710,6 +2716,12 @@ def _hw_data(osdata):
                         )
                         if key == "uuid":
                             grains["uuid"] = grains["uuid"].lower()
+                except UnicodeDecodeError:
+                    # Some firmwares provide non-valid 'product_name'
+                    # files, ignore them
+                    log.debug(
+                        "The content in /sys/devices/virtual/dmi/id/product_name is not valid"
+                    )
                 except (IOError, OSError) as err:
                     # PermissionError is new to Python 3, but corresponds to the EACESS and
                     # EPERM error numbers. Use those instead here for PY2 compatibility.

salt/modules/cmdmod.py

@@ -3132,14 +3132,20 @@ def run_chroot(
 
     if isinstance(cmd, (list, tuple)):
         cmd = " ".join([six.text_type(i) for i in cmd])
-    cmd = "chroot {0} {1} -c {2}".format(root, sh_, _cmd_quote(cmd))
+
+    # If runas and group are provided, we expect that the user lives
+    # inside the chroot, not outside.
+    if runas:
+        userspec = "--userspec {}:{}".format(runas, group if group else "")
+    else:
+        userspec = ""
+
+    cmd = "chroot {} {} {} -c {}".format(userspec, root, sh_, _cmd_quote(cmd))
 
     run_func = __context__.pop("cmd.run_chroot.func", run_all)
 
     ret = run_func(
         cmd,
-        runas=runas,
-        group=group,
         cwd=cwd,
         stdin=stdin,
         shell=shell,

salt/modules/helm.py

@@ -2,13 +2,15 @@
 """
 Interface with Helm
 
-:depends: helm_ package installed on minion's system.
+:depends: pyhelm_ Python package
+
+.. _pyhelm: https://pypi.org/project/pyhelm/
 
 .. note::
     This module use the helm-cli. The helm-cli binary have to be present in your Salt-Minion path.
 
 Helm-CLI vs Salt-Modules
---------------
+------------------------
 
 This module is a wrapper of the helm binary.
 All helm v3.0 command are implemented.

salt/modules/linux_shadow.py

@@ -12,6 +12,7 @@ from __future__ import absolute_import, print_function, unicode_literals
 
 import datetime
 import functools
+import logging
 
 # Import python libs
 import os
@@ -39,6 +40,8 @@ except ImportError:
 
 __virtualname__ = "shadow"
 
+log = logging.getLogger(__name__)
+
 
 def __virtual__():
     return __virtualname__ if __grains__.get("kernel", "") == "Linux" else False
@@ -373,6 +376,10 @@ def set_password(name, password, use_usermod=False, root=None):
 
         salt '*' shadow.set_password root '$1$UYCIxa628.9qXjpQCjM4a..'
     """
+    if __salt__["cmd.retcode"](["id", name], ignore_retcode=True) != 0:
+        log.warning("user %s does not exist, cannot set password", name)
+        return False
+
     if not salt.utils.data.is_true(use_usermod):
         # Edit the shadow file directly
         # ALT Linux uses tcb to store password hashes. More information found
@@ -388,21 +395,32 @@ def set_password(name, password, use_usermod=False, root=None):
         if not os.path.isfile(s_file):
             return ret
         lines = []
+        user_found = False
+        lstchg = six.text_type(
+            (datetime.datetime.today() - datetime.datetime(1970, 1, 1)).days
+        )
         with salt.utils.files.fopen(s_file, "rb") as fp_:
             for line in fp_:
                 line = salt.utils.stringutils.to_unicode(line)
                 comps = line.strip().split(":")
-                if comps[0] != name:
-                    lines.append(line)
-                    continue
-                changed_date = datetime.datetime.today() - datetime.datetime(1970, 1, 1)
-                comps[1] = password
-                comps[2] = six.text_type(changed_date.days)
-                line = ":".join(comps)
-                lines.append("{0}\n".format(line))
-        with salt.utils.files.fopen(s_file, "w+") as fp_:
-            lines = [salt.utils.stringutils.to_str(_l) for _l in lines]
-            fp_.writelines(lines)
+                if comps[0] == name:
+                    user_found = True
+                    comps[1] = password
+                    comps[2] = lstchg
+                    line = ":".join(comps) + "\n"
+                lines.append(line)
+        if not user_found:
+            log.warning("shadow entry not present for user %s, adding", name)
+            with salt.utils.files.fopen(s_file, "a+") as fp_:
+                fp_.write(
+                    "{name}:{password}:{lstchg}::::::\n".format(
+                        name=name, password=password, lstchg=lstchg
+                    )
+                )
+        else:
+            with salt.utils.files.fopen(s_file, "w+") as fp_:
+                lines = [salt.utils.stringutils.to_str(_l) for _l in lines]
+                fp_.writelines(lines)
         uinfo = info(name, root=root)
         return uinfo["passwd"] == password
     else:

salt/modules/monit.py

@@ -144,17 +144,27 @@ def status(svc_name=""):
         salt '*' monit.status
         salt '*' monit.status <service name>
     """
+
     cmd = "monit status"
     res = __salt__["cmd.run"](cmd)
-    prostr = "Process" + " " * 28
+
+    # Monit uses a different separator since 5.18.0
+    if version() < "5.18.0":
+        fieldlength = 33
+    else:
+        fieldlength = 28
+
+    separator = 3 + fieldlength
+    prostr = "Process" + " " * fieldlength
+
     s = res.replace("Process", prostr).replace("'", "").split("\n\n")
     entries = {}
     for process in s[1:-1]:
         pro = process.splitlines()
         tmp = {}
         for items in pro:
-            key = items[:36].strip()
-            tmp[key] = items[35:].strip()
+            key = items[:separator].strip()
+            tmp[key] = items[separator - 1 :].strip()
         entries[pro[0].split()[1]] = tmp
     if svc_name == "":
         ret = entries

salt/modules/nilrt_ip.py

@@ -554,7 +554,8 @@ def _change_state(interface, new_state):
         raise salt.exceptions.CommandExecutionError(
             "Invalid interface name: {0}".format(interface)
         )
-    if not _connected(service):
+    connected = _connected(service)
+    if (not connected and new_state == "up") or (connected and new_state == "down"):
         service = pyconnman.ConnService(os.path.join(SERVICE_PATH, service))
         try:
             state = service.connect() if new_state == "up" else service.disconnect()

salt/returners/appoptics_return.py

@@ -0,0 +1,225 @@
+# -*- coding: utf-8 -*-
+"""Salt returner to return highstate stats to AppOptics Metrics
+
+To enable this returner the minion will need the AppOptics Metrics
+client importable on the Python path and the following
+values configured in the minion or master config.
+
+The AppOptics python client can be found at:
+
+https://github.com/appoptics/python-appoptics-metrics
+
+.. code-block:: yaml
+
+    appoptics.api_token: abc12345def
+
+An example configuration that returns the total number of successes
+and failures for your salt highstate runs (the default) would look
+like this:
+
+.. code-block:: yaml
+
+    return: appoptics
+    appoptics.api_token: <token string here>
+
+
+The returner publishes the following metrics to AppOptics:
+
+- saltstack.failed
+- saltstack.passed
+- saltstack.retcode
+- saltstack.runtime
+- saltstack.total
+
+
+You can add a tags section to specify which tags should be attached to
+all metrics created by the returner.
+
+.. code-block:: yaml
+
+    appoptics.tags:
+      host_hostname_alias: <the minion ID - matches @host>
+      tier: <the tier/etc. of this node>
+      cluster: <the cluster name, etc.>
+
+
+If no tags are explicitly configured, then the tag key ``host_hostname_alias``
+will be set, with the minion's ``id`` grain being the value.
+
+In addition to the requested tags, for a highstate run each of these
+will be tagged with the ``key:value`` of ``state_type: highstate``.
+
+In order to return metrics for ``state.sls`` runs (distinct from highstates), you can
+specify a list of state names to the key ``appoptics.sls_states`` like so:
+
+.. code-block:: yaml
+
+    appoptics.sls_states:
+      - role_salt_master.netapi
+      - role_redis.config
+      - role_smarty.dummy
+
+
+This will report success and failure counts on runs of the
+``role_salt_master.netapi``, ``role_redis.config``, and
+``role_smarty.dummy`` states in addition to highstates.
+
+This will report the same metrics as above, but for these runs the
+metrics will be tagged with ``state_type: sls`` and ``state_name`` set to
+the name of the state that was invoked, e.g. ``role_salt_master.netapi``.
+
+"""
+
+# Import python libs
+from __future__ import absolute_import, print_function, unicode_literals
+
+import logging
+
+import salt.returners
+
+# Import Salt libs
+import salt.utils.jid
+
+# Import third party libs
+try:
+    import appoptics_metrics
+
+    HAS_APPOPTICS = True
+except ImportError:
+    HAS_APPOPTICS = False
+
+# Define the module's Virtual Name
+__virtualname__ = "appoptics"
+
+log = logging.getLogger(__name__)
+
+
+def __virtual__():
+    if not HAS_APPOPTICS:
+        log.error(
+            "The appoptics_return module couldn't load the appoptics_metrics module."
+        )
+        log.error("please make sure it is installed and is in the PYTHON_PATH.")
+        return (
+            False,
+            "Could not import appoptics_metrics module; "
+            "appoptics-metrics python client is not installed.",
+        )
+    return __virtualname__
+
+
+def _get_options(ret=None):
+    """
+    Get the appoptics options from salt.
+    """
+    attrs = {
+        "api_token": "api_token",
+        "api_url": "api_url",
+        "tags": "tags",
+        "sls_states": "sls_states",
+    }
+
+    _options = salt.returners.get_returner_options(
+        __virtualname__, ret, attrs, __salt__=__salt__, __opts__=__opts__
+    )
+
+    _options["api_url"] = _options.get("api_url", "api.appoptics.com")
+    _options["sls_states"] = _options.get("sls_states", [])
+    _options["tags"] = _options.get(
+        "tags", {"host_hostname_alias": __salt__["grains.get"]("id")}
+    )
+
+    log.debug("Retrieved appoptics options: {}".format(_options))
+    return _options
+
+
+def _get_appoptics(options):
+    """
+    Return an appoptics connection object.
+    """
+    conn = appoptics_metrics.connect(
+        options.get("api_token"),
+        sanitizer=appoptics_metrics.sanitize_metric_name,
+        hostname=options.get("api_url"),
+    )
+    log.info("Connected to appoptics.")
+    return conn
+
+
+def _calculate_runtimes(states):
+    results = {"runtime": 0.00, "num_failed_states": 0, "num_passed_states": 0}
+
+    for state, resultset in states.items():
+        if isinstance(resultset, dict) and "duration" in resultset:
+            # Count the pass vs failures
+            if resultset["result"]:
+                results["num_passed_states"] += 1
+            else:
+                results["num_failed_states"] += 1
+
+            # Count durations
+            results["runtime"] += resultset["duration"]
+
+    log.debug("Parsed state metrics: {}".format(results))
+    return results
+
+
+def _state_metrics(ret, options, tags):
+    # Calculate the runtimes and number of failed states.
+    stats = _calculate_runtimes(ret["return"])
+    log.debug("Batching Metric retcode with {}".format(ret["retcode"]))
+    appoptics_conn = _get_appoptics(options)
+    q = appoptics_conn.new_queue(tags=tags)
+
+    q.add("saltstack.retcode", ret["retcode"])
+    log.debug(
+        "Batching Metric num_failed_jobs with {}".format(stats["num_failed_states"])
+    )
+    q.add("saltstack.failed", stats["num_failed_states"])
+
+    log.debug(
+        "Batching Metric num_passed_states with {}".format(stats["num_passed_states"])
+    )
+    q.add("saltstack.passed", stats["num_passed_states"])
+
+    log.debug("Batching Metric runtime with {}".format(stats["runtime"]))
+    q.add("saltstack.runtime", stats["runtime"])
+
+    log.debug(
+        "Batching with  Metric total states {}".format(
+            (stats["num_failed_states"] + stats["num_passed_states"])
+        )
+    )
+    q.add(
+        "saltstack.highstate.total_states",
+        (stats["num_failed_states"] + stats["num_passed_states"]),
+    )
+    log.info("Sending metrics to appoptics.")
+    q.submit()
+
+
+def returner(ret):
+    """
+    Parse the return data and return metrics to AppOptics.
+
+    For each state that's provided in the configuration, return tagged metrics for
+    the result of that state if it's present.
+    """
+
+    options = _get_options(ret)
+    states_to_report = ["state.highstate"]
+    if options.get("sls_states"):
+        states_to_report.append("state.sls")
+    if ret["fun"] in states_to_report:
+        tags = options.get("tags", {}).copy()
+        tags["state_type"] = ret["fun"]
+        log.info("Tags for this run are {}".format(str(tags)))
+        matched_states = set(ret["fun_args"]).intersection(
+            set(options.get("sls_states", []))
+        )
+        # What can I do if a run has multiple states that match?
+        # In the mean time, find one matching state name and use it.
+        if matched_states:
+            tags["state_name"] = sorted(matched_states)[0]
+            log.debug("Found returned data from {}.".format(tags["state_name"]))
+        _state_metrics(ret, options, tags)

salt/states/file.py

@@ -3293,7 +3293,7 @@ def managed(
                     salt.utils.files.remove(sfn)
 
 
-_RECURSE_TYPES = ["user", "group", "mode", "ignore_files", "ignore_dirs"]
+_RECURSE_TYPES = ["user", "group", "mode", "ignore_files", "ignore_dirs", "silent"]
 
 
 def _get_recurse_set(recurse):
@@ -3381,6 +3381,9 @@ def directory(
         ``mode`` is defined, will recurse on both ``file_mode`` and ``dir_mode`` if
         they are defined.  If ``ignore_files`` or ``ignore_dirs`` is included, files or
         directories will be left unchanged respectively.
+        directories will be left unchanged respectively. If ``silent`` is defined,
+        individual file/directory change notifications will be suppressed.
+
         Example:
 
         .. code-block:: yaml
@@ -3818,6 +3821,9 @@ def directory(
             file_mode = None
             dir_mode = None
 
+        if "silent" in recurse_set:
+            ret["changes"] = {"recursion": "Changes silenced"}
+
         check_files = "ignore_files" not in recurse_set
         check_dirs = "ignore_dirs" not in recurse_set
 

salt/states/x509.py

@@ -63,6 +63,12 @@ the mine where it can be easily retrieved by other minions.
     /etc/pki/issued_certs:
       file.directory
 
+    /etc/pki/ca.crt:
+      x509.private_key_managed:
+        - name: /etc/pki/ca.key
+        - bits: 4096
+        - backup: True
+
     /etc/pki/ca.crt:
       x509.certificate_managed:
         - signing_private_key: /etc/pki/ca.key
@@ -77,10 +83,6 @@ the mine where it can be easily retrieved by other minions.
         - days_valid: 3650
         - days_remaining: 0
         - backup: True
-        - managed_private_key:
-            name: /etc/pki/ca.key
-            bits: 4096
-            backup: True
         - require:
           - file: /etc/pki
 
@@ -134,6 +136,12 @@ This state creates a private key then requests a certificate signed by ca accord
 
 .. code-block:: yaml
 
+    /etc/pki/www.crt:
+      x509.private_key_managed:
+        - name: /etc/pki/www.key
+        - bits: 4096
+        - backup: True
+
     /etc/pki/www.crt:
       x509.certificate_managed:
         - ca_server: ca
@@ -142,11 +150,6 @@ This state creates a private key then requests a certificate signed by ca accord
         - CN: www.example.com
         - days_remaining: 30
         - backup: True
-        - managed_private_key:
-            name: /etc/pki/www.key
-            bits: 4096
-            backup: True
-
 """
 
 # Import Python Libs
@@ -159,6 +162,7 @@ import re
 
 # Import Salt Libs
 import salt.exceptions
+import salt.utils.versions
 
 # Import 3rd-party libs
 from salt.ext import six
@@ -267,7 +271,8 @@ def private_key_managed(
 
     new:
         Always create a new key. Defaults to ``False``.
-        Combining new with :mod:`prereq <salt.states.requsities.preqreq>`, or when used as part of a `managed_private_key` can allow key rotation whenever a new certificate is generated.
+        Combining new with :mod:`prereq <salt.states.requsities.preqreq>`
+        can allow key rotation whenever a new certificate is generated.
 
     overwrite:
         Overwrite an existing private key if the provided passphrase cannot decrypt it.
@@ -365,8 +370,139 @@ def csr_managed(name, **kwargs):
     return ret
 
 
+def _certificate_info_matches(cert_info, required_cert_info, check_serial=False):
+    """
+    Return true if the provided certificate information matches the
+    required certificate information, i.e. it has the required common
+    name, subject alt name, organization, etc.
+
+    cert_info should be a dict as returned by x509.read_certificate.
+    required_cert_info should be a dict as returned by x509.create_certificate with testrun=True.
+    """
+    # don't modify the incoming dicts
+    cert_info = copy.deepcopy(cert_info)
+    required_cert_info = copy.deepcopy(required_cert_info)
+
+    ignored_keys = [
+        "Not Before",
+        "Not After",
+        "MD5 Finger Print",
+        "SHA1 Finger Print",
+        "SHA-256 Finger Print",
+        # The integrity of the issuer is checked elsewhere
+        "Issuer Public Key",
+    ]
+    for key in ignored_keys:
+        cert_info.pop(key, None)
+        required_cert_info.pop(key, None)
+
+    if not check_serial:
+        cert_info.pop("Serial Number", None)
+        required_cert_info.pop("Serial Number", None)
+        try:
+            cert_info["X509v3 Extensions"]["authorityKeyIdentifier"] = re.sub(
+                r"serial:([0-9A-F]{2}:)*[0-9A-F]{2}",
+                "serial:--",
+                cert_info["X509v3 Extensions"]["authorityKeyIdentifier"],
+            )
+            required_cert_info["X509v3 Extensions"]["authorityKeyIdentifier"] = re.sub(
+                r"serial:([0-9A-F]{2}:)*[0-9A-F]{2}",
+                "serial:--",
+                required_cert_info["X509v3 Extensions"]["authorityKeyIdentifier"],
+            )
+        except KeyError:
+            pass
+
+    diff = []
+    for k, v in six.iteritems(required_cert_info):
+        try:
+            if v != cert_info[k]:
+                diff.append(k)
+        except KeyError:
+            diff.append(k)
+
+    return len(diff) == 0, diff
+
+
+def _certificate_days_remaining(cert_info):
+    """
+    Get the days remaining on a certificate, defaulting to 0 if an error occurs.
+    """
+    try:
+        expiry = cert_info["Not After"]
+        return (
+            datetime.datetime.strptime(expiry, "%Y-%m-%d %H:%M:%S")
+            - datetime.datetime.now()
+        ).days
+    except KeyError:
+        return 0
+
+
+def _certificate_is_valid(name, days_remaining, append_certs, **cert_spec):
+    """
+    Return True if the given certificate file exists, is a certificate, matches the given specification, and has the required days remaining.
+
+    If False, also provide a message explaining why.
+    """
+    if not os.path.isfile(name):
+        return False, "{0} does not exist".format(name), {}
+
+    try:
+        cert_info = __salt__["x509.read_certificate"](certificate=name)
+        required_cert_info = __salt__["x509.create_certificate"](
+            testrun=True, **cert_spec
+        )
+        if not isinstance(required_cert_info, dict):
+            raise salt.exceptions.SaltInvocationError(
+                "Unable to create new certificate: x509 module error: {0}".format(
+                    required_cert_info
+                )
+            )
+
+        try:
+            issuer_public_key = required_cert_info["Issuer Public Key"]
+            # Verify the certificate has been signed by the ca_server or private_signing_key
+            if not __salt__["x509.verify_signature"](name, issuer_public_key):
+                errmsg = (
+                    "Certificate is not signed by private_signing_key"
+                    if "signing_private_key" in cert_spec
+                    else "Certificate is not signed by the requested issuer"
+                )
+                return False, errmsg, cert_info
+        except KeyError:
+            return (
+                False,
+                "New certificate does not include signing information",
+                cert_info,
+            )
+
+        matches, diff = _certificate_info_matches(
+            cert_info, required_cert_info, check_serial="serial_number" in cert_spec
+        )
+        if not matches:
+            return (
+                False,
+                "Certificate properties are different: {0}".format(", ".join(diff)),
+                cert_info,
+            )
+
+        actual_days_remaining = _certificate_days_remaining(cert_info)
+        if days_remaining != 0 and actual_days_remaining < days_remaining:
+            return (
+                False,
+                "Certificate needs renewal: {0} days remaining but it needs to be at least {1}".format(
+                    actual_days_remaining, days_remaining
+                ),
+                cert_info,
+            )
+
+        return True, "", cert_info
+    except salt.exceptions.SaltInvocationError as e:
+        return False, "{0} is not a valid certificate: {1}".format(name, str(e)), {}
+
+
 def certificate_managed(
-    name, days_remaining=90, managed_private_key=None, append_certs=None, **kwargs
+    name, days_remaining=90, append_certs=None, managed_private_key=None, **kwargs
 ):
     """
     Manage a Certificate
@@ -375,18 +511,19 @@ def certificate_managed(
         Path to the certificate
 
     days_remaining : 90
-        The minimum number of days remaining when the certificate should be
-        recreated. A value of 0 disables automatic renewal.
-
-    managed_private_key
-        Manages the private key corresponding to the certificate. All of the
-        arguments supported by :py:func:`x509.private_key_managed
-        <salt.states.x509.private_key_managed>` are supported. If `name` is not
-        specified or is the same as the name of the certificate, the private
-        key and certificate will be written together in the same file.
+        Recreate the certificate if the number of days remaining on it
+        are less than this number. The value should be less than
+        ``days_valid``, otherwise the certificate will be recreated
+        every time the state is run. A value of 0 disables automatic
+        renewal.
 
     append_certs:
         A list of certificates to be appended to the managed file.
+        They must be valid PEM files, otherwise an error will be thrown.
+
+    managed_private_key:
+        Has no effect since v2016.11 and will be removed in Salt Aluminium.
+        Use a separate x509.private_key_managed call instead.
 
     kwargs:
         Any arguments supported by :py:func:`x509.create_certificate
@@ -429,173 +566,89 @@ def certificate_managed(
     if "path" in kwargs:
         name = kwargs.pop("path")
 
-    file_args, kwargs = _get_file_args(name, **kwargs)
-
-    rotate_private_key = False
-    new_private_key = False
-    if managed_private_key:
-        private_key_args = {
-            "name": name,
-            "new": False,
-            "overwrite": False,
-            "bits": 2048,
-            "passphrase": None,
-            "cipher": "aes_128_cbc",
-            "verbose": True,
-        }
-        private_key_args.update(managed_private_key)
-        kwargs["public_key_passphrase"] = private_key_args["passphrase"]
-
-        if private_key_args["new"]:
-            rotate_private_key = True
-            private_key_args["new"] = False
-
-        if _check_private_key(
-            private_key_args["name"],
-            bits=private_key_args["bits"],
-            passphrase=private_key_args["passphrase"],
-            new=private_key_args["new"],
-            overwrite=private_key_args["overwrite"],
-        ):
-            private_key = __salt__["x509.get_pem_entry"](
-                private_key_args["name"], pem_type="RSA PRIVATE KEY"
-            )
-        else:
-            new_private_key = True
-            private_key = __salt__["x509.create_private_key"](
-                text=True,
-                bits=private_key_args["bits"],
-                passphrase=private_key_args["passphrase"],
-                cipher=private_key_args["cipher"],
-                verbose=private_key_args["verbose"],
-            )
-
-        kwargs["public_key"] = private_key
-
-    current_days_remaining = 0
-    current_comp = {}
-
-    if os.path.isfile(name):
-        try:
-            current = __salt__["x509.read_certificate"](certificate=name)
-            current_comp = copy.deepcopy(current)
-            if "serial_number" not in kwargs:
-                current_comp.pop("Serial Number")
-                if "signing_cert" not in kwargs:
-                    try:
-                        current_comp["X509v3 Extensions"][
-                            "authorityKeyIdentifier"
-                        ] = re.sub(
-                            r"serial:([0-9A-F]{2}:)*[0-9A-F]{2}",
-                            "serial:--",
-                            current_comp["X509v3 Extensions"]["authorityKeyIdentifier"],
-                        )
-                    except KeyError:
-                        pass
-            current_comp.pop("Not Before")
-            current_comp.pop("MD5 Finger Print")
-            current_comp.pop("SHA1 Finger Print")
-            current_comp.pop("SHA-256 Finger Print")
-            current_notafter = current_comp.pop("Not After")
-            current_days_remaining = (
-                datetime.datetime.strptime(current_notafter, "%Y-%m-%d %H:%M:%S")
-                - datetime.datetime.now()
-            ).days
-            if days_remaining == 0:
-                days_remaining = current_days_remaining - 1
-        except salt.exceptions.SaltInvocationError:
-            current = "{0} is not a valid Certificate.".format(name)
-    else:
-        current = "{0} does not exist.".format(name)
-
     if "ca_server" in kwargs and "signing_policy" not in kwargs:
         raise salt.exceptions.SaltInvocationError(
             "signing_policy must be specified if ca_server is."
         )
 
-    new = __salt__["x509.create_certificate"](testrun=True, **kwargs)
+    if "public_key" not in kwargs and "signing_private_key" not in kwargs:
+        raise salt.exceptions.SaltInvocationError(
+            "public_key or signing_private_key must be specified."
+        )
 
-    if isinstance(new, dict):
-        new_comp = copy.deepcopy(new)
-        new.pop("Issuer Public Key")
-        if "serial_number" not in kwargs:
-            new_comp.pop("Serial Number")
-            if "signing_cert" not in kwargs:
-                try:
-                    new_comp["X509v3 Extensions"]["authorityKeyIdentifier"] = re.sub(
-                        r"serial:([0-9A-F]{2}:)*[0-9A-F]{2}",
-                        "serial:--",
-                        new_comp["X509v3 Extensions"]["authorityKeyIdentifier"],
-                    )
-                except KeyError:
-                    pass
-        new_comp.pop("Not Before")
-        new_comp.pop("Not After")
-        new_comp.pop("MD5 Finger Print")
-        new_comp.pop("SHA1 Finger Print")
-        new_comp.pop("SHA-256 Finger Print")
-        new_issuer_public_key = new_comp.pop("Issuer Public Key")
-    else:
-        new_comp = new
-
-    new_certificate = False
-    if (
-        current_comp == new_comp
-        and current_days_remaining > days_remaining
-        and __salt__["x509.verify_signature"](name, new_issuer_public_key)
-    ):
-        certificate = __salt__["x509.get_pem_entry"](name, pem_type="CERTIFICATE")
-    else:
-        if rotate_private_key and not new_private_key:
-            new_private_key = True
-            private_key = __salt__["x509.create_private_key"](
-                text=True,
-                bits=private_key_args["bits"],
-                verbose=private_key_args["verbose"],
-            )
-            kwargs["public_key"] = private_key
-        new_certificate = True
-        certificate = __salt__["x509.create_certificate"](text=True, **kwargs)
-
-    file_args["contents"] = ""
-    private_ret = {}
     if managed_private_key:
-        if private_key_args["name"] == name:
-            file_args["contents"] = private_key
-        else:
-            private_file_args = copy.deepcopy(file_args)
-            unique_private_file_args, _ = _get_file_args(**private_key_args)
-            private_file_args.update(unique_private_file_args)
-            private_file_args["contents"] = private_key
-            private_ret = __states__["file.managed"](**private_file_args)
-            if not private_ret["result"]:
-                return private_ret
+        salt.utils.versions.warn_until(
+            "Aluminium",
+            "Passing 'managed_private_key' to x509.certificate_managed has no effect and will be removed Salt Aluminium. Use a separate x509.private_key_managed call instead.",
+        )
 
-    file_args["contents"] += salt.utils.stringutils.to_str(certificate)
+    ret = {"name": name, "result": False, "changes": {}, "comment": ""}
+
+    is_valid, invalid_reason, current_cert_info = _certificate_is_valid(
+        name, days_remaining, append_certs, **kwargs
+    )
+
+    if is_valid:
+        ret["result"] = True
+        ret["comment"] = "Certificate {0} is valid and up to date".format(name)
+        return ret
+
+    if __opts__["test"]:
+        ret["result"] = None
+        ret["comment"] = "Certificate {0} will be created".format(name)
+        ret["changes"]["Status"] = {
+            "Old": invalid_reason,
+            "New": "Certificate will be valid and up to date",
+        }
+        return ret
+
+    contents = __salt__["x509.create_certificate"](text=True, **kwargs)
+    # Check the module actually returned a cert and not an error message as a string
+    try:
+        __salt__["x509.read_certificate"](contents)
+    except salt.exceptions.SaltInvocationError as e:
+        ret["result"] = False
+        ret[
+            "comment"
+        ] = "An error occurred creating the certificate {0}. The result returned from x509.create_certificate is not a valid PEM file:\n{1}".format(
+            name, str(e)
+        )
+        return ret
 
     if not append_certs:
         append_certs = []
-    for append_cert in append_certs:
-        file_args["contents"] += __salt__["x509.get_pem_entry"](
-            append_cert, pem_type="CERTIFICATE"
-        )
+    for append_file in append_certs:
+        try:
+            append_file_contents = __salt__["x509.get_pem_entry"](
+                append_file, pem_type="CERTIFICATE"
+            )
+            contents += append_file_contents
+        except salt.exceptions.SaltInvocationError as e:
+            ret["result"] = False
+            ret[
+                "comment"
+            ] = "{0} is not a valid certificate file, cannot append it to the certificate {1}.\nThe error returned by the x509 module was:\n{2}".format(
+                append_file, name, str(e)
+            )
+            return ret
 
-    file_args["show_changes"] = False
-    ret = __states__["file.managed"](**file_args)
+    file_args, extra_args = _get_file_args(name, **kwargs)
+    file_args["contents"] = contents
+    file_ret = __states__["file.managed"](**file_args)
 
-    if ret["changes"]:
-        ret["changes"] = {"Certificate": ret["changes"]}
-    else:
-        ret["changes"] = {}
-    if private_ret and private_ret["changes"]:
-        ret["changes"]["Private Key"] = private_ret["changes"]
-    if new_private_key:
-        ret["changes"]["Private Key"] = "New private key generated"
-    if new_certificate:
-        ret["changes"]["Certificate"] = {
-            "Old": current,
-            "New": __salt__["x509.read_certificate"](certificate=certificate),
-        }
+    if file_ret["changes"]:
+        ret["changes"] = {"File": file_ret["changes"]}
+
+    ret["changes"]["Certificate"] = {
+        "Old": current_cert_info,
+        "New": __salt__["x509.read_certificate"](certificate=name),
+    }
+    ret["changes"]["Status"] = {
+        "Old": invalid_reason,
+        "New": "Certificate is valid and up to date",
+    }
+    ret["comment"] = "Certificate {0} is valid and up to date".format(name)
+    ret["result"] = True
 
     return ret
 

salt/utils/vt.py

@@ -123,6 +123,8 @@ class Terminal(object):
         # sys.stdXYZ streaming options
         stream_stdout=None,
         stream_stderr=None,
+        # Used for tests
+        force_receive_encoding=__salt_system_encoding__,
     ):
 
         # Let's avoid Zombies!!!
@@ -139,6 +141,7 @@ class Terminal(object):
         self.cwd = cwd
         self.env = env
         self.preexec_fn = preexec_fn
+        self.receive_encoding = force_receive_encoding
 
         # ----- Set the desired terminal size ------------------------------->
         if rows is None and cols is None:
@@ -160,6 +163,9 @@ class Terminal(object):
         self.child_fd = None
         self.child_fde = None
 
+        self.partial_data_stdout = b""
+        self.partial_data_stderr = b""
+
         self.closed = True
         self.flag_eof_stdout = False
         self.flag_eof_stderr = False
@@ -604,6 +610,14 @@ class Terminal(object):
                 if not rlist:
                     self.flag_eof_stdout = self.flag_eof_stderr = True
                     log.debug("End of file(EOL). Brain-dead platform.")
+                    if self.partial_data_stdout or self.partial_data_stderr:
+                        # There is data that was received but for which
+                        # decoding failed, attempt decoding again to generate
+                        # relevant exception
+                        return (
+                            salt.utils.stringutils.to_unicode(self.partial_data_stdout),
+                            salt.utils.stringutils.to_unicode(self.partial_data_stderr),
+                        )
                     return None, None
             elif self.__irix_hack:
                 # Irix takes a long time before it realizes a child was
@@ -615,6 +629,14 @@ class Terminal(object):
                 if not rlist:
                     self.flag_eof_stdout = self.flag_eof_stderr = True
                     log.debug("End of file(EOL). Slow platform.")
+                    if self.partial_data_stdout or self.partial_data_stderr:
+                        # There is data that was received but for which
+                        # decoding failed, attempt decoding again to generate
+                        # relevant exception
+                        return (
+                            salt.utils.stringutils.to_unicode(self.partial_data_stdout),
+                            salt.utils.stringutils.to_unicode(self.partial_data_stderr),
+                        )
                     return None, None
 
             stderr = ""
@@ -646,16 +668,56 @@ class Terminal(object):
                     return None, None
             # <---- Nothing to Process!? -------------------------------------
 
-            # ----- Process STDERR ------------------------------------------>
-            if self.child_fde in rlist:
+            # ----- Helper function for processing STDERR and STDOUT -------->
+            def read_and_decode_fd(fd, maxsize, partial_data_attr=None):
+                bytes_read = getattr(self, partial_data_attr, b"")
+                # Only read one byte if we already have some existing data
+                # to try and complete a split multibyte character
+                bytes_read += os.read(fd, maxsize if not bytes_read else 1)
                 try:
-                    stderr = self._translate_newlines(
+                    decoded_data = self._translate_newlines(
                         salt.utils.stringutils.to_unicode(
-                            os.read(self.child_fde, maxsize)
+                            bytes_read, self.receive_encoding
                         )
                     )
+                    if partial_data_attr is not None:
+                        setattr(self, partial_data_attr, b"")
+                    return decoded_data, False
+                except UnicodeDecodeError as ex:
+                    max_multibyte_character_length = 4
+                    if ex.start > (
+                        len(bytes_read) - max_multibyte_character_length
+                    ) and ex.end == len(bytes_read):
+                        # We weren't able to decode the received data possibly
+                        # because it is a multibyte character split across
+                        # blocks. Save what data we have to try and decode
+                        # later. If the error wasn't caused by a multibyte
+                        # character being split then the error start position
+                        # should remain the same each time we get here but the
+                        # length of the bytes_read will increase so we will
+                        # give up and raise an exception instead.
+                        if partial_data_attr is not None:
+                            setattr(self, partial_data_attr, bytes_read)
+                        else:
+                            # We haven't been given anywhere to store partial
+                            # data so raise the exception instead
+                            raise
+                        # No decoded data to return, but indicate that there
+                        # is buffered data
+                        return "", True
+                    else:
+                        raise
 
-                    if not stderr:
+            # <---- Helper function for processing STDERR and STDOUT ---------
+
+            # ----- Process STDERR ------------------------------------------>
+            if self.child_fde in rlist and not self.flag_eof_stderr:
+                try:
+                    stderr, partial_data = read_and_decode_fd(
+                        self.child_fde, maxsize, "partial_data_stderr"
+                    )
+
+                    if not stderr and not partial_data:
                         self.flag_eof_stderr = True
                         stderr = None
                     else:
@@ -682,15 +744,13 @@ class Terminal(object):
             # <---- Process STDERR -------------------------------------------
 
             # ----- Process STDOUT ------------------------------------------>
-            if self.child_fd in rlist:
+            if self.child_fd in rlist and not self.flag_eof_stdout:
                 try:
-                    stdout = self._translate_newlines(
-                        salt.utils.stringutils.to_unicode(
-                            os.read(self.child_fd, maxsize)
-                        )
+                    stdout, partial_data = read_and_decode_fd(
+                        self.child_fd, maxsize, "partial_data_stdout"
                     )
 
-                    if not stdout:
+                    if not stdout and not partial_data:
                         self.flag_eof_stdout = True
                         stdout = None
                     else:

tests/integration/files/file/base/x509/cert_signing.sls

@@ -26,10 +26,6 @@
     - days_valid: 3650
     - days_remaining: 0
     - backup: True
-    - managed_private_key:
-        name: {{ tmp_dir  }}/pki/ca.key
-        bits: 4096
-        backup: True
     - require:
       - file: {{ tmp_dir  }}/pki
       - {{ tmp_dir  }}/pki/ca.key
@@ -56,10 +52,6 @@ test_crt:
     - CN: minion
     - days_remaining: 30
     - backup: True
-    - managed_private_key:
-        name: {{ tmp_dir  }}/pki/test.key
-        bits: 4096
-        backup: True
     - require:
         - {{ tmp_dir  }}/pki/ca.crt
         - {{ tmp_dir  }}/pki/test.key

tests/integration/files/file/base/x509/self_signed.sls

@@ -0,0 +1,16 @@
+{% set keyfile = pillar['keyfile'] %}
+{% set crtfile = pillar['crtfile'] %}
+
+private_key:
+  x509.private_key_managed:
+    - name: {{ keyfile }}
+
+self_signed_cert:
+  x509.certificate_managed:
+    - name: {{ crtfile }}
+    - signing_private_key: {{ keyfile }}
+    - CN: localhost
+    - days_valid: 30
+    - days_remaining: 0
+    - require:
+      - x509: private_key

tests/integration/files/file/base/x509/self_signed_different_properties.sls

@@ -0,0 +1,18 @@
+{% set keyfile = pillar['keyfile'] %}
+{% set crtfile = pillar['crtfile'] %}
+{% set subjectAltName = pillar['subjectAltName'] %}
+
+private_key:
+  x509.private_key_managed:
+    - name: {{ keyfile }}
+
+self_signed_cert:
+  x509.certificate_managed:
+    - name: {{ crtfile }}
+    - signing_private_key: {{ keyfile }}
+    - CN: service.local
+    - subjectAltName: {{ subjectAltName }}
+    - days_valid: 90
+    - days_remaining: 30
+    - require:
+      - x509: private_key

tests/integration/files/file/base/x509/self_signed_expiry.sls

@@ -0,0 +1,18 @@
+{% set keyfile = pillar['keyfile'] %}
+{% set crtfile = pillar['crtfile'] %}
+{% set days_valid = pillar['days_valid'] %}
+{% set days_remaining = pillar['days_remaining'] %}
+
+private_key:
+  x509.private_key_managed:
+    - name: {{ keyfile }}
+
+self_signed_cert:
+  x509.certificate_managed:
+    - name: {{ crtfile }}
+    - signing_private_key: {{ keyfile }}
+    - CN: localhost
+    - days_valid: {{ days_valid }}
+    - days_remaining: {{ days_remaining }}
+    - require:
+      - x509: private_key

tests/integration/returners/test_appoptics_return.py

@@ -0,0 +1,63 @@
+# -*- coding: utf-8 -*-
+"""
+Tests for the appoptics returner
+"""
+# Import Python libs
+from __future__ import absolute_import, print_function, unicode_literals
+
+import logging
+
+# Import salt libs
+from salt.returners import appoptics_return
+
+# Import Salt Testing libs
+from tests.support.case import ShellCase
+
+log = logging.getLogger(__name__)
+
+# JOBS DIR and FILES
+MOCK_RET_HIGHSTATE = {
+    "fun_args": [],
+    "return": {
+        "test-return-state": {
+            "comment": "insertcommenthere",
+            "name": "test-state-1",
+            "start_time": "01: 19: 51.105566",
+            "result": True,
+            "duration": 3.645,
+            "__run_num__": 193,
+            "changes": {},
+            "__id__": "test-return-state",
+        },
+        "test-return-state2": {
+            "comment": "insertcommenthere",
+            "name": "test-state-2",
+            "start_time": "01: 19: 51.105566",
+            "result": False,
+            "duration": 3.645,
+            "__run_num__": 194,
+            "changes": {},
+            "__id__": "test-return-state",
+        },
+    },
+    "retcode": 2,
+    "success": True,
+    "fun": "state.highstate",
+    "id": "AppOptics-Test",
+    "out": "highstate",
+}
+
+
+class AppOpticsTest(ShellCase):
+    """
+    Test the AppOptics returner
+    """
+
+    def test_count_runtimes(self):
+        """
+        Test the calculations
+        """
+        results = appoptics_return._calculate_runtimes(MOCK_RET_HIGHSTATE["return"])
+        self.assertEqual(results["num_failed_states"], 1)
+        self.assertEqual(results["num_passed_states"], 1)
+        self.assertEqual(results["runtime"], 7.29)

tests/integration/states/test_x509.py

@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 from __future__ import absolute_import, unicode_literals
 
-import logging
+import datetime
 import os
 import textwrap
 
@@ -21,9 +21,6 @@ except ImportError:
     HAS_M2CRYPTO = False
 
 
-log = logging.getLogger(__name__)
-
-
 @skipIf(not HAS_M2CRYPTO, "Skip when no M2Crypto found")
 class x509Test(ModuleCase, SaltReturnAssertsMixin):
     @classmethod
@@ -79,11 +76,6 @@ class x509Test(ModuleCase, SaltReturnAssertsMixin):
             salt.utils.files.rm_rf(certs_path)
         self.run_function("saltutil.refresh_pillar")
 
-    def run_function(self, *args, **kwargs):  # pylint: disable=arguments-differ
-        ret = super(x509Test, self).run_function(*args, **kwargs)
-        log.debug("ret = %s", ret)
-        return ret
-
     @with_tempfile(suffix=".pem", create=False)
     def test_issue_49027(self, pemfile):
         ret = self.run_state("x509.pem_managed", name=pemfile, text=self.x509_cert_text)
@@ -110,7 +102,7 @@ class x509Test(ModuleCase, SaltReturnAssertsMixin):
 
     def test_cert_signing(self):
         ret = self.run_function(
-            "state.apply", ["test_cert"], pillar={"tmp_dir": RUNTIME_VARS.TMP}
+            "state.apply", ["x509.cert_signing"], pillar={"tmp_dir": RUNTIME_VARS.TMP}
         )
         key = "x509_|-test_crt_|-{}/pki/test.crt_|-certificate_managed".format(
             RUNTIME_VARS.TMP
@@ -119,3 +111,193 @@ class x509Test(ModuleCase, SaltReturnAssertsMixin):
         assert "changes" in ret[key]
         assert "Certificate" in ret[key]["changes"]
         assert "New" in ret[key]["changes"]["Certificate"]
+
+    @with_tempfile(suffix=".crt", create=False)
+    @with_tempfile(suffix=".key", create=False)
+    def test_self_signed_cert(self, keyfile, crtfile):
+        """
+        Self-signed certificate, no CA.
+        Run the state twice to confirm the cert is only created once
+        and its contents don't change.
+        """
+        first_run = self.run_function(
+            "state.apply",
+            ["x509.self_signed"],
+            pillar={"keyfile": keyfile, "crtfile": crtfile},
+        )
+        key = "x509_|-self_signed_cert_|-{}_|-certificate_managed".format(crtfile)
+        self.assertIn("New", first_run[key]["changes"]["Certificate"])
+        self.assertEqual(
+            "Certificate is valid and up to date",
+            first_run[key]["changes"]["Status"]["New"],
+        )
+        self.assertTrue(os.path.exists(crtfile), "Certificate was not created.")
+
+        with salt.utils.files.fopen(crtfile, "r") as first_cert:
+            cert_contents = first_cert.read()
+
+        second_run = self.run_function(
+            "state.apply",
+            ["x509.self_signed"],
+            pillar={"keyfile": keyfile, "crtfile": crtfile},
+        )
+        self.assertEqual({}, second_run[key]["changes"])
+        with salt.utils.files.fopen(crtfile, "r") as second_cert:
+            self.assertEqual(
+                cert_contents,
+                second_cert.read(),
+                "Certificate contents should not have changed.",
+            )
+
+    @with_tempfile(suffix=".crt", create=False)
+    @with_tempfile(suffix=".key", create=False)
+    def test_old_self_signed_cert_is_recreated(self, keyfile, crtfile):
+        """
+        Self-signed certificate, no CA.
+        First create a cert that expires in 30 days, then recreate
+        the cert because the second state run requires days_remaining
+        to be at least 90.
+        """
+        first_run = self.run_function(
+            "state.apply",
+            ["x509.self_signed_expiry"],
+            pillar={
+                "keyfile": keyfile,
+                "crtfile": crtfile,
+                "days_valid": 30,
+                "days_remaining": 10,
+            },
+        )
+        key = "x509_|-self_signed_cert_|-{0}_|-certificate_managed".format(crtfile)
+        self.assertEqual(
+            "Certificate is valid and up to date",
+            first_run[key]["changes"]["Status"]["New"],
+        )
+        expiry = datetime.datetime.strptime(
+            first_run[key]["changes"]["Certificate"]["New"]["Not After"],
+            "%Y-%m-%d %H:%M:%S",
+        )
+        self.assertEqual(29, (expiry - datetime.datetime.now()).days)
+        self.assertTrue(os.path.exists(crtfile), "Certificate was not created.")
+
+        with salt.utils.files.fopen(crtfile, "r") as first_cert:
+            cert_contents = first_cert.read()
+
+        second_run = self.run_function(
+            "state.apply",
+            ["x509.self_signed_expiry"],
+            pillar={
+                "keyfile": keyfile,
+                "crtfile": crtfile,
+                "days_valid": 180,
+                "days_remaining": 90,
+            },
+        )
+        self.assertEqual(
+            "Certificate needs renewal: 29 days remaining but it needs to be at least 90",
+            second_run[key]["changes"]["Status"]["Old"],
+        )
+        expiry = datetime.datetime.strptime(
+            second_run[key]["changes"]["Certificate"]["New"]["Not After"],
+            "%Y-%m-%d %H:%M:%S",
+        )
+        self.assertEqual(179, (expiry - datetime.datetime.now()).days)
+        with salt.utils.files.fopen(crtfile, "r") as second_cert:
+            self.assertNotEqual(
+                cert_contents,
+                second_cert.read(),
+                "Certificate contents should have changed.",
+            )
+
+    @with_tempfile(suffix=".crt", create=False)
+    @with_tempfile(suffix=".key", create=False)
+    def test_mismatched_self_signed_cert_is_recreated(self, keyfile, crtfile):
+        """
+        Self-signed certificate, no CA.
+        First create a cert, then run the state again with a different
+        subjectAltName. The cert should be recreated.
+        Finally, run once more with the same subjectAltName as the
+        second run. Nothing should change.
+        """
+        first_run = self.run_function(
+            "state.apply",
+            ["x509.self_signed_different_properties"],
+            pillar={
+                "keyfile": keyfile,
+                "crtfile": crtfile,
+                "subjectAltName": "DNS:alt.service.local",
+            },
+        )
+        key = "x509_|-self_signed_cert_|-{0}_|-certificate_managed".format(crtfile)
+        self.assertEqual(
+            "Certificate is valid and up to date",
+            first_run[key]["changes"]["Status"]["New"],
+        )
+        sans = first_run[key]["changes"]["Certificate"]["New"]["X509v3 Extensions"][
+            "subjectAltName"
+        ]
+        self.assertEqual("DNS:alt.service.local", sans)
+        self.assertTrue(os.path.exists(crtfile), "Certificate was not created.")
+
+        with salt.utils.files.fopen(crtfile, "r") as first_cert:
+            first_cert_contents = first_cert.read()
+
+        second_run_pillar = {
+            "keyfile": keyfile,
+            "crtfile": crtfile,
+            "subjectAltName": "DNS:alt1.service.local, DNS:alt2.service.local",
+        }
+        second_run = self.run_function(
+            "state.apply",
+            ["x509.self_signed_different_properties"],
+            pillar=second_run_pillar,
+        )
+        self.assertEqual(
+            "Certificate properties are different: X509v3 Extensions",
+            second_run[key]["changes"]["Status"]["Old"],
+        )
+        sans = second_run[key]["changes"]["Certificate"]["New"]["X509v3 Extensions"][
+            "subjectAltName"
+        ]
+        self.assertEqual("DNS:alt1.service.local, DNS:alt2.service.local", sans)
+        with salt.utils.files.fopen(crtfile, "r") as second_cert:
+            second_cert_contents = second_cert.read()
+            self.assertNotEqual(
+                first_cert_contents,
+                second_cert_contents,
+                "Certificate contents should have changed.",
+            )
+
+        third_run = self.run_function(
+            "state.apply",
+            ["x509.self_signed_different_properties"],
+            pillar=second_run_pillar,
+        )
+        self.assertEqual({}, third_run[key]["changes"])
+        with salt.utils.files.fopen(crtfile, "r") as third_cert:
+            self.assertEqual(
+                second_cert_contents,
+                third_cert.read(),
+                "Certificate contents should not have changed.",
+            )
+
+    @with_tempfile(suffix=".crt", create=False)
+    @with_tempfile(suffix=".key", create=False)
+    def test_certificate_managed_with_managed_private_key_does_not_error(
+        self, keyfile, crtfile
+    ):
+        """
+        Test using the deprecated managed_private_key arg in certificate_managed does not throw an error.
+
+        TODO: Remove this test in Aluminium when the arg is removed.
+        """
+        self.run_state("x509.private_key_managed", name=keyfile, bits=4096)
+        ret = self.run_state(
+            "x509.certificate_managed",
+            name=crtfile,
+            CN="localhost",
+            signing_private_key=keyfile,
+            managed_private_key={"name": keyfile, "bits": 4096},
+        )
+        key = "x509_|-{0}_|-{0}_|-certificate_managed".format(crtfile)
+        self.assertEqual(True, ret[key]["result"])

tests/unit/fileserver/test_roots.py

@@ -62,6 +62,12 @@ class RootsTest(TestCase, AdaptedConfigurationTestCaseMixin, LoaderModuleMockMix
                 os.chdir(cwd)
             cls.test_symlink_list_file_roots = {"base": [root_dir]}
         else:
+            dest_sym = os.path.join(RUNTIME_VARS.BASE_FILES, "dest_sym")
+            if not os.path.islink(dest_sym):
+                # Fix broken symlink by recreating it
+                if os.path.exists(dest_sym):
+                    os.remove(dest_sym)
+                os.symlink("source_sym", dest_sym)
             cls.test_symlink_list_file_roots = None
         cls.tmp_dir = tempfile.mkdtemp(dir=RUNTIME_VARS.TMP)
         full_path_to_file = os.path.join(RUNTIME_VARS.BASE_FILES, "testfile")

tests/unit/grains/test_core.py

@@ -1791,3 +1791,44 @@ class CoreGrainsTestCase(TestCase, LoaderModuleMockMixin):
         with patch.dict(os.environ, {"PATH": path}):
             result = core.path()
         assert result == {"path": path, "systempath": comps}, result
+
+    @skipIf(not salt.utils.platform.is_linux(), "System is not Linux")
+    @patch("os.path.exists")
+    @patch("salt.utils.platform.is_proxy")
+    def test__hw_data_linux_empty(self, is_proxy, exists):
+        is_proxy.return_value = False
+        exists.return_value = True
+        with patch("salt.utils.files.fopen", mock_open(read_data="")):
+            self.assertEqual(
+                core._hw_data({"kernel": "Linux"}),
+                {
+                    "biosreleasedate": "",
+                    "biosversion": "",
+                    "manufacturer": "",
+                    "productname": "",
+                    "serialnumber": "",
+                    "uuid": "",
+                },
+            )
+
+    @skipIf(not salt.utils.platform.is_linux(), "System is not Linux")
+    @patch("os.path.exists")
+    @patch("salt.utils.platform.is_proxy")
+    def test__hw_data_linux_unicode_error(self, is_proxy, exists):
+        def _fopen(*args):
+            class _File(object):
+                def __enter__(self):
+                    return self
+
+                def __exit__(self, *args):
+                    pass
+
+                def read(self):
+                    raise UnicodeDecodeError("enconding", b"", 1, 2, "reason")
+
+            return _File()
+
+        is_proxy.return_value = False
+        exists.return_value = True
+        with patch("salt.utils.files.fopen", _fopen):
+            self.assertEqual(core._hw_data({"kernel": "Linux"}), {})

tests/unit/modules/test_cmdmod.py

@@ -452,6 +452,21 @@ class CMDMODTestCase(TestCase, LoaderModuleMockMixin):
         else:
             raise RuntimeError
 
+    @skipIf(salt.utils.platform.is_windows(), "Do not run on Windows")
+    @skipIf(salt.utils.platform.is_darwin(), "Do not run on MacOS")
+    def test_run_cwd_in_combination_with_runas(self):
+        """
+        cmd.run executes command in the cwd directory
+        when the runas parameter is specified
+        """
+        cmd = "pwd"
+        cwd = "/tmp"
+        runas = os.getlogin()
+
+        with patch.dict(cmdmod.__grains__, {"os": "Darwin", "os_family": "Solaris"}):
+            stdout = cmdmod._run(cmd, cwd=cwd, runas=runas).get("stdout")
+        self.assertEqual(stdout, cwd)
+
     def test_run_all_binary_replace(self):
         """
         Test for failed decoding of binary data, for instance when doing
@@ -605,3 +620,38 @@ class CMDMODTestCase(TestCase, LoaderModuleMockMixin):
                 cmdmod.run_chroot("/mnt", "cmd", binds=["/var"])
                 self.assertEqual(mock_mount.call_count, 4)
                 self.assertEqual(mock_umount.call_count, 4)
+
+    @skipIf(salt.utils.platform.is_windows(), "Skip test on Windows")
+    def test_run_chroot_runas(self):
+        """
+        Test run_chroot when a runas parameter is provided
+        """
+        with patch.dict(
+            cmdmod.__salt__, {"mount.mount": MagicMock(), "mount.umount": MagicMock()}
+        ):
+            with patch("salt.modules.cmdmod.run_all") as run_all_mock:
+                cmdmod.run_chroot("/mnt", "ls", runas="foobar")
+        run_all_mock.assert_called_with(
+            "chroot --userspec foobar: /mnt /bin/sh -c ls",
+            bg=False,
+            clean_env=False,
+            cwd=None,
+            env=None,
+            ignore_retcode=False,
+            log_callback=None,
+            output_encoding=None,
+            output_loglevel="quiet",
+            pillar=None,
+            pillarenv=None,
+            python_shell=True,
+            reset_system_locale=True,
+            rstrip=True,
+            saltenv="base",
+            shell="/bin/bash",
+            stdin=None,
+            success_retcodes=None,
+            template=None,
+            timeout=None,
+            umask=None,
+            use_vt=False,
+        )

tests/unit/modules/test_linux_shadow.py

@@ -3,16 +3,20 @@
     :codeauthor: Erik Johnson <erik@saltstack.com>
 """
 
-# Import Pytohn libs
+# Import Python libs
 from __future__ import absolute_import, print_function, unicode_literals
 
+import textwrap
+
 # Import Salt Testing libs
 import salt.utils.platform
+import salt.utils.stringutils
 
 # Import 3rd-party libs
 from salt.ext import six
 from tests.support.helpers import skip_if_not_root
 from tests.support.mixins import LoaderModuleMockMixin
+from tests.support.mock import DEFAULT, MagicMock, mock_open, patch
 from tests.support.unit import TestCase, skipIf
 
 # Import salt libs
@@ -59,6 +63,95 @@ class LinuxShadowTest(TestCase, LoaderModuleMockMixin):
                 hash_info["pw_hash"],
             )
 
+    def test_set_password(self):
+        """
+        Test the corner case in which shadow.set_password is called for a user
+        that has an entry in /etc/passwd but not /etc/shadow.
+        """
+        data = {
+            "/etc/shadow": salt.utils.stringutils.to_bytes(
+                textwrap.dedent(
+                    """\
+                foo:orighash:17955::::::
+                bar:somehash:17955::::::
+                """
+                )
+            ),
+            "*": Exception("Attempted to open something other than /etc/shadow"),
+        }
+        isfile_mock = MagicMock(
+            side_effect=lambda x: True if x == "/etc/shadow" else DEFAULT
+        )
+        password = "newhash"
+        shadow_info_mock = MagicMock(return_value={"passwd": password})
+
+        #
+        # CASE 1: Normal password change
+        #
+        user = "bar"
+        user_exists_mock = MagicMock(
+            side_effect=lambda x, **y: 0 if x == ["id", user] else DEFAULT
+        )
+        with patch(
+            "salt.utils.files.fopen", mock_open(read_data=data)
+        ) as shadow_mock, patch("os.path.isfile", isfile_mock), patch.object(
+            shadow, "info", shadow_info_mock
+        ), patch.dict(
+            shadow.__salt__, {"cmd.retcode": user_exists_mock}
+        ), patch.dict(
+            shadow.__grains__, {"os": "CentOS"}
+        ):
+            result = shadow.set_password(user, password, use_usermod=False)
+
+        assert result
+        filehandles = shadow_mock.filehandles["/etc/shadow"]
+        # We should only have opened twice, once to read the contents and once
+        # to write.
+        assert len(filehandles) == 2
+        # We're rewriting the entire file
+        assert filehandles[1].mode == "w+"
+        # We should be calling writelines instead of write, to rewrite the
+        # entire file.
+        assert len(filehandles[1].writelines_calls) == 1
+        # Make sure we wrote the correct info
+        lines = filehandles[1].writelines_calls[0]
+        # Should only have the same two users in the file
+        assert len(lines) == 2
+        # The first line should be unchanged
+        assert lines[0] == "foo:orighash:17955::::::\n"
+        # The second line should have the new password hash
+        assert lines[1].split(":")[:2] == [user, password]
+
+        #
+        # CASE 2: Corner case: no /etc/shadow entry for user
+        #
+        user = "baz"
+        user_exists_mock = MagicMock(
+            side_effect=lambda x, **y: 0 if x == ["id", user] else DEFAULT
+        )
+        with patch(
+            "salt.utils.files.fopen", mock_open(read_data=data)
+        ) as shadow_mock, patch("os.path.isfile", isfile_mock), patch.object(
+            shadow, "info", shadow_info_mock
+        ), patch.dict(
+            shadow.__salt__, {"cmd.retcode": user_exists_mock}
+        ), patch.dict(
+            shadow.__grains__, {"os": "CentOS"}
+        ):
+            result = shadow.set_password(user, password, use_usermod=False)
+
+        assert result
+        filehandles = shadow_mock.filehandles["/etc/shadow"]
+        # We should only have opened twice, once to read the contents and once
+        # to write.
+        assert len(filehandles) == 2
+        # We're just appending to the file, not rewriting
+        assert filehandles[1].mode == "a+"
+        # We should only have written to the file once
+        assert len(filehandles[1].write_calls) == 1
+        # Make sure we wrote the correct info
+        assert filehandles[1].write_calls[0].split(":")[:2] == [user, password]
+
     @skip_if_not_root
     def test_list_users(self):
         """

tests/unit/modules/test_nilrt_ip.py

@@ -0,0 +1,39 @@
+# -*- coding: utf-8 -*-
+
+# Import python libs
+from __future__ import absolute_import, print_function, unicode_literals
+
+# Import Salt Libs
+import salt.modules.nilrt_ip as nilrt_ip
+
+# Import Salt Testing Libs
+from tests.support.mixins import LoaderModuleMockMixin
+from tests.support.mock import patch
+from tests.support.unit import TestCase
+
+
+class NilrtIPTestCase(TestCase, LoaderModuleMockMixin):
+    """
+    TestCase for salt.modules.nilrt_ip module
+    """
+
+    def setup_loader_modules(self):
+        return {nilrt_ip: {"__grains__": {"lsb_distrib_id": "not_nilrt"}}}
+
+    def test_change_state_down_state(self):
+        """
+        Tests _change_state when not connected
+        and new state is down
+        """
+        with patch("salt.modules.nilrt_ip._interface_to_service", return_value=True):
+            with patch("salt.modules.nilrt_ip._connected", return_value=False):
+                assert nilrt_ip._change_state("test_interface", "down")
+
+    def test_change_state_up_state(self):
+        """
+        Tests _change_state when connected
+        and new state is up
+        """
+        with patch("salt.modules.nilrt_ip._interface_to_service", return_value=True):
+            with patch("salt.modules.nilrt_ip._connected", return_value=True):
+                assert nilrt_ip._change_state("test_interface", "up")

tests/unit/states/test_file.py

@@ -3,6 +3,7 @@
 # Import python libs
 from __future__ import absolute_import, print_function, unicode_literals
 
+import logging
 import os
 import pprint
 import shutil
@@ -40,6 +41,9 @@ except ImportError:
 NO_DATEUTIL_REASON = "python-dateutil is not installed"
 
 
+log = logging.getLogger(__name__)
+
+
 class TestFileState(TestCase, LoaderModuleMockMixin):
     def setup_loader_modules(self):
         return {
@@ -1433,6 +1437,8 @@ class TestFileState(TestCase, LoaderModuleMockMixin):
 
         ret = {"name": name, "result": False, "comment": "", "changes": {}}
 
+        check_perms_ret = {"name": name, "result": False, "comment": "", "changes": {}}
+
         comt = "Must provide name to file.directory"
         ret.update({"comment": comt, "name": ""})
         self.assertDictEqual(filestate.directory(""), ret)
@@ -1444,9 +1450,9 @@ class TestFileState(TestCase, LoaderModuleMockMixin):
         mock_t = MagicMock(return_value=True)
         mock_f = MagicMock(return_value=False)
         if salt.utils.platform.is_windows():
-            mock_perms = MagicMock(return_value=ret)
+            mock_perms = MagicMock(return_value=check_perms_ret)
         else:
-            mock_perms = MagicMock(return_value=(ret, ""))
+            mock_perms = MagicMock(return_value=(check_perms_ret, ""))
         mock_uid = MagicMock(
             side_effect=[
                 "",
@@ -1501,7 +1507,7 @@ class TestFileState(TestCase, LoaderModuleMockMixin):
             "salt.states.file._check_directory_win", mock_check
         ):
             if salt.utils.platform.is_windows():
-                comt = "User salt is not available Group salt" " is not available"
+                comt = ""
             else:
                 comt = "User salt is not available Group saltstack" " is not available"
             ret.update({"comment": comt, "name": name})
@@ -1576,38 +1582,123 @@ class TestFileState(TestCase, LoaderModuleMockMixin):
                                 filestate.directory(name, user=user, group=group), ret
                             )
 
+                        if salt.utils.platform.is_windows():
+                            isdir_side_effect = [False, True, False]
+                        else:
+                            isdir_side_effect = [True, False, True, False]
                         with patch.object(
-                            os.path,
-                            "isdir",
-                            MagicMock(side_effect=[True, False, True, True]),
+                            os.path, "isdir", MagicMock(side_effect=isdir_side_effect)
                         ):
                             comt = "Failed to create directory {0}".format(name)
-                            ret.update({"comment": comt, "result": False})
+                            ret.update(
+                                {
+                                    "comment": comt,
+                                    "result": False,
+                                    "changes": {name: "New Dir"},
+                                }
+                            )
                             self.assertDictEqual(
                                 filestate.directory(name, user=user, group=group), ret
                             )
 
+                        check_perms_ret = {
+                            "name": name,
+                            "result": False,
+                            "comment": "",
+                            "changes": {},
+                        }
+                        if salt.utils.platform.is_windows():
+                            mock_perms = MagicMock(return_value=check_perms_ret)
+                        else:
+                            mock_perms = MagicMock(return_value=(check_perms_ret, ""))
+
+                        recurse = ["silent"]
+                        ret = {
+                            "name": name,
+                            "result": False,
+                            "comment": "Directory /etc/testdir updated",
+                            "changes": {"recursion": "Changes silenced"},
+                        }
+                        if salt.utils.platform.is_windows():
+                            ret["comment"] = ret["comment"].replace("/", "\\")
+                        with patch.dict(
+                            filestate.__salt__, {"file.check_perms": mock_perms}
+                        ):
+                            with patch.object(os.path, "isdir", mock_t):
+                                self.assertDictEqual(
+                                    filestate.directory(
+                                        name, user=user, recurse=recurse, group=group
+                                    ),
+                                    ret,
+                                )
+
+                        check_perms_ret = {
+                            "name": name,
+                            "result": False,
+                            "comment": "",
+                            "changes": {},
+                        }
+                        if salt.utils.platform.is_windows():
+                            mock_perms = MagicMock(return_value=check_perms_ret)
+                        else:
+                            mock_perms = MagicMock(return_value=(check_perms_ret, ""))
+
                         recurse = ["ignore_files", "ignore_dirs"]
-                        ret.update(
-                            {
-                                "comment": 'Must not specify "recurse" '
-                                'options "ignore_files" and '
-                                '"ignore_dirs" at the same '
-                                "time.",
-                                "changes": {},
-                            }
-                        )
-                        with patch.object(os.path, "isdir", mock_t):
-                            self.assertDictEqual(
-                                filestate.directory(
-                                    name, user=user, recurse=recurse, group=group
-                                ),
-                                ret,
-                            )
+                        ret = {
+                            "name": name,
+                            "result": False,
+                            "comment": 'Must not specify "recurse" '
+                            'options "ignore_files" and '
+                            '"ignore_dirs" at the same '
+                            "time.",
+                            "changes": {},
+                        }
+                        with patch.dict(
+                            filestate.__salt__, {"file.check_perms": mock_perms}
+                        ):
+                            with patch.object(os.path, "isdir", mock_t):
+                                self.assertDictEqual(
+                                    filestate.directory(
+                                        name, user=user, recurse=recurse, group=group
+                                    ),
+                                    ret,
+                                )
 
-                            self.assertDictEqual(
-                                filestate.directory(name, user=user, group=group), ret
-                            )
+                        comt = "Directory {0} updated".format(name)
+                        ret = {
+                            "name": name,
+                            "result": True,
+                            "comment": comt,
+                            "changes": {
+                                "group": "group",
+                                "mode": "0777",
+                                "user": "user",
+                            },
+                        }
+
+                        check_perms_ret = {
+                            "name": name,
+                            "result": True,
+                            "comment": "",
+                            "changes": {
+                                "group": "group",
+                                "mode": "0777",
+                                "user": "user",
+                            },
+                        }
+
+                        if salt.utils.platform.is_windows():
+                            _mock_perms = MagicMock(return_value=check_perms_ret)
+                        else:
+                            _mock_perms = MagicMock(return_value=(check_perms_ret, ""))
+                        with patch.object(os.path, "isdir", mock_t):
+                            with patch.dict(
+                                filestate.__salt__, {"file.check_perms": _mock_perms}
+                            ):
+                                self.assertDictEqual(
+                                    filestate.directory(name, user=user, group=group),
+                                    ret,
+                                )
 
     # 'recurse' function tests: 1
 

tests/unit/utils/test_vt.py

@@ -12,6 +12,8 @@
 # Import Python libs
 from __future__ import absolute_import, print_function, unicode_literals
 
+import functools
+import io
 import os
 import random
 import subprocess
@@ -19,17 +21,56 @@ import sys
 import time
 
 # Import Salt libs
+import salt.utils
 import salt.utils.files
 import salt.utils.platform
+import salt.utils.stringutils
 import salt.utils.vt
 
 # Import 3rd-party libs
 from salt.ext.six.moves import range  # pylint: disable=import-error,redefined-builtin
 
 # Import Salt Testing libs
+from tests.support.paths import CODE_DIR
 from tests.support.unit import TestCase, skipIf
 
 
+def stdout_fileno_available():
+    """
+        Tests if sys.stdout.fileno is available in this testing environment
+    """
+    try:
+        sys.stdout.fileno()
+        return True
+    except io.UnsupportedOperation:
+        return False
+
+
+def fixStdOutErrFileNoIfNeeded(func):
+    """
+        Decorator that sets stdout and stderr to their original objects if
+        sys.stdout.fileno() doesn't work and restores them after running the
+        decorated function. This doesn't check if the original objects actually
+        work. If they don't then the test environment is too broken to test
+        the VT.
+    """
+
+    @functools.wraps(func)
+    def wrapper_fixStdOutErrFileNoIfNeeded(*args, **kwargs):
+        original_stdout = os.sys.stdout
+        original_stderr = os.sys.stderr
+        if not stdout_fileno_available():
+            os.sys.stdout = os.sys.__stdout__
+            os.sys.stderr = os.sys.__stderr__
+        try:
+            return func(*args, **kwargs)
+        finally:
+            os.sys.stdout = original_stdout
+            os.sys.stderr = original_stderr
+
+    return wrapper_fixStdOutErrFileNoIfNeeded
+
+
 class VTTestCase(TestCase):
     @skipIf(
         True,
@@ -220,3 +261,136 @@ class VTTestCase(TestCase):
             self.assertIsNone(stdout)
         finally:
             term.close(terminate=True, kill=True)
+
+    @staticmethod
+    def generate_multibyte_stdout_unicode(block_size):
+        return b"\xE2\x80\xA6" * 4 * block_size
+
+    @staticmethod
+    def generate_multibyte_stderr_unicode(block_size):
+        return b"\x2E" + VTTestCase.generate_multibyte_stdout_unicode(block_size)
+
+    @skipIf(
+        salt.utils.platform.is_windows(), "Skip VT tests on windows, due to issue 54290"
+    )
+    @fixStdOutErrFileNoIfNeeded
+    def test_split_multibyte_characters_unicode(self):
+        """
+            Tests that the vt correctly handles multibyte characters that are
+            split between blocks of transmitted data.
+        """
+        block_size = 1024
+        encoding = "utf-8"
+        stdout_content = VTTestCase.generate_multibyte_stdout_unicode(block_size)
+        # stderr is offset by one byte to guarentee a split character in
+        # one of the output streams
+        stderr_content = VTTestCase.generate_multibyte_stderr_unicode(block_size)
+
+        expected_stdout = salt.utils.stringutils.to_unicode(stdout_content, encoding)
+        expected_stderr = salt.utils.stringutils.to_unicode(stderr_content, encoding)
+        python_command = "\n".join(
+            (
+                "import sys",
+                "import os",
+                "import tests.unit.utils.test_vt as test_vt",
+                (
+                    "os.write(sys.stdout.fileno(), "
+                    "test_vt.VTTestCase.generate_multibyte_stdout_unicode("
+                    + str(block_size)
+                    + "))"
+                ),
+                (
+                    "os.write(sys.stderr.fileno(), "
+                    "test_vt.VTTestCase.generate_multibyte_stderr_unicode("
+                    + str(block_size)
+                    + "))"
+                ),
+            )
+        )
+        term = salt.utils.vt.Terminal(
+            args=[sys.executable, "-c", '"' + python_command + '"'],
+            shell=True,
+            cwd=CODE_DIR,
+            stream_stdout=False,
+            stream_stderr=False,
+            force_receive_encoding=encoding,
+        )
+        buffer_o = buffer_e = salt.utils.stringutils.to_unicode("")
+        try:
+            while term.has_unread_data:
+                stdout, stderr = term.recv(block_size)
+                if stdout:
+                    buffer_o += stdout
+                if stderr:
+                    buffer_e += stderr
+
+            self.assertEqual(buffer_o, expected_stdout)
+            self.assertEqual(buffer_e, expected_stderr)
+        finally:
+            term.close(terminate=True, kill=True)
+
+    @staticmethod
+    def generate_multibyte_stdout_shiftjis(block_size):
+        return b"\x8B\x80" * 4 * block_size
+
+    @staticmethod
+    def generate_multibyte_stderr_shiftjis(block_size):
+        return b"\x2E" + VTTestCase.generate_multibyte_stdout_shiftjis(block_size)
+
+    @skipIf(
+        salt.utils.platform.is_windows(), "Skip VT tests on windows, due to issue 54290"
+    )
+    @fixStdOutErrFileNoIfNeeded
+    def test_split_multibyte_characters_shiftjis(self):
+        """
+            Tests that the vt correctly handles multibyte characters that are
+            split between blocks of transmitted data.
+            Uses shift-jis encoding to make sure code doesn't assume unicode.
+        """
+        block_size = 1024
+        encoding = "shift-jis"
+        stdout_content = VTTestCase.generate_multibyte_stdout_shiftjis(block_size)
+        stderr_content = VTTestCase.generate_multibyte_stderr_shiftjis(block_size)
+
+        expected_stdout = salt.utils.stringutils.to_unicode(stdout_content, encoding)
+        expected_stderr = salt.utils.stringutils.to_unicode(stderr_content, encoding)
+        python_command = "\n".join(
+            (
+                "import sys",
+                "import os",
+                "import tests.unit.utils.test_vt as test_vt",
+                (
+                    "os.write(sys.stdout.fileno(), "
+                    "test_vt.VTTestCase.generate_multibyte_stdout_shiftjis("
+                    + str(block_size)
+                    + "))"
+                ),
+                (
+                    "os.write(sys.stderr.fileno(), "
+                    "test_vt.VTTestCase.generate_multibyte_stderr_shiftjis("
+                    + str(block_size)
+                    + "))"
+                ),
+            )
+        )
+        term = salt.utils.vt.Terminal(
+            args=[sys.executable, "-c", '"' + python_command + '"'],
+            shell=True,
+            cwd=CODE_DIR,
+            stream_stdout=False,
+            stream_stderr=False,
+            force_receive_encoding=encoding,
+        )
+        buffer_o = buffer_e = salt.utils.stringutils.to_unicode("")
+        try:
+            while term.has_unread_data:
+                stdout, stderr = term.recv(block_size)
+                if stdout:
+                    buffer_o += stdout
+                if stderr:
+                    buffer_e += stderr
+
+            self.assertEqual(buffer_o, expected_stdout)
+            self.assertEqual(buffer_e, expected_stderr)
+        finally:
+            term.close(terminate=True, kill=True)