From 860252e5ea744b67e7d2250117b77559d9e386b0 Mon Sep 17 00:00:00 2001 From: Rich Megginson Date: Tue, 8 Jul 2025 14:49:30 -0600 Subject: [PATCH] refactor: support ansible 2.19, new ansible-lint Ansible 2.19 introduces some big changes https://docs.ansible.com/ansible/devel/porting_guides/porting_guide_core_2.19.html One big change is that data structures are no longer mutable by the use of python methods such as `__setitem__`, `setdefault`, `update`, etc. in Jinja constructs. Instead, items must use filters or other Jinja operations. One common idiom is to mutate each element in a list. Since we cannot do this "in-place" anymore, a common way to do this is: ```yaml - name: Construct a new list from an existing list and mutate each element set_fact: __new_list: "{{ __new_list | d([]) + [mutated_item] }}" loop: "{{ old_list }}" mutated_item: "{{ some value based on item from old list }}" - name: Reset original old list set_fact: old_list: "{{ __new_list }}" ``` Similarly with `dict` items: ```yaml - name: Construct a new dict from an existing dict and mutate each element set_fact: __new_dict: "{{ __new_dict | d({}) | combine(mutated_item) }}" loop: "{{ old_dict | dict2items }}" mutated_item: "{{ {item.key: mutation of item.value} }}" - name: Reset original old dict set_fact: old_dict: "{{ __new_dict }}" ``` Another big change is that a boolean expression in a `when` or similar construct must be converted to a boolean - we cannot rely on the implicit evaluation in a boolean context. For example, if `var` is some iterable, like a `dict`, `list`, or `string`, you used to be able to evaluate an empty value in a boolean context: ```yaml when: var # do this only if var is not empty ``` You now have to explicitly test for empty using `length`: ```yaml when: var | length > 0 # do this only if var is not empty ``` Similarly for `int` values - you cannot rely on `0` being evaluated as false and non-zero true - you must explicitly compare the values with `==` or `!=` These are the biggest changes. See the porting guide for others. Signed-off-by: Rich Megginson --- tasks/get_update_user_info.yml | 31 +++++----------- tasks/manage_user_info.yml | 64 ++++++++++++++++------------------ tests/tests_basic.yml | 16 ++++----- tests/tests_user_units.yml | 2 +- 4 files changed, 47 insertions(+), 66 deletions(-) diff --git a/tasks/get_update_user_info.yml b/tasks/get_update_user_info.yml index e9091b3..164f8cb 100644 --- a/tasks/get_update_user_info.yml +++ b/tasks/get_update_user_info.yml @@ -15,26 +15,11 @@ - name: Update systemd user info with new user info when: __systemd_user_name not in __systemd_user_info set_fact: - __systemd_user_info: | - {% set getent = ansible_facts["getent_passwd"][__systemd_user_name] %} - {% set rv = __systemd_user_info | d({}) %} - {% if __systemd_user_name not in rv %} - {% set _ = rv.__setitem__(__systemd_user_name, {}) %} - {% endif %} - {% if "xdg_dir" not in rv[__systemd_user_name] %} - {% set xdg_dir = "/run/user/" ~ getent[1] %} - {% set _ = rv[__systemd_user_name].update({"xdg_dir": xdg_dir}) %} - {% endif %} - {% if "units_dir" not in rv[__systemd_user_name] %} - {% if __systemd_user_name == "root" %} - {% set _ = rv[__systemd_user_name].update({"units_dir": __admin_units_dir}) %} - {% else %} - {% set units_dir = getent[4] ~ '/' ~ __user_units_dir %} - {% set _ = rv[__systemd_user_name].update({"units_dir": units_dir}) %} - {% endif %} - {% endif %} - {% if "group" not in rv[__systemd_user_name] %} - {% set group = getent[2] %} - {% set _ = rv[__systemd_user_name].update({"group": group}) %} - {% endif %} - {{ rv }} + __systemd_user_info: "{{ __systemd_user_info | combine({__systemd_user_name: user_dict}) }}" + vars: + getent: "{{ ansible_facts['getent_passwd'][__systemd_user_name] }}" + user_dict: + xdg_dir: /run/user/{{ getent[1] }} + units_dir: "{{ __admin_units_dir if __systemd_user_name == 'root' + else getent[4] ~ '/' ~ __user_units_dir }}" + group: "{{ getent[2] }}" diff --git a/tasks/manage_user_info.yml b/tasks/manage_user_info.yml index b0ad6db..329d2e7 100644 --- a/tasks/manage_user_info.yml +++ b/tasks/manage_user_info.yml @@ -32,43 +32,39 @@ loop_control: loop_var: __systemd_user_name + - name: Reset __systemd_dict_list + set_fact: + __systemd_dict_list: [] + # convert to the output format - name: Convert to list of dict with user data set_fact: - __systemd_dict_list: | - {% set rv = [] %} - {% for item in __systemd_list %} - {% set newitem = {} %} - {% if item is mapping %} - {% set _ = newitem.update(item) %} - {% else %} - {% set _ = newitem.update({"item": item}) %} - {% endif %} - {% if "state" not in newitem %} - {% set _ = newitem.update({"state": "present"}) %} - {% endif %} - {% if "user" not in newitem %} - {% set _ = newitem.update({"user": "root"}) %} - {% endif %} - {% set user = newitem["user"] %} - {% if "group" not in newitem %} - {% set _ = newitem.update({"group": __systemd_user_info[user]["group"]}) %} - {% endif %} - {% if "xdg_dir" not in newitem %} - {% set _ = newitem.update({"xdg_dir": __systemd_user_info[user]["xdg_dir"]}) %} - {% endif %} - {% if "units_dir" not in newitem %} - {% set _ = newitem.update({"units_dir": __systemd_user_info[user]["units_dir"]}) %} - {% endif %} - {% if "mode" not in newitem %} - {% set _ = newitem.update({"mode": (user == "root") | ternary("0644", "0600")}) %} - {% endif %} - {% if "dir_mode" not in newitem %} - {% set _ = newitem.update({"dir_mode": (user == "root") | ternary("0755", "0700")}) %} - {% endif %} - {% set _ = rv.append(newitem) %} - {% endfor %} - {{ rv }} + __systemd_dict_list: "{{ __systemd_dict_list + + [newitem | combine(newstate, newuser, newgroup, newxdg_dir, newunits_dir, newmode, newdir_mode)] }}" + loop: "{{ __systemd_list }}" + vars: + newitem: "{{ item if item is mapping else {'item': item} }}" + newstate: + state: "{{ item['state'] if item is mapping and 'state' in item + else 'present' }}" + newuser: + user: "{{ item['user'] if item is mapping and 'user' in item + else 'root' }}" + newgroup: + group: "{{ item['group'] if item is mapping and 'group' in item + else __systemd_user_info[newuser['user']]['group'] }}" + newxdg_dir: + xdg_dir: "{{ item['xdg_dir'] if item is mapping and 'xdg_dir' in item + else __systemd_user_info[newuser['user']]['xdg_dir'] }}" + newunits_dir: + units_dir: "{{ item['units_dir'] if item is mapping and 'units_dir' in item + else __systemd_user_info[newuser['user']]['units_dir'] }}" + newmode: + mode: "{{ item['mode'] if item is mapping and 'mode' in item + else (newuser['user'] == 'root') | ternary('0644', '0600') }}" + newdir_mode: + dir_mode: "{{ item['dir_mode'] if item is mapping and 'dir_mode' in item + else (newuser['user'] == 'root') | ternary('0755', '0700') }}" - name: Enable linger if needed command: loginctl enable-linger {{ item | quote }} diff --git a/tests/tests_basic.yml b/tests/tests_basic.yml index 189324d..e39aa5d 100644 --- a/tests/tests_basic.yml +++ b/tests/tests_basic.yml @@ -137,7 +137,7 @@ roles: - linux-system-roles.systemd tasks: - - name: Get UnitFileState= + - name: Get UnitFileState= - 2 # noqa command-instead-of-module command: systemctl show -p UnitFileState foo.service register: unit_file_state_now @@ -158,13 +158,13 @@ roles: - linux-system-roles.systemd pre_tasks: - - name: Save UnitFileState before calling role + - name: Save UnitFileState before calling role - 2 # noqa command-instead-of-module command: systemctl show -p UnitFileState sshd.service register: sshd_state_before changed_when: false tasks: - - name: Get UnitFileState= + - name: Get UnitFileState= - 3 # noqa command-instead-of-module command: systemctl show -p UnitFileState sshd.service register: sshd_state_after @@ -187,7 +187,7 @@ roles: - linux-system-roles.systemd tasks: - - name: Get UnitFileState= + - name: Get UnitFileState= - 4 # noqa command-instead-of-module command: systemctl show -p UnitFileState sshd.service register: sshd_state_now @@ -277,7 +277,7 @@ systemd_masked_units: - "{{ test_unit }}" - - name: Get test unit state + - name: Get test unit state - 2 # noqa command-instead-of-module command: systemctl show -p UnitFileState -p SubState "{{ test_unit }}" register: test_unit_state @@ -290,7 +290,7 @@ test_unit_state.stdout is search("UnitFileState=bad") - test_unit_state.stdout is search("SubState=dead") - - name: Ensure test unit is running and unmasked + - name: Ensure test unit is running and unmasked - 2 include_role: name: linux-system-roles.systemd vars: @@ -299,13 +299,13 @@ systemd_unmasked_units: - "{{ test_unit }}" - - name: Get test unit state + - name: Get test unit state - 3 # noqa command-instead-of-module command: systemctl show -p UnitFileState -p SubState "{{ test_unit }}" register: test_unit_state changed_when: false - - name: Ensure test unit running and unmasked + - name: Ensure test unit running and unmasked - 2 assert: that: - test_unit_state.stdout is search("UnitFileState=enabled") or diff --git a/tests/tests_user_units.yml b/tests/tests_user_units.yml index 705a6eb..c0ddda8 100644 --- a/tests/tests_user_units.yml +++ b/tests/tests_user_units.yml @@ -161,7 +161,7 @@ vars: systemd_disabled_units: "{{ __systemd_disabled_units }}" - - name: Get unit file state of units after + - name: Get unit file state of units after - 2 # noqa command-instead-of-module command: systemctl {{ scope }} show -p UnitFileState {{ item.item }} changed_when: false