zfs_delegate_admin.py: def current_perms: unable to parse unknown uid and gid

[papamoose]

Summary

There are few paths to take and it isn't up to me to decide for everyone. Feedback is appreciated before I submit a pull request.

Problem

When setting allow permissions using community.general.zfs_delegate_admin for particular users/groups there will be circumstances when a user/group in the output of zfs allow <dataset> is not known to the host system.

In that case the output of zfs allow <pool/dataset>
looks similar to this:

  $ sudo zfs allow tank/test
  ---- Permissions on tank/test ---------------------------------------
  Local+Descendent permissions: 
        user (unknown: 1002) hold
        user zfsuser receive 

Then when using community.general.zfs_delegate_admin the module fails for all delegated permissions just because it cannot parse user (unknown: 1002) hold.

Ansible output

failed: [hostname] (item=tank/test) => {"ansible_loop_var": "item", "changed": false, "item": "tank/vault", "msg": "Cannot parse user/group permission output by `zfs allow`: '\tuser (unknown: 1002) hold'"}

Replicate

Give a user zfs allow permissions and then delete that user.

1. useradd test
2. zfs allow -u test hold
3. userdel test

Without modifying zfs_delegate_admin.py the solution is to either:
a) make a user/group with a uid/gid that matches the one above on the host system
b) Remove uid (in this case) from the allow list: zfs unallow -u 1002 tank/test

I suggest that this module does something helpful to allow permissions to be set that can be set or provide a more helpful error message.

Solutions

There are few paths to take and it isn't up to me to decide for everyone. Feedback is appreciated before I submit a pull request.

Provide helpful error message:

It isn't this modules job to manage unknowns and should be considered an error. Tell the admin and fail.

--- zfs_delegate_admin.py	2023-02-05 00:16:05.017475032 -0700
+++ zfs_delegate_admin.py_helpful-error	2023-02-05 00:29:33.921425525 -0700
@@ -184,7 +184,10 @@
                 elif line.startswith('\teveryone '):
                     perms[scope]['e'] = line.split()[1].split(',')
             except ValueError:
-                self.module.fail_json(msg="Cannot parse user/group permission output by `zfs allow`: '%s'" % line)
+                if ' (unknown: ' in line:
+                    self.module.fail_json(msg="Unknown user/group found in zfs allow permission. Resolve the unknown user/group to allow this module to proceed: '%s'" % line)
+                else:
+                    self.module.fail_json(msg="Cannot parse user/group permission output by `zfs allow`: '%s'" % line)
         return perms
 
     def run_zfs_raw(self, subcommand=None, args=None):

Ignore unknowns

It isn't this modules job to manage unknown users or groups. Silently ignore them.

It might be nice to insert a warning instead of being silent. Let me know what function I should use to do this.

--- zfs_delegate_admin.py	2023-02-05 00:16:05.017475032 -0700
+++ zfs_delegate_admin.py_ignore	2023-02-05 00:22:07.026313007 -0700
@@ -178,11 +178,12 @@
             if not scope:
                 continue
             try:
-                if line.startswith('\tuser ') or line.startswith('\tgroup '):
-                    ent_type, ent, cur_perms = line.split()
-                    perms[scope][ent_type[0]][ent] = cur_perms.split(',')
-                elif line.startswith('\teveryone '):
-                    perms[scope]['e'] = line.split()[1].split(',')
+                if not ' (unknown: ' in line:
+                    if line.startswith('\tuser ') or line.startswith('\tgroup '):
+                        ent_type, ent, cur_perms = line.split()
+                        perms[scope][ent_type[0]][ent] = cur_perms.split(',')
+                    elif line.startswith('\teveryone '):
+                        perms[scope]['e'] = line.split()[1].split(',')
             except ValueError:
                 self.module.fail_json(msg="Cannot parse user/group permission output by `zfs allow`: '%s'" % line)
         return perms

remove ' (unknown: ' + ')' from the user field

This will leave just the uid/gid number and will allow the parser to proceed normally. This is almost like ignoring but I'm uncertain as to what happens later in the code. It seems like one could then fully manage all permissions and if a boolean was created and set remove unmanaged perms, which would then possibly include the ones without valid users/groups.

--- zfs_delegate_admin.py	2023-02-05 00:16:05.017475032 -0700
+++ zfs_delegate_admin.py_str-replace	2023-02-05 00:26:03.980186943 -0700
@@ -177,6 +177,9 @@
             scope = linemap.get(line, scope)
             if not scope:
                 continue
+            if ' (unknown: ' in line:
+                line = line.replace('(unknown: ','',1)
+                line = line.replace(')','',1)
             try:
                 if line.startswith('\tuser ') or line.startswith('\tgroup '):
                     ent_type, ent, cur_perms = line.split()

Issue Type

Bug Report

Component Name

community.general.zfs_delegate_admin

Ansible Version

$ ansible --version

ansible [core 2.14.0]
  config file = /home/username/nextcloud/ansible/cloud/ansible.cfg
  configured module search path = ['/home/username/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/username/.local/lib/python3.10/site-packages/ansible
  ansible collection location = /home/username/.ansible/collections:/usr/share/ansible/collections
  executable location = /home/username/.local/bin/ansible
  python version = 3.10.6 (main, Nov 14 2022, 16:10:14) [GCC 11.3.0] (/usr/bin/python3)
  jinja version = 3.1.2
  libyaml = True

Community.general Version

community.general 6.3.0

Configuration

.

OS / Environment

.

Steps to Reproduce

.

Expected Results

.

Actual Results

.

Code of Conduct

• I agree to follow the Ansible Code of Conduct

[ansibullbot]

Files identified in the description:

• plugins/modules/zfs_delegate_admin.py

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

[ansibullbot]

cc @bcoca @fishman @jasperla @jpdasma @mator @natefoo @scathatheworm @troy2914 @xen0l
click here for bot help

[felixfontein]

CC @lotheac @sriccio @felibb since you were interested in this module in the past.

[lotheac]

I think the zfs allow command is itself a bit problematic here, in that it unconditionally prints the user/group name instead of the uid/gid (unless getgpwuid/getgrgid failed, and the name isn't known) (see links below). For config management tools it'd be better to manage the permissions based on uids/gids, but since zfs allow doesn't have such an output flag, that doesn't seem possible in the context of ansible.

Practically, your suggestion of removing the "(unknown:)" seems like it could work. A better solution would be if someone added a command line flag for zfs allow/unallow to always print numeric id's (or more generally machine-parsable output).

https://github.com/openzfs/zfs/blob/6017fd9377b217481097dda1206132ec81fcc8ef/cmd/zfs/zfs_main.c#L5444-L5469
https://github.com/openzfs/zfs/blob/6017fd9377b217481097dda1206132ec81fcc8ef/cmd/zfs/zfs_main.c#L6142-L6149

[papamoose]

@lotheac I agree 100%. Until zfs allow has better output I've suggested pull request #5943.