From b323e5006873f5f5c043763092a757ca4542b7ff Mon Sep 17 00:00:00 2001
From: Roberto Alfieri <ralfieri@redhat.com>
Date: Thu, 5 Oct 2023 21:41:19 +0200
Subject: [PATCH] Make `edpm_iscsid` role compliant with ansible-lint
 `production` profile

- Fixed a couple of checks against boolean using a more "ansible" way
- Added "mode" in `file` and `copy` tasks to hardening security
- Replaced the use of `podman run` inside `command` tasks with `podman_container` module from `containers.podman` collection
- Implemented an handler task instead of a standard task with a `when: $foo.changed` condition as for ansible best practices
- Replaced `systemctl` usage inside a `command` module with `ansible.builtin.service_facts` module and adapted `when` conditions for systemd task

Signed-off-by: Roberto Alfieri <ralfieri@redhat.com>
---
 roles/edpm_iscsid/handlers/main.yml   |  8 ++++
 roles/edpm_iscsid/tasks/configure.yml | 69 +++++++++++++--------------
 roles/edpm_iscsid/tasks/install.yml   | 11 ++---
 3 files changed, 45 insertions(+), 43 deletions(-)
 create mode 100644 roles/edpm_iscsid/handlers/main.yml

diff --git a/roles/edpm_iscsid/handlers/main.yml b/roles/edpm_iscsid/handlers/main.yml
new file mode 100644
index 000000000..3e0ca800b
--- /dev/null
+++ b/roles/edpm_iscsid/handlers/main.yml
@@ -0,0 +1,8 @@
+---
+
+- name: Record the iscsid container restart is required
+  become: true
+  ansible.builtin.file:
+    path: /etc/iscsi/.iscsid_restart_required
+    state: touch
+    mode: "0600"
diff --git a/roles/edpm_iscsid/tasks/configure.yml b/roles/edpm_iscsid/tasks/configure.yml
index 06e788b7a..ee95f087a 100644
--- a/roles/edpm_iscsid/tasks/configure.yml
+++ b/roles/edpm_iscsid/tasks/configure.yml
@@ -26,57 +26,52 @@
 # .initiator_reset sentinel file.
 
 - name: Ensure the system has a unique IQN
-  when: initiator_reset_state.stat.exists == False
+  when: not initiator_reset_state.stat.exists
   become: true
   block:
 
-  - name: Generate a unique IQN
-    ansible.builtin.command: podman run -ti --rm --name iscsid_config {{ edpm_iscsid_image }} /usr/sbin/iscsi-iname
-    register: iscsi_iname
+    - name: Generate a unique IQN
+      containers.podman.podman_container:
+        name: iscsid_config
+        image: "{{ edpm_iscsid_image }}"
+        rm: true
+        tty: true
+        detach: false
+        command: /usr/sbin/iscsi-iname
+      register: iscsi_iname
 
-  - name: Save the new IQN
-    ansible.builtin.copy:
-      dest: /etc/iscsi/initiatorname.iscsi
-      content: "InitiatorName={{ iscsi_iname.stdout }}"
+    - name: Save the new IQN
+      ansible.builtin.copy:
+        dest: /etc/iscsi/initiatorname.iscsi
+        mode: "0644"
+        content: "InitiatorName={{ iscsi_iname.stdout | trim }}"
 
-  - name: Record the IQN has been reset
-    ansible.builtin.file:
-      path: /etc/iscsi/.initiator_reset
-      state: touch
+    - name: Record the IQN has been reset
+      ansible.builtin.file:
+        path: /etc/iscsi/.initiator_reset
+        mode: "0600"
+        state: touch
 
-- name: Check if /etc/iscsi/iscsid.conf exists
+- name: Check if /etc/iscsi/iscsid.conf exists on the host
   ansible.builtin.stat:
     path: /etc/iscsi/iscsid.conf
   register: result
 
-- name: Create /etc/iscsi/iscsid.conf if necessary
-  when: result.stat.exists == False
-  become: true
-  block:
-
-  - name: Fetch iscsid.conf from the iscsid container
-    ansible.builtin.command: podman run -ti --rm --name iscsid_config {{ edpm_iscsid_image }} cat /etc/iscsi/iscsid.conf
-    register: iscsid_conf
-
-  - name: Create a local copy of iscsid.conf
-    ansible.builtin.copy:
-      dest: /etc/iscsi/iscsid.conf
-      content: "{{ iscsid_conf.stdout }}"
-      mode: 0600
-    check_mode: false
+- name: Copy iscsid.conf from the iscsid container to the host
+  when: not result.stat.exists
+  containers.podman.podman_container:
+    name: iscsid_config
+    image: "{{ edpm_iscsid_image }}"
+    rm: true
+    command: cp /etc/iscsi/iscsid.conf /host/etc/iscsi/
+    volume:
+      - /etc/iscsi:/host/etc/iscsi
 
 - name: Write CHAP algorithms
+  become: true
   ansible.builtin.lineinfile:
     path: "/etc/iscsi/iscsid.conf"
     line: "node.session.auth.chap_algs = {{ edpm_iscsid_chap_algs }}"
     regexp: "^node.session.auth.chap_algs"
     insertafter: "^#node.session.auth.chap.algs"
-  become: true
-  register: modify_stat
-
-- name: Record the iscsid container restart is required
-  when : modify_stat.changed
-  become: true
-  ansible.builtin.file:
-   path: /etc/iscsi/.iscsid_restart_required
-   state: touch
+  notify: Record the iscsid container restart is required
diff --git a/roles/edpm_iscsid/tasks/install.yml b/roles/edpm_iscsid/tasks/install.yml
index 30da6364e..f9afcbec7 100644
--- a/roles/edpm_iscsid/tasks/install.yml
+++ b/roles/edpm_iscsid/tasks/install.yml
@@ -43,10 +43,8 @@
         enabled: false
       when: stat_iscsid_socket.stat.exists
 
-    - name: Check if iscsi.service is enabled
-      ansible.builtin.command: systemctl is-enabled --quiet iscsi.service
-      failed_when: false
-      register: iscsi_service_enabled_result
+    - name: Gather services facts
+      ansible.builtin.service_facts:
 
     - name: Stop iscsi.service
       ansible.builtin.systemd:
@@ -54,5 +52,6 @@
         state: stopped
         enabled: false
       when:
-        - iscsi_service_enabled_result is changed
-        - iscsi_service_enabled_result.rc == 0
+        - ansible_facts.services["iscsi.service"] is defined
+        - ansible_facts.services["iscsi.service"]["status"] != "not-found"
+        - ansible_facts.services["iscsi.service"]["status"] == "enabled"