Properly resolve the policy name

salt/modules/win_lgpo.py

@@ -5658,25 +5658,48 @@ def _getAdmlPresentationRefId(adml_data, ref_id):
     helper function to check for a presentation label for a policy element
     '''
     search_results = adml_data.xpath('//*[@*[local-name() = "refId"] = "{0}"]'.format(ref_id))
-    prepended_text = ''
+    alternate_label = ''
     if search_results:
         for result in search_results:
             the_localname = etree.QName(result.tag).localname
-            if the_localname == 'textBox' \
-                    or the_localname == 'comboBox':
+
+            # We want to prefer result.text as the label, however, if it is none
+            # we will fall back to this method for getting the label
+            # Brings some code back from:
+            # https://github.com/saltstack/salt/pull/55823/files#diff-b2e4dac5ccc17ab548f245371ec5d6faL5658
+            if result.text is None:
+                # Get the label from the text element above the referenced
+                # element. For example:
+                # --- taken from AppPrivacy.adml ---
+                # <text>Force allow these specific apps (use Package Family Names):</text>
+                # <multiTextBox refId="LetAppsSyncWithDevices_ForceAllowTheseApps_List"/>
+                # In this case, the label for the refId is the text element
+                # above it.
+                presentation_element = PRESENTATION_ANCESTOR_XPATH(result)
+                if presentation_element:
+                    presentation_element = presentation_element[0]
+                    if TEXT_ELEMENT_XPATH(presentation_element):
+                        for p_item in presentation_element.getchildren():
+                            if p_item == result:
+                                break
+                            if etree.QName(p_item.tag).localname == 'text':
+                                if getattr(p_item, 'text'):
+                                    alternate_label = getattr(p_item, 'text').rstrip()
+                        if alternate_label.endswith('.'):
+                            alternate_label = ''
+
+            if the_localname in ['textBox', 'comboBox']:
                 label_items = result.xpath('.//*[local-name() = "label"]')
                 for label_item in label_items:
                     if label_item.text:
-                        return (prepended_text + ' ' + label_item.text.rstrip().rstrip(':')).lstrip()
-            elif the_localname == 'decimalTextBox' \
-                    or the_localname == 'longDecimalTextBox' \
-                    or the_localname == 'dropdownList' \
-                    or the_localname == 'listBox' \
-                    or the_localname == 'checkBox' \
-                    or the_localname == 'text' \
-                    or the_localname == 'multiTextBox':
+                        return label_item.text.rstrip().rstrip(':')
+            elif the_localname in ['decimalTextBox', 'longDecimalTextBox',
+                                   'dropdownList', 'listBox', 'checkBox',
+                                   'text', 'multiTextBox']:
                 if result.text:
-                    return (prepended_text + ' ' + result.text.rstrip().rstrip(':')).lstrip()
+                    return result.text.rstrip().rstrip(':')
+                else:
+                    return alternate_label.rstrip(':')
     return None
 
 
@@ -6139,6 +6162,10 @@ def _processValueItem(element, reg_key, reg_valuename, policy, parent_element,
 
         if standard_element_expected_string and not check_deleted:
             if this_element_value is not None:
+                # Sometimes values come in as strings
+                if isinstance(this_element_value, str):
+                    log.debug('Converting {0} to bytes'.format(this_element_value))
+                    this_element_value = this_element_value.encode('utf-32-le')
                 expected_string = b''.join(['['.encode('utf-16-le'),
                                             reg_key,
                                             encoded_null,

salt/states/win_lgpo.py

@@ -112,6 +112,7 @@
 import salt.utils.data
 import salt.utils.dictdiffer
 import salt.utils.json
+import salt.utils.stringutils
 import salt.utils.versions
 import salt.utils.win_functions
 
@@ -152,6 +153,35 @@ def _compare_policies(new_policy, current_policy):
             return False
 
 
+def _convert_to_unicode(data):
+    '''
+    Helper function that makes sure all items in the dictionary are unicode for
+    comparing the existing state with the desired state. This function is only
+    needed for Python 2 and can be removed once we've migrated to Python 3.
+
+    The data returned by the current settings sometimes has a mix of unicode and
+    string values (these don't matter in Py3). This causes the comparison to
+    say it's not in the correct state even though it is. They basically compares
+    apples to apples, etc.
+
+    Also, in Python 2, the utf-16 encoded strings remain utf-16 encoded (each
+    character separated by `/x00`) In Python 3 it returns a utf-8 string. This
+    will just remove all the null bytes (`/x00`), again comparing apples to
+    apples.
+    '''
+    if isinstance(data, six.string_types):
+        data = data.replace('\x00', '')
+        return salt.utils.stringutils.to_unicode(data)
+    elif isinstance(data, dict):
+        return dict((_convert_to_unicode(k),
+                     _convert_to_unicode(v))
+                    for k, v in data.items())
+    elif isinstance(data, list):
+        return list(_convert_to_unicode(v) for v in data)
+    else:
+        return data
+
+
 def set_(name,
          setting=None,
          policy_class=None,
@@ -342,6 +372,9 @@ def set_(name,
                     requested_policy_check = salt.utils.json.loads(requested_policy_json)
                     current_policy_check = salt.utils.json.loads(current_policy_json)
 
+                    if six.PY2:
+                        current_policy_check = _convert_to_unicode(current_policy_check)
+
                     # Are the requested and current policies identical
                     policies_are_equal = _compare_policies(
                         requested_policy_check, current_policy_check)

tests/unit/modules/test_win_lgpo.py

@@ -15,31 +15,27 @@
 
 # Import Salt Libs
 import salt.config
-import salt.modules.cmdmod
-import salt.modules.file
-import salt.modules.win_file as win_file
+import salt.loader
 import salt.modules.win_lgpo as win_lgpo
+import salt.states.win_lgpo
 import salt.utils.platform
-import salt.utils.win_dacl
-import salt.utils.win_lgpo_auditpol
-import salt.utils.win_reg
+import salt.utils.stringutils
+
+# Import 3rd Party Libs
+import salt.ext.six as six
+
+# We're going to actually use the loader, without grains (slow)
+opts = salt.config.DEFAULT_MINION_OPTS.copy()
+utils = salt.loader.utils(opts)
+modules = salt.loader.minion_mods(opts, utils=utils)
 
 LOADER_DICTS = {
     win_lgpo: {
-        '__salt__': {
-            'file.file_exists': salt.modules.file.file_exists,
-            'file.makedirs': win_file.makedirs_,
-            'file.write': salt.modules.file.write,
-            'file.remove': win_file.remove,
-            'cmd.run': salt.modules.cmdmod.run},
-        '__opts__': salt.config.DEFAULT_MINION_OPTS.copy(),
-        '__utils__': {
-            'reg.read_value': salt.utils.win_reg.read_value,
-            'auditpol.get_auditpol_dump':
-                salt.utils.win_lgpo_auditpol.get_auditpol_dump}},
-    win_file: {
-        '__utils__': {
-            'dacl.set_perms': salt.utils.win_dacl.set_perms}}}
+        '__opts__': opts,
+        '__salt__': modules,
+        '__utils__': utils,
+    }
+}
 
 
 class WinLGPOTestCase(TestCase):
@@ -605,13 +601,16 @@ def _get_policy_adm_setting(self, policy_name, policy_class,
             policy_class=policy_class,
             adml_language='en-US')
         if success:
-            return salt.modules.win_lgpo._get_policy_adm_setting(
+            results = salt.modules.win_lgpo._get_policy_adm_setting(
                 admx_policy=policy_obj,
                 policy_class=policy_class,
                 adml_language='en-US',
                 return_full_policy_names=return_full_policy_names,
                 hierarchical_return=hierarchical_return
             )
+            if six.PY2:
+                results = salt.states.win_lgpo._convert_to_unicode(results)
+            return results
         return 'Policy Not Found'
 
     def test_point_and_print_enabled(self):
@@ -631,7 +630,7 @@ def test_point_and_print_enabled(self):
                     True,
                 'PointAndPrint_TrustedServers_Chk':
                     True,
-                u'PointAndPrint_TrustedServers_Edit':
+                'PointAndPrint_TrustedServers_Edit':
                     'fakeserver1;fakeserver2'}}
         self.assertDictEqual(result, expected)
 
@@ -655,7 +654,7 @@ def test_point_and_print_enabled_hierarchical(self):
                                 True,
                             'PointAndPrint_TrustedServers_Chk':
                                 True,
-                            u'PointAndPrint_TrustedServers_Edit':
+                            'PointAndPrint_TrustedServers_Edit':
                                 'fakeserver1;fakeserver2'}}}}}
         self.assertDictEqual(result, expected)
 
@@ -674,8 +673,8 @@ def test_point_and_print_enabled_full_names(self):
                     'Show warning and elevation prompt',
                 'Users can only point and print to machines in their forest':
                     True,
-                u'Users can only point and print to these servers': True,
-                u'When updating drivers for an existing connection':
+                'Users can only point and print to these servers': True,
+                'When updating drivers for an existing connection':
                     'Show warning only'}}
         self.assertDictEqual(result, expected)
 
@@ -699,8 +698,36 @@ def test_point_and_print_enabled_full_names_hierarchical(self):
                             'Users can only point and print to machines in '
                             'their forest':
                                 True,
-                            u'Users can only point and print to these servers':
+                            'Users can only point and print to these servers':
                                 True,
-                            u'When updating drivers for an existing connection':
+                            'When updating drivers for an existing connection':
                                 'Show warning only'}}}}}
         self.assertDictEqual(result, expected)
+
+
+@skipIf(not salt.utils.platform.is_windows(), 'System is not Windows')
+class WinLGPOGetPolicyFromPolicyResources(TestCase, LoaderModuleMockMixin):
+    '''
+    Test functions related to policy info gathered from ADMX/ADML files
+    '''
+    adml_data = None
+
+    def setup_loader_modules(self):
+        return LOADER_DICTS
+
+    def setUp(self):
+        if self.adml_data is None:
+            self.adml_data = win_lgpo._get_policy_resources('en-US')
+
+    def test__getAdmlPresentationRefId(self):
+        ref_id = 'LetAppsAccessAccountInfo_Enum'
+        expected = 'Default for all apps'
+        result = win_lgpo._getAdmlPresentationRefId(self.adml_data, ref_id)
+        self.assertEqual(result, expected)
+
+    def test__getAdmlPresentationRefId_result_text_is_none(self):
+        ref_id = 'LetAppsAccessAccountInfo_UserInControlOfTheseApps_List'
+        expected = 'Put user in control of these specific apps (use Package ' \
+                   'Family Names)'
+        result = win_lgpo._getAdmlPresentationRefId(self.adml_data, ref_id)
+        self.assertEqual(result, expected)

tests/unit/states/test_win_lgpo.py

@@ -13,37 +13,26 @@
 
 # Import Salt Libs
 import salt.config
-import salt.modules.cmdmod
-import salt.modules.file
-import salt.modules.win_file as win_file
-import salt.modules.win_lgpo as win_lgpo_mod
+import salt.loader
 import salt.states.win_lgpo as win_lgpo
 import salt.utils.platform
-import salt.utils.win_dacl
-import salt.utils.win_lgpo_auditpol
-import salt.utils.win_reg
+import salt.utils.stringutils
+
+# Import 3rd Party Libs
+import salt.ext.six as six
+
+# We're going to actually use the loader, without grains (slow)
+opts = salt.config.DEFAULT_MINION_OPTS.copy()
+utils = salt.loader.utils(opts)
+modules = salt.loader.minion_mods(opts, utils=utils)
 
 LOADER_DICTS = {
     win_lgpo: {
-        '__salt__': {
-            'lgpo.get_policy': win_lgpo_mod.get_policy,
-            'lgpo.get_policy_info': win_lgpo_mod.get_policy_info,
-            'lgpo.set': win_lgpo_mod.set_}},
-    win_lgpo_mod: {
-        '__salt__': {
-            'cmd.run': salt.modules.cmdmod.run,
-            'file.file_exists': salt.modules.file.file_exists,
-            'file.makedirs': win_file.makedirs_,
-            'file.remove': win_file.remove,
-            'file.write': salt.modules.file.write},
-        '__opts__': salt.config.DEFAULT_MINION_OPTS.copy(),
-        '__utils__': {
-            'reg.read_value': salt.utils.win_reg.read_value,
-            'auditpol.get_auditpol_dump':
-                salt.utils.win_lgpo_auditpol.get_auditpol_dump}},
-    win_file: {
-        '__utils__': {
-            'dacl.set_perms': salt.utils.win_dacl.set_perms}}}
+        '__opts__': opts,
+        '__salt__': modules,
+        '__utils__': utils,
+    }
+}
 
 
 class WinLGPOComparePoliciesTestCase(TestCase):
@@ -193,6 +182,7 @@ def test_current_element_naming_style(self):
         with patch.dict(win_lgpo.__opts__, {'test': False}):
             result = win_lgpo.set_(name='test_state',
                                    computer_policy=computer_policy)
+            result = win_lgpo._convert_to_unicode(result)
         expected = {
             'Point and Print Restrictions': {
                 'Enter fully qualified server names separated by '
@@ -203,9 +193,9 @@ def test_current_element_naming_style(self):
                 'Users can only point and print to machines in '
                 'their forest':
                     True,
-                u'Users can only point and print to these servers':
+                'Users can only point and print to these servers':
                     True,
-                u'When updating drivers for an existing connection':
+                'When updating drivers for an existing connection':
                     'Show warning only'}}
         self.assertDictEqual(
             result['changes']['new']['Computer Configuration'], expected)
@@ -231,6 +221,8 @@ def test_old_element_naming_style(self):
         with patch.dict(win_lgpo.__opts__, {'test': False}):
             result = win_lgpo.set_(name='test_state',
                                    computer_policy=computer_policy)
+            if six.PY2:
+                result = win_lgpo._convert_to_unicode(result)
         expected = {
             'Point and Print Restrictions': {
                 'Enter fully qualified server names separated by '
@@ -241,9 +233,9 @@ def test_old_element_naming_style(self):
                 'Users can only point and print to machines in '
                 'their forest':
                     True,
-                u'Users can only point and print to these servers':
+                'Users can only point and print to these servers':
                     True,
-                u'When updating drivers for an existing connection':
+                'When updating drivers for an existing connection':
                     'Show warning only'}}
         self.assertDictEqual(
             result['changes']['new']['Computer Configuration'], expected)
@@ -332,7 +324,7 @@ def test_current_element_naming_style(self):
             'comment': 'All specified policies are properly configured'}
         self.assertDictEqual(result['changes'], expected['changes'])
         self.assertTrue(result['result'])
-        self.assertEqual(result['comment'], result['comment'])
+        self.assertEqual(result['comment'], expected['comment'])
 
     def test_old_element_naming_style(self):
         computer_policy = {
@@ -362,7 +354,7 @@ def test_old_element_naming_style(self):
                        'All specified policies are properly configured'}
         self.assertDictEqual(result['changes'], expected['changes'])
         self.assertTrue(result['result'])
-        self.assertEqual(result['comment'], result['comment'])
+        self.assertEqual(result['comment'], expected['comment'])
 
     def test_invalid_elements(self):
         computer_policy = {