Merge 6782f88e39 into 795d8e34e7

ci: replace freebsd 13.2 with 13.3

The devel version of ansible-test has been updated to include support for FreeBSD 13.3, so this change swaps out 13.2 accordingly.
See ansible-collections/news-for-maintainers#67

Reviewed-by: Hideki Saito <saito@fgrep.org>

.azure-pipelines/azure-pipelines.yml

@@ -178,8 +178,8 @@ stages:
           targets:
             - name: RHEL 9.3
               test: rhel/9.3
-            - name: FreeBSD 13.2
-              test: freebsd/13.2
+            - name: FreeBSD 13.3
+              test: freebsd/13.3
   - stage: Remote_2_16
     displayName: Remote 2.16
     dependsOn: []

changelogs/fragments/197-acl-fix-performance.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - acl - Fix module performance (https://github.com/ansible-collections/ansible.posix/pull/197).

plugins/modules/acl.py

@@ -141,9 +141,13 @@ acl:
 
 import os
 import platform
+import fcntl
 
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils._text import to_native
+from ansible.module_utils.compat import selectors
+from ansible.module_utils.common.text.converters import to_native, to_text, to_bytes
+from ansible.module_utils.six import b
 
 
 def split_entry(entry):
@@ -223,7 +227,7 @@ def build_command(module, mode, path, follow, default, recursive, recalculate_ma
     return cmd
 
 
-def acl_changed(module, cmd):
+def acl_changed(module, cmd, check_rc=True):
     '''Returns true if the provided command affects the existing ACLs, false otherwise.'''
     # FreeBSD do not have a --test flag, so by default, it is safer to always say "true"
     if platform.system().lower() == 'freebsd':
@@ -231,11 +235,63 @@ def acl_changed(module, cmd):
 
     cmd = cmd[:]  # lists are mutables so cmd would be overwritten without this
     cmd.insert(1, '--test')
-    lines = run_acl(module, cmd)
+    module._acl_changed = False
 
-    for line in lines:
-        if not line.endswith('*,*'):
-            return True
+    def _process_stdout_from_pipe(proc, _acl_module=module):
+        stdout = b''
+        try:
+            selector = selectors.DefaultSelector()
+        except (IOError, OSError):
+            # Failed to detect default selector for the given platform
+            # Select PollSelector which is supported by major platforms
+            selector = selectors.PollSelector()
+
+        selector.register(proc.stdout, selectors.EVENT_READ)
+        if os.name == 'posix':
+            fcntl.fcntl(proc.stdout.fileno(), fcntl.F_SETFL, fcntl.fcntl(proc.stdout.fileno(), fcntl.F_GETFL) | os.O_NONBLOCK)
+
+        while True:
+            events = selector.select(1)
+            for key, event in events:
+                b_chunk = key.fileobj.read()
+                if b_chunk == b(''):
+                    selector.unregister(key.fileobj)
+                if key.fileobj == proc.stdout:
+                    stdout = b_chunk
+                if _acl_module._acl_changed:
+                    continue
+            lines = []
+            for l in stdout.splitlines():
+                lines.append(l.strip())
+            for line in lines:
+                if not line.endswith(b'*,*'):
+                    proc.terminate()
+                    _acl_module._acl_changed = True
+                    proc.returncode = 0
+
+            # only break out if no pipes are left to read or
+            # the pipes are completely read and
+            # the process is terminated
+            if (not events or not selector.get_map()) and proc.poll() is not None:
+                break
+            # No pipes are left to read but process is not yet terminated
+            # Only then it is safe to wait for the process to be finished
+            # NOTE: Actually proc.poll() is always None here if no selectors are left
+            elif not selector.get_map() and proc.poll() is None:
+                proc.wait()
+                # The process is terminated. Since no pipes to read from are
+                # left, there is no need to call select() again.
+                break
+
+    try:
+        (rc, out, err) = module.run_command(
+            cmd, check_rc=check_rc,
+            before_communicate_callback=_process_stdout_from_pipe)
+    except Exception as e:
+        module.fail_json(msg=to_native(e))
+
+    if module._acl_changed:
+        return True
     return False
 
 
@@ -356,7 +412,10 @@ def main():
 
         if changed and not module.check_mode:
             run_acl(module, command)
-        msg = "%s is present" % entry
+        if recursive:
+            msg = "%s is present recursively" % entry
+        else:
+            msg = "%s is present" % entry
 
     elif state == 'absent':
         entry = build_entry(etype, entity, use_nfsv4_acls)
@@ -368,15 +427,23 @@ def main():
 
         if changed and not module.check_mode:
             run_acl(module, command, False)
-        msg = "%s is absent" % entry
+        if recursive:
+            msg = "%s is absent recursively" % entry
+        else:
+            msg = "%s is absent" % entry
 
     elif state == 'query':
         msg = "current acl"
 
-    acl = run_acl(
-        module,
-        build_command(module, 'get', path, follow, default, recursive, recalculate_mask)
-    )
+    if recursive:
+        acl = []
+        warn = "Not showing resulting acls in the recursive mode"
+        module.exit_json(changed=changed, msg=msg, acl=acl, warnings=warn)
+    else:
+        acl = run_acl(
+            module,
+            build_command(module, 'get', path, follow, default, recursive, recalculate_mask)
+        )
 
     module.exit_json(changed=changed, msg=msg, acl=acl)
 

tests/integration/targets/selinux/tasks/selogin.yml

@@ -5,7 +5,7 @@
 - name: Attempt to add mapping without 'seuser'
   register: selogin_error
   ignore_errors: true
-  community.general.system.selogin:
+  community.general.selogin:
     login: seuser
 - name: Verify failure
   ansible.builtin.assert:
@@ -19,7 +19,7 @@
     - false
     - true
     - false
-  community.general.system.selogin:
+  community.general.selogin:
     login: seuser
     seuser: staff_u
 - name: New mapping- verify functionality and check_mode
@@ -37,7 +37,7 @@
     - false
     - true
     - false
-  community.general.system.selogin:
+  community.general.selogin:
     login: seuser
     seuser: user_u
 - name: Changed mapping- verify functionality and check_mode
@@ -55,7 +55,7 @@
     - false
     - true
     - false
-  community.general.system.selogin:
+  community.general.selogin:
     login: seuser
     state: absent
 - name: Delete mapping- verify functionality and check_mode

tests/unit/compat/mock.py

@@ -64,8 +64,7 @@ if sys.version_info >= (3,) and sys.version_info < (3, 4, 4):
             # newline that our naive format() added
             data_as_list[-1] = data_as_list[-1][:-1]
 
-        for line in data_as_list:
-            yield line
+        yield from data_as_list
 
     def mock_open(mock=None, read_data=''):
         """
@@ -93,8 +92,7 @@ if sys.version_info >= (3,) and sys.version_info < (3, 4, 4):
             if handle.readline.return_value is not None:
                 while True:
                     yield handle.readline.return_value
-            for line in _data:
-                yield line
+            yield from _data
 
         global file_spec
         if file_spec is None: