From 21cd24a0dddb9e2f12c431e65f44df538654213d Mon Sep 17 00:00:00 2001 From: Sloane Hertel Date: Mon, 3 Jun 2019 06:12:33 -0400 Subject: [PATCH] improve conditional warnings (#57190) * Fix order for warning on templated conditionals Fix bare variable warnings when the variable is a boolean * changelog * Add tests for cases that should and should not give warnings If the behavior may change when the default behavior for CONDITIONAL_BARE_VARS becomes False there should be a warning. Boolean type conditionals will not change in behavior so don't warn. * oops, forgot to add files * typo --- .../57190-improve-warnings-for-when.yaml | 3 +++ lib/ansible/playbook/conditional.py | 16 ++++++++++------ test/integration/targets/conditionals/runme.sh | 9 +++++++++ .../targets/conditionals/test_no_warnings.yml | 18 ++++++++++++++++++ .../targets/conditionals/test_warnings.yml | 14 ++++++++++++++ 5 files changed, 54 insertions(+), 6 deletions(-) create mode 100644 changelogs/fragments/57190-improve-warnings-for-when.yaml create mode 100644 test/integration/targets/conditionals/test_no_warnings.yml create mode 100644 test/integration/targets/conditionals/test_warnings.yml 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 00000000000..0bfee5a168d --- /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 08c0ba52e71..296c213d6d5 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 65a12da8049..934443a5238 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 00000000000..932804476be --- /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 00000000000..4186cd01127 --- /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