From 2b7204527b0906172e5ba719f1a0fb64070c7b5e Mon Sep 17 00:00:00 2001 From: Matt Davis <6775756+nitzmahone@users.noreply.github.com> Date: Thu, 5 Jun 2025 12:21:46 -0700 Subject: [PATCH] Restore 2.18 vault tag YAML dump behavior (#85275) * Doing conditional redaction/formatting needs other bits that aren't ready for 2.19. Co-authored-by: Matt Clay --- .../fragments/templates_types_datatagging.yml | 2 - lib/ansible/_internal/_yaml/_dumper.py | 16 +------ lib/ansible/cli/inventory.py | 6 +-- lib/ansible/plugins/filter/core.py | 6 +-- .../targets/templating/tasks/main.yml | 23 ---------- test/units/_internal/_yaml/test_dumper.py | 42 +++++-------------- test/units/parsing/yaml/test_dumper.py | 25 ++++------- 7 files changed, 23 insertions(+), 97 deletions(-) diff --git a/changelogs/fragments/templates_types_datatagging.yml b/changelogs/fragments/templates_types_datatagging.yml index 9b63a67d015..74777fc80c1 100644 --- a/changelogs/fragments/templates_types_datatagging.yml +++ b/changelogs/fragments/templates_types_datatagging.yml @@ -140,8 +140,6 @@ deprecated_features: - conditionals - Conditionals using Jinja templating delimiters (e.g., ``{{``, ``{%``) should be rewritten as expressions without delimiters, unless the entire conditional value is a single template that resolves to a trusted string expression. This is useful for dynamic indirection of conditional expressions, but is limited to trusted literal string expressions. - templating - The ``disable_lookups`` option has no effect, since plugins must be updated to apply trust before any templating can be performed. - - to_yaml/to_nice_yaml filters - Implicit YAML dumping of vaulted value ciphertext is deprecated. - Set `dump_vault_tags` to explicitly specify the desired behavior. - plugin error handling - The ``AnsibleError`` constructor arg ``suppress_extended_error`` is deprecated. Using ``suppress_extended_error=True`` has the same effect as ``show_content=False``. - config - The ``ACTION_WARNINGS`` config has no effect. It previously disabled command warnings, which have since been removed. diff --git a/lib/ansible/_internal/_yaml/_dumper.py b/lib/ansible/_internal/_yaml/_dumper.py index 30c276e9cfd..44708f6f524 100644 --- a/lib/ansible/_internal/_yaml/_dumper.py +++ b/lib/ansible/_internal/_yaml/_dumper.py @@ -34,12 +34,6 @@ class _BaseDumper(SafeDumper, metaclass=abc.ABCMeta): class AnsibleDumper(_BaseDumper): """A simple stub class that allows us to add representers for our custom types.""" - # DTFIX0: need a better way to handle serialization controls during YAML dumping - def __init__(self, *args, dump_vault_tags: bool | None = None, **kwargs) -> None: - super().__init__(*args, **kwargs) - - self._dump_vault_tags = dump_vault_tags - @classmethod def _register_representers(cls) -> None: cls.add_multi_representer(AnsibleTaggedObject, cls.represent_ansible_tagged_object) @@ -49,15 +43,7 @@ class AnsibleDumper(_BaseDumper): cls.add_multi_representer(_jinja_common.VaultExceptionMarker, cls.represent_vault_exception_marker) def get_node_from_ciphertext(self, data: object) -> ScalarNode | None: - if self._dump_vault_tags is not False and (ciphertext := VaultHelper.get_ciphertext(data, with_tags=False)): - # deprecated: description='enable the deprecation warning below' core_version='2.23' - # if self._dump_vault_tags is None: - # Display().deprecated( - # msg="Implicit YAML dumping of vaulted value ciphertext is deprecated.", - # version="2.27", - # help_text="Set `dump_vault_tags` to explicitly specify the desired behavior.", - # ) - + if ciphertext := VaultHelper.get_ciphertext(data, with_tags=False): return self.represent_scalar('!vault', ciphertext, style='|') return None diff --git a/lib/ansible/cli/inventory.py b/lib/ansible/cli/inventory.py index 1a2d9d75ef4..1775f7f945a 100755 --- a/lib/ansible/cli/inventory.py +++ b/lib/ansible/cli/inventory.py @@ -14,7 +14,6 @@ import sys import typing as t import argparse -import functools from ansible import constants as C from ansible import context @@ -162,11 +161,10 @@ class InventoryCLI(CLI): def dump(stuff): if context.CLIARGS['yaml']: import yaml + from ansible.parsing.yaml.dumper import AnsibleDumper - # DTFIX0: need shared infra to smuggle custom kwargs to dumpers, since yaml.dump cannot (as of PyYAML 6.0.1) - dumper = functools.partial(AnsibleDumper, dump_vault_tags=True) - results = to_text(yaml.dump(stuff, Dumper=dumper, default_flow_style=False, allow_unicode=True)) + results = to_text(yaml.dump(stuff, Dumper=AnsibleDumper, default_flow_style=False, allow_unicode=True)) elif context.CLIARGS['toml']: results = toml_dumps(stuff) else: diff --git a/lib/ansible/plugins/filter/core.py b/lib/ansible/plugins/filter/core.py index 60d370b5a68..a082590557d 100644 --- a/lib/ansible/plugins/filter/core.py +++ b/lib/ansible/plugins/filter/core.py @@ -48,11 +48,9 @@ UUID_NAMESPACE_ANSIBLE = uuid.UUID('361E6D51-FAEC-444A-9079-341386DA8E2E') @accept_lazy_markers -def to_yaml(a, *_args, default_flow_style: bool | None = None, dump_vault_tags: bool | None = None, **kwargs) -> str: +def to_yaml(a, *_args, default_flow_style: bool | None = None, **kwargs) -> str: """Serialize input as terse flow-style YAML.""" - dumper = partial(AnsibleDumper, dump_vault_tags=dump_vault_tags) - - return yaml.dump(a, Dumper=dumper, allow_unicode=True, default_flow_style=default_flow_style, **kwargs) + return yaml.dump(a, Dumper=AnsibleDumper, allow_unicode=True, default_flow_style=default_flow_style, **kwargs) @accept_lazy_markers diff --git a/test/integration/targets/templating/tasks/main.yml b/test/integration/targets/templating/tasks/main.yml index 43efb84861c..e600aa531d3 100644 --- a/test/integration/targets/templating/tasks/main.yml +++ b/test/integration/targets/templating/tasks/main.yml @@ -234,29 +234,6 @@ - result is failed - result.msg is contains("undecryptable variable") - - name: undecryptable vault values in a scalar template should trip on YAML serialization - debug: - msg: '{{ dict_with_undecryptable_vault | to_yaml(dump_vault_tags=False) }}' - ignore_errors: true - register: result - - - assert: - that: - - result is failed - - result.msg is contains("undecryptable variable") - - - name: undecryptable vault values in a list of templates should trip on YAML serialization - debug: - msg: - - '{{ dict_with_undecryptable_vault | to_yaml(dump_vault_tags=False) }}' - ignore_errors: true - register: result - - - assert: - that: - - result is failed - - result.msg is contains("undecryptable variable") - - name: vaulted values are not trusted for templating vars: # value is {{ "not trusted" }} encrypted with "blarblar" diff --git a/test/units/_internal/_yaml/test_dumper.py b/test/units/_internal/_yaml/test_dumper.py index 870c1369cf9..0fa4ac76297 100644 --- a/test/units/_internal/_yaml/test_dumper.py +++ b/test/units/_internal/_yaml/test_dumper.py @@ -2,12 +2,11 @@ from __future__ import annotations import pytest -from ansible.errors import AnsibleUndefinedVariable, AnsibleTemplateError +from ansible.errors import AnsibleUndefinedVariable from ansible.parsing.utils.yaml import from_yaml from ansible.parsing.vault import EncryptedString from ansible.template import Templar, trust_as_template, is_trusted_as_template from units.mock.vault_helper import VaultTestHelper -from units.test_utils.controller.display import emits_warnings undecryptable_value = EncryptedString( ciphertext="$ANSIBLE_VAULT;1.1;AES256\n" @@ -17,33 +16,22 @@ undecryptable_value = EncryptedString( ) -@pytest.mark.parametrize("filter_name, dump_vault_tags", ( - ("to_yaml", True), - ("to_yaml", False), - ("to_yaml", None), # cover the future case that will trigger a deprecation warning - ("to_nice_yaml", True), - ("to_nice_yaml", False), - ("to_nice_yaml", None), # cover the future case that will trigger a deprecation warning +@pytest.mark.parametrize("filter_name", ( + "to_yaml", + "to_nice_yaml", )) -def test_yaml_dump(filter_name: str, _vault_secrets_context: VaultTestHelper, dump_vault_tags: bool) -> None: +def test_yaml_dump(filter_name: str, _vault_secrets_context: VaultTestHelper) -> None: """Verify YAML dumping round-trips only values which are expected to be supported.""" payload = dict( trusted=trust_as_template('trusted'), untrusted="untrusted", decryptable=VaultTestHelper().make_encrypted_string("hi mom"), + undecryptable=undecryptable_value, ) - if dump_vault_tags: - payload.update( - undecryptable=undecryptable_value, - ) - original = dict(a_list=[payload]) templar = Templar(variables=dict(original=original)) - - with emits_warnings(warning_pattern=[], deprecation_pattern=[]): # this will require updates once implicit vault tag dumping is deprecated - result = templar.template(trust_as_template(f"{{{{ original | {filter_name}(dump_vault_tags={dump_vault_tags}) }}}}")) - + result = templar.template(trust_as_template(f"{{{{ original | {filter_name} }}}}")) data = from_yaml(trust_as_template(result)) assert len(data) == len(original) @@ -58,13 +46,12 @@ def test_yaml_dump(filter_name: str, _vault_secrets_context: VaultTestHelper, du assert is_trusted_as_template(result_item['trusted']) assert is_trusted_as_template(result_item['untrusted']) # round-tripping trust is NOT supported - assert is_trusted_as_template(result_item['decryptable']) is not dump_vault_tags + assert not is_trusted_as_template(result_item['decryptable']) - if dump_vault_tags: - assert result_item['decryptable']._ciphertext == original_item['decryptable']._ciphertext - assert result_item['undecryptable']._ciphertext == original_item['undecryptable']._ciphertext + assert result_item['decryptable']._ciphertext == original_item['decryptable']._ciphertext + assert result_item['undecryptable']._ciphertext == original_item['undecryptable']._ciphertext - assert not is_trusted_as_template(result_item['undecryptable']) + assert not is_trusted_as_template(result_item['undecryptable']) def test_yaml_dump_undefined() -> None: @@ -74,13 +61,6 @@ def test_yaml_dump_undefined() -> None: templar.template(trust_as_template("{{ dict_with_undefined | to_yaml }}")) -def test_yaml_dump_undecryptable_without_vault_tags(_vault_secrets_context: VaultTestHelper) -> None: - templar = Templar(variables=dict(list_with_undecrypted=[undecryptable_value])) - - with pytest.raises(AnsibleTemplateError, match="undecryptable"): - templar.template(trust_as_template('{{ list_with_undecrypted | to_yaml(dump_vault_tags=false) }}')) - - @pytest.mark.parametrize("value, expected", ( ((1, 2, 3), "[1, 2, 3]\n"), ({1, 2, 3}, "!!set {1: null, 2: null, 3: null}\n"), diff --git a/test/units/parsing/yaml/test_dumper.py b/test/units/parsing/yaml/test_dumper.py index 5e268f34168..6e1ca8f694b 100644 --- a/test/units/parsing/yaml/test_dumper.py +++ b/test/units/parsing/yaml/test_dumper.py @@ -86,20 +86,14 @@ class TestAnsibleDumper(unittest.TestCase, YamlTestUtils): self._dump_string(_DEFAULT_UNDEF, dumper=self.dumper) -@pytest.mark.parametrize("filter_impl, dump_vault_tags, expected_output, expected_warning", [ - (to_yaml, True, "!vault |-\n ciphertext\n", None), - (to_yaml, None, "!vault |-\n ciphertext\n", "Implicit YAML dumping"), - (to_yaml, False, "secret plaintext\n", None), - (to_nice_yaml, True, "!vault |-\n ciphertext\n", None), - (to_nice_yaml, None, "!vault |-\n ciphertext\n", "Implicit YAML dumping"), - (to_nice_yaml, False, "secret plaintext\n", None), +@pytest.mark.parametrize("filter_impl, expected_output", [ + (to_yaml, "!vault |-\n ciphertext\n"), + (to_nice_yaml, "!vault |-\n ciphertext\n"), ]) def test_vaulted_value_dump( - filter_impl: t.Callable, - dump_vault_tags: bool | None, - expected_output: str, - expected_warning: str | None, - mocker: pytest_mock.MockerFixture + filter_impl: t.Callable, + expected_output: str, + mocker: pytest_mock.MockerFixture ) -> None: """Validate that strings tagged VaultedValue are represented properly.""" value = VaultedValue(ciphertext="ciphertext").tag("secret plaintext") @@ -108,15 +102,10 @@ def test_vaulted_value_dump( _deprecated_spy = mocker.spy(Display(), 'deprecated') - res = filter_impl(value, dump_vault_tags=dump_vault_tags) + res = filter_impl(value) assert res == expected_output - # deprecated: description='enable the assertion for the deprecation warning below' core_version='2.21' - # if expected_warning: - # assert _deprecated_spy.call_count == 1 - # assert expected_warning in _deprecated_spy.call_args.kwargs['msg'] - _test_tag = Deprecated(msg="test")