From 9f0a8075e36056ff4c292a6a11fe52bc48403156 Mon Sep 17 00:00:00 2001 From: Matt Davis <6775756+nitzmahone@users.noreply.github.com> Date: Thu, 5 Jun 2025 17:21:48 -0700 Subject: [PATCH] Prevent template lookup and action from masking `ansible_managed` value (#85075) * deprecate DEFAULT_MANAGED_STR and prevent masking of ansible_managed var * adjust public API behavior * restore backward-compatible behavior on existing public API --- .../fragments/templates_types_datatagging.yml | 2 - .../fragments/unmask_ansible_managed.yml | 3 + .../_internal/_templating/_template_vars.py | 72 +++++++++++++++++++ lib/ansible/config/base.yml | 4 ++ lib/ansible/plugins/action/template.py | 10 ++- lib/ansible/plugins/lookup/template.py | 9 ++- lib/ansible/template/__init__.py | 62 +++------------- .../lookup_template/tasks/ansible_managed.yml | 18 +++++ .../targets/lookup_template/tasks/main.yml | 1 + .../templates/uses_ansible_managed.j2 | 1 + .../template/tasks/ansible_managed.yml | 20 ++++++ .../targets/template/tasks/main.yml | 20 +----- .../templates/uses_ansible_managed.j2 | 1 + test/sanity/code-smell/black.json | 1 + test/units/template/test_template.py | 11 +++ 15 files changed, 158 insertions(+), 77 deletions(-) create mode 100644 changelogs/fragments/unmask_ansible_managed.yml create mode 100644 lib/ansible/_internal/_templating/_template_vars.py create mode 100644 test/integration/targets/lookup_template/tasks/ansible_managed.yml create mode 100644 test/integration/targets/lookup_template/templates/uses_ansible_managed.j2 create mode 100644 test/integration/targets/template/tasks/ansible_managed.yml create mode 100644 test/integration/targets/template/templates/uses_ansible_managed.j2 diff --git a/changelogs/fragments/templates_types_datatagging.yml b/changelogs/fragments/templates_types_datatagging.yml index 74777fc80c1..69c05535456 100644 --- a/changelogs/fragments/templates_types_datatagging.yml +++ b/changelogs/fragments/templates_types_datatagging.yml @@ -127,8 +127,6 @@ bugfixes: Previously, there were cases where a non-list could be received. deprecated_features: - - templating - The ``ansible_managed`` variable available for certain templating scenarios, such as the ``template`` action and ``template`` lookup has been deprecated. - Define and use a custom variable instead of relying on ``ansible_managed``. - display - The ``Display.get_deprecation_message`` method has been deprecated. Call ``Display.deprecated`` to display a deprecation message, or call it with ``removed=True`` to raise an ``AnsibleError``. - config - The ``DEFAULT_JINJA2_NATIVE`` option has no effect. diff --git a/changelogs/fragments/unmask_ansible_managed.yml b/changelogs/fragments/unmask_ansible_managed.yml new file mode 100644 index 00000000000..80f79ea1663 --- /dev/null +++ b/changelogs/fragments/unmask_ansible_managed.yml @@ -0,0 +1,3 @@ +minor_changes: +- template action and lookup plugin - The value of the ``ansible_managed`` variable (if set) will not be masked by the ``template`` action and lookup. + Previously, the value calculated by the ``DEFAULT_MANAGED_STR`` configuration option always masked the variable value during plugin execution, preventing runtime customization. diff --git a/lib/ansible/_internal/_templating/_template_vars.py b/lib/ansible/_internal/_templating/_template_vars.py new file mode 100644 index 00000000000..b5b1e4c0e9a --- /dev/null +++ b/lib/ansible/_internal/_templating/_template_vars.py @@ -0,0 +1,72 @@ +from __future__ import annotations as _annotations + +import datetime as _datetime +import os as _os +import pwd as _pwd +import time as _time + +from ansible import constants as _constants +from ansible.module_utils._internal import _datatag + + +def generate_ansible_template_vars( + path: str, + fullpath: str | None = None, + dest_path: str | None = None, + include_ansible_managed: bool = True, +) -> dict[str, object]: + """ + Generate and return a dictionary with variable metadata about the template specified by `fullpath`. + If `fullpath` is `None`, `path` will be used instead. + """ + # deprecated description="update the ansible.windows collection to inline this logic instead of calling this internal function" core_version="2.23" + if fullpath is None: + fullpath = _os.path.abspath(path) + + template_path = fullpath + template_stat = _os.stat(template_path) + + template_uid: int | str + + try: + template_uid = _pwd.getpwuid(template_stat.st_uid).pw_name + except KeyError: + template_uid = template_stat.st_uid + + temp_vars = dict( + template_host=_os.uname()[1], + template_path=path, + template_mtime=_datetime.datetime.fromtimestamp(template_stat.st_mtime), + template_uid=template_uid, + template_run_date=_datetime.datetime.now(), + template_destpath=dest_path, + template_fullpath=fullpath, + ) + + if include_ansible_managed: # only inject the config default value if the variable wasn't set + temp_vars['ansible_managed'] = _generate_ansible_managed(template_stat) + + return temp_vars + + +def _generate_ansible_managed(template_stat: _os.stat_result) -> str: + """Generate and return the `ansible_managed` variable.""" + # deprecated description="remove the `_generate_ansible_managed` function and use a constant instead" core_version="2.23" + + from ansible.template import trust_as_template + + managed_default = _constants.config.get_config_value('DEFAULT_MANAGED_STR') + + managed_str = managed_default.format( + # IMPORTANT: These values must be constant strings to avoid template injection. + # Use Jinja template expressions where variables are needed. + host="{{ template_host }}", + uid="{{ template_uid }}", + file="{{ template_path }}", + ) + + ansible_managed = _time.strftime(managed_str, _time.localtime(template_stat.st_mtime)) + ansible_managed = _datatag.AnsibleTagHelper.tag_copy(managed_default, ansible_managed) + ansible_managed = trust_as_template(ansible_managed) + + return ansible_managed diff --git a/lib/ansible/config/base.yml b/lib/ansible/config/base.yml index 70fb48e9ea2..b9c9b11c1d3 100644 --- a/lib/ansible/config/base.yml +++ b/lib/ansible/config/base.yml @@ -906,6 +906,10 @@ DEFAULT_MANAGED_STR: ini: - {key: ansible_managed, section: defaults} yaml: {key: defaults.ansible_managed} + deprecated: + why: The `ansible_managed` variable can be set just like any other variable, or a different variable can be used. + version: "2.23" + alternatives: Set the `ansible_managed` variable, or use any custom variable in templates. DEFAULT_MODULE_ARGS: name: Adhoc default arguments default: ~ diff --git a/lib/ansible/plugins/action/template.py b/lib/ansible/plugins/action/template.py index e9e46826dfc..b1c611e9a9d 100644 --- a/lib/ansible/plugins/action/template.py +++ b/lib/ansible/plugins/action/template.py @@ -25,7 +25,8 @@ from ansible.module_utils.common.text.converters import to_bytes, to_text, to_na from ansible.module_utils.parsing.convert_bool import boolean from ansible.module_utils.six import string_types from ansible.plugins.action import ActionBase -from ansible.template import generate_ansible_template_vars, trust_as_template +from ansible.template import trust_as_template +from ansible._internal._templating import _template_vars class ActionModule(ActionBase): @@ -115,7 +116,12 @@ class ActionModule(ActionBase): # add ansible 'template' vars temp_vars = task_vars.copy() - temp_vars.update(generate_ansible_template_vars(self._task.args.get('src', None), fullpath=source, dest_path=dest)) + temp_vars.update(_template_vars.generate_ansible_template_vars( + path=self._task.args.get('src', None), + fullpath=source, + dest_path=dest, + include_ansible_managed='ansible_managed' not in temp_vars, # do not clobber ansible_managed when set by the user + )) overrides = dict( block_start_string=block_start_string, diff --git a/lib/ansible/plugins/lookup/template.py b/lib/ansible/plugins/lookup/template.py index a68539f4dad..e4f459c218f 100644 --- a/lib/ansible/plugins/lookup/template.py +++ b/lib/ansible/plugins/lookup/template.py @@ -105,7 +105,8 @@ import os from ansible.errors import AnsibleError from ansible.plugins.lookup import LookupBase -from ansible.template import generate_ansible_template_vars, trust_as_template +from ansible.template import trust_as_template +from ansible._internal._templating import _template_vars from ansible.utils.display import Display @@ -157,7 +158,11 @@ class LookupModule(LookupBase): # argument. # FIXME: why isn't this a chainmap with a sacrificial bottom layer? vars = deepcopy(variables) - vars.update(generate_ansible_template_vars(term, fullpath=lookupfile)) + vars.update(_template_vars.generate_ansible_template_vars( + path=term, + fullpath=lookupfile, + include_ansible_managed='ansible_managed' not in vars, # do not clobber ansible_managed when set by the user + )) vars.update(lookup_template_vars) overrides = dict( diff --git a/lib/ansible/template/__init__.py b/lib/ansible/template/__init__.py index edab980e4eb..7e3a93bf3ba 100644 --- a/lib/ansible/template/__init__.py +++ b/lib/ansible/template/__init__.py @@ -1,22 +1,18 @@ from __future__ import annotations as _annotations import contextlib as _contextlib -import datetime as _datetime import io as _io import os as _os -import pwd as _pwd -import time as _time import typing as _t from jinja2 import environment as _environment from ansible import _internal -from ansible import constants as _constants from ansible import errors as _errors from ansible._internal._datatag import _tags, _wrappers -from ansible._internal._templating import _jinja_bits, _engine, _jinja_common +from ansible._internal._templating import _jinja_bits, _engine, _jinja_common, _template_vars + from ansible.module_utils import datatag as _module_utils_datatag -from ansible.module_utils._internal import _datatag from ansible.utils.display import Display as _Display if _t.TYPE_CHECKING: # pragma: nocover @@ -352,57 +348,17 @@ class Templar: ) -def generate_ansible_template_vars(path: str, fullpath: str | None = None, dest_path: str | None = None) -> dict[str, object]: +def generate_ansible_template_vars( + path: str, + fullpath: str | None = None, + dest_path: str | None = None, +) -> dict[str, object]: """ Generate and return a dictionary with variable metadata about the template specified by `fullpath`. If `fullpath` is `None`, `path` will be used instead. """ - if fullpath is None: - fullpath = _os.path.abspath(path) - - template_path = fullpath - template_stat = _os.stat(template_path) - - template_uid: int | str - - try: - template_uid = _pwd.getpwuid(template_stat.st_uid).pw_name - except KeyError: - template_uid = template_stat.st_uid - - managed_default = _constants.config.get_config_value('DEFAULT_MANAGED_STR') - - managed_str = managed_default.format( - # IMPORTANT: These values must be constant strings to avoid template injection. - # Use Jinja template expressions where variables are needed. - host="{{ template_host }}", - uid="{{ template_uid }}", - file="{{ template_path }}", - ) - - ansible_managed = _time.strftime(managed_str, _time.localtime(template_stat.st_mtime)) - # DTFIX7: this should not be tag_copy, it should either be an origin copy or some kind of derived origin - ansible_managed = _datatag.AnsibleTagHelper.tag_copy(managed_default, ansible_managed) - ansible_managed = trust_as_template(ansible_managed) - ansible_managed = _module_utils_datatag.deprecate_value( - value=ansible_managed, - msg="The `ansible_managed` variable is deprecated.", - help_text="Define and use a custom variable instead.", - version='2.23', - ) - - temp_vars = dict( - template_host=_os.uname()[1], - template_path=path, - template_mtime=_datetime.datetime.fromtimestamp(template_stat.st_mtime), - template_uid=template_uid, - template_run_date=_datetime.datetime.now(), - template_destpath=dest_path, - template_fullpath=fullpath, - ansible_managed=ansible_managed, - ) - - return temp_vars + # deprecated description="deprecate `generate_ansible_template_vars`, collections should inline the necessary variables" core_version="2.23" + return _template_vars.generate_ansible_template_vars(path=path, fullpath=fullpath, dest_path=dest_path, include_ansible_managed=True) def trust_as_template(value: _TTrustable) -> _TTrustable: diff --git a/test/integration/targets/lookup_template/tasks/ansible_managed.yml b/test/integration/targets/lookup_template/tasks/ansible_managed.yml new file mode 100644 index 00000000000..21c0f740956 --- /dev/null +++ b/test/integration/targets/lookup_template/tasks/ansible_managed.yml @@ -0,0 +1,18 @@ +# deprecated: description='ansible_managed has been removed' core_version='2.23' +- name: invoke template lookup with content using default injected `ansible_managed` + debug: + msg: "{{ lookup('template', 'uses_ansible_managed.j2') }}" + register: default_lookup_result + +- name: invoke template lookup with content using explicitly set `ansible_managed` var + vars: + ansible_managed: '{{ "a " ~ "dynamic " ~ "ansible_managed value" }}' + debug: + msg: "{{ lookup('template', 'uses_ansible_managed.j2') }}" + register: overridden_lookup_result + +- name: validate ansible_managed results + assert: + that: + - default_lookup_result.msg is contains "Ansible managed" + - overridden_lookup_result.msg is contains "a dynamic ansible_managed value" diff --git a/test/integration/targets/lookup_template/tasks/main.yml b/test/integration/targets/lookup_template/tasks/main.yml index 48838f64079..a0b9bf47b9a 100644 --- a/test/integration/targets/lookup_template/tasks/main.yml +++ b/test/integration/targets/lookup_template/tasks/main.yml @@ -32,6 +32,7 @@ - lookup('template', 'dict.j2') is not mapping - include_tasks: trim_blocks.yml +- include_tasks: ansible_managed.yml - name: Verify templates with no rendered content return `None` assert: diff --git a/test/integration/targets/lookup_template/templates/uses_ansible_managed.j2 b/test/integration/targets/lookup_template/templates/uses_ansible_managed.j2 new file mode 100644 index 00000000000..ef7e08e741c --- /dev/null +++ b/test/integration/targets/lookup_template/templates/uses_ansible_managed.j2 @@ -0,0 +1 @@ +{{ ansible_managed }} diff --git a/test/integration/targets/template/tasks/ansible_managed.yml b/test/integration/targets/template/tasks/ansible_managed.yml new file mode 100644 index 00000000000..26c864c98f2 --- /dev/null +++ b/test/integration/targets/template/tasks/ansible_managed.yml @@ -0,0 +1,20 @@ +# deprecated: description='ansible_managed has been removed' core_version='2.23' +- name: invoke template action with content using default injected `ansible_managed` + template: + src: uses_ansible_managed.j2 + dest: '{{output_dir}}/default_am.txt' + register: default_action_result + +- name: invoke template action with content using explicitly set `ansible_managed` var + vars: + ansible_managed: '{{ "a " ~ "dynamic " ~ "ansible_managed value" }}' + template: + src: uses_ansible_managed.j2 + dest: '{{output_dir}}/overridden_am.txt' + register: overridden_action_result + +- name: validate ansible_managed results + assert: + that: + - lookup('file', default_action_result.dest) is contains "Ansible managed" + - lookup('file', overridden_action_result.dest) is contains "a dynamic ansible_managed value" diff --git a/test/integration/targets/template/tasks/main.yml b/test/integration/targets/template/tasks/main.yml index d0c59569e57..1a7ff3ae0aa 100644 --- a/test/integration/targets/template/tasks/main.yml +++ b/test/integration/targets/template/tasks/main.yml @@ -19,6 +19,8 @@ - set_fact: output_dir: "{{ lookup('env', 'OUTPUT_DIR') }}" +- include_tasks: ansible_managed.yml + - name: render a template which has no content template: src: none.j2 @@ -512,12 +514,6 @@ src: foo.unix.txt dest: '{{ output_dir }}/foo.unix.txt' -- name: Dump templated file (Unix) - command: hexdump -C {{ output_dir }}/foo.unix.templated - -- name: Dump expected file (Unix) - command: hexdump -C {{ output_dir }}/foo.unix.txt - - name: compare templated file to known good (Unix) command: diff -u {{ output_dir }}/foo.unix.templated {{ output_dir }}/foo.unix.txt register: diff_result @@ -559,12 +555,6 @@ src: foo.dos.txt dest: '{{ output_dir }}/foo.dos.txt' -- name: Dump templated file (DOS) - command: hexdump -C {{ output_dir }}/foo.dos.templated - -- name: Dump expected file (DOS) - command: hexdump -C {{ output_dir }}/foo.dos.txt - - name: compare templated file to known good (DOS) command: diff -u {{ output_dir }}/foo.dos.templated {{ output_dir }}/foo.dos.txt register: diff_result @@ -581,12 +571,6 @@ src: foo.unix.txt dest: '{{ output_dir }}/foo.unix.txt' -- name: Dump templated file (Unix) - command: hexdump -C {{ output_dir }}/foo.unix.templated - -- name: Dump expected file (Unix) - command: hexdump -C {{ output_dir }}/foo.unix.txt - - name: compare templated file to known good (Unix) command: diff -u {{ output_dir }}/foo.unix.templated {{ output_dir }}/foo.unix.txt register: diff_result diff --git a/test/integration/targets/template/templates/uses_ansible_managed.j2 b/test/integration/targets/template/templates/uses_ansible_managed.j2 new file mode 100644 index 00000000000..ef7e08e741c --- /dev/null +++ b/test/integration/targets/template/templates/uses_ansible_managed.j2 @@ -0,0 +1 @@ +{{ ansible_managed }} diff --git a/test/sanity/code-smell/black.json b/test/sanity/code-smell/black.json index d7730ba3b4e..e55888926e3 100644 --- a/test/sanity/code-smell/black.json +++ b/test/sanity/code-smell/black.json @@ -3,6 +3,7 @@ ".azure-pipelines/", "lib/ansible/_internal/", "lib/ansible/module_utils/_internal/", + "lib/ansible/template/", "lib/ansible/utils/collection_loader/__init__.py", "packaging/", "test/sanity/code-smell/" diff --git a/test/units/template/test_template.py b/test/units/template/test_template.py index de92edc9018..09f64107219 100644 --- a/test/units/template/test_template.py +++ b/test/units/template/test_template.py @@ -6,9 +6,11 @@ import pathlib import typing as t import pytest +import pytest_mock from contextlib import nullcontext +from ansible import constants from ansible import errors as _errors from ansible import template as _template from ansible._internal._templating import _engine, _jinja_bits @@ -370,3 +372,12 @@ def test_copy_with_new_env_with_none() -> None: copied = templar.copy_with_new_env(variable_start_string=None) assert copied.template(trust_as_template('{{ True }}')) is True + + +def test_generate_ansible_template_vars(mocker: pytest_mock.MockerFixture, tmp_path: pathlib.Path) -> None: + """Verify public API passes through ansible_managed.""" + mocker.patch.object(constants.config, 'get_config_value', return_value='value from config') + + tvars = _template.generate_ansible_template_vars(path="path", fullpath=str(tmp_path), dest_path=str(tmp_path)) + + assert tvars['ansible_managed'] == 'value from config'