diff --git a/changelogs/fragments/templates_types_datatagging.yml b/changelogs/fragments/templates_types_datatagging.yml index 69c05535456..23f320ac06d 100644 --- a/changelogs/fragments/templates_types_datatagging.yml +++ b/changelogs/fragments/templates_types_datatagging.yml @@ -92,7 +92,6 @@ breaking_changes: - templating - Access to ``_`` prefixed attributes and methods, and methods with known side effects, is no longer permitted. In cases where a matching mapping key is present, the associated value will be returned instead of an error. This increases template environment isolation and ensures more consistent behavior between the ``.`` and ``[]`` operators. - - inventory - Invalid variable names provided by inventories result in an inventory parse failure. This behavior is now consistent with other variable name usages throughout Ansible. - internals - The ``ansible.utils.native_jinja`` Python module has been removed. - internals - The ``AnsibleLoader`` and ``AnsibleDumper`` classes for working with YAML are now factory functions and cannot be extended. - public API - The ``ansible.vars.fact_cache.FactCache`` wrapper has been removed. diff --git a/changelogs/fragments/variable_names.yml b/changelogs/fragments/variable_names.yml new file mode 100644 index 00000000000..6e7c01d0730 --- /dev/null +++ b/changelogs/fragments/variable_names.yml @@ -0,0 +1,7 @@ +bugfixes: + - variables - Added Jinja scalar singletons (``true``, ``false``, ``none``) to invalid Ansible variable name detection. + Previously, variables with these names could be assigned without error, but could not be resolved. +minor_changes: + - variables - Removed restriction on usage of most Python keywords as Ansible variable names. +deprecated_features: + - inventory plugins - Setting invalid Ansible variable names in inventory plugins is deprecated. diff --git a/lib/ansible/_internal/_templating/_jinja_bits.py b/lib/ansible/_internal/_templating/_jinja_bits.py index 5b982b3b7b0..82aedf5b670 100644 --- a/lib/ansible/_internal/_templating/_jinja_bits.py +++ b/lib/ansible/_internal/_templating/_jinja_bits.py @@ -78,6 +78,21 @@ The values following this prefix up to the first newline are parsed as Jinja2 te To include this literal value at the start of a string, a space or other character must precede it. """ +JINJA_KEYWORDS = frozenset( + { + # scalar singletons (see jinja2.nodes.Name.can_assign) + 'true', + 'false', + 'none', + 'True', + 'False', + 'None', + # other + 'not', # unary operator always applicable to names + } +) +"""Names which have special meaning to Jinja and cannot be resolved as variable names.""" + display = Display() diff --git a/lib/ansible/inventory/group.py b/lib/ansible/inventory/group.py index c7b7a7af351..0cf97db4dc6 100644 --- a/lib/ansible/inventory/group.py +++ b/lib/ansible/inventory/group.py @@ -26,7 +26,7 @@ from ansible import constants as C from ansible.errors import AnsibleError from ansible.module_utils.common.text.converters import to_native, to_text from ansible.utils.display import Display -from ansible.utils.vars import combine_vars +from ansible.utils.vars import combine_vars, validate_variable_name from . import helpers # this is left as a module import to facilitate easier unit test patching @@ -221,6 +221,11 @@ class Group: def set_variable(self, key: str, value: t.Any) -> None: key = helpers.remove_trust(key) + try: + validate_variable_name(key) + except AnsibleError as ex: + Display().deprecated(msg=f'Accepting inventory variable with invalid name {key!r}.', version='2.23', help_text=ex._help_text, obj=ex.obj) + if key == 'ansible_group_priority': self.set_priority(int(value)) else: diff --git a/lib/ansible/inventory/host.py b/lib/ansible/inventory/host.py index f41cdd71fed..45992648834 100644 --- a/lib/ansible/inventory/host.py +++ b/lib/ansible/inventory/host.py @@ -22,8 +22,10 @@ import typing as t from collections.abc import Mapping, MutableMapping +from ansible.errors import AnsibleError from ansible.inventory.group import Group, InventoryObjectType from ansible.parsing.utils.addresses import patterns +from ansible.utils.display import Display from ansible.utils.vars import combine_vars, get_unique_id, validate_variable_name from . import helpers # this is left as a module import to facilitate easier unit test patching @@ -117,7 +119,10 @@ class Host: def set_variable(self, key: str, value: t.Any) -> None: key = helpers.remove_trust(key) - validate_variable_name(key) + try: + validate_variable_name(key) + except AnsibleError as ex: + Display().deprecated(msg=f'Accepting inventory variable with invalid name {key!r}.', version='2.23', help_text=ex._help_text, obj=ex.obj) if key in self.vars and isinstance(self.vars[key], MutableMapping) and isinstance(value, Mapping): self.vars = combine_vars(self.vars, {key: value}) diff --git a/lib/ansible/utils/vars.py b/lib/ansible/utils/vars.py index 939831f613c..5fffc807639 100644 --- a/lib/ansible/utils/vars.py +++ b/lib/ansible/utils/vars.py @@ -28,6 +28,7 @@ from json import dumps from ansible import constants as C from ansible import context from ansible._internal import _json +from ansible._internal._templating import _jinja_bits from ansible.errors import AnsibleError, AnsibleOptionsError from ansible.module_utils.datatag import native_type_name from ansible.module_utils.common.text.converters import to_native, to_text @@ -252,6 +253,8 @@ def isidentifier(ident): Originally posted at https://stackoverflow.com/a/29586366 """ + # deprecated: description='Use validate_variable_name instead.' core_version='2.23' + if not isinstance(ident, str): return False @@ -269,7 +272,7 @@ def isidentifier(ident): def validate_variable_name(name: object) -> None: """Validate the given variable name is valid, raising an AnsibleError if it is not.""" - if isinstance(name, str) and isidentifier(name): + if isinstance(name, str) and name.isidentifier() and name.isascii() and name not in _jinja_bits.JINJA_KEYWORDS: return if isinstance(name, (str, int, float, bool, type(None))): diff --git a/test/integration/targets/set_fact/set_fact.yml b/test/integration/targets/set_fact/set_fact.yml index eab42d4eb1f..da41af9bae5 100644 --- a/test/integration/targets/set_fact/set_fact.yml +++ b/test/integration/targets/set_fact/set_fact.yml @@ -17,7 +17,7 @@ - name: Attempt to use a template to set an invalid variable name with set_fact set_fact: - "{{ 'continue' }}": value + "{{ 'true' }}": value register: result ignore_errors: yes @@ -25,7 +25,7 @@ assert: that: - result is failed - - result.msg is contains "Invalid variable name 'continue'" + - result.msg is contains "Invalid variable name 'true'" - name: Attempt to use a template to set an invalid variable name type with set_fact set_fact: diff --git a/test/units/template/test_template.py b/test/units/template/test_template.py index 09f64107219..1a515e14e3a 100644 --- a/test/units/template/test_template.py +++ b/test/units/template/test_template.py @@ -1,6 +1,7 @@ from __future__ import annotations import io +import keyword import pathlib import typing as t @@ -381,3 +382,23 @@ def test_generate_ansible_template_vars(mocker: pytest_mock.MockerFixture, tmp_p tvars = _template.generate_ansible_template_vars(path="path", fullpath=str(tmp_path), dest_path=str(tmp_path)) assert tvars['ansible_managed'] == 'value from config' + + +_ALLOWED_PYTHON_KEYWORDS = sorted(set(keyword.kwlist) - _jinja_bits.JINJA_KEYWORDS) +"""Python keywords which Jinja allows as variable names.""" + + +@pytest.mark.parametrize("keyword_name", _ALLOWED_PYTHON_KEYWORDS) +def test_set_get_keyword(keyword_name: str) -> None: + """Verify Python keywords that are not Jinja keywords can be freely used to set and get variables in templates.""" + template_set_get = TRUST.tag(f"{{% set {keyword_name} = 42 %}}{{{{ {keyword_name} }}}}") + + assert Templar().template(template_set_get) == 42 + + +@pytest.mark.parametrize("keyword_name", _ALLOWED_PYTHON_KEYWORDS) +def test_if_get_keyword(keyword_name: str) -> None: + """Verify Python keywords that are not Jinja keywords can be freely used as variables in templates and template conditionals.""" + template_set_get = TRUST.tag(f"{{% if {keyword_name} == 42 %}}{{{{ {keyword_name} }}}}{{% endif %}}") + + assert Templar(variables={keyword_name: 42}).template(template_set_get) == 42