community.general icon indicating copy to clipboard operation
community.general copied to clipboard

zfs: fix multi-line value in user-defined property

Open batrla opened this issue 1 year ago • 33 comments

SUMMARY

Fixes ZFS module which breaks if ZFS dataset property contains multi-line value. The root cause is a bug in zfs.py that causes parsing error of 'zfs get' output. The ZFS module uses 'zfs get -H -p -o property,value,source all dataset' to retrieve from dataset list of its property names, their values and source (whether the property was set locally or inherited for example). However, the problem is the output in value field can contain control characters like \n or \t. This causes ambiguity since the same characters are used to separate properties and fields on a line. The output of 'zfs get all' is basically not parseable as it contains values of multiple properties. One needs to use 'zfs get individual_property' to get single property value to avoid ambiguity.

Test case:

- name: basic configuration
  hosts: localhost

  tasks:
  - name: create ZFS with user defined property and multiline value
    zfs:
      name: rpool/myfs
      state: present
      extra_zfs_properties:
        my.custom.property:blah: "one\ntwo\n"

  - name: update user defined property multiline value
    zfs:
      name: rpool/myfs
      state: present
      extra_zfs_properties:
        my.custom.property:blah: "one\ntwo\nthree\nfour\n"
Without the fix
TASK [create ZFS with user defined property and multiline value] ***************

The full traceback is:
Traceback (most recent call last):

[ ... striping lines to make boring error output shorter ... ]

 line 295, in <module>\n  File \"/tmp/ansible_zfs_payload_xteg4tbc/ansible_zfs_payload.zip/ansible_collections/community/general/plugins/modules/zfs.py\", line 274, in main\n  File \"/tmp/ansible_zfs_payload_xteg4tbc/ansible_zfs_payload.zip/ansible_collections/community/general/plugins/modules/zfs.py\", line 196, in set_properties_if_changed\n  File \"/tmp/ansible_zfs_payload_xteg4tbc/ansible_zfs_payload.zip/ansible_collections/community/general/plugins/modules/zfs.py\", line 224, in get_current_properties\nValueError: not enough values to unpack (expected 3, got 2)\n",
    "module_stdout": "",
    "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error",
    "rc": 1
}
With the fix
TASK [create ZFS with user defined property and multiline value] ***************
ok: [localhost] => {
    "changed": false,
    "diff": {
        "after": {
            "extra_zfs_properties": {
                "my.custom.property:blah": "one\ntwo\n"
            }
        },
        "after_header": "rpool/myfs",
        "before": {
            "extra_zfs_properties": {
                "my.custom.property:blah": "one\ntwo\nthree\nfour\n"
            }

^ this is because of test case re-run. The ZFS dataset initially had property value of  "one\ntwo\nthree\nfour\n"

        },
        "before_header": "rpool/myfs"
    },
    "invocation": {
        "module_args": {
            "extra_zfs_properties": {
                "my.custom.property:blah": "one\ntwo\n"
            },
            "name": "rpool/myfs",
            "origin": null,
            "state": "present"
        }
    },
    "my.custom.property:blah": "one\ntwo\n",
    "name": "rpool/myfs",
    "state": "present"
}

TASK [update user defined property multiline value] ****************************

ok: [localhost] => {
    "changed": false,
    "diff": {
        "after": {
            "extra_zfs_properties": {
                "my.custom.property:blah": "one\ntwo\nthree\nfour\n"
            }
        },
        "after_header": "rpool/myfs",
        "before": {
            "extra_zfs_properties": {
                "my.custom.property:blah": "one\ntwo\n"
            }
        },
        "before_header": "rpool/myfs"
    },
    "invocation": {
        "module_args": {
            "extra_zfs_properties": {
                "my.custom.property:blah": "one\ntwo\nthree\nfour\n"
            },
            "name": "rpool/myfs",
            "origin": null,
            "state": "present"
        }
    },
    "my.custom.property:blah": "one\ntwo\nthree\nfour\n",
    "name": "rpool/myfs",
    "state": "present"
}
Result

When test case with fix completes, multi-line ZFS property is set:

$ zfs get all rpool/myfs | tail
rpool/myfs  version                  6                      -
rpool/myfs  vscan                    off                    default
rpool/myfs  writelimit               default                default
rpool/myfs  xattr                    on                     default
rpool/myfs  zoned                    off                    default
rpool/myfs  my.custom.property:blah  one
two
three
four
    local

batrla avatar Mar 29 '23 19:03 batrla