[conditional] Remove support for bare variables (#74208)

Change:
- Variables used in ``when`` conditionals are no longer parsed and
  attempted to be converted to booleans. All non-empty strings are
  considered true (empty strings, false).

Test Plan:
- Updated existing tests
- Added a bunch of new tests with various kinds of truthy/falsy
  values.

Tickets:
- Fixes #74134

Signed-off-by: Rick Elrod <rick@elrod.me>

changelogs/fragments/conditional-bare-vars.yml

@@ -0,0 +1,2 @@
+breaking_changes:
+  - conditionals - ``when`` conditionals no longer automatically parse string booleans such as ``"true"`` and ``"false"`` into actual booleans. Any non-empty string is now considered true. The ``CONDITIONAL_BARE_VARS`` configuration variable no longer has any effect.

docs/docsite/rst/porting_guides/porting_guide_core_2.12.rst

@@ -31,7 +31,19 @@ No notable changes
 Deprecated
 ==========
 
-No notable changes
+* Bare variables in conditionals: ``when`` conditionals no longer automatically parse string booleans such as ``"true"`` and ``"false"`` into actual booleans. Any variable containing a non-empty string is considered true. This was previously configurable with the ``CONDITIONAL_BARE_VARS`` configuration option (and the ``ANSIBLE_CONDITIONAL_BARE_VARS`` environment variable). This setting no longer has any effect. Users can work around the issue by using the ``|bool`` filter:
+
+.. code-block:: yaml
+
+    vars:
+      teardown: 'false'
+
+    tasks:
+      - include_tasks: teardown.yml
+        when: teardown | bool
+
+      - include_tasks: provision.yml
+        when: not teardown | bool
 
 
 Modules

lib/ansible/config/base.yml

@@ -330,20 +330,6 @@ COLOR_WARN:
   env: [{name: ANSIBLE_COLOR_WARN}]
   ini:
   - {key: warn, section: colors}
-CONDITIONAL_BARE_VARS:
-  name: Allow bare variable evaluation in conditionals
-  default: False
-  type: boolean
-  description:
-    - With this setting on (True), running conditional evaluation 'var' is treated differently than 'var.subkey' as the first is evaluated
-      directly while the second goes through the Jinja2 parser. But 'false' strings in 'var' get evaluated as booleans.
-    - With this setting off they both evaluate the same but in cases in which 'var' was 'false' (a string) it won't get evaluated as a boolean anymore.
-    - Currently this setting defaults to 'True' but will soon change to 'False' and the setting itself will be removed in the future.
-    - Expect that this setting eventually will be deprecated after 2.12
-  env: [{name: ANSIBLE_CONDITIONAL_BARE_VARS}]
-  ini:
-  - {key: conditional_bare_variables, section: defaults}
-  version_added: "2.8"
 COVERAGE_REMOTE_OUTPUT:
   name: Sets the output directory and filename prefix to generate coverage run info.
   description:

lib/ansible/playbook/conditional.py

@@ -127,12 +127,6 @@ class Conditional:
                             '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
 
@@ -140,12 +134,7 @@ 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 %r as a bare variable, this behaviour will go away and you might need to add " | bool"'
-                                   ' (if you would like to evaluate input string from prompt) or " is truthy"'
-                                   ' (if you would like to apply Python\'s evaluation method) to the expression in the future. '
-                                   'Also see CONDITIONAL_BARE_VARS configuration toggle' % original,
-                                   version="2.12", collection_name='ansible.builtin')
             if not isinstance(conditional, text_type) or conditional == "":
                 return conditional
 

test/integration/targets/conditionals/play.yml

@@ -6,10 +6,6 @@
   vars_files:
     - vars/main.yml
   tasks:
-    - name: set conditional bare vars status
-      set_fact:
-        bare: "{{lookup('config', 'CONDITIONAL_BARE_VARS')|bool}}"
-
     - name: test conditional '=='
       shell: echo 'testing'
       when: 1 == 1
@@ -164,6 +160,136 @@
         - "result.stdout == 'testing'"
         - "result.rc == 0"
 
