diff --git a/lib/ansible/playbook/role/__init__.py b/lib/ansible/playbook/role/__init__.py index 52cc12e4eaf..4bff70d3641 100644 --- a/lib/ansible/playbook/role/__init__.py +++ b/lib/ansible/playbook/role/__init__.py @@ -19,10 +19,10 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type -from ansible.compat.six import iteritems - +import collections import os +from ansible.compat.six import iteritems, binary_type, text_type from ansible.errors import AnsibleError, AnsibleParserError from ansible.playbook.attribute import FieldAttribute from ansible.playbook.base import Base @@ -41,25 +41,54 @@ __all__ = ['Role', 'hash_params'] # the role due to the fact that it would require the use of self # in a static method. This is also used in the base class for # strategies (ansible/plugins/strategy/__init__.py) + def hash_params(params): - if not isinstance(params, dict): - if isinstance(params, list): - return frozenset(params) + """ + Construct a data structure of parameters that is hashable. + + This requires changing any mutable data structures into immutable ones. + We chose a frozenset because role parameters have to be unique. + + .. warning:: this does not handle unhashable scalars. Two things + mitigate that limitation: + + 1) There shouldn't be any unhashable scalars specified in the yaml + 2) Our only choice would be to return an error anyway. + """ + # Any container is unhashable if it contains unhashable items (for + # instance, tuple() is a Hashable subclass but if it contains a dict, it + # cannot be hashed) + if isinstance(params, collections.Container) and not isinstance(params, (text_type, binary_type)): + if isinstance(params, collections.Mapping): + try: + # Optimistically hope the contents are all hashable + new_params = frozenset(params.items()) + except TypeError: + new_params = set() + for k, v in params.items(): + # Hash each entry individually + new_params.update((k, hash_params(v))) + new_params = frozenset(new_params) + + elif isinstance(params, (collections.Set, collections.Sequence)): + try: + # Optimistically hope the contents are all hashable + new_params = frozenset(params) + except TypeError: + new_params = set() + for v in params: + # Hash each entry individually + new_params.update(hash_params(v)) + new_params = frozenset(new_params) else: - return params - else: - s = set() - for k,v in iteritems(params): - if isinstance(v, dict): - s.update((k, hash_params(v))) - elif isinstance(v, list): - things = [] - for item in v: - things.append(hash_params(item)) - s.update((k, tuple(things))) - else: - s.update((k, v)) - return frozenset(s) + # This is just a guess. + new_params = frozenset(params) + return new_params + + # Note: We do not handle unhashable scalars but our only choice would be + # to raise an error there anyway. + return frozenset((params,)) + class Role(Base, Become, Conditional, Taggable): diff --git a/test/units/playbook/test_role.py b/test/units/playbook/test_role.py index 8e62729303b..1e93ec535ff 100644 --- a/test/units/playbook/test_role.py +++ b/test/units/playbook/test_role.py @@ -19,18 +19,107 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type +import collections + from ansible.compat.tests import unittest from ansible.compat.tests.mock import patch, MagicMock from ansible.errors import AnsibleError, AnsibleParserError from ansible.playbook.block import Block -from ansible.playbook.role import Role -from ansible.playbook.role.include import RoleInclude from ansible.playbook.task import Task from units.mock.loader import DictDataLoader from units.mock.path import mock_unfrackpath_noop +from ansible.playbook.role import Role +from ansible.playbook.role.include import RoleInclude +from ansible.playbook.role import hash_params + + +class TestHashParams(unittest.TestCase): + def test(self): + params = {'foo': 'bar'} + res = hash_params(params) + self._assert_set(res) + self._assert_hashable(res) + + def _assert_hashable(self, res): + a_dict = {} + try: + a_dict[res] = res + except TypeError as e: + self.fail('%s is not hashable: %s' % (res, e)) + + def _assert_set(self, res): + self.assertIsInstance(res, frozenset) + + def test_dict_tuple(self): + params = {'foo': (1, 'bar',)} + res = hash_params(params) + self._assert_set(res) + + def test_tuple(self): + params = (1, None, 'foo') + res = hash_params(params) + self._assert_hashable(res) + + def test_tuple_dict(self): + params = ({'foo': 'bar'}, 37) + res = hash_params(params) + self._assert_hashable(res) + + def test_list(self): + params = ['foo', 'bar', 1, 37, None] + res = hash_params(params) + self._assert_set(res) + self._assert_hashable(res) + + def test_dict_with_list_value(self): + params = {'foo': [1, 4, 'bar']} + res = hash_params(params) + self._assert_set(res) + self._assert_hashable(res) + + def test_empty_set(self): + params = set([]) + res = hash_params(params) + self._assert_hashable(res) + self._assert_set(res) + + def test_generator(self): + def my_generator(): + for i in ['a', 1, None, {}]: + yield i + + params = my_generator() + res = hash_params(params) + self._assert_hashable(res) + + def test_container_but_not_iterable(self): + # This is a Container that is not iterable, which is unlikely but... + class MyContainer(collections.Container): + def __init__(self, some_thing): + self.data = [] + self.data.append(some_thing) + + def __contains__(self, item): + return item in self.data + + def __hash__(self): + return hash(self.data) + + def __len__(self): + return len(self.data) + + def __call__(self): + return False + + foo = MyContainer('foo bar') + params = foo + + self.assertRaises(TypeError, hash_params, params) + + class TestRole(unittest.TestCase): def setUp(self):