Merge pull request #156 from nhosoi/RHELPLAN-73804

Fix issues found by linters - enable all tests on all repos - remove suppressions

.ansible-lint

@@ -1,2 +1,3 @@
 warn_list:  # or 'skip_list' to silence them completely                       │
   - '106'  # Role name {} does not match ``^[a-z][a-z0-9_]+$`` pattern
+  - '306'  # Shells that use pipes should set the pipefail option

.github/workflows/ansible-centos7.yml

@@ -6,7 +6,8 @@ jobs:
   build:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v2
+      - name: checkout PR
+        uses: actions/checkout@v2
 
       - name: ansible check with centos:7
         uses: roles-ansible/check-ansible-centos-centos7-action@master

.github/workflows/ansible-centos8.yml

@@ -6,7 +6,8 @@ jobs:
   build:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v2
+      - name: checkout PR
+        uses: actions/checkout@v2
 
       - name: ansible check with centos:8
         uses: roles-ansible/check-ansible-centos-centos8-action@master

.github/workflows/ansible-debian-buster.yml

@@ -7,7 +7,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       # Important: This sets up your GITHUB_WORKSPACE environment variable
-    - uses: actions/checkout@v2
+      - name: checkout PR
+        uses: actions/checkout@v2
 
       - name: ansible check with debian:buster (10)
         uses: roles-ansible/check-ansible-debian-buster-action@master

.github/workflows/ansible-debian-stretch.yml

@@ -7,7 +7,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       # Important: This sets up your GITHUB_WORKSPACE environment variable
-    - uses: actions/checkout@v2
+      - name: checkout PR
+        uses: actions/checkout@v2
 
       - name: ansible check with debian:stretch (9)
         uses: roles-ansible/check-ansible-debian-stretch-action@master

.github/workflows/ansible-debian.yml

@@ -7,7 +7,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       # Important: This sets up your GITHUB_WORKSPACE environment variable
-    - uses: actions/checkout@v2
+      - name: checkout PR
+        uses: actions/checkout@v2
 
       - name: ansible check with debian:latest
         uses: roles-ansible/check-ansible-debian-latest-action@master

.github/workflows/ansible-fedora.yml

@@ -7,7 +7,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       # Important: This sets up your GITHUB_WORKSPACE environment variable
-    - uses: actions/checkout@v2
+      - name: checkout PR
+        uses: actions/checkout@v2
 
       - name: ansible check with fedora:latest
         uses: roles-ansible/check-ansible-fedora-latest-action@master

.github/workflows/ansible-lint.yml

@@ -3,32 +3,35 @@ name: Ansible Lint  # feel free to pick your own name
 on: [push, pull_request]
 
 jobs:
-#  test-ansible28:
+  # test-ansible28:
-#    runs-on: ubuntu-latest
+  #   runs-on: ubuntu-latest
-#    steps:
+  #   steps:
-#    - uses: actions/checkout@v2
+  #     - name: checkout PR
-#    - name: Lint Ansible Playbook
+  #       uses: actions/checkout@v2
-#      uses: ansible/ansible-lint-action@master
+  #     - name: Lint Ansible Playbook
-#      with:
+  #       uses: ansible/ansible-lint-action@master
-#        targets: "tests/test_*.yml"
+  #       with:
-#        override-deps: |
+  #         targets: "tests/test_*.yml"
-#          ansible==2.8
+  #         override-deps: |
-#        args: ""
+  #           ansible==2.8
-#  test-ansible29:
+  #         args: ""
-#    runs-on: ubuntu-latest
+  # test-ansible29:
-#    steps:
+  #   runs-on: ubuntu-latest
-#    - uses: actions/checkout@v2
+  #   steps:
-#    - name: Lint Ansible Playbook
+  #     - name: checkout PR
-#      uses: ansible/ansible-lint-action@master
+  #       uses: actions/checkout@v2
-#      with:
+  #     - name: Lint Ansible Playbook
-#        targets: "tests/test_*.yml"
+  #       uses: ansible/ansible-lint-action@master
-#        override-deps: |
+  #       with:
-#          ansible==2.9
+  #         targets: "tests/test_*.yml"
-#        args: ""
+  #         override-deps: |
+  #           ansible==2.9
+  #         args: ""
   test-ansible210:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
+      - name: checkout PR
+        uses: actions/checkout@v2
       - name: Lint Ansible Playbook
         uses: ansible/ansible-lint-action@master
         with:

.github/workflows/ansible-ubuntu.yml

@@ -7,7 +7,8 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       # Important: This sets up your GITHUB_WORKSPACE environment variable
-    - uses: actions/checkout@v2
+      - name: checkout PR
+        uses: actions/checkout@v2
 
       - name: ansible check with ubuntu:latest
         uses: roles-ansible/check-ansible-ubuntu-latest-action@master

.yamllint.yaml

