Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 64 additions & 0 deletions lib/vsc/filesystem/posix.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import errno
import os
import stat
import tempfile

from vsc.utils import fancylogger
from vsc.utils.patterns import Singleton
Expand Down Expand Up @@ -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}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're going to purge all this at some point and use logging.xyz. Not sure if you want to change this already then.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's confusing to mix self.log and logging in the same code. So I won't make that change just here. When the switch is made to logging we can do that everywhere at once.

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:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the approach with a tmpfile? Can't it be give as arguments to setfacl ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's extremely painful to do it through arguments on the command line, there is no option to pass all the ACLs as an inline list of sorts.

To replace all ACLs, you need to first wipe the ACLs with -b, then add each new rule individually, and at the same time detect which one is default or not, because default ones must me passed with -d.

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
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
Expand Down
39 changes: 39 additions & 0 deletions test/posix.py
Original file line number Diff line number Diff line change
Expand Up @@ -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']
)