From db819d2514d053235746eef7d28fa841e3f5c053 Mon Sep 17 00:00:00 2001 From: Matt Davis <6775756+nitzmahone@users.noreply.github.com> Date: Wed, 25 Jun 2025 12:43:57 -0700 Subject: [PATCH] ensure transform_to_native_types converts keys (#85389) * added basic key visitor support to variable visitor (off by default) * transform_to_native_types enables new key visit/conversion * add test Co-authored-by: Matt Clay --- lib/ansible/_internal/_json/__init__.py | 19 ++++++++++++++----- .../_internal/_json/_profiles/_legacy.py | 2 +- lib/ansible/utils/vars.py | 1 + test/units/utils/test_vars.py | 16 +++++++++++++++- 4 files changed, 31 insertions(+), 7 deletions(-) diff --git a/lib/ansible/_internal/_json/__init__.py b/lib/ansible/_internal/_json/__init__.py index 21f56f68a79..33ea6e84446 100644 --- a/lib/ansible/_internal/_json/__init__.py +++ b/lib/ansible/_internal/_json/__init__.py @@ -81,6 +81,7 @@ class AnsibleVariableVisitor: convert_custom_scalars: bool = False, convert_to_native_values: bool = False, apply_transforms: bool = False, + visit_keys: bool = False, encrypted_string_behavior: EncryptedStringBehavior = EncryptedStringBehavior.DECRYPT, ): super().__init__() # supports StateTrackingMixIn @@ -92,6 +93,7 @@ class AnsibleVariableVisitor: self.convert_custom_scalars = convert_custom_scalars self.convert_to_native_values = convert_to_native_values self.apply_transforms = apply_transforms + self.visit_keys = visit_keys self.encrypted_string_behavior = encrypted_string_behavior if apply_transforms: @@ -134,6 +136,13 @@ class AnsibleVariableVisitor: return result + def _visit_key(self, key: t.Any) -> t.Any: + """Internal implementation to recursively visit a key if visit_keys is enabled.""" + if not self.visit_keys: + return key + + return self._visit(None, key) # key=None prevents state tracking from seeing the key as value + def _visit(self, key: t.Any, value: _T) -> _T: """Internal implementation to recursively visit a data structure's contents.""" self._current = key # supports StateTrackingMixIn @@ -144,15 +153,15 @@ class AnsibleVariableVisitor: value = self._template_engine.transform(value) value_type = type(value) - # DTFIX0: need to handle native copy for keys too if self.convert_to_native_values and isinstance(value, _datatag.AnsibleTaggedObject): value = value._native_copy() value_type = type(value) result: _T - # DTFIX0: the visitor is ignoring dict/mapping keys except for debugging and schema-aware checking, it should be doing type checks on keys - # keep in mind the allowed types for keys is a more restrictive set than for values (str and tagged str only, not EncryptedString) + # DTFIX-FUTURE: Visitor generally ignores dict/mapping keys by default except for debugging and schema-aware checking. + # It could be checking keys destined for variable storage to apply more strict rules about key shape and type. + # DTFIX0: some type lists being consulted (the ones from datatag) are probably too permissive, and perhaps should not be dynamic if (result := self._early_visit(value, value_type)) is not _sentinel: @@ -160,7 +169,7 @@ class AnsibleVariableVisitor: # DTFIX7: de-duplicate and optimize; extract inline generator expressions and fallback function or mapping for native type calculation? elif value_type in _ANSIBLE_ALLOWED_MAPPING_VAR_TYPES: # check mappings first, because they're also collections with self: # supports StateTrackingMixIn - result = AnsibleTagHelper.tag_copy(value, ((k, self._visit(k, v)) for k, v in value.items()), value_type=value_type) + result = AnsibleTagHelper.tag_copy(value, ((self._visit_key(k), self._visit(k, v)) for k, v in value.items()), value_type=value_type) elif value_type in _ANSIBLE_ALLOWED_NON_SCALAR_COLLECTION_VAR_TYPES: with self: # supports StateTrackingMixIn result = AnsibleTagHelper.tag_copy(value, (self._visit(k, v) for k, v in enumerate(t.cast(t.Iterable, value))), value_type=value_type) @@ -174,7 +183,7 @@ class AnsibleVariableVisitor: result = str(value) # type: ignore[assignment] elif self.convert_mapping_to_dict and _internal.is_intermediate_mapping(value): with self: # supports StateTrackingMixIn - result = {k: self._visit(k, v) for k, v in value.items()} # type: ignore[assignment] + result = {self._visit_key(k): self._visit(k, v) for k, v in value.items()} # type: ignore[assignment] elif self.convert_sequence_to_list and _internal.is_intermediate_iterable(value): with self: # supports StateTrackingMixIn result = [self._visit(k, v) for k, v in enumerate(t.cast(t.Iterable, value))] # type: ignore[assignment] diff --git a/lib/ansible/_internal/_json/_profiles/_legacy.py b/lib/ansible/_internal/_json/_profiles/_legacy.py index d9f26302d90..4c2f36a2c16 100644 --- a/lib/ansible/_internal/_json/_profiles/_legacy.py +++ b/lib/ansible/_internal/_json/_profiles/_legacy.py @@ -1,6 +1,6 @@ """ Backwards compatibility profile for serialization other than inventory (which should use inventory_legacy for backward-compatible trust behavior). -Behavior is equivalent to pre 2.18 `AnsibleJSONEncoder` with vault_to_text=True. +Behavior is equivalent to pre 2.19 `AnsibleJSONEncoder` with vault_to_text=True. """ from __future__ import annotations as _annotations diff --git a/lib/ansible/utils/vars.py b/lib/ansible/utils/vars.py index 5fffc807639..fb983103fd1 100644 --- a/lib/ansible/utils/vars.py +++ b/lib/ansible/utils/vars.py @@ -306,6 +306,7 @@ def transform_to_native_types( convert_custom_scalars=True, convert_to_native_values=True, apply_transforms=True, + visit_keys=True, # ensure that keys are also converted encrypted_string_behavior=_json.EncryptedStringBehavior.REDACT if redact else _json.EncryptedStringBehavior.DECRYPT, ) diff --git a/test/units/utils/test_vars.py b/test/units/utils/test_vars.py index 620c6352018..b8c1b40d848 100644 --- a/test/units/utils/test_vars.py +++ b/test/units/utils/test_vars.py @@ -23,8 +23,10 @@ from collections import defaultdict from unittest import mock import unittest + from ansible.errors import AnsibleError -from ansible.utils.vars import combine_vars, merge_hash +from ansible._internal._datatag._tags import Origin +from ansible.utils.vars import combine_vars, merge_hash, transform_to_native_types class TestVariableUtils(unittest.TestCase): @@ -274,3 +276,15 @@ class TestVariableUtils(unittest.TestCase): "b": high['b'] + [1, 1, 2] } self.assertEqual(merge_hash(low, high, True, 'prepend_rp'), expected) + + +def test_transform_to_native_types() -> None: + """Verify that transform_to_native_types results in native types for both keys and values.""" + value = {Origin(description="blah").tag("tagged_key"): Origin(description="blah").tag("value with tagged key")} + + result = transform_to_native_types(value) + + assert result == dict(tagged_key="value with tagged key") + + assert all(type(key) is str for key in result.keys()) # pylint: disable=unidiomatic-typecheck + assert all(type(value) is str for value in result.values()) # pylint: disable=unidiomatic-typecheck