acl: correctly assert needed changes when recursive is true
SUMMARY
Right now, when setting recursive ACLs on a directory, all files in the directory are tested to check if a change is needed. If a single file has expected ACLs already set, then the test returns False and no changes are applied.
Fixes#592
ISSUE TYPE
Bugfix Pull Request
COMPONENT NAME
acl
ADDITIONAL INFORMATION
I'm very much a beginner in Python, so any criticism is welcome.
Reviewed-by: Hideki Saito <saito@fgrep.org>
(cherry picked from commit c4b1c2b0fb)
Co-authored-by: centosinfra-prod-github-app[bot] <161850885+centosinfra-prod-github-app[bot]@users.noreply.github.com>
Fix all deprecated module_utils imports before ansible-core 2.24 removal
SUMMARY
Fixes all deprecated ansible.module_utils imports across the entire collection that will be removed in ansible-core 2.24.
This PR comprehensively addresses deprecation warnings reported in #686 by updating import statements in 20 files to use the new recommended import paths, and removes 8 unused test utility files that contained deprecated imports.
Deprecated imports replaced:
Deprecated import
Replacement
ansible.module_utils._text
ansible.module_utils.common.text.converters
ansible.module_utils.common._collections_compat
collections.abc
ansible.module_utils.six.moves.shlex_quote
shlex.quote
ansible.module_utils.six.moves.reduce
functools.reduce
ansible.module_utils.six.moves.urllib.parse.urlparse
urllib.parse.urlparse
ansible.module_utils.six.string_types
basestring/str (Python 2/3 compatible)
ansible.module_utils.six.text_type
str
ansible.module_utils.six.PY3
Removed (simplified Python 2/3 conditionals)
ansible.module_utils.six.with_metaclass
Native metaclass= syntax
ansible.module_utils.six.iteritems
dict.items()
Files fixed (20 files, 1 commit per file for easier review):
plugins/action/patch.py
plugins/action/synchronize.py
plugins/callback/cgroup_perf_recap.py
plugins/callback/json.py
plugins/callback/jsonl.py
plugins/callback/profile_roles.py
plugins/callback/profile_tasks.py
plugins/modules/acl.py
plugins/modules/authorized_key.py
plugins/modules/firewalld_info.py
plugins/modules/mount.py
plugins/modules/patch.py
plugins/modules/rhel_rpm_ostree.py
plugins/modules/rpm_ostree_upgrade.py
plugins/modules/seboolean.py
plugins/modules/synchronize.py
plugins/modules/sysctl.py
plugins/shell/csh.py
plugins/shell/fish.py
tests/unit/modules/system/test_mount.py
Files deleted (8 unused test utility files):
These files are dead code - none of them are imported or used anywhere in the test suite or the collection. Removing them also addresses Python 2.7 compatibility concerns raised in code review, as several contained deprecated imports that would be incorrect to fix for Python 2.
tests/unit/compat/builtins.py
tests/unit/mock/loader.py
tests/unit/mock/path.py
tests/unit/mock/procenv.py
tests/unit/mock/vault_helper.py
tests/unit/mock/yaml_helper.py
tests/unit/modules/conftest.py
tests/unit/modules/utils.py
Completeness verified with:
git grep -n -P '_compat|utils._text|utils.six' -- '*.py' | grep -v yml
This command returns no results, confirming all deprecated imports have been replaced.
Notes on Python 2.7 compatibility:
For modules that may run on Python 2.7 managed hosts (e.g., authorized_key.py, synchronize.py, sysctl.py), Python 2/3 compatible fallbacks were used instead of direct Python 3 replacements:
authorized_key.py: try/except ImportError for urllib.parse.urlparse (falls back to urlparse on Python 2)
synchronize.py: try/except ImportError for shlex.quote (falls back to pipes.quote on Python 2)
sysctl.py: uses sys.version_info to set string_types to str on Python 3 (basestring on Python 2)
Also removes corresponding pylint:ansible-bad-import-from entries from tests/sanity/ignore-2.21.txt and tests/sanity/ignore-2.22.txt where applicable.
Fixes#686
ISSUE TYPE
Bugfix Pull Request
ADDITIONAL INFORMATION
Approach:
Each file is fixed in a separate commit for easier code review. The changelog fragment is added in a final commit. Corresponding pylint:ansible-bad-import-from ignore entries in tests/sanity/ignore-2.21.txt and tests/sanity/ignore-2.22.txt are removed in the same commit as the file fix (or the file removal commit).
CI results:
All 59 checks passing (Azure Pipelines sanity, units, lint, Docker, Remote across ansible-core 2.17 through devel, and Zuul ansible/check).
Reviewed-by: Felix Fontein <felix@fontein.de>
Reviewed-by: Pavel Bar
Reviewed-by: Abhijeet Kasurde
(cherry picked from commit 2022c1bd86)
Co-authored-by: centosinfra-prod-github-app[bot] <161850885+centosinfra-prod-github-app[bot]@users.noreply.github.com>
Fixes#462 notice permission denied on authorized_key module
SUMMARY
As of right now the authorized_key module does not notice on an "absent" if a authorized_keys file is simply not readable to the executing user. I am trying to fix that
ISSUE TYPE
Bugfix Pull Request
COMPONENT NAME
authorized_key
ADDITIONAL INFORMATION
Execute as a user that does not have access to the root users authorized keys file
- name: Delete key from root user
ansible.posix.authorized_key:
state: absent
user: root
key: ssh-rsa xxxxxxxx
- name: Delete key from root user
become: true
ansible.posix.authorized_key:
state: absent
user: root
key: ssh-rsa xxxxxxxx
The one without become will succeed before my change and will fail with a permission denied error after my change. The 2nd task will actually remove a key from root user if become privileges are available for the executing user
Reviewed-by: Brian Coca
Reviewed-by: Klaas Demter
Reviewed-by: Felix Fontein <felix@fontein.de>
Reviewed-by: Hideki Saito <saito@fgrep.org>
(cherry picked from commit 72a6eb9729)
Co-authored-by: softwarefactory-project-zuul[bot] <33884098+softwarefactory-project-zuul[bot]@users.noreply.github.com>
Use module.warn() instead of returning warnings
SUMMARY
Returning warnings as warnings has been deprecated.
Ref: #635.
ISSUE TYPE
Bugfix Pull Request
COMPONENT NAME
firewalld_info
mount
Reviewed-by: Hideki Saito <saito@fgrep.org>
(cherry picked from commit 6da1331018)
Co-authored-by: softwarefactory-project-zuul[bot] <33884098+softwarefactory-project-zuul[bot]@users.noreply.github.com>
[Breaking Change] [firewalld] Change type of icmp_block_inversion option from str to bool
SUMMARY
Changed the type of icmp_block_inversion option from str to bool
Fixes#586
ISSUE TYPE
Bugfix Pull Request
COMPONENT NAME
ansible.posix.firewalld
ADDITIONAL INFORMATION
Related #582 and #584
Reviewed-by: Adam Miller <admiller@redhat.com>
Reviewed-by: Andrew Klychkov <aklychko@redhat.com>
Remove comment from fstab entry on updating.
SUMMARY
Fix#595.
ISSUE TYPE
Bugfix Pull Request
COMPONENT NAME
ansible.posix.mount
ADDITIONAL INFORMATION
Reviewed-by: Hideki Saito <saito@fgrep.org>
Reviewed-by: Vladimir Botka <vbotka@gmail.com>
authorized_key: Allow local path to a key
SUMMARY
Add option to specify an absolute path to file with SSH key(s) for authorized_key
ISSUE TYPE
Feature Pull Request
COMPONENT NAME
authorized_key
ADDITIONAL INFORMATION
Before this change you would need to get key using ansible.builtin.slurp or something like ansible.builtin.command: cat <file> with register
I tried to keep it as simple as possible
# Now this is possible
- name: Set authorized keys taken from path
ansible.posix.authorized_key:
user: charlie
state: present
key: /home/charlie/.ssh/id_rsa.pub
Reviewed-by: Hideki Saito <saito@fgrep.org>
Reviewed-by: alexander
(feat) add no_log option for 'opts' parameter
SUMMARY
Allows you to set no_log on just the opts parameter.
This is useful for CIFS/SMB mounts that would otherwise leak secrets.
Adds feature from issue: . #497
ISSUE TYPE
Feature Pull Request
COMPONENT NAME
mount
Reviewed-by: Hideki Saito <saito@fgrep.org>
Firewalld: Add functionality to set forwarding
SUMMARY
Adds firewalld functionality to do the equivalent of firewall-cmd --add-forwarding --zone={zone}.
Functionality is exactly analogous to the firewall-cmd --add-masquerade --zone={zone} already present.
Fixes#529
ISSUE TYPE
Feature Pull Request
COMPONENT NAME
firewalld
ADDITIONAL INFORMATION
Usage:
- ansible.posix.firewalld:
forward: true
state: enabled
permanent: true
zone: internal
Reviewed-by: Abhijeet Kasurde
Reviewed-by: Hideki Saito <saito@fgrep.org>
Sometimes it's necessary to configure SELinux before it's enabled on the
system. There's `ignore_selinux_state` which should allow it. Before
this change `seboolean` module failed on SELinux disabled system even
with `ignore_selinux_state: true` and SELinux policy installed while
`semanage boolean` worked as expected:
$ ansible -i 192.168.121.153, -m seboolean -a "name=ssh_sysadm_login state=on ignore_selinux_state=true" all
192.168.121.153 | FAILED! => {
"ansible_facts": {
"discovered_interpreter_python": "/usr/bin/python3"
},
"changed": false,
"msg": "Failed to get list of boolean names"
}
$ ssh root@192.168.121.153 semanage boolean -l | grep ssh_sysadm_login
ssh_sysadm_login (off , off) Allow ssh to sysadm login
It's caused by `selinux.security_get_boolean_names()` and
`selinux.security_get_boolean_active(name)` which required SELinux
enabled system.
This change adds a fallback to semanage API which works in SELinux
disabled system when SELinux targeted policy is installed:
ANSIBLE_LIBRARY=plugins/modules ansible -i 192.168.121.153, -m seboolean -a "name=ssh_sysadm_login state=on persistent=true ignore_selinux_state=true" all
192.168.121.153 | CHANGED => {
"ansible_facts": {
"discovered_interpreter_python": "/usr/bin/python3"
},
"changed": true,
"name": "ssh_sysadm_login",
"persistent": true,
"state": true
}
$ ssh root@192.168.121.153 semanage boolean -l | grep ssh_sysadm_login
ssh_sysadm_login (on , on) Allow ssh to sysadm login
Note that without `persistent=true` this module is effectively NO-OP now.
Signed-off-by: Petr Lautrbach <lautrbach@redhat.com>
the CI failures are unrelated and shouldn't even be showing up ... I'm going to sort that out separately but that doesn't need to prevent this merge, all relevant CI tests passed
firewalld: make offline do something
SUMMARY
ansible.posix.firewalld has an offline flag, but it currently does not do anything. What most people expect it to do is allow the task to proceed even when firewalld is offline, so it makes the most sense for it to override the immediate flag and prevent the module from throwing an error in that case.
Fixes#81.
ISSUE TYPE
Feature Pull Request
COMPONENT NAME
firewalld
ADDITIONAL INFORMATION
Reviewed-by: Adam Miller <admiller@redhat.com>
Fix adding interface to zone when firewalld is offline
SUMMARY
Fixes issue #357
The existing implementation had several issues which have been resolved by this PR:
incorrectly assumed some zone always exists that contains the interface
incorrectly included the logic to add the interface to the target zone inside of the condition checking if the interface is already assigned to a different zone (and needs to be removed)
passed an invalid argument to the constructor for FirewallClientZoneSettings
ISSUE TYPE
Bugfix Pull Request
COMPONENT NAME
ansible.posix.firewalld
ADDITIONAL INFORMATION
- name: Add lo interface to trusted zone
ansible.posix.firewalld:
interface: lo
zone: trusted
permanent: Yes
state: enabled
Before
TASK [firewalld : Add lo interface to trusted zone] ****************************
task path: /root/ansible_collections/ansible/posix/tests/output/.tmp/integration/firewalld-gpgqwc7n-ÅÑŚÌβŁÈ/tests/integration/targets/firewalld/tasks/interface_test_cases.yml:7
Using module file /root/ansible_collections/ansible/posix/plugins/modules/firewalld.py
Pipelining is enabled.
<testhost> ESTABLISH LOCAL CONNECTION FOR USER: root
<testhost> EXEC /bin/sh -c '/usr/bin/python3.10 && sleep 0'
The full traceback is:
File "/tmp/ansible_ansible.posix.firewalld_payload_2vetziz9/ansible_ansible.posix.firewalld_payload.zip/ansible_collections/ansible/posix/plugins/module_utils/firewalld.py", line 112, in action_handler
return action_func(*action_func_args)
File "/tmp/ansible_ansible.posix.firewalld_payload_2vetziz9/ansible_ansible.posix.firewalld_payload.zip/ansible_collections/ansible/posix/plugins/modules/firewalld.py", line 481, in set_enabled_permanent
fatal: [testhost]: FAILED! => {
"changed": false,
"invocation": {
"module_args": {
"icmp_block": null,
"icmp_block_inversion": null,
"immediate": false,
"interface": "lo",
"masquerade": null,
"offline": null,
"permanent": true,
"port": null,
"port_forward": null,
"rich_rule": null,
"service": null,
"source": null,
"state": "enabled",
"target": null,
"timeout": 0,
"zone": "trusted"
}
},
"msg": "ERROR: Exception caught: list index out of range Permanent operation"
}
After
TASK [firewalld : Add lo interface to trusted zone] ****************************
task path: /root/ansible_collections/ansible/posix/tests/output/.tmp/integration/firewalld-tr92i6e1-ÅÑŚÌβŁÈ/tests/integration/targets/firewalld/tasks/interface_test_cases.yml:7
Using module file /root/ansible_collections/ansible/posix/plugins/modules/firewalld.py
Pipelining is enabled.
<testhost> ESTABLISH LOCAL CONNECTION FOR USER: root
<testhost> EXEC /bin/sh -c '/usr/bin/python3.10 && sleep 0'
changed: [testhost] => {
"changed": true,
"invocation": {
"module_args": {
"icmp_block": null,
"icmp_block_inversion": null,
"immediate": false,
"interface": "lo",
"masquerade": null,
"offline": null,
"permanent": true,
"port": null,
"port_forward": null,
"rich_rule": null,
"service": null,
"source": null,
"state": "enabled",
"target": null,
"timeout": 0,
"zone": "trusted"
}
},
"msg": "Permanent operation, Changed lo to zone trusted, (offline operation: only on-disk configs were altered)"
}
Reviewed-by: Adam Miller <admiller@redhat.com>
firewalld_info: fixed typo in default_zone and improved examples
SUMMARY
There was a typo in the docs of firewalld_info. Furthermore I slightly improved the examples by showcasing how to use the data gathered by this module.
ISSUE TYPE
Docs Pull Request
COMPONENT NAME
firewalld_info
ADDITIONAL INFORMATION
I'm not sure whether I should also update the file docs/ansible.posix.firewalld_info_module.rst but I suspect it will be generated automatically?
Reviewed-by: Hideki Saito <saito@fgrep.org>
The rhel_facts module must use keyword arguments. The current
code gives this error:
```
Traceback (most recent call last):
...
File "/tmp/ansible_ansible.posix.rhel_facts_payload_y10oy_4m/.../rhel_facts.py", line 72, in main
TypeError: exit_json() takes 1 positional argument but 2 were given
```
The fix is to use all keyword arguments like other facts plugins.
firewalld: Add support for protocol parameter
SUMMARY
Fixes#416 - This PR implements the --add-protocol/--remove-protocol parameters for firewalld.
I have just copied and rewritten the code from service parameter. Please look carefully :)
ISSUE TYPE
Feature Pull Request
COMPONENT NAME
firewalld
ADDITIONAL INFORMATION
- name: Allow OSPF traffic
ansible.posix.firewalld:
protocol: ospf
zone: work
state: enabled
permanent: true
Reviewed-by: Hideki Saito <saito@fgrep.org>
Updat ZoneTransaction to support operations when firewalld is offline
SUMMARY
Fixes#398 by checking the flag self.fw_offline and calling the offline specific APIs when the flag is true.
ISSUE TYPE
Bugfix Pull Request
COMPONENT NAME
ansible.posix.firewalld
ADDITIONAL INFORMATION
When attempting to add or remove zones, the ansible.posix.firewalld module would always call APIs as if it was online.
Specifically, the ZoneTransaction class assumed that self.fw was a FirewallClient, but when the daemon is offline, it is instead either a Firewall or Firewall_test based on the version of firewalld installed.
See #398 for additional background.
Sample task
- name: 'Zone example - Create new zone'
ansible.posix.firewalld:
zone: "example"
state: "present"
permanent: Yes
Before
The full traceback is:
File "/tmp/ansible_ansible.posix.firewalld_payload_04lptorx/ansible_ansible.posix.firewalld_payload.zip/ansible_collections/ansible/posix/plugins/module_utils/firewalld.py", line 111, in action_handler
return action_func(*action_func_args)
File "/tmp/ansible_ansible.posix.firewalld_payload_04lptorx/ansible_ansible.posix.firewalld_payload.zip/ansible_collections/ansible/posix/plugins/modules/firewalld.py", line 678, in get_enabled_permanent
fatal: [boot]: FAILED! => {
"changed": false,
"invocation": {
"module_args": {
"icmp_block": null,
"icmp_block_inversion": null,
"immediate": false,
"interface": null,
"masquerade": null,
"offline": null,
"permanent": true,
"port": null,
"port_forward": null,
"rich_rule": null,
"service": null,
"source": null,
"state": "present",
"target": null,
"timeout": 0,
"zone": "example"
}
},
"msg": "ERROR: Exception caught: 'FirewallConfig' object is not callable"
}
After
changed: [boot] => {
"changed": true,
"invocation": {
"module_args": {
"icmp_block": null,
"icmp_block_inversion": null,
"immediate": false,
"interface": null,
"masquerade": null,
"offline": null,
"permanent": true,
"port": null,
"port_forward": null,
"rich_rule": null,
"service": null,
"source": null,
"state": "present",
"target": null,
"timeout": 0,
"zone": "example"
}
},
"msg": "Permanent operation, Added zone example, Changed zone example to present, (offline operation: only on-disk configs were altered)"
Reviewed-by: Gonéri Le Bouder <goneri@lebouder.net>
Fix boolean values in docs
SUMMARY
Fixes#397
ISSUE TYPE
Docs Pull Request
COMPONENT NAME
Docs of several modules
ADDITIONAL INFORMATION
Notes about testing. I'm not sure how to test collections properly. I have ran ansible-test sanity --python 3.10 and ansible-test units --python 3.10 in venv with ansible-core 2.14 (with 0 return code in both cases). To run units test successfully I had to install pytest-forked pip package in addition to this one pytest-xdist from test-requirements.txt.
Note about issue #397. I haven't changed yes here because in this case it is value from /etc/vfstab. Also I've changed yes for parameters masquerade and icmp_block_inversion because they both are converted to bool by boolean function and will be changed to boolean in a future release (1, 2), for required too.
Reviewed-by: Felix Fontein <felix@fontein.de>
Reviewed-by: Hideki Saito <saito@fgrep.org>
