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

Plugin development documentation requires updates which address handling Unsafe value wrappers #83255

Open
1 task done
drawks opened this issue May 15, 2024 · 5 comments
Open
1 task done

Comments

@drawks
Copy link

drawks commented May 15, 2024

Summary

Since the changes introduced in #82293 it is unclear how plugin developers are meant to intentionally access the the raw value/types which are wrapped in various AnsibleUnsafe proxy wrappers. The current ansible code calls a private method _strip_unsafe against various proxy objects, but that same method is marked for deprecation in the 2.17 release.

The documentation would benefit from a clear explanation as to why the unsafe proxy types are used along with some examples of how such objects can be worked with in code paths which require the concrete underlying types. For example, the current developer documentation could demonstrate passing some AnsibleUnsafe wrapped values which have been captured in a callback plugin's v2_runner_on_ok method to an external JSON/YAML/Protobuf based API.

Issue Type

Documentation Report

Component Name

devel/docs/docsite/rst/dev_guide/developing_plugins.rst

Ansible Version

$ ansible --version
ansible [core 2.14.14]
  config file = /etc/ansible/ansible.cfg
  configured module search path = ['/home/vagrant/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3.9/site-packages/ansible
  ansible collection location = /home/vagrant/.ansible/collections:/usr/share/ansible/collections
  executable location = /usr/bin/ansible
  python version = 3.9.18 (main, Jan 24 2024, 00:00:00) [GCC 11.4.1 20231218 (Red Hat 11.4.1-3)] (/usr/bin/python3)
  jinja version = 3.1.2
  libyaml = True

Configuration

# if using a version older than ansible-core 2.12 you should omit the '-t all'
$ ansible-config dump --only-changed -t all
CONFIG_FILE() = /etc/ansible/ansible.cfg

OS / Environment

Redhat Enterprise Linux 9.4

Additional Information

Without the documentation guidance as requested I fear that the overall quality of plugins will degrade as various plugin developers reach for a wide range of arcane solutions which may be unnecessary or become broken again in a future release when the next code/logic changes in the proxy wrappers happens.

Code of Conduct

  • I agree to follow the Ansible Code of Conduct
@ansibot ansibot added needs_triage Needs a first human triage before being processed. affects_2.14 labels May 15, 2024
@ansibot
Copy link
Contributor

ansibot commented May 15, 2024

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.

@sivel
Copy link
Member

sivel commented May 15, 2024

Can you explain more about how you are using the data? If you are using the JSON and YAML encoders included with ansible, then it should be handled for you. Ultimately since we are inverting the trust model, and unsafe won't really be marked as such, we're not recommending any specific general work arounds. The goal is that the devs shouldn't need to concern themselves.

@drawks
Copy link
Author

drawks commented May 15, 2024

Can you explain more about how you are using the data?

I have a callback plugin which captures some values from tasks which are completed and serializes those values as YAML for passing to another tool. The specifics of my use case seem unimportant aside from the fact that I'm using the API as described and provided to add this feature and the datatype proxy wrappers as implemented in 2.14.14 (and later?) provide additional obstructions to passing the data to other non-ansible consumers.

If you are using the JSON and YAML encoders included with ansible, then it should be handled for you.

Maybe I'm missing where this is spelled out in the documentation. Where are the encoders which are included with ansible that are meant to be used by plugin developers?

in 2.14.14 I found that ansible.module_utils.common.json uses, the aforementioned, ._strip_unsafe method, but no equivalent special handling is found in ansible.module_utils.common.yaml

in the devel branch I see that _strip_unsafe is not present in the ansible.module_utils.common.json anylonger and instead there are some other calls which are conditional on is_unsafe and appear to handling casting to serialization safe types. Again there is no similar code paths or handling in the ansible.module_utils.common.yaml.

And in either case, even if the documentation had guided me to those ansible provided modules AND the YAML version did work for my case, which it doesn't, I would still have no such guidance for using some other arbitrary serialization library.

Ultimately since we are inverting the trust model, and unsafe won't really be marked as such, we're not recommending any specific general work arounds. The goal is that the devs shouldn't need to concern themselves.

I'm glad to hear that the plan is for this not to be a concern for myself as a developer, but it currently is a concern and I've neither found any guidance in the documentation that clears up how this is currently meant to be handled in the release as provided in the current RedHat release.

AFAICT my code, which calls the _strip_unsafe method before serializing as YAML is /as correct/ as the referenced JSON module provided internal to ansible, but as mentioned above, this method is marked for deprecation and the current development branch neither provides a working YAML solution nor documentation that would apply in the case that another serialization beside JSON or YAML is used.

@sivel
Copy link
Member

sivel commented May 15, 2024

>>> import yaml
>>> from ansible.utils.unsafe_proxy import AnsibleUnsafeText
>>> from ansible.parsing.yaml.dumper import AnsibleDumper
>>> yaml.dump({'foo': AnsibleUnsafeText('bar')}, Dumper=AnsibleDumper)
'foo: bar\n'

@drawks
Copy link
Author

drawks commented May 15, 2024

Cool, so now you've given an example for YAML, which I truly appreciate, but there is still no real pointer in the documentation that would have directed me to this, nor is there a general solution for serialization beyond YAML and JSON.

@bcoca bcoca added data_tagging and removed needs_triage Needs a first human triage before being processed. labels May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants