As mentioned in Data Tagging preview and testing, collection maintainers need to test their collections to ensure compatibility with core-2.19.
This is the topic where you can add your advice to other collection maintainers on how to achieve this.
2 Likes
If your unit tests start failing because you mock module arguments like this:
from ansible.module_utils import basic
args = json.dumps({'ANSIBLE_MODULE_ARGS': args})
basic._ANSIBLE_ARGS = to_bytes(args)
... execute the module ...
This no longer works, you need to use the ansible.module_utils.testing.patch_module_args() context manager instead:
from ansible.module_utils.testing import patch_module_args
with patch_module_args(args):
... execute the module ...
One way to handle this for both older and newer ansible-core versions is to create your own context manager:
@contextlib.contextmanager
def set_module_args(args):
"""
Context manager that sets module arguments for AnsibleModule
"""
if '_ansible_remote_tmp' not in args:
args['_ansible_remote_tmp'] = '/tmp'
if '_ansible_keep_remote_files' not in args:
args['_ansible_keep_remote_files'] = False
try:
from ansible.module_utils.testing import patch_module_args
except ImportError:
# Before data tagging support was merged, this was the way to go:
from ansible.module_utils import basic
serialized_args = to_bytes(json.dumps({'ANSIBLE_MODULE_ARGS': args}))
with patch.object(basic, '_ANSIBLE_ARGS', serialized_args):
yield
else:
# With data tagging support, we have a new helper for this:
with patch_module_args(args):
yield
Then you can always run:
with set_module_args({...}):
... execute the module ...
4 Likes
I thought Iâd found a good and suitable solution, but I like yours much better - thanks!
My approach was to check against the version, but using the import with try/catch is much more robust. Also, with my approach, I had to define the test cases as functions and pass them as function argument.
Just for the sake of history and understanding, hereâs my approach:
def set_module_args(args: dict, run_test_fn):
"""Patches module arguments in a version-compatible way and executes the test logic."""
if (
version.parse(ansible_version).base_version
>= version.parse("2.19").base_version
):
from ansible.module_utils.testing import patch_module_args
with patch_module_args(args):
run_test_fn()
else:
args = json.dumps({"ANSIBLE_MODULE_ARGS": args})
basic._ANSIBLE_ARGS = to_bytes(args)
run_test_fn()
def test_payload_prometheus(self):
expected_payload = {
"access": "proxy",
"basicAuth": False,
...
}
def run_test():
module = grafana_datasource.setup_module_object()
payload = grafana_datasource.get_datasource_payload(module.params)
self.assertEqual(payload, expected_payload)
set_module_args(
{
"url": "https://grafana.example.com",
"url_username": "admin",
...
},
run_test
)
1 Like
One other change I had to do in several collections I forgot to mention before: if youâre looking for specific (deprecation) warnings in module results in unit tests, you might see failures, since warnings is no longer a list of strings, but a list of ansible.module_utils.common.messages.WarningSummar objects. Iâm using the following code to work around that issue:
try:
from ansible.module_utils.common.messages import WarningSummary as _WarningSummary
except ImportError:
_WarningSummary = None
def extract_warnings_texts(result):
"""
Given the results dictionary of a module, extracts the warnings as a list of strings.
"""
warnings = []
if result.get('warnings'):
for warning in result['warnings']:
if _WarningSummary and isinstance(warning, _WarningSummary):
warnings.append(warning.details[0].msg)
continue
warnings.append(warning)
return warnings
This allows me to write tests like assert "Foo" in extract_warnings_texts(result) instead of assert "Foo" in result["warnings"].
1 Like
Hi @felixfontein I am taking a look at that issue you brought up about integration tests in
I was able to replicate the issue by reverting the changes you merged and testing against ansible-core devel branch. And though I can easily make that deprecation go away (with a minor adjustment in one of the mock classes), when doing the tests that expect errors, if I try and assert the resulting exception it blows up. Checking the results, exception="(traceback unavailable)" as you have already marked before. The way you âsolvedâ it was to look for that specific message, but I would like to better understand what is really happening with tasks when they go wrong.
I have tried to follow the discussions about DT but I donât recall anything specific to that. Are we not getting tracebacks anymore, at all? If so, then I should completely remove that assertion (instead of adding that message to it). Is the traceback going to be available in a different result data structure?
@russoz the problem is totally unrelated to Data Tagging, the problem is that the test modules in the module_helper test still use deprecated features of community.general that will be removed from community.general 11.0.0. Unfortunately ansible-test seems to be suppressing all deprecation messages, so you donât see them except if you register the result and show it with ansible.builtin.debug. For example, in
- name: test msimpleda 2
msimpleda:
a: 2
register: simple2
- name: Show results
debug:
var: simple2
you should see two deprecations for the msimpleda call. But you donât see any of them, you only see deprecations in the debug task:
TASK [module_helper : test msimpleda 2] ****************************************
task path: /path/to/community/general/tests/output/.tmp/integration/module_helper-5ec1dnp5-Ă
ĂĹĂβĹĂ/tests/integration/targets/module_helper/tasks/msimpleda.yml:32
Using module file /path/to/community/general/tests/output/.tmp/integration/module_helper-5ec1dnp5-Ă
ĂĹĂβĹĂ/tests/integration/targets/module_helper/library/msimpleda.py
Pipelining is enabled.
<testhost> ESTABLISH LOCAL CONNECTION FOR USER: felix
<testhost> EXEC /bin/sh -c '/usr/bin/python && sleep 0'
ok: [testhost] => {
"a": 2,
"attr2": "def",
"changed": false,
"invocation": {
"module_args": {
"a": 2
}
}
}
TASK [module_helper : Show results] ********************************************
task path: /path/to/community/general/tests/output/.tmp/integration/module_helper-5ec1dnp5-Ă
ĂĹĂβĹĂ/tests/integration/targets/module_helper/tasks/msimpleda.yml:37
ok: [testhost] => {
"simple2": {
"a": 2,
"attr2": "def",
"changed": false,
"deprecations": [
{
"collection_name": null,
"msg": "This class is using the old VarDict from ModuleHelper, which is deprecated. Set the class variable use_old_vardict to False and make the necessary adjustments.The old VarDict class will be removed in community.general 11.0.0",
"plugin": {
"requested_name": "msimpleda",
"resolved_name": "msimpleda",
"type": "module"
},
"version": "11.0.0"
},
{
"collection_name": null,
"msg": "Attribute attr2 is deprecated",
"plugin": {
"requested_name": "msimpleda",
"resolved_name": "msimpleda",
"type": "module"
},
"version": "9.9.9"
}
],
"failed": false
}
}
And itâs not that theyâre hidden there because they already appeared earlier. The whole ansible-test integration -vvv module_helper never printed any deprecation. Hereâs the code in ansible-test that explicitly disables deprecations: ansible/test/lib/ansible_test/_internal/ansible_util.py at aab732cb826db93265b03ca6f6f9eb1a03746975 ¡ ansible/ansible ¡ GitHub
Hi @felixfontein !
Ive gotten a few inventory plugins that have this error with 2.19
ansible.errors.AnsibleParserError: Could not add host a1 to group ip1: Encountered untrusted template or expression.
Not sure how to approach this. Any ideas? Heres the full trace for one example
self = InventoryModule(plugin_type='inventory', ansible_name=None, load_name=None)
groups = {'cost': 'cost_usd < 90', 'ip1': 'ip_address == "1.1.1.1"', 'ip2': 'ip_address != "1.1.1.1"'}
variables = {'fqdn': 'a1', 'group_names': [], 'inventory_dir': None, 'inventory_file': None, ...}, host = 'a1', strict = True
fetch_hostvars = True
def _add_host_to_composed_groups(self, groups, variables, host, strict=False, fetch_hostvars=True):
""" helper to create complex groups for plugins based on jinja2 conditionals, hosts that meet the conditional are added to group"""
# process each 'group entry'
if groups and isinstance(groups, dict):
if fetch_hostvars:
variables = combine_vars(variables, self.inventory.get_host(host).get_vars())
self.templar.available_variables = variables
for group_name in groups:
conditional = groups[group_name]
group_name = self._sanitize_group_name(group_name)
try:
result = self.templar.evaluate_conditional(conditional)
except Exception as e:
if strict:
> raise AnsibleParserError("Could not add host %s to group %s: %s" % (host, group_name, to_native(e)))
E ansible.errors.AnsibleParserError: Could not add host a1 to group ip1: Encountered untrusted template or expression.
@mikemorency your problem is likely because the string youâre templating is not marked as safe (for whatever reason). If youâre sure itâs safe to template, you can explicitly mark it as safe. Check out the plugins/lookup/dependent.py part of the community.general PR: Make ready for data tagging by felixfontein ¡ Pull Request #9833 ¡ ansible-collections/community.general ¡ GitHub That code works both with ansible-core < 2.19 and >= 2.19. (Youâre probably interested in the _make_safe() function, which you should call on the conditional youâre passing into self.templar.evaluate_conditional().)
(If this is happening in the tests, simply marking the input as trusted is fine. If this is happening on the production side, youâll have to check where the data comes from and why it isnât marked as trusted.)
I guess that was my next question. This is coming from an inventory plugin test. So I think I should mark the inputs as trusted.
But that implies that users of my inventory plugin will also need to mark their inputs as trusted where appropriate before moving to 2.19, right?
The other option, (because we are using the helper method here ansible/lib/ansible/plugins/inventory/__init__.py at 9f894b81c2137716299ea2071143b72e2a66b3f7 ¡ ansible/ansible ¡ GitHub), is to mark all inputs that could contain a template as safe for the user I think?
Yes, thatâs needed.
They wonât, because content loaded from the inventory source is already marked as trusted. Itâs only a problem in tests where you provide the data from a âfakeâ source. (I did actually test this, and it worked fine in my tests. Iâd also try it out in your case, but I would be surprised if it doesnât work.)
Oh nice, thats great then. Thank you!
@felixfontein I ran into another inventory issue.
Marking the groups inputs as safe worked. But for other parameters, like compose or keyed_groups, I am having issues.
For example, if I have an inventory config
....
compose:
datacenter: "(path | split('/'))[1]"
In 2.18 this produces "datacenter": "Eco-Datacenter",
In 2.19 this produces "datacenter": { "__ansible_unsafe": "Eco-Datacenter" }
The inventory plugin is calling LookupBase._set_composite_vars to get these values. I would expect that method to treat the inputs as trusted since they are coming from the inventory config file like you said
Actually, looking at the rest of the output, there are a ton of string properties being marked as unsafe. Many of them come directly from the inventory source. It seems like I need to process every property I pull from my inventory source and mark them as safe before adding them to the hostvars?
- Do you have that issue in tests, or in real-world situations?
- Do you see that in ansible-inventoryâs JSON output, or in actual Ansible variables? The JSON output uses
__ansible_unsafeto encode unsafe values, thatâs totally normal.
(By default everything coming from the inventory source should be unsafe. Before Data Tagging you had to do this manually, only now itâs done by default. Otherwise you end up with tons of security issues, see for example Remote Code Execution in Ansible dynamic inventory plugins | die-welt.net)
The JSON output uses
__ansible_unsafeto encode unsafe values, thatâs totally normal.
I see. Yes, I get "datacenter": { "__ansible_unsafe": "Eco-Datacenter" } when running ansible-inventory, but when use the variable in a playbook, I get "datacenter": "Eco-Datacenter".
So thats expected and not a problem 
I think I am good again, thank you!
BTW, two related links to __ansible_unsafe:
1 Like
Hi everyone -
Iâve changed the forum tag from data-tagging to templating and did a heavy-handed edit of topic titles in a similar manner. I know this is a bit of a hammer approach, but it was felt that folks were seeing data tagging as a feature, and thus ignoring the bigger impacting changes in core and the need for testing in the templating area.
1 Like