@@ -1,21 +0,0 @@
----
-# Based on ansible-lint config
-extends: default
-
-rules:
-  braces: {max-spaces-inside: 1, level: error}
-  brackets: {max-spaces-inside: 1, level: error}
-  colons: {max-spaces-after: -1, level: error}
-  commas: {max-spaces-after: -1, level: error}
-  comments: disable
-  comments-indentation: disable
-  document-start: disable
-  empty-lines: {max: 3, level: error}
-  hyphens: {level: error}
-  indentation: disable
-  key-duplicates: enable
-  line-length: disable
-  new-line-at-end-of-file: disable
-  new-lines: {type: unix}
-  trailing-spaces: disable
-  truthy: disable

.yamllint.yml

@@ -0,0 +1,18 @@
+# SPDX-License-Identifier: MIT
+---
+extends: .yamllint_defaults.yml
+# possible customizations over the base yamllint config
+# skip the yaml files in the /tests/ directory
+# NOTE: If you want to customize `ignore` you'll have to
+# copy in all of the config from .yamllint.yml, then
+# add your own - so if you want to just add /tests/ to
+# be ignored, you'll have to add the ignores from the base
+ignore: |
+  /.tox/
+  /.github/
+# /tests/
+# skip checking line length
+# NOTE: the above does not apply to `rules` - you do not
+# have to copy all of the rules from the base config
+rules:
+  line-length: disable

.yamllint_defaults.yml

@@ -0,0 +1,16 @@
+# SPDX-License-Identifier: MIT
+---
+ignore: |
+  /.tox/
+extends: default
+rules:
+  braces:
+    max-spaces-inside: 1
+    level: error
+  brackets:
+    max-spaces-inside: 1
+    level: error
+  truthy:
+    allowed-values: ["yes", "no", "true", "false"]
+    level: error
+  document-start: disable

handlers/main.yml

@@ -19,10 +19,15 @@
 #  https://www.ibm.com/developerworks/community/blogs/brian/entry/scripting_the_stop_and_restart_of_src_controlled_processes_on_aix6
 - name: Reload sshd Service (AIX)
   shell: |
+    set -eu
+    if set -o | grep pipefail 2>&1 /dev/null ; then
+      set -o pipefail
+    fi
     stopsrc -s sshd
     until $(lssrc -s sshd | grep -q inoperative); do sleep 1; done
     startsrc -s sshd
   listen: reload_sshd
+  changed_when: false
   when:
     - sshd_allow_reload|bool
     - ansible_os_family == 'AIX'

tasks/install.yml

@@ -50,14 +50,19 @@
       {% endif %}
   block:
     - name: Make sure hostkeys are available
-      shell: >
+      shell: |
+        set -eu
+        if set -o | grep pipefail 2>&1 /dev/null ; then
+          set -o pipefail
+        fi
         {% if sshd_sysconfig %}
-          source /etc/sysconfig/sshd;
+          source /etc/sysconfig/sshd
         {% endif %}
         ssh-keygen -q -t {{ item | regex_search('(rsa|dsa|ecdsa|ed25519)') }} -f {{ item }} -C '' -N ''
       args:
         creates: "{{ item }}"
       loop: "{{ __sshd_verify_hostkeys | from_json | list }}"
+      changed_when: false
 
     - name: Make sure private hostkeys have expected permissions
       file:
@@ -75,14 +80,15 @@
       tempfile:
         state: directory
       register: sshd_test_hostkey
-      changed_when: False
+      changed_when: false
       when:
         - __sshd_hostkeys_from_config | from_json == []
         - sshd_config_file != "/etc/ssh/sshd_config"
 
     - name: Generate temporary hostkey
-      shell: "ssh-keygen -q -t rsa -f {{ sshd_test_hostkey.path }}/rsa_key -C '' -N ''"
+      command: >
-      changed_when: False
+        ssh-keygen -q -t rsa -f '{{ sshd_test_hostkey.path }}/rsa_key' -C '' -N ''
+      changed_when: false
       when: sshd_test_hostkey.path is defined
 
     - name: Make sure sshd runtime directory is present
@@ -119,7 +125,7 @@
       file:
         path: "{{ sshd_test_hostkey.path }}"
         state: absent
-      changed_when: False
+      changed_when: false
       when: sshd_test_hostkey.path is defined
 
 - name: Install systemd service files

tests/tasks/backup.yml

@@ -6,17 +6,21 @@
   tempfile:
     state: directory
   register: __sshd_test_backup
-  changed_when: False
+  changed_when: false
   when:
     - sshd_test_backup_skip is not defined
 
 - name: Backup files
-  shell: >
+  shell: |
+    if set -o | grep pipefail 2>&1 /dev/null ; then
+      set -o pipefail
+    fi
+    set -eu
     if test -f {{ item }}; then
-      mkdir -p {{ __sshd_test_backup.path }}/$(dirname {{ item }});
+      mkdir -p {{ __sshd_test_backup.path }}/$(dirname {{ item }})
       cp {{ item }} {{ __sshd_test_backup.path }}/$(dirname {{ item }})
     fi
