Get rid of use_nfsv4_alcs option in ansible.posix.acl

[dolfinus]

SUMMARY

There is an option use_nfsv4_acls: true which allows to use NFSv4 ACL rules instead of POSIX ones. It does not work at all.
It was already mentioned here ansible/ansible#58679, but it has been closed after ansible.posix was moved to a separated repo.

ISSUE TYPE

• Bug Report

COMPONENT NAME

ansible.posix.acl

ANSIBLE VERSION

ansible 2.9.6
  config file = None
  configured module search path = [u'/root/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.7.5 (default, Mar 12 2021, 14:55:44) [GCC 4.8.5 20150623 (Red Hat 4.8.5-44.0.3)]

But the same thing with ansible 2.12 on python 3.7.

COLLECTION VERSION

Installing 'ansible.posix:1.2.0' to '/root/.ansible/collections/ansible_collections/ansible/posix'

CONFIGURATION

ANSIBLE_FORCE_COLOR(env: ANSIBLE_FORCE_COLOR) = True
DEPRECATION_WARNINGS(env: ANSIBLE_DEPRECATION_WARNINGS) = False

OS / ENVIRONMENT

Oracle Linux Server 7.8
Linux 4.14.35-1902.300.11.el7uek.x86_64

STEPS TO REPRODUCE

- name: Create shared folder
  file:
    path: /shared_folder
    state: directory
    owner: root
    group: root
    mode: 0770
- name: Allow user access to his subfolder 
  ansible.posix.acl:
    path: /shared_folder/{{ item }}
    state: present
    entity: '{{ item }}'
    etype: user
    permissions: rwx
    default: true
    recursive: true
    use_nfsv4_acls: true
    with_items:
    - me

EXPECTED RESULTS

Users cannot see content of /shared_folder, but each user could access to /shared_folder/%username%

ACTUAL RESULTS

The error is raised:

setfacl: Option -m: Invalid argument near character 18

The command which is actually being executed is:

setfacl --test -d -m user:me:rwx:allow --recursive --no-mask /shared_folder/me

This is caused by this line:

ansible.posix/plugins/modules/acl.py

Lines 180 to 181 in b3e395a

	if use_nfsv4_acls:
	return ':'.join([etype, entity, permissions, 'allow'])

:allow suffix is added to a command, but setfacl accepts only 3 items delimited by :, not 4: user:me:rwx

NFSv4 ACL does allow 4 items in the rule, but module should use nfs4_setfacl/nfs4_getfacl instead of pure setfacl/getfacl.

But it's not enough to add another if block in the module which will select command name because these 2 set of commands accepts completely different options:

>>> setfacl -h
setfacl 2.2.51 -- set file access control lists
Usage: setfacl [-bkndRLP] { -m|-M|-x|-X ... } file ...
  -m, --modify=acl        modify the current ACL(s) of file(s)
  -M, --modify-file=file  read ACL entries to modify from file
  -x, --remove=acl        remove entries from the ACL(s) of file(s)
  -X, --remove-file=file  read ACL entries to remove from file
  -b, --remove-all        remove all extended ACL entries
  -k, --remove-default    remove the default ACL
      --set=acl           set the ACL of file(s), replacing the current ACL
      --set-file=file     read ACL entries to set from file
      --mask              do recalculate the effective rights mask
  -n, --no-mask           don't recalculate the effective rights mask
  -d, --default           operations apply to the default ACL
  -R, --recursive         recurse into subdirectories
  -L, --logical           logical walk, follow symbolic links
  -P, --physical          physical walk, do not follow symbolic links
      --restore=file      restore ACLs (inverse of `getfacl -R')
      --test              test mode (ACLs are not modified)
  -v, --version           print version and exit
  -h, --help              this help text

nfs4_getfacl -h
nfs4_getfacl 0.3.3 -- get NFSv4 file or directory access control lists.
Usage: nfs4_getfacl file
  -H, --more-help       display ACL format information
  -?, -h, --help        display this help text
  -R --recursive        recurse into subdirectories

nfs4_setfacl does not allow to pass neither of -m, --test --no-mask options at all.

More than that, they are using completely different set of permissions:

nfs4_getfacl 0.3.3 -- get NFSv4 file or directory access control lists.

An NFSv4 ACL consists of one or more NFSv4 ACEs, each delimited by commas or whitespace.
An NFSv4 ACE is written as a colon-delimited, 4-field string in the following format:

    <type>:<flags>:<principal>:<permissions>

    * <type> - one of:
        'A'  allow
        'D'  deny
        'U'  audit
        'L'  alarm

    * <flags> - zero or more (depending on <type>) of:
        'f'  file-inherit
        'd'  directory-inherit
        'p'  no-propagate-inherit
        'i'  inherit-only
        'S'  successful-access
        'F'  failed-access
        'g'  group (denotes that <principal> is a group)

    * <principal> - named user or group, or one of: "OWNER@", "GROUP@", "EVERYONE@"

    * <permissions> - one or more of:
        'r'  read-data / list-directory
        'w'  write-data / create-file
        'a'  append-data / create-subdirectory
        'x'  execute
        'd'  delete
        'D'  delete-child (directories only)
        't'  read-attrs
        'T'  write-attrs
        'n'  read-named-attrs
        'N'  write-named-attrs
        'c'  read-ACL
        'C'  write-ACL
        'o'  write-owner
        'y'  synchronize

And according to this help message allow is not the correct one.

So my proposal is to completely remove use_nfsv4_acls because it cannot be used at all.

[dolfinus]

Or there should be completely new module like ansible.posix.nfs4_acl with another set of options, rules and commands being executed.

[saito-hideki]

Thank you for reporting this issue! It seems that this issue wasn't taken over from core to collections for some reason.
As you pointed out, this issue is also confirmed in the current collections.

[mchouque]

I too would love some nfsv4 love. So what's the desired outcome: two different acl modules (acl and nfs4_acl for example) or do we want to keep everything under the same umbrella?

I mean they're sufficiently different enough it'd make sense to have them separated.

[sergesal]

Also having same issue with this module. For our project we use it for both local and nfs file systems. Created this quick patch for now to workaround this nfsv4 issue. We mostly use it for adding group permissions so it all works now. But definitely this module needs to be fixed for the NFS.

--- /usr/share/ansible/collections/ansible_collections/ansible/posix/plugins/modules/acl.py     2021-12-21 02:30:31.973993959 +0000
+++ acl.py      2021-12-19 01:38:40.914818707 +0000
@@ -178,7 +178,7 @@
 def build_entry(etype, entity, permissions=None, use_nfsv4_acls=False):
     '''Builds and returns an entry string. Does not include the permissions bit if they are not provided.'''
     if use_nfsv4_acls:
-        return ':'.join([etype, entity, permissions, 'allow'])
+        return ':'.join(['A', 'g' if etype == 'group' else '', entity, permissions + 'tcy'])
 
     if permissions:
         return etype + ':' + entity + ':' + permissions
@@ -186,13 +186,13 @@
     return etype + ':' + entity
 
 
-def build_command(module, mode, path, follow, default, recursive, recalculate_mask, entry=''):
+def build_command(module, mode, path, follow, default, recursive, recalculate_mask, entry='', use_nfsv4_acls=False):
     '''Builds and returns a getfacl/setfacl command.'''
     if mode == 'set':
-        cmd = [module.get_bin_path('setfacl', True)]
-        cmd.extend(['-m', entry])
+        cmd = [module.get_bin_path('nfs4_setfacl' if use_nfsv4_acls else 'setfacl', True)]
+        cmd.extend(['-a' if use_nfsv4_acls else '-m', entry])
     elif mode == 'rm':
-        cmd = [module.get_bin_path('setfacl', True)]
+        cmd = [module.get_bin_path('nfs4_setfacl' if use_nfsv4_acls else 'setfacl', True)]
         cmd.extend(['-x', entry])
     else:  # mode == 'get'
         cmd = [module.get_bin_path('getfacl', True)]
@@ -222,7 +222,7 @@
     return cmd
 
 
-def acl_changed(module, cmd):
+def acl_changed(module, cmd, entity, use_nfsv4_acls=False):
     '''Returns true if the provided command affects the existing ACLs, false otherwise.'''
     # FreeBSD do not have a --test flag, so by default, it is safer to always say "true"
     if platform.system().lower() == 'freebsd':
@@ -232,11 +232,18 @@
     cmd.insert(1, '--test')
     lines = run_acl(module, cmd)
 
+    counter = 0
     for line in lines:
-        if not line.endswith('*,*'):
-            return True
-    return False
+        if line.endswith('*,*') and not use_nfsv4_acls:
+            return False
+        if use_nfsv4_acls and entity in line.split(':'):
+            counter += 1
+
+    # Works for addition
+    if counter == 2:
+        return False
 
+    return True
 
 def run_acl(module, cmd, check_rc=True):
 
@@ -312,7 +319,7 @@
             module.fail_json(msg="'recalculate_mask' MUST NOT be set to 'mask' or 'no_mask' when 'state=query'.")
 
     if not entry:
-        if state == 'absent' and permissions:
+        if state == 'absent' and permissions and not use_nfsv4_acls:
             module.fail_json(msg="'permissions' MUST NOT be set when 'state=absent'.")
 
         if state == 'absent' and not entity:
@@ -349,21 +356,24 @@
         entry = build_entry(etype, entity, permissions, use_nfsv4_acls)
         command = build_command(
             module, 'set', path, follow,
-            default, recursive, recalculate_mask, entry
+            default, recursive, recalculate_mask, entry, use_nfsv4_acls
         )
-        changed = acl_changed(module, command)
+        changed = acl_changed(module, command, entity, use_nfsv4_acls)
 
         if changed and not module.check_mode:
             run_acl(module, command)
         msg = "%s is present" % entry
 
     elif state == 'absent':
-        entry = build_entry(etype, entity, use_nfsv4_acls)
+        if use_nfsv4_acls:
+            entry = build_entry(etype, entity, permissions, use_nfsv4_acls)
+        else:
+            entry = build_entry(etype, entity, use_nfsv4_acls)
         command = build_command(
             module, 'rm', path, follow,
-            default, recursive, recalculate_mask, entry
+            default, recursive, recalculate_mask, entry, use_nfsv4_acls
         )
-        changed = acl_changed(module, command)
+        changed = acl_changed(module, command, entity, use_nfsv4_acls)
 
         if changed and not module.check_mode:
             run_acl(module, command, False)