Skip to content

Commit

Permalink
rework url routing
Browse files Browse the repository at this point in the history
[noissue]
  • Loading branch information
gerrod3 committed Apr 4, 2024
1 parent 0bf2deb commit a2745ad
Show file tree
Hide file tree
Showing 17 changed files with 104 additions and 86 deletions.
3 changes: 1 addition & 2 deletions pulpcore/app/models/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,9 @@
batch_qs,
get_url,
get_view_name_for_model,
get_domain,
get_domain_pk,
cache_key,
reverse
reverse,
)
from pulpcore.constants import ALL_KNOWN_CONTENT_CHECKSUMS, PROTECTED_REPO_VERSION_MESSAGE
from pulpcore.download.factory import DownloaderFactory
Expand Down
12 changes: 0 additions & 12 deletions pulpcore/app/path_api_urls.py

This file was deleted.

22 changes: 18 additions & 4 deletions pulpcore/app/serializers/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,29 @@
# Field mixins


def _reverse(obj):
"""Include domain-path in reverse call if DOMAIN_ENABLED."""

if settings.DOMAIN_ENABLED:

@functools.wraps(reverse)
def _patched_reverse(viewname, request=None, args=None, kwargs=None, **extra):
kwargs = kwargs or {}
domain_name = obj.pulp_domain.name if hasattr(obj, "pulp_domain") else "default"
kwargs["pulp_domain"] = domain_name
return reverse(viewname, request=request, args=args, kwargs=kwargs, **extra)

return _patched_reverse

return reverse


class HrefFieldMixin:
"""A mixin to configure related fields to generate relative hrefs."""

def get_url(self, obj, view_name, request, *args, **kwargs):
# Use the Pulp reverse method to display relative hrefs.
self.reverse = reverse
if settings.DOMAIN_ENABLED:
domain_name = obj.pulp_domain.name if hasattr(obj, "pulp_domain") else "default"
kwargs["pulp_domain"] = domain_name
self.reverse = _reverse(obj)
return super().get_url(obj, view_name, request, *args, **kwargs)


Expand Down
3 changes: 2 additions & 1 deletion pulpcore/app/serializers/repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,8 @@ def to_representation(self, obj):
for count_detail in obj.counts.all():
count_type = count_detail.get_count_type_display()
item_dict = {
"count": count_detail.count, "href": count_detail.get_content_href(request=request)
"count": count_detail.count,
"href": count_detail.get_content_href(request=request),
}
to_return[count_type][count_detail.content_type] = item_dict

Expand Down
1 change: 0 additions & 1 deletion pulpcore/app/serializers/task.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from gettext import gettext as _

from django.conf import settings
from rest_framework import serializers

from pulpcore.app import models
Expand Down
12 changes: 8 additions & 4 deletions pulpcore/app/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@

# API Root
API_ROOT = "/pulp/"
API_ROOT_REWRITE_HEADER = 'X-API-Root'
API_ROOT_REWRITE_HEADER = None

# Application definition

Expand Down Expand Up @@ -419,7 +419,7 @@
ENVVAR_FOR_DYNACONF="PULP_SETTINGS",
load_dotenv=False,
validators=[
# api_root_validator, #running into a bug with this when running tests, ask bruno/pedro
api_root_validator,
cache_validator,
content_origin_validator,
sha256_validator,
Expand Down Expand Up @@ -511,7 +511,11 @@
finally:
connection.close()

settings.set("V3_API_ROOT", settings.API_ROOT + "api/v3/") # Not user configurable
settings.set("V3_DOMAIN_API_ROOT", settings.API_ROOT + "<slug:pulp_domain>/api/v3/")
if settings.API_ROOT_REWRITE_HEADER:
api_root = "/<path:api_root>/"
else:
api_root = settings.API_ROOT
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("/"))
settings.set("V3_DOMAIN_API_ROOT_NO_FRONT_SLASH", settings.V3_DOMAIN_API_ROOT.lstrip("/"))
31 changes: 15 additions & 16 deletions pulpcore/app/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@
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


