Skip to content

Commit 8d47d9c

Browse files
committed
fix(roles): set become: false on tasks delegated to localhost
When a playbook sets `become: true` at the play level, tasks delegated to the controller (`delegate_to: 'localhost'`) inherit that and try to call sudo on the controller, failing with `sudo: a password is required` when the controller has no passwordless sudo. Set `become: false` explicitly on every such task across the collection. Also sets the missing `run_once: true` in `repo_monitoring_plugins/tasks/RedHat.yml` to match the Debian variant, demonstrates the pattern in `roles/example`, and documents it in CONTRIBUTING.md. Closes #242
1 parent 8227fdb commit 8d47d9c

68 files changed

Lines changed: 145 additions & 4 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2424

2525
### Changed
2626

27+
* **role:example**: Demonstrate the `delegate_to: 'localhost'` + `become: false` pattern (download on the controller, copy to the target) so role authors can copy it consistently.
2728
* **role:apache_httpd**: bump Core Rule Set to 4.26.0
2829
* **role:apache_httpd**: Update the two reverse-proxy snippets in `EXAMPLES.md` to use `ProxyPass` instead of `RewriteRule ^/(.*) ... [proxy,last]`. The RewriteRule variant `%`-decodes the URI pattern and forwards characters such as `?` unencoded to the backend, which breaks WebDAV apps (file-not-found on rename in Nextcloud). The examples now also carry a comment explaining the choice and link to the corresponding [blog post](https://www.linuxfabrik.ch/blog/nextcloud-rewriterules-vs-proxypass).
2930

@@ -50,6 +51,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
5051

5152
### Fixed
5253

54+
* **roles**: Set `become: false` on tasks delegated to localhost across the collection. Previously these tasks inherited `become: true` from the playbook level and tried to call `sudo` on the Ansible controller, which fails on controllers without a passwordless sudo setup with `sudo: a password is required`. Affected are all `repo_*` roles, the `*_vm` cloud roles (`exoscale_vm`, `hetzner_vm`, `infomaniak_vm`), all `icingaweb2_module_*` roles that download artefacts, `monitoring_plugins`, `shared`, plus several others. Existing playbooks that were working without playbook-level `become: true` are unaffected ([#242](https://github.com/Linuxfabrik/lfops/issues/242)).
55+
* **role:repo_monitoring_plugins**: Add the missing `run_once: true` on the local repo-key download task on Red Hat platforms, matching the Debian variant. The key is now downloaded once per run instead of once per host.
5356
* **role:network**: README still claimed the role disables zeroconf, but the corresponding `NOZEROCONF=yes` task was removed in 2024 (NetworkManager no longer adds the zeroconf route by default). Bring the README in line with what the role actually does and call out the Hetzner-specific `hc-utils` cleanup explicitly.
5457
* **role:haveged**: Setting `haveged__service_state: 'stopped'` produced the invalid systemctl command `stopp` because of a `[:-2]` slice in the task name. The role now uses `ansible.builtin.service` directly with the configured state, so all four valid values (`reloaded` / `restarted` / `started` / `stopped`) work as expected.
5558
* **role:unattended_upgrades**: Correct README description; the role deactivates Unattended Upgrades by setting both `APT::Periodic` flags to `0` in `/etc/apt/apt.conf.d/20auto-upgrades` (Debian/Ubuntu), it does not remove the `unattended-upgrades` package.

CONTRIBUTING.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,21 @@ When creating a new role, make sure to deliver:
259259
* `ansible.builtin.template` over `ansible.builtin.copy`, `ansible.builtin.lineinfile` or `ansible.builtin.blockinfile`. Templating the whole file leads to more consistent, deterministic, and expected results.
260260
* Do not use `state: 'latest'` for the `ansible.builtin.package` module as this is not idempotent. Always use `state: 'present'`.
261261
* Always use `delegate_to: 'localhost'` instead of `local_action`.
262+
* Always set `become: false` on every task delegated to localhost. When a play sets `become: true` at the play level (the typical case), it propagates to delegated tasks too and tries to escalate via sudo on the Ansible controller. On a controller without passwordless sudo this fails with `sudo: a password is required`, even though the delegated task only writes to `/tmp` or hits a remote API and does not need root locally. Example:
263+
264+
```yaml
265+
- name: 'curl --output /tmp/ansible.example.tar.gz https://example.com/releases/example.tar.gz'
266+
ansible.builtin.get_url:
267+
url: 'https://example.com/releases/example.tar.gz'
268+
dest: '/tmp/ansible.example.tar.gz'
269+
mode: 0o644
270+
delegate_to: 'localhost'
271+
become: false
272+
run_once: true
273+
changed_when: false # not an actual config change on the target
274+
check_mode: false # run task even if `--check` is specified
275+
```
276+
262277
* Always provide `changed_when`, `creates`, or `removes` for `ansible.builtin.command` and `ansible.builtin.shell` tasks to ensure idempotency. Use `changed_when: false` for read-only commands.
263278
* When deploying files with `ansible.builtin.template`, always set `backup`, `src`, `dest`, `owner`, `group`, and `mode`.
264279
* Prefer `ansible.builtin.assert` over `ansible.builtin.fail` with `when` for validation checks. There is basically no technical difference; this guideline is only for consistency.

roles/acme_sh/tasks/issue-cert.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
check_mode: false # run task even if `--check` is specified
77
register: 'acme_sh__curl_result'
88
delegate_to: 'localhost'
9+
become: false
910

1011
- name: 'check if acme-challenge was reachable'
1112
ansible.builtin.assert:

roles/apache_solr/tasks/main.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
dest: '/tmp/solr-{{ apache_solr__version }}.tgz'
1212
checksum: '{{ apache_solr__checksum }}'
1313
delegate_to: 'localhost'
14+
become: false
1415
check_mode: false # run task even if `--check` is specified
1516
changed_when: false # just gathering info, no actual change
1617
when:
@@ -22,6 +23,7 @@
2223
dest: '/tmp/solr-{{ apache_solr__version }}.tgz'
2324
checksum: '{{ apache_solr__checksum }}'
2425
delegate_to: 'localhost'
26+
become: false
2527
check_mode: false # run task even if `--check` is specified
2628
changed_when: false # just gathering info, no actual change
2729
when:

roles/blocky/tasks/main.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
register: 'blocky__github_release'
3131
check_mode: false # run task even if `--check` is specified
3232
delegate_to: 'localhost'
33+
become: false
3334
run_once: true
3435

3536
- name: 'Store the latest release version'
@@ -45,6 +46,7 @@
4546
dest: '/tmp/ansible.blocky_{{ blocky__github_version }}_Linux_x86_64.tar.gz'
4647
mode: 0o644
4748
delegate_to: 'localhost'
49+
become: false
4850
changed_when: false # not an actual config change on the server
4951
check_mode: false # run task even if `--check` is specified
5052

@@ -53,6 +55,7 @@
5355
src: '/tmp/ansible.blocky_{{ blocky__github_version }}_Linux_x86_64.tar.gz'
5456
dest: '/tmp'
5557
delegate_to: 'localhost'
58+
become: false
5659
ignore_errors: '{{ ansible_check_mode }}' # ignore errors if `--check` is specified
5760

5861
- name: 'scp /tmp/blocky {{ inventory_hostname }}:/usr/local/sbin/blocky'

roles/example/tasks/main.yml

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,13 +84,26 @@
8484
name: '{{ __example__required_packages }}'
8585
state: 'present'
8686

87-
- name: 'curl --output /tmp/example-{{ example__version }}.tar.gz https://example.com/releases/example-{{ example__version }}.tar.gz'
87+
# download on the controller (`delegate_to: 'localhost'`) and copy to the target afterwards,
88+
# so that targets without internet access can still be provisioned.
89+
# always set `become: false` on tasks delegated to localhost. otherwise `become: true`
90+
# from the playbook level propagates and triggers sudo on the controller, which is
91+
# unneeded for a download into /tmp and usually fails because no sudo password is set.
92+
- name: 'curl --output /tmp/ansible.example-{{ example__version }}.tar.gz https://example.com/releases/example-{{ example__version }}.tar.gz' # noqa risky-file-permissions (temporary file)
8893
ansible.builtin.get_url:
8994
url: 'https://example.com/releases/example-{{ example__version }}.tar.gz'
90-
dest: '/tmp/example-{{ example__version }}.tar.gz'
91-
owner: 'root'
92-
group: 'root'
95+
dest: '/tmp/ansible.example-{{ example__version }}.tar.gz'
9396
mode: 0o644
97+
delegate_to: 'localhost'
98+
become: false
99+
run_once: true
100+
changed_when: false # not an actual config change on the target
101+
check_mode: false # run task even if `--check` is specified
102+
103+
- name: 'Copy /tmp/ansible.example-{{ example__version }}.tar.gz to the target' # noqa risky-file-permissions (temporary file)
104+
ansible.builtin.copy:
105+
src: '/tmp/ansible.example-{{ example__version }}.tar.gz'
106+
dest: '/tmp/example-{{ example__version }}.tar.gz'
94107

95108
- name: 'tar xzf /tmp/example-{{ example__version }}.tar.gz -C /opt/example'
96109
ansible.builtin.unarchive:

roles/exoscale_vm/tasks/main.yml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
name: '{{ exoscale_vm__name }}'
99
state: 'present'
1010
delegate_to: 'localhost'
11+
become: false
1112
when:
1213
- 'exoscale_vm__security_group_rules is defined and exoscale_vm__security_group_rules | length > 0'
1314
- 'exoscale_vm__state != "absent"'
@@ -26,6 +27,7 @@
2627
type: '{{ item["type"] | default("ingress") }}'
2728
loop: '{{ exoscale_vm__security_group_rules }}'
2829
delegate_to: 'localhost'
30+
become: false
2931
when:
3032
- 'exoscale_vm__security_group_rules is defined and exoscale_vm__security_group_rules | length > 0'
3133
- 'exoscale_vm__state != "absent"'
@@ -61,6 +63,7 @@
6163
--output-format json
6264
register: 'exoscale_vm__instance_list_result'
6365
delegate_to: 'localhost'
66+
become: false
6467
changed_when: false # just gathering information, no actual change happening here
6568

6669
- name: 'Create the VM at Exoscale'
@@ -82,6 +85,7 @@
8285
{% endif %}
8386
'{{ exoscale_vm__name }}'
8487
delegate_to: 'localhost'
88+
become: false
8589
when:
8690
- 'exoscale_vm__state != "absent"'
8791
- 'exoscale_vm__name not in exoscale_vm__instance_list_result["stdout"] | from_json | map(attribute="name")'
@@ -95,6 +99,7 @@
9599
--force
96100
'{{ exoscale_vm__name }}'
97101
delegate_to: 'localhost'
102+
become: false
98103
when:
99104
- 'exoscale_vm__state == "absent"'
100105
- 'exoscale_vm__name in exoscale_vm__instance_list_result["stdout"] | from_json | map(attribute="name")'
@@ -122,6 +127,7 @@
122127
- 'exoscale_vm__state != "absent"'
123128
- 'item["cidr"] is defined'
124129
delegate_to: 'localhost'
130+
become: false
125131

126132
- name: "Manage the VM's networks"
127133
ngine_io.cloudstack.cs_instance_nic:
@@ -135,6 +141,7 @@
135141
loop: '{{ exoscale_vm__private_networks }}'
136142
when: 'exoscale_vm__state != "absent"'
137143
delegate_to: 'localhost'
144+
become: false
138145

139146
tags:
140147
- 'exoscale_vm'
@@ -151,6 +158,7 @@
151158
name: '{{ exoscale_vm__name }}'
152159
state: 'absent'
153160
delegate_to: 'localhost'
161+
become: false
154162
when:
155163
- 'exoscale_vm__security_group_rules is defined and exoscale_vm__security_group_rules | length > 0'
156164
- 'exoscale_vm__state == "absent"'

roles/firewall/tasks/main.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@
119119
changed_when: false # not a config change on the server
120120
check_mode: false # run task even if `--check` is specified
121121
delegate_to: 'localhost'
122+
become: false
122123
run_once: true
123124
vars:
124125
__firewall__fwbuilder_repo_url: >-
@@ -138,6 +139,7 @@
138139
single_branch: true
139140
changed_when: false # not a config change on the server
140141
delegate_to: 'localhost'
142+
become: false
141143
run_once: true
142144
check_mode: false # run task even if `--check` is specified
143145
vars:

roles/glpi_agent/tasks/main.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
register: 'glpi_agent__github_release'
1414
check_mode: false # run task even if `--check` is specified
1515
delegate_to: 'localhost'
16+
become: false
1617
run_once: true
1718

1819
- name: 'Store the latest release version'
@@ -28,6 +29,7 @@
2829
dest: '/tmp/ansible.glpi-agent-{{ glpi_agent__github_version }}-1.noarch.rpm'
2930
mode: 0o644
3031
delegate_to: 'localhost'
32+
become: false
3133
changed_when: false # not an actual config change on the server
3234
check_mode: false # run task even if `--check` is specified
3335

roles/grafana/tasks/main.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,7 @@
272272
loop_control:
273273
label: '{{ item["json"]["name"] | d(item) }}'
274274
delegate_to: 'localhost'
275+
become: false
275276
when:
276277
- 'not grafana__skip_token_to_bitwarden'
277278
- 'not (item["skipped"] is defined and item["skipped"])'

0 commit comments

Comments
 (0)