From c9a004227eba544cd1a8e9c3c7ce1e5463a5b418 Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Wed, 23 Sep 2015 08:56:36 -0400 Subject: [PATCH] Improve error catching from malformed playbook data Fixes #12478 --- lib/ansible/playbook/base.py | 6 +- lib/ansible/playbook/block.py | 83 +++++++++++++-------------- lib/ansible/playbook/helpers.py | 12 ++-- lib/ansible/playbook/play.py | 25 ++++++-- lib/ansible/playbook/role/metadata.py | 5 +- 5 files changed, 71 insertions(+), 60 deletions(-) diff --git a/lib/ansible/playbook/base.py b/lib/ansible/playbook/base.py index c4fa631b610..bee42e2f3d5 100644 --- a/lib/ansible/playbook/base.py +++ b/lib/ansible/playbook/base.py @@ -159,6 +159,9 @@ class Base: assert ds is not None + # cache the datastructure internally + setattr(self, '_ds', ds) + # the variable manager class is used to manage and merge variables # down to a single dictionary for reference in templating, etc. self._variable_manager = variable_manager @@ -192,9 +195,6 @@ class Base: # run early, non-critical validation self.validate() - # cache the datastructure internally - setattr(self, '_ds', ds) - # return the constructed object return self diff --git a/lib/ansible/playbook/block.py b/lib/ansible/playbook/block.py index c0a9ca9bcde..d6337d32d26 100644 --- a/lib/ansible/playbook/block.py +++ b/lib/ansible/playbook/block.py @@ -19,6 +19,7 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type +from ansible.errors import AnsibleParserError from ansible.playbook.attribute import Attribute, FieldAttribute from ansible.playbook.base import Base from ansible.playbook.become import Become @@ -96,53 +97,49 @@ class Block(Base, Become, Conditional, Taggable): return super(Block, self).preprocess_data(ds) def _load_block(self, attr, ds): - return load_list_of_tasks( - ds, - play=self._play, - block=self, - role=self._role, - task_include=self._task_include, - variable_manager=self._variable_manager, - loader=self._loader, - use_handlers=self._use_handlers, - ) + try: + return load_list_of_tasks( + ds, + play=self._play, + block=self, + role=self._role, + task_include=self._task_include, + variable_manager=self._variable_manager, + loader=self._loader, + use_handlers=self._use_handlers, + ) + except AssertionError: + raise AnsibleParserError("A malformed block was encountered.", obj=self._ds) def _load_rescue(self, attr, ds): - return load_list_of_tasks( - ds, - play=self._play, - block=self, - role=self._role, - task_include=self._task_include, - variable_manager=self._variable_manager, - loader=self._loader, - use_handlers=self._use_handlers, - ) + try: + return load_list_of_tasks( + ds, + play=self._play, + block=self, + role=self._role, + task_include=self._task_include, + variable_manager=self._variable_manager, + loader=self._loader, + use_handlers=self._use_handlers, + ) + except AssertionError: + raise AnsibleParserError("A malformed block was encountered.", obj=self._ds) def _load_always(self, attr, ds): - return load_list_of_tasks( - ds, - play=self._play, - block=self, - role=self._role, - task_include=self._task_include, - variable_manager=self._variable_manager, - loader=self._loader, - use_handlers=self._use_handlers, - ) - - # not currently used - #def _load_otherwise(self, attr, ds): - # return load_list_of_tasks( - # ds, - # play=self._play, - # block=self, - # role=self._role, - # task_include=self._task_include, - # variable_manager=self._variable_manager, - # loader=self._loader, - # use_handlers=self._use_handlers, - # ) + try: + return load_list_of_tasks( + ds, + play=self._play, + block=self, + role=self._role, + task_include=self._task_include, + variable_manager=self._variable_manager, + loader=self._loader, + use_handlers=self._use_handlers, + ) + except AssertionError: + raise AnsibleParserError("A malformed block was encountered.", obj=self._ds) def copy(self, exclude_parent=False, exclude_tasks=False): def _dupe_task_list(task_list, new_block): diff --git a/lib/ansible/playbook/helpers.py b/lib/ansible/playbook/helpers.py index 958e72206c6..c4f11c1c8ed 100644 --- a/lib/ansible/playbook/helpers.py +++ b/lib/ansible/playbook/helpers.py @@ -34,8 +34,7 @@ def load_list_of_blocks(ds, play, parent_block=None, role=None, task_include=Non # we import here to prevent a circular dependency with imports from ansible.playbook.block import Block - if not isinstance(ds, (list, type(None))): - raise AnsibleParserError('block has bad type: "%s". Expecting "list"' % type(ds).__name__, obj=ds) + assert isinstance(ds, (list, type(None))) block_list = [] if ds: @@ -74,13 +73,11 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h from ansible.playbook.handler import Handler from ansible.playbook.task import Task - if not isinstance(ds, list): - raise AnsibleParserError('task has bad type: "%s". Expected "list"' % type(ds).__name__, obj=ds) + assert isinstance(ds, list) task_list = [] for task in ds: - if not isinstance(task, dict): - raise AnsibleParserError('task/handler has bad type: "%s". Expected "dict"' % type(task).__name__, obj=task) + assert isinstance(task, dict) if 'block' in task: t = Block.load( @@ -113,8 +110,7 @@ def load_list_of_roles(ds, play, current_role_path=None, variable_manager=None, # we import here to prevent a circular dependency with imports from ansible.playbook.role.include import RoleInclude - if not isinstance(ds, list): - raise AnsibleParserError('roles has bad type: "%s". Expectes "list"' % type(ds).__name__, obj=ds) + assert isinstance(ds, list) roles = [] for role_def in ds: diff --git a/lib/ansible/playbook/play.py b/lib/ansible/playbook/play.py index 53246776519..55f994f305e 100644 --- a/lib/ansible/playbook/play.py +++ b/lib/ansible/playbook/play.py @@ -157,28 +157,40 @@ class Play(Base, Taggable, Become): Loads a list of blocks from a list which may be mixed tasks/blocks. Bare tasks outside of a block are given an implicit block. ''' - return load_list_of_blocks(ds=ds, play=self, variable_manager=self._variable_manager, loader=self._loader) + try: + return load_list_of_blocks(ds=ds, play=self, variable_manager=self._variable_manager, loader=self._loader) + except AssertionError: + raise AnsibleParserError("A malformed block was encountered.", obj=self._ds) def _load_pre_tasks(self, attr, ds): ''' Loads a list of blocks from a list which may be mixed tasks/blocks. Bare tasks outside of a block are given an implicit block. ''' - return load_list_of_blocks(ds=ds, play=self, variable_manager=self._variable_manager, loader=self._loader) + try: + return load_list_of_blocks(ds=ds, play=self, variable_manager=self._variable_manager, loader=self._loader) + except AssertionError: + raise AnsibleParserError("A malformed block was encountered.", obj=self._ds) def _load_post_tasks(self, attr, ds): ''' Loads a list of blocks from a list which may be mixed tasks/blocks. Bare tasks outside of a block are given an implicit block. ''' - return load_list_of_blocks(ds=ds, play=self, variable_manager=self._variable_manager, loader=self._loader) + try: + return load_list_of_blocks(ds=ds, play=self, variable_manager=self._variable_manager, loader=self._loader) + except AssertionError: + raise AnsibleParserError("A malformed block was encountered.", obj=self._ds) def _load_handlers(self, attr, ds): ''' Loads a list of blocks from a list which may be mixed handlers/blocks. Bare handlers outside of a block are given an implicit block. ''' - return load_list_of_blocks(ds=ds, play=self, use_handlers=True, variable_manager=self._variable_manager, loader=self._loader) + try: + return load_list_of_blocks(ds=ds, play=self, use_handlers=True, variable_manager=self._variable_manager, loader=self._loader) + except AssertionError: + raise AnsibleParserError("A malformed block was encountered.", obj=self._ds) def _load_roles(self, attr, ds): ''' @@ -189,7 +201,10 @@ class Play(Base, Taggable, Become): if ds is None: ds = [] - role_includes = load_list_of_roles(ds, play=self, variable_manager=self._variable_manager, loader=self._loader) + try: + role_includes = load_list_of_roles(ds, play=self, variable_manager=self._variable_manager, loader=self._loader) + except AssertionError: + raise AnsibleParserError("A malformed role declaration was encountered.", obj=self._ds) roles = [] for ri in role_includes: diff --git a/lib/ansible/playbook/role/metadata.py b/lib/ansible/playbook/role/metadata.py index cd56e606331..7d84acf1885 100644 --- a/lib/ansible/playbook/role/metadata.py +++ b/lib/ansible/playbook/role/metadata.py @@ -72,7 +72,10 @@ class RoleMetadata(Base): if self._owner: current_role_path = os.path.dirname(self._owner._role_path) - return load_list_of_roles(ds, play=self._owner._play, current_role_path=current_role_path, variable_manager=self._variable_manager, loader=self._loader) + try: + return load_list_of_roles(ds, play=self._owner._play, current_role_path=current_role_path, variable_manager=self._variable_manager, loader=self._loader) + except AssertionError: + raise AnsibleParserError("A malformed list of role dependencies was encountered.", obj=self._ds) def _load_galaxy_info(self, attr, ds): '''