-
Notifications
You must be signed in to change notification settings - Fork 12
add replace_acl method to PosixOperations #99
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 2 commits
5b32dce
2d6cda0
287ed2d
ff1113c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| self.log.debug(f"Input file for {setfacl_exe} created in: {acl_file.name}") | ||
| acl_file.write('\n'.join(permissions).encode('utf-8')) | ||
lexming marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| 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 | ||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.logandloggingin 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.