Fix assorted ansible-lint errors

- no-free-form (except for "include_role: name=x")
- deprecated-module (include → include_tasks)
- role-name (xxx-yyy → xxx_yyy)
- command-instead-of-shell
- literal-compare
- schema[tasks]
- name[casing]

Also includes a .ansible-lint file that adds all tests to skip_list, so
that they can be reviewed one by one over time.

See https://ansible-lint.readthedocs.io/configuring/ for details.

.ansible-lint

@@ -0,0 +1,30 @@
+---
+
+exclude_paths:
+  - tpa-venv/
+
+enable_list: []
+warn_list: []
+skip_list:
+  - args[module]  # Validating module arguments.
+  - command-instead-of-module  # Using command rather than module.
+  - experimental  # all rules tagged as experimental
+  - fqcn[action-core]  # Use FQCN for builtin actions.
+  - ignore-errors  # Use failed_when and specify error conditions instead of using ignore_errors.
+  - jinja[invalid]  # Rule that looks inside jinja2 templates.
+  - jinja[spacing]  # Rule that looks inside jinja2 templates.
+  - key-order[task]  # Ensure specific order of keys in mappings.
+  - name[missing]  # Rule for checking task and play names.
+  - name[template]  # Rule for checking task and play names.
+  - no-changed-when  # Commands should not change things if nothing needs doing.
+  - no-free-form  # Rule for detecting discouraged free-form syntax for action modules.
+  - no-handler  # Tasks that run when changed should likely be handlers.
+  - package-latest  # Package installs should not use latest.
+  - risky-file-permissions  # File permissions unset or incorrect.
+  - risky-shell-pipe  # Shells that use pipes should set the pipefail option.
+  - role-name[path]  # Role name {0} does not match ``^[a-z][a-z0-9_]*$`` pattern.
+  - schema[requirements]  # Perform JSON Schema Validation for known lintable kinds.
+  - var-naming  # All variables should be named using only lowercase and underscores.
+  # This rule tries to validate platforms/{common,aws}/inventory/*.yml
+  # as static inventory files (but they're task lists).
+  - schema[inventory]  # Perform JSON Schema Validation for known lintable kinds.

architectures/lib/fini.yml

@@ -32,7 +32,7 @@
       is not empty
 
   - include_role:
-      name: post-deploy
+      name: post_deploy
       apply:
         tags: post-deploy
     tags: always

roles/barman/pkg/tasks/src.yml

@@ -18,11 +18,16 @@
     barman_git_ref is defined
 
 - name: Remove old build directory
-  file: state=absent path="{{ barman_build_dir }}" force=yes
+  file:
+    path: "{{ barman_build_dir }}"
+    state: absent
+    force: yes
   tags: [build-clean, barman-clean]
 
 - name: Ensure build directory exists
-  file: state=directory path="{{ barman_build_dir }}"
+  file:
+    path: "{{ barman_build_dir }}"
+    state: directory
 
 # The source directory may be read-only, and setup.py does not make it
 # convenient to do an out-of-tree build the way we do for other things,

roles/facts/tasks/main.yml

@@ -20,9 +20,9 @@
 - include_role: name=postgres/facts
   when: >
     'postgres' in role and
-    pgdata_initialised|default('false') == True and
+    pgdata_initialised|default('false') is true and
     pgdata_user|default('UNKNOWN') != 'UNKNOWN' and
-    postgres_running|default('false') == True
+    postgres_running|default('false') is true
 
 # We may change default values over time, but the changes are not always
 # worth forcing a reload or restart on existing clusters. Now that we've

roles/final/tasks/main.yml

@@ -63,7 +63,7 @@
 # backups at all.
 
 - include_role:
-    name: barman/first-backup
+    name: barman/first_backup
     apply:
       tags: first-backup
   with_items: "{{ backup|default([]) }}"

roles/haproxy/tasks/main.yml

@@ -27,6 +27,7 @@
     'postgres' in role
     and groups['role_haproxy']|default([]) is not empty
 
