Skip to content

Commit a7ec25a

Browse files
authored
AAP-53946 [devel-only] Forever ensure that ansible_base.lib.* is importable (ansible#844)
Makes static content in ansible_base.lib importable in absence of Django Fixes ansible#443 Additionally adds a check so that we keep this importable.
1 parent 829846e commit a7ec25a

File tree

8 files changed

+191
-14
lines changed

8 files changed

+191
-14
lines changed

.github/workflows/linting.yml

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ jobs:
2222
command: check_black
2323
- name: api-isort
2424
command: check_isort
25+
- name: pure-python-imports
26+
command: check_pure_python_imports
2527
steps:
2628
- name: Install make
2729
run: sudo apt install make
@@ -38,5 +40,18 @@ jobs:
3840
- name: Install requirments
3941
run: pip3.11 install -r requirements/requirements_dev.txt
4042

43+
- name: Install test dependencies for pure-python-imports
44+
if: matrix.tests.name == 'pure-python-imports'
45+
run: |
46+
pip3.11 install -r requirements/requirements.in
47+
pip3.11 install -r requirements/requirements_testing.txt
48+
pip3.11 install -r requirements/requirements_channels.in
49+
pip3.11 install -r requirements/requirements_redis_client.in
50+
51+
- name: Run pure-python-imports check
52+
if: matrix.tests.name == 'pure-python-imports'
53+
run: python tools/scripts/check_pure_python_imports.py
54+
4155
- name: Run check ${{ matrix.tests.name }}
56+
if: matrix.tests.name != 'pure-python-imports'
4257
run: make ${{ matrix.tests.command }}

Makefile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ check_flake8:
4949
check_isort:
5050
tox -e isort -- --check $(CHECK_SYNTAX_FILES)
5151

52+
5253
## Starts a postgres container in the background if one is not running
5354
# Options:
5455
# -d, --detatch: run the container in background

ansible_base/jwt_consumer/common/util.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
from cryptography.hazmat.primitives import hashes, serialization
88
from cryptography.hazmat.primitives.asymmetric import padding
99

10-
from ansible_base.jwt_consumer.common.cert import JWTCert, JWTCertException
1110
from ansible_base.lib.utils.settings import get_setting
1211

1312
logger = logging.getLogger('ansible_base.jwt_consumer.common.util')
@@ -32,6 +31,8 @@ def generate_x_trusted_proxy_header(key: str) -> str:
3231

3332

3433
def validate_x_trusted_proxy_header(header_value: str, ignore_cache=False) -> bool:
34+
from ansible_base.jwt_consumer.common.cert import JWTCert, JWTCertException
35+
3536
try:
3637
cert = JWTCert()
3738
cert.get_decryption_key(ignore_cache=ignore_cache)

ansible_base/lib/utils/create_system_user.py

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,22 @@
1-
import logging
2-
from typing import Optional, Tuple, Type
1+
from __future__ import annotations
32

4-
from django.core.exceptions import ImproperlyConfigured
5-
from django.db import models
6-
from django.utils.translation import gettext as _
3+
import logging
4+
from typing import TYPE_CHECKING, Optional, Tuple, Type
75

86
from ansible_base.lib.utils.settings import get_setting
97

8+
if TYPE_CHECKING:
9+
from django.db import models
10+
1011
logger = logging.getLogger('ansible_base.lib.utils.create_system_user')
1112

1213
"""
1314
These functions are in its own file because it is loaded during migrations so it has no access to models.
1415
"""
1516

1617

17-
def create_system_user(user_model: Type[models.Model]) -> models.Model: # Note: We can't load models here so we can typecast to anything better than Model
18+
def create_system_user(user_model: Type[models.Model]) -> models.Model:
19+
# Note: We can't load models here so we can typecast to anything better than Model
1820
from ansible_base.lib.abstract_models.user import AbstractDABUser
1921

2022
#
@@ -63,4 +65,8 @@ def get_system_username() -> Tuple[Optional[str], str]:
6365
return str(value), setting_name
6466

6567
logger.error(f"Expected get_setting to return a string for {setting_name}, got {type(value)}")
68+
69+
from django.core.exceptions import ImproperlyConfigured
70+
from django.utils.translation import gettext as _
71+
6672
raise ImproperlyConfigured(_("Setting %(setting_name)s needs to be a string not a %(type)s") % {'setting_name': setting_name, 'type': type(value)})

ansible_base/lib/utils/requests.py

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
1-
import logging
2-
from typing import Optional
1+
from __future__ import annotations
32

4-
from crum import get_current_request
5-
from django.http import HttpRequest
3+
import logging
4+
from typing import TYPE_CHECKING, Optional
65

7-
from ansible_base.jwt_consumer.common.util import validate_x_trusted_proxy_header
86
from ansible_base.lib.utils.settings import get_setting
97

8+
if TYPE_CHECKING:
9+
from django.http import HttpRequest
10+
1011
logger = logging.getLogger('ansible_base.lib.uitls.requests')
1112

1213

@@ -43,6 +44,8 @@ def get_remote_hosts(request: HttpRequest, get_first_only: bool = False) -> list
4344
# If we are connected to from a trusted proxy then we can add some additional headers
4445
try:
4546
if 'HTTP_X_TRUSTED_PROXY' in request.META:
47+
from ansible_base.jwt_consumer.common.util import validate_x_trusted_proxy_header
48+
4649
if validate_x_trusted_proxy_header(request.META['HTTP_X_TRUSTED_PROXY']):
4750
# The last entry in x-forwarded-for from envoy can be trusted implicitly
4851
# https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_conn_man/headers#x-forwarded-for
@@ -68,10 +71,14 @@ def get_remote_hosts(request: HttpRequest, get_first_only: bool = False) -> list
6871
def is_proxied_request(request: Optional[HttpRequest] = None) -> bool:
6972
"Return true if request claims to be from a proxy and the header validates as such."
7073
if request is None:
74+
from crum import get_current_request
75+
7176
request = get_current_request()
7277
if request is None:
7378
# e.g. being called by CLI or something
7479
return False
7580
if x_trusted_proxy := request.META.get("HTTP_X_TRUSTED_PROXY"):
81+
from ansible_base.jwt_consumer.common.util import validate_x_trusted_proxy_header
82+
7683
return validate_x_trusted_proxy_header(x_trusted_proxy)
7784
return False

test_app/tests/jwt_consumer/common/test_util.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ def test_validate_trusted_proxy_header_bad_cached_key_but_correct_setting(self,
1313
{"key": rsa_keypair.public, "cached": False},
1414
]
1515
with override_settings(ANSIBLE_BASE_JWT_KEY=rsa_keypair.public):
16-
with mock.patch("ansible_base.jwt_consumer.common.util.JWTCert.get_decryption_key", create_mock_method(field_dicts)):
16+
with mock.patch("ansible_base.jwt_consumer.common.cert.JWTCert.get_decryption_key", create_mock_method(field_dicts)):
1717
assert validate_x_trusted_proxy_header(generate_x_trusted_proxy_header(rsa_keypair.private))
1818

1919
def test_validate_trusted_proxy_header_no_key(self, caplog):

test_app/tests/lib/utils/test_requests.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ def test_get_remote_hosts_alternate_headers_behind_trusted_proxy(rsa_keypair):
103103
assert remote_hosts == ['5.6.7.8', '9.10.11.12', '5.6.7.8', 'a.b.c.d']
104104

105105

106-
@mock.patch("ansible_base.lib.utils.requests.validate_x_trusted_proxy_header", side_effect=Exception)
106+
@mock.patch("ansible_base.jwt_consumer.common.util.validate_x_trusted_proxy_header", side_effect=Exception)
107107
def test_get_remote_hosts_validate_trusted_proxy_header_exception(mock_validate: mock.MagicMock, rsa_keypair, caplog):
108108
with override_settings(ANSIBLE_BASE_JWT_KEY=rsa_keypair.public):
109109
headers = {
Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
#!/usr/bin/env python3
2+
# Generated by Claude Code (Sonnet 4)
3+
4+
"""
5+
Script to test all modules in ansible_base/lib for pure Python compatibility.
6+
This script ensures that modules in the lib folder can be imported without
7+
requiring Django apps to be initialized.
8+
"""
9+
10+
import os
11+
import subprocess
12+
import sys
13+
from pathlib import Path
14+
15+
16+
def find_python_modules(lib_path):
17+
"""Find all Python modules in the lib directory."""
18+
modules = []
19+
lib_path = Path(lib_path)
20+
21+
for py_file in lib_path.rglob("*.py"):
22+
if py_file.name == "__init__.py":
23+
# Convert path to module name (including ansible_base prefix)
24+
relative_path = py_file.parent.relative_to(lib_path.parent.parent)
25+
module_name = str(relative_path).replace(os.sep, ".")
26+
modules.append(module_name)
27+
else:
28+
# Convert path to module name (including ansible_base prefix)
29+
relative_path = py_file.relative_to(lib_path.parent.parent)
30+
module_name = str(relative_path)[:-3].replace(os.sep, ".") # Remove .py extension
31+
modules.append(module_name)
32+
33+
return sorted(modules)
34+
35+
36+
def test_pure_python_import(module_name, base_path):
37+
"""Test if a module can be imported with pure Python (no Django setup)."""
38+
test_script = f'''
39+
import sys
40+
sys.path.insert(0, "{base_path}")
41+
42+
try:
43+
import {module_name}
44+
print("SUCCESS")
45+
except Exception as e:
46+
print(f"ERROR: {{e}}")
47+
'''
48+
49+
# Run in a clean environment without Django setup
50+
env = os.environ.copy()
51+
if 'DJANGO_SETTINGS_MODULE' in env:
52+
del env['DJANGO_SETTINGS_MODULE']
53+
54+
try:
55+
result = subprocess.run(['python', '-c', test_script], capture_output=True, text=True, env=env, timeout=30)
56+
57+
if result.returncode == 0 and "SUCCESS" in result.stdout:
58+
return True, None
59+
else:
60+
error_msg = result.stderr.strip() or result.stdout.strip()
61+
return False, error_msg
62+
except subprocess.TimeoutExpired:
63+
return False, "Import test timed out"
64+
except Exception as e:
65+
return False, f"Failed to run test: {e}"
66+
67+
68+
def main():
69+
"""Main function to test all modules in ansible_base/lib."""
70+
script_dir = Path(__file__).parent.parent.parent # Go up two levels since we're in tools/scripts/
71+
lib_path = script_dir / "ansible_base" / "lib"
72+
73+
if not lib_path.exists():
74+
print(f"ERROR: {lib_path} does not exist")
75+
sys.exit(1)
76+
77+
print("Testing pure Python import compatibility for ansible_base/lib modules...")
78+
print("=" * 70)
79+
80+
modules = find_python_modules(lib_path)
81+
82+
# Modules that are allowed to fail because they inherently require Django
83+
allowed_failures = {
84+
# Abstract models inherently require Django to be initialized
85+
'ansible_base.lib.abstract_models',
86+
'ansible_base.lib.abstract_models.common',
87+
'ansible_base.lib.abstract_models.immutable',
88+
'ansible_base.lib.abstract_models.organization',
89+
'ansible_base.lib.abstract_models.team',
90+
'ansible_base.lib.abstract_models.user',
91+
# View classes require Django REST framework
92+
'ansible_base.lib.utils.views.ansible_base',
93+
'ansible_base.lib.utils.views.django_app_api',
94+
'ansible_base.lib.utils.views.permissions',
95+
'ansible_base.lib.utils.views.urls',
96+
# Router classes require Django REST framework
97+
'ansible_base.lib.routers',
98+
'ansible_base.lib.routers.association_resource_router',
99+
# These modules require Django settings to be configured at import time
100+
'ansible_base.lib.backends.prefixed_user_auth',
101+
'ansible_base.lib.dynamic_config.dynamic_urls',
102+
'ansible_base.lib.serializers.common',
103+
'ansible_base.lib.testing.fixtures',
104+
'ansible_base.lib.testing.util',
105+
'ansible_base.lib.utils.auth',
106+
}
107+
108+
failed_modules = []
109+
passed_modules = []
110+
expected_failures = []
111+
112+
for module_name in modules:
113+
print(f"Testing {module_name}... ", end="", flush=True)
114+
success, error = test_pure_python_import(module_name, str(script_dir))
115+
116+
if success:
117+
print("✓ PASS")
118+
passed_modules.append(module_name)
119+
else:
120+
if module_name in allowed_failures:
121+
print("✗ EXPECTED FAIL")
122+
expected_failures.append((module_name, error))
123+
else:
124+
print("✗ UNEXPECTED FAIL")
125+
print(f" Error: {error}")
126+
failed_modules.append((module_name, error))
127+
128+
print("\n" + "=" * 70)
129+
print(f"Results: {len(passed_modules)} passed, {len(failed_modules)} unexpected failures, {len(expected_failures)} expected failures")
130+
131+
if failed_modules:
132+
print("\nUnexpected failures:")
133+
for module_name, error in failed_modules:
134+
print(f" - {module_name}: {error}")
135+
136+
print("\nThese modules should be importable with pure Python.")
137+
print("Consider moving Django-specific imports inline within functions.")
138+
sys.exit(1)
139+
else:
140+
print("\nAll importable modules passed! ✓")
141+
if expected_failures:
142+
print(f"({len(expected_failures)} modules have expected failures due to Django dependencies)")
143+
sys.exit(0)
144+
145+
146+
if __name__ == "__main__":
147+
main()

0 commit comments

Comments
 (0)