Skip to content

Commit

Permalink
Add auto-fixing implementation for partial-become rule (ansible#3692)
Browse files Browse the repository at this point in the history
Co-authored-by: Bradley A. Thornton <[email protected]>
  • Loading branch information
shatakshiiii and cidrblock authored Sep 8, 2023
1 parent 453269b commit 8cd9281
Show file tree
Hide file tree
Showing 11 changed files with 437 additions and 116 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/tox.yml
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ jobs:
WSLENV: FORCE_COLOR:PYTEST_REQPASS:TOXENV:GITHUB_STEP_SUMMARY
# Number of expected test passes, safety measure for accidental skip of
# tests. Update value if you add/remove tests.
PYTEST_REQPASS: 821
PYTEST_REQPASS: 822
steps:
- name: Activate WSL1
if: "contains(matrix.shell, 'wsl')"
Expand Down
21 changes: 10 additions & 11 deletions examples/playbooks/rule-partial-become-without-become-fail.yml
Original file line number Diff line number Diff line change
@@ -1,28 +1,27 @@
---
- hosts: localhost
name: Use of become_user without become play
- name: Use of become_user without become at play level
hosts: localhost
become_user: root

tasks:
- ansible.builtin.debug:
- name: A task without issues
ansible.builtin.debug:
msg: hello

- hosts: localhost

- name: Use of become_user without become at task level
hosts: localhost
tasks:
- name: Use of become_user without become task
ansible.builtin.command: whoami
become_user: postgres
changed_when: false

- hosts: localhost

- name: Use of become_user without become at task level
hosts: localhost
tasks:
- name: A block with become and become_user on different tasks
block:
- name: Sample become
become: true
ansible.builtin.command: whoami
- name: Sample become_user
become_user: postgres
ansible.builtin.command: whoami
become_user: true
changed_when: false
20 changes: 12 additions & 8 deletions examples/playbooks/rule-partial-become-without-become-pass.yml
Original file line number Diff line number Diff line change
@@ -1,35 +1,39 @@
---
- hosts: localhost
- name: Test play
hosts: localhost
become_user: root
become: true

tasks:
- ansible.builtin.debug:
- name: Debug
ansible.builtin.debug:
msg: hello

- hosts: localhost

- name: Test play
hosts: localhost
tasks:
- name: Foo
ansible.builtin.command: whoami
become_user: postgres
become: true
changed_when: false

- hosts: localhost
become: true
- name: Test play
hosts: localhost

tasks:
- name: Accepts a become from higher scope
ansible.builtin.command: whoami
become_user: postgres
changed_when: false

- hosts: localhost
- name: Test play
hosts: localhost
become_user: postgres
become: true

tasks:
- name: Accepts a become from a lower scope
ansible.builtin.command: whoami
become: true
become_user: root
changed_when: false
4 changes: 4 additions & 0 deletions examples/playbooks/tasks/partial_become.yml/main.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
- name: Included with partial become
ansible.builtin.debug:
msg: Included with partial become
56 changes: 56 additions & 0 deletions examples/playbooks/transform-partial-become.transformed.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
---
# The play has become_user and the task has become
# this is fixable, copy the become_user to the task
# and remove from the play
- name: Play 1
hosts: localhost
tasks:
- name: A block
block:
- name: Debug
ansible.builtin.debug:
msg: hello
become: true
become_user: root

# The task has become_user but the play does not
# this is fixable, remove the become_user from the task
- name: Play 2
hosts: localhost
tasks:
- name: A block
block:
- name: Debug
ansible.builtin.debug:
msg: hello

# The task has become_user and the play has become
# this is fixable, add become to the task
- name: Play 3
hosts: localhost
become: true
tasks:
- name: A block
block:
- name: Debug
ansible.builtin.debug:
msg: hello
become: true
become_user: root

# The play has become_user but has an include
# this is not fixable, the include could be called from multiple playbooks
- name: Play 4
hosts: localhost
become_user: root
tasks:
- name: A block
block:
- name: Debug
ansible.builtin.debug:
msg: hello
become: true

- name: Include
ansible.builtin.include_tasks:
file: ../tasks/partial_become/main.yml
56 changes: 56 additions & 0 deletions examples/playbooks/transform-partial-become.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
---
# The play has become_user and the task has become
# this is fixable, copy the become_user to the task
# and remove from the play
- name: Play 1
hosts: localhost
become_user: root
tasks:
- name: A block
block:
- name: Debug
ansible.builtin.debug:
msg: hello
become: true

# The task has become_user but the play does not
# this is fixable, remove the become_user from the task
- name: Play 2
hosts: localhost
tasks:
- name: A block
block:
- name: Debug
ansible.builtin.debug:
msg: hello
become_user: root

# The task has become_user and the play has become
# this is fixable, add become to the task
- name: Play 3
hosts: localhost
become: true
tasks:
- name: A block
block:
- name: Debug
ansible.builtin.debug:
msg: hello
become_user: root

# The play has become_user but has an include
# this is not fixable, the include could be called from multiple playbooks
- name: Play 4
hosts: localhost
become_user: root
tasks:
- name: A block
block:
- name: Debug
ansible.builtin.debug:
msg: hello
become: true

- name: Include
ansible.builtin.include_tasks:
file: ../tasks/partial_become/main.yml
86 changes: 83 additions & 3 deletions src/ansiblelint/rules/partial_become.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@ This rule checks that privilege escalation is activated when changing users.
To perform an action as a different user with the `become_user` directive, you
must set `become: true`.

This rule can produce the following messages:

- `partial-become[play]`: become_user requires become to work as expected, at
play level.
- `partial-become[task]`: become_user requires become to work as expected, at
task level.

!!! warning

While Ansible inherits have of `become` and `become_user` from upper levels,
Expand All @@ -19,12 +26,13 @@ must set `become: true`.
---
- name: Example playbook
hosts: localhost
become: true # <- Activates privilege escalation.
tasks:
- name: Start the httpd service as the apache user
ansible.builtin.service:
name: httpd
state: started
become_user: apache # <- Does not change the user because "become: true" is not set.
become_user: apache # <- Does not change the user because "become: true" is not set.
```
## Correct Code
Expand All @@ -37,6 +45,78 @@ must set `become: true`.
ansible.builtin.service:
name: httpd
state: started
become: true # <- Activates privilege escalation.
become_user: apache # <- Changes the user with the desired privileges.
become: true # <- Activates privilege escalation.
become_user: apache # <- Changes the user with the desired privileges.

# Stand alone playbook alternative, applies to all tasks

- name: Example playbook
hosts: localhost
become: true # <- Activates privilege escalation.
become_user: apache # <- Changes the user with the desired privileges.
tasks:
- name: Start the httpd service as the apache user
ansible.builtin.service:
name: httpd
state: started
```
## Problematic Code
```yaml
---
- name: Example playbook 1
hosts: localhost
become: true # <- Activates privilege escalation.
tasks:
- name: Include a task file
ansible.builtin.include_tasks: tasks.yml
```
```yaml
---
- name: Example playbook 2
hosts: localhost
tasks:
- name: Include a task file
ansible.builtin.include_tasks: tasks.yml
```
```yaml
# tasks.yml
- name: Start the httpd service as the apache user
ansible.builtin.service:
name: httpd
state: started
become_user: apache # <- Does not change the user because "become: true" is not set.
```
## Correct Code
```yaml
---
- name: Example playbook 1
hosts: localhost
tasks:
- name: Include a task file
ansible.builtin.include_tasks: tasks.yml
```
```yaml
---
- name: Example playbook 2
hosts: localhost
tasks:
- name: Include a task file
ansible.builtin.include_tasks: tasks.yml
```
```yaml
# tasks.yml
- name: Start the httpd service as the apache user
ansible.builtin.service:
name: httpd
state: started
become: true # <- Activates privilege escalation.
become_user: apache # <- Does not change the user because "become: true" is not set.
```
Loading

0 comments on commit 8cd9281

Please sign in to comment.