Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add auto-fixing implementation for partial-become rule #3692

Merged
merged 12 commits into from
Sep 8, 2023
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