-  changed_when: False
+  changed_when: false
   loop: "{{ __sshd_test_backup_files | d([]) }}"
   when:
     - __sshd_test_backup is defined

tests/tasks/restore.yml

@@ -1,12 +1,16 @@
 ---
 - name: Restore backed up files and remove what was not present
-  shell: >
+  shell: |
+    set -eu
+    if set -o | grep pipefail 2>&1 /dev/null ; then
+      set -o pipefail
+    fi
     if test -f {{ __sshd_test_backup.path }}/{{ item }}; then
       cp {{ __sshd_test_backup.path }}/{{ item }} $(dirname {{ item }})
     elif test -f {{ item }}; then
       rm {{ item }}
     fi
-  changed_when: False
+  changed_when: false
   loop: "{{ __sshd_test_backup_files | d([]) }}"
   when:
     - __sshd_test_backup is defined
@@ -16,7 +20,7 @@
   file:
     path: "{{ __sshd_test_backup.path }}"
     state: absent
-  changed_when: False
+  changed_when: false
   when:
     - __sshd_test_backup is defined
     - __sshd_test_backup.path is defined
@@ -25,7 +29,7 @@
   service:
     name: sshd
     state: reloaded
-  changed_when: False
+  changed_when: false
   when:
     - __sshd_test_backup is defined
     - ansible_virtualization_type|default(None) != 'docker'

tests/tests_hostkeys.yml

@@ -54,11 +54,13 @@
             src: "{{ main_sshd_config }}"
           register: config
 
-      - stat:
+        - name: Get stat of private key
+          stat:
             path: /tmp/ssh_host_rsa_key2
           register: privkey
 
-      - stat:
+        - name: Get stat of public key
+          stat:
             path: /tmp/ssh_host_rsa_key2.pub
           register: pubkey
 

tests/tests_os_defaults.yml

@@ -11,22 +11,24 @@
       include_tasks: tasks/backup.yml
 
     - name: Show effective configuration before running role (system defaults)
-    shell: >
+      shell: |
+        set -eu
+        if set -o | grep pipefail 2>&1 /dev/null ; then
+          set -o pipefail
+        fi
         if test ! -f /etc/ssh/ssh_host_rsa_key; then
           ssh-keygen -q -t rsa -f /etc/ssh/ssh_host_rsa_key -C '' -N ''
-      fi;
+        fi
         sshd -T
       register: runtime_before
+      changed_when: false
     - name: Configure sshd
       include_role:
         name: ansible-sshd
     - name: Show effective configuration after running role (role defaults)
-    shell: sshd -T
+      command: sshd -T
       register: runtime_after
-  - debug:
+      changed_when: false
-      var: ansible_facts['distribution']
-  - debug:
-      var: ansible_facts['distribution_major_version']
     - name: Check that the effective configuration did not change from OS defaults
       assert:
         that:

tests/tests_sshd_enable.yml

@@ -25,12 +25,17 @@
       register: config
 
     - name: Print effective configuration
-    shell: >
+      shell: |
+        set -eu
+        if set -o | grep pipefail 2>&1 /dev/null ; then
+          set -o pipefail
+        fi
         if test ! -f /etc/ssh/ssh_host_rsa_key; then
           ssh-keygen -q -t rsa -f /etc/ssh/ssh_host_rsa_key -C '' -N ''
-      fi;
+        fi
         sshd -T
       register: runtime
+      changed_when: false
 
     - name: Check the options were not applied
       # note, the options are in lower-case here

tests/tests_sysconfig.yml

@@ -28,18 +28,28 @@
 
         - name: Evaluate sysconfig similarly as systemd
           shell: |
+            set -eu
+            if set -o | grep pipefail 2>&1 /dev/null ; then
+              set -o pipefail
+            fi
             source /etc/sysconfig/sshd
-          echo "CP=|$CRYPTO_POLICY|"
+            echo "CP=|${CRYPTO_POLICY:-}|"
-          echo "RNG=|$SSH_USE_STRONG_RNG|"
+            echo "RNG=|${SSH_USE_STRONG_RNG:-}|"
           register: evaluation
+          changed_when: false
 
         - name: Evaluate sysconfig similarly as systemd on RHEL 8
           shell: |
+            set -eu
+            if set -o | grep pipefail 2>&1 /dev/null ; then
+              set -o pipefail
+            fi
             source /etc/crypto-policies/back-ends/opensshserver.config
             source /etc/sysconfig/sshd
-          echo "CP=|$CRYPTO_POLICY|"
+            echo "CP=|${CRYPTO_POLICY:-}|"
-          echo "RNG=|$SSH_USE_STRONG_RNG|"
+            echo "RNG=|${SSH_USE_STRONG_RNG:-}|"
           register: evaluation8
+          changed_when: false
           when:
             - ansible_facts['os_family'] == "RedHat"
             - ansible_facts['distribution_major_version'] == "8"