Added find_api_root() to standardize URL building-blocks.

CHANGES/+api_root_changes.feature

@@ -0,0 +1 @@
+Added `pulpcore.app.find_url.find_api_root()` to standardize Pulp url-creation.

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("<slug:pulp_domain>", "default")
-)
-
+_, V3_API_ROOT = find_api_root(domain="default")
 
 CHECKSUM_LEN = {
     "md5": 32,

pulpcore/app/find_url.py

@@ -0,0 +1,49 @@
+from django.conf import settings
+
+DOMAIN_SLUG = "<slug:pulp_domain>"
+REWRITE_SLUG = "/<path:api_root>/"
+
+
+# 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, <root>/api/<version>)
+
+    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/<slug:pulp_domain>/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: ("/<path:api_root>/", "/<path:api_root>/api/v3/")
+        find_api_root(version="v4", domain="foo", lstrip=True), API_ROOT_REWRITE_HEADER :
+            ("<path:api_root>/", "<path:api_root>/default/api/v4/")
+    Returns:
+        (str, str) : (API_ROOT (possibly re-written), API_ROOT/api/<version>/
+                     (with <domain> 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

pulpcore/app/settings.py

@@ -630,6 +630,9 @@ def otel_middleware_hook(settings):
     api_root = "/<path: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 + "<slug:pulp_domain>/api/v3/")
 settings.set("V3_API_ROOT_NO_FRONT_SLASH", settings.V3_API_ROOT.lstrip("/"))

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

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("/<path:api_root>/", 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,17 +210,18 @@ 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",
     ),
 ]
 
 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:

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:

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(
-        "<path:api_root>", 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("<path:api_root>", 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]

pulpcore/plugin/find_url.py

@@ -0,0 +1,3 @@
+from pulpcore.app.find_url import find_api_root
+
+__all__ = ["find_api_root"]

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("<slug:pulp_domain>", "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("<path:api_root>", 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")

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

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("<slug:pulp_domain>", "default")
-)
-if settings.API_ROOT_REWRITE_HEADER:
-    API_ROOT = API_ROOT.replace("<path:api_root>", 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,
             )