firewalld_info: Use module.warn (#715)

* Use module.warn to display warnings instead of module.exit_json

Fixes: #710

Signed-off-by: Abhijeet Kasurde <Akasurde@redhat.com>

changelogs/fragments/638_fix_recursive_acl.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - acl - correctly assert needed changes when pointing to a directory and recursive is set to true.

changelogs/fragments/firewalld_info_warnings.yml

@@ -0,0 +1,3 @@
+---
+minor_changes:
+    - firewalld_info - use module.warn instead of passing `warnings` to `exit_json` (https://github.com/ansible-collections/ansible.posix/issues/710).

plugins/modules/acl.py

@@ -244,16 +244,16 @@ def acl_changed(module, cmd, entry, use_nfsv4_acls=False):
     lines = run_acl(module, cmd)
     counter = 0
     for line in lines:
-        if line.endswith('*,*') and not use_nfsv4_acls:
-            return False
+        if not use_nfsv4_acls and not line.endswith('*,*'):
+            return True
         # if use_nfsv4_acls and entry is listed
         if use_nfsv4_acls and entry == line:
             counter += 1
 
     # The current 'nfs4_setfacl --test' lists a new entry,
-    #  which will be added at the top of list, followed by the existing entries.
-    # So if the entry has already been registered, the entry should be find twice.
-    if counter == 2:
+    # which will be added at the top of the list, followed by the existing entries.
+    # So if the entry has already been registered, the entry should be found twice.
+    if not use_nfsv4_acls or counter == 2:
         return False
     return True
 

plugins/modules/firewalld_info.py

@@ -333,10 +333,6 @@ def main():
     if not HAS_FIREWALLD:
         module.fail_json(msg=missing_required_lib('python-firewall'))
 
-    # If you want to show warning messages in the task running process,
-    # you can append the message to the 'warn' list.
-    warn = list()
-
     try:
         client = fw_client.FirewallClient()
 
@@ -356,8 +352,7 @@ def main():
             collect_zones = list(set(specified_zones) & set(all_zones))
             ignore_zones = list(set(specified_zones) - set(collect_zones))
             if ignore_zones:
-                warn.append(
-                    'Please note: zone:(%s) have been ignored in the gathering process.' % ','.join(ignore_zones))
+                module.warn(f'Please note: zone:({",".join(ignore_zones)}) have been ignored in the gathering process.')
         else:
             collect_zones = get_all_zones(client)
 
@@ -396,7 +391,6 @@ def main():
     result['collected_zones'] = collect_zones
     result['undefined_zones'] = ignore_zones
     result['firewalld_info'] = firewalld_info
-    result['warnings'] = warn
     module.exit_json(**result)
 
 

tests/integration/targets/acl/tasks/acl.yml

@@ -1,20 +1,6 @@
 ---
 # (c) 2017, Martin Krizek <mkrizek@redhat.com>
-
-# This file is part of Ansible
-#
-# Ansible is free software: you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation, either version 3 of the License, or
-# (at your option) any later version.
-#
-# Ansible is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
 - name: Create ansible user
   ansible.builtin.user:
@@ -43,15 +29,17 @@
 
 - name: Create ansible dir
   ansible.builtin.file:
-    path: "{{ test_dir }}"
+    path: "{{ item.path }}"
     state: directory
-    mode: "0755"
+    mode: "{{ item.mode }}"
+  loop:
+    - { path: "{{ test_dir }}", mode: "0755" }
+    - { path: "{{ test_recursive_dir }}", mode: "0755" }
 
 - name: Install acl package
   ansible.builtin.package:
     name: acl
     state: present
-
 ##############################################################################
 - name: Grant ansible user read access to a file
   ansible.posix.acl:
@@ -249,3 +237,38 @@
       - "'default:mask::rwx' in getfacl_output.stdout_lines"
       - "'default:other::r-x' in getfacl_output.stdout_lines"
       - "'default:group:{{ test_group }}:rw-' not in getfacl_output.stdout_lines"
+
+##############################################################################
+
+- name: create file
+  ansible.builtin.copy:
+    dest: "{{ test_recursive_dir }}/txt.txt"
+    mode: '0440'
+    content: "hw"
+
+- name: Change ACLs recursively
+  ansible.posix.acl:
+    path: "{{ test_recursive_dir }}"
+    entity: "{{ test_user }}"
+    etype: user
+    permissions: rX
+    state: present
+    recursive: true
+  register: output_acl_change
+
+- name: Remove ACLs recursively again
+  ansible.posix.acl:
+    path: "{{ test_recursive_dir }}"
+    entity: "{{ test_user }}"
+    etype: user
+    permissions: r
+    state: present
+    recursive: true
+  register: output_acl_remove
+
+- assert:
+    that:
+      - output_acl_change is changed
+      - output_acl_change is not failed
+      - output_acl_remove is changed
+      - output_acl_remove is not failed

tests/integration/targets/acl/tasks/main.yml

@@ -1,20 +1,6 @@
 ---
 # (c) 2017, Martin Krizek <mkrizek@redhat.com>
-
-# This file is part of Ansible
-#
-# Ansible is free software: you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation, either version 3 of the License, or
-# (at your option) any later version.
-#
-# Ansible is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
 - name: Test ACL
   vars:
@@ -22,6 +8,7 @@
     test_group: ansible_group
     test_file: "{{ output_dir }}/ansible file"
     test_dir: "{{ output_dir }}/ansible_dir/with some space"
+    test_recursive_dir: "{{ output_dir }}/recursive_dir"
   block:
     - name: Include tests task file
       ansible.builtin.include_tasks: acl.yml

tests/integration/targets/firewalld_info/tasks/run_tests_in_started.yml

@@ -20,6 +20,7 @@
 - name: Assert turn active_zones true
   ansible.builtin.assert:
     that:
+      - result.collected_zones == ['public']
 
 - name: Ensure firewalld_zones with zone list
   ansible.posix.firewalld_info:
@@ -31,3 +32,6 @@
 - name: Assert specified zones
   ansible.builtin.assert:
     that:
+      - result.collected_zones == ['public']
+      - result.undefined_zones == ['invalid_zone']
+      - '"invalid_zone" in result.warnings[0]'