Do not use mutable defaults in FieldAttribute, instead allow supplying a callable for defaults of mutable types. Fixes #46824 (#46833)

pull/46930/head
Matt Martz 6 years ago committed by GitHub
parent 0dd17b521f
commit a06a5ded61
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

@ -0,0 +1,2 @@
bugfixes:
- FieldAttribute - Do not use mutable defaults, instead allow supplying a callable for defaults of mutable types (https://github.com/ansible/ansible/issues/46824)

@ -87,19 +87,8 @@ class Attribute:
self.extend = extend self.extend = extend
self.prepend = prepend self.prepend = prepend
if default is not None and self.isa in _CONTAINERS: if default is not None and self.isa in _CONTAINERS and not callable(default):
if default: raise TypeError('defaults for FieldAttribute may not be mutable, please provide a callable instead')
self.default = deepcopy(default)
else:
# Don't need to deepcopy default if the container is empty
# Note: switch to try: except once Python3 is more widespread
if hasattr(default, 'copy'):
self.default = default.copy()
else:
# list on python2 does not have .copy()
self.default = copy(default)
else:
self.default = default
def __eq__(self, other): def __eq__(self, other):
return other.priority == self.priority return other.priority == self.priority

@ -162,6 +162,9 @@ class FieldAttributeBase(with_metaclass(BaseMeta, object)):
# need a unique object here (all members contained within are # need a unique object here (all members contained within are
# unique already). # unique already).
self._attributes = self._attributes.copy() self._attributes = self._attributes.copy()
for key, value in self._attributes.items():
if callable(value):
self._attributes[key] = value()
# and init vars, avoid using defaults in field declaration as it lives across plays # and init vars, avoid using defaults in field declaration as it lives across plays
self.vars = dict() self.vars = dict()

@ -32,9 +32,9 @@ from ansible.playbook.taggable import Taggable
class Block(Base, Become, Conditional, Taggable): class Block(Base, Become, Conditional, Taggable):
# main block fields containing the task lists # main block fields containing the task lists
_block = FieldAttribute(isa='list', default=[], inherit=False) _block = FieldAttribute(isa='list', default=list, inherit=False)
_rescue = FieldAttribute(isa='list', default=[], inherit=False) _rescue = FieldAttribute(isa='list', default=list, inherit=False)
_always = FieldAttribute(isa='list', default=[], inherit=False) _always = FieldAttribute(isa='list', default=list, inherit=False)
# other fields # other fields
_delegate_to = FieldAttribute(isa='string') _delegate_to = FieldAttribute(isa='string')

@ -49,7 +49,7 @@ class Conditional:
to be run conditionally when a condition is met or skipped. to be run conditionally when a condition is met or skipped.
''' '''
_when = FieldAttribute(isa='list', default=[], extend=True, prepend=True) _when = FieldAttribute(isa='list', default=list, extend=True, prepend=True)
def __init__(self, loader=None): def __init__(self, loader=None):
# when used directly, this class needs a loader, but we want to # when used directly, this class needs a loader, but we want to

@ -63,22 +63,22 @@ class Play(Base, Taggable, Become):
_gather_timeout = FieldAttribute(isa='int', default=None, always_post_validate=True) _gather_timeout = FieldAttribute(isa='int', default=None, always_post_validate=True)
# Variable Attributes # Variable Attributes
_vars_files = FieldAttribute(isa='list', default=[], priority=99) _vars_files = FieldAttribute(isa='list', default=list, priority=99)
_vars_prompt = FieldAttribute(isa='list', default=[], always_post_validate=False) _vars_prompt = FieldAttribute(isa='list', default=list, always_post_validate=False)
# Role Attributes # Role Attributes
_roles = FieldAttribute(isa='list', default=[], priority=90) _roles = FieldAttribute(isa='list', default=list, priority=90)
# Block (Task) Lists Attributes # Block (Task) Lists Attributes
_handlers = FieldAttribute(isa='list', default=[]) _handlers = FieldAttribute(isa='list', default=list)
_pre_tasks = FieldAttribute(isa='list', default=[]) _pre_tasks = FieldAttribute(isa='list', default=list)
_post_tasks = FieldAttribute(isa='list', default=[]) _post_tasks = FieldAttribute(isa='list', default=list)
_tasks = FieldAttribute(isa='list', default=[]) _tasks = FieldAttribute(isa='list', default=list)
# Flag/Setting Attributes # Flag/Setting Attributes
_force_handlers = FieldAttribute(isa='bool', always_post_validate=True) _force_handlers = FieldAttribute(isa='bool', always_post_validate=True)
_max_fail_percentage = FieldAttribute(isa='percent', always_post_validate=True) _max_fail_percentage = FieldAttribute(isa='percent', always_post_validate=True)
_serial = FieldAttribute(isa='list', default=[], always_post_validate=True) _serial = FieldAttribute(isa='list', default=list, always_post_validate=True)
_strategy = FieldAttribute(isa='string', default=C.DEFAULT_STRATEGY, always_post_validate=True) _strategy = FieldAttribute(isa='string', default=C.DEFAULT_STRATEGY, always_post_validate=True)
_order = FieldAttribute(isa='string', always_post_validate=True) _order = FieldAttribute(isa='string', always_post_validate=True)

@ -178,8 +178,8 @@ class PlayContext(Base):
# general flags # general flags
_verbosity = FieldAttribute(isa='int', default=0) _verbosity = FieldAttribute(isa='int', default=0)
_only_tags = FieldAttribute(isa='set', default=set()) _only_tags = FieldAttribute(isa='set', default=set)
_skip_tags = FieldAttribute(isa='set', default=set()) _skip_tags = FieldAttribute(isa='set', default=set)
_force_handlers = FieldAttribute(isa='bool', default=False) _force_handlers = FieldAttribute(isa='bool', default=False)
_start_at_task = FieldAttribute(isa='string') _start_at_task = FieldAttribute(isa='string')
_step = FieldAttribute(isa='bool', default=False) _step = FieldAttribute(isa='bool', default=False)

@ -35,7 +35,7 @@ from ansible.template import Templar
class PlaybookInclude(Base, Conditional, Taggable): class PlaybookInclude(Base, Conditional, Taggable):
_import_playbook = FieldAttribute(isa='string') _import_playbook = FieldAttribute(isa='string')
_vars = FieldAttribute(isa='dict', default=dict()) _vars = FieldAttribute(isa='dict', default=dict)
@staticmethod @staticmethod
def load(data, basedir, variable_manager=None, loader=None): def load(data, basedir, variable_manager=None, loader=None):

@ -39,7 +39,7 @@ class RoleMetadata(Base):
''' '''
_allow_duplicates = FieldAttribute(isa='bool', default=False) _allow_duplicates = FieldAttribute(isa='bool', default=False)
_dependencies = FieldAttribute(isa='list', default=[]) _dependencies = FieldAttribute(isa='list', default=list)
_galaxy_info = FieldAttribute(isa='GalaxyInfo') _galaxy_info = FieldAttribute(isa='GalaxyInfo')
def __init__(self, owner=None): def __init__(self, owner=None):

@ -30,7 +30,7 @@ from ansible.template import Templar
class Taggable: class Taggable:
untagged = frozenset(['untagged']) untagged = frozenset(['untagged'])
_tags = FieldAttribute(isa='list', default=[], listof=(string_types, int), extend=True) _tags = FieldAttribute(isa='list', default=list, listof=(string_types, int), extend=True)
def __init__(self): def __init__(self):
super(Taggable, self).__init__() super(Taggable, self).__init__()

@ -70,22 +70,22 @@ class Task(Base, Conditional, Taggable, Become):
# inheritance is only triggered if the 'current value' is None, # inheritance is only triggered if the 'current value' is None,
# default can be set at play/top level object and inheritance will take it's course. # default can be set at play/top level object and inheritance will take it's course.
_args = FieldAttribute(isa='dict', default=dict()) _args = FieldAttribute(isa='dict', default=dict)
_action = FieldAttribute(isa='string') _action = FieldAttribute(isa='string')
_async_val = FieldAttribute(isa='int', default=0, alias='async') _async_val = FieldAttribute(isa='int', default=0, alias='async')
_changed_when = FieldAttribute(isa='list', default=[]) _changed_when = FieldAttribute(isa='list', default=list)
_delay = FieldAttribute(isa='int', default=5) _delay = FieldAttribute(isa='int', default=5)
_delegate_to = FieldAttribute(isa='string') _delegate_to = FieldAttribute(isa='string')
_delegate_facts = FieldAttribute(isa='bool') _delegate_facts = FieldAttribute(isa='bool')
_failed_when = FieldAttribute(isa='list', default=[]) _failed_when = FieldAttribute(isa='list', default=list)
_loop = FieldAttribute() _loop = FieldAttribute()
_loop_control = FieldAttribute(isa='class', class_type=LoopControl, inherit=False) _loop_control = FieldAttribute(isa='class', class_type=LoopControl, inherit=False)
_notify = FieldAttribute(isa='list') _notify = FieldAttribute(isa='list')
_poll = FieldAttribute(isa='int', default=10) _poll = FieldAttribute(isa='int', default=10)
_register = FieldAttribute(isa='string') _register = FieldAttribute(isa='string')
_retries = FieldAttribute(isa='int', default=3) _retries = FieldAttribute(isa='int', default=3)
_until = FieldAttribute(isa='list', default=[]) _until = FieldAttribute(isa='list', default=list)
# deprecated, used to be loop and loop_args but loop has been repurposed # deprecated, used to be loop and loop_args but loop has been repurposed
_loop_with = FieldAttribute(isa='string', private=True, inherit=False) _loop_with = FieldAttribute(isa='string', private=True, inherit=False)

@ -0,0 +1,25 @@
---
- hosts: testhost
gather_facts: false
tasks:
- name: Static imports should expose vars at parse time, not at execution time
assert:
that:
- static_defaults_var == 'static_defaults'
- static_vars_var == 'static_vars'
- import_role:
name: static
- assert:
that:
- static_tasks_var == 'static_tasks'
- static_defaults_var == 'static_defaults'
- static_vars_var == 'static_vars'
- hosts: testhost
gather_facts: false
tasks:
- name: Ensure vars from import_roles do not bleed between plays
assert:
that:
- static_defaults_var is undefined
- static_vars_var is undefined

@ -81,3 +81,4 @@ ANSIBLE_STRATEGY='free' ansible-playbook tasks/test_include_dupe_loop.yml -i ../
test "$(grep -c '"item=foo"' test_include_dupe_loop.out)" = 3 test "$(grep -c '"item=foo"' test_include_dupe_loop.out)" = 3
ansible-playbook public_exposure/playbook.yml -i ../../inventory "$@" ansible-playbook public_exposure/playbook.yml -i ../../inventory "$@"
ansible-playbook public_exposure/no_bleeding.yml -i ../../inventory "$@"

@ -347,13 +347,13 @@ class BaseSubClass(base.Base):
_test_attr_list = FieldAttribute(isa='list', listof=string_types, always_post_validate=True) _test_attr_list = FieldAttribute(isa='list', listof=string_types, always_post_validate=True)
_test_attr_list_no_listof = FieldAttribute(isa='list', always_post_validate=True) _test_attr_list_no_listof = FieldAttribute(isa='list', always_post_validate=True)
_test_attr_list_required = FieldAttribute(isa='list', listof=string_types, required=True, _test_attr_list_required = FieldAttribute(isa='list', listof=string_types, required=True,
default=[], always_post_validate=True) default=list, always_post_validate=True)
_test_attr_string = FieldAttribute(isa='string', default='the_test_attr_string_default_value') _test_attr_string = FieldAttribute(isa='string', default='the_test_attr_string_default_value')
_test_attr_string_required = FieldAttribute(isa='string', required=True, _test_attr_string_required = FieldAttribute(isa='string', required=True,
default='the_test_attr_string_default_value') default='the_test_attr_string_default_value')
_test_attr_percent = FieldAttribute(isa='percent', always_post_validate=True) _test_attr_percent = FieldAttribute(isa='percent', always_post_validate=True)
_test_attr_set = FieldAttribute(isa='set', default=set(), always_post_validate=True) _test_attr_set = FieldAttribute(isa='set', default=set, always_post_validate=True)
_test_attr_dict = FieldAttribute(isa='dict', default={'a_key': 'a_value'}, always_post_validate=True) _test_attr_dict = FieldAttribute(isa='dict', default=lambda: {'a_key': 'a_value'}, always_post_validate=True)
_test_attr_class = FieldAttribute(isa='class', class_type=ExampleSubClass) _test_attr_class = FieldAttribute(isa='class', class_type=ExampleSubClass)
_test_attr_class_post_validate = FieldAttribute(isa='class', class_type=ExampleSubClass, _test_attr_class_post_validate = FieldAttribute(isa='class', class_type=ExampleSubClass,
always_post_validate=True) always_post_validate=True)

Loading…
Cancel
Save