From 619f2f2ec1a45441a1b64e62c848c3cc970db1ac Mon Sep 17 00:00:00 2001
From: Alexey Pronin <24279065+vulnbe@users.noreply.github.com>
Date: Wed, 17 Oct 2018 01:00:32 +0300
Subject: [PATCH 01/32] Fix incorrect id handling in file state

---
 salt/states/file.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/salt/states/file.py b/salt/states/file.py
index efe7c254f2c0..8f3c4a39e964 100644
--- a/salt/states/file.py
+++ b/salt/states/file.py
@@ -3188,7 +3188,7 @@ def directory(name,
             # NOTE: Should this be enough to stop the whole check altogether?
     if recurse_set:
         if 'user' in recurse_set:
-            if user:
+            if user or isinstance(user, int):
                 uid = __salt__['file.user_to_uid'](user)
                 # file.user_to_uid returns '' if user does not exist. Above
                 # check for user is not fatal, so we need to be sure user
@@ -3206,7 +3206,7 @@ def directory(name,
         else:
             user = None
         if 'group' in recurse_set:
-            if group:
+            if group or isinstance(group, int):
                 gid = __salt__['file.group_to_gid'](group)
                 # As above with user, we need to make sure group exists.
                 if isinstance(gid, six.string_types):

From 1e10c6e589e2c49ddd59ac6251cb59f83d399f4c Mon Sep 17 00:00:00 2001
From: Jodok Batlogg <jodok@batlogg.com>
Date: Sun, 23 Sep 2018 21:51:37 +0200
Subject: [PATCH 02/32] add support for clonenum parameter

---
 salt/templates/rh_ip/rh7_eth.jinja | 1 +
 1 file changed, 1 insertion(+)

diff --git a/salt/templates/rh_ip/rh7_eth.jinja b/salt/templates/rh_ip/rh7_eth.jinja
index 5fec4fd7b075..78e6d314ad3e 100644
--- a/salt/templates/rh_ip/rh7_eth.jinja
+++ b/salt/templates/rh_ip/rh7_eth.jinja
@@ -17,6 +17,7 @@ DEVICE="{{name}}"
 {%endif%}{% if ipaddr %}IPADDR="{{ipaddr}}"
 {%endif%}{% if ipaddr_start %}IPADDR_START="{{ipaddr_start}}"
 {%endif%}{% if ipaddr_end %}IPADDR_END="{{ipaddr_end}}"
+{%endif%}{% if clonenum_start %}CLONENUM_START="{{clonenum_start}}"
 {%endif%}{% if netmask %}NETMASK="{{netmask}}"
 {%endif%}{% if prefix %}PREFIX="{{prefix}}"
 {%endif%}{% if ipaddrs %}{% for i in ipaddrs -%}

From 92b5ea023ea463843f9baa9496fb025b0f883fba Mon Sep 17 00:00:00 2001
From: twangboy <slee@saltstack.com>
Date: Mon, 29 Oct 2018 14:06:30 -0600
Subject: [PATCH 03/32] Create salt util that copies file security info

---
 salt/utils/atomicfile.py |   6 +-
 salt/utils/win_dacl.py   | 170 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 172 insertions(+), 4 deletions(-)

diff --git a/salt/utils/atomicfile.py b/salt/utils/atomicfile.py
index 7a9338bddee1..3658a5150cab 100644
--- a/salt/utils/atomicfile.py
+++ b/salt/utils/atomicfile.py
@@ -122,11 +122,11 @@ def close(self):
             return
         self._fh.close()
         if os.path.isfile(self._filename):
-            shutil.copymode(self._filename, self._tmp_filename)
             if salt.utils.win_dacl.HAS_WIN32:
-                owner = salt.utils.win_dacl.get_owner(self._filename)
-                salt.utils.win_dacl.set_owner(self._tmp_filename, owner)
+                salt.utils.win_dacl.copy_security(
+                    source=self._filename, target=self._tmp_filename)
             else:
+                shutil.copymode(self._filename, self._tmp_filename)
                 st = os.stat(self._filename)
                 os.chown(self._tmp_filename, st.st_uid, st.st_gid)
         atomic_rename(self._tmp_filename, self._filename)
diff --git a/salt/utils/win_dacl.py b/salt/utils/win_dacl.py
index 221aaeb5c0f8..c88ee2ac6ed2 100644
--- a/salt/utils/win_dacl.py
+++ b/salt/utils/win_dacl.py
@@ -1432,7 +1432,7 @@ def set_permissions(obj_name,
         obj_dacl = dacl(obj_type=obj_type)
     else:
         obj_dacl = dacl(obj_name, obj_type)
-        obj_dacl.rm_ace(principal, access_mode)
+        obj_dacl.re_ace(principal, access_mode)
 
     obj_dacl.add_ace(principal, access_mode, permissions, applies_to)
 
@@ -1689,3 +1689,171 @@ def get_inheritance(obj_name, obj_type='file'):
             return True
 
     return False
+
+
+def copy_security(source,
+                  target,
+                  obj_type='file',
+                  copy_owner=True,
+                  copy_group=True,
+                  copy_dacl=True,
+                  copy_sacl=True):
+    '''
+    Copy the security descriptor of the Source to the Target. You can specify a
+    specific portion of the security descripto to copy using one of the `copy_*`
+    parameters.
+
+    .. note::
+        At least one `copy_*` parameter must be ``True``
+
+    .. note::
+        The user account running this command must have the following
+        privileges:
+
+        - SeTakeOwnershipPrivilege
+        - SeRestorePrivilege
+        - SeSecurityPrivilege
+
+    Args:
+
+        source (str):
+            The full path to the source. This is where the security info will be
+            copied from
+
+        target (str):
+            The full path to the target. This is where the security info will be
+            applied
+
+        obj_type (str): file
+            The type of object to query. This value changes the format of the
+            ``obj_name`` parameter as follows:
+            - file: indicates a file or directory
+                - a relative path, such as ``FileName.txt`` or ``..\FileName``
+                - an absolute path, such as ``C:\DirName\FileName.txt``
+                - A UNC name, such as ``\\ServerName\ShareName\FileName.txt``
+            - service: indicates the name of a Windows service
+            - printer: indicates the name of a printer
+            - registry: indicates a registry key
+                - Uses the following literal strings to denote the hive:
+                    - HKEY_LOCAL_MACHINE
+                    - MACHINE
+                    - HKLM
+                    - HKEY_USERS
+                    - USERS
+                    - HKU
+                    - HKEY_CURRENT_USER
+                    - CURRENT_USER
+                    - HKCU
+                    - HKEY_CLASSES_ROOT
+                    - CLASSES_ROOT
+                    - HKCR
+                - Should be in the format of ``HIVE\Path\To\Key``. For example,
+                    ``HKLM\SOFTWARE\Windows``
+            - registry32: indicates a registry key under WOW64. Formatting is
+                the same as it is for ``registry``
+            - share: indicates a network share
+
+        copy_owner (bool): True
+            ``True`` copies owner information. Default is ``True``
+
+        copy_group (bool): True
+            ``True`` copies group information. Default is ``True``
+
+        copy_dacl (bool): True
+            ``True`` copies the DACL. Default is ``True``
+
+        copy_sacl (bool): True
+            ``True`` copies the SACL. Default is ``True``
+
+    Returns:
+        bool: ``True`` if successful
+
+    Raises:
+        SaltInvocationError: When parameters are invalid
+        CommandExecutionError: On failure to set security
+
+    Usage:
+
+    .. code-block:: python
+
+        salt.utils.win_dacl.copy_security(
+            source='C:\\temp\\source_file.txt',
+            target='C:\\temp\\target_file.txt',
+            obj_type='file')
+
+        salt.utils.win_dacl.copy_security(
+            source='HKLM\\SOFTWARE\\salt\\test_source',
+            target='HKLM\\SOFTWARE\\salt\\test_target',
+            obj_type='registry',
+            copy_owner=False)
+    '''
+    obj_dacl = dacl(obj_type=obj_type)
+    if 'registry' in obj_type.lower():
+        source = obj_dacl.get_reg_name(source)
+        log.info('Source converted to: %s', source)
+        target = obj_dacl.get_reg_name(target)
+        log.info('Target converted to: %s', target)
+
+    # Set flags
+    try:
+        obj_type_flag = flags().obj_type[obj_type.lower()]
+    except KeyError:
+        raise SaltInvocationError(
+            'Invalid "obj_type" passed: {0}'.format(obj_type))
+
+    security_flags = 0
+    if copy_owner:
+        security_flags |= win32security.OWNER_SECURITY_INFORMATION
+    if copy_group:
+        security_flags |= win32security.GROUP_SECURITY_INFORMATION
+    if copy_dacl:
+        security_flags |= win32security.DACL_SECURITY_INFORMATION
+    if copy_sacl:
+        security_flags |= win32security.SACL_SECURITY_INFORMATION
+
+    if not security_flags:
+        raise SaltInvocationError(
+            'One of copy_owner, copy_group, copy_dacl, or copy_sacl must be '
+            'True')
+
+    # To set the owner to something other than the logged in user requires
+    # SE_TAKE_OWNERSHIP_NAME and SE_RESTORE_NAME privileges
+    # Enable them for the logged in user
+    # Setup the privilege set
+    new_privs = set()
+    luid = win32security.LookupPrivilegeValue('', 'SeTakeOwnershipPrivilege')
+    new_privs.add((luid, win32con.SE_PRIVILEGE_ENABLED))
+    luid = win32security.LookupPrivilegeValue('', 'SeRestorePrivilege')
+    new_privs.add((luid, win32con.SE_PRIVILEGE_ENABLED))
+    luid = win32security.LookupPrivilegeValue('', 'SeSecurityPrivilege')
+    new_privs.add((luid, win32con.SE_PRIVILEGE_ENABLED))
+
+    # Get the current token
+    p_handle = win32api.GetCurrentProcess()
+    t_handle = win32security.OpenProcessToken(
+        p_handle,
+        win32security.TOKEN_ALL_ACCESS | win32con.TOKEN_ADJUST_PRIVILEGES)
+
+    # Enable the privileges
+    win32security.AdjustTokenPrivileges(t_handle, 0, new_privs)
+
+    # Load object Security Info from the Source
+    sec = win32security.GetNamedSecurityInfo(
+        source, obj_type_flag, security_flags)
+
+    # The following return None if the corresponding flag is not set
+    sd_sid = sec.GetSecurityDescriptorOwner()
+    sd_gid = sec.GetSecurityDescriptorGroup()
+    sd_dacl = sec.GetSecurityDescriptorDacl()
+    sd_sacl = sec.GetSecurityDescriptorSacl()
+
+    # Set Security info on the target
+    try:
+        win32security.SetNamedSecurityInfo(
+            target, obj_type_flag, security_flags, sd_sid, sd_gid, sd_dacl,
+            sd_sacl)
+    except pywintypes.error as exc:
+        raise CommandExecutionError(
+            'Failed to set security info: {0}'.format(exc.strerror))
+
+    return True

From d18401370e8a908fbd58c7e0597d3e17ec119bf0 Mon Sep 17 00:00:00 2001
From: twangboy <slee@saltstack.com>
Date: Mon, 29 Oct 2018 14:15:07 -0600
Subject: [PATCH 04/32] Fix typo

---
 salt/utils/win_dacl.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/salt/utils/win_dacl.py b/salt/utils/win_dacl.py
index c88ee2ac6ed2..b640a1ac25b4 100644
--- a/salt/utils/win_dacl.py
+++ b/salt/utils/win_dacl.py
@@ -1432,7 +1432,7 @@ def set_permissions(obj_name,
         obj_dacl = dacl(obj_type=obj_type)
     else:
         obj_dacl = dacl(obj_name, obj_type)
-        obj_dacl.re_ace(principal, access_mode)
+        obj_dacl.rm_ace(principal, access_mode)
 
     obj_dacl.add_ace(principal, access_mode, permissions, applies_to)
 

From 1ef712a178b14ea63f44fec930ac839af08b40b6 Mon Sep 17 00:00:00 2001
From: rallytime <nicole@saltstack.com>
Date: Thu, 25 Oct 2018 14:37:35 -0400
Subject: [PATCH 05/32] Add various ssh tests to the filemap for
 salt/utils/vt.py changes

Some of the salt-ssh tests use the salt/utils/vt.py functions. A recent
change in that file caused about 90 tests to start failing on the branch.

We need to make sure these tests run when changes to the vt.py util are
made in PRs.
---
 tests/filename_map.yml | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tests/filename_map.yml b/tests/filename_map.yml
index 0f768f2dacc3..76df97f9e4cb 100644
--- a/tests/filename_map.yml
+++ b/tests/filename_map.yml
@@ -220,6 +220,16 @@ salt/utils/schedule.py:
   - integration.scheduler.test_postpone
   - integration.scheduler.test_skip
 
+salt/utils/vt.py:
+  - integration.cli.test_custom_module
+  - integration.cli.test_grains
+  - integration.ssh.test_grains
+  - integration.ssh.test_jinja_filters
+  - integration.ssh.test_mine
+  - integration.ssh.test_pillar
+  - integration.ssh.test_raw
+  - integration.ssh.test_state
+
 salt/wheel/*:
   - integration.wheel.test_client
 

From 69c02fca4a9d2840f1eb44bc4351d39d273e77c6 Mon Sep 17 00:00:00 2001
From: twangboy <slee@saltstack.com>
Date: Mon, 29 Oct 2018 15:34:48 -0600
Subject: [PATCH 06/32] Fix some lint and some typos

---
 salt/utils/win_dacl.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/salt/utils/win_dacl.py b/salt/utils/win_dacl.py
index b640a1ac25b4..5f57ffc29429 100644
--- a/salt/utils/win_dacl.py
+++ b/salt/utils/win_dacl.py
@@ -1698,10 +1698,10 @@ def copy_security(source,
                   copy_group=True,
                   copy_dacl=True,
                   copy_sacl=True):
-    '''
+    r'''
     Copy the security descriptor of the Source to the Target. You can specify a
-    specific portion of the security descripto to copy using one of the `copy_*`
-    parameters.
+    specific portion of the security descriptor to copy using one of the
+    `copy_*` parameters.
 
     .. note::
         At least one `copy_*` parameter must be ``True``

From 20bf3067b90653540e0efa227368bdb76f123f11 Mon Sep 17 00:00:00 2001
From: Anthony Shaw <anthonyshaw@apache.org>
Date: Tue, 30 Oct 2018 15:47:19 +1100
Subject: [PATCH 07/32] reproduce bug in 47425

---
 tests/unit/states/test_netyang.py | 41 +++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 tests/unit/states/test_netyang.py

diff --git a/tests/unit/states/test_netyang.py b/tests/unit/states/test_netyang.py
new file mode 100644
index 000000000000..da97a40fd9cb
--- /dev/null
+++ b/tests/unit/states/test_netyang.py
@@ -0,0 +1,41 @@
+# -*- coding: utf-8 -*-
+'''
+    :codeauthor: Anthony Shaw <anthonyshaw@apache.org>
+'''
+# Import Python libs
+from __future__ import absolute_import, print_function, unicode_literals
+import os
+
+# Import Salt Libs
+import salt.states.netyang as netyang
+
+# Import Salt Testing Libs
+from tests.support.mixins import LoaderModuleMockMixin
+from tests.support.unit import skipIf, TestCase
+from tests.support.mock import (
+    NO_MOCK,
+    NO_MOCK_REASON,
+    MagicMock,
+    patch
+)
+
+TEST_DATA = {
+    'foo': 'bar'
+}
+
+@skipIf(NO_MOCK, NO_MOCK_REASON)
+class NetyangTestCase(TestCase, LoaderModuleMockMixin):
+    def setup_loader_modules(self):
+        return {netyang: {}}
+
+
+    def test_managed(self):
+        ret = {'changes': {}, 'comment': '',
+               'name': 'salt', 'result': True}
+        revision_mock = MagicMock(return_value='abcdef')
+
+        with patch.dict(netyang.__salt__,
+                        {'hg.revision': revision_mock,}):
+            with patch.dict(netyang.__opts__, {'test': False}):
+                self.assertDictEqual(netyang.managed('test', ('model1',)), ret)
+                assert revision_mock.called

From 220e57f84814a301d12b71fea58d600937ef0ed6 Mon Sep 17 00:00:00 2001
From: Anthony Shaw <anthonyshaw@apache.org>
Date: Tue, 30 Oct 2018 16:08:37 +1100
Subject: [PATCH 08/32] create 2 simple unit tests for the netyang state module
 and fix issue in #47425

---
 salt/states/netyang.py            |  4 +--
 tests/unit/states/test_netyang.py | 46 ++++++++++++++++++++++++-------
 2 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/salt/states/netyang.py b/salt/states/netyang.py
index e5914768bd71..fc7a0633ad17 100644
--- a/salt/states/netyang.py
+++ b/salt/states/netyang.py
@@ -77,7 +77,6 @@ def __virtual__():
 
 def managed(name,
             data,
-            *models,
             **kwargs):
     '''
     Manage the device configuration given the input data structured
@@ -143,6 +142,7 @@ def managed(name,
                 config:
                   description: "description example"
     '''
+    models = kwargs.get('models', None)
     if isinstance(models, tuple) and isinstance(models[0], list):
         models = models[0]
     ret = salt.utils.napalm.default_ret(name)
@@ -206,7 +206,6 @@ def managed(name,
 
 def configured(name,
                data,
-               *models,
                **kwargs):
     '''
     Configure the network device, given the input data strucuted
@@ -276,6 +275,7 @@ def configured(name,
                 config:
                   description: "description example"
     '''
+    models = kwargs.get('models', None)
     if isinstance(models, tuple) and isinstance(models[0], list):
         models = models[0]
     ret = salt.utils.napalm.default_ret(name)
diff --git a/tests/unit/states/test_netyang.py b/tests/unit/states/test_netyang.py
index da97a40fd9cb..6c933c206189 100644
--- a/tests/unit/states/test_netyang.py
+++ b/tests/unit/states/test_netyang.py
@@ -23,19 +23,45 @@
     'foo': 'bar'
 }
 
+
 @skipIf(NO_MOCK, NO_MOCK_REASON)
 class NetyangTestCase(TestCase, LoaderModuleMockMixin):
     def setup_loader_modules(self):
         return {netyang: {}}
 
-
     def test_managed(self):
-        ret = {'changes': {}, 'comment': '',
-               'name': 'salt', 'result': True}
-        revision_mock = MagicMock(return_value='abcdef')
-
-        with patch.dict(netyang.__salt__,
-                        {'hg.revision': revision_mock,}):
-            with patch.dict(netyang.__opts__, {'test': False}):
-                self.assertDictEqual(netyang.managed('test', ('model1',)), ret)
-                assert revision_mock.called
+        ret = {'changes': {}, 'comment': 'Loaded.',
+               'name': 'test', 'result': False}
+        parse = MagicMock(return_value='abcdef')
+        temp_file = MagicMock(return_value='')
+        compliance_report = MagicMock(return_value={'complies': False})
+        load_config = MagicMock(return_value={'comment': 'Loaded.'})
+        file_remove = MagicMock()
+
+        with patch('salt.utils.files.fopen'):
+            with patch.dict(netyang.__salt__,
+                            {'temp.file': temp_file,
+                             'napalm_yang.parse': parse,
+                             'napalm_yang.load_config': load_config,
+                             'napalm_yang.compliance_report': compliance_report,
+                             'file.remove': file_remove}):
+                with patch.dict(netyang.__opts__, {'test': False}):
+                    self.assertDictEqual(netyang.managed('test', 'test', models=('model1',)), ret)
+                    assert parse.called
+                    assert temp_file.called
+                    assert compliance_report.called
+                    assert load_config.called
+                    assert file_remove.called
+
+    def test_configured(self):
+        ret = {'changes': {}, 'comment': 'Loaded.',
+               'name': 'test', 'result': False}
+        load_config = MagicMock(return_value={'comment': 'Loaded.'})
+
+        with patch('salt.utils.files.fopen'):
+            with patch.dict(netyang.__salt__,
+                            {'napalm_yang.load_config': load_config}):
+                with patch.dict(netyang.__opts__, {'test': False}):
+                    self.assertDictEqual(netyang.configured('test', 'test', models=('model1',)), ret)
+
+                    assert load_config.called

From ba526ad149837baccc2588cf5fe52576f5ec9ba7 Mon Sep 17 00:00:00 2001
From: Anthony Shaw <anthonyshaw@apache.org>
Date: Tue, 30 Oct 2018 19:16:14 +1100
Subject: [PATCH 09/32] remove unused import

---
 tests/unit/states/test_netyang.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/unit/states/test_netyang.py b/tests/unit/states/test_netyang.py
index 6c933c206189..08df735b93c2 100644
--- a/tests/unit/states/test_netyang.py
+++ b/tests/unit/states/test_netyang.py
@@ -4,7 +4,6 @@
 '''
 # Import Python libs
 from __future__ import absolute_import, print_function, unicode_literals
-import os
 
 # Import Salt Libs
 import salt.states.netyang as netyang

From 529687a8559c3e1bda4c852298f0fbade3c2c415 Mon Sep 17 00:00:00 2001
From: Anthony Shaw <anthonyshaw@apache.org>
Date: Thu, 1 Nov 2018 08:41:53 +1100
Subject: [PATCH 10/32] update test assertions

---
 tests/unit/states/test_netyang.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/unit/states/test_netyang.py b/tests/unit/states/test_netyang.py
index 08df735b93c2..dd00a01cc5c5 100644
--- a/tests/unit/states/test_netyang.py
+++ b/tests/unit/states/test_netyang.py
@@ -30,7 +30,8 @@ def setup_loader_modules(self):
 
     def test_managed(self):
         ret = {'changes': {}, 'comment': 'Loaded.',
-               'name': 'test', 'result': False}
+               'name': 'test', 'result': False,
+               'pchanges': {'compliance_report': {'complies': False}}}
         parse = MagicMock(return_value='abcdef')
         temp_file = MagicMock(return_value='')
         compliance_report = MagicMock(return_value={'complies': False})
@@ -54,7 +55,8 @@ def test_managed(self):
 
     def test_configured(self):
         ret = {'changes': {}, 'comment': 'Loaded.',
-               'name': 'test', 'result': False}
+               'name': 'test', 'result': False,
+               'pchanges': {}}
         load_config = MagicMock(return_value={'comment': 'Loaded.'})
 
         with patch('salt.utils.files.fopen'):

From 56c87ff6900bde45f01966b61fb87c665f4505f8 Mon Sep 17 00:00:00 2001
From: "Gareth J. Greenaway" <gareth@wiked.org>
Date: Thu, 1 Nov 2018 08:46:31 -0700
Subject: [PATCH 11/32] Backporting #48087 to 2018.3

---
 salt/modules/freebsd_sysctl.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/salt/modules/freebsd_sysctl.py b/salt/modules/freebsd_sysctl.py
index 630147c066e0..7265748fa3d3 100644
--- a/salt/modules/freebsd_sysctl.py
+++ b/salt/modules/freebsd_sysctl.py
@@ -165,7 +165,7 @@ def persist(name, value, config='/etc/sysctl.conf'):
     if not edited:
         nlines.append("{0}\n".format(_formatfor(name, value, config)))
     with salt.utils.files.fopen(config, 'w+') as ofile:
-        nlines = [salt.utils.stringutils.to_str(_l) for _l in nlines]
+        nlines = [salt.utils.stringutils.to_str(_l) + '\n' for _l in nlines]
         ofile.writelines(nlines)
     if config != '/boot/loader.conf':
         assign(name, value)

From f3dee43bf0b01f84dedfddc5ee3308bf216db45d Mon Sep 17 00:00:00 2001
From: "Gareth J. Greenaway" <gareth@wiked.org>
Date: Thu, 1 Nov 2018 15:02:21 -0700
Subject: [PATCH 12/32] Updating the beacon state module to ensure that the
 format of the beacon data that is being sent along to the beacon execution
 module is in the right format.

---
 salt/modules/beacons.py                 |  4 +-
 salt/states/beacon.py                   |  8 +++-
 tests/integration/states/test_beacon.py | 49 +++++++++++++++++++++++++
 3 files changed, 58 insertions(+), 3 deletions(-)
 create mode 100644 tests/integration/states/test_beacon.py

diff --git a/salt/modules/beacons.py b/salt/modules/beacons.py
index 3df9fa829ea2..5f440a10c40b 100644
--- a/salt/modules/beacons.py
+++ b/salt/modules/beacons.py
@@ -134,7 +134,7 @@ def add(name, beacon_data, **kwargs):
 
     .. code-block:: bash
 
-        salt '*' beacons.add ps "[{'salt-master': 'stopped', 'apache2': 'stopped'}]"
+        salt '*' beacons.add ps "[{'salt-master': 'stopped'}, {'apache2': 'stopped'}]"
 
     '''
     ret = {'comment': 'Failed to add beacon {0}.'.format(name),
@@ -207,7 +207,7 @@ def modify(name, beacon_data, **kwargs):
 
     .. code-block:: bash
 
-        salt '*' beacons.modify ps "[{'salt-master': 'stopped', 'apache2': 'stopped'}]"
+        salt '*' beacons.modify ps "[{'salt-master': 'stopped'}, {'apache2': 'stopped'}]"
     '''
 
     ret = {'comment': '',
diff --git a/salt/states/beacon.py b/salt/states/beacon.py
index 6d7aa0e392fa..362a2c365a4e 100644
--- a/salt/states/beacon.py
+++ b/salt/states/beacon.py
@@ -34,6 +34,12 @@
 '''
 from __future__ import absolute_import, print_function, unicode_literals
 
+# Import Salt libs
+from salt.ext import six
+
+import logging
+log = logging.getLogger(__name__)
+
 
 def present(name,
             save=False,
@@ -54,7 +60,7 @@ def present(name,
            'comment': []}
 
     current_beacons = __salt__['beacons.list'](return_yaml=False)
-    beacon_data = [kwargs]
+    beacon_data = [{k: v} for k, v in six.iteritems(kwargs)]
 
     if name in current_beacons:
 
diff --git a/tests/integration/states/test_beacon.py b/tests/integration/states/test_beacon.py
new file mode 100644
index 000000000000..97b039cda302
--- /dev/null
+++ b/tests/integration/states/test_beacon.py
@@ -0,0 +1,49 @@
+# -*- coding: utf-8 -*-
+'''
+Integration tests for the beacon states
+'''
+
+# Import Python Libs
+from __future__ import absolute_import, print_function, unicode_literals
+
+# Import Salt Testing Libs
+from tests.support.case import ModuleCase
+from tests.support.helpers import destructiveTest
+
+import logging
+log = logging.getLogger(__name__)
+
+
+@destructiveTest
+class BeaconStateTestCase(ModuleCase):
+    '''
+    Test zookeeper states
+    '''
+    def setUp(self):
+        '''
+        '''
+        pass
+
+    def tearDown(self):
+        pass
+
+    def test_present_absent(self):
+        kwargs = {'/': '38%', 'interval': 5}
+        ret = self.run_state(
+            'beacon.present',
+            name='diskusage',
+            **kwargs
+        )
+        self.assertSaltTrueReturn(ret)
+
+        ret = self.run_function('beacons.list', return_yaml=False)
+        self.assertEqual(ret, {'diskusage': [{'interval': 5}, {'/': u'38%'}]})
+
+        ret = self.run_state(
+            'beacon.absent',
+            name='diskusage',
+        )
+        self.assertSaltTrueReturn(ret)
+
+        ret = self.run_function('beacons.list', return_yaml=False)
+        self.assertEqual(ret, {'beacons': {}})

From 677d0908fd7d25bbb68bef80b6fa4e45a26f4e5a Mon Sep 17 00:00:00 2001
From: "Gareth J. Greenaway" <gareth@wiked.org>
Date: Thu, 1 Nov 2018 15:05:42 -0700
Subject: [PATCH 13/32] Remove zookeeper reference.

---
 tests/integration/states/test_beacon.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/integration/states/test_beacon.py b/tests/integration/states/test_beacon.py
index 97b039cda302..2c22842b9c4c 100644
--- a/tests/integration/states/test_beacon.py
+++ b/tests/integration/states/test_beacon.py
@@ -17,7 +17,7 @@
 @destructiveTest
 class BeaconStateTestCase(ModuleCase):
     '''
-    Test zookeeper states
+    Test beacon states
     '''
     def setUp(self):
         '''

From 2390f471ce2be48db78b32088d28468989294100 Mon Sep 17 00:00:00 2001
From: "Gareth J. Greenaway" <gareth@wiked.org>
Date: Thu, 1 Nov 2018 15:22:18 -0700
Subject: [PATCH 14/32] Adding SaltReturnAssertsMixin back in

---
 tests/integration/states/test_beacon.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/integration/states/test_beacon.py b/tests/integration/states/test_beacon.py
index 2c22842b9c4c..9f5c2a8d2958 100644
--- a/tests/integration/states/test_beacon.py
+++ b/tests/integration/states/test_beacon.py
@@ -9,13 +9,14 @@
 # Import Salt Testing Libs
 from tests.support.case import ModuleCase
 from tests.support.helpers import destructiveTest
+from tests.support.mixins import SaltReturnAssertsMixin
 
 import logging
 log = logging.getLogger(__name__)
 
 
 @destructiveTest
-class BeaconStateTestCase(ModuleCase):
+class BeaconStateTestCase(ModuleCase, SaltReturnAssertsMixin):
     '''
     Test beacon states
     '''

From c2354a9cb1cbb6b431deffe00f3c872514bc99f8 Mon Sep 17 00:00:00 2001
From: "Gareth J. Greenaway" <gareth@wiked.org>
Date: Thu, 1 Nov 2018 16:47:02 -0700
Subject: [PATCH 15/32] Fixing beacon state test.  Adding a reset function to
 beacon module to clear out beacon configuration.  Useful for tests runs to
 ensure bits are left over between runs.

---
 salt/beacons/__init__.py                  |  6 ++++
 salt/minion.py                            |  2 ++
 salt/modules/beacons.py                   | 35 +++++++++++++++++++++++
 tests/integration/modules/test_beacons.py |  6 ++++
 tests/integration/states/test_beacon.py   |  8 ++++--
 5 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/salt/beacons/__init__.py b/salt/beacons/__init__.py
index c91f4c6fdffd..d464e247402d 100644
--- a/salt/beacons/__init__.py
+++ b/salt/beacons/__init__.py
@@ -428,3 +428,9 @@ def disable_beacon(self, name):
                        tag='/salt/minion/minion_beacon_disabled_complete')
 
         return True
+
+    def reset(self):
+        '''
+        Reset the beacons to defaults
+        '''
+        self.opts['beacons'] = {}
diff --git a/salt/minion.py b/salt/minion.py
index 6dd72778d8bf..5e59db2a6e2f 100644
--- a/salt/minion.py
+++ b/salt/minion.py
@@ -2214,6 +2214,8 @@ def manage_beacons(self, tag, data):
             self.beacons.list_available_beacons()
         elif func == 'validate_beacon':
             self.beacons.validate_beacon(name, beacon_data)
+        elif func == 'reset':
+            self.beacons.reset()
 
     def environ_setenv(self, tag, data):
         '''
diff --git a/salt/modules/beacons.py b/salt/modules/beacons.py
index 5f440a10c40b..cd2694431589 100644
--- a/salt/modules/beacons.py
+++ b/salt/modules/beacons.py
@@ -571,3 +571,38 @@ def disable_beacon(name, **kwargs):
             # Effectively a no-op, since we can't really return without an event system
             ret['comment'] = 'Event module not available. Beacon disable job failed.'
     return ret
+
+
+def reset(**kwargs):
+    '''
+    Resest beacon configuration on the minion
+
+    CLI Example:
+
+    .. code-block:: bash
+
+        salt '*' beacons.reset
+    '''
+
+    ret = {'comment': [],
+           'result': True}
+
+    if 'test' in kwargs and kwargs['test']:
+        ret['comment'] = 'Beacons would be reset.'
+    else:
+        try:
+            eventer = salt.utils.event.get_event('minion', opts=__opts__)
+            res = __salt__['event.fire']({'func': 'reset'}, 'manage_beacons')
+            if res:
+                event_ret = eventer.get_event(tag='/salt/minion/minion_beacon_reset_complete', wait=30)
+                if event_ret and event_ret['complete']:
+                    ret['result'] = True
+                    ret['comment'] = 'Beacon configuration reset.'
+                else:
+                    ret['result'] = False
+                    ret['comment'] = event_ret['comment']
+                return ret
+        except KeyError:
+            # Effectively a no-op, since we can't really return without an event system
+            ret['comment'] = 'Event module not available. Beacon disable job failed.'
+    return ret
diff --git a/tests/integration/modules/test_beacons.py b/tests/integration/modules/test_beacons.py
index 9cd11a734885..e1cd333705ae 100644
--- a/tests/integration/modules/test_beacons.py
+++ b/tests/integration/modules/test_beacons.py
@@ -31,6 +31,9 @@ def tearDown(self):
         if os.path.isfile(self.beacons_config_file_path):
             os.unlink(self.beacons_config_file_path)
 
+        # Reset beacons
+        self.run_function('beacons.reset')
+
     def test_add_and_delete(self):
         '''
         Test adding and deleting a beacon
@@ -81,6 +84,9 @@ def tearDown(self):
         self.run_function('beacons.delete', ['ps'])
         self.run_function('beacons.save')
 
+        # Reset beacons
+        self.run_function('beacons.reset')
+
     def test_disable(self):
         '''
         Test disabling beacons
diff --git a/tests/integration/states/test_beacon.py b/tests/integration/states/test_beacon.py
index 9f5c2a8d2958..e6914db36135 100644
--- a/tests/integration/states/test_beacon.py
+++ b/tests/integration/states/test_beacon.py
@@ -23,10 +23,10 @@ class BeaconStateTestCase(ModuleCase, SaltReturnAssertsMixin):
     def setUp(self):
         '''
         '''
-        pass
+        self.run_function('beacons.reset')
 
     def tearDown(self):
-        pass
+        self.run_function('beacons.reset')
 
     def test_present_absent(self):
         kwargs = {'/': '38%', 'interval': 5}
@@ -38,7 +38,9 @@ def test_present_absent(self):
         self.assertSaltTrueReturn(ret)
 
         ret = self.run_function('beacons.list', return_yaml=False)
-        self.assertEqual(ret, {'diskusage': [{'interval': 5}, {'/': u'38%'}]})
+        self.assertTrue('diskusage' in ret)
+        self.assertTrue({'interval': 5} in ret['diskusage'])
+        self.assertTrue({'/': '38%'} in ret['diskusage'])
 
         ret = self.run_state(
             'beacon.absent',

From d9fe28a29a4e7149057e923fd6095821c952b22a Mon Sep 17 00:00:00 2001
From: lomeroe <github@proxyman.com>
Date: Thu, 4 Oct 2018 13:18:45 -0500
Subject: [PATCH 16/32] fix clobbering of admx_search_results which was keeping
 some policies from being properly detected

add a handful of debug statements to help with debugging admx/adml
policy discovery

validate text attribute exists on item before joining it with prepended
text
---
 salt/modules/win_lgpo.py | 31 ++++++++++++++++++++++++-------
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/salt/modules/win_lgpo.py b/salt/modules/win_lgpo.py
index 68e517990d19..f566b9646016 100644
--- a/salt/modules/win_lgpo.py
+++ b/salt/modules/win_lgpo.py
@@ -3094,9 +3094,15 @@ def _getAdmlPresentationRefId(adml_data, ref_id):
                         else:
                             if etree.QName(p_item.tag).localname == 'text':
                                 if prepended_text:
-                                    prepended_text = ' '.join([prepended_text, p_item.text.rstrip()])
+                                    if getattr(p_item, 'text', ''):
+                                        prepended_text = ' '.join([prepended_text, getattr(p_item, 'text', '').rstrip()])
+                                    else:
+                                        prepended_text = ''
                                 else:
-                                    prepended_text = p_item.text.rstrip()
+                                    if getattr(p_item, 'text', ''):
+                                        prepended_text = getattr(p_item, 'text', '').rstrip()
+                                    else:
+                                        prepended_text = ''
                             else:
                                 prepended_text = ''
                     if prepended_text.endswith('.'):
@@ -4832,12 +4838,14 @@ def _lookup_admin_template(policy_name,
             suggested_policies = ''
             adml_to_remove = []
             if len(adml_search_results) > 1:
+                log.debug('multiple ADML entries found matching the policy name %s', policy_name)
                 multiple_adml_entries = True
                 for adml_search_result in adml_search_results:
                     if not getattr(adml_search_result, 'text', '').strip() == policy_name:
                         adml_to_remove.append(adml_search_result)
                     else:
                         if hierarchy:
+                            log.debug('we have hierarchy of %s', hierarchy)
                             display_name_searchval = '$({0}.{1})'.format(
                                     adml_search_result.tag.split('}')[1],
                                     adml_search_result.attrib['id'])
@@ -4847,8 +4855,8 @@ def _lookup_admin_template(policy_name,
                                 display_name_searchval,
                                 policy_class)
                             admx_results = []
-                            admx_search_results = admx_policy_definitions.xpath(policy_search_string, namespaces=adml_search_result.nsmap)
-                            for search_result in admx_search_results:
+                            these_admx_search_results = admx_policy_definitions.xpath(policy_search_string, namespaces=adml_search_result.nsmap)
+                            for search_result in these_admx_search_results:
                                 log.debug('policy_name == %s', policy_name)
                                 this_hierarchy = _build_parent_list(search_result,
                                                                     admx_policy_definitions,
@@ -4856,11 +4864,17 @@ def _lookup_admin_template(policy_name,
                                                                     adml_policy_resources)
                                 this_hierarchy.reverse()
                                 if hierarchy != this_hierarchy:
-                                    adml_to_remove.append(adml_search_result)
+                                    msg = 'hierarchy %s does not match this item\'s hierarchy of %s'
+                                    log.debug(msg, hierarchy, this_hierarchy)
+                                    if len(these_admx_search_results) == 1:
+                                        log.debug('only 1 admx was found and it does not match this adml, it is safe to remove from the list')
+                                        adml_to_remove.append(adml_search_result)
                                 else:
+                                    log.debug('hierarchy %s matches item\'s hierarchy of %s', hierarchy, this_hierarchy)
+                                    log.debug('search_result %s added to results', search_result)
                                     admx_results.append(search_result)
                             if len(admx_results) == 1:
-                                admx_search_results = admx_results
+                                admx_search_results.append(admx_results[0])
             for adml in adml_to_remove:
                 if adml in adml_search_results:
                     adml_search_results.remove(adml)
@@ -4875,12 +4889,15 @@ def _lookup_admin_template(policy_name,
                         adml_search_result.attrib['id'])
                 log.debug('searching for displayName == %s', display_name_searchval)
                 if not admx_search_results:
+                    log.debug('search for an admx entry matching display_name %s and registry_class %s', display_name_searchval, policy_class)
                     admx_search_results = ADMX_DISPLAYNAME_SEARCH_XPATH(
                             admx_policy_definitions,
                             display_name=display_name_searchval,
                             registry_class=policy_class)
                 if admx_search_results:
-                    if len(admx_search_results) == 1 or hierarchy and not multiple_adml_entries:
+                    log.debug('processing admx_search_results of {0}'.format(admx_search_results))
+                    log.debug('multiple_adml_entries is {0}'.format(multiple_adml_entries))
+                    if (len(admx_search_results) == 1 or hierarchy) and not multiple_adml_entries:
                         found = False
                         for search_result in admx_search_results:
                             found = False

From bc94b8e2961526958bee1c0591e6e8566a706170 Mon Sep 17 00:00:00 2001
From: lomeroe <github@proxyman.com>
Date: Tue, 9 Oct 2018 15:28:00 -0500
Subject: [PATCH 17/32] update method for creating size field of **delvals
 items for py3 compatibility

---
 salt/modules/win_lgpo.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/salt/modules/win_lgpo.py b/salt/modules/win_lgpo.py
index f566b9646016..3b1be903129f 100644
--- a/salt/modules/win_lgpo.py
+++ b/salt/modules/win_lgpo.py
@@ -3497,7 +3497,7 @@ def _processValueItem(element, reg_key, reg_valuename, policy, parent_element,
                                          encoded_semicolon,
                                          chr(registry.vtype[this_vtype]).encode('utf-32-le'),
                                          encoded_semicolon,
-                                         chr(len(' {0}'.format(chr(0)).encode('utf-16-le'))).encode('utf-32-le'),
+                                         six.unichr(len(' {0}'.format(chr(0)).encode('utf-16-le'))).encode('utf-32-le'),
                                          encoded_semicolon,
                                          ' '.encode('utf-16-le'),
                                          encoded_null,
@@ -3558,7 +3558,7 @@ def _processValueItem(element, reg_key, reg_valuename, policy, parent_element,
                                             encoded_semicolon,
                                             chr(registry.vtype[this_vtype]).encode('utf-32-le'),
                                             encoded_semicolon,
-                                            chr(len(' {0}'.format(chr(0)))).encode('utf-32-le'),
+                                            six.unichr(len(' {0}'.format(chr(0)).encode('utf-16-le'))).encode('utf-32-le'),
                                             encoded_semicolon,
                                             ' '.encode('utf-16-le'),
                                             encoded_null,

From 271bd70cf1edf8f4173d52160077a27b8a9f812c Mon Sep 17 00:00:00 2001
From: lomeroe <github@proxyman.com>
Date: Wed, 10 Oct 2018 11:46:41 -0500
Subject: [PATCH 18/32] capture and print exception information

in _regexSearchKeyValueCombo use byte ']' when searching for end of
policy configuration in registry.pol file (py3 fix), additionally move 2
characters from ']' location to skip ']' and the '\x00' due to the
utf-16-le encoding to find the end of the element

this was causing _regexSearchKeyValueCombo to return the element without
the ending ']\x00' and if/when the element was replaced two ']\x00'
existed at the end of the element
---
 salt/modules/win_lgpo.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/salt/modules/win_lgpo.py b/salt/modules/win_lgpo.py
index 3b1be903129f..afb79913e487 100644
--- a/salt/modules/win_lgpo.py
+++ b/salt/modules/win_lgpo.py
@@ -2925,8 +2925,8 @@ def _findOptionValueInSeceditFile(option):
                     return True, _line.split('=')[1].strip()
         return True, 'Not Defined'
     # TODO: This needs to be more specific
-    except Exception:
-        log.exception('error occurred while trying to get secedit data')
+    except Exception as e:
+        log.exception('error %s occurred while trying to get secedit data', e)
         return False, None
 
 
@@ -4185,7 +4185,7 @@ def _regexSearchKeyValueCombo(policy_data, policy_regpath, policy_regkey):
                                b'\00;'])
         match = re.search(_thisSearch, policy_data, re.IGNORECASE)
         if match:
-            return policy_data[match.start():(policy_data.index(']', match.end())) + 1]
+            return policy_data[match.start():(policy_data.index(b']', match.end())) + 2]
 
     return None
 
@@ -4689,8 +4689,8 @@ def _writeAdminTemplateRegPolFile(admtemplate_data,
                            policy_data.admx_registry_classes[registry_class]['gpt_extension_location'],
                            policy_data.admx_registry_classes[registry_class]['gpt_extension_guid'])
     # TODO: This needs to be more specific or removed
-    except Exception:
-        log.exception('Unhandled exception %s occurred while attempting to write Adm Template Policy File')
+    except Exception as e:
+        log.exception('Unhandled exception %s occurred while attempting to write Adm Template Policy File', e)
         return False
     return True
 

From c283f50074547d9c0d510cded818d034273d4dac Mon Sep 17 00:00:00 2001
From: lomeroe <github@proxyman.com>
Date: Thu, 11 Oct 2018 13:22:50 -0500
Subject: [PATCH 19/32] Log a warning message instead of an exception when a
 SID cannot be converted to a username (for user rights assignments)

Add a module test file
---
 salt/modules/win_lgpo.py                   |   2 +-
 tests/integration/modules/test_win_lgpo.py | 210 +++++++++++++++++++++
 2 files changed, 211 insertions(+), 1 deletion(-)
 create mode 100644 tests/integration/modules/test_win_lgpo.py

diff --git a/salt/modules/win_lgpo.py b/salt/modules/win_lgpo.py
index afb79913e487..854cbc2a5534 100644
--- a/salt/modules/win_lgpo.py
+++ b/salt/modules/win_lgpo.py
@@ -2605,8 +2605,8 @@ def _sidConversion(cls, val, **kwargs):
                     userSid = '{0}'.format(userSid[0])
             # TODO: This needs to be more specific
             except Exception:
-                log.exception('Handle this explicitly')
                 userSid = win32security.ConvertSidToStringSid(_sid)
+                log.warning('Unable to convert SID "%s" to a friendly name.  The SID will be disaplayed instead of a user/group name.', userSid)
             usernames.append(userSid)
         return usernames
 
diff --git a/tests/integration/modules/test_win_lgpo.py b/tests/integration/modules/test_win_lgpo.py
new file mode 100644
index 000000000000..ef1f1646231e
--- /dev/null
+++ b/tests/integration/modules/test_win_lgpo.py
@@ -0,0 +1,210 @@
+# -*- coding: utf-8 -*-
+
+# Import Python libs
+from __future__ import absolute_import, print_function, unicode_literals
+import os
+import textwrap
+import re
+import io
+
+# Import Salt Testing libs
+from tests.support.case import ModuleCase
+from tests.support.unit import skipIf
+from tests.support.helpers import destructiveTest, generate_random_name
+from tests.support.runtests import RUNTIME_VARS
+
+# Import Salt libs
+import salt.utils.files
+import salt.utils.platform
+
+@skipIf(not salt.utils.platform.is_windows(), 'windows test only')
+class WinLgpoTest(ModuleCase):
+    '''
+    Tests for salt.modules.win_lgpo
+    '''
+
+    def setUp(self):
+        '''
+        setup function
+
+        downloads and extracts the lgpo.exe tool into c:\windows\system32
+        for use in validating the registry.pol files
+        '''
+        ret = self.run_function('archive.unzip',
+                                'https://www.microsoft.com/en-us/download/confirmation.aspx?id=55319&6B49FDFB-8E5B-4B07-BC31-15695C5A2143=1'
+                                dest='c:\\windows\\system32')
+
+    @destructiveTest
+    def test_set_computer_policy(self):
+        '''
+        Test setting/unsetting/changing multiple policies
+        '''
+
+        def _testSeceditPolicy(policy_name,
+                               policy_config,
+                               expected_regex,
+                               cumulative_rights_assignments=True):
+            '''
+            Takes a secedit policy name and config and validates that the expected
+            output is returned from secedit
+
+            policy_name
+                name of the secedit policy to configure
+            policy_config
+                the configuration of the policy
+            expected_regexes
+                the expected regexes to be found in the secedit output file
+            '''
+            ret = self.run_function('lgpo.set_computer_policy',
+                                    policy_name,
+                                    policy_config,
+                                    cumulative_rights_assignments=cumulative_rights_assignments)
+            self.assertTrue(ret)
+            secedit_output_file = os.path.join(RUNTIME_VARS.TMP, generate_random_name('secedit-output-'))
+            secedit_output = self.run_function(
+                    'cmd.run',
+                    'secedit /export /cfg {0}'.format(secedit_output_file))
+            secedit_file_content = None
+            if secedit_output:
+                with io.open(secedit_output_file, encoding='utf-16') as _reader:
+                    _secdata = _reader.read()
+            for expected_regex in expected_regexes:
+                match = re.search(
+                        expected_regex,
+                        lgpo_output,
+                        re.IGNORECASE)
+                assert match not None
+        def _testComputerAdmxPolicy(policy_name,
+                                    policy_config,
+                                    expected_regexes):
+            '''
+            Takes a ADMX policy name and config and validates that the expected
+            output is returned from lgpo looking at the Registry.pol file
+
+            policy_name
+                name of the ADMX policy to configure
+            policy_config
+                the configuration of the policy
+            expected_regexes
+                the expected regexes to be found in the lgpo parse output
+            '''
+            ret = self.run_function('lgpo.set_computer_policy',
+                                    policy_name,
+                                    policy_config)
+            self.assertTrue(ret)
+            lgpo_output = self.run_function(
+                    'cmd.run',
+                    'lgpo.exe /parse /m c:\\Windows\\System32\\GroupPolicy\\Machine\\Registry.pol')
+            for expected_regex in expected_regexes:
+                match = re.search(
+                        expected_regex,
+                        lgpo_output,
+                        re.IGNORECASE)
+                assert match not None
+
+        # configure RA_Unsolicit
+        _testComputerAdmxPolicy('RA_Unsolicit',
+                                {
+                                    'Configure Offer Remote Access': Enabled,
+                                    'Permit remote control of this computer': 'Allow helpers to remotely control the computer',
+                                    'Helpers': ['administrators', 'user1']
+                                },
+                                [
+                                    r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services\\RAUnsolicit[\s]*user1[\s]*SZ:user1[\s]*',
+                                    r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services\\RAUnsolicit[\s]*administrators[\s]*SZ:administrators[\s]*',
+                                    r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fAllowUnsolicited[\s]*DWORD:1',
+                                    r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fAllowUnsolicitedFullControl[\s]*DWORD:1',
+                                ])
+        # Disable RA_Unsolicit
+        _testComputerAdmxPolicy('RA_Unsolicit',
+                                'Disabled',
+                                [
+                                    r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fAllowUnsolicited[\s]*DWORD:0',
+                                    r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fAllowUnsolicitedFullControl[\s]*DELETE',
+                                    r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services\\RAUnsolicit[\s]*\*[\s]*DELETEALLVALUES',
+                                ])
+        # Not Configure RA_Unsolicit
+        _testComputerAdmxPolicy('RA_Unsolicit',
+                                'Not Configured',
+                                [r'; Source file:  c:\\windows\\system32\\grouppolicy\\machine\\registry.pol[\s]*; PARSING COMPLETED.'])
+
+        # Disable Configure NTP Client
+        _testComputerAdmxPolicy('System\Windows Time Service\Time Providers\Configure Windows NTP Client',
+                                'Disabled',
+                                [
+                                    r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\Parameters[\s]*NtpServer[\s]*DELETE',
+                                    r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\Parameters[\s]*Type[\s]*DELETE',
+                                    r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*CrossSiteSyncFlags[\s]*DELETE',
+                                    r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*ResolvePeerBackoffMinutes[\s]*DELETE',
+                                    r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*ResolvePeerBackoffMaxTimes[\s]*DELETE',
+                                    r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*SpecialPollInterval[\s]*DELETE',
+                                    r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*EventLogFlags[\s]*DELETE'
+                                ]
+        # Enable Configure NTP Client
+         _testComputerAdmxPolicy('System\Windows Time Service\Time Providers\Configure Windows NTP Client',
+                                 {
+                                    'NtpServer': 'time.windows.com,0x9',
+                                    'Type': 'NT5DS',
+                                    'CrossSiteSyncFlags': 2,
+                                    'ResolvePeerBackoffMinutes': 15,
+                                    'ResolvePeerBackoffMaxTimes': 7,
+                                    'W32TIME_SpecialPollInterval': 3600,
+                                    'W32TIME_NtpClientEventLogFlags': 0
+                                 },
+                                 [
+                                    r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\Parameters[\s]*NtpServer[\s]*SZ:time.windows.com,0x9',
+                                    r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\Parameters[\s]*Type[\s]*SZ:NT5DS',
+                                    r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*CrossSiteSyncFlags[\s]*DWORD:2',
+                                    r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*ResolvePeerBackoffMinutes[\s]*DWORD:15',
+                                    r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*ResolvePeerBackoffMaxTimes[\s]*DWORD:7',
+                                    r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*SpecialPollInterval[\s]*DWORD:3600',
+                                    r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*EventLogFlags[\s]*DWORD:0',
+                                 ])
+        # set Configure NTP Client to 'Not Configured'
+        _testComputerAdmxPolicy('System\Windows Time Service\Time Providers\Configure Windows NTP Client',
+                                'Not Configured',
+                                [r'; Source file:  c:\\windows\\system32\\grouppolicy\\machine\\registry.pol[\s]*; PARSING COMPLETED.'])
+
+        # disable Configure Automatic Updates
+        _testComputerAdmxPolicy('Windows Components\Windows Update\Configure Automatic Updates',
+                                'Disabled',
+                                [
+                                    r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate\\AU[\s]*NoAutoUpdate[\s]*DWORD:1',
+                                    r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate\\AU[\s]*AUOptions[\s]*DELETE',
+                                    r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate\\AU[\s]*AutomaticMaintenanceEnabled[\s]*DELETE',
+                                    r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate\\AU[\s]*ScheduledInstallDay[\s]*DELETE',
+                                    r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate\\AU[\s]*ScheduledInstallTime[\s]*DELETE',
+                                    r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate\\AU[\s]*AllowMUUpdateService[\s]*DELETE'
+                                ])
+        # set Configure Automatic Updates to 'Not Configured'
+        _testComputerAdmxPolicy('Windows Components\Windows Update\Configure Automatic Updates',
+                                'Not Configured',
+                                [r'; Source file:  c:\\windows\\system32\\grouppolicy\\machine\\registry.pol[\s]*; PARSING COMPLETED.'])
+        
+        # disable PasswordComplexity
+        _testSeceditPolicy('Password must meet complexity requirements',
+                           'Disabled',
+                           [r'^PasswordComplexity = 0'])
+        # enable PasswordComplexity
+        _testSeceditPolicy('PasswordComplexity',
+                           'Disabled',
+                           [r'^PasswordComplexity = 1'])
+        # set Minimum password length
+        _testSeceditPolicy('Minimum password length',
+                           10,
+                           [r'^MinimumPasswordLength = 10'])
+        # set MinimumPasswordLength = 0
+        _testSeceditPolicy('MinimumPasswordLength',
+                           0,
+                           [r'^MinimumPasswordLength = 0'])
+        # set SeNetworkLogonRight to only Administrators
+        _testSeceditPolicy('Access this computer from the network',
+                           ['Administrators'],
+                           [r'^SeNetworkLogonRight = \*S-1-5-32-544'],
+                           cumulative_rights_assignments=False)
+        # set SeNetworkLogonRight back to the default
+        _testSeceditPolicy('SeNetworkLogonRight',
+                           ['Everyone', 'Administrators', 'Users', 'Backup Operators'],
+                           [r'^SeNetworkLogonRight = \*S-1-1-0,\*S-1-5-32-544,\*S-1-5-32-545,\*S-1-5-32-551'])
+    def tearDown(self):
+        pass

From 42840ecd9b601ff3f6900c69bc1d9eb9074b1c9c Mon Sep 17 00:00:00 2001
From: lomeroe <github@proxyman.com>
Date: Thu, 11 Oct 2018 14:58:20 -0500
Subject: [PATCH 20/32] add missing comma in function call

---
 tests/integration/modules/test_win_lgpo.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/integration/modules/test_win_lgpo.py b/tests/integration/modules/test_win_lgpo.py
index ef1f1646231e..0cd8b95e8993 100644
--- a/tests/integration/modules/test_win_lgpo.py
+++ b/tests/integration/modules/test_win_lgpo.py
@@ -31,7 +31,7 @@ def setUp(self):
         for use in validating the registry.pol files
         '''
         ret = self.run_function('archive.unzip',
-                                'https://www.microsoft.com/en-us/download/confirmation.aspx?id=55319&6B49FDFB-8E5B-4B07-BC31-15695C5A2143=1'
+                                'https://www.microsoft.com/en-us/download/confirmation.aspx?id=55319&6B49FDFB-8E5B-4B07-BC31-15695C5A2143=1',
                                 dest='c:\\windows\\system32')
 
     @destructiveTest

From 2a79626538543e7894a42da1d313f2143556f645 Mon Sep 17 00:00:00 2001
From: lomeroe <github@proxyman.com>
Date: Wed, 17 Oct 2018 11:03:01 -0500
Subject: [PATCH 21/32] update test to actually work

update lgpo module to catch another possible case of multiple ADML names
---
 salt/modules/win_lgpo.py                   |   3 +
 tests/integration/modules/test_win_lgpo.py | 511 ++++++++++++++-------
 2 files changed, 350 insertions(+), 164 deletions(-)

diff --git a/salt/modules/win_lgpo.py b/salt/modules/win_lgpo.py
index 854cbc2a5534..7aed73aa55b8 100644
--- a/salt/modules/win_lgpo.py
+++ b/salt/modules/win_lgpo.py
@@ -4856,6 +4856,9 @@ def _lookup_admin_template(policy_name,
                                 policy_class)
                             admx_results = []
                             these_admx_search_results = admx_policy_definitions.xpath(policy_search_string, namespaces=adml_search_result.nsmap)
+                            if not these_admx_search_results:
+                                log.debug('No admx was found for the adml entry %s, it will be removed', display_name_searchval)
+                                adml_to_remove.append(adml_search_result)
                             for search_result in these_admx_search_results:
                                 log.debug('policy_name == %s', policy_name)
                                 this_hierarchy = _build_parent_list(search_result,
diff --git a/tests/integration/modules/test_win_lgpo.py b/tests/integration/modules/test_win_lgpo.py
index 0cd8b95e8993..474144057908 100644
--- a/tests/integration/modules/test_win_lgpo.py
+++ b/tests/integration/modules/test_win_lgpo.py
@@ -6,6 +6,7 @@
 import textwrap
 import re
 import io
+import logging
 
 # Import Salt Testing libs
 from tests.support.case import ModuleCase
@@ -16,6 +17,9 @@
 # Import Salt libs
 import salt.utils.files
 import salt.utils.platform
+import salt.utils.win_reg as reg
+
+log = logging.getLogger(__name__)
 
 @skipIf(not salt.utils.platform.is_windows(), 'windows test only')
 class WinLgpoTest(ModuleCase):
@@ -23,188 +27,367 @@ class WinLgpoTest(ModuleCase):
     Tests for salt.modules.win_lgpo
     '''
 
-    def setUp(self):
+    def _testRegistryPolicy(self,
+                            policy_name,
+                            policy_config,
+                            registry_value_hive,
+                            registry_value_path,
+                            registry_value_vname,
+                            expected_value_data):
+        '''
+        Takes a registry based policy name and config and validates taht the
+        expected registry value exists and has the correct data
+
+        policy_name
+            name of the registry based policy to configure
+        policy_config
+            the configuration of the policy
+        registry_value_hive
+            the registry hive that the policy registry path is in
+        registry_value_path
+            the registry value path that the policy updates
+        registry_value_vname
+            the registry value name
+        expected_value_data
+            the expected data that the value will contain
+        '''
+        ret = self.run_function('lgpo.set_computer_policy',
+                                (policy_name, policy_config))
+        self.assertTrue(ret)
+        val = reg.read_value(
+                registry_value_hive,
+                registry_value_path,
+                registry_value_vname)
+        if val['success']:
+            assert val['vdata'] == expected_value_data, 'The registry value data {0} does not match the expected value {1} for policy {2}'.format(
+                        val['vdata'],
+                        expected_value_data,
+                        policy_name)
+        else:
+            self.assertTrue(False, msg='Failed to obtain the registry data for policy {0}'.format(policy_name))
+
+    def _testSeceditPolicy(self,
+                           policy_name,
+                           policy_config,
+                           expected_regexes,
+                           cumulative_rights_assignments=True):
+        '''
+        Takes a secedit policy name and config and validates that the expected
+        output is returned from secedit
+
+        policy_name
+            name of the secedit policy to configure
+        policy_config
+            the configuration of the policy
+        expected_regexes
+            the expected regexes to be found in the secedit output file
+        '''
+        ret = self.run_function('lgpo.set_computer_policy',
+                                (policy_name, policy_config),
+                                cumulative_rights_assignments=cumulative_rights_assignments)
+        self.assertTrue(ret)
+        secedit_output_file = os.path.join(RUNTIME_VARS.TMP, generate_random_name('secedit-output-'))
+        secedit_output = self.run_function(
+                'cmd.run',
+                (),
+                cmd='secedit /export /cfg {0}'.format(secedit_output_file))
+        secedit_file_content = None
+        if secedit_output:
+            with io.open(secedit_output_file, encoding='utf-16') as _reader:
+                secedit_file_content = _reader.read()
+        for expected_regex in expected_regexes:
+            match = re.search(
+                    expected_regex,
+                    secedit_file_content,
+                    re.IGNORECASE|re.MULTILINE)
+            assert match != None, 'Failed validating policy "{0}" configuration, regex "{1}" not found in secedit output'.format(policy_name, expected_regex)
+
+    def _testComputerAdmxPolicy(self,
+                                policy_name,
+                                policy_config,
+                                expected_regexes):
+        '''
+        Takes a ADMX policy name and config and validates that the expected
+        output is returned from lgpo looking at the Registry.pol file
+
+        policy_name
+            name of the ADMX policy to configure
+        policy_config
+            the configuration of the policy
+        expected_regexes
+            the expected regexes to be found in the lgpo parse output
+        '''
+        ret = self.run_function('lgpo.set_computer_policy',
+                                (policy_name, policy_config))
+        log.debug('lgpo set_computer_policy ret == %s', ret)
+        self.assertTrue(ret)
+        lgpo_output = self.run_function(
+                'cmd.run',
+                (),
+                cmd='lgpo.exe /parse /m c:\\Windows\\System32\\GroupPolicy\\Machine\\Registry.pol')
+        for expected_regex in expected_regexes:
+            match = re.search(
+                    expected_regex,
+                    lgpo_output,
+                    re.IGNORECASE)
+            assert match != None, 'Failed validating policy "{0}" configuration, regex "{1}" not found in lgpo output'.format(policy_name, expected_regex)
+
+    @classmethod
+    def setUpClass(cls):
         '''
         setup function
 
         downloads and extracts the lgpo.exe tool into c:\windows\system32
         for use in validating the registry.pol files
         '''
-        ret = self.run_function('archive.unzip',
-                                'https://www.microsoft.com/en-us/download/confirmation.aspx?id=55319&6B49FDFB-8E5B-4B07-BC31-15695C5A2143=1',
-                                dest='c:\\windows\\system32')
+        if not os.path.exists('c:\\windows\\system32\\lgpo.exe'):
+            log.debug('lgpo.exe does not exist, attempting to download/extract')
+            ret = cls().run_function('state.single',
+                                     ('archive.extracted', 'c:\\windows\\system32'),
+                                     source='https://download.microsoft.com/download/8/5/C/85C25433-A1B0-4FFA-9429-7E023E7DA8D8/LGPO.zip',
+                                     archive_format='zip',
+                                     source_hash='sha256=6ffb6416366652993c992280e29faea3507b5b5aa661c33ba1af31f48acea9c4',
+                                     enforce_toplevel=False)
+            log.debug('ret from archive.unzip == %s', ret)
 
     @destructiveTest
-    def test_set_computer_policy(self):
-        '''
-        Test setting/unsetting/changing multiple policies
-        '''
-
-        def _testSeceditPolicy(policy_name,
-                               policy_config,
-                               expected_regex,
-                               cumulative_rights_assignments=True):
-            '''
-            Takes a secedit policy name and config and validates that the expected
-            output is returned from secedit
-
-            policy_name
-                name of the secedit policy to configure
-            policy_config
-                the configuration of the policy
-            expected_regexes
-                the expected regexes to be found in the secedit output file
-            '''
-            ret = self.run_function('lgpo.set_computer_policy',
-                                    policy_name,
-                                    policy_config,
-                                    cumulative_rights_assignments=cumulative_rights_assignments)
-            self.assertTrue(ret)
-            secedit_output_file = os.path.join(RUNTIME_VARS.TMP, generate_random_name('secedit-output-'))
-            secedit_output = self.run_function(
-                    'cmd.run',
-                    'secedit /export /cfg {0}'.format(secedit_output_file))
-            secedit_file_content = None
-            if secedit_output:
-                with io.open(secedit_output_file, encoding='utf-16') as _reader:
-                    _secdata = _reader.read()
-            for expected_regex in expected_regexes:
-                match = re.search(
-                        expected_regex,
-                        lgpo_output,
-                        re.IGNORECASE)
-                assert match not None
-        def _testComputerAdmxPolicy(policy_name,
-                                    policy_config,
-                                    expected_regexes):
-            '''
-            Takes a ADMX policy name and config and validates that the expected
-            output is returned from lgpo looking at the Registry.pol file
-
-            policy_name
-                name of the ADMX policy to configure
-            policy_config
-                the configuration of the policy
-            expected_regexes
-                the expected regexes to be found in the lgpo parse output
-            '''
-            ret = self.run_function('lgpo.set_computer_policy',
-                                    policy_name,
-                                    policy_config)
-            self.assertTrue(ret)
-            lgpo_output = self.run_function(
-                    'cmd.run',
-                    'lgpo.exe /parse /m c:\\Windows\\System32\\GroupPolicy\\Machine\\Registry.pol')
-            for expected_regex in expected_regexes:
-                match = re.search(
-                        expected_regex,
-                        lgpo_output,
-                        re.IGNORECASE)
-                assert match not None
-
-        # configure RA_Unsolicit
-        _testComputerAdmxPolicy('RA_Unsolicit',
-                                {
-                                    'Configure Offer Remote Access': Enabled,
-                                    'Permit remote control of this computer': 'Allow helpers to remotely control the computer',
-                                    'Helpers': ['administrators', 'user1']
-                                },
-                                [
-                                    r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services\\RAUnsolicit[\s]*user1[\s]*SZ:user1[\s]*',
-                                    r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services\\RAUnsolicit[\s]*administrators[\s]*SZ:administrators[\s]*',
-                                    r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fAllowUnsolicited[\s]*DWORD:1',
-                                    r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fAllowUnsolicitedFullControl[\s]*DWORD:1',
-                                ])
-        # Disable RA_Unsolicit
-        _testComputerAdmxPolicy('RA_Unsolicit',
-                                'Disabled',
-                                [
-                                    r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fAllowUnsolicited[\s]*DWORD:0',
-                                    r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fAllowUnsolicitedFullControl[\s]*DELETE',
-                                    r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services\\RAUnsolicit[\s]*\*[\s]*DELETEALLVALUES',
-                                ])
-        # Not Configure RA_Unsolicit
-        _testComputerAdmxPolicy('RA_Unsolicit',
-                                'Not Configured',
-                                [r'; Source file:  c:\\windows\\system32\\grouppolicy\\machine\\registry.pol[\s]*; PARSING COMPLETED.'])
-
+    def test_set_computer_policy_NTP_Client(self):
+        '''
+        Test setting/unsetting/changing NTP Client policies
+        '''
         # Disable Configure NTP Client
-        _testComputerAdmxPolicy('System\Windows Time Service\Time Providers\Configure Windows NTP Client',
-                                'Disabled',
-                                [
-                                    r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\Parameters[\s]*NtpServer[\s]*DELETE',
-                                    r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\Parameters[\s]*Type[\s]*DELETE',
-                                    r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*CrossSiteSyncFlags[\s]*DELETE',
-                                    r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*ResolvePeerBackoffMinutes[\s]*DELETE',
-                                    r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*ResolvePeerBackoffMaxTimes[\s]*DELETE',
-                                    r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*SpecialPollInterval[\s]*DELETE',
-                                    r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*EventLogFlags[\s]*DELETE'
-                                ]
+        self._testComputerAdmxPolicy('System\Windows Time Service\Time Providers\Configure Windows NTP Client',
+                                     'Disabled',
+                                     [
+                                         r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\Parameters[\s]*NtpServer[\s]*DELETE',
+                                         r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\Parameters[\s]*Type[\s]*DELETE',
+                                         r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*CrossSiteSyncFlags[\s]*DELETE',
+                                         r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*ResolvePeerBackoffMinutes[\s]*DELETE',
+                                         r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*ResolvePeerBackoffMaxTimes[\s]*DELETE',
+                                         r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*SpecialPollInterval[\s]*DELETE',
+                                         r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*EventLogFlags[\s]*DELETE'
+                                     ])
         # Enable Configure NTP Client
-         _testComputerAdmxPolicy('System\Windows Time Service\Time Providers\Configure Windows NTP Client',
-                                 {
-                                    'NtpServer': 'time.windows.com,0x9',
-                                    'Type': 'NT5DS',
-                                    'CrossSiteSyncFlags': 2,
-                                    'ResolvePeerBackoffMinutes': 15,
-                                    'ResolvePeerBackoffMaxTimes': 7,
-                                    'W32TIME_SpecialPollInterval': 3600,
-                                    'W32TIME_NtpClientEventLogFlags': 0
-                                 },
-                                 [
-                                    r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\Parameters[\s]*NtpServer[\s]*SZ:time.windows.com,0x9',
-                                    r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\Parameters[\s]*Type[\s]*SZ:NT5DS',
-                                    r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*CrossSiteSyncFlags[\s]*DWORD:2',
-                                    r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*ResolvePeerBackoffMinutes[\s]*DWORD:15',
-                                    r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*ResolvePeerBackoffMaxTimes[\s]*DWORD:7',
-                                    r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*SpecialPollInterval[\s]*DWORD:3600',
-                                    r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*EventLogFlags[\s]*DWORD:0',
-                                 ])
+        self._testComputerAdmxPolicy('System\Windows Time Service\Time Providers\Configure Windows NTP Client',
+                                     {
+                                         'NtpServer': 'time.windows.com,0x9',
+                                         'Type': 'NT5DS',
+                                         'CrossSiteSyncFlags': 2,
+                                         'ResolvePeerBackoffMinutes': 15,
+                                         'ResolvePeerBackoffMaxTimes': 7,
+                                         'W32TIME_SpecialPollInterval': 3600,
+                                         'W32TIME_NtpClientEventLogFlags': 0
+                                     },
+                                     [
+                                         r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\Parameters[\s]*NtpServer[\s]*SZ:time.windows.com,0x9',
+                                         r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\Parameters[\s]*Type[\s]*SZ:NT5DS',
+                                         r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*CrossSiteSyncFlags[\s]*DWORD:2',
+                                         r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*ResolvePeerBackoffMinutes[\s]*DWORD:15',
+                                         r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*ResolvePeerBackoffMaxTimes[\s]*DWORD:7',
+                                         r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*SpecialPollInterval[\s]*DWORD:3600',
+                                         r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*EventLogFlags[\s]*DWORD:0',
+                                     ])
         # set Configure NTP Client to 'Not Configured'
-        _testComputerAdmxPolicy('System\Windows Time Service\Time Providers\Configure Windows NTP Client',
-                                'Not Configured',
-                                [r'; Source file:  c:\\windows\\system32\\grouppolicy\\machine\\registry.pol[\s]*; PARSING COMPLETED.'])
+        self._testComputerAdmxPolicy('System\Windows Time Service\Time Providers\Configure Windows NTP Client',
+                                     'Not Configured',
+                                     [r'; Source file:  c:\\windows\\system32\\grouppolicy\\machine\\registry.pol[\s]*; PARSING COMPLETED.'])
 
+    @destructiveTest
+    def test_set_computer_policy_RA_Unsolicit(self):
+        '''
+        Test setting/unsetting/changing RA_Unsolicit policy
+        '''
+
+        # Disable RA_Unsolicit
+        log.debug('Attempting to disable RA_Unsolicit')
+        self._testComputerAdmxPolicy('RA_Unsolicit',
+                                     'Disabled',
+                                     [
+                                         r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fAllowUnsolicited[\s]*DWORD:0',
+                                         r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fAllowUnsolicitedFullControl[\s]*DELETE',
+                                         r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services\\RAUnsolicit[\s]*\*[\s]*DELETEALLVALUES',
+                                     ])
+        # configure RA_Unsolicit
+        log.debug('Attempting to configure RA_Unsolicit')
+        self._testComputerAdmxPolicy('RA_Unsolicit',
+                                     {
+                                         'Configure Offer Remote Access': 'Enabled',
+                                         'Permit remote control of this computer': 'Allow helpers to remotely control the computer',
+                                         'Helpers': ['administrators', 'user1']
+                                     },
+                                     [
+                                         r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services\\RAUnsolicit[\s]*user1[\s]*SZ:user1[\s]*',
+                                         r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services\\RAUnsolicit[\s]*administrators[\s]*SZ:administrators[\s]*',
+                                         r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fAllowUnsolicited[\s]*DWORD:1',
+                                         r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fAllowUnsolicitedFullControl[\s]*DWORD:1',
+                                     ])
+        # Not Configure RA_Unsolicit
+        log.debug('Attempting to set RA_Unsolicit to Not Configured')
+        self._testComputerAdmxPolicy('RA_Unsolicit',
+                                     'Not Configured',
+                                     [r'; Source file:  c:\\windows\\system32\\grouppolicy\\machine\\registry.pol[\s]*; PARSING COMPLETED.'])
+
+    @destructiveTest
+    def test_set_computer_policy_WindowsUpdate(self):
+        '''
+        Test setting/unsetting/changing WindowsUpdate policy
+        '''
         # disable Configure Automatic Updates
-        _testComputerAdmxPolicy('Windows Components\Windows Update\Configure Automatic Updates',
-                                'Disabled',
-                                [
-                                    r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate\\AU[\s]*NoAutoUpdate[\s]*DWORD:1',
-                                    r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate\\AU[\s]*AUOptions[\s]*DELETE',
-                                    r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate\\AU[\s]*AutomaticMaintenanceEnabled[\s]*DELETE',
-                                    r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate\\AU[\s]*ScheduledInstallDay[\s]*DELETE',
-                                    r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate\\AU[\s]*ScheduledInstallTime[\s]*DELETE',
-                                    r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate\\AU[\s]*AllowMUUpdateService[\s]*DELETE'
-                                ])
+        self._testComputerAdmxPolicy('Windows Components\Windows Update\Configure Automatic Updates',
+                                     'Disabled',
+                                     [
+                                         r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate\\AU[\s]*NoAutoUpdate[\s]*DWORD:1',
+                                         r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate\\AU[\s]*AUOptions[\s]*DELETE',
+                                         r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate\\AU[\s]*AutomaticMaintenanceEnabled[\s]*DELETE',
+                                         r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate\\AU[\s]*ScheduledInstallDay[\s]*DELETE',
+                                         r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate\\AU[\s]*ScheduledInstallTime[\s]*DELETE',
+                                         r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate\\AU[\s]*AllowMUUpdateService[\s]*DELETE'
+                                 ])
         # set Configure Automatic Updates to 'Not Configured'
-        _testComputerAdmxPolicy('Windows Components\Windows Update\Configure Automatic Updates',
-                                'Not Configured',
-                                [r'; Source file:  c:\\windows\\system32\\grouppolicy\\machine\\registry.pol[\s]*; PARSING COMPLETED.'])
-        
+        self._testComputerAdmxPolicy('Windows Components\Windows Update\Configure Automatic Updates',
+                                     'Not Configured',
+                                     [r'; Source file:  c:\\windows\\system32\\grouppolicy\\machine\\registry.pol[\s]*; PARSING COMPLETED.'])
+
+    @destructiveTest
+    def test_set_computer_policy_ClipboardRedirection(self):
+        '''
+        Test setting/unsetting/changing ClipboardRedirection policy
+        '''
+        # Enable/Disable/Not Configured "Do not allow Clipboard redirection"
+        self._testComputerAdmxPolicy('Windows Components\Remote Desktop Services\Remote Desktop Session Host\Device and Resource Redirection\Do not allow Clipboard redirection',
+                                     'Enabled',
+                                     [r'Computer[\s]*Software\\Policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fDisableClip[\s]*DWORD:1'])
+        self._testComputerAdmxPolicy('Windows Components\Remote Desktop Services\Remote Desktop Session Host\Device and Resource Redirection\Do not allow Clipboard redirection',
+                                     'Disabled',
+                                     [r'Computer[\s]*Software\\Policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fDisableClip[\s]*DWORD:0'])
+        self._testComputerAdmxPolicy('Windows Components\Remote Desktop Services\Remote Desktop Session Host\Device and Resource Redirection\Do not allow Clipboard redirection',
+                                     'Not Configured',
+                                     [r'; Source file:  c:\\windows\\system32\\grouppolicy\\machine\\registry.pol[\s]*; PARSING COMPLETED.'])
+
+    @destructiveTest
+    def test_set_computer_policy_PasswordComplexity(self):
+        '''
+        Test setting/unsetting/changing PasswordComplexity
+        '''
         # disable PasswordComplexity
-        _testSeceditPolicy('Password must meet complexity requirements',
-                           'Disabled',
-                           [r'^PasswordComplexity = 0'])
+        self._testSeceditPolicy('Password must meet complexity requirements',
+                                'Disabled',
+                                [r'^PasswordComplexity = 0'])
         # enable PasswordComplexity
-        _testSeceditPolicy('PasswordComplexity',
-                           'Disabled',
-                           [r'^PasswordComplexity = 1'])
+        self._testSeceditPolicy('PasswordComplexity',
+                                'Enabled',
+                                [r'^PasswordComplexity = 1'])
+
+    @destructiveTest
+    def test_set_computer_policy_PasswordLen(self):
+        '''
+        Test setting/unsetting/changing PasswordLength
+        '''
         # set Minimum password length
-        _testSeceditPolicy('Minimum password length',
-                           10,
-                           [r'^MinimumPasswordLength = 10'])
+        self._testSeceditPolicy('Minimum password length',
+                                10,
+                                [r'^MinimumPasswordLength = 10'])
         # set MinimumPasswordLength = 0
-        _testSeceditPolicy('MinimumPasswordLength',
-                           0,
-                           [r'^MinimumPasswordLength = 0'])
+        self._testSeceditPolicy('MinPasswordLen',
+                                0,
+                                [r'^MinimumPasswordLength = 0'])
+
+    @destructiveTest
+    def test_set_computer_policy_SeNetworkLogonRight(self):
+        '''
+        Test setting/unsetting/changing PasswordLength
+        '''
         # set SeNetworkLogonRight to only Administrators
-        _testSeceditPolicy('Access this computer from the network',
-                           ['Administrators'],
-                           [r'^SeNetworkLogonRight = \*S-1-5-32-544'],
-                           cumulative_rights_assignments=False)
+        self._testSeceditPolicy('Access this computer from the network',
+                                ['Administrators'],
+                                [r'^SeNetworkLogonRight = \*S-1-5-32-544'],
+                                cumulative_rights_assignments=False)
         # set SeNetworkLogonRight back to the default
-        _testSeceditPolicy('SeNetworkLogonRight',
-                           ['Everyone', 'Administrators', 'Users', 'Backup Operators'],
-                           [r'^SeNetworkLogonRight = \*S-1-1-0,\*S-1-5-32-544,\*S-1-5-32-545,\*S-1-5-32-551'])
+        self._testSeceditPolicy('SeNetworkLogonRight',
+                                ['Everyone', 'Administrators', 'Users', 'Backup Operators'],
+                                [r'^SeNetworkLogonRight = \*S-1-1-0,\*S-1-5-32-544,\*S-1-5-32-545,\*S-1-5-32-551'])
+
+    @destructiveTest
+    def test_set_computer_policy_multipleAdmxPolicies(self):
+        '''
+        Tests setting several ADMX policies in succession and validating the configuration w/lgop
+        '''
+        # set one policy
+        self._testComputerAdmxPolicy('Windows Components\Remote Desktop Services\Remote Desktop Session Host\Device and Resource Redirection\Do not allow Clipboard redirection',
+                                     'Disabled',
+                                     [r'Computer[\s]*Software\\Policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fDisableClip[\s]*DWORD:0'])
+
+        # set another policy and make sure both this policy and the previous are okay
+        self._testComputerAdmxPolicy('RA_Unsolicit',
+                                     {
+                                         'Configure Offer Remote Access': 'Enabled',
+                                         'Permit remote control of this computer': 'Allow helpers to remotely control the computer',
+                                         'Helpers': ['administrators', 'user1']
+                                     },
+                                     [
+                                         r'Computer[\s]*Software\\Policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fDisableClip[\s]*DWORD:0',
+                                         r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services\\RAUnsolicit[\s]*user1[\s]*SZ:user1[\s]*',
+                                         r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services\\RAUnsolicit[\s]*administrators[\s]*SZ:administrators[\s]*',
+                                         r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fAllowUnsolicited[\s]*DWORD:1',
+                                         r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fAllowUnsolicitedFullControl[\s]*DWORD:1',
+                                     ])
+        # Configure Automatic Updates and validate everything is still okay
+        self._testComputerAdmxPolicy('Windows Components\Windows Update\Configure Automatic Updates',
+                                     'Disabled',
+                                     [
+                                         r'Computer[\s]*Software\\Policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fDisableClip[\s]*DWORD:0',
+                                         r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services\\RAUnsolicit[\s]*user1[\s]*SZ:user1[\s]*',
+                                         r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services\\RAUnsolicit[\s]*administrators[\s]*SZ:administrators[\s]*',
+                                         r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fAllowUnsolicited[\s]*DWORD:1',
+                                         r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fAllowUnsolicitedFullControl[\s]*DWORD:1',
+                                         r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate\\AU[\s]*NoAutoUpdate[\s]*DWORD:1',
+                                         r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate\\AU[\s]*AUOptions[\s]*DELETE',
+                                         r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate\\AU[\s]*AutomaticMaintenanceEnabled[\s]*DELETE',
+                                         r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate\\AU[\s]*ScheduledInstallDay[\s]*DELETE',
+                                         r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate\\AU[\s]*ScheduledInstallTime[\s]*DELETE',
+                                         r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate\\AU[\s]*AllowMUUpdateService[\s]*DELETE'
+                                     ])
+
+    @destructiveTest
+    def test_set_computer_policy_DisableDomainCreds(self):
+        '''
+        Tests Enable/Disable of DisableDomainCreds policy
+        '''
+        self._testRegistryPolicy('DisableDomainCreds',
+                                 'Enabled',
+                                 'HKEY_LOCAL_MACHINE',
+                                 'SYSTEM\\CurrentControlSet\\Control\\Lsa',
+                                 'DisableDomainCreds',
+                                 1)
+        self._testRegistryPolicy(
+                'Network access: Do not allow storage of passwords and credentials for network authentication',
+                'Disabled',
+                'HKEY_LOCAL_MACHINE',
+                'SYSTEM\\CurrentControlSet\\Control\\Lsa',
+                'DisableDomainCreds',
+                0)
+
+    @destructiveTest
+    def test_set_computer_policy_ForceGuest(self):
+        '''
+        Tests changing ForceGuest policy
+        '''
+        self._testRegistryPolicy('ForceGuest',
+                                 'Guest only - local users authenticate as Guest',
+                                 'HKEY_LOCAL_MACHINE',
+                                 'SYSTEM\\CurrentControlSet\\Control\\Lsa',
+                                 'ForceGuest',
+                                 1)
+        self._testRegistryPolicy(
+                'Network access: Sharing and security model for local accounts',
+                'Classic - local users authenticate as themselves',
+                'HKEY_LOCAL_MACHINE',
+                'SYSTEM\\CurrentControlSet\\Control\\Lsa',
+                'ForceGuest',
+                0)
+
     def tearDown(self):
-        pass
+        ret = self.run_function('state.single',
+                                ('file.absent', 'c:\\windows\\system32\\grouppolicy\\machine\\registry.pol'))
+        ret = self.run_function('state.single',
+                                ('file.absent', 'c:\\windows\\system32\\grouppolicy\\user\\registry.pol'))

From 94b33350a86a7a6df2349ad8a64d455b16cc580a Mon Sep 17 00:00:00 2001
From: lomeroe <github@proxyman.com>
Date: Wed, 17 Oct 2018 16:19:20 -0500
Subject: [PATCH 22/32] lint fixes in test

---
 tests/integration/modules/test_win_lgpo.py | 35 +++++++++++-----------
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/tests/integration/modules/test_win_lgpo.py b/tests/integration/modules/test_win_lgpo.py
index 474144057908..ceba3b44b1f0 100644
--- a/tests/integration/modules/test_win_lgpo.py
+++ b/tests/integration/modules/test_win_lgpo.py
@@ -3,7 +3,6 @@
 # Import Python libs
 from __future__ import absolute_import, print_function, unicode_literals
 import os
-import textwrap
 import re
 import io
 import logging
@@ -21,6 +20,7 @@
 
 log = logging.getLogger(__name__)
 
+
 @skipIf(not salt.utils.platform.is_windows(), 'windows test only')
 class WinLgpoTest(ModuleCase):
     '''
@@ -58,13 +58,12 @@ def _testRegistryPolicy(self,
                 registry_value_hive,
                 registry_value_path,
                 registry_value_vname)
+        self.assertTrue(val['success'], msg='Failed to obtain the registry data for policy {0}'.format(policy_name))
         if val['success']:
-            assert val['vdata'] == expected_value_data, 'The registry value data {0} does not match the expected value {1} for policy {2}'.format(
+            self.assertEqual(val['vdata'], expected_value_data, 'The registry value data {0} does not match the expected value {1} for policy {2}'.format(
                         val['vdata'],
                         expected_value_data,
-                        policy_name)
-        else:
-            self.assertTrue(False, msg='Failed to obtain the registry data for policy {0}'.format(policy_name))
+                        policy_name))
 
     def _testSeceditPolicy(self,
                            policy_name,
@@ -99,8 +98,8 @@ def _testSeceditPolicy(self,
             match = re.search(
                     expected_regex,
                     secedit_file_content,
-                    re.IGNORECASE|re.MULTILINE)
-            assert match != None, 'Failed validating policy "{0}" configuration, regex "{1}" not found in secedit output'.format(policy_name, expected_regex)
+                    re.IGNORECASE | re.MULTILINE)
+            self.assertIsNotNone(match, 'Failed validating policy "{0}" configuration, regex "{1}" not found in secedit output'.format(policy_name, expected_regex))
 
     def _testComputerAdmxPolicy(self,
                                 policy_name,
@@ -130,7 +129,7 @@ def _testComputerAdmxPolicy(self,
                     expected_regex,
                     lgpo_output,
                     re.IGNORECASE)
-            assert match != None, 'Failed validating policy "{0}" configuration, regex "{1}" not found in lgpo output'.format(policy_name, expected_regex)
+            self.assertIsNotNone(match, 'Failed validating policy "{0}" configuration, regex "{1}" not found in lgpo output'.format(policy_name, expected_regex))
 
     @classmethod
     def setUpClass(cls):
@@ -140,7 +139,7 @@ def setUpClass(cls):
         downloads and extracts the lgpo.exe tool into c:\windows\system32
         for use in validating the registry.pol files
         '''
-        if not os.path.exists('c:\\windows\\system32\\lgpo.exe'):
+        if not os.path.exists(r'c:\windows\system32\lgpo.exe'):
             log.debug('lgpo.exe does not exist, attempting to download/extract')
             ret = cls().run_function('state.single',
                                      ('archive.extracted', 'c:\\windows\\system32'),
@@ -156,7 +155,7 @@ def test_set_computer_policy_NTP_Client(self):
         Test setting/unsetting/changing NTP Client policies
         '''
         # Disable Configure NTP Client
-        self._testComputerAdmxPolicy('System\Windows Time Service\Time Providers\Configure Windows NTP Client',
+        self._testComputerAdmxPolicy(r'System\Windows Time Service\Time Providers\Configure Windows NTP Client',
                                      'Disabled',
                                      [
                                          r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\Parameters[\s]*NtpServer[\s]*DELETE',
@@ -168,7 +167,7 @@ def test_set_computer_policy_NTP_Client(self):
                                          r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*EventLogFlags[\s]*DELETE'
                                      ])
         # Enable Configure NTP Client
-        self._testComputerAdmxPolicy('System\Windows Time Service\Time Providers\Configure Windows NTP Client',
+        self._testComputerAdmxPolicy(r'System\Windows Time Service\Time Providers\Configure Windows NTP Client',
                                      {
                                          'NtpServer': 'time.windows.com,0x9',
                                          'Type': 'NT5DS',
@@ -188,7 +187,7 @@ def test_set_computer_policy_NTP_Client(self):
                                          r'Computer[\s]*Software\\Policies\\Microsoft\\W32time\\TimeProviders\\NtpClient[\s]*EventLogFlags[\s]*DWORD:0',
                                      ])
         # set Configure NTP Client to 'Not Configured'
-        self._testComputerAdmxPolicy('System\Windows Time Service\Time Providers\Configure Windows NTP Client',
+        self._testComputerAdmxPolicy(r'System\Windows Time Service\Time Providers\Configure Windows NTP Client',
                                      'Not Configured',
                                      [r'; Source file:  c:\\windows\\system32\\grouppolicy\\machine\\registry.pol[\s]*; PARSING COMPLETED.'])
 
@@ -233,7 +232,7 @@ def test_set_computer_policy_WindowsUpdate(self):
         Test setting/unsetting/changing WindowsUpdate policy
         '''
         # disable Configure Automatic Updates
-        self._testComputerAdmxPolicy('Windows Components\Windows Update\Configure Automatic Updates',
+        self._testComputerAdmxPolicy(r'Windows Components\Windows Update\Configure Automatic Updates',
                                      'Disabled',
                                      [
                                          r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate\\AU[\s]*NoAutoUpdate[\s]*DWORD:1',
@@ -254,13 +253,13 @@ def test_set_computer_policy_ClipboardRedirection(self):
         Test setting/unsetting/changing ClipboardRedirection policy
         '''
         # Enable/Disable/Not Configured "Do not allow Clipboard redirection"
-        self._testComputerAdmxPolicy('Windows Components\Remote Desktop Services\Remote Desktop Session Host\Device and Resource Redirection\Do not allow Clipboard redirection',
+        self._testComputerAdmxPolicy(r'Windows Components\Remote Desktop Services\Remote Desktop Session Host\Device and Resource Redirection\Do not allow Clipboard redirection',
                                      'Enabled',
                                      [r'Computer[\s]*Software\\Policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fDisableClip[\s]*DWORD:1'])
-        self._testComputerAdmxPolicy('Windows Components\Remote Desktop Services\Remote Desktop Session Host\Device and Resource Redirection\Do not allow Clipboard redirection',
+        self._testComputerAdmxPolicy(r'Windows Components\Remote Desktop Services\Remote Desktop Session Host\Device and Resource Redirection\Do not allow Clipboard redirection',
                                      'Disabled',
                                      [r'Computer[\s]*Software\\Policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fDisableClip[\s]*DWORD:0'])
-        self._testComputerAdmxPolicy('Windows Components\Remote Desktop Services\Remote Desktop Session Host\Device and Resource Redirection\Do not allow Clipboard redirection',
+        self._testComputerAdmxPolicy(r'Windows Components\Remote Desktop Services\Remote Desktop Session Host\Device and Resource Redirection\Do not allow Clipboard redirection',
                                      'Not Configured',
                                      [r'; Source file:  c:\\windows\\system32\\grouppolicy\\machine\\registry.pol[\s]*; PARSING COMPLETED.'])
 
@@ -313,7 +312,7 @@ def test_set_computer_policy_multipleAdmxPolicies(self):
         Tests setting several ADMX policies in succession and validating the configuration w/lgop
         '''
         # set one policy
-        self._testComputerAdmxPolicy('Windows Components\Remote Desktop Services\Remote Desktop Session Host\Device and Resource Redirection\Do not allow Clipboard redirection',
+        self._testComputerAdmxPolicy(r'Windows Components\Remote Desktop Services\Remote Desktop Session Host\Device and Resource Redirection\Do not allow Clipboard redirection',
                                      'Disabled',
                                      [r'Computer[\s]*Software\\Policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fDisableClip[\s]*DWORD:0'])
 
@@ -332,7 +331,7 @@ def test_set_computer_policy_multipleAdmxPolicies(self):
                                          r'Computer[\s]*Software\\policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fAllowUnsolicitedFullControl[\s]*DWORD:1',
                                      ])
         # Configure Automatic Updates and validate everything is still okay
-        self._testComputerAdmxPolicy('Windows Components\Windows Update\Configure Automatic Updates',
+        self._testComputerAdmxPolicy(r'Windows Components\Windows Update\Configure Automatic Updates',
                                      'Disabled',
                                      [
                                          r'Computer[\s]*Software\\Policies\\Microsoft\\Windows NT\\Terminal Services[\s]*fDisableClip[\s]*DWORD:0',

From 05f29511a4f05f0fdd5935766d514b9428c9dde6 Mon Sep 17 00:00:00 2001
From: lomeroe <github@proxyman.com>
Date: Wed, 17 Oct 2018 16:28:04 -0500
Subject: [PATCH 23/32] more lint fixes

---
 tests/integration/modules/test_win_lgpo.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/integration/modules/test_win_lgpo.py b/tests/integration/modules/test_win_lgpo.py
index ceba3b44b1f0..83a01f1a1f4a 100644
--- a/tests/integration/modules/test_win_lgpo.py
+++ b/tests/integration/modules/test_win_lgpo.py
@@ -142,7 +142,7 @@ def setUpClass(cls):
         if not os.path.exists(r'c:\windows\system32\lgpo.exe'):
             log.debug('lgpo.exe does not exist, attempting to download/extract')
             ret = cls().run_function('state.single',
-                                     ('archive.extracted', 'c:\\windows\\system32'),
+                                     ('archive.extracted', r'c:\windows\system32'),
                                      source='https://download.microsoft.com/download/8/5/C/85C25433-A1B0-4FFA-9429-7E023E7DA8D8/LGPO.zip',
                                      archive_format='zip',
                                      source_hash='sha256=6ffb6416366652993c992280e29faea3507b5b5aa661c33ba1af31f48acea9c4',
@@ -243,7 +243,7 @@ def test_set_computer_policy_WindowsUpdate(self):
                                          r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate\\AU[\s]*AllowMUUpdateService[\s]*DELETE'
                                  ])
         # set Configure Automatic Updates to 'Not Configured'
-        self._testComputerAdmxPolicy('Windows Components\Windows Update\Configure Automatic Updates',
+        self._testComputerAdmxPolicy(r'Windows Components\Windows Update\Configure Automatic Updates',
                                      'Not Configured',
                                      [r'; Source file:  c:\\windows\\system32\\grouppolicy\\machine\\registry.pol[\s]*; PARSING COMPLETED.'])
 

From 02181d385abe6e52cf64072980bb1fcd0a4b9bd9 Mon Sep 17 00:00:00 2001
From: lomeroe <github@proxyman.com>
Date: Thu, 18 Oct 2018 16:13:41 -0500
Subject: [PATCH 24/32] add fix/test for #50079

ADML display names that have a single match in each Computer/User policy
would not work with the "short" display name, requiring either the
"long" name or the explicit policy name
---
 salt/modules/win_lgpo.py                   | 15 +++-
 tests/integration/modules/test_win_lgpo.py | 90 +++++++++++++++++++---
 2 files changed, 90 insertions(+), 15 deletions(-)

diff --git a/salt/modules/win_lgpo.py b/salt/modules/win_lgpo.py
index 7aed73aa55b8..140e3194e23b 100644
--- a/salt/modules/win_lgpo.py
+++ b/salt/modules/win_lgpo.py
@@ -4878,6 +4878,18 @@ def _lookup_admin_template(policy_name,
                                     admx_results.append(search_result)
                             if len(admx_results) == 1:
                                 admx_search_results.append(admx_results[0])
+                        else:
+                            # verify the ADMX correlated to this ADML is in the same class
+                            # that we are looking for
+                            display_name_searchval = '$({0}.{1})'.format(
+                                    adml_search_result.tag.split('}')[1],
+                                    adml_search_result.attrib['id'])
+                            these_admx_search_results = ADMX_DISPLAYNAME_SEARCH_XPATH(
+                                    admx_policy_definitions,
+                                    display_name=display_name_searchval,
+                                    registry_class=policy_class)
+                            if not these_admx_search_results:
+                                adml_to_remove.append(adml_search_result)
             for adml in adml_to_remove:
                 if adml in adml_search_results:
                     adml_search_results.remove(adml)
@@ -4956,9 +4968,6 @@ def _lookup_admin_template(policy_name,
                                                                '\\'.join(this_parent_list)])
                             else:
                                 suggested_policies = '\\'.join(this_parent_list)
-                else:
-                    msg = 'Unable to find a policy with the name "{0}".'.format(policy_name)
-                    return (False, None, [], msg)
             if suggested_policies:
                 msg = ('ADML policy name "{0}" is used as the display name'
                        ' for multiple policies.'
diff --git a/tests/integration/modules/test_win_lgpo.py b/tests/integration/modules/test_win_lgpo.py
index 83a01f1a1f4a..7a52185d3d6b 100644
--- a/tests/integration/modules/test_win_lgpo.py
+++ b/tests/integration/modules/test_win_lgpo.py
@@ -26,6 +26,7 @@ class WinLgpoTest(ModuleCase):
     '''
     Tests for salt.modules.win_lgpo
     '''
+    osrelease = None
 
     def _testRegistryPolicy(self,
                             policy_name,
@@ -104,7 +105,8 @@ def _testSeceditPolicy(self,
     def _testComputerAdmxPolicy(self,
                                 policy_name,
                                 policy_config,
-                                expected_regexes):
+                                expected_regexes,
+                                assert_true=True):
         '''
         Takes a ADMX policy name and config and validates that the expected
         output is returned from lgpo looking at the Registry.pol file
@@ -115,30 +117,51 @@ def _testComputerAdmxPolicy(self,
             the configuration of the policy
         expected_regexes
             the expected regexes to be found in the lgpo parse output
+        assert_true
+            set to false if expecting the module run to fail
         '''
         ret = self.run_function('lgpo.set_computer_policy',
                                 (policy_name, policy_config))
         log.debug('lgpo set_computer_policy ret == %s', ret)
-        self.assertTrue(ret)
-        lgpo_output = self.run_function(
-                'cmd.run',
-                (),
-                cmd='lgpo.exe /parse /m c:\\Windows\\System32\\GroupPolicy\\Machine\\Registry.pol')
-        for expected_regex in expected_regexes:
-            match = re.search(
-                    expected_regex,
+        if assert_true:
+            self.assertTrue(ret)
+            lgpo_output = self.run_function(
+                    'cmd.run',
+                    (),
+                    cmd='lgpo.exe /parse /m c:\\Windows\\System32\\GroupPolicy\\Machine\\Registry.pol')
+            # validate that the lgpo output doesn't say the format is invalid
+            self.assertIsNone(
+                re.search(
+                    r'Invalid file format\.',
                     lgpo_output,
-                    re.IGNORECASE)
-            self.assertIsNotNone(match, 'Failed validating policy "{0}" configuration, regex "{1}" not found in lgpo output'.format(policy_name, expected_regex))
+                    re.IGNORECASE), 'Failed validating Registry.pol file format')
+            # validate that the regexes we expect are in the output
+            for expected_regex in expected_regexes:
+                match = re.search(
+                        expected_regex,
+                        lgpo_output,
+                        re.IGNORECASE)
+                self.assertIsNotNone(match, 'Failed validating policy "{0}" configuration, regex "{1}" not found in lgpo output'.format(policy_name, expected_regex))
+        else:
+            # expecting it to fail
+            self.assertNotEqual(ret, True)
 
     @classmethod
     def setUpClass(cls):
         '''
-        setup function
+        class setup function, only runs once
 
         downloads and extracts the lgpo.exe tool into c:\windows\system32
         for use in validating the registry.pol files
+
+        gets osrelease grain for tests that are only applicable to certain
+        windows versions
         '''
+        osrelease_grains = cls().run_function('grains.item', ['osrelease'])
+        if 'osrelease' in osrelease_grains:
+            cls.osrelease = osrelease_grains['osrelease']
+        else:
+            log.debug('Unable to get osrelease grain')
         if not os.path.exists(r'c:\windows\system32\lgpo.exe'):
             log.debug('lgpo.exe does not exist, attempting to download/extract')
             ret = cls().run_function('state.single',
@@ -385,7 +408,50 @@ def test_set_computer_policy_ForceGuest(self):
                 'ForceGuest',
                 0)
 
+    @destructiveTest
+    def test_set_computer_policy_DisableUXWUAccess(self):
+        '''
+        Tests changing DisableUXWUAccess
+        #50079 shows using the 'Remove access to use all Windows Update features' failed
+        Policy only exists on 2016
+        '''
+        valid_osreleases = ['2016Server']
+        if self.osrelease not in valid_osreleases:
+            self.skipTest('DisableUXWUAccess policy is only applicable if the osrelease grain is {0}'.format(' or '.join(valid_osreleases)))
+        else:
+            self._testComputerAdmxPolicy(r'DisableUXWUAccess',
+                                         'Enabled',
+                                         [r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate[\s]*SetDisableUXWUAccess[\s]*DWORD:1'])
+            self._testComputerAdmxPolicy(r'Remove access to use all Windows Update features',
+                                         'Disabled',
+                                         [r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate[\s]*SetDisableUXWUAccess[\s]*DWORD:0'])
+            self._testComputerAdmxPolicy(r'Windows Components\Windows Update\Remove access to use all Windows Update features',
+                                         'Not Configured',
+                                         [r'; Source file:  c:\\windows\\system32\\grouppolicy\\machine\\registry.pol[\s]*; PARSING COMPLETED.'])
+
+    @destructiveTest
+    def test_set_computer_policy_Access_data_sources_across_domains(self):
+        '''
+        Tests that a policy that has multiple names
+        '''
+        self._testComputerAdmxPolicy(r'Access data sources across domains',
+                                     'Enabled',
+                                     [],
+                                     assert_true=False)
+        self._testComputerAdmxPolicy(r'Windows Components\Internet Explorer\Internet Control Panel\Security Page\Internet Zone\Access data sources across domains',
+                                     {'Access data sources across domains': 'Prompt'},
+                                     [r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\CurrentVersion\\Internet Settings\\Zones\\3[\s]*1406[\s]*DWORD:1'])
+        self._testComputerAdmxPolicy(r'Windows Components\Internet Explorer\Internet Control Panel\Security Page\Internet Zone\Access data sources across domains',
+                                     {'Access data sources across domains': 'Enable'},
+                                     [r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\CurrentVersion\\Internet Settings\\Zones\\3[\s]*1406[\s]*DWORD:0'])
+        self._testComputerAdmxPolicy(r'Windows Components\Internet Explorer\Internet Control Panel\Security Page\Internet Zone\Access data sources across domains',
+                                     'Disabled',
+                                     [r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\CurrentVersion\\Internet Settings\\Zones\\3[\s]*1406[\s]*DELETE'])
+
     def tearDown(self):
+        '''
+        tearDown method, runs after each test
+        '''
         ret = self.run_function('state.single',
                                 ('file.absent', 'c:\\windows\\system32\\grouppolicy\\machine\\registry.pol'))
         ret = self.run_function('state.single',

From 0a14505e21f30444bc00b569f386f219d622e6cb Mon Sep 17 00:00:00 2001
From: lomeroe <github@proxyman.com>
Date: Fri, 19 Oct 2018 11:24:41 -0500
Subject: [PATCH 25/32] change backslashes in comment string to fix lint error

---
 tests/integration/modules/test_win_lgpo.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/integration/modules/test_win_lgpo.py b/tests/integration/modules/test_win_lgpo.py
index 7a52185d3d6b..c3136b2de15a 100644
--- a/tests/integration/modules/test_win_lgpo.py
+++ b/tests/integration/modules/test_win_lgpo.py
@@ -151,7 +151,7 @@ def setUpClass(cls):
         '''
         class setup function, only runs once
 
-        downloads and extracts the lgpo.exe tool into c:\windows\system32
+        downloads and extracts the lgpo.exe tool into c:/windows/system32
         for use in validating the registry.pol files
 
         gets osrelease grain for tests that are only applicable to certain

From 377ec095abcc4059f4db744587da1a948d597dca Mon Sep 17 00:00:00 2001
From: lomeroe <github@proxyman.com>
Date: Fri, 19 Oct 2018 12:57:54 -0500
Subject: [PATCH 26/32] add a test for #47784

---
 tests/integration/modules/test_win_lgpo.py | 42 ++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/tests/integration/modules/test_win_lgpo.py b/tests/integration/modules/test_win_lgpo.py
index c3136b2de15a..601d700e0df8 100644
--- a/tests/integration/modules/test_win_lgpo.py
+++ b/tests/integration/modules/test_win_lgpo.py
@@ -448,6 +448,48 @@ def test_set_computer_policy_Access_data_sources_across_domains(self):
                                      'Disabled',
                                      [r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\CurrentVersion\\Internet Settings\\Zones\\3[\s]*1406[\s]*DELETE'])
 
+    @destructiveTest
+    def test_set_computer_policy_ActiveHours(self):
+        '''
+        Test configuring the ActiveHours policy, #47784
+        Only applies to 2016Server
+        # activehours.sls
+            active_hours_policy:
+              lgpo.set:
+                - computer_policy:
+                    'ActiveHours':
+                        'ActiveHoursStartTime': '8 AM'
+                        'ActiveHoursEndTime': '7 PM'
+        '''
+        valid_osreleases = ['2016Server']
+        if self.osrelease not in valid_osreleases:
+            self.skipTest('ActiveHours policy is only applicable if the osrelease grain is {0}'.format(' or '.join(valid_osreleases)))
+        else:
+            self._testComputerAdmxPolicy(r'ActiveHours',
+                                         {'ActiveHoursStartTime': '8 AM', 'ActiveHoursEndTime': '7 PM'},
+                                         [
+                                            r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate[\s]*SetActiveHours[\s]*DWORD:1',
+                                            r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate[\s]*ActiveHoursStart[\s]*DWORD:8',
+                                            r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate[\s]*ActiveHoursEnd[\s]*DWORD:19'
+                                         ])
+            self._testComputerAdmxPolicy(r'ActiveHours',
+                                         {'ActiveHoursStartTime': '5 AM', 'ActiveHoursEndTime': '10 PM'},
+                                         [
+                                            r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate[\s]*SetActiveHours[\s]*DWORD:1',
+                                            r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate[\s]*ActiveHoursStart[\s]*DWORD:5',
+                                            r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate[\s]*ActiveHoursEnd[\s]*DWORD:22'
+                                         ])
+            self._testComputerAdmxPolicy('Turn off auto-restart for updates during active hours',
+                                         'Disabled',
+                                         [
+                                            r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate[\s]*SetActiveHours[\s]*DWORD:0',
+                                            r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate[\s]*ActiveHoursStart[\s]*DELETE',
+                                            r'Computer[\s]*Software\\Policies\\Microsoft\\Windows\\WindowsUpdate[\s]*ActiveHoursEnd[\s]*DELETE'
+                                         ])
+            self._testComputerAdmxPolicy(r'Windows Components\Windows Update\Turn off auto-restart for updates during active hours',
+                                         'Not Configured',
+                                         [r'; Source file:  c:\\windows\\system32\\grouppolicy\\machine\\registry.pol[\s]*; PARSING COMPLETED.'])
+
     def tearDown(self):
         '''
         tearDown method, runs after each test

From 0e011ad805a5546b0d03acc66f922a3ccdeb81ea Mon Sep 17 00:00:00 2001
From: lomeroe <github@proxyman.com>
Date: Wed, 24 Oct 2018 10:58:02 -0500
Subject: [PATCH 27/32] add runTest method to class for PY2

---
 tests/integration/modules/test_win_lgpo.py | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tests/integration/modules/test_win_lgpo.py b/tests/integration/modules/test_win_lgpo.py
index 601d700e0df8..b87fb7c6f9d7 100644
--- a/tests/integration/modules/test_win_lgpo.py
+++ b/tests/integration/modules/test_win_lgpo.py
@@ -146,6 +146,12 @@ def _testComputerAdmxPolicy(self,
             # expecting it to fail
             self.assertNotEqual(ret, True)
 
+    def runTest(self):
+        '''
+        runTest method
+        '''
+        pass
+
     @classmethod
     def setUpClass(cls):
         '''

From dc58252e522170ac17eefe84635d4aa70825f1e9 Mon Sep 17 00:00:00 2001
From: lomeroe <github@proxyman.com>
Date: Fri, 2 Nov 2018 08:00:44 -0500
Subject: [PATCH 28/32] update to use a single line if statement when dealing
 with prepended text

add comment on using +2
---
 salt/modules/win_lgpo.py | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/salt/modules/win_lgpo.py b/salt/modules/win_lgpo.py
index 140e3194e23b..897374d41b23 100644
--- a/salt/modules/win_lgpo.py
+++ b/salt/modules/win_lgpo.py
@@ -3094,15 +3094,9 @@ def _getAdmlPresentationRefId(adml_data, ref_id):
                         else:
                             if etree.QName(p_item.tag).localname == 'text':
                                 if prepended_text:
-                                    if getattr(p_item, 'text', ''):
-                                        prepended_text = ' '.join([prepended_text, getattr(p_item, 'text', '').rstrip()])
-                                    else:
-                                        prepended_text = ''
+                                    prepended_text = ' '.join((text for text in (prepended_text, getattr(p_item, 'text', '').rstrip()) if text))
                                 else:
-                                    if getattr(p_item, 'text', ''):
-                                        prepended_text = getattr(p_item, 'text', '').rstrip()
-                                    else:
-                                        prepended_text = ''
+                                    prepended_text = getattr(p_item, 'text', '').rstrip()
                             else:
                                 prepended_text = ''
                     if prepended_text.endswith('.'):
@@ -4185,6 +4179,8 @@ def _regexSearchKeyValueCombo(policy_data, policy_regpath, policy_regkey):
                                b'\00;'])
         match = re.search(_thisSearch, policy_data, re.IGNORECASE)
         if match:
+            # add 2 so we get the ']' and the \00
+            # to return the full policy entry
             return policy_data[match.start():(policy_data.index(b']', match.end())) + 2]
 
     return None

From b8ded8bf5f7914af2213234ce52711f77f4db10c Mon Sep 17 00:00:00 2001
From: "Gareth J. Greenaway" <gareth@wiked.org>
Date: Fri, 2 Nov 2018 12:23:24 -0700
Subject: [PATCH 29/32] Updating the swap function in the mount.swap function
 also check the device name when checking the fstab data.  Updating tests to
 reflect new behavior.

---
 salt/states/mount.py            |  2 +-
 tests/unit/states/test_mount.py | 49 ++++++++++++++++++++++++++++++++-
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/salt/states/mount.py b/salt/states/mount.py
index 5764db207bdf..69f5ee5dea92 100644
--- a/salt/states/mount.py
+++ b/salt/states/mount.py
@@ -764,7 +764,7 @@ def swap(name, persist=True, config='/etc/fstab'):
         else:
             fstab_data = __salt__['mount.fstab'](config)
         if __opts__['test']:
-            if name not in fstab_data:
+            if name not in fstab_data and name not in [fstab_data[item]['device'] for item in fstab_data]:
                 ret['result'] = None
                 if name in on_:
                     ret['comment'] = ('Swap {0} is set to be added to the '
diff --git a/tests/unit/states/test_mount.py b/tests/unit/states/test_mount.py
index 1aae0acf99dc..3e3a75d3cd8b 100644
--- a/tests/unit/states/test_mount.py
+++ b/tests/unit/states/test_mount.py
@@ -236,12 +236,14 @@ def test_swap(self):
         mock_swp = MagicMock(return_value=[name])
         mock_fs = MagicMock(return_value={'none': {'device': name,
                                                    'fstype': 'xfs'}})
+        mock_fs_diff = MagicMock(return_value={'none': {'device': 'something_else',
+                                               'fstype': 'xfs'}})
         mock_aixfs = MagicMock(return_value={name: {'dev': name,
                                                    'fstype': 'jfs2'}})
         mock_emt = MagicMock(return_value={})
         with patch.dict(mount.__grains__, {'os': 'test'}):
             with patch.dict(mount.__salt__, {'mount.swaps': mock_swp,
-                                             'mount.fstab': mock_fs,
+                                             'mount.fstab': mock_fs_diff,
                                              'file.is_link': mock_f}):
                 with patch.dict(mount.__opts__, {'test': True}):
                     comt = ('Swap {0} is set to be added to the '
@@ -249,6 +251,51 @@ def test_swap(self):
                     ret.update({'comment': comt})
                     self.assertDictEqual(mount.swap(name), ret)
 
+                with patch.dict(mount.__opts__, {'test': False}):
+                    comt = ('Swap {0} already active'.format(name))
+                    ret.update({'comment': comt, 'result': True})
+                    self.assertDictEqual(mount.swap(name, persist=False), ret)
+
+                    with patch.dict(mount.__salt__, {'mount.fstab': mock_emt,
+                                                     'mount.set_fstab': mock}):
+                        comt = ('Swap {0} already active'.format(name))
+                        ret.update({'comment': comt, 'result': True})
+                        self.assertDictEqual(mount.swap(name), ret)
+
+                        comt = ('Swap /mnt/sdb already active. '
+                                'Added new entry to the fstab.')
+                        ret.update({'comment': comt, 'result': True,
+                                    'changes': {'persist': 'new'}})
+                        self.assertDictEqual(mount.swap(name), ret)
+
+                        comt = ('Swap /mnt/sdb already active. '
+                                'Updated the entry in the fstab.')
+                        ret.update({'comment': comt, 'result': True,
+                                    'changes': {'persist': 'update'}})
+                        self.assertDictEqual(mount.swap(name), ret)
+
+                        comt = ('Swap /mnt/sdb already active. '
+                                'However, the fstab was not found.')
+                        ret.update({'comment': comt, 'result': False,
+                                    'changes': {}})
+                        self.assertDictEqual(mount.swap(name), ret)
+
+        ret = {'name': name,
+               'result': None,
+               'comment': '',
+               'changes': {}}
+
+        mock = MagicMock(side_effect=['present', 'new', 'change', 'bad config'])
+        mock_emt = MagicMock(return_value={})
+        with patch.dict(mount.__grains__, {'os': 'test'}):
+            with patch.dict(mount.__salt__, {'mount.swaps': mock_swp,
+                                             'mount.fstab': mock_fs,
+                                             'file.is_link': mock_f}):
+                with patch.dict(mount.__opts__, {'test': True}):
+                    comt = ('Swap {0} already active'.format(name))
+                    ret.update({'comment': comt, 'result': True})
+                    self.assertDictEqual(mount.swap(name), ret)
+
                 with patch.dict(mount.__opts__, {'test': False}):
                     comt = ('Swap {0} already active'.format(name))
                     ret.update({'comment': comt, 'result': True})

From b4b0a71a0f681f1919e5f4f9668c05e5982a0106 Mon Sep 17 00:00:00 2001
From: twangboy <slee@saltstack.com>
Date: Fri, 2 Nov 2018 14:30:31 -0600
Subject: [PATCH 30/32] Fix test_matcher on Windows

Fixes a timeout issue in test_salt_documentation_arguments_not_assumed
---
 tests/integration/shell/test_matcher.py | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/tests/integration/shell/test_matcher.py b/tests/integration/shell/test_matcher.py
index 9a2459c43772..39df8f0dc384 100644
--- a/tests/integration/shell/test_matcher.py
+++ b/tests/integration/shell/test_matcher.py
@@ -352,17 +352,20 @@ def test_salt_documentation_arguments_not_assumed(self):
             self.skipTest('This test is failing in Arch due to a bug in salt-testing. '
                           'Skipping until salt-testing can be upgraded. For more information, '
                           'see https://github.com/saltstack/salt-jenkins/issues/324.')
-        data = self.run_salt('-d -t 20')
+        timeout = 20
+        if os_family == 'Windows':
+            timeout = 60
+        data = self.run_salt('-d', timeout=timeout)
         if data:
             assert 'user.add:' in data
-        data = self.run_salt('"*" -d -t 20')
+        data = self.run_salt('"*" -d', timeout=timeout)
         if data:
             assert 'user.add:' in data
-        data = self.run_salt('"*" -d user -t 20')
+        data = self.run_salt('"*" -d user', timeout=timeout)
         assert 'user.add:' in data
-        data = self.run_salt('"*" sys.doc -d user -t 20')
+        data = self.run_salt('"*" sys.doc -d user', timeout=timeout)
         assert 'user.add:' in data
-        data = self.run_salt('"*" sys.doc user -t 20')
+        data = self.run_salt('"*" sys.doc user', timeout=timeout)
         assert 'user.add:' in data
 
     def test_salt_documentation_too_many_arguments(self):

From d968b7df0abed1b1f4cc544b71b540364539ee28 Mon Sep 17 00:00:00 2001
From: twangboy <slee@saltstack.com>
Date: Fri, 2 Nov 2018 16:17:36 -0600
Subject: [PATCH 31/32] Adds timeouts to the manage.down runner

Passes timeouts to the runner from the test
---
 salt/runners/manage.py                   | 9 +++++++--
 tests/integration/runners/test_manage.py | 4 ++--
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/salt/runners/manage.py b/salt/runners/manage.py
index 7b906874f240..34a7d20b4e3e 100644
--- a/salt/runners/manage.py
+++ b/salt/runners/manage.py
@@ -149,7 +149,7 @@ def key_regen():
     return msg
 
 
-def down(removekeys=False, tgt='*', tgt_type='glob'):
+def down(removekeys=False, tgt='*', tgt_type='glob', timeout=None, gather_job_timeout=None):
     '''
     .. versionchanged:: 2017.7.0
         The ``expr_form`` argument has been renamed to ``tgt_type``, earlier
@@ -167,7 +167,12 @@ def down(removekeys=False, tgt='*', tgt_type='glob'):
         salt-run manage.down tgt="webservers" tgt_type="nodegroup"
 
     '''
-    ret = status(output=False, tgt=tgt, tgt_type=tgt_type).get('down', [])
+    ret = status(output=False,
+                 tgt=tgt,
+                 tgt_type=tgt_type,
+                 timeout=timeout,
+                 gather_job_timeout=gather_job_timeout
+    ).get('down', [])
     for minion in ret:
         if removekeys:
             wheel = salt.wheel.Wheel(__opts__)
diff --git a/tests/integration/runners/test_manage.py b/tests/integration/runners/test_manage.py
index 2aa03dc19b25..cdd3e02c6bff 100644
--- a/tests/integration/runners/test_manage.py
+++ b/tests/integration/runners/test_manage.py
@@ -17,7 +17,7 @@ def test_up(self):
         '''
         manage.up
         '''
-        ret = self.run_run_plus('manage.up')
+        ret = self.run_run_plus('manage.up', timeout=60)
         self.assertIn('minion', ret['return'])
         self.assertIn('sub_minion', ret['return'])
         self.assertTrue(any('- minion' in out for out in ret['out']))
@@ -27,7 +27,7 @@ def test_down(self):
         '''
         manage.down
         '''
-        ret = self.run_run_plus('manage.down')
+        ret = self.run_run_plus('manage.down', timeout=60)
         self.assertNotIn('minion', ret['return'])
         self.assertNotIn('sub_minion', ret['return'])
         self.assertNotIn('minion', ret['out'])

From b0341ea7dc3887486c5e7ec408a4c4df0104ab47 Mon Sep 17 00:00:00 2001
From: twangboy <slee@saltstack.com>
Date: Mon, 5 Nov 2018 12:15:06 -0700
Subject: [PATCH 32/32] Remove flaky test

---
 tests/integration/shell/test_matcher.py | 26 -------------------------
 1 file changed, 26 deletions(-)

diff --git a/tests/integration/shell/test_matcher.py b/tests/integration/shell/test_matcher.py
index 39df8f0dc384..dd936fc626cb 100644
--- a/tests/integration/shell/test_matcher.py
+++ b/tests/integration/shell/test_matcher.py
@@ -342,32 +342,6 @@ def test_salt_documentation(self):
         data = self.run_salt('-d "*" user')
         self.assertIn('user.add:', data)
 
-    @flaky
-    def test_salt_documentation_arguments_not_assumed(self):
-        '''
-        Test to see if we're not auto-adding '*' and 'sys.doc' to the call
-        '''
-        os_family = self.run_call('--local grains.get os_family')[1].strip()
-        if os_family == 'Arch':
-            self.skipTest('This test is failing in Arch due to a bug in salt-testing. '
-                          'Skipping until salt-testing can be upgraded. For more information, '
-                          'see https://github.com/saltstack/salt-jenkins/issues/324.')
-        timeout = 20
-        if os_family == 'Windows':
-            timeout = 60
-        data = self.run_salt('-d', timeout=timeout)
-        if data:
-            assert 'user.add:' in data
-        data = self.run_salt('"*" -d', timeout=timeout)
-        if data:
-            assert 'user.add:' in data
-        data = self.run_salt('"*" -d user', timeout=timeout)
-        assert 'user.add:' in data
-        data = self.run_salt('"*" sys.doc -d user', timeout=timeout)
-        assert 'user.add:' in data
-        data = self.run_salt('"*" sys.doc user', timeout=timeout)
-        assert 'user.add:' in data
-
     def test_salt_documentation_too_many_arguments(self):
         '''
         Test to see if passing additional arguments shows an error