Skip to content

Conversation

@lexming
Copy link
Contributor

@lexming lexming commented Sep 11, 2025

In VUB we are going to start controlling access to our VOs with ACLs. To this end we need a method in vsc-filesystems that allows to set those ACLs.

PosixOperations.replace_acl will replace the ACLs of an object with the given ACL entries. It uses NFSv4 formatting on nfs4 and gpfs mounts and POSIX formatting anywhere else.

This is needed by hpcugent/vsc-administration#159

Moreover, on GPFS we need the following changes to default settings to make ACL behaviour on filesets be usable:

  • --allow-permission-change=chmodAndUpdateAcl: otherwise chmod commands reset the NFSv4 ACLs. This change only impacts files/folders with NFS4 ACLs, behaviour without ACLs or with POSIX ACLs is unaffected.
  • --allow-permission-inherit=inheritAclAndAddMode: keep special permissions (owner, group, everyone) out of the ACL inheritance to continue manage those with the usual mod bits and umask.

"""
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.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants