Merge pull request #2817 from mtnbikenc/os_firewall-refactor

Refactor os_firewall role

4 files changed, 26 insertions, 105 deletions

diff --git a/roles/os_firewall/README.md b/roles/os_firewall/README.md
index c6c70b81d..bb7fc2384 100644
--- a/roles/os_firewall/README.md
+++ b/roles/os_firewall/README.md
@@ -31,7 +31,6 @@ Use iptables and open tcp ports 80 and 443:
 ---
 - hosts: servers
   vars:
-    os_firewall_use_firewalld: false
     os_firewall_allow:
     - service: httpd
       port: 80/tcp
@@ -46,6 +45,7 @@ Use firewalld and open tcp port 443 and close previously open tcp port 80:
 ---
 - hosts: servers
   vars:
+    os_firewall_use_firewalld: true
     os_firewall_allow:
     - service: https
       port: 443/tcp
diff --git a/roles/os_firewall/meta/main.yml b/roles/os_firewall/meta/main.yml
index 6df7c9f2b..4cfc72011 100644
--- a/roles/os_firewall/meta/main.yml
+++ b/roles/os_firewall/meta/main.yml
@@ -6,11 +6,11 @@ galaxy_info:
   license: Apache License, Version 2.0
   min_ansible_version: 1.7
   platforms:
-  - name: EL
-    versions:
-    - 7
+    - name: EL
+      versions:
+        - 7
   categories:
-  - system
+    - system
 allow_duplicates: yes
 dependencies:
-- { role: openshift_facts }
+  - role: openshift_facts
diff --git a/roles/os_firewall/tasks/firewall/firewalld.yml b/roles/os_firewall/tasks/firewall/firewalld.yml
index a5b733cb7..a6039b5f1 100644
--- a/roles/os_firewall/tasks/firewall/firewalld.yml
+++ b/roles/os_firewall/tasks/firewall/firewalld.yml
@@ -2,87 +2,44 @@
 - name: Install firewalld packages
   package: name=firewalld state=present
   when: not openshift.common.is_containerized | bool
-  register: install_result
-
-- name: Check if iptables-services is installed
-  command: rpm -q iptables-services
-  register: pkg_check
-  failed_when: pkg_check.rc > 1
-  changed_when: no
 
 - name: Ensure iptables services are not enabled
-  service:
+  systemd:
     name: "{{ item }}"
     state: stopped
     enabled: no
+    masked: yes
   with_items:
-  - iptables
-  - ip6tables
-  when: pkg_check.rc == 0
-
-- name: Reload systemd units
-  command: systemctl daemon-reload
-  when: install_result | changed
-
-- name: Determine if firewalld service masked
-  command: >
-    systemctl is-enabled firewalld
-  register: os_firewall_firewalld_masked_output
-  changed_when: false
-  failed_when: false
-
-- name: Unmask firewalld service
-  command: >
-    systemctl unmask firewalld
-  when: os_firewall_firewalld_masked_output.stdout == "masked"
+    - iptables
+    - ip6tables
+  register: task_result
+  failed_when: "task_result|failed and 'Could not find' not in task_result.msg"
 
 - name: Start and enable firewalld service
-  service:
+  systemd:
     name: firewalld
     state: started
     enabled: yes
+    masked: no
+    daemon_reload: yes
   register: result
 
 - name: need to pause here, otherwise the firewalld service starting can sometimes cause ssh to fail
   pause: seconds=10
   when: result | changed
 
-- name: Mask iptables services
-  command: systemctl mask "{{ item }}"
-  register: result
-  changed_when: "'iptables' in result.stdout"
-  with_items:
-  - iptables
-  - ip6tables
-  when: pkg_check.rc == 0
-  ignore_errors: yes
-
-# TODO: Ansible 1.9 will eliminate the need for separate firewalld tasks for
-# enabling rules and making them permanent with the immediate flag
 - name: Add firewalld allow rules
   firewalld:
     port: "{{ item.port }}"
-    permanent: false
-    state: enabled
-  with_items: "{{ os_firewall_allow }}"
-
-- name: Persist firewalld allow rules
-  firewalld:
-    port: "{{ item.port }}"
     permanent: true
+    immediate: true
     state: enabled
   with_items: "{{ os_firewall_allow }}"
 
 - name: Remove firewalld allow rules
   firewalld:
     port: "{{ item.port }}"
-    permanent: false
-    state: disabled
-  with_items: "{{ os_firewall_deny }}"
-
-- name: Persist removal of firewalld allow rules
-  firewalld:
-    port: "{{ item.port }}"
     permanent: true
+    immediate: true
     state: disabled
   with_items: "{{ os_firewall_deny }}"
diff --git a/roles/os_firewall/tasks/firewall/iptables.yml b/roles/os_firewall/tasks/firewall/iptables.yml
index 366ede8fd..704819d8a 100644
--- a/roles/os_firewall/tasks/firewall/iptables.yml
+++ b/roles/os_firewall/tasks/firewall/iptables.yml
@@ -1,64 +1,28 @@
 ---
-- name: Check if firewalld is installed
-  command: rpm -q firewalld
-  args:
-    # Disables the following warning:
-    # Consider using yum, dnf or zypper module rather than running rpm
-    warn: no
-  register: pkg_check
-  failed_when: pkg_check.rc > 1
-  changed_when: no
 
 - name: Ensure firewalld service is not enabled
-  service:
+  systemd:
     name: firewalld
     state: stopped
     enabled: no
-  when: pkg_check.rc == 0
-
-# TODO: submit PR upstream to add mask/unmask to service module
-- name: Mask firewalld service
-  command: systemctl mask firewalld
-  register: result
-  changed_when: "'firewalld' in result.stdout"
-  when: pkg_check.rc == 0
-  ignore_errors: yes
+    masked: yes
+  register: task_result
+  failed_when: "task_result|failed and 'Could not find' not in task_result.msg"
 
 - name: Install iptables packages
   package: name={{ item }} state=present
   with_items:
-  - iptables
-  - iptables-services
-  register: install_result
+    - iptables
+    - iptables-services
   when: not openshift.common.is_atomic | bool
 
-- name: Reload systemd units
-  command: systemctl daemon-reload
-  when: install_result | changed
-
-- name: Determine if iptables service masked
-  command: >
-    systemctl is-enabled {{ item }}
-  with_items:
-  - iptables
-  - ip6tables
-  register: os_firewall_iptables_masked_output
-  changed_when: false
-  failed_when: false
-
-- name: Unmask iptables service
-  command: >
-    systemctl unmask {{ item }}
-  with_items:
-  - iptables
-  - ip6tables
-  when: "'masked' in os_firewall_iptables_masked_output.results | map(attribute='stdout')"
-
 - name: Start and enable iptables service
-  service:
+  systemd:
     name: iptables
     state: started
     enabled: yes
+    masked: no
+    daemon_reload: yes
   register: result
 
 - name: need to pause here, otherwise the iptables service starting can sometimes cause ssh to fail