class ViewSetNode:
Expand Down Expand Up @@ -178,7 +182,7 @@ class PulpDefaultRouter(routers.DefaultRouter):
SpectacularRedocView.as_view(
authentication_classes=[],
permission_classes=[],
url=f"{settings.V3_API_ROOT}docs/api.json?include_html=1&pk_path=1",
url=f"{V3_API_ROOT}docs/api.json?include_html=1&pk_path=1",
),
name="schema-redoc",
),
Expand All @@ -187,36 +191,31 @@ class PulpDefaultRouter(routers.DefaultRouter):
SpectacularSwaggerView.as_view(
authentication_classes=[],
permission_classes=[],
url=f"{settings.V3_API_ROOT}docs/api.json?include_html=1&pk_path=1",
url=f"{V3_API_ROOT}docs/api.json?include_html=1&pk_path=1",
),
name="schema-swagger",
),
]

urlpatterns = [
path(f"{API_ROOT}", include(special_views)),
path(API_ROOT, include(special_views)),
path("auth/", include("rest_framework.urls")),
path(settings.V3_API_ROOT_NO_FRONT_SLASH, include(docs_and_status)),
]


def no_schema_view(old_path, name=None):
"""Take in a path and return a new duplicate path that will not show up in the API Schema."""
@extend_schema(exclude=True)
class NoSchema(old_path.callback.cls):
pass

new_view = NoSchema.as_view(**old_path.callback.initkwargs)
return path(str(old_path.pattern), new_view, name=name)


if settings.DOMAIN_ENABLED:
# Ensure Docs and Status endpoints are available within domains, but are not shown in API schema
docs_and_status_no_schema = []
for p in docs_and_status:

@extend_schema(exclude=True)
class NoSchema(p.callback.cls):
pass

view = NoSchema.as_view(**p.callback.initkwargs)
name = p.name + "-domains" if p.name else None
docs_and_status_no_schema.append(no_schema_view(p, name=name))
urlpatterns.append(path(API_ROOT, include(docs_and_status_no_schema)))
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)))

