From bd1acb74c8c3e3587d807b723449ce55d8b99812 Mon Sep 17 00:00:00 2001 From: Rick Elrod Date: Tue, 13 Apr 2021 10:22:02 -0500 Subject: [PATCH] [playbook/play.py] Increase error test coverage (#74217) Change: - Mostly increase error coverage for various conditions in play.py - Also fix a string in an error, where get_name() was called before self.name was read in, so get_name() was always ''. Test Plan: - new tests Signed-off-by: Rick Elrod * Fix regex for py2 and py3 Signed-off-by: Rick Elrod * py2 hates me Signed-off-by: Rick Elrod --- lib/ansible/playbook/play.py | 4 +- .../targets/playbook/empty_hosts.yml | 4 + .../targets/playbook/malformed_post_tasks.yml | 2 + .../targets/playbook/malformed_pre_tasks.yml | 2 + .../targets/playbook/malformed_roles.yml | 2 + .../targets/playbook/malformed_tasks.yml | 2 + .../playbook/malformed_vars_prompt.yml | 3 + .../targets/playbook/old_style_role.yml | 3 + .../targets/playbook/remote_user_and_user.yml | 6 ++ .../targets/playbook/roles_null.yml | 3 + test/integration/targets/playbook/runme.sh | 77 +++++++++++++++++++ .../targets/playbook/some_vars.yml | 2 + test/integration/targets/playbook/user.yml | 23 ++++++ .../targets/playbook/vars_files_null.yml | 3 + .../targets/playbook/vars_files_string.yml | 6 ++ .../targets/playbook/vars_prompt_null.yml | 3 + test/units/playbook/test_play.py | 10 ++- 17 files changed, 152 insertions(+), 3 deletions(-) create mode 100644 test/integration/targets/playbook/empty_hosts.yml create mode 100644 test/integration/targets/playbook/malformed_post_tasks.yml create mode 100644 test/integration/targets/playbook/malformed_pre_tasks.yml create mode 100644 test/integration/targets/playbook/malformed_roles.yml create mode 100644 test/integration/targets/playbook/malformed_tasks.yml create mode 100644 test/integration/targets/playbook/malformed_vars_prompt.yml create mode 100644 test/integration/targets/playbook/old_style_role.yml create mode 100644 test/integration/targets/playbook/remote_user_and_user.yml create mode 100644 test/integration/targets/playbook/roles_null.yml create mode 100644 test/integration/targets/playbook/some_vars.yml create mode 100644 test/integration/targets/playbook/user.yml create mode 100644 test/integration/targets/playbook/vars_files_null.yml create mode 100644 test/integration/targets/playbook/vars_files_string.yml create mode 100644 test/integration/targets/playbook/vars_prompt_null.yml diff --git a/lib/ansible/playbook/play.py b/lib/ansible/playbook/play.py index 13206875d42..03f11a803b4 100644 --- a/lib/ansible/playbook/play.py +++ b/lib/ansible/playbook/play.py @@ -130,8 +130,8 @@ class Play(Base, Taggable, CollectionSearch): # this should never happen, but error out with a helpful message # to the user if it does... if 'remote_user' in ds: - raise AnsibleParserError("both 'user' and 'remote_user' are set for %s. " - "The use of 'user' is deprecated, and should be removed" % self.get_name(), obj=ds) + raise AnsibleParserError("both 'user' and 'remote_user' are set for this play. " + "The use of 'user' is deprecated, and should be removed", obj=ds) ds['remote_user'] = ds['user'] del ds['user'] diff --git a/test/integration/targets/playbook/empty_hosts.yml b/test/integration/targets/playbook/empty_hosts.yml new file mode 100644 index 00000000000..c9493c0933d --- /dev/null +++ b/test/integration/targets/playbook/empty_hosts.yml @@ -0,0 +1,4 @@ +- hosts: [] + tasks: + - debug: + msg: does not run diff --git a/test/integration/targets/playbook/malformed_post_tasks.yml b/test/integration/targets/playbook/malformed_post_tasks.yml new file mode 100644 index 00000000000..4c13411521b --- /dev/null +++ b/test/integration/targets/playbook/malformed_post_tasks.yml @@ -0,0 +1,2 @@ +- hosts: localhost + post_tasks: 123 diff --git a/test/integration/targets/playbook/malformed_pre_tasks.yml b/test/integration/targets/playbook/malformed_pre_tasks.yml new file mode 100644 index 00000000000..6c58477fb8f --- /dev/null +++ b/test/integration/targets/playbook/malformed_pre_tasks.yml @@ -0,0 +1,2 @@ +- hosts: localhost + pre_tasks: 123 diff --git a/test/integration/targets/playbook/malformed_roles.yml b/test/integration/targets/playbook/malformed_roles.yml new file mode 100644 index 00000000000..35db56e7ae1 --- /dev/null +++ b/test/integration/targets/playbook/malformed_roles.yml @@ -0,0 +1,2 @@ +- hosts: localhost + roles: 123 diff --git a/test/integration/targets/playbook/malformed_tasks.yml b/test/integration/targets/playbook/malformed_tasks.yml new file mode 100644 index 00000000000..123c059fff2 --- /dev/null +++ b/test/integration/targets/playbook/malformed_tasks.yml @@ -0,0 +1,2 @@ +- hosts: localhost + tasks: 123 diff --git a/test/integration/targets/playbook/malformed_vars_prompt.yml b/test/integration/targets/playbook/malformed_vars_prompt.yml new file mode 100644 index 00000000000..5447197d5cc --- /dev/null +++ b/test/integration/targets/playbook/malformed_vars_prompt.yml @@ -0,0 +1,3 @@ +- hosts: localhost + vars_prompt: + - foo: bar diff --git a/test/integration/targets/playbook/old_style_role.yml b/test/integration/targets/playbook/old_style_role.yml new file mode 100644 index 00000000000..015f263a4b9 --- /dev/null +++ b/test/integration/targets/playbook/old_style_role.yml @@ -0,0 +1,3 @@ +- hosts: localhost + roles: + - foo,bar diff --git a/test/integration/targets/playbook/remote_user_and_user.yml b/test/integration/targets/playbook/remote_user_and_user.yml new file mode 100644 index 00000000000..c9e2389dfe5 --- /dev/null +++ b/test/integration/targets/playbook/remote_user_and_user.yml @@ -0,0 +1,6 @@ +- hosts: localhost + remote_user: a + user: b + tasks: + - debug: + msg: did not run diff --git a/test/integration/targets/playbook/roles_null.yml b/test/integration/targets/playbook/roles_null.yml new file mode 100644 index 00000000000..d06bcd15dc5 --- /dev/null +++ b/test/integration/targets/playbook/roles_null.yml @@ -0,0 +1,3 @@ +- name: null roles is okay + hosts: localhost + roles: null diff --git a/test/integration/targets/playbook/runme.sh b/test/integration/targets/playbook/runme.sh index ec646dd312d..8af4e8b4beb 100755 --- a/test/integration/targets/playbook/runme.sh +++ b/test/integration/targets/playbook/runme.sh @@ -8,8 +8,85 @@ ansible-playbook -i ../../inventory types.yml -v "$@" # test timeout ansible-playbook -i ../../inventory timeout.yml -v "$@" +# our Play class allows for 'user' or 'remote_user', but not both. +# first test that both user and remote_user work individually +set +e +result="$(ansible-playbook -i ../../inventory user.yml -v "$@" 2>&1)" +set -e +grep -q "worked with user" <<< "$result" +grep -q "worked with remote_user" <<< "$result" + +# then test that the play errors if user and remote_user both exist +echo "EXPECTED ERROR: Ensure we fail properly if a play has both user and remote_user." +set +e +result="$(ansible-playbook -i ../../inventory remote_user_and_user.yml -v "$@" 2>&1)" +set -e +grep -q "ERROR! both 'user' and 'remote_user' are set for this play." <<< "$result" + +# test that playbook errors if len(plays) == 0 echo "EXPECTED ERROR: Ensure we fail properly if a playbook is an empty list." set +e result="$(ansible-playbook -i ../../inventory empty.yml -v "$@" 2>&1)" set -e grep -q "ERROR! A playbook must contain at least one play" <<< "$result" + +# test that play errors if len(hosts) == 0 +echo "EXPECTED ERROR: Ensure we fail properly if a play has 0 hosts." +set +e +result="$(ansible-playbook -i ../../inventory empty_hosts.yml -v "$@" 2>&1)" +set -e +grep -q "ERROR! Hosts list cannot be empty - please check your playbook" <<< "$result" + +# test that play errors if tasks is malformed +echo "EXPECTED ERROR: Ensure we fail properly if tasks is malformed." +set +e +result="$(ansible-playbook -i ../../inventory malformed_tasks.yml -v "$@" 2>&1)" +set -e +grep -q "ERROR! A malformed block was encountered while loading tasks: 123 should be a list or None" <<< "$result" + +# test that play errors if pre_tasks is malformed +echo "EXPECTED ERROR: Ensure we fail properly if pre_tasks is malformed." +set +e +result="$(ansible-playbook -i ../../inventory malformed_pre_tasks.yml -v "$@" 2>&1)" +set -e +grep -q "ERROR! A malformed block was encountered while loading pre_tasks" <<< "$result" + +# test that play errors if post_tasks is malformed +echo "EXPECTED ERROR: Ensure we fail properly if post_tasks is malformed." +set +e +result="$(ansible-playbook -i ../../inventory malformed_post_tasks.yml -v "$@" 2>&1)" +set -e +grep -q "ERROR! A malformed block was encountered while loading post_tasks" <<< "$result" + +# test roles: null -- it gets converted to [] internally +ansible-playbook -i ../../inventory roles_null.yml -v "$@" + +# test roles: 123 -- errors +echo "EXPECTED ERROR: Ensure we fail properly if roles is malformed." +set +e +result="$(ansible-playbook -i ../../inventory malformed_roles.yml -v "$@" 2>&1)" +set -e +grep -q "ERROR! A malformed role declaration was encountered." <<< "$result" + +# test roles: ["foo,bar"] -- errors about old style +echo "EXPECTED ERROR: Ensure we fail properly if old style role is given." +set +e +result="$(ansible-playbook -i ../../inventory old_style_role.yml -v "$@" 2>&1)" +set -e +grep -q "ERROR! Invalid old style role requirement: foo,bar" <<< "$result" + +# test vars prompt that has no name +echo "EXPECTED ERROR: Ensure we fail properly if vars_prompt has no name." +set +e +result="$(ansible-playbook -i ../../inventory malformed_vars_prompt.yml -v "$@" 2>&1)" +set -e +grep -q "ERROR! Invalid vars_prompt data structure, missing 'name' key" <<< "$result" + +# test vars_prompt: null +ansible-playbook -i ../../inventory vars_prompt_null.yml -v "$@" + +# test vars_files: null +ansible-playbook -i ../../inventory vars_files_null.yml -v "$@" + +# test vars_files: filename.yml +ansible-playbook -i ../../inventory vars_files_string.yml -v "$@" diff --git a/test/integration/targets/playbook/some_vars.yml b/test/integration/targets/playbook/some_vars.yml new file mode 100644 index 00000000000..78353654714 --- /dev/null +++ b/test/integration/targets/playbook/some_vars.yml @@ -0,0 +1,2 @@ +a_variable: yep +another: hi diff --git a/test/integration/targets/playbook/user.yml b/test/integration/targets/playbook/user.yml new file mode 100644 index 00000000000..8b4029b82d3 --- /dev/null +++ b/test/integration/targets/playbook/user.yml @@ -0,0 +1,23 @@ +- hosts: localhost + tasks: + - command: whoami + register: whoami + + - assert: + that: + - whoami is successful + + - set_fact: + me: "{{ whoami.stdout }}" + +- hosts: localhost + user: "{{ me }}" + tasks: + - debug: + msg: worked with user ({{ me }}) + +- hosts: localhost + remote_user: "{{ me }}" + tasks: + - debug: + msg: worked with remote_user ({{ me }}) diff --git a/test/integration/targets/playbook/vars_files_null.yml b/test/integration/targets/playbook/vars_files_null.yml new file mode 100644 index 00000000000..64c21c66b4e --- /dev/null +++ b/test/integration/targets/playbook/vars_files_null.yml @@ -0,0 +1,3 @@ +- name: null vars_files is okay + hosts: localhost + vars_files: null diff --git a/test/integration/targets/playbook/vars_files_string.yml b/test/integration/targets/playbook/vars_files_string.yml new file mode 100644 index 00000000000..9191d3c1165 --- /dev/null +++ b/test/integration/targets/playbook/vars_files_string.yml @@ -0,0 +1,6 @@ +- hosts: localhost + vars_files: some_vars.yml + tasks: + - assert: + that: + - 'a_variable == "yep"' diff --git a/test/integration/targets/playbook/vars_prompt_null.yml b/test/integration/targets/playbook/vars_prompt_null.yml new file mode 100644 index 00000000000..4fdfa7c1006 --- /dev/null +++ b/test/integration/targets/playbook/vars_prompt_null.yml @@ -0,0 +1,3 @@ +- name: null vars prompt is okay + hosts: localhost + vars_prompt: null diff --git a/test/units/playbook/test_play.py b/test/units/playbook/test_play.py index 725c28eac47..5ce861ea091 100644 --- a/test/units/playbook/test_play.py +++ b/test/units/playbook/test_play.py @@ -22,7 +22,7 @@ __metaclass__ = type from units.compat import unittest from units.compat.mock import patch, MagicMock -from ansible.errors import AnsibleParserError +from ansible.errors import AnsibleAssertionError, AnsibleParserError from ansible.playbook.play import Play from units.mock.loader import DictDataLoader @@ -64,6 +64,14 @@ class TestPlay(unittest.TestCase): ) self.assertRaises(AnsibleParserError, Play.load, play_data) + def test_play_with_bad_ds_type(self): + play_data = [] + self.assertRaisesRegexp( + AnsibleAssertionError, + r"while preprocessing data \(\[\]\), ds should be a dict but was a <(?:class|type) 'list'>", + Play.load, + play_data) + def test_play_with_tasks(self): p = Play.load(dict( name="test play",