Raise an error on invalid FA.isa value (#80040)

Avoids bad definitions of playbook classes

changelogs/fragments/isa-value-check.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - Raise an error when an incorrect ``isa`` type is passed to ``FieldAttribute``.

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):

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)
+    loop_var = NonInheritableFieldAttribute(isa='string', default='item', always_post_validate=True)
-    index_var = NonInheritableFieldAttribute(isa='str', always_post_validate=True)
+    index_var = NonInheritableFieldAttribute(isa='string', always_post_validate=True)
-    label = NonInheritableFieldAttribute(isa='str')
+    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)

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,

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)

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']
+        self.assertRaises(
-        ds = {'test_attr_unknown_isa': a_list}
+            AnsibleAssertionError,
-        bsc = self._base_validate(ds)
+            self._base_validate,
-        self.assertEqual(bsc.test_attr_unknown_isa, a_list)
+            {'test_attr_unknown_isa': True}
+        )
 
     def test_attr_method(self):
         ds = {'test_attr_method': 'value from the ds'}