if "social_django" in settings.INSTALLED_APPS:
urlpatterns.append(
Expand Down
9 changes: 4 additions & 5 deletions pulpcore/app/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

from django.conf import settings
from django.db.models import Model, Sum
from django.urls import Resolver404, resolve, get_urlconf
from django.urls import Resolver404, resolve
from opentelemetry import metrics

from rest_framework.serializers import ValidationError
Expand All @@ -40,10 +40,9 @@ def reverse(viewname, args=None, kwargs=None, request=None, relative_url=True, *
"""
kwargs = kwargs or {}
if settings.DOMAIN_ENABLED:
kwargs.setdefault('pulp_domain', get_domain().name)
api_root = getattr(request, 'api_root', STRIPPED_API_ROOT)
if api_root != STRIPPED_API_ROOT or get_urlconf() == "pulpcore.app.path_api_urls":
kwargs.setdefault('api_root', api_root)
kwargs.setdefault("pulp_domain", get_domain().name)
if settings.API_ROOT_REWRITE_HEADER:
kwargs.setdefault("api_root", getattr(request, "api_root", STRIPPED_API_ROOT))
if relative_url:
request = None
return drf_reverse(viewname, args=args, kwargs=kwargs, request=request, **extra)
Expand Down
2 changes: 2 additions & 0 deletions pulpcore/app/viewsets/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,8 @@ def get_resource(uri, model=None):
elif key == "pulp_domain":
if hasattr(model, "pulp_domain"):
kwargs["pulp_domain__name"] = value
elif key == "api_root":
continue
else:
kwargs[key] = value

Expand Down
24 changes: 11 additions & 13 deletions pulpcore/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
from django.http.response import Http404
from django.conf import settings
from django.core.exceptions import MiddlewareNotUsed
from django.urls import set_urlconf


class DomainMiddleware:
Expand Down Expand Up @@ -50,8 +49,8 @@ class APIRootRewriteMiddleware:
When API_ROOT_REWRITE_HEADER is set, this middleware will check for the existence of the header
on the request and if set it will add the new API_ROOT to the request context and remove the
path from the view_kwargs. If the header API_ROOT does not match the url path's API_ROOT this
middleware will return a 404. If the header is not set on the request this middleware does
nothing.
middleware will return a 404. If the header is not set on the request this middleware will set
`api_root` on the request context to the default setting's API_ROOT value.
When API_ROOT_REWRITE_HEADER is not set, this middleware will be marked as unused.
"""
Expand All @@ -64,10 +63,11 @@ def __init__(self, get_response):
def __call__(self, request):
# Code to be executed for each request before
# the view (and later middleware) are called.
if new_api_root := request.headers.get(settings.API_ROOT_REWRITE_HEADER):
setattr(request, 'api_root', new_api_root)
setattr(request, 'urlconf', 'pulpcore.app.path_api_urls')
set_urlconf('pulpcore.app.path_api_urls')
if settings.API_ROOT_REWRITE_HEADER in request.headers:
api_root = request.headers[settings.API_ROOT_REWRITE_HEADER].strip("/")
else:
api_root = settings.API_ROOT.strip("/")
setattr(request, "api_root", api_root)

response = self.get_response(request)

Expand All @@ -78,11 +78,9 @@ def __call__(self, request):
return response

def process_view(self, request, view_func, view_args, view_kwargs):
if new_api_root := getattr(request, 'api_root', None):
# Ensure that the requested URL's API_ROOT matches the header's API_ROOT
# Should I be less strict in the check and strip '/' from beginning and end?
api_root = view_kwargs.pop('api_root', None)
if api_root and api_root != new_api_root:
raise Http404()
# Ensure that the requested URL's API_ROOT matches the header's/default API_ROOT
api_root = view_kwargs.pop("api_root", None)
if api_root and api_root != request.api_root:
raise Http404()

return None
11 changes: 9 additions & 2 deletions pulpcore/openapi/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,12 @@
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
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(">", "}")


# Python does not distinguish integer sizes. The safest assumption is that they are large.
extend_schema_field(OpenApiTypes.INT64)(serializers.IntegerField)

Expand All @@ -51,6 +54,7 @@ class PulpAutoSchema(AutoSchema):
"patch": "partial_update",
"delete": "delete",
}
V3_API = API_ROOT_NO_FRONT_SLASH.replace("{pulp_domain}/", "")

def _tokenize_path(self):
"""
Expand Down Expand Up @@ -78,7 +82,7 @@ def _tokenize_path(self):
if not tokenized_path and getattr(self.view, "get_view_name", None):
tokenized_path.extend(self.view.get_view_name().split())

path = "/".join(tokenized_path).replace(settings.V3_API_ROOT_NO_FRONT_SLASH, "")
path = "/".join(tokenized_path).replace(self.V3_API, "")
tokenized_path = path.split("/")

return tokenized_path
Expand Down Expand Up @@ -414,6 +418,9 @@ def parse(self, input_request, public):

schema = view.schema

if settings.API_ROOT_REWRITE_HEADER:
path = path.replace("{api_root}", settings.API_ROOT.strip("/"))

if input_request is None or "pk_path" not in query_params:
path = self.convert_endpoint_path_params(path, view, schema)

Expand Down
5 changes: 0 additions & 5 deletions pulpcore/tasking/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

from django.db import connection, transaction
from django.db.models import Model, Max
from django.urls import get_urlconf, set_urlconf
from django_guid import get_guid
from pulpcore.app.apps import MODULE_PLUGIN_VERSIONS
from pulpcore.app.models import Task
Expand Down Expand Up @@ -68,7 +67,6 @@ def _execute_task(task):
func = getattr(module, function_name)
args = task.enc_args or ()
kwargs = task.enc_kwargs or {}
set_urlconf(kwargs.pop("_urlconf", None))
result = func(*args, **kwargs)
if asyncio.iscoroutine(result):
_logger.debug(_("Task is coroutine %s"), task.pk)
Expand Down Expand Up @@ -160,9 +158,6 @@ def dispatch(
resources = exclusive_resources + [f"shared:{resource}" for resource in shared_resources]

notify_workers = False
if urlconf := get_urlconf():
kwargs = kwargs or {}
kwargs["_urlconf"] = urlconf
with contextlib.ExitStack() as stack:
with transaction.atomic():
# Task creation need to be serialized so that pulp_created will provide a stable order
Expand Down
4 changes: 4 additions & 0 deletions pulpcore/tests/functional/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1101,6 +1101,10 @@ def pulp_api_v3_path(pulp_settings, pulp_domain_enabled):
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


Expand Down

0 comments on commit a2745ad

Please sign in to comment.