Skip to content

Commit

Permalink
ipasudorule: Don't become or gather_facts and use only true/false
Browse files Browse the repository at this point in the history
Unless there's a real need to use privileged access or to gather Ansble
facts upfront, we should always set "become: false" and
"gather_facts: false". In the case that only a few Ansible facts are
required, 'ansible.builtin.setup' with 'gather_subset' should be used.

As the YAML 1.2 standard dictates, boolean values should only use 'true'
or 'false' values.

This patch fixes these issues in the 'sudorule' test suite.
  • Loading branch information
rjeffman committed Nov 5, 2024
1 parent 69a9896 commit 45f3651
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 20 deletions.
4 changes: 2 additions & 2 deletions tests/sudorule/test_sudorule.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
- name: Test sudorule
hosts: "{{ ipa_test_host | default('ipaserver') }}"
become: true
gather_facts: true
gather_facts: false

tasks:

Expand Down Expand Up @@ -1157,7 +1157,7 @@
hostmask: 192.168.120.0/24
action: member
register: result
check_mode: yes
check_mode: true
failed_when: not result.changed or result.failed

- name: Ensure sudorule hostmask member is present
Expand Down
19 changes: 14 additions & 5 deletions tests/sudorule/test_sudorule_categories.yml
Original file line number Diff line number Diff line change
@@ -1,13 +1,22 @@
---
- name: Test sudorule user category
hosts: ipaserver
become: yes
gather_facts: yes
become: false
gather_facts: false

tasks:
- name: Get Domain from the server name
ansible.builtin.set_fact:
ipaserver_domain: "{{ ansible_facts['fqdn'].split('.')[1:] | join('.') }}"
- name: Test sudorule single hostnames
block:
# setup test environment
- name: Ensure ipaserver_domain is set
when: ipaserver_domain is not defined
block:
- name: Retrieve host information
ansible.builtin.setup:
gather_subset: dns
- name: Get Domain from the server name
ansible.builtin.set_fact:
ipaserver_domain: "{{ ansible_facts['fqdn'].split('.')[1:] | join('.') }}"

- name: Ensure sudorules are absent
ipasudorule:
Expand Down
4 changes: 2 additions & 2 deletions tests/sudorule/test_sudorule_client_context.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
---
- name: Test sudorule
hosts: ipaclients, ipaserver
become: no
gather_facts: no
become: false
gather_facts: false

tasks:
- name: Include FreeIPA facts.
Expand Down
6 changes: 3 additions & 3 deletions tests/sudorule/test_sudorule_member_case_insensitive.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
---
- name: Test sudorule members should be case insensitive.
hosts: "{{ ipa_test_host | default('ipaserver') }}"
become: no
gather_facts: no
become: false
gather_facts: false

vars:
groups_present:
Expand Down Expand Up @@ -37,7 +37,7 @@
ipahost:
ipaadmin_password: SomeADMINpassword
name: "{{ item }}.{{ ipa_domain }}"
force: yes
force: true
loop: "{{ groups_present }}"

- name: Ensure test users exist.
Expand Down
19 changes: 12 additions & 7 deletions tests/sudorule/test_sudorule_single_hostnames.yml
Original file line number Diff line number Diff line change
@@ -1,17 +1,22 @@
---
- name: Test sudorule with single hostnames.
hosts: "{{ ipa_test_host | default('ipaserver') }}"
become: no
gather_facts: no
become: false
gather_facts: false

tasks:
- name: Test sudorule single hostnames
block:
# setup test environment
- name: Get Domain from the server name
ansible.builtin.set_fact:
ipaserver_domain: "{{ ansible_facts['fqdn'].split('.')[1:] | join('.') }}"
- name: Ensure ipaserver_domain is set
when: ipaserver_domain is not defined
block:
- name: Retrieve host information
ansible.builtin.setup:
gather_subset: dns
- name: Get Domain from the server name
ansible.builtin.set_fact:
ipaserver_domain: "{{ ansible_facts['fqdn'].split('.')[1:] | join('.') }}"

- name: Ensure test sudo rule is absent
ipasudorule:
Expand All @@ -24,9 +29,9 @@
ipaadmin_password: SomeADMINpassword
hosts:
- name: "host01.{{ ipaserver_domain }}"
force: yes
force: true
- name: "host02.{{ ipaserver_domain }}"
force: yes
force: true

# start tests
- name: Ensure sudorule exist with host member using FQDN.
Expand Down
6 changes: 5 additions & 1 deletion tests/sudorule/test_sudorules.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
- name: Test sudorule
hosts: "{{ ipa_test_host | default('ipaserver') }}"
become: false
gather_facts: true # required for ansible_facts['fqdn']
gather_facts: false

module_defaults:
ipauser:
Expand All @@ -27,6 +27,10 @@
tasks:

# setup
- name: Ensure ansible facts for DNS are available
ansible.builtin.setup:
gather_subset: dns

- name: Ensure test users are present
ipauser:
users:
Expand Down

0 comments on commit 45f3651

Please sign in to comment.