From cfa7acbc19548b3953fc0bfdb78ee9f0e9c7ea05 Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Wed, 8 Mar 2023 18:48:43 +0100 Subject: [PATCH] Raise an error on invalid FA.isa value (#80040) Avoids bad definitions of playbook classes --- changelogs/fragments/isa-value-check.yml | 2 ++ lib/ansible/playbook/base.py | 2 ++ lib/ansible/playbook/loop_control.py | 6 +++--- lib/ansible/playbook/role/metadata.py | 11 +---------- lib/ansible/playbook/task.py | 2 +- test/units/playbook/test_base.py | 11 ++++++----- 6 files changed, 15 insertions(+), 19 deletions(-) create mode 100644 changelogs/fragments/isa-value-check.yml diff --git a/changelogs/fragments/isa-value-check.yml b/changelogs/fragments/isa-value-check.yml new file mode 100644 index 00000000000..4924aa6f772 --- /dev/null +++ b/changelogs/fragments/isa-value-check.yml @@ -0,0 +1,2 @@ +minor_changes: + - Raise an error when an incorrect ``isa`` type is passed to ``FieldAttribute``. diff --git a/lib/ansible/playbook/base.py b/lib/ansible/playbook/base.py index cb2a8829464..94f8a155d4b 100644 --- a/lib/ansible/playbook/base.py +++ b/lib/ansible/playbook/base.py @@ -486,6 +486,8 @@ class FieldAttributeBase: if not isinstance(value, attribute.class_type): raise TypeError("%s is not a valid %s (got a %s instead)" % (name, attribute.class_type, type(value))) value.post_validate(templar=templar) + else: + raise AnsibleAssertionError(f"Unknown value for attribute.isa: {attribute.isa}") return value def set_to_context(self, name): diff --git a/lib/ansible/playbook/loop_control.py b/lib/ansible/playbook/loop_control.py index d69e14fb94a..4df0a73ffe7 100644 --- a/lib/ansible/playbook/loop_control.py +++ b/lib/ansible/playbook/loop_control.py @@ -25,9 +25,9 @@ from ansible.playbook.base import FieldAttributeBase class LoopControl(FieldAttributeBase): - loop_var = NonInheritableFieldAttribute(isa='str', default='item', always_post_validate=True) - index_var = NonInheritableFieldAttribute(isa='str', always_post_validate=True) - label = NonInheritableFieldAttribute(isa='str') + loop_var = NonInheritableFieldAttribute(isa='string', default='item', always_post_validate=True) + index_var = NonInheritableFieldAttribute(isa='string', always_post_validate=True) + label = NonInheritableFieldAttribute(isa='string') pause = NonInheritableFieldAttribute(isa='float', default=0, always_post_validate=True) extended = NonInheritableFieldAttribute(isa='bool', always_post_validate=True) extended_allitems = NonInheritableFieldAttribute(isa='bool', default=True, always_post_validate=True) diff --git a/lib/ansible/playbook/role/metadata.py b/lib/ansible/playbook/role/metadata.py index a4dbcf7e9ab..8366a630a24 100644 --- a/lib/ansible/playbook/role/metadata.py +++ b/lib/ansible/playbook/role/metadata.py @@ -41,7 +41,7 @@ class RoleMetadata(Base, CollectionSearch): allow_duplicates = NonInheritableFieldAttribute(isa='bool', default=False) dependencies = NonInheritableFieldAttribute(isa='list', default=list) - galaxy_info = NonInheritableFieldAttribute(isa='GalaxyInfo') + galaxy_info = NonInheritableFieldAttribute(isa='dict') argument_specs = NonInheritableFieldAttribute(isa='dict', default=dict) def __init__(self, owner=None): @@ -110,15 +110,6 @@ class RoleMetadata(Base, CollectionSearch): except AssertionError as e: raise AnsibleParserError("A malformed list of role dependencies was encountered.", obj=self._ds, orig_exc=e) - def _load_galaxy_info(self, attr, ds): - ''' - This is a helper loading function for the galaxy info entry - in the metadata, which returns a GalaxyInfo object rather than - a simple dictionary. - ''' - - return ds - def serialize(self): return dict( allow_duplicates=self._allow_duplicates, diff --git a/lib/ansible/playbook/task.py b/lib/ansible/playbook/task.py index f6b6caf1883..f76c430b232 100644 --- a/lib/ansible/playbook/task.py +++ b/lib/ansible/playbook/task.py @@ -75,7 +75,7 @@ class Task(Base, Conditional, Taggable, CollectionSearch, Notifiable, Delegatabl changed_when = NonInheritableFieldAttribute(isa='list', default=list) delay = NonInheritableFieldAttribute(isa='int', default=5) failed_when = NonInheritableFieldAttribute(isa='list', default=list) - loop = NonInheritableFieldAttribute() + loop = NonInheritableFieldAttribute(isa='list') loop_control = NonInheritableFieldAttribute(isa='class', class_type=LoopControl, default=LoopControl) poll = NonInheritableFieldAttribute(isa='int', default=C.DEFAULT_POLL_INTERVAL) register = NonInheritableFieldAttribute(isa='string', static=True) diff --git a/test/units/playbook/test_base.py b/test/units/playbook/test_base.py index f5559ad3606..bedd96a8443 100644 --- a/test/units/playbook/test_base.py +++ b/test/units/playbook/test_base.py @@ -21,7 +21,7 @@ __metaclass__ = type from units.compat import unittest -from ansible.errors import AnsibleParserError +from ansible.errors import AnsibleParserError, AnsibleAssertionError from ansible.module_utils.six import string_types from ansible.playbook.attribute import FieldAttribute, NonInheritableFieldAttribute from ansible.template import Templar @@ -581,10 +581,11 @@ class TestBaseSubClass(TestBase): bsc.post_validate, templar) def test_attr_unknown(self): - a_list = ['some string'] - ds = {'test_attr_unknown_isa': a_list} - bsc = self._base_validate(ds) - self.assertEqual(bsc.test_attr_unknown_isa, a_list) + self.assertRaises( + AnsibleAssertionError, + self._base_validate, + {'test_attr_unknown_isa': True} + ) def test_attr_method(self): ds = {'test_attr_method': 'value from the ds'}