+    - name: not test bare conditional
+      shell: echo 'testing'
+      when: not test_bare
+      register: result
+
+    - name: assert did not run
+      assert:
+        that:
+        - result is skipped
+
+    - name: empty string is false
+      shell: echo 'testing'
+      when: string_lit_empty
+      register: result
+
+    - name: assert did not run
+      assert:
+        that:
+        - result is skipped
+
+    - name: not empty string is true
+      shell: echo 'testing'
+      when: not string_lit_empty
+      register: result
+
+    - name: assert ran
+      assert:
+        that:
+        - result is not skipped
+
+    - name: literal 0 is false
+      shell: echo 'testing'
+      when: int_lit_0
+      register: result
+
+    - name: assert did not run
+      assert:
+        that:
+        - result is skipped
+
+    - name: not literal 0 is true
+      shell: echo 'testing'
+      when: not int_lit_0
+      register: result
+
+    - name: assert ran
+      assert:
+        that:
+        - result is not skipped
+
+    - name: literal 1 is true
+      shell: echo 'testing'
+      when: int_lit_1
+      register: result
+
+    - name: assert ran
+      assert:
+        that:
+        - result is not skipped
+
+    - name: not literal 1 is false
+      shell: echo 'testing'
+      when: not int_lit_1
+      register: result
+
+    - name: assert did not run
+      assert:
+        that:
+        - result is skipped
+
+    - name: null is false
+      shell: echo 'testing'
+      when: lit_null
+      register: result
+
+    - name: assert did not run
+      assert:
+        that:
+        - result is skipped
+
+    - name: literal string "true" is true
+      shell: echo 'testing'
+      when: string_lit_true
+      register: result
+
+    - name: assert ran
+      assert:
+        that:
+        - result is not skipped
+
+    - name: not literal string "true" is false
+      shell: echo 'testing'
+      when: not string_lit_true
+      register: result
+
+    - name: assert did not run
+      assert:
+        that:
+        - result is skipped
+
+    - name: literal string "false" is true (nonempty string)
+      shell: echo 'testing'
+      when: string_lit_false
+      register: result
+
+    - name: assert ran
+      assert:
+        that:
+        - result is not skipped
+
+    - name: not literal string "false" is false
+      shell: echo 'testing'
+      when: not string_lit_false
+      register: result
+
+    - name: assert did not run
+      assert:
+        that:
+        - result is skipped
+
+    - name: not literal string "true" is false
+      shell: echo 'testing'
+      when: not string_lit_true
+      register: result
+
+    - name: assert did not run
+      assert:
+        that:
+        - result is skipped
+
     - name: test conditional using a variable
       shell: echo 'testing'
       when: test_bare_var == 123
@@ -195,23 +321,13 @@
 
     - debug: var={{item}}
       loop:
-        - bare
         - result
         - test_bare_nested_bad
 
-    - name: assert that the bad nested conditional is skipped since 'bare' since 'string' template is resolved to 'false'
+    - name: assert that the bad nested conditional ran (it is a non-empty string, so truthy)
       assert:
         that:
-         - result is skipped
+         - result is not skipped
-
-      when: bare|bool
-
-    - name: assert that the bad nested conditional did run since non bare 'string' is untemplated but 'trueish'
-      assert:
-        that:
-         - result is skipped
-      when: not bare|bool
-         - result is changed
 
     - name: test bad conditional based on nested variables with bool filter
       shell: echo 'testing'
@@ -223,6 +339,7 @@
         that:
         - result is skipped
 
+
     #-----------------------------------------------------------------------
     # proper booleanification tests (issue #8629)
 
@@ -382,7 +499,6 @@
 
     - name: Deal with multivar equality
       tags: ['leveldiff']
-      when: not bare|bool
       vars:
           toplevel_hash:
             hash_var_one: justastring

test/integration/targets/conditionals/runme.sh

@@ -2,14 +2,4 @@
 
 set -eux
 
-ANSIBLE_CONDITIONAL_BARE_VARS=1 ansible-playbook -i ../../inventory play.yml "$@"
+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

test/integration/targets/conditionals/test_no_warnings.yml

@@ -1,18 +0,0 @@
-- 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

test/integration/targets/conditionals/test_warnings.yml

@@ -1,14 +0,0 @@
-- 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

test/integration/targets/conditionals/vars/main.yml

@@ -20,3 +20,10 @@ test_bare: true
 test_bare_var: 123
 test_bare_nested_good: "test_bare_var == 123"
 test_bare_nested_bad: "{{test_bare_var}} == 321"
+
+string_lit_true: "true"
+string_lit_false: "false"
+string_lit_empty: ""
+lit_null: null
+int_lit_0: 0
+int_lit_1: 1

test/sanity/ignore.txt

@@ -144,7 +144,6 @@ lib/ansible/modules/yum_repository.py validate-modules:undocumented-parameter
 lib/ansible/parsing/vault/__init__.py pylint:blacklisted-name
 lib/ansible/playbook/base.py pylint:blacklisted-name
 lib/ansible/playbook/collectionsearch.py required-and-default-attributes  # https://github.com/ansible/ansible/issues/61460
-lib/ansible/playbook/conditional.py pylint:ansible-deprecated-version
 lib/ansible/playbook/helpers.py pylint:ansible-deprecated-version
 lib/ansible/playbook/helpers.py pylint:blacklisted-name
 lib/ansible/playbook/play_context.py pylint:ansible-deprecated-version