-- set_fact: initialise_haproxy=True
+- set_fact:
+    initialise_haproxy: true
   when: >
     'haproxy' in role

roles/harp/service/tasks/main.yml

@@ -40,7 +40,7 @@
   - set_fact:
       postgres_service_name: harp-postgres.target
 
-  - name:
+  - name: Ensure postgres service is enabled
     systemd:
       name: "{{ postgres_service_name }}"
       enabled: yes

roles/pem/agent/config/final/tasks/register-server.yml

@@ -3,7 +3,8 @@
 # © Copyright EnterpriseDB UK Limited 2015-2022 - All rights reserved.
 
 - name: Change the default monitoring database for agents on BDR nodes
-  set_fact: asb_host_db="{{ bdr_database }}"
+  set_fact:
+    asb_host_db: "{{ bdr_database }}"
   when: >
     'bdr' in role
 

roles/pem/server/config/final/tasks/generate-db-certificates.yml

@@ -13,6 +13,9 @@
   register: is_pem_server_ca_cert_exists
 
 - name: Generate certificates
+  when:
+    not is_pem_server_ca_key_exists.stat.exists
+    and not is_pem_server_ca_cert_exists.stat.exists
   block:
     - name: Generate ca key
       postgresql_query:
@@ -92,5 +95,3 @@
       include_role: name=postgres/restart
       vars:
         postgres_service_end_state: restarted
-
-  when: is_pem_server_ca_key_exists.stat.exists == False and is_pem_server_ca_cert_exists.stat.exists == False

roles/pem/server/config/webserver/tasks/configure_httpd_module.yml

@@ -5,13 +5,13 @@
 - name: "Configure Add-ons for {{ pem_web_server_name[ansible_os_family] }}..."
   block:
     - name: Check the mod_ssl
-      shell: /usr/sbin/a2query -m ssl
+      command: /usr/sbin/a2query -m ssl
       register: mod_ssl
       ignore_errors: true
       no_log: true
 
     - name: Check the mod_gnutls
-      shell: /usr/sbin/a2query -m gnutls
+      command: /usr/sbin/a2query -m gnutls
       register: mod_gnutls
       ignore_errors: true
       no_log: true

roles/pem/server/config/webserver/tasks/main.yml

@@ -72,12 +72,12 @@
 - include_tasks: pem_sites.yml
 
 - name: Run setup.py to create pem.db
-  shell: "{{ pem_python_executable }} /usr/edb/pem/web/setup.py"
+  command: "{{ pem_python_executable }} /usr/edb/pem/web/setup.py"
   become_user: pem
   become: yes
 
 - name: Configure SELinux security policy for PEM
-  shell: "{{ pem_base_dir }}/bin/configure-selinux.sh"
+  command: "{{ pem_base_dir }}/bin/configure-selinux.sh"
 
 # XXX: What triggers this restart? Is it the SELinux config?
 - name: "Restart httpd service - {{ pem_web_server_service_name[ansible_os_family] }}"

roles/pem/server/config/webserver/tasks/pem_sites.yml

@@ -14,10 +14,10 @@
   become: yes
   tags: [pem-server, pem-webserver]
 
-- name: ssl-pem configuration (Debian/Ubuntu)
+- name: Enable ssl-pem configuration (Debian/Ubuntu)
   block:
     - name: "Enable the apache2 site: {{ pem_branding_edb_wl }}-ssl-pem.conf"
-      shell: "/usr/sbin/a2ensite -m {{ pem_branding_edb_wl }}-ssl-pem"
+      command: "/usr/sbin/a2ensite -m {{ pem_branding_edb_wl }}-ssl-pem"
 
     - name: "Install {{ pem_branding_edb_wl }}-gnutls-pem.conf"
       template:
@@ -28,7 +28,7 @@
         mode: 0644
 
     - name: "Enable the apache2 site: {{ pem_branding_edb_wl }}-gnutls-pem.conf"
