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

Docker* connection plugins: allow to pass extra environment variables when running commands #940

Merged
merged 3 commits into from
Jul 25, 2024

Conversation

felixfontein
Copy link
Collaborator

SUMMARY

Fixes #937.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

docker connection plugin
docker_api connection plugin

@agowa
Copy link

agowa commented Jul 25, 2024

I tested your pr locally and it appears to work just fine. However after looking into the code I'd like to bring your attention to the little (often overlooked) detail of yaml that the key of a dict can be any arbitrary node and even contain itself...

The content of a mapping node is an unordered set of key/value node pairs, with the restriction that each of the keys is unique. YAML places no further restrictions on the nodes. In particular, keys may be arbitrary nodes, the same node may be used as the value of several key/value pairs and a mapping could even contain itself as a key or a value.

And a "node" in this context is defined as:

A YAML node represents a single native data structure. Such nodes have content of one of three kinds: scalar, sequence or mapping.

https://yaml.org/spec/1.2.2/#3211-nodes

I do not know how much the parser within ansible-core will already filter out, however as you did a type check on the value doing another one on the key may also be a good idea.

(Some examples: https://yaml.org/spec/1.2.2/#example-single-pair-implicit-entries)

@felixfontein
Copy link
Collaborator Author

I do not know how much the parser within ansible-core will already filter out,

It does not filter anything out, and in fact you can even got worse things since you could create a lookup plugin or Jinja2 filter that returns arbitrary Python objects and pass them on to the plugin. So basically the dictionary can contain arbitrary Python objects as keys or values. (For modules this isn't possible, there everything is passed through JSON so dictionary keys can only be strings.)

however as you did a type check on the value doing another one on the key may also be a good idea.

If you look more closely you can see that I actually check both the key and the value (for val, what in ((k, 'Key'), (v, 'Value')):, here k, v are key and value in the dictionary), exactly for this reason :)

@felixfontein felixfontein merged commit 7464002 into ansible-collections:main Jul 25, 2024
127 checks passed
@felixfontein felixfontein deleted the env branch July 25, 2024 19:26
@agowa
Copy link

agowa commented Jul 26, 2024

oh, sorry then. I must have overlooked that one line. I somehow directly jumped to the "isinstanceof" one...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

community.docker.docker add environment var overwrite
2 participants