diff --git a/changelog/64126.fixed.md b/changelog/64126.fixed.md new file mode 100644 index 000000000000..fb6cf7c46b4c --- /dev/null +++ b/changelog/64126.fixed.md @@ -0,0 +1 @@ +lgpo_reg.set_value now returns ``True`` on success instead of ``None`` diff --git a/salt/modules/win_lgpo_reg.py b/salt/modules/win_lgpo_reg.py index cc678549ae4e..4052de62bd3d 100644 --- a/salt/modules/win_lgpo_reg.py +++ b/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 diff --git a/salt/states/win_lgpo_reg.py b/salt/states/win_lgpo_reg.py index 01b10e4e6101..7a514068acb8 100644 --- a/salt/states/win_lgpo_reg.py +++ b/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 diff --git a/salt/utils/win_lgpo_reg.py b/salt/utils/win_lgpo_reg.py index da4c4377631d..0a96d584df8b 100644 --- a/salt/utils/win_lgpo_reg.py +++ b/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) diff --git a/tests/pytests/unit/modules/test_win_lgpo_reg.py b/tests/pytests/unit/modules/test_win_lgpo_reg.py index df0b4624eae8..6d4a824b308b 100644 --- a/tests/pytests/unit/modules/test_win_lgpo_reg.py +++ b/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 diff --git a/tests/pytests/unit/states/test_win_lgpo_reg.py b/tests/pytests/unit/states/test_win_lgpo_reg.py index f57262869f4e..d2ca5cc74337 100644 --- a/tests/pytests/unit/states/test_win_lgpo_reg.py +++ b/tests/pytests/unit/states/test_win_lgpo_reg.py @@ -84,8 +84,6 @@ def test_value_present(empty_reg_pol): expected = { "changes": { "new": { - "name": "MyValue", - "key": "SOFTWARE\\MyKey", "data": 1, "type": "REG_DWORD", }, @@ -111,14 +109,10 @@ def test_value_present_existing_change(reg_pol): expected = { "changes": { "new": { - "name": "MyValue1", - "key": "SOFTWARE\\MyKey1", "data": 2, "type": "REG_DWORD", }, "old": { - "name": "MyValue1", - "key": "SOFTWARE\\MyKey1", "data": "squidward", "type": "REG_SZ", }, @@ -183,14 +177,10 @@ def test_value_present_existing_disabled(reg_pol): "changes": { "new": { "data": 2, - "key": "SOFTWARE\\MyKey1", - "name": "MyValue2", "type": "REG_DWORD", }, "old": { "data": "**del.MyValue2", - "key": "SOFTWARE\\MyKey1", - "name": "MyValue2", "type": "REG_SZ", }, }, @@ -213,13 +203,11 @@ def test_value_disabled(empty_reg_pol): "changes": { "new": { "data": "**del.MyValue1", - "key": "SOFTWARE\\MyKey1", - "name": "MyValue1", "type": "REG_SZ", }, "old": {}, }, - "comment": "Registry.pol value enabled", + "comment": "Registry.pol value disabled", "name": "MyValue1", "result": True, } @@ -238,16 +226,12 @@ def test_value_disabled_existing_change(reg_pol): "changes": { "new": { "data": "**del.MyValue1", - "key": "SOFTWARE\\MyKey1", - "name": "MyValue1", }, "old": { "data": "squidward", - "key": "SOFTWARE\\MyKey1", - "name": "MyValue1", }, }, - "comment": "Registry.pol value enabled", + "comment": "Registry.pol value disabled", "name": "MyValue1", "result": True, } @@ -299,8 +283,6 @@ def test_value_absent(reg_pol): "new": {}, "old": { "data": "squidward", - "key": "SOFTWARE\\MyKey1", - "name": "MyValue1", "type": "REG_SZ", }, }, @@ -335,8 +317,6 @@ def test_value_absent_disabled(reg_pol): "new": {}, "old": { "data": "**del.MyValue2", - "key": "SOFTWARE\\MyKey1", - "name": "MyValue2", "type": "REG_SZ", }, },