-      shell: "/usr/sbin/a2ensite -m {{ pem_branding_edb_wl }}-gnutls-pem"
+      command: "/usr/sbin/a2ensite -m {{ pem_branding_edb_wl }}-gnutls-pem"
 
   when: ansible_os_family == 'Debian'
   become: yes

roles/pgbackupapi/service/tasks/ssl.yml

@@ -57,7 +57,7 @@
     check_result_varname: "pgbapi"
 
 # Update trusted CA cert to include our CA
-- name: copy CA to shared trusted CA
+- name: Copy CA to shared trusted CA
   copy:
     src: "{{ ssl_ca_file }}"
     remote_src: yes

roles/pgbouncer/tasks/main.yml

@@ -16,6 +16,7 @@
     'postgres' in role or
     'pgbouncer' in role
 
-- set_fact: initialise_pgbouncer=True
+- set_fact:
+    initialise_pgbouncer: true
   when: >
     'pgbouncer' in role

roles/pkg/download/tasks/os/RedHat/download.yml

@@ -33,7 +33,7 @@
     |list|length > 0
 
 - name: Generate repository metadata with createrepo for RH7
-  shell: createrepo .
+  command: createrepo .
   args:
     chdir: "{{ _download_dir }}"
   when: >

roles/platforms/common/tasks/main.yml

@@ -15,7 +15,8 @@
 # after we're done modifying instance roles.
 
 - name: Create role-based groups for roles which affect other instances
-  group_by: key="role_{{ item }}"
+  group_by:
+    key: "role_{{ item }}"
   when: item in role
   with_items:
   - log-server
@@ -142,7 +143,8 @@
     and 'pem-server' not in role
 
 - name: Create role-based groups
-  group_by: key="role_{{ item }}"
+  group_by:
+    key: "role_{{ item }}"
   with_items: "{{ role }}"
 
 - name: Create group(s) based on the value of bdr_node_group

roles/postgres/config/final/tasks/pgbouncer.yml

@@ -64,4 +64,5 @@
     dbs: "{{ auth_function_dbs|default({}) }}"
   when:
     - item not in ['template0', 'bdr_supervisordb']
-    - item not in dbs or dbs[item].pgbouncer_can_execute == false
+    - item not in dbs
+      or dbs[item].pgbouncer_can_execute is false

roles/postgres/initdb/tasks/main.yml

@@ -116,6 +116,7 @@
         postgres_conf_dir != postgres_data_dir
       changed_when: true
 
-    - set_fact: pgdata_initialised=true
+    - set_fact:
+        pgdata_initialised: true
   when:
     not pgdata_initialised and 'replica' not in role

roles/postgres/src/tasks/main.yml

@@ -18,11 +18,16 @@
     postgres_git_ref is defined
 
 - name: Remove old build directory
-  file: state=absent path="{{ postgres_build_dir }}" force=yes
+  file:
+    path: "{{ postgres_build_dir }}"
+    state: absent
+    force: yes
   tags: [build-clean, postgres-clean]
 
 - name: Ensure build directory exists
-  file: state=directory path="{{ postgres_build_dir }}"
+  file:
+    path: "{{ postgres_build_dir }}"
+    state: directory
 
 - name: Configure Postgres
   shell: >

roles/postgres/update/tasks/main.yml

@@ -20,6 +20,7 @@
     hook: "{{ cluster_dir }}/hooks/postgres-post-update.yml"
   tags: always
 
-- fail: msg="Please redeploy to update extensions built from source"
+- fail:
+    msg: Please redeploy to update extensions built from source
   when: >
     install_from_source is not empty

roles/postgres/update/tasks/src.yml

@@ -2,4 +2,5 @@
 
 # © Copyright EnterpriseDB UK Limited 2015-2022 - All rights reserved.
 
-- fail: msg="Please redeploy to update a source installation"
+- fail:
+    msg: "Please redeploy to update a source installation"

roles/repmgr/replica/final/tasks/clone.yml

@@ -63,4 +63,5 @@
 
 - include_role: name=postgres/config
 
-- set_fact: pgdata_initialised=True
+- set_fact:
+    pgdata_initialised: true

roles/repmgr/src/tasks/main.yml

