From b5ab5b909642115cdb0490d2e91c51e3c9c4e01f Mon Sep 17 00:00:00 2001 From: Leif Liddy Date: Thu, 31 Aug 2023 03:23:32 +0200 Subject: [PATCH 1/5] rebased changes and added new test --- salt/states/linux_acl.py | 31 +-- tests/pytests/unit/states/test_linux_acl.py | 244 ++++++++++++++++++++ 2 files changed, 262 insertions(+), 13 deletions(-) diff --git a/salt/states/linux_acl.py b/salt/states/linux_acl.py index b854ffb818b7..72cc6151c4e1 100644 --- a/salt/states/linux_acl.py +++ b/salt/states/linux_acl.py @@ -131,8 +131,8 @@ def present(name, acl_type, acl_name="", perms="", recurse=False, force=False): """ ret = {"name": name, "result": True, "changes": {}, "comment": ""} - _octal = {"r": 4, "w": 2, "x": 1, "-": 0} - _octal_lookup = {0: "-", 1: "r", 2: "w", 4: "x"} + _octal = {"r": 4, "w": 2, "x": 1, "X": 1, "-": 0} + _octal_lookup = {4: "r", 2: "w", 1: "x", 0: "-"} if not os.path.exists(name): ret["comment"] = "{} does not exist".format(name) @@ -174,7 +174,8 @@ def present(name, acl_type, acl_name="", perms="", recurse=False, force=False): user = None if user: - octal_sum = sum(_octal.get(i, i) for i in perms) + octal_new = sum(_octal.get(i, i) for i in perms) + conditional_x = bool(perms.endswith("X")) need_refresh = False # If recursive check all paths retrieved via acl.getfacl if recurse: @@ -188,11 +189,15 @@ def present(name, acl_type, acl_name="", perms="", recurse=False, force=False): else: _current_perms_path = __current_perms[path] for user_acl in _current_perms_path.get(_acl_type, []): - if ( - _search_name in user_acl - and user_acl[_search_name]["octal"] == octal_sum - ): - acl_found = True + if _search_name in user_acl: + octal_current = user_acl[_search_name]["octal"] + executable = bool(octal_current % 2 == 1) + if octal_current == octal_new or ( + conditional_x + and not executable + and octal_current == (octal_new - 1) + ): + acl_found = True if not acl_found: need_refresh = True break @@ -208,17 +213,17 @@ def present(name, acl_type, acl_name="", perms="", recurse=False, force=False): ret["comment"] = "Permissions are in the desired state" else: _num = user[_search_name]["octal"] - new_perms = "{}{}{}".format( - _octal_lookup[_num & 1], - _octal_lookup[_num & 2], + old_perms = "{}{}{}".format( _octal_lookup[_num & 4], + _octal_lookup[_num & 2], + _octal_lookup[_num & 1], ) changes = { "new": {"acl_name": acl_name, "acl_type": acl_type, "perms": perms}, "old": { "acl_name": acl_name, "acl_type": acl_type, - "perms": new_perms, + "perms": old_perms, }, } @@ -227,7 +232,7 @@ def present(name, acl_type, acl_name="", perms="", recurse=False, force=False): { "comment": ( "Updated permissions will be applied for " - "{}: {} -> {}".format(acl_name, new_perms, perms) + "{}: {} -> {}".format(acl_name, old_perms, perms) ), "result": None, "changes": changes, diff --git a/tests/pytests/unit/states/test_linux_acl.py b/tests/pytests/unit/states/test_linux_acl.py index 60bbe55f51c0..0119e6385f3c 100644 --- a/tests/pytests/unit/states/test_linux_acl.py +++ b/tests/pytests/unit/states/test_linux_acl.py @@ -266,6 +266,250 @@ def test_present(): == ret ) +def test_present_conditional_x(): + """ + Test to ensure a Linux ACL containing a conditional X is applied correctly + """ + maxDiff = None + name = "/root" + acl_type = "users" + acl_name = "damian" + perms = "rwX" + + mock = MagicMock( + side_effect=[ + {name: {acl_type: [{acl_name: {"octal": 5}}]}}, + {name: {acl_type: [{acl_name: {"octal": 5}}]}}, + {name: {acl_type: [{acl_name: {"octal": 5}}]}}, + {name: {acl_type: [{}]}}, + {name: {acl_type: [{}]}}, + {name: {acl_type: [{}]}}, + { + name: {acl_type: [{acl_name: {"octal": 7}}]}, + name + "/foo": {acl_type: [{acl_name: {"octal": 5}}]}, + }, + { + name: {acl_type: [{acl_name: {"octal": 7}}]}, + name + "/foo": {acl_type: [{acl_name: {"octal": 7}}]}, + }, + {name: {acl_type: ""}}, + { + name: {"defaults": {"users": [{acl_name: {"octal": 7}}]}}, + name + "/foo": {"defaults": {"users": [{acl_name: {"octal": 7}}]}}, + }, + { + name: {"defaults": {"users": [{acl_name: {"octal": 7}}]}}, + name + "/foo": {"defaults": {"users": [{acl_name: {"octal": 7}}]}}, + }, + { + name: {"defaults": {"users": [{acl_name: {"octal": 7}}]}}, + name + "/foo": {"defaults": {"users": [{acl_name: {"octal": 7}}]}}, + }, + ] + ) + mock_modfacl = MagicMock(return_value=True) + + with patch.dict(linux_acl.__salt__, {"acl.getfacl": mock}): + # Update - test=True + with patch.dict(linux_acl.__opts__, {"test": True}): + comt = "Updated permissions will be applied for {}: r-x -> {}".format( + acl_name, perms + ) + ret = { + "name": name, + "comment": comt, + "changes": { + "new": { + "acl_name": acl_name, + "acl_type": acl_type, + "perms": perms, + }, + "old": { + "acl_name": acl_name, + "acl_type": acl_type, + "perms": "r-x", + }, + }, + "result": None, + } + + assert linux_acl.present(name, acl_type, acl_name, perms) == ret + # Update - test=False + with patch.dict(linux_acl.__salt__, {"acl.modfacl": mock_modfacl}): + with patch.dict(linux_acl.__opts__, {"test": False}): + comt = "Updated permissions for {}".format(acl_name) + ret = { + "name": name, + "comment": comt, + "changes": { + "new": { + "acl_name": acl_name, + "acl_type": acl_type, + "perms": perms, + }, + "old": { + "acl_name": acl_name, + "acl_type": acl_type, + "perms": "r-x", + }, + }, + "result": True, + } + assert linux_acl.present(name, acl_type, acl_name, perms) == ret + # Update - modfacl error + with patch.dict( + linux_acl.__salt__, + {"acl.modfacl": MagicMock(side_effect=CommandExecutionError("Custom err"))}, + ): + with patch.dict(linux_acl.__opts__, {"test": False}): + comt = "Error updating permissions for {}: Custom err".format(acl_name) + ret = { + "name": name, + "comment": comt, + "changes": {}, + "result": False, + } + assert linux_acl.present(name, acl_type, acl_name, perms) == ret + # New - test=True + with patch.dict(linux_acl.__salt__, {"acl.modfacl": mock_modfacl}): + with patch.dict(linux_acl.__opts__, {"test": True}): + comt = "New permissions will be applied for {}: {}".format( + acl_name, perms + ) + ret = { + "name": name, + "comment": comt, + "changes": { + "new": { + "acl_name": acl_name, + "acl_type": acl_type, + "perms": perms, + } + }, + "result": None, + } + assert linux_acl.present(name, acl_type, acl_name, perms) == ret + # New - test=False + with patch.dict(linux_acl.__salt__, {"acl.modfacl": mock_modfacl}): + with patch.dict(linux_acl.__opts__, {"test": False}): + comt = "Applied new permissions for {}".format(acl_name) + ret = { + "name": name, + "comment": comt, + "changes": { + "new": { + "acl_name": acl_name, + "acl_type": acl_type, + "perms": perms, + } + }, + "result": True, + } + assert linux_acl.present(name, acl_type, acl_name, perms) == ret + # New - modfacl error + with patch.dict( + linux_acl.__salt__, + {"acl.modfacl": MagicMock(side_effect=CommandExecutionError("Custom err"))}, + ): + with patch.dict(linux_acl.__opts__, {"test": False}): + comt = "Error updating permissions for {}: Custom err".format(acl_name) + ret = { + "name": name, + "comment": comt, + "changes": {}, + "result": False, + } + assert linux_acl.present(name, acl_type, acl_name, perms) == ret + + # New - recurse true + with patch.dict(linux_acl.__salt__, {"acl.getfacl": mock}): + # Update - test=True + with patch.dict(linux_acl.__opts__, {"test": True}): + comt = "Updated permissions will be applied for {}: rwx -> {}".format( + acl_name, perms + ) + ret = { + "name": name, + "comment": comt, + "changes": { + "new": { + "acl_name": acl_name, + "acl_type": acl_type, + "perms": perms, + }, + "old": { + "acl_name": acl_name, + "acl_type": acl_type, + "perms": "rwx", + }, + }, + "result": None, + } + + assert ( + linux_acl.present(name, acl_type, acl_name, perms, recurse=True) + == ret + ) + + # New - recurse true - nothing to do + with patch.dict(linux_acl.__salt__, {"acl.getfacl": mock}): + # Update - test=True + with patch.dict(linux_acl.__opts__, {"test": True}): + comt = "Permissions are in the desired state" + ret = {"name": name, "comment": comt, "changes": {}, "result": True} + + assert ( + linux_acl.present(name, acl_type, acl_name, perms, recurse=True) + == ret + ) + + # No acl type + comt = "ACL Type does not exist" + ret = {"name": name, "comment": comt, "result": False, "changes": {}} + assert linux_acl.present(name, acl_type, acl_name, perms) == ret + + # default recurse false - nothing to do + with patch.dict(linux_acl.__salt__, {"acl.getfacl": mock}): + # Update - test=True + with patch.dict(linux_acl.__opts__, {"test": True}): + comt = "Permissions are in the desired state" + ret = {"name": name, "comment": comt, "changes": {}, "result": True} + + assert ( + linux_acl.present( + name, "d:" + acl_type, acl_name, perms, recurse=False + ) + == ret + ) + + # default recurse false - nothing to do + with patch.dict(linux_acl.__salt__, {"acl.getfacl": mock}): + # Update - test=True + with patch.dict(linux_acl.__opts__, {"test": True}): + comt = "Permissions are in the desired state" + ret = {"name": name, "comment": comt, "changes": {}, "result": True} + + assert ( + linux_acl.present( + name, "d:" + acl_type, acl_name, perms, recurse=False + ) + == ret + ) + + # default recurse true - nothing to do + with patch.dict(linux_acl.__salt__, {"acl.getfacl": mock}): + # Update - test=True + with patch.dict(linux_acl.__opts__, {"test": True}): + comt = "Permissions are in the desired state" + ret = {"name": name, "comment": comt, "changes": {}, "result": True} + + assert ( + linux_acl.present( + name, "d:" + acl_type, acl_name, perms, recurse=True + ) + == ret + ) + def test_absent(): """ From a4db532ff75d52c40ec9f057920f9c96f18cc2b1 Mon Sep 17 00:00:00 2001 From: Leif Liddy Date: Sat, 30 Sep 2023 05:55:19 +0200 Subject: [PATCH 2/5] updated syntax according to lint reqs --- salt/states/linux_acl.py | 12 ++++++------ tests/pytests/unit/states/test_linux_acl.py | 18 +++++++++--------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/salt/states/linux_acl.py b/salt/states/linux_acl.py index 72cc6151c4e1..14875994fa60 100644 --- a/salt/states/linux_acl.py +++ b/salt/states/linux_acl.py @@ -135,7 +135,7 @@ def present(name, acl_type, acl_name="", perms="", recurse=False, force=False): _octal_lookup = {4: "r", 2: "w", 1: "x", 0: "-"} if not os.path.exists(name): - ret["comment"] = "{} does not exist".format(name) + ret["comment"] = f"{name} does not exist" ret["result"] = False return ret @@ -255,7 +255,7 @@ def present(name, acl_type, acl_name="", perms="", recurse=False, force=False): ) ret.update( { - "comment": "Updated permissions for {}".format(acl_name), + "comment": f"Updated permissions for {acl_name}", "result": True, "changes": changes, } @@ -296,7 +296,7 @@ def present(name, acl_type, acl_name="", perms="", recurse=False, force=False): ) ret.update( { - "comment": "Applied new permissions for {}".format(acl_name), + "comment": f"Applied new permissions for {acl_name}", "result": True, "changes": changes, } @@ -340,7 +340,7 @@ def absent(name, acl_type, acl_name="", perms="", recurse=False): ret = {"name": name, "result": True, "changes": {}, "comment": ""} if not os.path.exists(name): - ret["comment"] = "{} does not exist".format(name) + ret["comment"] = f"{name} does not exist" ret["result"] = False return ret @@ -438,7 +438,7 @@ def list_present(name, acl_type, acl_names=None, perms="", recurse=False, force= _octal = {"r": 4, "w": 2, "x": 1, "-": 0} _octal_perms = sum(_octal.get(i, i) for i in perms) if not os.path.exists(name): - ret["comment"] = "{} does not exist".format(name) + ret["comment"] = f"{name} does not exist" ret["result"] = False return ret @@ -727,7 +727,7 @@ def list_absent(name, acl_type, acl_names=None, recurse=False): ret = {"name": name, "result": True, "changes": {}, "comment": ""} if not os.path.exists(name): - ret["comment"] = "{} does not exist".format(name) + ret["comment"] = f"{name} does not exist" ret["result"] = False return ret diff --git a/tests/pytests/unit/states/test_linux_acl.py b/tests/pytests/unit/states/test_linux_acl.py index 0119e6385f3c..dad2cab6d6f2 100644 --- a/tests/pytests/unit/states/test_linux_acl.py +++ b/tests/pytests/unit/states/test_linux_acl.py @@ -93,7 +93,7 @@ def test_present(): # Update - test=False with patch.dict(linux_acl.__salt__, {"acl.modfacl": mock_modfacl}): with patch.dict(linux_acl.__opts__, {"test": False}): - comt = "Updated permissions for {}".format(acl_name) + comt = f"Updated permissions for {acl_name}" ret = { "name": name, "comment": comt, @@ -118,7 +118,7 @@ def test_present(): {"acl.modfacl": MagicMock(side_effect=CommandExecutionError("Custom err"))}, ): with patch.dict(linux_acl.__opts__, {"test": False}): - comt = "Error updating permissions for {}: Custom err".format(acl_name) + comt = f"Error updating permissions for {acl_name}: Custom err" ret = { "name": name, "comment": comt, @@ -148,7 +148,7 @@ def test_present(): # New - test=False with patch.dict(linux_acl.__salt__, {"acl.modfacl": mock_modfacl}): with patch.dict(linux_acl.__opts__, {"test": False}): - comt = "Applied new permissions for {}".format(acl_name) + comt = f"Applied new permissions for {acl_name}" ret = { "name": name, "comment": comt, @@ -168,7 +168,7 @@ def test_present(): {"acl.modfacl": MagicMock(side_effect=CommandExecutionError("Custom err"))}, ): with patch.dict(linux_acl.__opts__, {"test": False}): - comt = "Error updating permissions for {}: Custom err".format(acl_name) + comt = f"Error updating permissions for {acl_name}: Custom err" ret = { "name": name, "comment": comt, @@ -362,7 +362,7 @@ def test_present_conditional_x(): {"acl.modfacl": MagicMock(side_effect=CommandExecutionError("Custom err"))}, ): with patch.dict(linux_acl.__opts__, {"test": False}): - comt = "Error updating permissions for {}: Custom err".format(acl_name) + comt = f"Error updating permissions for {acl_name}: Custom err" ret = { "name": name, "comment": comt, @@ -392,7 +392,7 @@ def test_present_conditional_x(): # New - test=False with patch.dict(linux_acl.__salt__, {"acl.modfacl": mock_modfacl}): with patch.dict(linux_acl.__opts__, {"test": False}): - comt = "Applied new permissions for {}".format(acl_name) + comt = f"Applied new permissions for {acl_name}" ret = { "name": name, "comment": comt, @@ -412,7 +412,7 @@ def test_present_conditional_x(): {"acl.modfacl": MagicMock(side_effect=CommandExecutionError("Custom err"))}, ): with patch.dict(linux_acl.__opts__, {"test": False}): - comt = "Error updating permissions for {}: Custom err".format(acl_name) + comt = f"Error updating permissions for {acl_name}: Custom err" ret = { "name": name, "comment": comt, @@ -639,7 +639,7 @@ def test_list_present(): {"acl.modfacl": MagicMock(side_effect=CommandExecutionError("Custom err"))}, ): with patch.dict(linux_acl.__opts__, {"test": False}): - comt = "Error updating permissions for {}: Custom err".format(acl_names) + comt = f"Error updating permissions for {acl_names}: Custom err" expected = { "name": name, "comment": comt, @@ -697,7 +697,7 @@ def test_list_present(): {"acl.modfacl": MagicMock(side_effect=CommandExecutionError("Custom err"))}, ): with patch.dict(linux_acl.__opts__, {"test": False}): - comt = "Error updating permissions for {}: Custom err".format(acl_names) + comt = f"Error updating permissions for {acl_names}: Custom err" expected = { "name": name, "comment": comt, From 94e1b30fcfc68a65a7ccf971f2e0e223c4cd6d2f Mon Sep 17 00:00:00 2001 From: Leif Liddy Date: Sat, 30 Sep 2023 06:01:08 +0200 Subject: [PATCH 3/5] corrected typo --- tests/pytests/unit/states/test_linux_acl.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/pytests/unit/states/test_linux_acl.py b/tests/pytests/unit/states/test_linux_acl.py index dad2cab6d6f2..85fa120a3968 100644 --- a/tests/pytests/unit/states/test_linux_acl.py +++ b/tests/pytests/unit/states/test_linux_acl.py @@ -337,7 +337,7 @@ def test_present_conditional_x(): # Update - test=False with patch.dict(linux_acl.__salt__, {"acl.modfacl": mock_modfacl}): with patch.dict(linux_acl.__opts__, {"test": False}): - comt = "Updated permissions for {}".format(acl_name) + comt = f"Updated permissions for {acl_name}" ret = { "name": name, "comment": comt, From 7bfe49c2ad1f28b62194020c890ab4f514ffa760 Mon Sep 17 00:00:00 2001 From: Leif Liddy Date: Sat, 30 Sep 2023 06:31:53 +0200 Subject: [PATCH 4/5] added blank line --- tests/pytests/unit/states/test_linux_acl.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/pytests/unit/states/test_linux_acl.py b/tests/pytests/unit/states/test_linux_acl.py index 85fa120a3968..59fa2f399311 100644 --- a/tests/pytests/unit/states/test_linux_acl.py +++ b/tests/pytests/unit/states/test_linux_acl.py @@ -266,6 +266,7 @@ def test_present(): == ret ) + def test_present_conditional_x(): """ Test to ensure a Linux ACL containing a conditional X is applied correctly From a5347fc72b11595d309a64e8bb47fec77534ab0a Mon Sep 17 00:00:00 2001 From: Leif Liddy Date: Mon, 11 Dec 2023 05:46:09 +0100 Subject: [PATCH 5/5] add changelog file --- changelog/62852.added.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/62852.added.md diff --git a/changelog/62852.added.md b/changelog/62852.added.md new file mode 100644 index 000000000000..cf8dc7a4e155 --- /dev/null +++ b/changelog/62852.added.md @@ -0,0 +1 @@ +added conditional X functionality to linux_acl