diff --git a/changelogs/fragments/57190-improve-warnings-for-when.yaml b/changelogs/fragments/57190-improve-warnings-for-when.yaml new file mode 100644 index 0000000000..0bfee5a168 --- /dev/null +++ b/changelogs/fragments/57190-improve-warnings-for-when.yaml @@ -0,0 +1,3 @@ +bugfixes: + - Only warn for bare variables if they are not type boolean (https://github.com/ansible/ansible/issues/53428) + - Fix regression warning on jinja2 delimiters in when statements (https://github.com/ansible/ansible/issues/56830) diff --git a/lib/ansible/playbook/conditional.py b/lib/ansible/playbook/conditional.py index 08c0ba52e7..296c213d6d 100644 --- a/lib/ansible/playbook/conditional.py +++ b/lib/ansible/playbook/conditional.py @@ -114,16 +114,17 @@ class Conditional: if isinstance(conditional, bool): return conditional - if C.CONDITIONAL_BARE_VARS: - if conditional in all_vars and VALID_VAR_REGEX.match(conditional): - display.deprecated('evaluating %s as a bare variable, this behaviour will go away and you might need to add |bool' - ' to the expression in the future. Also see CONDITIONAL_BARE_VARS configuration toggle.' % conditional, "2.12") - conditional = all_vars[conditional] - if templar.is_template(conditional): display.warning('conditional statements should not include jinja2 ' 'templating delimiters such as {{ }} or {%% %%}. ' 'Found: %s' % conditional) + + bare_vars_warning = False + if C.CONDITIONAL_BARE_VARS: + if conditional in all_vars and VALID_VAR_REGEX.match(conditional): + conditional = all_vars[conditional] + bare_vars_warning = True + # make sure the templar is using the variables specified with this method templar.available_variables = all_vars @@ -131,6 +132,9 @@ class Conditional: # if the conditional is "unsafe", disable lookups disable_lookups = hasattr(conditional, '__UNSAFE__') conditional = templar.template(conditional, disable_lookups=disable_lookups) + if bare_vars_warning and not isinstance(conditional, bool): + display.deprecated('evaluating %s as a bare variable, this behaviour will go away and you might need to add |bool' + ' to the expression in the future. Also see CONDITIONAL_BARE_VARS configuration toggle.' % conditional, "2.12") if not isinstance(conditional, text_type) or conditional == "": return conditional diff --git a/test/integration/targets/conditionals/runme.sh b/test/integration/targets/conditionals/runme.sh index 65a12da804..934443a523 100755 --- a/test/integration/targets/conditionals/runme.sh +++ b/test/integration/targets/conditionals/runme.sh @@ -4,3 +4,12 @@ set -eux ANSIBLE_CONDITIONAL_BARE_VARS=1 ansible-playbook -i ../../inventory play.yml "$@" ANSIBLE_CONDITIONAL_BARE_VARS=0 ansible-playbook -i ../../inventory play.yml "$@" + +export ANSIBLE_CONDITIONAL_BARE_VARS=1 +export ANSIBLE_DEPRECATION_WARNINGS=True + +# No warnings for conditionals that are already type bool +test "$(ansible-playbook -i ../../inventory test_no_warnings.yml "$@" 2>&1 | grep -c '\[DEPRECATION WARNING\]')" = 0 + +# Warn for bare vars of other types since they may be interpreted differently when CONDITIONAL_BARE_VARS defaults to False +test "$(ansible-playbook -i ../../inventory test_warnings.yml "$@" 2>&1 | grep -c '\[DEPRECATION WARNING\]')" = 2 diff --git a/test/integration/targets/conditionals/test_no_warnings.yml b/test/integration/targets/conditionals/test_no_warnings.yml new file mode 100644 index 0000000000..932804476b --- /dev/null +++ b/test/integration/targets/conditionals/test_no_warnings.yml @@ -0,0 +1,18 @@ +- hosts: testhost + gather_facts: false + vars: + boolean_var: false + nested: + bool_var: false + tasks: + - name: Run tasks with previous warnings requesting the bool filter on type boolean vars + block: + - debug: + when: boolean_var + - debug: + when: nested.bool_var + - debug: + when: double_interpolated + vars: + double_interpolated: "{{ other }}" + other: false diff --git a/test/integration/targets/conditionals/test_warnings.yml b/test/integration/targets/conditionals/test_warnings.yml new file mode 100644 index 0000000000..4186cd0112 --- /dev/null +++ b/test/integration/targets/conditionals/test_warnings.yml @@ -0,0 +1,14 @@ +- hosts: testhost + gather_facts: false + vars: + str_boolean_var: 'false' + tasks: + - name: Run tasks with warnings for conditionals that will change in behavior depending on CONDITIONAL_BARE_VARS + block: + - debug: + when: str_boolean_var + - debug: + when: double_interpolated + vars: + double_interpolated: other + other: false