From 3324debd685df0c8861b5833d343d24a52e80f9c Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Wed, 18 Mar 2026 13:59:57 -0400 Subject: [PATCH 1/3] Remove typeguard, add mypy --- .github/workflows/linting.yml | 10 ++++++ Makefile | 6 +++- pyproject.toml | 32 ++++++++++++++++++- requirements/requirements_dev.txt | 4 ++- test_app/settings.py | 12 ------- .../authenticator_plugins/test_ldap.py | 3 -- test_app/tests/lib/utils/test_duration.py | 7 ++-- test_app/tests/lib/utils/test_hashing.py | 4 --- test_app/tests/lib/utils/test_validation.py | 2 -- 9 files changed, 51 insertions(+), 29 deletions(-) diff --git a/.github/workflows/linting.yml b/.github/workflows/linting.yml index 10e5b0a67..7004d63ca 100644 --- a/.github/workflows/linting.yml +++ b/.github/workflows/linting.yml @@ -25,6 +25,8 @@ jobs: command: check_black - name: api-isort command: check_isort + - name: api-mypy + command: check_mypy - name: pure-python-imports command: check_pure_python_imports steps: @@ -40,9 +42,17 @@ jobs: with: python-version: 3.12 + - name: Install build requirements for mypy + if: matrix.tests.name == 'api-mypy' + run: sudo apt-get update && sudo apt-get install -y libsasl2-dev libldap2-dev libssl-dev libxmlsec1-dev + - name: Install requirments run: pip3.12 install -r requirements/requirements_dev.txt + - name: Install all dependencies for mypy + if: matrix.tests.name == 'api-mypy' + run: pip3.12 install -r requirements/requirements_all.txt + - name: Install test dependencies for pure-python-imports if: matrix.tests.name == 'pure-python-imports' run: | diff --git a/Makefile b/Makefile index be2d13514..9ce3c4bb7 100644 --- a/Makefile +++ b/Makefile @@ -11,7 +11,7 @@ COMPOSE_UP_OPTS ?= DOCKER_COMPOSE ?= docker compose .PHONY: PYTHON_VERSION clean build\ - check lint check_black check_flake8 check_isort git_hooks_config + check lint check_black check_flake8 check_isort check_mypy git_hooks_config PYTHON_VERSION: @echo "$(subst python,,$(PYTHON))" @@ -49,6 +49,10 @@ check_flake8: check_isort: tox -e isort -- --check $(CHECK_SYNTAX_FILES) +## Run mypy type checking +check_mypy: + tox -e mypy + ## Starts a postgres container in the background if one is not running # Options: diff --git a/pyproject.toml b/pyproject.toml index 0bf530001..8f3105141 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -108,7 +108,7 @@ legacy_tox_ini = """ py312-in-files labels = test = py311-check, py312-check, py311, py312, py311-sqlite, py312-sqlite, py312-django4, py311-in-files, py312-in-files - lint = flake8, black, isort + lint = flake8, black, isort, mypy [testenv] deps = @@ -181,8 +181,38 @@ legacy_tox_ini = """ isort==6.0.0 docker = commands = isort {posargs:.} + + [testenv:mypy] + deps = + -r{toxinidir}/requirements/requirements_all.txt + -r{toxinidir}/requirements/requirements_dev.txt + docker = + commands = mypy ansible_base {posargs} """ +[tool.mypy] +python_version = "3.11" +plugins = ["mypy_django_plugin.main", "mypy_drf_plugin.main"] +ignore_missing_imports = true +follow_imports = "silent" +warn_unused_configs = true +warn_redundant_casts = true +warn_unused_ignores = true +check_untyped_defs = false +disallow_untyped_defs = false +# Start permissive - tighten these as the codebase gets annotated +disable_error_code = [ + "attr-defined", "assignment", "union-attr", "var-annotated", "misc", + "has-type", "used-before-def", "return", "return-value", + "arg-type", "call-arg", "index", "operator", "valid-type", + "import-untyped", "str-bytes-safe", "syntax", + "dict-item", "name-defined", "django-manager-missing", +] +exclude = ["test_app/", "migrations/", "\\.tox/", "build/", "\\.venv/", "venv/"] + +[tool.django-stubs] +django_settings_module = "test_app.settings" + [tool.coverage.run] omit = ["test_app/*", "manage.py"] relative_files = true diff --git a/requirements/requirements_dev.txt b/requirements/requirements_dev.txt index 037e3e8b1..a31db7387 100644 --- a/requirements/requirements_dev.txt +++ b/requirements/requirements_dev.txt @@ -12,7 +12,9 @@ isort==6.0.0 # Linting tool, if changed update pyproject.toml as well jsonschema tox tox-docker -typeguard +mypy +django-stubs[compatible-mypy] +djangorestframework-stubs[compatible-mypy] openapi-spec-validator>=0.6.1 # Adapted to changing import locations in jsonschema pytest pytest-asyncio diff --git a/test_app/settings.py b/test_app/settings.py index cbab33745..1ab1242fb 100644 --- a/test_app/settings.py +++ b/test_app/settings.py @@ -5,20 +5,8 @@ by default it will be development. """ -import sys - from ansible_base.lib.dynamic_config import export, factory, load_envvars, load_standard_settings_files -if "pytest" in sys.modules: - # https://github.com/agronholm/typeguard/issues/260 - # Enable runtime type checking only for running tests - # must be done here because python hooks will not reliably call the - # typguard plugin setup before other plugins which setup Django, which loads settings. - # Lower in this settings file, the dynamic config imports ansible_base - from typeguard import install_import_hook - - install_import_hook(packages=["ansible_base"]) - # Create a the standard DYNACONF instance which will come with DAB defaults # this loads the files passed to `settings_files` list # and environment specific file e.g: development_{filename}.py diff --git a/test_app/tests/authentication/authenticator_plugins/test_ldap.py b/test_app/tests/authentication/authenticator_plugins/test_ldap.py index 1496e601d..2773b8364 100644 --- a/test_app/tests/authentication/authenticator_plugins/test_ldap.py +++ b/test_app/tests/authentication/authenticator_plugins/test_ldap.py @@ -6,7 +6,6 @@ import pytest from django_auth_ldap import config from rest_framework.serializers import ValidationError -from typeguard import suppress_type_checks from ansible_base.authentication.authenticator_plugins.ldap import ( _MUST_BE_AN_ARRAY_MESSAGE, @@ -372,7 +371,6 @@ def test_ldap_backend_authenticate_empty_username_password( assert response.status_code == 401 -@suppress_type_checks @pytest.mark.django_db @mock.patch("rest_framework.views.APIView.authentication_classes", [SessionAuthentication]) @mock.patch("ansible_base.authentication.authenticator_plugins.ldap.LDAPBackend.authenticate") @@ -404,7 +402,6 @@ def test_ldap_backend_authenticate_valid_user( assert response.data['results'][0]['name'] == ldap_authenticator.name -@suppress_type_checks @pytest.mark.django_db @mock.patch("rest_framework.views.APIView.authentication_classes", [SessionAuthentication]) @mock.patch("ansible_base.authentication.authenticator_plugins.ldap.LDAPBackend.authenticate") diff --git a/test_app/tests/lib/utils/test_duration.py b/test_app/tests/lib/utils/test_duration.py index 4bcd7998b..2ae70dcb8 100644 --- a/test_app/tests/lib/utils/test_duration.py +++ b/test_app/tests/lib/utils/test_duration.py @@ -156,11 +156,8 @@ def test_convert_to_seconds_invalid_default_type(invalid_default_value, expected """ Test that non-integer default values log a warning with stack trace and use 10 instead. - Note: Only booleans are tested here because the project uses typeguard for runtime - type checking, which prevents other invalid types (str, float, list, dict, None) - from even reaching the function. This is the expected behavior - typeguard provides - the first line of defense, and our isinstance check catches booleans (which are - technically ints in Python but should not be accepted as defaults). + Note: Only booleans are tested here because bool is a subclass of int in Python, + so it passes isinstance(x, int) checks but should not be accepted as defaults. The stack_info=True in the logger call provides developers with a full stack trace showing exactly where convert_to_seconds was called with an invalid default. diff --git a/test_app/tests/lib/utils/test_hashing.py b/test_app/tests/lib/utils/test_hashing.py index 90b83365f..da4bebdcd 100644 --- a/test_app/tests/lib/utils/test_hashing.py +++ b/test_app/tests/lib/utils/test_hashing.py @@ -1,7 +1,6 @@ import uuid from rest_framework.serializers import CharField, IntegerField, Serializer, UUIDField -from typeguard import suppress_type_checks from ansible_base.lib.utils.hashing import hash_serializer_data @@ -14,19 +13,16 @@ class DataSerializer(Serializer): uuid = UUIDField() -@suppress_type_checks def test_hash_serializer_data_idempotency(): """Test hashing same data gives same output""" assert hash_serializer_data(DATA, DataSerializer) == hash_serializer_data(DATA, DataSerializer) -@suppress_type_checks def test_hash_serializer_data_difference(): """Test hashing different data changes the hash""" assert hash_serializer_data(DATA, DataSerializer) != hash_serializer_data({**DATA, **{"id": 4567}}, DataSerializer) -@suppress_type_checks def test_hash_serializer_with_nested_field(): """Test hashing can be performed on nested data""" NESTED_DATA = {"field": {"name": "foo", "id": 1234}} diff --git a/test_app/tests/lib/utils/test_validation.py b/test_app/tests/lib/utils/test_validation.py index cb2824497..9fc3317de 100644 --- a/test_app/tests/lib/utils/test_validation.py +++ b/test_app/tests/lib/utils/test_validation.py @@ -1,6 +1,5 @@ import pytest from rest_framework.exceptions import ValidationError -from typeguard import suppress_type_checks from ansible_base.lib.utils.validation import ( _is_valid_domain_format, @@ -16,7 +15,6 @@ ) -@suppress_type_checks @pytest.mark.parametrize( "valid,url,schemes,allow_plain_hostname", [ From 00b2dca544e9578b92f8d5d5018af48196814127 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Wed, 18 Mar 2026 20:25:15 -0400 Subject: [PATCH 2/3] Work through a few mypy rules --- ansible_base/activitystream/serializers.py | 2 ++ .../authentication/authenticator_plugins/ldap.py | 8 ++++---- ansible_base/authentication/models/authenticator.py | 2 +- .../authentication/models/authenticator_map.py | 4 ++-- .../authentication/models/authenticator_user.py | 2 +- ansible_base/authentication/utils/authentication.py | 2 +- .../authentication/utils/authenticator_map.py | 7 ++++--- ansible_base/authentication/utils/claims.py | 9 +++++---- ansible_base/jwt_consumer/common/util.py | 4 ++-- ansible_base/lib/channels/middleware.py | 2 +- ansible_base/lib/redis/client.py | 11 ++++++++--- .../lib/routers/association_resource_router.py | 2 +- ansible_base/lib/utils/create_system_user.py | 2 +- ansible_base/lib/utils/db.py | 2 +- ansible_base/oauth2_provider/utils.py | 2 +- ansible_base/rbac/claims.py | 6 +++--- ansible_base/rbac/managed.py | 2 +- ansible_base/rbac/models/content_type.py | 6 +++--- ansible_base/rbac/models/role.py | 5 +++-- ansible_base/rbac/permission_registry.py | 3 ++- ansible_base/rbac/validators.py | 4 ++-- ansible_base/resource_registry/registry.py | 2 +- ansible_base/resource_registry/resource_server.py | 2 +- ansible_base/resource_registry/service_client.py | 2 +- .../utils/resource_type_processor.py | 2 +- pyproject.toml | 9 +++++---- requirements/requirements_dev.txt | 2 ++ 27 files changed, 60 insertions(+), 46 deletions(-) diff --git a/ansible_base/activitystream/serializers.py b/ansible_base/activitystream/serializers.py index 01d4a93d3..b479f08ef 100644 --- a/ansible_base/activitystream/serializers.py +++ b/ansible_base/activitystream/serializers.py @@ -32,10 +32,12 @@ class Meta: def get_content_type_model(self, obj) -> Optional[str]: if obj.content_type: return obj.content_type.model + return None def get_related_content_type_model(self, obj) -> Optional[str]: if obj.related_content_type: return obj.related_content_type.model + return None def _field_value_to_python(self, entry, field_name, value): model = entry.content_type.model_class() diff --git a/ansible_base/authentication/authenticator_plugins/ldap.py b/ansible_base/authentication/authenticator_plugins/ldap.py index 52c58a0be..85d5bdadf 100644 --- a/ansible_base/authentication/authenticator_plugins/ldap.py +++ b/ansible_base/authentication/authenticator_plugins/ldap.py @@ -466,7 +466,7 @@ def __init__(self, prefix: str = 'AUTH_LDAP_', defaults: dict = {}): # Group type needs to be an object instead of a String so instantiate it group_type_class = find_class_in_modules(defaults["GROUP_TYPE"]) - setattr(self, 'GROUP_TYPE', group_type_class(**defaults['GROUP_TYPE_PARAMS'])) + setattr(self, 'GROUP_TYPE', group_type_class(**defaults['GROUP_TYPE_PARAMS'])) # type: ignore[operator] class AuthenticatorPlugin(LDAPBackend, AbstractAuthenticatorPlugin): @@ -482,9 +482,9 @@ def __init__(self, database_instance=None, *args, **kwargs): self.configuration_encrypted_fields = ['BIND_PASSWORD'] self.set_logger(logger) - def authenticate(self, request, username=None, password=None, **kwargs) -> (object, dict, list): + def authenticate(self, request, username=None, password=None, **kwargs) -> tuple[object, dict, list] | None: if not username or not password: - return + return None users_groups = [] if not self.database_instance: @@ -563,7 +563,7 @@ def authenticate(self, request, username=None, password=None, **kwargs) -> (obje user_details={}, extra_data=user_from_ldap.ldap_user.attrs.data, ) - return update_user_claims(user_from_ldap, self.database_instance, users_groups) + return update_user_claims(user_from_ldap, self.database_instance, users_groups) # type: ignore[return-value] except Exception: logger.exception(f"Encountered an error authenticating to LDAP {self.database_instance.name}") return None diff --git a/ansible_base/authentication/models/authenticator.py b/ansible_base/authentication/models/authenticator.py index fbec14ebe..bda815dbc 100644 --- a/ansible_base/authentication/models/authenticator.py +++ b/ansible_base/authentication/models/authenticator.py @@ -13,7 +13,7 @@ def get_next_authenticator_order(): return largest_order_authenticator['order'] + 1 if largest_order_authenticator else 1 -class Authenticator(UniqueNamedCommonModel): +class Authenticator(UniqueNamedCommonModel): # type: ignore[django-manager-missing] ignore_relations = ['authenticator_users'] enabled = fields.BooleanField(default=False, help_text="Should this authenticator be enabled.") create_objects = fields.BooleanField(default=True, help_text="Allow authenticator to create objects (users, teams, organizations).") diff --git a/ansible_base/authentication/models/authenticator_map.py b/ansible_base/authentication/models/authenticator_map.py index e9c2f6da3..938a9b468 100644 --- a/ansible_base/authentication/models/authenticator_map.py +++ b/ansible_base/authentication/models/authenticator_map.py @@ -14,11 +14,11 @@ class Meta: constraints = [ models.CheckConstraint( name="%(app_label)s_%(class)s_require_org_team_if_team_map", - check=(~models.Q(map_type='team') | models.Q(team__isnull=False) & models.Q(organization__isnull=False)), + check=(~models.Q(map_type='team') | models.Q(team__isnull=False) & models.Q(organization__isnull=False)), # type: ignore[call-arg] ), models.CheckConstraint( name="%(app_label)s_%(class)s_require_org_if_org_map", - check=(~models.Q(map_type='organization') | models.Q(organization__isnull=False)), + check=(~models.Q(map_type='organization') | models.Q(organization__isnull=False)), # type: ignore[call-arg] ), ] unique_together = ['name', 'authenticator'] diff --git a/ansible_base/authentication/models/authenticator_user.py b/ansible_base/authentication/models/authenticator_user.py index 17d53319b..77ba457d6 100644 --- a/ansible_base/authentication/models/authenticator_user.py +++ b/ansible_base/authentication/models/authenticator_user.py @@ -23,7 +23,7 @@ def b64_encode_binary_data_in_dict(obj: Any) -> Any: return obj -class AuthenticatorUser(AbstractUserSocialAuth, AbstractCommonModel): +class AuthenticatorUser(AbstractUserSocialAuth, AbstractCommonModel): # type: ignore[django-manager-missing] """ This appends extra information on the local user model that includes extra data returned by the authenticators and links the user to the authenticator that they used to login. diff --git a/ansible_base/authentication/utils/authentication.py b/ansible_base/authentication/utils/authentication.py index e4677a870..8b749729a 100644 --- a/ansible_base/authentication/utils/authentication.py +++ b/ansible_base/authentication/utils/authentication.py @@ -71,7 +71,7 @@ def migrate_from_existing_authenticator( ) if len(migrate_users) == 0: - return + return None try: main_user = AuthenticatorUser.objects.get(uid=uid, provider=authenticator).user diff --git a/ansible_base/authentication/utils/authenticator_map.py b/ansible_base/authentication/utils/authenticator_map.py index 5d96eeeca..4607c41f1 100644 --- a/ansible_base/authentication/utils/authenticator_map.py +++ b/ansible_base/authentication/utils/authenticator_map.py @@ -37,6 +37,7 @@ def check_expansion_syntax(value: Optional[str]) -> Optional[TranslatedString]: if has_expansion(value) and not _EXPANSION_RE.search(value): return _("Expansion only supports the format {% for_attr_value(attribute) %}") + return None def expand_syntax(attributes: dict, auth_map: AuthenticatorMap) -> list[Dict[str, str]]: @@ -191,7 +192,7 @@ def check_role_type(map_type: Optional[str], role: Optional[str], org: Optional[ if not _is_rbac_installed(): errors['role'] = _("You specified a role without RBAC installed ") - return errors + return errors # type: ignore[return-value] from ansible_base.rbac.models import RoleDefinition @@ -201,7 +202,7 @@ def check_role_type(map_type: Optional[str], role: Optional[str], org: Optional[ # system role is allowed for map type == role without further conditions if is_system_role and map_type == 'role': - return errors + return errors # type: ignore[return-value] is_org_role, is_team_role = False, False if not is_system_role: @@ -227,4 +228,4 @@ def check_role_type(map_type: Optional[str], role: Optional[str], org: Optional[ except ObjectDoesNotExist: errors['role'] = _("RoleDefinition {role} doesn't exist").format(role=role) - return errors + return errors # type: ignore[return-value] diff --git a/ansible_base/authentication/utils/claims.py b/ansible_base/authentication/utils/claims.py index 3f49f9fbe..2d32d4706 100644 --- a/ansible_base/authentication/utils/claims.py +++ b/ansible_base/authentication/utils/claims.py @@ -240,7 +240,7 @@ def process_groups(trigger_condition: dict, groups: list, map_id: int, tracking_ groups = [f"{group}".casefold() for group in groups] trigger_condition = _lowercase_group_triggers(trigger_condition) - invalid_conditions = set(trigger_condition.keys()) - set(TRIGGER_DEFINITION['groups']['keys'].keys()) + invalid_conditions = set(trigger_condition.keys()) - set(TRIGGER_DEFINITION['groups']['keys'].keys()) # type: ignore[index] if invalid_conditions: logger.warning(f"[{tracking_id}] The conditions {', '.join(invalid_conditions)} for groups in mapping {map_id} are invalid and won't be processed") @@ -283,6 +283,7 @@ def has_access_with_join(current_access: Optional[bool], new_access: bool, condi if condition == 'and': return current_access and new_access + return None def _lowercase_value(value: Any) -> Any: @@ -353,7 +354,7 @@ def _validate_join_condition(join_condition, map_id: int, tracking_id: str) -> s Returns: Valid join condition ('or' or 'and') """ - if join_condition not in TRIGGER_DEFINITION['attributes']['keys']['join_condition']['choices']: + if join_condition not in TRIGGER_DEFINITION['attributes']['keys']['join_condition']['choices']: # type: ignore[index] logger.warning(f"[{tracking_id}] Trigger join_condition {join_condition} on authenticator map {map_id} is invalid and will be set to 'or'") return 'or' return join_condition @@ -373,7 +374,7 @@ def _validate_attribute_conditions(attribute: str, condition: dict, map_id: int, True if conditions are valid and should be processed, False if should be skipped """ # Warn if there are any invalid conditions, we are just going to ignore them - invalid_conditions = set(condition.keys()) - set(TRIGGER_DEFINITION['attributes']['keys']['*']['keys'].keys()) + invalid_conditions = set(condition.keys()) - set(TRIGGER_DEFINITION['attributes']['keys']['*']['keys'].keys()) # type: ignore[index] if invalid_conditions: logger.warning( f"[{tracking_id}] The conditions {', '.join(invalid_conditions)} for attribute {attribute} " @@ -567,7 +568,7 @@ def _process_user_value( user_str = f"{a_user_value}".casefold() if _is_case_insensitivity_enabled() else f"{a_user_value}" # Evaluate condition - result = evaluate_fn(user_str, trigger_value) + result = evaluate_fn(user_str, trigger_value) # type: ignore[operator] has_access = has_access_with_join(has_access, result, join_condition) # Log result diff --git a/ansible_base/jwt_consumer/common/util.py b/ansible_base/jwt_consumer/common/util.py index e4ab672ed..60b83b1b7 100644 --- a/ansible_base/jwt_consumer/common/util.py +++ b/ansible_base/jwt_consumer/common/util.py @@ -61,7 +61,7 @@ def validate_x_trusted_proxy_header(header_value: str, ignore_cache=False) -> bo logger.warning(f"Timestamp {timestamp} was too old by {header_age_ms}ms to be valid-alter trusted_header_timeout if needed") return False except ValueError: - logger.warning(f"Unable to convert timestamp (base64) {b64encode(timestamp.encode('UTF-8'))} into an integer") + logger.warning(f"Unable to convert timestamp (base64) {b64encode(timestamp.encode('UTF-8')).decode()} into an integer") return False try: @@ -71,7 +71,7 @@ def validate_x_trusted_proxy_header(header_value: str, ignore_cache=False) -> bo return False try: - public_key.verify( + public_key.verify( # type: ignore[call-arg] signature_bytes, bytes(f'{_SHARED_SECRET}-{timestamp}', 'utf-8'), padding.PSS(mgf=padding.MGF1(hashes.SHA256()), salt_length=padding.PSS.MAX_LENGTH), diff --git a/ansible_base/lib/channels/middleware.py b/ansible_base/lib/channels/middleware.py index 47d1085ab..d792bd3cc 100644 --- a/ansible_base/lib/channels/middleware.py +++ b/ansible_base/lib/channels/middleware.py @@ -17,7 +17,7 @@ def _get_authenticated_user(scope: dict): request = HttpRequest() request.META = {_http_key(k.decode()): v.decode() for (k, v) in scope["headers"]} - auth_classes = [auth() for auth in api_settings.DEFAULT_AUTHENTICATION_CLASSES] + auth_classes = [auth() for auth in api_settings.DEFAULT_AUTHENTICATION_CLASSES] # type: ignore[operator] try: return Request(request, authenticators=auth_classes).user except Exception: diff --git a/ansible_base/lib/redis/client.py b/ansible_base/lib/redis/client.py index 942117dee..c9296d72f 100644 --- a/ansible_base/lib/redis/client.py +++ b/ansible_base/lib/redis/client.py @@ -40,7 +40,7 @@ class DABRedisRespectACLFlushMixin: At some point in the future we may also want to override flushall as well """ - def flushdb(self, asynchronous: Optional[bool] = None, **kwargs) -> ResponseT: + def flushdb(self, asynchronous: Optional[bool] = None, **kwargs) -> ResponseT: # type: ignore[return] if asynchronous is not None: logger.warning("DABRedis clients implement an ACL friendly FLUSHDB which can not be async") @@ -106,6 +106,11 @@ def connect(self, index: int = 0) -> Union[DABRedis, DABRedisCluster]: class RedisClientGetter: + url: str + connection_settings: dict + mode: str + redis_hosts: str + def _redis_parse_url(self) -> None: if self.url == '': # If there is no URL we have nothing to do @@ -190,7 +195,7 @@ def _get_hosts(self) -> None: raise ImproperlyConfigured(_('Unable to parse redis_hosts, see logs for more details')) def __init__(self, *args, **kwargs): - self.url = '' + self.url: str = '' def get_client(self, url: str = '', **kwargs) -> Union[DABRedis, DABRedisCluster]: # remove our settings which are invalid to the parent classes @@ -265,7 +270,7 @@ def get_redis_status(url: str = '', timeout: int = _DEFAULT_STATUS_TIMEOUT_SEC, response['status'] = determine_cluster_node_status(response['cluster_nodes']) # Now our status should be STATUS_GOOD or STATUS_DEGRADED # There is one more check we need to do and that is if the cluster_info did not return ok we are in a bad state - if response['cluster_info']['cluster_state'] != _REDIS_CLUSTER_OK_STATUS: + if response['cluster_info']['cluster_state'] != _REDIS_CLUSTER_OK_STATUS: # type: ignore[index] response['status'] = STATUS_FAILED except Exception as e: response['status'] = STATUS_FAILED diff --git a/ansible_base/lib/routers/association_resource_router.py b/ansible_base/lib/routers/association_resource_router.py index 83649d8c4..a37a8e89b 100644 --- a/ansible_base/lib/routers/association_resource_router.py +++ b/ansible_base/lib/routers/association_resource_router.py @@ -271,7 +271,7 @@ def __dir__(self): for method in exclude_list: setattr(AssociatedViewSetType, method, attribute_raiser) - class AssociatedViewSet(viewset, AssociationViewSetMethodsMixin, metaclass=AssociatedViewSetType): + class AssociatedViewSet(viewset, AssociationViewSetMethodsMixin, metaclass=AssociatedViewSetType): # type: ignore[valid-type] """Adjusted version of given viewset for related endpoint with only list views""" pass diff --git a/ansible_base/lib/utils/create_system_user.py b/ansible_base/lib/utils/create_system_user.py index 5cbdc23a7..6d8e1cb15 100644 --- a/ansible_base/lib/utils/create_system_user.py +++ b/ansible_base/lib/utils/create_system_user.py @@ -15,7 +15,7 @@ """ -def create_system_user(user_model: Type[models.Model]) -> models.Model: +def create_system_user(user_model: Type[models.Model]) -> Optional[models.Model]: # Note: We can't load models here so we can typecast to anything better than Model from ansible_base.lib.abstract_models.user import AbstractDABUser diff --git a/ansible_base/lib/utils/db.py b/ansible_base/lib/utils/db.py index 4d96a3e6b..3529aba2e 100644 --- a/ansible_base/lib/utils/db.py +++ b/ansible_base/lib/utils/db.py @@ -223,7 +223,7 @@ def combine_settings_dict(settings_dict1: dj_db_dict, settings_dict2: dj_db_dict settings_dict['OPTIONS'].update(extra_options) # Apply overrides from nested OPTIONS for the listener connection for k, v in settings_dict2.get('OPTIONS', {}).items(): - settings_dict['OPTIONS'][k] = v + settings_dict['OPTIONS'][k] = v # type: ignore[index] return settings_dict diff --git a/ansible_base/oauth2_provider/utils.py b/ansible_base/oauth2_provider/utils.py index 51d3d341d..74344cea1 100644 --- a/ansible_base/oauth2_provider/utils.py +++ b/ansible_base/oauth2_provider/utils.py @@ -7,7 +7,7 @@ User = get_user_model() -def is_external_account(user: User) -> Optional[Authenticator]: +def is_external_account(user: "User") -> Optional[Authenticator]: # type: ignore[valid-type] """ Determines whether the user is associated with any external login source. If they are, return the source. Otherwise, None. diff --git a/ansible_base/rbac/claims.py b/ansible_base/rbac/claims.py index 64e1e011d..60a15874c 100644 --- a/ansible_base/rbac/claims.py +++ b/ansible_base/rbac/claims.py @@ -237,7 +237,7 @@ def get_user_claims(user: Model) -> dict[str, Union[list[str], dict[str, Union[s global_roles = _get_user_global_roles(user) # Build final claims structure - return {'objects': object_arrays, 'object_roles': object_roles, 'global_roles': global_roles} + return {'objects': object_arrays, 'object_roles': object_roles, 'global_roles': global_roles} # type: ignore[dict-item] # ---- for claims saving ---- @@ -413,9 +413,9 @@ def get_user_claims_hashable_form(claims: dict) -> dict[str, Union[list[str], di ansible_ids.append(ansible_id) # Sort ansible_ids for deterministic ordering - hashable_claims['object_roles'][role_name] = sorted(ansible_ids) + hashable_claims['object_roles'][role_name] = sorted(ansible_ids) # type: ignore[index] - return hashable_claims + return hashable_claims # type: ignore[return-value] def get_claims_hash(hashable_claims: dict) -> str: diff --git a/ansible_base/rbac/managed.py b/ansible_base/rbac/managed.py index 687610666..b6e91153b 100644 --- a/ansible_base/rbac/managed.py +++ b/ansible_base/rbac/managed.py @@ -33,7 +33,7 @@ def get_permissions(self, apps) -> set[str]: return self.permission_list def get_translated_name(self) -> str: - return _(self.name) + return _(self.name) # type: ignore[return-value] def get_content_type(self, apps): model = self.get_model(apps) diff --git a/ansible_base/rbac/models/content_type.py b/ansible_base/rbac/models/content_type.py index db80a9f77..23a0b1c4f 100644 --- a/ansible_base/rbac/models/content_type.py +++ b/ansible_base/rbac/models/content_type.py @@ -40,7 +40,7 @@ def _add_to_cache(self, using: str, ct: django_models.Model) -> None: def _get_from_cache(self, opts: Options, service: str) -> django_models.Model: """Return a cached ``DABContentType`` for ``opts`` and ``service``.""" key = (service, opts.app_label, opts.model_name) - return self._cache[self.db][key] + return self._cache[self.db][key] # type: ignore[index] def _get_opts(self, model: Union[Type[django_models.Model], django_models.Model], for_concrete_model: bool) -> Options: """Return the ``Options`` object for ``model``.""" @@ -109,9 +109,9 @@ def get_for_models( ct = self._get_from_cache(opts, model_service) except KeyError: needed_models[(model_service, opts.app_label)].add(opts.model_name) - needed_opts[(model_service, opts.app_label, opts.model_name)].append(model) + needed_opts[(model_service, opts.app_label, opts.model_name)].append(model) # type: ignore[index] else: - results[model] = ct + results[model] = ct # type: ignore[index] if needed_opts: condition = django_models.Q( diff --git a/ansible_base/rbac/models/role.py b/ansible_base/rbac/models/role.py index 22afe338e..5ccbabe53 100644 --- a/ansible_base/rbac/models/role.py +++ b/ansible_base/rbac/models/role.py @@ -80,12 +80,12 @@ def contribute_to_class(self, cls: Type[models.Model], name: str) -> None: def give_creator_permissions(self, user, obj) -> Optional['RoleUserAssignment']: if not permission_registry.is_registered(obj): - return # Exit before getting content type, which will not exist + return None # Exit before getting content type, which will not exist # If the user is a superuser, no need to bother giving the creator permissions for super_flag in settings.ANSIBLE_BASE_BYPASS_SUPERUSER_FLAGS: if getattr(user, super_flag): - return + return None needed_actions = settings.ANSIBLE_BASE_CREATOR_DEFAULTS @@ -120,6 +120,7 @@ def give_creator_permissions(self, user, obj) -> Optional['RoleUserAssignment']: # reverse sync the assignment maybe_reverse_sync_assignment(assignment) return assignment + return None def get_or_create(self, permissions=(), defaults=None, **kwargs): """Override get_or_create to support lookup by permissions list. diff --git a/ansible_base/rbac/permission_registry.py b/ansible_base/rbac/permission_registry.py index 0af451cf3..c9f6b2a43 100644 --- a/ansible_base/rbac/permission_registry.py +++ b/ansible_base/rbac/permission_registry.py @@ -27,7 +27,7 @@ class PermissionRegistry: def __init__(self): - self._registry: set[Model] = set() # model registry + self._registry: set[type[Model]] = set() # model registry self._name_to_model = dict() self._parent_fields = dict() self._managed_roles = dict() # code-defined role definitions, managed=True @@ -92,6 +92,7 @@ def get_managed_role_constructor_by_name(self, name: str) -> Optional[ManagedRol for managed_role in self._managed_roles.values(): if managed_role.name == name: return managed_role + return None def register_managed_role_constructor(self, shortname: str, managed_role: ManagedRoleConstructor) -> None: """Add the given managed role to the managed role registry""" diff --git a/ansible_base/rbac/validators.py b/ansible_base/rbac/validators.py index bc62ee1ca..38b861b6f 100644 --- a/ansible_base/rbac/validators.py +++ b/ansible_base/rbac/validators.py @@ -20,7 +20,7 @@ def system_roles_enabled(): def prnt_model_name(model: Optional[Union[Type[Model], Type[RemoteObject]]]) -> str: - return model._meta.model_name if model else 'global role' + return model._meta.model_name if model else 'global role' # type: ignore[return-value] def prnt_codenames(codename_set: set[str]) -> str: @@ -67,7 +67,7 @@ def get_descendent_models_from_db(main_ct: Model): def permissions_allowed_for_role(cls) -> dict[Union[Type[Model], Type[RemoteObject]], list[str]]: "Permission codenames valid for a RoleDefinition of given class, organized by permission class" if cls is None: - return permissions_allowed_for_system_role() + return permissions_allowed_for_system_role() # type: ignore[return-value] if not permission_registry.is_registered(cls): raise ValidationError(f'Django-ansible-base RBAC does not track permissions for model {cls._meta.model_name}') diff --git a/ansible_base/resource_registry/registry.py b/ansible_base/resource_registry/registry.py index 7a10369ef..7671b2d00 100644 --- a/ansible_base/resource_registry/registry.py +++ b/ansible_base/resource_registry/registry.py @@ -120,4 +120,4 @@ def get_registry() -> ResourceRegistry: return ResourceRegistry(resource_list, api_config()) else: - return False + return False # type: ignore[return-value] diff --git a/ansible_base/resource_registry/resource_server.py b/ansible_base/resource_registry/resource_server.py index 1094fcf42..62f2a8483 100644 --- a/ansible_base/resource_registry/resource_server.py +++ b/ansible_base/resource_registry/resource_server.py @@ -17,7 +17,7 @@ class ResourceServerConfig(TypedDict): def get_resource_server_config() -> ResourceServerConfig: defaults = {"JWT_ALGORITHM": "HS256", "VALIDATE_HTTPS": True} defaults.update(settings.RESOURCE_SERVER) - return defaults + return defaults # type: ignore[return-value] def get_service_token(user_id=None, expiration=60, **kwargs): diff --git a/ansible_base/resource_registry/service_client.py b/ansible_base/resource_registry/service_client.py index 03b77b77a..e814a82de 100644 --- a/ansible_base/resource_registry/service_client.py +++ b/ansible_base/resource_registry/service_client.py @@ -87,7 +87,7 @@ def jwt(self) -> str: """ if self._jwt is None or self._jwt_timeout is None or time.time() >= self._jwt_timeout: self.refresh_jwt() - return self._jwt + return self._jwt # type: ignore[return-value] @property def requests_auth_kwargs(self) -> dict: diff --git a/ansible_base/resource_registry/utils/resource_type_processor.py b/ansible_base/resource_registry/utils/resource_type_processor.py index 94b4f3801..1239776a0 100644 --- a/ansible_base/resource_registry/utils/resource_type_processor.py +++ b/ansible_base/resource_registry/utils/resource_type_processor.py @@ -66,4 +66,4 @@ def save(self, validated_data: Dict[str, Any], is_new: bool = False, skip_keys: if old_permissions != set(permissions): self.instance.permissions.set(permissions) changed = True - return (changed, self.instance) + return (changed, self.instance) # type: ignore[return-value] diff --git a/pyproject.toml b/pyproject.toml index 8f3105141..5eef433d5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -203,13 +203,14 @@ disallow_untyped_defs = false # Start permissive - tighten these as the codebase gets annotated disable_error_code = [ "attr-defined", "assignment", "union-attr", "var-annotated", "misc", - "has-type", "used-before-def", "return", "return-value", - "arg-type", "call-arg", "index", "operator", "valid-type", - "import-untyped", "str-bytes-safe", "syntax", - "dict-item", "name-defined", "django-manager-missing", + "arg-type", ] exclude = ["test_app/", "migrations/", "\\.tox/", "build/", "\\.venv/", "venv/"] +[[tool.mypy.overrides]] +module = "ansible_base.lib.dynamic_config.dynamic_settings" +disable_error_code = ["has-type", "used-before-def", "name-defined"] + [tool.django-stubs] django_settings_module = "test_app.settings" diff --git a/requirements/requirements_dev.txt b/requirements/requirements_dev.txt index a31db7387..32cde3c2f 100644 --- a/requirements/requirements_dev.txt +++ b/requirements/requirements_dev.txt @@ -15,6 +15,8 @@ tox-docker mypy django-stubs[compatible-mypy] djangorestframework-stubs[compatible-mypy] +types-requests +types-tabulate openapi-spec-validator>=0.6.1 # Adapted to changing import locations in jsonschema pytest pytest-asyncio From 6a541b30d9ead0e8fe9cb00aeb0d617782211053 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Thu, 19 Mar 2026 09:55:42 -0400 Subject: [PATCH 3/3] Review comments --- ansible_base/authentication/authenticator_plugins/ldap.py | 4 ++-- ansible_base/lib/redis/client.py | 2 +- ansible_base/resource_registry/resource_server.py | 5 +++++ 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/ansible_base/authentication/authenticator_plugins/ldap.py b/ansible_base/authentication/authenticator_plugins/ldap.py index 85d5bdadf..952153313 100644 --- a/ansible_base/authentication/authenticator_plugins/ldap.py +++ b/ansible_base/authentication/authenticator_plugins/ldap.py @@ -482,7 +482,7 @@ def __init__(self, database_instance=None, *args, **kwargs): self.configuration_encrypted_fields = ['BIND_PASSWORD'] self.set_logger(logger) - def authenticate(self, request, username=None, password=None, **kwargs) -> tuple[object, dict, list] | None: + def authenticate(self, request, username=None, password=None, **kwargs): if not username or not password: return None users_groups = [] @@ -563,7 +563,7 @@ def authenticate(self, request, username=None, password=None, **kwargs) -> tuple user_details={}, extra_data=user_from_ldap.ldap_user.attrs.data, ) - return update_user_claims(user_from_ldap, self.database_instance, users_groups) # type: ignore[return-value] + return update_user_claims(user_from_ldap, self.database_instance, users_groups) except Exception: logger.exception(f"Encountered an error authenticating to LDAP {self.database_instance.name}") return None diff --git a/ansible_base/lib/redis/client.py b/ansible_base/lib/redis/client.py index c9296d72f..16109b1e7 100644 --- a/ansible_base/lib/redis/client.py +++ b/ansible_base/lib/redis/client.py @@ -109,7 +109,7 @@ class RedisClientGetter: url: str connection_settings: dict mode: str - redis_hosts: str + redis_hosts: str | None def _redis_parse_url(self) -> None: if self.url == '': diff --git a/ansible_base/resource_registry/resource_server.py b/ansible_base/resource_registry/resource_server.py index 62f2a8483..0b3abfad8 100644 --- a/ansible_base/resource_registry/resource_server.py +++ b/ansible_base/resource_registry/resource_server.py @@ -17,6 +17,11 @@ class ResourceServerConfig(TypedDict): def get_resource_server_config() -> ResourceServerConfig: defaults = {"JWT_ALGORITHM": "HS256", "VALIDATE_HTTPS": True} defaults.update(settings.RESOURCE_SERVER) + + for key in ("URL", "SECRET_KEY"): + if key not in defaults: + raise KeyError(f"RESOURCE_SERVER setting is missing required key: {key}") + return defaults # type: ignore[return-value]