From 5b32dceff43a20170d4a0647b77eaca86837ab11 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Thu, 11 Sep 2025 19:38:09 +0200 Subject: [PATCH 1/4] add replace_acl method to PosixOperations --- lib/vsc/filesystem/posix.py | 64 +++++++++++++++++++++++++++++++++++++ setup.py | 2 +- test/posix.py | 39 ++++++++++++++++++++++ 3 files changed, 104 insertions(+), 1 deletion(-) diff --git a/lib/vsc/filesystem/posix.py b/lib/vsc/filesystem/posix.py index aa8547c..88686f6 100644 --- a/lib/vsc/filesystem/posix.py +++ b/lib/vsc/filesystem/posix.py @@ -22,6 +22,7 @@ import errno import os import stat +import tempfile from vsc.utils import fancylogger from vsc.utils.patterns import Singleton @@ -558,3 +559,66 @@ def create_stat_directory(self, path, permissions, uid, gid, override_permission self.log.debug("Path %s already exists with correct ownership", path) return created + + def replace_acl(self, path, permissions): + """ + Overwrite ACL permissions on given path + """ + path = self._sanity_check(path) + fs = self._what_filesystem(path) + if fs is None: + self.log.warning(f"ACL replacement requested on filesystem with unknown support: {path}") + return False + + if not isinstance(permissions, (tuple, list,)): + raise PosixOperationError("Given ACL permissions are not a list or tuple") + + replace_acl_method = self._replace_acl_posix + if fs[0] in ['gpfs', 'nfs4']: + replace_acl_method = self._replace_acl_nfs + + return replace_acl_method(path, permissions) + + def _replace_acl_posix(self, path, permissions): + """ + Overwrite ACL permissions in POSIX format on given path + """ + setfacl_exe = 'setfacl' + + with tempfile.NamedTemporaryFile() as acl_file: + self.log.debug(f"Input file for {setfacl_exe} created in: {acl_file.name}") + acl_file.write('\n'.join(permissions).encode('utf-8')) + + setfacl_cmd = [ + setfacl_exe, + f'--set-file={acl_file.name}', + path + ] + + try: + ec, out = self._execute(setfacl_cmd) + except FileNotFoundError as err: + raise PosixOperationError(f"Cannot set POSIX ACLs, command {setfacl_exe} not found") from err + else: + return ec, out + + def _replace_acl_nfs(self, path, permissions): + """ + Overwrite ACL permissions in NFSv4 format on given path + """ + acl_entries = ','.join(permissions) + + setfacl_exe = 'nfs4_setfacl' + setfacl_cmd = [ + 'nfs4_setfacl', + '-s', + f'"{acl_entries}"', + path + ] + + try: + ec, _ = self._execute(setfacl_cmd) + except FileNotFoundError as err: + raise PosixOperationError(f"Cannot set NFSv4 ACLs, command {setfacl_exe} not found") from err + else: + return ec diff --git a/setup.py b/setup.py index d1b4db6..2495c1d 100755 --- a/setup.py +++ b/setup.py @@ -37,7 +37,7 @@ PACKAGE = { - "version": "2.3.2", + "version": "2.4.0", "author": [sdw, ag, kh, kw], "maintainer": [sdw, ag, kh, kw, wdp], "setup_requires": ["vsc-install >= 0.15.2"], diff --git a/test/posix.py b/test/posix.py index 2a23b2f..037d6d6 100644 --- a/test/posix.py +++ b/test/posix.py @@ -240,3 +240,42 @@ def test_create_stat_dir_existing_not_dir(self, mock_chmod, mock_chown, mock_mak mock_os_stat.assert_called_with(test_path) mock_makedirs.assert_not_called + + @mock.patch('vsc.filesystem.posix.PosixOperations._execute') + @mock.patch('vsc.filesystem.posix.PosixOperations._what_filesystem') + @mock.patch('vsc.filesystem.posix.tempfile.NamedTemporaryFile') + def test_replace_acl(self, mock_tempfile, mock_what_filesystem, mock_execute): + """ + Test replacement of ACLs + """ + mock_aclfile = mock.MagicMock() + mock_aclfile.name = "/tmp/acl_input_file" + mock_context = mock.MagicMock() + mock_context.__enter__.return_value = mock_aclfile + mock_tempfile.return_value = mock_context + mock_execute.return_value = (0, "") + test_path = '/tmp/test' + + # POSIX ACLs + mock_what_filesystem.return_value = ['posix', '/data', 0, '127.0.0.1@tcp'] + + test_acl_posix = "user::rwx" + self.assertRaises(PosixOperationError, self.po.replace_acl, test_path, test_acl_posix) + + test_acl_posix = ["user::rwx", "group::r-x", "other::r-x"] + self.po.replace_acl(test_path, test_acl_posix) + mock_execute.assert_called_with( + ['setfacl', '--set-file=/tmp/acl_input_file', '/tmp/test'] + ) + + # NFSv4 ACLs + mock_what_filesystem.return_value = ['nfs4', '/data', 0, '127.0.0.1@tcp'] + + test_acl_posix = "A:d:OWNER@:rwaDdxtTnNcoy" + self.assertRaises(PosixOperationError, self.po.replace_acl, test_path, test_acl_posix) + + test_acl_posix = ["A:d:OWNER@:rwaDdxtTnNcoy", "A:dg:GROUP@:rxtncy", "A:fd:EVERYONE@:tncy"] + self.po.replace_acl(test_path, test_acl_posix) + mock_execute.assert_called_with( + ['nfs4_setfacl', '-s', '"A:d:OWNER@:rwaDdxtTnNcoy,A:dg:GROUP@:rxtncy,A:fd:EVERYONE@:tncy"', '/tmp/test'] + ) From 2d6cda0659c1460a666d73f07514bc1a02ca0685 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Thu, 18 Sep 2025 10:56:20 +0200 Subject: [PATCH 2/4] set --allow-permission-change=chmodAndUpdateAcl on new filesets in GPFS --- lib/vsc/filesystem/gpfs.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/vsc/filesystem/gpfs.py b/lib/vsc/filesystem/gpfs.py index 67db120..a21f08a 100644 --- a/lib/vsc/filesystem/gpfs.py +++ b/lib/vsc/filesystem/gpfs.py @@ -816,6 +816,11 @@ def make_fileset(self, new_fileset_path, fileset_name=None, parent_fileset_name= GpfsOperationError) mmcrfileset_options += ['--inode-space', parent_fileset_name] + # Enable update of ACL permissions on chmod commands in all cases + # default setting is chmodAndSetAcl, which makes chmod update POSIX ACLs, but fully replace NFS4 ACLs + # by swithcing to chmodAndUpdateAcl, chmod behaves in the same way (updates) both ACL types + mmcrfileset_options += ['--allow-permission-change', 'chmodAndUpdateAcl'] + (ec, out) = self._execute('mmcrfileset', mmcrfileset_options, True) if ec > 0: self.log.raiseException( From 287ed2da9307257481a803f004cec42ad4e52ce3 Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Thu, 18 Sep 2025 11:49:48 +0200 Subject: [PATCH 3/4] add debug logs with the contents of the new ACL entries being set --- lib/vsc/filesystem/posix.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/vsc/filesystem/posix.py b/lib/vsc/filesystem/posix.py index 88686f6..5313798 100644 --- a/lib/vsc/filesystem/posix.py +++ b/lib/vsc/filesystem/posix.py @@ -587,7 +587,9 @@ def _replace_acl_posix(self, path, permissions): with tempfile.NamedTemporaryFile() as acl_file: self.log.debug(f"Input file for {setfacl_exe} created in: {acl_file.name}") - acl_file.write('\n'.join(permissions).encode('utf-8')) + acl_entries = '\n'.join(permissions) + acl_file.write(acl_entries.encode('utf-8')) + self.log.debug(f"Setting POSIX ACLs on '{path}' to:\n{acl_entries}") setfacl_cmd = [ setfacl_exe, @@ -607,6 +609,7 @@ def _replace_acl_nfs(self, path, permissions): Overwrite ACL permissions in NFSv4 format on given path """ acl_entries = ','.join(permissions) + self.log.debug(f"Setting NFSv4 ACLs on '{path}' to: {acl_entries}") setfacl_exe = 'nfs4_setfacl' setfacl_cmd = [ From ff1113c157096f7862eadc7bca72387f1ad0c2db Mon Sep 17 00:00:00 2001 From: Alex Domingo Date: Mon, 29 Sep 2025 12:32:24 +0200 Subject: [PATCH 4/4] set allow-permission-inherit to inheritAclAndAddMode in new filesets --- lib/vsc/filesystem/gpfs.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/vsc/filesystem/gpfs.py b/lib/vsc/filesystem/gpfs.py index a21f08a..4118940 100644 --- a/lib/vsc/filesystem/gpfs.py +++ b/lib/vsc/filesystem/gpfs.py @@ -821,6 +821,11 @@ def make_fileset(self, new_fileset_path, fileset_name=None, parent_fileset_name= # by swithcing to chmodAndUpdateAcl, chmod behaves in the same way (updates) both ACL types mmcrfileset_options += ['--allow-permission-change', 'chmodAndUpdateAcl'] + # Remove special permissions (i.e. owner, group, everyone) from inheritance rules in ACLs + # this allows to control special permissions as usual with mod bits and umask + # only named ACLs will be inherited + mmcrfileset_options += ['--allow-permission-inherit', 'inheritAclAndAddMode'] + (ec, out) = self._execute('mmcrfileset', mmcrfileset_options, True) if ec > 0: self.log.raiseException(