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..70f34798ff4 --- /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: + api_root = REWRITE_SLUG + else: + api_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"{api_root}{domain}/api/{version}/" + else: + path = f"{api_root}{DOMAIN_SLUG}/api/{version}/" + else: + path = f"{api_root}api/{version}/" + if lstrip: + return api_root.lstrip("/"), path.lstrip("/") + else: + return api_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..80abb018850 100644 --- a/pulpcore/app/templatetags/pulp_urls.py +++ b/pulpcore/app/templatetags/pulp_urls.py @@ -1,7 +1,8 @@ from django.template.defaultfilters import stringfilter, urlize as orig_urlize, register -from django.conf import settings from django.utils.safestring import SafeData, mark_safe +from pulpcore.app.find_url import find_api_root + @register.filter(needs_autoescape=True) @stringfilter @@ -11,13 +12,19 @@ def urlize(text, autoescape=True): This filter overwrites the django default implementation to also handle pulp api hrefs. """ + # urlize() will turn strings into links of they're of the form protocol://site/path. + # We force it to recognize Pulp-api-links by replacing strings in the incoming text of + # the form "API_ROOT/api/v3/whatever", forcing them to look like + # "http://SENTINEL.org/(string)", urlize()ing the result, then undoing the replace to + # lose the http://SENTINEL.org's. + _, current_path = find_api_root(set_domain=False, rewrite_header=False) safe_input = isinstance(text, SafeData) - text = text.replace(settings.V3_API_ROOT, "http://SENTINEL.org" + settings.V3_API_ROOT) + text = text.replace(current_path, "http://SENTINEL.org" + current_path) 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_path, current_path) if safe_input: text = mark_safe(text) return text diff --git a/pulpcore/app/urls.py b/pulpcore/app/urls.py index d21923ff2b4..193e3c68d8e 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,11 @@ 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_NOFRONT = find_api_root(lstrip=True, set_domain=True, rewrite_header=True) +_, PATH_NODOMAIN_NOREWRITE_NOFRONT = find_api_root( + lstrip=True, set_domain=False, rewrite_header=False +) +_, PATH_NODOMAIN_REWRITE_NOFRONT = find_api_root(lstrip=True, set_domain=False, rewrite_header=True) class ViewSetNode: @@ -203,7 +201,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_NOFRONT}docs/api.json?include_html=1&pk_path=1", ) ), name="schema-redoc", @@ -214,7 +212,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_NOFRONT}docs/api.json?include_html=1&pk_path=1", ) ), name="schema-swagger", @@ -222,9 +220,10 @@ class PulpDefaultRouter(routers.DefaultRouter): ] urlpatterns = [ - path(API_ROOT, include(special_views)), + path(PATH_DOMAIN_REWRITE_NOFRONT, 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_NOFRONT, include(docs_and_status)), ] if settings.DOMAIN_ENABLED: @@ -239,7 +238,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_NOFRONT, include(docs_and_status_no_schema))) if "social_django" in settings.INSTALLED_APPS: urlpatterns.append( @@ -251,7 +250,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_NOFRONT, 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..6e35f134d7a 100644 --- a/pulpcore/openapi/__init__.py +++ b/pulpcore/openapi/__init__.py @@ -32,16 +32,21 @@ 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_NOFRONT = 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_NOFRONT = FULL_API_PATH_NOFRONT.replace("slug:", "") if settings.API_ROOT_REWRITE_HEADER: - API_ROOT_NO_FRONT_SLASH = API_ROOT_NO_FRONT_SLASH.replace( + FULL_API_PATH_NOFRONT = FULL_API_PATH_NOFRONT.replace( "", settings.API_ROOT.strip("/") ) -API_ROOT_NO_FRONT_SLASH = API_ROOT_NO_FRONT_SLASH.replace("<", "{").replace(">", "}") + +# Final massage to make api-root "openapi compatible" +FULL_API_PATH_NOFRONT = FULL_API_PATH_NOFRONT.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 +66,7 @@ class PulpAutoSchema(AutoSchema): "patch": "partial_update", "delete": "delete", } - V3_API = API_ROOT_NO_FRONT_SLASH.replace("{pulp_domain}/", "") + V3_API = FULL_API_PATH_NOFRONT.replace("{pulp_domain}/", "") def _tokenize_path(self): """ @@ -118,7 +123,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..c3fb3cc3116 100644 --- a/pulpcore/tests/unit/test_pulp_urls.py +++ b/pulpcore/tests/unit/test_pulp_urls.py @@ -1,22 +1,15 @@ -import pytest import json from pulpcore.app.templatetags import pulp_urls - - -@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/" +from pulpcore.app.find_url import find_api_root def test_urlize_basic_url(): """ Text starts with V3_API_ROOT. Should be made clickable. """ - txt = "/baz/api/v3/foo/bar/" + _, curr_path = find_api_root(set_domain=False, rewrite_header=False) + txt = f"{curr_path}foo/bar/" ret = pulp_urls.urlize(txt) assert ret == f'{txt}' @@ -25,11 +18,12 @@ def test_urlize_quoted_href(): """ Text contains V3_API_ROOT in quotes. Should be made clickable. """ - txt = json.dumps({"pulp_href": "/baz/api/v3/foo/bar/"}) + _, curr_path = find_api_root(set_domain=False, rewrite_header=False) + txt = json.dumps({"pulp_href": f"{curr_path}foo/bar/"}) ret = pulp_urls.urlize(txt) assert ret == ( - '{"pulp_href": "/baz/api/v3/foo/bar/"}' + f'{{"pulp_href": "{curr_path}foo/bar/"}}' ) @@ -37,19 +31,20 @@ def test_urlize_mixed_bag(): """ Text contains lots of stuff in json. """ + _, curr_path = find_api_root(set_domain=False, rewrite_header=False) txt = json.dumps( { - "pulp_href": "/baz/api/v3/foo/bar/", - "related": "/baz/api/v3/other/", + "pulp_href": f"{curr_path}foo/bar/", + "related": f"{curr_path}other/", "description": "Test XSS .", } ) ret = pulp_urls.urlize(txt) assert ret == ( - '{"pulp_href": "/baz/api/v3/foo/bar/", ' - '"related": "/baz/api/v3/other/", ' + f'{{"pulp_href": "{curr_path}foo/bar/", ' + f'"related": "{curr_path}other/", ' ""description": " ""Test XSS <script>alert('ALERT')</script>."}" ) @@ -59,10 +54,12 @@ def test_urlize_quoted_href_no_autoescape(): """ Text contains V3_API_ROOT in quotes. Should be made clickable. """ - txt = json.dumps({"pulp_href": "/baz/api/v3/foo/bar/"}) + _, curr_path = find_api_root(set_domain=False, rewrite_header=False) + + txt = json.dumps({"pulp_href": f"{curr_path}foo/bar/"}) ret = pulp_urls.urlize(txt, autoescape=False) assert ret == ( - '{"pulp_href": "/baz/api/v3/foo/bar/"}' + f'{{"pulp_href": "{curr_path}foo/bar/"}}' ) @@ -70,9 +67,10 @@ def test_urlize_url_xss_autoescape(): """ Text starts with API_ROOT, includes XSS. Should be made clickable, escape XSS. """ - txt = "/baz/api/v3/foo/bar/blech/" + _, curr_path = find_api_root(set_domain=False, rewrite_header=False) + txt = f"{curr_path}foo/bar/blech/" escapified_linked_text = ( - '/baz/api/v3/foo/bar/' + f'{curr_path}foo/bar/' "<script>alert('ALERT!')</script>blech/" ) ret = pulp_urls.urlize(txt) @@ -83,9 +81,10 @@ def test_urlize_url_xss_no_autoescape(): """ Text starts with API_ROOT, includes XSS. Should be made clickable, not escape XSS. """ - txt = "/baz/api/v3/foo/bar/blech/" + _, curr_path = find_api_root(set_domain=False, rewrite_header=False) + txt = f"{curr_path}foo/bar/blech/" escapified_linked_text = ( - '/baz/api/v3/foo/bar/' + f'{curr_path}foo/bar/' "blech/" ) ret = pulp_urls.urlize(txt, autoescape=False) 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, )