From ed33f9820b71a6e650ab1141ff06f2eb566c5a9f Mon Sep 17 00:00:00 2001 From: Rich Megginson Date: Tue, 17 Jun 2025 18:40:19 -0600 Subject: [PATCH 1/3] refactor: Ansible 2.19 support 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 --- .dev-tools/10_top.j2 | 22 +++++++----- tasks/main.yml | 75 +++++++++++------------------------------ templates/ssh_config.j2 | 22 +++++++----- tests/tests_backup.yml | 2 +- 4 files changed, 48 insertions(+), 73 deletions(-) diff --git a/.dev-tools/10_top.j2 b/.dev-tools/10_top.j2 index 3ffe9b10..849e6a88 100644 --- a/.dev-tools/10_top.j2 +++ b/.dev-tools/10_top.j2 @@ -1,7 +1,7 @@ {{ ansible_managed | comment }} {{ "system_role:ssh" | comment(prefix="", postfix="") }} {% macro render_option(key, value, indent=false) %} -{% if value is defined %} +{% if value is defined and value is not none %} {% if value is sameas true %} {% if indent %} {% endif %} {{ key }} yes @@ -13,22 +13,28 @@ {{ key }} {{ value | string }} {% else %} {% for i in value %} +{% if i is none %} +{{- '' -}} +{% else %} {% if indent %} {% endif %} {{ key }} {{ i | string }} +{% endif %} {% endfor %} {% endif %} +{% else %} +{{- '' -}} {% endif %} {% endmacro %} {% macro body_option(key, override) %} -{% set value = undefined %} -{% if override is defined %} -{% set value = override %} -{% elif ssh[key] is defined %} -{% set value = ssh[key] %} +{% if override is defined and override is not none %} +{{ render_option(key, override) -}} +{% elif ssh[key] is defined and ssh[key] is not none %} +{{ render_option(key, ssh[key]) -}} {% elif __ssh_defaults[key] is defined and not __ssh_skip_defaults | trim | bool %} -{% set value = __ssh_defaults[key] %} +{{ render_option(key, __ssh_defaults[key]) -}} +{% else %} +{{- '' -}} {% endif %} -{{ render_option(key, value) -}} {% endmacro %} {% macro match_block(match_list) %} {% if match_list["Condition"] is defined %} diff --git a/tasks/main.yml b/tasks/main.yml index 68361eb8..739a9fc5 100644 --- a/tasks/main.yml +++ b/tasks/main.yml @@ -58,61 +58,24 @@ owner: "{{ __ssh_config_owner | trim }}" group: "{{ __ssh_config_group | trim }}" mode: "{{ __ssh_config_mode | trim }}" - validate: >- - {% if __ssh_supports_validate %} - ssh -G -F %s example.com - {% else %} - true %s - {% endif %} + validate: "{{ __ssh_supports_validate | ternary('ssh -G -F %s example.com', 'true %s') }}" backup: "{{ ssh_backup }}" vars: - __ssh_skip_defaults: >- - {% if ssh_skip_defaults != 'auto' %} - {{ ssh_skip_defaults }} - {% elif ssh_user is not none %} - true - {% else %} - {% if ssh_drop_in_name is not none and __ssh_supports_drop_in %} - true - {% else %} - false - {% endif %} - {% endif %} - __ssh_config_file: >- - {% if ssh_config_file is not none %} - {{ ssh_config_file }} - {% elif ssh_user is not none and - ansible_facts['getent_passwd'] is defined %} - {{ ansible_facts['getent_passwd'][ssh_user][4] }}/.ssh/config - {% else %} - {% if ssh_drop_in_name is not none and __ssh_supports_drop_in %} - {{ __ssh_drop_in_template | replace("{name}", ssh_drop_in_name) }} - {% else %} - /etc/ssh/ssh_config - {% endif %} - {% endif %} - __ssh_config_owner: >- - {% if ssh_config_owner is not none %} - {{ ssh_config_owner }} - {% elif ssh_user is not none %} - {{ ssh_user }} - {% else %} - root - {% endif %} - __ssh_config_group: >- - {% if ssh_config_group is not none %} - {{ ssh_config_group }} - {% elif ssh_user is not none and - ansible_facts['getent_passwd'] is defined %} - {{ ssh_user }} - {% else %} - root - {% endif %} - __ssh_config_mode: >- - {% if ssh_config_mode is not none %} - {{ ssh_config_mode }} - {% elif ssh_user is not none %} - 600 - {% else %} - 644 - {% endif %} + __ssh_skip_defaults: "{{ ssh_skip_defaults if ssh_skip_defaults != 'auto' + else (ssh_user is not none) or + (ssh_drop_in_name is not none and __ssh_supports_drop_in) }}" + __ssh_config_file: "{{ ssh_config_file if ssh_config_file is not none + else ansible_facts['getent_passwd'][ssh_user][4] ~ '/.ssh/config' + if ssh_user is not none and ansible_facts['getent_passwd'] is defined + else __ssh_drop_in_template | replace('{name}', ssh_drop_in_name) + if ssh_drop_in_name is not none and __ssh_supports_drop_in + else '/etc/ssh/ssh_config' }}" + __ssh_config_owner: "{{ ssh_config_owner if ssh_config_owner is not none + else ssh_user if ssh_user is not none + else 'root' }}" + __ssh_config_group: "{{ ssh_config_group if ssh_config_group is not none + else ssh_user if ssh_user is not none and ansible_facts['getent_passwd'] is defined + else 'root' }}" + __ssh_config_mode: "{{ ssh_config_mode if ssh_config_mode is not none + else '0600' if ssh_user is not none + else '0644' }}" diff --git a/templates/ssh_config.j2 b/templates/ssh_config.j2 index a875ae0f..1304060d 100644 --- a/templates/ssh_config.j2 +++ b/templates/ssh_config.j2 @@ -1,7 +1,7 @@ {{ ansible_managed | comment }} {{ "system_role:ssh" | comment(prefix="", postfix="") }} {% macro render_option(key, value, indent=false) %} -{% if value is defined %} +{% if value is defined and value is not none %} {% if value is sameas true %} {% if indent %} {% endif %} {{ key }} yes @@ -13,22 +13,28 @@ {{ key }} {{ value | string }} {% else %} {% for i in value %} +{% if i is none %} +{{- '' -}} +{% else %} {% if indent %} {% endif %} {{ key }} {{ i | string }} +{% endif %} {% endfor %} {% endif %} +{% else %} +{{- '' -}} {% endif %} {% endmacro %} {% macro body_option(key, override) %} -{% set value = undefined %} -{% if override is defined %} -{% set value = override %} -{% elif ssh[key] is defined %} -{% set value = ssh[key] %} +{% if override is defined and override is not none %} +{{ render_option(key, override) -}} +{% elif ssh[key] is defined and ssh[key] is not none %} +{{ render_option(key, ssh[key]) -}} {% elif __ssh_defaults[key] is defined and not __ssh_skip_defaults | trim | bool %} -{% set value = __ssh_defaults[key] %} +{{ render_option(key, __ssh_defaults[key]) -}} +{% else %} +{{- '' -}} {% endif %} -{{ render_option(key, value) -}} {% endmacro %} {% macro match_block(match_list) %} {% if match_list["Condition"] is defined %} diff --git a/tests/tests_backup.yml b/tests/tests_backup.yml index 19d10dd0..95c258ea 100644 --- a/tests/tests_backup.yml +++ b/tests/tests_backup.yml @@ -40,7 +40,7 @@ ssh_ForwardX11Trusted: 'yes' # noqa var-naming register: second_run - - name: Find new backups files + - name: Find new backups files again ansible.builtin.find: paths: "{{ main_ssh_config_path }}" patterns: "{{ main_ssh_config_name }}.*@*~" From 5b4746b82ed4c0bd0d21a5a0706662dd8e4e9d8d Mon Sep 17 00:00:00 2001 From: Rich Megginson Date: Wed, 18 Jun 2025 07:05:22 -0600 Subject: [PATCH 2/3] improve indentation --- .dev-tools/10_top.j2 | 8 ++++---- templates/ssh_config.j2 | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.dev-tools/10_top.j2 b/.dev-tools/10_top.j2 index 849e6a88..e51cb1d2 100644 --- a/.dev-tools/10_top.j2 +++ b/.dev-tools/10_top.j2 @@ -16,7 +16,7 @@ {% if i is none %} {{- '' -}} {% else %} -{% if indent %} {% endif %} +{% if indent %} {% endif %} {{ key }} {{ i | string }} {% endif %} {% endfor %} @@ -27,11 +27,11 @@ {% endmacro %} {% macro body_option(key, override) %} {% if override is defined and override is not none %} -{{ render_option(key, override) -}} +{{ render_option(key, override) -}} {% elif ssh[key] is defined and ssh[key] is not none %} -{{ render_option(key, ssh[key]) -}} +{{ render_option(key, ssh[key]) -}} {% elif __ssh_defaults[key] is defined and not __ssh_skip_defaults | trim | bool %} -{{ render_option(key, __ssh_defaults[key]) -}} +{{ render_option(key, __ssh_defaults[key]) -}} {% else %} {{- '' -}} {% endif %} diff --git a/templates/ssh_config.j2 b/templates/ssh_config.j2 index 1304060d..9eb43e95 100644 --- a/templates/ssh_config.j2 +++ b/templates/ssh_config.j2 @@ -16,7 +16,7 @@ {% if i is none %} {{- '' -}} {% else %} -{% if indent %} {% endif %} +{% if indent %} {% endif %} {{ key }} {{ i | string }} {% endif %} {% endfor %} @@ -27,11 +27,11 @@ {% endmacro %} {% macro body_option(key, override) %} {% if override is defined and override is not none %} -{{ render_option(key, override) -}} +{{ render_option(key, override) -}} {% elif ssh[key] is defined and ssh[key] is not none %} -{{ render_option(key, ssh[key]) -}} +{{ render_option(key, ssh[key]) -}} {% elif __ssh_defaults[key] is defined and not __ssh_skip_defaults | trim | bool %} -{{ render_option(key, __ssh_defaults[key]) -}} +{{ render_option(key, __ssh_defaults[key]) -}} {% else %} {{- '' -}} {% endif %} From 1d738876a7cb0811d54f666eb58200fd4a463bc9 Mon Sep 17 00:00:00 2001 From: Rich Megginson Date: Wed, 18 Jun 2025 09:27:27 -0600 Subject: [PATCH 3/3] fix formatting --- tasks/main.yml | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tasks/main.yml b/tasks/main.yml index 739a9fc5..7cbe5a65 100644 --- a/tasks/main.yml +++ b/tasks/main.yml @@ -64,12 +64,15 @@ __ssh_skip_defaults: "{{ ssh_skip_defaults if ssh_skip_defaults != 'auto' else (ssh_user is not none) or (ssh_drop_in_name is not none and __ssh_supports_drop_in) }}" - __ssh_config_file: "{{ ssh_config_file if ssh_config_file is not none - else ansible_facts['getent_passwd'][ssh_user][4] ~ '/.ssh/config' - if ssh_user is not none and ansible_facts['getent_passwd'] is defined - else __ssh_drop_in_template | replace('{name}', ssh_drop_in_name) - if ssh_drop_in_name is not none and __ssh_supports_drop_in - else '/etc/ssh/ssh_config' }}" + __ssh_config_file: "{{ ssh_config_file + if ssh_config_file is not none + else + ansible_facts['getent_passwd'][ssh_user][4] ~ '/.ssh/config' + if ssh_user is not none and ansible_facts['getent_passwd'] is defined + else + __ssh_drop_in_template | replace('{name}', ssh_drop_in_name) + if ssh_drop_in_name is not none and __ssh_supports_drop_in + else '/etc/ssh/ssh_config' }}" __ssh_config_owner: "{{ ssh_config_owner if ssh_config_owner is not none else ssh_user if ssh_user is not none else 'root' }}"