refactor: Ansible 2.19 support (#311)

Ansible 2.19 introduces some big changes
https://docs.ansible.com/ansible/devel/porting_guides/porting_guide_core_2.19.html

A 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 `!=`

In macros in templates, the implicit return value is now `none` - we have to
ensure that the macro returns an empty string in these cases - to do this,
use `{{- '' -}}`

The `ansible_managed` variable cannot be overwritten - use a temp variable.

This also fixes some ansible-lint issues with the new ansible-lint

* Task names in the same file must be unique
* Stricter checking for spacing
* Stricter enforcement of the "no quotes in when/that expressions"

These are the biggest changes.  See the porting guide for others.

Signed-off-by: Rich Megginson <rmeggins@redhat.com>

Author: richm

meta/10_top.j2

@@ -1,39 +1,46 @@
 {% 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 %}
+{%       if indent %}  {% endif %}
 {{ key }} yes
 {%     elif value is sameas false %}
-{%     if indent %}  {% endif %}
+{%       if indent %}  {% endif %}
 {{ key }} no
 {%     elif value is string or value is number %}
-{%     if indent %}  {% endif %}
+{%       if indent %}  {% endif %}
 {{ key }} {{ value | string }}
 {%     else %}
 {%       for i in value %}
-{%     if indent %}  {% endif %}
+{%         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 %}
+{{     render_option(key, override) -}}
 {%   elif __sshd_config[key] is defined %}
-{%     set value = __sshd_config[key] %}
+{{     render_option(key, __sshd_config[key]) -}}
 {%   elif sshd_main_config_file is not none
         and sshd_config_file | dirname == sshd_main_config_file ~ '.d' %}
 {#     Do not use the defaults from main file to avoid recursion #}
+{{- '' -}}
 {%   elif __sshd_defaults[key] is defined and not sshd_skip_defaults %}
 {%     if key == 'HostKey' and __sshd_fips_mode %}
-{%       set value = __sshd_defaults[key] | difference(__sshd_hostkeys_nofips) %}
+{{       render_option(key, __sshd_defaults[key] | difference(__sshd_hostkeys_nofips)) -}}
 {%     else %}
-{%       set value = __sshd_defaults[key] %}
+{{       render_option(key, __sshd_defaults[key]) -}}
 {%     endif %}
+{%   else %}
+{{- '' -}}
 {%   endif %}
-{{ render_option(key,value) -}}
 {% endmacro %}
 {% macro match_block(match_list) %}
 {%   if match_list["Condition"] is defined %}

tasks/install.yml

@@ -30,13 +30,13 @@
 - name: Check FIPS mode
   ansible.builtin.include_tasks: check_fips.yml
   when:
-    - __sshd_hostkeys_nofips | d([])
+    - __sshd_hostkeys_nofips | d([]) | length > 0
 
 - name: Make sure hostkeys are available and have expected permissions
   vars:
     &share_vars # 'MAo=' evaluates to '0\n' in base 64 encoding, which is default
     __sshd_fips_mode: >-
-      {{ __sshd_hostkeys_nofips | d([]) and
+      {{ __sshd_hostkeys_nofips | d([]) | length > 0 and
          (__sshd_kernel_fips_mode.content | d('MAo=') | b64decode | trim == '1' or
           __sshd_userspace_fips_mode.content | d('MAo=') | b64decode | trim != '0') }}
     # This mimics the macro body_option() in sshd_config.j2

templates/sshd_config.j2

@@ -1,41 +1,48 @@
 {{ ansible_managed | comment }}
 {{ "willshersystems:ansible-sshd" | 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 %}
+{%       if indent %}  {% endif %}
 {{ key }} yes
 {%     elif value is sameas false %}
-{%     if indent %}  {% endif %}
+{%       if indent %}  {% endif %}
 {{ key }} no
 {%     elif value is string or value is number %}
-{%     if indent %}  {% endif %}
+{%       if indent %}  {% endif %}
 {{ key }} {{ value | string }}
 {%     else %}
 {%       for i in value %}
-{%     if indent %}  {% endif %}
+{%         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 %}
+{{     render_option(key, override) -}}
 {%   elif __sshd_config[key] is defined %}
-{%     set value = __sshd_config[key] %}
+{{     render_option(key, __sshd_config[key]) -}}
 {%   elif sshd_main_config_file is not none
         and sshd_config_file | dirname == sshd_main_config_file ~ '.d' %}
 {#     Do not use the defaults from main file to avoid recursion #}
+{{- '' -}}
 {%   elif __sshd_defaults[key] is defined and not sshd_skip_defaults %}
 {%     if key == 'HostKey' and __sshd_fips_mode %}
-{%       set value = __sshd_defaults[key] | difference(__sshd_hostkeys_nofips) %}
+{{       render_option(key, __sshd_defaults[key] | difference(__sshd_hostkeys_nofips)) -}}
 {%     else %}
-{%       set value = __sshd_defaults[key] %}
+{{       render_option(key, __sshd_defaults[key]) -}}
 {%     endif %}
+{%   else %}
+{{- '' -}}
 {%   endif %}
-{{ render_option(key,value) -}}
 {% endmacro %}
 {% macro match_block(match_list) %}
 {%   if match_list["Condition"] is defined %}

templates/sshd_config_snippet.j2

@@ -1,39 +1,46 @@
 {% macro render_option(key,value,indent=true) %}
-{%   if value is defined %}
+{%   if value is defined and value is not none %}
 {%     if value is sameas true %}
-{%     if indent %}  {% endif %}
+{%       if indent %}  {% endif %}
 {{ key }} yes
 {%     elif value is sameas false %}
-{%     if indent %}  {% endif %}
+{%       if indent %}  {% endif %}
 {{ key }} no
 {%     elif value is string or value is number %}
-{%     if indent %}  {% endif %}
+{%       if indent %}  {% endif %}
 {{ key }} {{ value | string }}
 {%     else %}
 {%       for i in value %}
-{%     if indent %}  {% endif %}
+{%         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 %}
+{{     render_option(key, override) -}}
 {%   elif __sshd_config[key] is defined %}
-{%     set value = __sshd_config[key] %}
+{{     render_option(key, __sshd_config[key]) -}}
 {%   elif sshd_main_config_file is not none
         and sshd_config_file | dirname == sshd_main_config_file ~ '.d' %}
 {#     Do not use the defaults from main file to avoid recursion #}
+{{- '' -}}
 {%   elif __sshd_defaults[key] is defined and not sshd_skip_defaults %}
 {%     if key == 'HostKey' and __sshd_fips_mode %}
-{%       set value = __sshd_defaults[key] | difference(__sshd_hostkeys_nofips) %}
+{{       render_option(key, __sshd_defaults[key] | difference(__sshd_hostkeys_nofips)) -}}
 {%     else %}
-{%       set value = __sshd_defaults[key] %}
+{{       render_option(key, __sshd_defaults[key]) -}}
 {%     endif %}
+{%   else %}
+{{- '' -}}
 {%   endif %}
-{{ render_option(key,value) -}}
 {% endmacro %}
 {% macro match_block(match_list) %}
 {%   if match_list["Condition"] is defined %}

tests/tasks/check_header.yml

@@ -9,8 +9,8 @@
 - name: Check for presence of ansible managed header, fingerprint
   ansible.builtin.assert:
     that:
-      - ansible_managed in content
+      - __ansible_managed in content
       - __fingerprint in content
   vars:
     content: "{{ (__file_content | d(__content)).content | b64decode }}"
-    ansible_managed: "{{ lookup('template', 'get_ansible_managed.j2') }}"
+    __ansible_managed: "{{ lookup('template', 'get_ansible_managed.j2') }}"

tests/tests_all_options.yml

@@ -83,7 +83,7 @@
         - ansible_facts['os_family'] != "RedHat" or
           ansible_facts['distribution_major_version'] | int == 6
 
-    - name: Get list of options from manual page
+    - name: Show list of options from manual page
       ansible.builtin.shell: >-
         set -eu; set -o | grep -q pipefail && set -o pipefail; man sshd_config | cat
       changed_when: false

tests/tests_backup.yml

@@ -40,7 +40,7 @@
         sshd_Banner: /tmp/banner  # noqa var-naming
       register: second_run
 
-    - name: Find new backups files
+    - name: Find new backups files after reconfiguration
       ansible.builtin.find:
         paths: "{{ main_sshd_config_path }}"
         patterns: "{{ main_sshd_config_name }}.*@*~"

tests/tests_config_namespace.yml

@@ -68,16 +68,16 @@
         - name: Check content of configuration file (blocks)
           ansible.builtin.assert:
             that:
-              - "config.content | b64decode | regex_search('Match all\\s*PasswordAuthentication yes')"
-              - "config.content | b64decode | regex_search('Match all\\s*PasswordAuthentication no')"
+              - config.content | b64decode | regex_search('Match all\\s*PasswordAuthentication yes') | length > 0
+              - config.content | b64decode | regex_search('Match all\\s*PasswordAuthentication no') | length > 0
           when:
             - ansible_facts['os_family'] != 'RedHat' or ansible_facts['distribution_major_version'] != '6'
 
         - name: Check content of configuration file (blocks for RHEL 6)
           ansible.builtin.assert:
             that:
-              - "config.content | b64decode | regex_search('Match address \\*\\s*PasswordAuthentication yes')"
-              - "config.content | b64decode | regex_search('Match address \\*\\s*PasswordAuthentication no')"
+              - config.content | b64decode | regex_search('Match address \\*\\s*PasswordAuthentication yes') | length > 0
+              - config.content | b64decode | regex_search('Match address \\*\\s*PasswordAuthentication no') | length > 0
           when:
             - ansible_facts['os_family'] == 'RedHat'
             - ansible_facts['distribution_major_version'] == '6'
@@ -89,12 +89,12 @@
               - "'PasswordAuthentication yes' in config.content | b64decode"
               - "'Match user root' in config.content | b64decode"
               - "'AllowAgentForwarding no' in config.content | b64decode"
-              - "config.content | b64decode | regex_search('Match user root\\s*AllowAgentForwarding no')"
+              - config.content | b64decode | regex_search('Match user root\\s*AllowAgentForwarding no') | length > 0
               - "'PermitRootLogin no' in config.content | b64decode"
               - "'PasswordAuthentication no' in config.content | b64decode"
               - "'Match Address 127.0.0.1' in config.content | b64decode"
               - "'Banner /etc/issue' in config.content | b64decode"
-              - "config.content | b64decode | regex_search('Match Address 127.0.0.1\\s*Banner /etc/issue')"
+              - config.content | b64decode | regex_search('Match Address 127.0.0.1\\s*Banner /etc/issue') | length > 0
 
         - name: Check the configuration values are effective
           # note, the options are in lower-case here

tests/tests_firewall_selinux.yml

@@ -68,7 +68,7 @@
         sshd_config:
           Port: 222
 
-    - name: Verify the options are correctly set
+    - name: Verify the options are correctly set with non-standard port
       tags: tests::verify
       block:
         - name: Flush handlers
@@ -98,7 +98,7 @@
             - 22
             - 222
 
-    - name: Verify the options are correctly set
+    - name: Verify the options are correctly set with multiple ports
       tags: tests::verify
       block:
         - name: Flush handlers

tests/tests_hostkeys_fips.yml

@@ -98,7 +98,7 @@
       ansible.builtin.include_role:
         name: ansible-sshd
 
-    - name: Verify the options are correctly set
+    - name: Verify the options are correctly set after reconfiguration
       when:
         - ansible_facts['os_family'] == 'RedHat'
         - ansible_facts['distribution_major_version'] | int > 6