From f649dc71e70bd5d7e46a12a06aaab366e5c5bd65 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Wed, 9 Dec 2015 07:21:00 -0800 Subject: [PATCH] avoid persistent containers in attribute defaults moved from the field attribute declaration and created a placeholder which then is resolved in the field attribute class. this is to avoid unwanted persistent of the defaults across objects which introduces stealth bugs when multiple objects of the same kind are used in succession while not overriding the default values. --- lib/ansible/playbook/attribute.py | 11 +++++++++++ lib/ansible/playbook/block.py | 6 +++--- lib/ansible/playbook/conditional.py | 2 +- lib/ansible/playbook/play.py | 16 ++++++++-------- lib/ansible/playbook/play_context.py | 4 ++-- lib/ansible/playbook/playbook_include.py | 2 +- lib/ansible/playbook/role/metadata.py | 2 +- lib/ansible/playbook/taggable.py | 2 +- lib/ansible/playbook/task.py | 2 +- 9 files changed, 29 insertions(+), 18 deletions(-) diff --git a/lib/ansible/playbook/attribute.py b/lib/ansible/playbook/attribute.py index 703d9dbca1e..ce7ed6d8fe7 100644 --- a/lib/ansible/playbook/attribute.py +++ b/lib/ansible/playbook/attribute.py @@ -32,6 +32,17 @@ class Attribute: self.priority = priority self.always_post_validate = always_post_validate + # This is here to avoid `default=` unwanted persistence across object instances + # We cannot rely on None as some fields use it to skip the code + # that would detect an empty container as a user error + if self.default == '_ansible_container': + if self.isa == 'list': + self.default = [] + elif self.isa == 'dict': + self.default = {} + elif self.isa == 'set': + self.default = set() + def __eq__(self, other): return other.priority == self.priority diff --git a/lib/ansible/playbook/block.py b/lib/ansible/playbook/block.py index f2d9c82833a..66009b028af 100644 --- a/lib/ansible/playbook/block.py +++ b/lib/ansible/playbook/block.py @@ -30,9 +30,9 @@ from ansible.playbook.taggable import Taggable class Block(Base, Become, Conditional, Taggable): - _block = FieldAttribute(isa='list', default=[]) - _rescue = FieldAttribute(isa='list', default=[]) - _always = FieldAttribute(isa='list', default=[]) + _block = FieldAttribute(isa='list', default='_ansible_container') + _rescue = FieldAttribute(isa='list', default='_ansible_container') + _always = FieldAttribute(isa='list', default='_ansible_container') _delegate_to = FieldAttribute(isa='list') _delegate_facts = FieldAttribute(isa='bool', default=False) diff --git a/lib/ansible/playbook/conditional.py b/lib/ansible/playbook/conditional.py index fc178e2fa1d..a5b3ca725f8 100644 --- a/lib/ansible/playbook/conditional.py +++ b/lib/ansible/playbook/conditional.py @@ -33,7 +33,7 @@ class Conditional: to be run conditionally when a condition is met or skipped. ''' - _when = FieldAttribute(isa='list', default=[]) + _when = FieldAttribute(isa='list', default='_ansible_container') def __init__(self, loader=None): # when used directly, this class needs a loader, but we want to diff --git a/lib/ansible/playbook/play.py b/lib/ansible/playbook/play.py index ed61416e951..e08c8c60016 100644 --- a/lib/ansible/playbook/play.py +++ b/lib/ansible/playbook/play.py @@ -64,22 +64,22 @@ class Play(Base, Taggable, Become): # Connection _gather_facts = FieldAttribute(isa='bool', default=None, always_post_validate=True) - _hosts = FieldAttribute(isa='list', default=[], required=True, listof=string_types, always_post_validate=True) + _hosts = FieldAttribute(isa='list', default='_ansible_container', required=True, listof=string_types, always_post_validate=True) _name = FieldAttribute(isa='string', default='', always_post_validate=True) # Variable Attributes - _vars_files = FieldAttribute(isa='list', default=[], priority=99) - _vars_prompt = FieldAttribute(isa='list', default=[], always_post_validate=True) + _vars_files = FieldAttribute(isa='list', default='_ansible_container', priority=99) + _vars_prompt = FieldAttribute(isa='list', default='_ansible_container', always_post_validate=True) _vault_password = FieldAttribute(isa='string', always_post_validate=True) # Role Attributes - _roles = FieldAttribute(isa='list', default=[], priority=90) + _roles = FieldAttribute(isa='list', default='_ansible_container', priority=90) # Block (Task) Lists Attributes - _handlers = FieldAttribute(isa='list', default=[]) - _pre_tasks = FieldAttribute(isa='list', default=[]) - _post_tasks = FieldAttribute(isa='list', default=[]) - _tasks = FieldAttribute(isa='list', default=[]) + _handlers = FieldAttribute(isa='list', default='_ansible_container') + _pre_tasks = FieldAttribute(isa='list', default='_ansible_container') + _post_tasks = FieldAttribute(isa='list', default='_ansible_container') + _tasks = FieldAttribute(isa='list', default='_ansible_container') # Flag/Setting Attributes _any_errors_fatal = FieldAttribute(isa='bool', default=False, always_post_validate=True) diff --git a/lib/ansible/playbook/play_context.py b/lib/ansible/playbook/play_context.py index bb3f5aa7a71..6dc2684bd70 100644 --- a/lib/ansible/playbook/play_context.py +++ b/lib/ansible/playbook/play_context.py @@ -171,8 +171,8 @@ class PlayContext(Base): # general flags _verbosity = FieldAttribute(isa='int', default=0) - _only_tags = FieldAttribute(isa='set', default=set()) - _skip_tags = FieldAttribute(isa='set', default=set()) + _only_tags = FieldAttribute(isa='set', default='_ansible_container') + _skip_tags = FieldAttribute(isa='set', default='_ansible_container') _check_mode = FieldAttribute(isa='bool', default=False) _force_handlers = FieldAttribute(isa='bool', default=False) _start_at_task = FieldAttribute(isa='string') diff --git a/lib/ansible/playbook/playbook_include.py b/lib/ansible/playbook/playbook_include.py index d9af2ba5237..52081c41539 100644 --- a/lib/ansible/playbook/playbook_include.py +++ b/lib/ansible/playbook/playbook_include.py @@ -35,7 +35,7 @@ class PlaybookInclude(Base, Conditional, Taggable): _name = FieldAttribute(isa='string') _include = FieldAttribute(isa='string') - _vars = FieldAttribute(isa='dict', default=dict()) + _vars = FieldAttribute(isa='dict', default='_ansible_container') @staticmethod def load(data, basedir, variable_manager=None, loader=None): diff --git a/lib/ansible/playbook/role/metadata.py b/lib/ansible/playbook/role/metadata.py index 58b59145a1c..4bb7d0ce02b 100644 --- a/lib/ansible/playbook/role/metadata.py +++ b/lib/ansible/playbook/role/metadata.py @@ -40,7 +40,7 @@ class RoleMetadata(Base): ''' _allow_duplicates = FieldAttribute(isa='bool', default=False) - _dependencies = FieldAttribute(isa='list', default=[]) + _dependencies = FieldAttribute(isa='list', default='_ansible_container') _galaxy_info = FieldAttribute(isa='GalaxyInfo') def __init__(self, owner=None): diff --git a/lib/ansible/playbook/taggable.py b/lib/ansible/playbook/taggable.py index 8f5cfa09344..37e3261e80d 100644 --- a/lib/ansible/playbook/taggable.py +++ b/lib/ansible/playbook/taggable.py @@ -29,7 +29,7 @@ from ansible.template import Templar class Taggable: untagged = frozenset(['untagged']) - _tags = FieldAttribute(isa='list', default=[], listof=(string_types,int)) + _tags = FieldAttribute(isa='list', default='_ansible_container', listof=(string_types,int)) def __init__(self): super(Taggable, self).__init__() diff --git a/lib/ansible/playbook/task.py b/lib/ansible/playbook/task.py index 17f1952e39c..53a9a3c3931 100644 --- a/lib/ansible/playbook/task.py +++ b/lib/ansible/playbook/task.py @@ -64,7 +64,7 @@ class Task(Base, Conditional, Taggable, Become): # will be used if defined # might be possible to define others - _args = FieldAttribute(isa='dict', default=dict()) + _args = FieldAttribute(isa='dict', default='_ansible_container') _action = FieldAttribute(isa='string') _any_errors_fatal = FieldAttribute(isa='bool')