Don't fail when registry.pol missing

changelog/64126.fixed.md

@@ -0,0 +1 @@
+lgpo_reg.set_value now returns ``True`` on success instead of ``None``

salt/modules/win_lgpo_reg.py

@@ -137,9 +137,10 @@ def write_reg_pol(data, policy_class="Machine"):
 
     Raises:
         SaltInvocationError: Invalid policy class
+        CommandExecutionError: On failure
 
     Returns:
-        None
+        bool: True if successful
 
     CLI Example:
 
@@ -175,7 +176,6 @@ def get_value(key, v_name, policy_class="Machine"):
     file.
 
     Args:
-
         key (str): The registry key where the value name resides
 
         v_name (str): The value name to retrieve
@@ -228,7 +228,6 @@ def get_key(key, policy_class="Machine"):
     Get all the values set in a key in the ``Registry.pol`` file.
 
     Args:
-
         key (str): The registry key where the values reside
 
         policy_class (str): The registry class to read from. Can be one of the
@@ -278,7 +277,6 @@ def set_value(
     style policies. This is the equivalent of setting a policy to ``Enabled``
 
     Args:
-
         key (str): The registry key path
 
         v_name (str): The registry value name within the key
@@ -305,14 +303,14 @@ def set_value(
 
             Default is ``Machine``
 
-    Returns:
-        bool: ``True`` if successful, otherwise ``False``
-
     Raises:
         SaltInvocationError: Invalid policy_class
         SaltInvocationError: Invalid v_type
         SaltInvocationError: v_data doesn't match v_type
 
+    Returns:
+        bool: ``True`` if successful, otherwise ``False``
+
     CLI Example:
 
     .. code-block:: bash
@@ -385,7 +383,7 @@ def set_value(
 
     write_reg_pol(pol_data)
 
-    salt.utils.win_reg.set_value(
+    return salt.utils.win_reg.set_value(
         hive=hive,
         key=key,
         vname=v_name,
@@ -401,7 +399,6 @@ def disable_value(key, v_name, policy_class="machine"):
     to ``Disabled`` in the Group Policy editor (``gpedit.msc``)
 
     Args:
-
         key (str): The registry key path
 
         v_name (str): The registry value name within the key
@@ -415,13 +412,14 @@ def disable_value(key, v_name, policy_class="machine"):
 
             Default is ``Machine``
 
+    Raises:
+        SaltInvocationError: Invalid policy_class
+        CommandExecutionError: On failure
+
     Returns:
         bool: ``True`` if successful, otherwise ``False``
         None: If already disabled
 
-    Raises:
-        SaltInvocationError: Invalid policy_class
-
     CLI Example:
 
     .. code-block:: bash
@@ -468,7 +466,7 @@ def disable_value(key, v_name, policy_class="machine"):
 
     write_reg_pol(pol_data)
 
-    salt.utils.win_reg.delete_value(hive=hive, key=key, vname=v_name)
+    return salt.utils.win_reg.delete_value(hive=hive, key=key, vname=v_name)
 
 
 def delete_value(key, v_name, policy_class="Machine"):
@@ -478,7 +476,6 @@ def delete_value(key, v_name, policy_class="Machine"):
     ``Not Configured``.
 
     Args:
-
         key (str): The registry key path
 
         v_name (str): The registry value name within the key
@@ -492,13 +489,14 @@ def delete_value(key, v_name, policy_class="Machine"):
 
             Default is ``Machine``
 
+    Raises:
+        SaltInvocationError: Invalid policy_class
+        CommandExecutionError: On failure
+
     Returns:
         bool: ``True`` if successful, otherwise ``False``
         None: Key/value not present
 
-    Raises:
-        SaltInvocationError: Invalid policy_class
-
     CLI Example:
 
     .. code-block:: bash
@@ -538,7 +536,7 @@ def delete_value(key, v_name, policy_class="Machine"):
 
     write_reg_pol(pol_data)
 
-    salt.utils.win_reg.delete_value(hive=hive, key=key, vname=v_name)
+    return salt.utils.win_reg.delete_value(hive=hive, key=key, vname=v_name)
 
 
 # This is for testing different settings and verifying that we are writing the

salt/states/win_lgpo_reg.py

@@ -72,23 +72,6 @@ def __virtual__():
     return __virtualname__
 
 
-def _format_changes(changes, key, v_name):
-    """
-    Reformat the changes dictionary to group new and old together.
-    """
-    new_changes = {"new": {}, "old": {}}
-    for item in changes:
-        if changes[item]["new"]:
-            new_changes["new"][item] = changes[item]["new"]
-            new_changes["new"]["key"] = key
-            new_changes["new"]["name"] = v_name
-        if changes[item]["old"]:
-            new_changes["old"][item] = changes[item]["old"]
-            new_changes["old"]["key"] = key
-            new_changes["old"]["name"] = v_name
-    return new_changes
-
-
 def value_present(name, key, v_data, v_type="REG_DWORD", policy_class="Machine"):
     r"""
     Ensure a registry setting is present in the Registry.pol file.
@@ -170,12 +153,16 @@ def value_present(name, key, v_data, v_type="REG_DWORD", policy_class="Machine")
         key=key, v_name=name, policy_class=policy_class
     )
 
-    changes = salt.utils.data.compare_dicts(old, new)
-
-    if changes:
+    if str(new["data"]) == v_data and new["type"] == v_type:
         ret["comment"] = "Registry.pol value has been set"
-        ret["changes"] = _format_changes(changes, key, name)
         ret["result"] = True
+    else:
+        ret["comment"] = "Failed to set Registry.pol value"
+
+    changes = salt.utils.data.recursive_diff(old, new)
+
+    if changes:
+        ret["changes"] = changes
 
     return ret
 
@@ -238,12 +225,16 @@ def value_disabled(name, key, policy_class="Machine"):
         key=key, v_name=name, policy_class=policy_class
     )
 
-    changes = salt.utils.data.compare_dicts(old, new)
+    if "**del." in str(new["data"]) and new["type"] == "REG_SZ":
+        ret["comment"] = "Registry.pol value disabled"
+        ret["result"] = True
+    else:
+        ret["comment"] = "Failed to disable Registry.pol value"
+
+    changes = salt.utils.data.recursive_diff(old, new)
 
     if changes:
-        ret["comment"] = "Registry.pol value enabled"
-        ret["changes"] = _format_changes(changes, key, name)
-        ret["result"] = True
+        ret["changes"] = changes
 
     return ret
 
@@ -306,14 +297,17 @@ def value_absent(name, key, policy_class="Machine"):
         key=key, v_name=name, policy_class=policy_class
     )
 
-    if new is None:
+    if not new:
+        ret["comment"] = "Registry.pol value deleted"
+        ret["result"] = True
+        # We're setting this here in case new is None
         new = {}
+    else:
+        ret["comment"] = "Failed to delete Registry.pol value"
 
-    changes = salt.utils.data.compare_dicts(old, new)
+    changes = salt.utils.data.recursive_diff(old, new)
 
     if changes:
-        ret["comment"] = "Registry.pol value deleted"
-        ret["changes"] = _format_changes(changes, key, name)
-        ret["result"] = True
+        ret["changes"] = changes
 
     return ret

salt/utils/win_lgpo_reg.py

@@ -67,13 +67,11 @@ def search_reg_pol(search_string, policy_data):
     gpt.ini
 
     Args:
-
         search_string (str): The string to search for
 
         policy_data (str): The data to be searched
 
     Returns:
-
         bool: ``True`` if the regex search_string is found, otherwise ``False``
     """
     if policy_data:
@@ -91,7 +89,6 @@ def read_reg_pol_file(reg_pol_path):
     Helper function to read the content of the Registry.pol file
 
     Args:
-
         reg_pol_path (str): The path to the Registry.pol file
 
     Returns:
@@ -120,7 +117,6 @@ def write_reg_pol_data(
     to be processed
 
     Args:
-
         data_to_write (bytes): Data to write into the user/machine registry.pol
             file
 
@@ -132,6 +128,12 @@ def write_reg_pol_data(
         gpt_extension_guid (str): ADMX registry extension guid for the class
 
         gpt_ini_path (str): The path to the gpt.ini file
+
+    Returns:
+        bool: True if successful
+
+    Raises:
+        CommandExecutionError: On failure
     """
     # Write Registry.pol file
     if not os.path.exists(policy_file_path):
@@ -254,6 +256,7 @@ def write_reg_pol_data(
             )
             log.exception(msg)
             raise CommandExecutionError(msg)
+    return True
 
 
 def reg_pol_to_dict(policy_data):
@@ -273,6 +276,12 @@ def reg_pol_to_dict(policy_data):
     # https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-gpreg/5c092c22-bf6b-4e7f-b180-b20743d368f5
 
     reg_pol_header = REG_POL_HEADER.encode("utf-16-le")
+
+    # If policy_data is None, that means the Registry.pol file is missing
+    # So, we'll create it
+    if policy_data is None:
+        policy_data = reg_pol_header
+
     if not policy_data.startswith(reg_pol_header):
         msg = "LGPO_REG Util: Invalid Header. Registry.pol may be corrupt"
         raise CommandExecutionError(msg)

tests/pytests/unit/modules/test_win_lgpo_reg.py

@@ -152,12 +152,16 @@ def test_get_key_invalid_policy_class():
 
 
 def test_set_value(empty_reg_pol):
-    expected = {"data": 1, "type": "REG_DWORD"}
     key = "SOFTWARE\\MyKey"
     v_name = "MyValue"
-    lgpo_reg.set_value(key=key, v_name=v_name, v_data="1")
+    # Test command return
+    result = lgpo_reg.set_value(key=key, v_name=v_name, v_data="1")
+    assert result is True
+    # Test value actually set in Registry.pol
+    expected = {"data": 1, "type": "REG_DWORD"}
     result = lgpo_reg.get_value(key=key, v_name=v_name)
     assert result == expected
+    # Test that the registry value has been set
     expected = {
         "hive": "HKLM",
         "key": key,
@@ -249,14 +253,18 @@ def test_set_value_invalid_reg_dword():
 
 
 def test_disable_value(reg_pol):
+    key = "SOFTWARE\\MyKey1"
+    # Test that the command completed successfully
+    result = lgpo_reg.disable_value(key=key, v_name="MyValue1")
+    assert result is True
+    # Test that the value was actually set in Registry.pol
     expected = {
         "**del.MyValue1": {"data": " ", "type": "REG_SZ"},
         "**del.MyValue2": {"data": " ", "type": "REG_SZ"},
     }
-    key = "SOFTWARE\\MyKey1"
-    lgpo_reg.disable_value(key=key, v_name="MyValue1")
     result = lgpo_reg.get_key(key=key)
     assert result == expected
+    # Test that the registry value has been removed
     result = salt.utils.win_reg.value_exists(hive="HKLM", key=key, vname="MyValue1")
     assert result is False
 
@@ -283,16 +291,20 @@ def test_disable_value_invalid_policy_class():
 
 
 def test_delete_value_existing(reg_pol):
+    key = "SOFTWARE\\MyKey1"
+    # Test that the command completes successfully
+    result = lgpo_reg.delete_value(key=key, v_name="MyValue1")
+    assert result is True
+    # Test that the value is actually removed from Registry.pol
     expected = {
         "**del.MyValue2": {
             "data": " ",
             "type": "REG_SZ",
         },
     }
-    key = "SOFTWARE\\MyKey1"
-    lgpo_reg.delete_value(key=key, v_name="MyValue1")
     result = lgpo_reg.get_key(key=key)
     assert result == expected
+    # Test that the registry entry has been removed
     result = salt.utils.win_reg.value_exists(hive="HKLM", key=key, vname="MyValue2")
     assert result is False