From 8b97f42e459d199702bbbf02c5ecb62ff1894af5 Mon Sep 17 00:00:00 2001 From: Grant Gainey Date: Sat, 28 Mar 2026 13:51:21 -0400 Subject: [PATCH] Added find_api_root() to standardize URL building-blocks. --- CHANGES/+api_root_changes.feature | 1 + pulp_file/tests/unit/test_serializers.py | 8 +--- pulpcore/app/find_url.py | 49 ++++++++++++++++++++++++ pulpcore/app/settings.py | 3 ++ pulpcore/app/templatetags/pulp_urls.py | 7 ++-- pulpcore/app/urls.py | 25 ++++++------ pulpcore/app/util.py | 5 +-- pulpcore/openapi/__init__.py | 21 +++++----- pulpcore/plugin/find_url.py | 3 ++ pulpcore/pytest_plugin.py | 24 ++++++------ pulpcore/tests/unit/test_pulp_urls.py | 4 +- pulpcore/tests/unit/test_viewsets.py | 21 +++------- 12 files changed, 106 insertions(+), 65 deletions(-) create mode 100644 CHANGES/+api_root_changes.feature create mode 100644 pulpcore/app/find_url.py create mode 100644 pulpcore/plugin/find_url.py diff --git a/CHANGES/+api_root_changes.feature b/CHANGES/+api_root_changes.feature new file mode 100644 index 00000000000..cb8416841fd --- /dev/null +++ b/CHANGES/+api_root_changes.feature @@ -0,0 +1 @@ +Added `pulpcore.app.find_url.find_api_root()` to standardize Pulp url-creation. diff --git a/pulp_file/tests/unit/test_serializers.py b/pulp_file/tests/unit/test_serializers.py index b57d6410d51..aecbdcde533 100644 --- a/pulp_file/tests/unit/test_serializers.py +++ b/pulp_file/tests/unit/test_serializers.py @@ -6,13 +6,9 @@ from pulp_file.app.models import FileContent from pulpcore.plugin.models import Artifact +from pulpcore.plugin.find_url import find_api_root -V3_API_ROOT = ( - settings.V3_API_ROOT - if not settings.DOMAIN_ENABLED - else settings.V3_DOMAIN_API_ROOT.replace("", "default") -) - +_, V3_API_ROOT = find_api_root(domain="default") CHECKSUM_LEN = { "md5": 32, diff --git a/pulpcore/app/find_url.py b/pulpcore/app/find_url.py new file mode 100644 index 00000000000..4c0dfb3cb39 --- /dev/null +++ b/pulpcore/app/find_url.py @@ -0,0 +1,49 @@ +from django.conf import settings + +DOMAIN_SLUG = "" +REWRITE_SLUG = "//" + + +# We isolate this utility-function to avoid pulling in random "Django App must be configured" +# code segments from the rest of the .util methods/imports +def find_api_root(version="v3", set_domain=True, domain=None, lstrip=False, rewrite_header=True): + """ + Returns the tuple (api-root, /api/) + + Args: + version (str): API-version desired + set_domain (bool): Should the domain-slug be included if DOMAIN_ENABLED? + domain (str): Domain-name to replace DOMAIN_SLUG + lstrip (bool): Should the full version have it's leading-/ stripped + rewrite_header (bool): Should API_ROOT_REWRITE_HEADER be honored or not + + Examples: + find_api_root() : ("/pulp/", "/pulp/api/v3/") + find_api_root(), DOMAIN_ENABLED: ("/pulp/", "/pulp//api/v3/") + find_api_root(domain="default"), DOMAIN_ENABLED : ("/pulp/", "/pulp/default/api/v3/") + find_api_root(lstrip=True) : ("pulp/", "pulp/api/v3/") + find_api_root(), API_ROOT_REWRITE_HEADER: ("//", "//api/v3/") + find_api_root(version="v4", domain="foo", lstrip=True), API_ROOT_REWRITE_HEADER : + ("/", "/default/api/v4/") + Returns: + (str, str) : (API_ROOT (possibly re-written), API_ROOT/api// + (with if enabled)) + """ + # Some current path-building wants to ignore REWRITE - make that possible + if rewrite_header and settings.API_ROOT_REWRITE_HEADER: + root = REWRITE_SLUG + else: + root = settings.API_ROOT + + # Some current path-building wants to ignore DOMAIN - make that possible + if set_domain and settings.DOMAIN_ENABLED: + if domain: + path = f"{root}{domain}/api/{version}/" + else: + path = f"{root}{DOMAIN_SLUG}/api/{version}/" + else: + path = f"{root}api/{version}/" + if lstrip: + return root.lstrip("/"), path.lstrip("/") + else: + return root, path diff --git a/pulpcore/app/settings.py b/pulpcore/app/settings.py index 0791d75f288..6e85b1dac70 100644 --- a/pulpcore/app/settings.py +++ b/pulpcore/app/settings.py @@ -630,6 +630,9 @@ def otel_middleware_hook(settings): api_root = "//" else: api_root = settings.API_ROOT +# protocol://host:port/{API_ROOT}{domain}/api/{version}/ +# All of the below are DEPRECATED, and should be replaced by calling +# pulpcore.plugin.find_url.find_api_root() (q.v.) settings.set("V3_API_ROOT", api_root + "api/v3/") # Not user configurable settings.set("V3_DOMAIN_API_ROOT", api_root + "/api/v3/") settings.set("V3_API_ROOT_NO_FRONT_SLASH", settings.V3_API_ROOT.lstrip("/")) diff --git a/pulpcore/app/templatetags/pulp_urls.py b/pulpcore/app/templatetags/pulp_urls.py index ed4a7279334..c545293ad9d 100644 --- a/pulpcore/app/templatetags/pulp_urls.py +++ b/pulpcore/app/templatetags/pulp_urls.py @@ -1,5 +1,5 @@ -from django.template.defaultfilters import stringfilter, urlize as orig_urlize, register from django.conf import settings +from django.template.defaultfilters import stringfilter, urlize as orig_urlize, register from django.utils.safestring import SafeData, mark_safe @@ -11,13 +11,14 @@ def urlize(text, autoescape=True): This filter overwrites the django default implementation to also handle pulp api hrefs. """ + current_root = settings.API_ROOT + "api/" safe_input = isinstance(text, SafeData) - text = text.replace(settings.V3_API_ROOT, "http://SENTINEL.org" + settings.V3_API_ROOT) + text = text.replace(current_root, "http://SENTINEL.org" + current_root) if safe_input: text = mark_safe(text) text = orig_urlize(text, autoescape=autoescape) safe_input = isinstance(text, SafeData) - text = text.replace("http://SENTINEL.org" + settings.V3_API_ROOT, settings.V3_API_ROOT) + text = text.replace("http://SENTINEL.org" + current_root, current_root) if safe_input: text = mark_safe(text) return text diff --git a/pulpcore/app/urls.py b/pulpcore/app/urls.py index d21923ff2b4..ff2b4448f39 100644 --- a/pulpcore/app/urls.py +++ b/pulpcore/app/urls.py @@ -14,6 +14,7 @@ from rest_framework.routers import APIRootView from pulpcore.app.apps import pulp_plugin_configs +from pulpcore.plugin.find_url import find_api_root from pulpcore.app.views import ( LivezView, OrphansView, @@ -30,14 +31,9 @@ ReclaimSpaceViewSet, ) -if settings.DOMAIN_ENABLED: - API_ROOT = settings.V3_DOMAIN_API_ROOT_NO_FRONT_SLASH -else: - API_ROOT = settings.V3_API_ROOT_NO_FRONT_SLASH -if settings.API_ROOT_REWRITE_HEADER: - V3_API_ROOT = settings.V3_API_ROOT.replace("//", settings.API_ROOT) -else: - V3_API_ROOT = settings.V3_API_ROOT +_, PATH_DOMAIN_REWRITE_NFS = find_api_root(lstrip=True, set_domain=True, rewrite_header=True) +_, PATH_NODOMAIN_NOREWRITE_NFS = find_api_root(lstrip=True, set_domain=False, rewrite_header=False) +_, PATH_NODOMAIN_REWRITE_NFS = find_api_root(lstrip=True, set_domain=False, rewrite_header=True) class ViewSetNode: @@ -203,7 +199,7 @@ class PulpDefaultRouter(routers.DefaultRouter): SpectacularRedocView.as_view( authentication_classes=[], permission_classes=[], - url=f"{V3_API_ROOT}docs/api.json?include_html=1&pk_path=1", + url=f"/{PATH_NODOMAIN_NOREWRITE_NFS}docs/api.json?include_html=1&pk_path=1", ) ), name="schema-redoc", @@ -214,7 +210,7 @@ class PulpDefaultRouter(routers.DefaultRouter): SpectacularSwaggerView.as_view( authentication_classes=[], permission_classes=[], - url=f"{V3_API_ROOT}docs/api.json?include_html=1&pk_path=1", + url=f"/{PATH_NODOMAIN_NOREWRITE_NFS}docs/api.json?include_html=1&pk_path=1", ) ), name="schema-swagger", @@ -222,9 +218,10 @@ class PulpDefaultRouter(routers.DefaultRouter): ] urlpatterns = [ - path(API_ROOT, include(special_views)), + path(PATH_DOMAIN_REWRITE_NFS, include(special_views)), path("auth/", include("rest_framework.urls")), - path(settings.V3_API_ROOT_NO_FRONT_SLASH, include(docs_and_status)), + # docs/status aren't "inside" a domain + path(PATH_NODOMAIN_REWRITE_NFS, include(docs_and_status)), ] if settings.DOMAIN_ENABLED: @@ -239,7 +236,7 @@ class NoSchema(p.callback.cls): view = NoSchema.as_view(**p.callback.initkwargs) name = p.name + "-domains" if p.name else None docs_and_status_no_schema.append(path(str(p.pattern), view, name=name)) - urlpatterns.insert(-1, path(API_ROOT, include(docs_and_status_no_schema))) + urlpatterns.insert(-1, path(PATH_DOMAIN_REWRITE_NFS, include(docs_and_status_no_schema))) if "social_django" in settings.INSTALLED_APPS: urlpatterns.append( @@ -251,7 +248,7 @@ class NoSchema(p.callback.cls): all_routers = [root_router] + vs_tree.register_with(root_router) for router in all_routers: - urlpatterns.append(path(API_ROOT, include(router.urls))) + urlpatterns.append(path(PATH_DOMAIN_REWRITE_NFS, include(router.urls))) # If plugins define a urls.py, include them into the root namespace. for plugin_pattern in plugin_patterns: diff --git a/pulpcore/app/util.py b/pulpcore/app/util.py index ed09256b1c3..2d7ddbad95d 100644 --- a/pulpcore/app/util.py +++ b/pulpcore/app/util.py @@ -31,7 +31,6 @@ # a little cache so viewset_for_model doesn't have to iterate over every app every time _model_viewset_cache = {} -STRIPPED_API_ROOT = settings.API_ROOT.strip("/") def reverse(viewname, args=None, kwargs=None, request=None, relative_url=True, **extra): @@ -45,7 +44,7 @@ def reverse(viewname, args=None, kwargs=None, request=None, relative_url=True, * if settings.DOMAIN_ENABLED: kwargs.setdefault("pulp_domain", get_domain().name) if settings.API_ROOT_REWRITE_HEADER: - kwargs.setdefault("api_root", getattr(request, "api_root", STRIPPED_API_ROOT)) + kwargs.setdefault("api_root", getattr(request, "api_root", settings.API_ROOT.strip("/"))) if relative_url: request = None return drf_reverse(viewname, args=args, kwargs=kwargs, request=request, **extra) @@ -79,7 +78,6 @@ def get_url(model, domain=None, request=None): kwargs["pk"] = model.pk else: view_action = "list" - return reverse(get_view_name_for_model(model, view_action), kwargs=kwargs, request=request) @@ -299,7 +297,6 @@ def get_view_name_for_model(model_obj, view_action): if isinstance(model_obj, models.MasterModel): model_obj = model_obj.cast() viewset = get_viewset_for_model(model_obj) - # return the complete view name, joining the registered viewset base name with # the requested view method. for router in all_routers: diff --git a/pulpcore/openapi/__init__.py b/pulpcore/openapi/__init__.py index 7ab0f17d021..2c7eefb2cb9 100644 --- a/pulpcore/openapi/__init__.py +++ b/pulpcore/openapi/__init__.py @@ -32,16 +32,19 @@ from pulpcore.app.apps import pulp_plugin_configs from pulpcore.app.loggers import deprecation_logger +from pulpcore.plugin.find_url import find_api_root +# Get the API-ROOT for this installation +_unused, FULL_API_PATH_NFS = find_api_root(lstrip=True) + +# Massage some api-affecting vars to "genericize" them for the spec if settings.DOMAIN_ENABLED: - API_ROOT_NO_FRONT_SLASH = settings.V3_DOMAIN_API_ROOT_NO_FRONT_SLASH.replace("slug:", "") -else: - API_ROOT_NO_FRONT_SLASH = settings.V3_API_ROOT_NO_FRONT_SLASH + FULL_API_PATH_NFS = FULL_API_PATH_NFS.replace("slug:", "") if settings.API_ROOT_REWRITE_HEADER: - API_ROOT_NO_FRONT_SLASH = API_ROOT_NO_FRONT_SLASH.replace( - "", settings.API_ROOT.strip("/") - ) -API_ROOT_NO_FRONT_SLASH = API_ROOT_NO_FRONT_SLASH.replace("<", "{").replace(">", "}") + FULL_API_PATH_NFS = FULL_API_PATH_NFS.replace("", settings.API_ROOT.strip("/")) + +# Final massage to make api-root "openapi compatible" +FULL_API_PATH_NFS = FULL_API_PATH_NFS.replace("<", "{").replace(">", "}") # Python does not distinguish integer sizes. The safest assumption is that they are large. extend_schema_field(OpenApiTypes.INT64)(serializers.IntegerField) @@ -61,7 +64,7 @@ class PulpAutoSchema(AutoSchema): "patch": "partial_update", "delete": "delete", } - V3_API = API_ROOT_NO_FRONT_SLASH.replace("{pulp_domain}/", "") + V3_API = FULL_API_PATH_NFS.replace("{pulp_domain}/", "") def _tokenize_path(self): """ @@ -118,7 +121,7 @@ class MyViewSet(ViewSet): tokenized_path = self._tokenize_path() subpath = "/".join(tokenized_path) - operation_keys = subpath.replace(settings.V3_API_ROOT_NO_FRONT_SLASH, "").split("/") + operation_keys = subpath.replace(self.V3_API, "").split("/") operation_keys = [i.title() for i in operation_keys] if len(operation_keys) > 2: del operation_keys[1] diff --git a/pulpcore/plugin/find_url.py b/pulpcore/plugin/find_url.py new file mode 100644 index 00000000000..c968c2f8e0b --- /dev/null +++ b/pulpcore/plugin/find_url.py @@ -0,0 +1,3 @@ +from pulpcore.app.find_url import find_api_root + +__all__ = ["find_api_root"] diff --git a/pulpcore/pytest_plugin.py b/pulpcore/pytest_plugin.py index fd7a19134af..4eb9abbcdd2 100644 --- a/pulpcore/pytest_plugin.py +++ b/pulpcore/pytest_plugin.py @@ -30,6 +30,8 @@ add_recording_route, ) +from pulpcore.plugin.find_url import find_api_root + try: import pulp_smash # noqa: F401 except ImportError: @@ -629,7 +631,7 @@ def _settings_factory(storage_class=None, storage_settings=None): ] def get_installation_storage_option(key, backend): - value = pulp_settings.STORAGES["default"]["OPTIONS"].get(key) + value = pulp_settings.STORAGES["default"].get("OPTIONS", {}).get(key) # Some FileSystem backend options may be defined in the top settings module if backend == "pulpcore.app.models.storage.FileSystem" and not value: value = getattr(pulp_settings, key, None) @@ -787,18 +789,18 @@ def pulp_content_origin_with_prefix(pulp_settings, bindings_cfg): @pytest.fixture(scope="session") def pulp_api_v3_path(pulp_settings, pulp_domain_enabled): + root, path = find_api_root(set_domain=True, rewrite_header=False, domain="default") + + # Check that find_api_root() is doing what the tests expect if pulp_domain_enabled: - v3_api_root = pulp_settings.V3_DOMAIN_API_ROOT - v3_api_root = v3_api_root.replace("", "default") + v3_api_root = f"{pulp_settings.API_ROOT}default/api/v3/" else: - v3_api_root = pulp_settings.V3_API_ROOT - if v3_api_root is None: - raise RuntimeError( - "This fixture requires the server to have the `V3_API_ROOT` setting set." - ) - if pulp_settings.API_ROOT_REWRITE_HEADER: - v3_api_root = v3_api_root.replace("", pulp_settings.API_ROOT.strip("/")) - return v3_api_root + v3_api_root = f"{pulp_settings.API_ROOT}api/v3/" + assert path == v3_api_root + + if path is None: + raise RuntimeError("This fixture requires the server to have the `API_ROOT` setting set.") + return path @pytest.fixture(scope="session") diff --git a/pulpcore/tests/unit/test_pulp_urls.py b/pulpcore/tests/unit/test_pulp_urls.py index 29e21aea2ef..f706544826b 100644 --- a/pulpcore/tests/unit/test_pulp_urls.py +++ b/pulpcore/tests/unit/test_pulp_urls.py @@ -7,9 +7,7 @@ @pytest.fixture(autouse=True) def api_root(settings): settings.DOMAIN_ENABLED = True - # TODO: dynaconf lazy settings... - # settings.API_ROOT = "/baz/" - settings.V3_API_ROOT = "/baz/api/v3/" + settings.API_ROOT = "/baz/" def test_urlize_basic_url(): diff --git a/pulpcore/tests/unit/test_viewsets.py b/pulpcore/tests/unit/test_viewsets.py index 26bac2f0e0c..6f1f10a6531 100644 --- a/pulpcore/tests/unit/test_viewsets.py +++ b/pulpcore/tests/unit/test_viewsets.py @@ -1,18 +1,11 @@ from uuid import uuid4 -from django.conf import settings from django.test import TestCase from rest_framework.serializers import ValidationError as DRFValidationError - +from pulpcore.plugin.find_url import find_api_root from pulp_file.app import models, viewsets -API_ROOT = ( - settings.V3_API_ROOT - if not settings.DOMAIN_ENABLED - else settings.V3_DOMAIN_API_ROOT.replace("", "default") -) -if settings.API_ROOT_REWRITE_HEADER: - API_ROOT = API_ROOT.replace("", settings.API_ROOT.strip("/")) +_, V3_PATH = find_api_root(domain="default", rewrite_header=False, lstrip=False) class TestGetResource(TestCase): @@ -30,7 +23,7 @@ def test_no_errors(self): repo = models.FileRepository.objects.create(name="foo") viewset = viewsets.FileRepositoryViewSet() resource = viewset.get_resource( - "{api_root}repositories/file/file/{pk}/".format(api_root=API_ROOT, pk=repo.pk), + "{path}repositories/file/file/{pk}/".format(path=V3_PATH, pk=repo.pk), models.FileRepository, ) assert repo == resource @@ -46,7 +39,7 @@ def test_multiple_matches(self): with self.assertRaises(DRFValidationError): # matches all repositories viewset.get_resource( - "{api_root}repositories/file/file/".format(api_root=API_ROOT), + "{path}repositories/file/file/".format(path=V3_PATH), models.FileRepository, ) @@ -69,7 +62,7 @@ def test_resource_does_not_exist(self): with self.assertRaises(DRFValidationError): viewset.get_resource( - "{api_root}repositories/file/file/{pk}/".format(api_root=API_ROOT, pk=pk), + "{path}repositories/file/file/{pk}/".format(path=V3_PATH, pk=pk), models.FileRepository, ) @@ -84,8 +77,6 @@ def test_resource_with_field_error(self): with self.assertRaises(DRFValidationError): # has no repo versions yet viewset.get_resource( - "{api_root}repositories/file/file/{pk}/versions/1/".format( - api_root=API_ROOT, pk=repo.pk - ), + "{path}repositories/file/file/{pk}/versions/1/".format(path=V3_PATH, pk=repo.pk), models.FileRepository, )