Add some critical types to module_utils and fix logic

This change adds minimal types and uncovers several logic bugs, where
users of controller.get_one_or_many assumed it will not return a list.

plugins/module_utils/arguments.py

@@ -3,13 +3,13 @@
 # Copyright: Contributors to the Ansible project
 # Simplified BSD License (see licenses/simplified_bsd.txt or https://opensource.org/licenses/BSD-2-Clause)
 
-from __future__ import absolute_import, division, print_function
+from __future__ import absolute_import, annotations, division, print_function
 
 __metaclass__ = type
 
 from ansible.module_utils.basic import env_fallback
 
-AUTH_ARGSPEC = {
+AUTH_ARGSPEC: dict[str, dict[str, object]] = {
     "controller_host": {
         "fallback": (env_fallback, ["CONTROLLER_HOST"]),
         "required": True,

plugins/module_utils/common.py

@@ -5,10 +5,14 @@
 
 from __future__ import absolute_import, division, print_function
 
+from typing import Any, Optional, Union
+
 __metaclass__ = type
 
 
-def to_list_of_dict(result):
+def to_list_of_dict(
+    result: Optional[Union[dict[str, bool], list[dict[Any, Any]]]]
+) -> list[dict]:
     if result is None:
         return []
     if not isinstance(result, list):

plugins/module_utils/controller.py

@@ -2,7 +2,13 @@
 
 # Copyright: Contributors to the Ansible project
 # Simplified BSD License (see licenses/simplified_bsd.txt or https://opensource.org/licenses/BSD-2-Clause)
-from __future__ import absolute_import, division, print_function
+from __future__ import absolute_import, annotations, division, print_function
+
+from typing import List, Optional, Union
+
+from ansible.module_utils.basic import AnsibleModule
+
+from .client import Client
 
 __metaclass__ = type
 
@@ -13,7 +19,7 @@ class Controller:
     IDENTITY_FIELDS = {"users": "username"}
     ENCRYPTED_STRING = "$encrypted$"
 
-    def __init__(self, client, module):
+    def __init__(self, client: Client, module: AnsibleModule) -> None:
         self.client = client
         self.module = module
         self.result = {"changed": False}
@@ -24,7 +30,7 @@ def __init__(self, client, module):
             self.update_secrets = True
 
     @staticmethod
-    def get_name_field_from_endpoint(endpoint):
+    def get_name_field_from_endpoint(endpoint: str) -> str:
         return Controller.IDENTITY_FIELDS.get(endpoint, "name")
 
     def get_endpoint(self, endpoint, **kwargs):
@@ -81,8 +87,13 @@ def fail_wanted_one(self, response, endpoint, query_params):
         msg = f"Request to {display_endpoint} returned {response.json['count']} items, expected 1"
         raise EDAError(msg)
 
-    def get_exactly_one(self, endpoint, name=None, **kwargs):
-        return self.get_one_or_many(endpoint, name=name, allow_none=False, **kwargs)
+    def get_exactly_one(
+        self, endpoint, name=None, **kwargs
+    ) -> Optional[dict[str, bool]]:
+        result = self.get_one_or_many(endpoint, name=name, allow_none=False, **kwargs)
+        if isinstance(result, list):
+            raise RuntimeError("This should never happen.")
+        return result
 
     def resolve_name_to_id(self, endpoint, name):
         return self.get_exactly_one(endpoint, name)["id"]
@@ -95,7 +106,7 @@ def get_one_or_many(
         check_exists=False,
         want_one=True,
         **kwargs,
-    ):
+    ) -> Union[None, dict[str, bool], List]:
         new_kwargs = kwargs.copy()
         response = None
 
@@ -142,6 +153,7 @@ def get_one_or_many(
         if check_exists:
             self.result["id"] = response.json["results"][0]["id"]
             return self.result
+        return None
 
     def create_if_needed(
         self,

plugins/modules/activation.py

@@ -145,6 +145,7 @@
   sample: 37
 """
 
+from typing import Any
 
 from ansible.module_utils.basic import AnsibleModule
 
@@ -154,7 +155,7 @@
 from ..module_utils.errors import EDAError
 
 
-def lookup(module, controller, endpoint, name):
+def lookup(module: AnsibleModule, controller: Controller, endpoint, name):
     result = None
     try:
         result = controller.resolve_name_to_id(endpoint, name)
@@ -163,7 +164,7 @@ def lookup(module, controller, endpoint, name):
     return result
 
 
-def create_params(module, controller):
+def create_params(module: AnsibleModule, controller: Controller) -> dict[str, Any]:
     activation_params = {}
 
     # Get the project id
@@ -182,7 +183,7 @@ def create_params(module, controller):
         params = {"data": {"project_id": project_id}}
     if module.params.get("rulebook_name"):
         try:
-            rulebook = controller.get_one_or_many(
+            rulebook = controller.get_exactly_one(
                 "rulebooks", name=module.params["rulebook_name"], **params
             )
         except EDAError as e:
@@ -270,7 +271,7 @@ def create_params(module, controller):
     return activation_params
 
 
-def main():
+def main() -> None:
     argument_spec = dict(
         name=dict(type="str", required=True),
         description=dict(type="str"),
@@ -324,7 +325,7 @@ def main():
 
     # Attempt to find rulebook activation based on the provided name
     try:
-        activation = controller.get_one_or_many("activations", name=name)
+        activation = controller.get_exactly_one("activations", name=name)
     except EDAError as e:
         module.fail_json(msg=f"Failed to get rulebook activation: {e}")
 

plugins/modules/activation_info.py

@@ -87,7 +87,7 @@
 from ..module_utils.errors import EDAError
 
 
-def main():
+def main() -> None:
     argument_spec = dict(
         name=dict(type="str", required=False),
     )

plugins/modules/controller_token.py

@@ -85,7 +85,7 @@
 from ..module_utils.errors import EDAError
 
 
-def main():
+def main() -> None:
     argument_spec = dict(
         name=dict(required=True),
         description=dict(),

plugins/modules/credential.py

@@ -101,7 +101,7 @@ def lookup(module, controller, endpoint, name):
     return result
 
 
-def main():
+def main() -> None:
     argument_spec = dict(
         name=dict(type="str", required=True),
         new_name=dict(type="str"),

plugins/modules/credential_info.py

@@ -83,7 +83,7 @@
 from ..module_utils.errors import EDAError
 
 
-def main():
+def main() -> None:
     argument_spec = dict(
         name=dict(type="str", required=False),
     )

plugins/modules/credential_type.py

@@ -90,7 +90,7 @@
 from ..module_utils.errors import EDAError
 
 
-def main():
+def main() -> None:
     argument_spec = dict(
         name=dict(type="str", required=True),
         new_name=dict(type="str"),

plugins/modules/credential_type_info.py

@@ -4,7 +4,7 @@
 # Copyright: Contributors to the Ansible project
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
-from __future__ import absolute_import, division, print_function
+from __future__ import absolute_import, annotations, division, print_function
 
 __metaclass__ = type
 
@@ -81,7 +81,7 @@
 from ..module_utils.errors import EDAError
 
 
-def main():
+def main() -> None:
     argument_spec = dict(
         name=dict(type="str", required=False),
     )

plugins/modules/decision_environment.py

@@ -96,7 +96,7 @@
 from ..module_utils.errors import EDAError
 
 
-def main():
+def main() -> None:
     argument_spec = dict(
         name=dict(required=True),
         new_name=dict(),
@@ -155,7 +155,7 @@ def main():
     credential_type = None
     if credential:
         try:
-            credential_type = controller.get_one_or_many(
+            credential_type = controller.get_exactly_one(
                 "eda-credentials", name=credential
             )
         except EDAError as eda_err:

plugins/modules/decision_environment_info.py

@@ -60,6 +60,8 @@
   ]
 """
 
+from typing import Any
+
 from ansible.module_utils.basic import AnsibleModule
 
 from ..module_utils.arguments import AUTH_ARGSPEC
@@ -69,8 +71,8 @@
 from ..module_utils.errors import EDAError
 
 
-def main():
-    argument_spec = dict(
+def main() -> None:
+    argument_spec: dict[str, Any] = dict(
         name=dict(),
     )
 
@@ -90,7 +92,6 @@ def main():
     controller = Controller(client, module)
 
     decision_environment_name = module.params.get("name")
-    ret = {}
 
     try:
         ret = controller.get_one_or_many(
@@ -100,6 +101,7 @@ def main():
         )
     except EDAError as eda_err:
         module.fail_json(msg=str(eda_err))
+        raise  # https://github.com/ansible/ansible/pull/83814
 
     module.exit_json(decision_environments=to_list_of_dict(ret))
 

plugins/modules/project.py

@@ -81,6 +81,8 @@
     state: absent
 """
 
+from typing import Any
+
 from ansible.module_utils.basic import AnsibleModule
 
 from ..module_utils.arguments import AUTH_ARGSPEC
@@ -89,7 +91,7 @@
 from ..module_utils.errors import EDAError
 
 
-def main():
+def main() -> None:
     argument_spec = dict(
         name=dict(required=True),
         new_name=dict(),
@@ -135,7 +137,7 @@ def main():
             module.fail_json(msg=str(eda_err))
         module.exit_json(**ret)
 
-    project_type_params = {}
+    project_type_params: dict[str, Any] = {}
     if description:
         project_type_params["description"] = description
     if url:
@@ -144,11 +146,12 @@ def main():
     credential_id = None
     if credential:
         try:
-            credential_id = controller.get_one_or_many(
+            credential_id = controller.get_exactly_one(
                 "eda-credentials", name=credential
             )
         except EDAError as eda_err:
             module.fail_json(msg=str(eda_err))
+            raise  # https://github.com/ansible/ansible/pull/83814
 
     if credential_id is not None:
         # this is resolved earlier, so save an API call and don't do it again

plugins/modules/project_info.py

@@ -73,6 +73,8 @@
 """  # NOQA
 
 # pylint: disable=wrong-import-position,
+from typing import Any
+
 from ansible.module_utils.basic import AnsibleModule
 
 # pylint: disable=relative-beyond-top-level
@@ -82,8 +84,8 @@
 from ..module_utils.errors import EDAError
 
 
-def main():
-    argument_spec = dict(
+def main() -> None:
+    argument_spec: dict[str, Any] = dict(
         name=dict(),
     )
 
@@ -104,19 +106,19 @@ def main():
 
     project_name = module.params.get("name")
 
-    ret = {}
+    ret: list[Any] = []
 
     try:
-        ret = controller.get_one_or_many(
+        result = controller.get_one_or_many(
             project_endpoint, name=project_name, want_one=False
         )
     except EDAError as eda_err:
         module.fail_json(msg=str(eda_err))
 
-    if ret is None:
+    if result is None:
         ret = []
-    if not isinstance(ret, list):
-        ret = [ret]
+    if not isinstance(result, list):
+        ret = [result]
     module.exit_json(projects=ret)
 
 

tests/sanity/ignore-2.15.txt

@@ -0,0 +1 @@
+ignore-2.17.txt

tests/sanity/ignore-2.16.txt

@@ -0,0 +1 @@
+ignore-2.17.txt

tests/sanity/ignore-2.17.txt

@@ -0,0 +1,12 @@
+plugins/module_utils/common.py import-3.7!skip
+plugins/module_utils/common.py import-3.8!skip
+plugins/modules/activation.py import-3.7!skip
+plugins/modules/activation.py import-3.8!skip
+plugins/modules/activation_info.py import-3.7!skip
+plugins/modules/activation_info.py import-3.8!skip
+plugins/modules/credential_info.py import-3.7!skip
+plugins/modules/credential_info.py import-3.8!skip
+plugins/modules/credential_type_info.py import-3.7!skip
+plugins/modules/credential_type_info.py import-3.8!skip
+plugins/modules/decision_environment_info.py import-3.7!skip
+plugins/modules/decision_environment_info.py import-3.8!skip

tests/sanity/ignore-2.18.txt

@@ -0,0 +1 @@
+ignore-2.17.txt