Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[conditional] Remove support for bare variables #74208

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions changelogs/fragments/conditional-bare-vars.yml
Original file line number Diff line number Diff line change
@@ -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.
14 changes: 13 additions & 1 deletion docs/docsite/rst/porting_guides/porting_guide_core_2.12.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 0 additions & 14 deletions lib/ansible/config/base.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
13 changes: 1 addition & 12 deletions lib/ansible/playbook/conditional.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,25 +127,14 @@ def _check_conditional(self, conditional, templar, all_vars):
'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

try:
# 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

Expand Down
150 changes: 133 additions & 17 deletions test/integration/targets/conditionals/play.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

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
- result is not skipped

- name: test bad conditional based on nested variables with bool filter
shell: echo 'testing'
Expand All @@ -223,6 +339,7 @@
that:
- result is skipped


#-----------------------------------------------------------------------
# proper booleanification tests (issue #8629)

Expand Down Expand Up @@ -382,7 +499,6 @@

- name: Deal with multivar equality
tags: ['leveldiff']
when: not bare|bool
vars:
toplevel_hash:
hash_var_one: justastring
Expand Down
12 changes: 1 addition & 11 deletions test/integration/targets/conditionals/runme.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,4 @@

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
ansible-playbook -i ../../inventory play.yml "$@"
18 changes: 0 additions & 18 deletions test/integration/targets/conditionals/test_no_warnings.yml

This file was deleted.

14 changes: 0 additions & 14 deletions test/integration/targets/conditionals/test_warnings.yml

This file was deleted.

7 changes: 7 additions & 0 deletions test/integration/targets/conditionals/vars/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 0 additions & 1 deletion test/sanity/ignore.txt
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,6 @@ lib/ansible/parsing/vault/__init__.py pylint:blacklisted-name
lib/ansible/playbook/__init__.py pylint:ansible-deprecated-version
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
Expand Down