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

Validate context after update of magic variables #31179

Open
wants to merge 2 commits into
base: devel
from

Conversation

@jeromew
Copy link
Contributor

commented Oct 2, 2017

SUMMARY

Currently, you cannot use magic variables as variables in become_user for example. The post_validate step will do a fatal error because these variables are undefined.

By moving the post_validate step after these variables are added to the context, it becomes possible to use them in templates.

This used to work in 1.8, when doing 'sudo_user: "{{ansible_ssh_user}}"'
it now breaks with 'become_user: "{{ansible_user}}"'

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

executor/task_executor.py

ANSIBLE VERSION
ansible 2.4.1.0 (stable-2.4 4025b47629) last updated 2017/09/29 09:21:06 (GMT +000)
  config file = /home/juser/ansible.cfg
  configured module search path = [u'/home/juser/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /home/juser/ansiwork-git/lib/ansible
  executable location = /home/juser/ansiwork-git/bin/ansible
  python version = 2.7.5 (default, Jun 26 2014, 11:55:39) [GCC 4.8.2 20131212 (Red Hat 4.8.2-7)]
ADDITIONAL INFORMATION

you can test with the following playbook

- hosts: ovh-1
  gather_facts: no
  tasks:
  - name: works
    debug:
      msg: "hello {{ ansible_user }}"


- hosts: ovh-1
  gather_facts: no
  tasks:
  - name: does not work
    debug:
      msg: "hello {{ ansible_user }}"
    become: yes
    become_user: "{{ansible_user}}"
command
ansible-playbook -u juser -i ansiconf/production ansiconf/sample.yml



result before
PLAY [ovh-1] ***************************************************************************************************************************************************************

TASK [Echo current user] ***************************************************************************************************************************************************
ok: [xxx.ovh.net] => {
    "msg": "hello juser"
}

PLAY [ovh-1] ***************************************************************************************************************************************************************

TASK [Echo current user] ***************************************************************************************************************************************************
fatal: [xxx.ovh.net]: FAILED! => {"msg": "The field 'become_user' has an invalid value, which includes an undefined variable. The error was: 'ansible_user' is undefined\nexception type: <class 'ansible.errors.AnsibleUndefinedVariable'>\nexception: 'ansible_user' is undefined"}
        to retry, use: --limit @/home/juser/ansiconf/sample.retry

PLAY RECAP *****************************************************************************************************************************************************************
xxx.ovh.net           : ok=1    changed=0    unreachable=0    failed=1




result after
PLAY [ovh-1] ***************************************************************************************************************************************************************

TASK [Echo current user] ***************************************************************************************************************************************************
ok: [vps33957.ovh.net] => {
    "msg": "hello juser"
}

PLAY [ovh-1] ***************************************************************************************************************************************************************

TASK [Echo current user] ***************************************************************************************************************************************************
ok: [xxx.ovh.net] => {
    "msg": "hello juser"
}

PLAY RECAP *****************************************************************************************************************************************************************
xxx.ovh.net           : ok=2    changed=0    unreachable=0    failed=0

```
Validate context after update of magic variables
Currently, you cannot use magic variables as variables in `become_user` for example. The post_validate step will do a fatal error because these variables are undefined.

By moving the post_validate step after these variables are added to the context, it becomes possible to use them in templates.

@bcoca bcoca removed the needs_triage label Oct 2, 2017

@bcoca

This comment has been minimized.

Copy link
Member

commented Oct 2, 2017

this shoudl not be moved, just read the comments between the 2 positions

@jeromew

This comment has been minimized.

Copy link
Contributor Author

commented Oct 2, 2017

Hello @bcoca,

I think I read those but may have been too fast in my reading. Are you referring to

# now that the play context is finalized, if the remote_addr is not set
# default to using the host's address field as the remote address

I admit I did not take time to consider the implications of my PR for remote_addr (+ all the tests pass)

The things is that I don't see how it is possible to "post_validate" templated data when all the variables needed for the templates are not yet available.

In my case, with the current devel code, "{{ansible_user}}" is not yet added to the variables (this is done by update_vars) when the templates are rendered, which makes it impossible to have this variable in a connection info templated variable. Basically, none of the magic variables in MAGIC_VARIABLE_MAPPING are available for become_user: or other connection info variables.

Can you help me reach a point that would be acceptable to you so that I could use the MAGIC_VARIABLE_MAPPING variables in "become_user" templates ? {{ansible_ssh_user}} used to be allowed in sudo_user:. I am trying to update my scripts hence my PR.

@bcoca

This comment has been minimized.

Copy link
Member

commented Oct 2, 2017

its a complex part of the code, i need time to look into it, in any case this is a noop:
become_user: "{{ansible_user}}"

as become skips any action when the login user is the same as the become user

@jeromew

This comment has been minimized.

Copy link
Contributor Author

commented Oct 2, 2017

yes I realized it's a noop.

In my ansible scripts, I have a base role for a user where a user has to do a list of operations under its own name. It takes the name of the user as a variable, let's say "u".

I run this role for many users (including the "remote_user"), for which remote_user has sudo access,
so in the tasks of this "base-user" role, I have a 'become_user: "{{u}}"'

I can then have a playbook (simplified) saying

  • {become: yes, role: base-user, u: "{{ ansible_user }}" }
  • {become: yes, role: base-user, u: user1 }
  • {become: yes, role: base-user, u: user2 }

The fact that "become_user" cannot be templated with ansible_user in newer ansible versions (it was possible with 1.8) breaks the symmetry of this role and I can't find a way to fix this (except from hardcoding the remote_user username somewhere which I would prefer to avoid)

@jeromew

This comment has been minimized.

Copy link
Contributor Author

commented Oct 3, 2017

@bcoca,

for what it's worth, let me add 3 "ux" things regarding where this is coming from and why I sent this PR.

  • "become_user" can be templated with variables, but not with "magic variables" like "ansible_user". This is very hard to understand as an ansible user
  • "remote_user" can be set (via --u, config, ..) but cannot be read (as far as i know) except by using "ansible_user", which is not available in "become_user". It feels very weird to set a config but have no access to it in a template
  • "google is my friend" was not my friend on this, because there are many people struggling with these. If the connection info vars cannot be fully templated with magic variables, we should at least document that there are 2 different tiers of variables :

a) connection info vars that can be templated with user defined variables
b) task variables that can be templated with user defined variables & magic variables
(I know that this is more complicated than that, and such a documentation may already exist)

Now that I read the code, I understand why, but this was very frustrating.

@bcoca

This comment has been minimized.

Copy link
Member

commented Oct 3, 2017

@jeromew I understand this is an issue, that is why i have not closed this PR as I look for a way to solve it w/o breaking anything else.

The design around remote_user and keywords in general is for 'ansible to read' not as much for the user to reuse. This is a limitation we are trying to overcome, but as you have seen it is not easy w/o breaking existing behaviors.

I'm not sure where the 'google is your friend' thing came from, I did not dismiss you nor this issue. Even
though I have not dropped everything else to fix this immediately, it does not mean this is not important nor that I'm not looking to solve it.

@jeromew

This comment has been minimized.

Copy link
Contributor Author

commented Oct 3, 2017

@bcoca, sorry the "google is your friend" was not meant as a negative comment on the community and I appreciate the time that you already have spent on this.

I am not an english native speaker and now that you point at it I understand how this expression can sound negative in the context. This was not my intent.

I was simply willing to say that when googling about problems with "remote_user" or "ansible_user", "ansible_ssh_user", all sort of results appear, probably because of different behaviors in different versions and that there seem to be a lot of smoke in the way people talk about these variables which makes it hard to have a clear understanding of how these variables behave.

shame on me anyway to have waited so long before upgrading my scripts and thanks for putting so much effort in deprecations warnings. I think it would have been impossible without them !

@bcoca

This comment has been minimized.

Copy link
Member

commented Oct 3, 2017

yes, sadly this has changed a lot across versions as it was never well defined, we are trying to get there but also in a way that does not break existing playbooks (or breaks the least amount of them).

bcoca added a commit to bcoca/ansible that referenced this pull request Oct 5, 2017
template x2
possible fix for ansible#31179
@bcoca bcoca referenced this pull request Oct 5, 2017

@ansibot ansibot added the stale_ci label Oct 11, 2017

@ansibot ansibot added bug and removed bugfix_pull_request labels Mar 2, 2018

@ansibot ansibot added the core_review label Oct 25, 2018

@gundalow gundalow self-assigned this Jan 11, 2019

@gundalow

This comment has been minimized.

Copy link
Contributor

commented Jan 11, 2019

Note to self:

  • Verify using a normal variable if this works
  • Test using different connection methods
  • ansible_user is a special param, may need a switch to enable, check with @bcoca
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.