@@ -18,12 +18,16 @@
     repmgr_git_ref is defined
 
 - name: Remove old build directory
-  file: state=absent path="{{ repmgr_build_dir }}" force=yes
+  file:
+    path: "{{ repmgr_build_dir }}"
+    state: absent
+    force: yes
   tags: [build-clean, repmgr-clean]
 
 - name: Ensure build directory exists
-  file: state=directory path="{{ repmgr_build_dir }}"
-
+  file:
+    path: "{{ repmgr_build_dir }}"
+    state: directory
 
 - name: Configure repmgr if necessary
   shell: >

roles/repmgr/tasks/main.yml

@@ -15,4 +15,5 @@
   when: >
     'postgres' in role
 
-- set_fact: initialise_replication=True
+- set_fact:
+    initialise_replication: true

roles/selftest/tasks/main.yml

@@ -3,7 +3,8 @@
 # © Copyright EnterpriseDB UK Limited 2015-2022 - All rights reserved.
 
 - name: Gather facts on localhost
-  setup: gather_subset=min
+  setup:
+    gather_subset: min
 
 - name: Check for Python 3.6.x+ and Ansible 2.9.x
   assert:

roles/src/install/tasks/main.yml

@@ -21,11 +21,16 @@
 
 - block:
   - name: Remove old build directory
-    file: state=absent path="{{ build_directory }}" force=yes
+    file:
+      path: "{{ build_directory }}"
+      state: absent
+      force: yes
     tags: build-clean
 
   - name: Ensure build directory exists
-    file: state=directory path="{{ build_directory }}"
+    file:
+      path: "{{ build_directory }}"
+      state: directory
   when:
     build_directory|default(source_directory) != source_directory
 

roles/sys/repositories/tasks/os/Debian/repositories.yml

@@ -143,4 +143,5 @@
     repository_files: "{{ _2q_repositories|map(attribute='file')|list }}"
 
 - name: Update apt cache
-  apt: update_cache=yes
+  apt:
+    update_cache: yes

roles/sys/rsyslog/tasks/main.yml

@@ -74,7 +74,7 @@
       notify:
         - Note rsyslog restart required
 
-    - name: configure TCP listening on log-server
+    - name: Configure TCP listening on log-server
       copy:
         src: "{{ item }}"
         dest: "/etc/rsyslog.d/{{ item }}"

roles/sys/tasks/main.yml

@@ -9,11 +9,11 @@
 # We invoke sys/bootstrap to check whether the basic requirements are
 # satisfied, and install them otherwise; but instances without outside
 # network access can't install anything that's missing, so we invoke
-# sys/local-repo first (in such an environment, the local-repo must
+# sys/local_repo first (in such an environment, the local-repo must
 # provide all packages required during deployment).
 
 - include_role:
-    name: sys/local-repo
+    name: sys/local_repo
     apply:
       tags: local-repo
   tags: local-repo

roles/sys/upgrade/tasks/os/Debian.yml

@@ -2,4 +2,6 @@
 
 # © Copyright EnterpriseDB UK Limited 2015-2022 - All rights reserved.
 
-- apt: update_cache=yes upgrade=dist
+- apt:
+    update_cache: yes
+    upgrade: dist

roles/sys/upgrade/tasks/os/RedHat.yml

@@ -2,4 +2,7 @@
 
 # © Copyright EnterpriseDB UK Limited 2015-2022 - All rights reserved.
 
-- yum: update_cache=yes name=* state=latest
+- yum:
+    update_cache: yes
+    name: "*"
+    state: latest

roles/test/tasks/camo.yml

@@ -47,7 +47,7 @@
       msg: "Test cluster is unsupported. Requires haproxy backends to be camo pairs"
     when: haproxy_node is undefined
 
-  - name: get haproxy haproxy_backend
+  - name: Get haproxy_backend name
     postgresql_query:
       conninfo: "host={{ haproxy_node }} port={{ pgbouncer_port }}  dbname={{bdr_database}} user={{postgres_user}}"
       queries: