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

AnsibleUnsafeText not casted to basestring when calling from_yaml #70357

Closed
m-bucher opened this issue Jun 29, 2020 · 8 comments
Closed

AnsibleUnsafeText not casted to basestring when calling from_yaml #70357

m-bucher opened this issue Jun 29, 2020 · 8 comments
Labels
affects_2.9 This issue/PR affects Ansible v2.9 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

@m-bucher
Copy link

SUMMARY

Using a Jinja2 expression using lookup() and from_yaml produces an error, if the C-Extension of PyYAML is used.
The reason seems to be that Ansible's from_yaml-filter calls the YAML-parser with an instance of AnsibleUnsafeText
The C-extension of PyYAML explicitly denies subclasses of string/basestring: https://github.com/yaml/pyyaml/blob/23c952fe08b2e7ea0f8d7673f45b17547e331f4b/ext/_yaml.pyx#L299

See also https://bugs.launchpad.net/ubuntu/+source/ansible/+bug/1880226

ISSUE TYPE
  • Bug Report
COMPONENT NAME

core

ANSIBLE VERSION
ansible 2.9.9
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/root/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.7.5 (default, Apr  2 2020, 13:16:51) [GCC 4.8.5 20150623 (Red Hat 4.8.5-39)]
CONFIGURATION

OS / ENVIRONMENT

CentOS 7
In my case Ansible only uses the C implentation of PyYAML, if salt has been installed from https://repo.saltstack.com/yum/redhat/7/x86_64/3000/
I do not use the PyYAML package from the saltstack-repository and even if I do it does not work.
As soon as I uninstall the salt-package, Ansible uses the native-python YAML-parser of PyYAML and the filter works.

STEPS TO REPRODUCE

It is important that Ansible uses PyYAML with the libyaml C-library.
If it uses the YAML-parser written in plain python the error does not occur.
For me this was -strangely- only the case if salt was installed (see OS / ENVIRONMENT)

---
- hosts: localhost
  tasks:
  - debug:
      msg: "{{ lookup('file', 'testfile.yaml') | from_yaml}}"
...

testfile.yaml:

---
Hello: World
...
EXPECTED RESULTS

The file content read by lookup() should be accepted by the from_yaml-filter.

ACTUAL RESULTS
TASK [debug] **********************************************************************************************************************************************
fatal: [localhost]: FAILED! => {"msg": "Unexpected templating type error occurred on ({{ lookup('file', 'testfile.yaml') | from_yaml}}): a string or stream input is required"}
@ansibot
Copy link
Contributor

ansibot commented Jun 29, 2020

Files identified in the description:
None

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 Jun 29, 2020
@sivel
Copy link
Member

sivel commented Jun 29, 2020

Related PR: #65002

We could potentially merge the above, and use it within from_yaml, as this fits the use case and description of unwrap_var:

This should be used minimally, and only in very specific cases, such as when passing data to a remote
Python API where the receiving python code does not understand our subclasses, potentially due to
performing type checks like if type(value) == 'str':

@samdoran samdoran removed the needs_triage Needs a first human triage before being processed. label Jun 30, 2020
@ansibot ansibot added the has_pr This issue has an associated PR. label Jun 30, 2020
sivel added a commit to sivel/ansible that referenced this issue Jun 30, 2020
@sivel
Copy link
Member

sivel commented Jun 30, 2020

Can you verify if #65002 resolves your issue?

@m-bucher
Copy link
Author

m-bucher commented Jul 1, 2020

Yes, it does.
Thanks @sivel !

@sivel
Copy link
Member

sivel commented Jul 1, 2020

I wanted to understand why this is happening, and it led me to the fact that salt is doing something that is probably not the best idea. However, in Ansible 2.10 this would no longer happen.

  1. Before 2.10 we had a saltstack connection plugin
  2. That plugin imported the salt python lib
  3. In our code, to pre-populate our configs, we did a list(connection_loader.all(class_only=True)) which caused all connection plugins to be imported
  4. The salt python lib does yaml.SafeLoader = yaml.CSafeLoader which means it infects all uses of yaml.SafeLoader.
  5. Even though we weren't explicitly using CSafeLoader in our from_yaml filter, because salt was installed, it effectively forced all uses of SafeLoader to actually use CSafeLoader.

In the end, I'm going to make some changes to explicitly select the Loader, so what is done in #65002 will still be necessary.

sivel added a commit to sivel/ansible that referenced this issue Jul 2, 2020
@MrGitBlob
Copy link

I've also just hit this.
Can't even use debug to dump a var, after installing python2-salt (v 3000-138.1) on el7.

As soon as python2-salt goes in, i'm getting this:
[WARNING]: Failure using method (v2_runner_on_ok) in callback plugin (<ansible.plugins.callback.yaml.CallbackModule object at 0x7faec54eeb50>): value must be a string

Remove python2-salt, and it works again.

@sivel
Copy link
Member

sivel commented Sep 2, 2020

In Ansible 2.10 this will no longer be a problem. Salt does a bad thing in its code, by monkey patching CLoader overtop of Loader. Because we have a salt connection plugin in Ansible <2.10, and we pre-load all connection plugins, it causes the salt connection to import salt, which taints pyyaml. We had not used the CLoader in this code path, so our code would not have created the issue.

Unfortunately due to the way the salt code is written, there isn't much we can do here In Ansible versions before 2.10.

@s-hertel
Copy link
Contributor

s-hertel commented Sep 3, 2021

Closing as per above

@s-hertel s-hertel closed this as completed Sep 3, 2021
@ansible ansible locked and limited conversation to collaborators Oct 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.9 This issue/PR affects Ansible v2.9 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