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

omit does not work properly with become #75692

Closed
1 task done
mohd-akram opened this issue Sep 13, 2021 · 3 comments · Fixed by #78917
Closed
1 task done

omit does not work properly with become #75692

mohd-akram opened this issue Sep 13, 2021 · 3 comments · Fixed by #78917
Labels
affects_2.15 bug This issue/PR relates to a bug. has_pr This issue has an associated PR. support:core This issue/PR relates to code supported by the Ansible Engineering Team.

Comments

@mohd-akram
Copy link

Summary

omit does not seem to work properly with become.

Issue Type

Bug Report

Component Name

become

Ansible Version

$ ansible --version
ansible 2.9.25
  config file = /etc/ansible/ansible.cfg
  configured module search path = ['/home/mohamed/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3.9/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 3.9.7 (default, Aug 30 2021, 00:00:00) [GCC 11.2.1 20210728 (Red Hat 11.2.1-1)]

Configuration

$ ansible-config dump --only-changed

OS / Environment

Fedora 34

Steps to Reproduce

- hosts: localhost
  become: yes
  tasks:
    - shell: echo $USER
    - shell: echo $USER
      become: '{{ omit }}'
ansible-playbook --ask-become-pass --verbose become-omit.yml

Expected Results

USER=root in both cases

Actual Results

Using /etc/ansible/ansible.cfg as config file
BECOME password: 
[WARNING]: provided hosts list is empty, only localhost is available. Note that the implicit localhost does not match 'all'

PLAY [localhost] *************************************************************************************************************************************

TASK [Gathering Facts] *******************************************************************************************************************************
ok: [localhost]

TASK [shell] *****************************************************************************************************************************************
changed: [localhost] => {"changed": true, "cmd": "echo $USER", "delta": "0:00:00.006134", "end": "2021-09-13 14:43:44.075108", "rc": 0, "start": "2021-09-13 14:43:44.068974", "stderr": "", "stderr_lines": [], "stdout": "root", "stdout_lines": ["root"]}

TASK [shell] *****************************************************************************************************************************************
changed: [localhost] => {"changed": true, "cmd": "echo $USER", "delta": "0:00:00.005727", "end": "2021-09-13 14:43:44.411115", "rc": 0, "start": "2021-09-13 14:43:44.405388", "stderr": "", "stderr_lines": [], "stdout": "mohamed", "stdout_lines": ["mohamed"]}

PLAY RECAP *******************************************************************************************************************************************
localhost                  : ok=3    changed=2    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0

Code of Conduct

  • I agree to follow the Ansible Code of Conduct
@ansibot
Copy link
Contributor

ansibot commented Sep 13, 2021

Files identified in the description:

  • lib/ansible/playbook/become.py

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

@ansibot ansibot added affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Sep 13, 2021
@mkrizek
Copy link
Contributor

mkrizek commented Sep 13, 2021

I am not sure if this is something that we want to address yet but potential POC fix might be:

diff --git a/lib/ansible/playbook/base.py b/lib/ansible/playbook/base.py
index 564ce0cd3a5..4842b8ce91b 100644
--- a/lib/ansible/playbook/base.py
+++ b/lib/ansible/playbook/base.py
@@ -652,6 +652,13 @@ class FieldAttributeBase(with_metaclass(BaseMeta, object)):
                 # if this evaluated to the omit value, set the value back to
                 # the default specified in the FieldAttribute and move on
                 if omit_value is not None and value == omit_value:
+                    if attribute.inherit:
+                        setattr(self, name, Sentinel)
+                        try:
+                            setattr(self, name, self._get_parent_attribute(name))
+                        except AttributeError:
+                            # PlayContext, the value will be set in TE via PC.set_task_and_variable_override
+                            pass
+                        continue
                     if callable(attribute.default):
                         setattr(self, name, attribute.default())
                     else:

EDIT: diff updated

@sivel sivel removed the needs_triage Needs a first human triage before being processed. label Sep 14, 2021
@jborean93 jborean93 added affects_2.14 and removed affects_2.9 This issue/PR affects Ansible v2.9 labels Jun 8, 2022
@bcoca
Copy link
Member

bcoca commented Sep 28, 2022

The bug is due to 'omit' forcing the keyword to it's default value (none/false), not it's inherited value (yes/true).
https://github.com/ansible/ansible/blob/devel/lib/ansible/playbook/base.py#L530

bcoca added a commit to bcoca/ansible that referenced this issue Sep 28, 2022
  ensure we use context/inheritance when calculating value,
  using default only when context is unavailable.

  fixes ansible#75692
@ansibot ansibot added the has_pr This issue has an associated PR. label Sep 28, 2022
bcoca added a commit that referenced this issue Sep 30, 2022
* omit keyword should reset to context

  ensure we use context/inheritance when calculating value,
  using default only when context is unavailable.

  fixes #75692
bcoca added a commit to bcoca/ansible that referenced this issue Sep 30, 2022
* omit keyword should reset to context

  ensure we use context/inheritance when calculating value,
  using default only when context is unavailable.

  fixes ansible#75692

(cherry picked from commit 9650ddb)
bcoca added a commit to bcoca/ansible that referenced this issue Sep 30, 2022
* omit keyword should reset to context

  ensure we use context/inheritance when calculating value,
  using default only when context is unavailable.

  fixes ansible#75692

(cherry picked from commit 9650ddb)
bcoca added a commit to bcoca/ansible that referenced this issue Oct 4, 2022
* omit keyword should reset to context

  ensure we use context/inheritance when calculating value,
  using default only when context is unavailable.

  fixes ansible#75692

(cherry picked from commit 9650ddb)
sivel pushed a commit that referenced this issue Oct 6, 2022
* omit keyword should reset to context (#78917)

* omit keyword should reset to context

  ensure we use context/inheritance when calculating value,
  using default only when context is unavailable.

  fixes #75692

(cherry picked from commit 9650ddb)

* fixes to FA inheritance (#78990)

finalized applies to all field attributes
fix getting parent value
also remove unused/needed extend/prepend signature
moar testing

(cherry picked from commit ff6e4da)

* setup role needs it's own info
@ansible ansible locked and limited conversation to collaborators Oct 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.15 bug This issue/PR relates to a bug. has_pr This issue has an associated PR. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants