From 6445757b17ad89d8297cfba51d9b9d68bbdffd35 Mon Sep 17 00:00:00 2001 From: Matthias Dellweg Date: Thu, 24 Apr 2025 11:04:03 +0200 Subject: [PATCH 1/2] Create a parallel v4 API closes #6462 --- CHANGES/6462.feature | 1 + .../commands/analyze-publication.py | 4 +- pulpcore/app/models/repository.py | 2 +- pulpcore/app/response.py | 8 +- pulpcore/app/serializers/base.py | 26 +++- pulpcore/app/serializers/fields.py | 4 +- pulpcore/app/serializers/task.py | 4 +- pulpcore/app/settings.py | 12 +- pulpcore/app/urls.py | 122 ++++++++++++------ pulpcore/app/util.py | 4 +- pulpcore/app/viewsets/base.py | 2 +- .../tests/unit/serializers/test_domain.py | 2 +- 12 files changed, 144 insertions(+), 47 deletions(-) create mode 100644 CHANGES/6462.feature diff --git a/CHANGES/6462.feature b/CHANGES/6462.feature new file mode 100644 index 0000000000..481acb4290 --- /dev/null +++ b/CHANGES/6462.feature @@ -0,0 +1 @@ +Added a /pulp/api/v4/ namespace in parallel with the existing /pulp/api/v3/ namespace. This is disabled by default (`settings.ENABLE_V4_API`) and should be used only for development & experimentation. \ No newline at end of file diff --git a/pulpcore/app/management/commands/analyze-publication.py b/pulpcore/app/management/commands/analyze-publication.py index 21e897e65a..6c38013710 100644 --- a/pulpcore/app/management/commands/analyze-publication.py +++ b/pulpcore/app/management/commands/analyze-publication.py @@ -48,7 +48,9 @@ def handle(self, *args, **options): published_artifacts = publication.published_artifact.select_related( "content_artifact__artifact" ).order_by("relative_path") - artifact_href_prefix = reverse(get_view_name_for_model(Artifact, "list")) + artifact_href_prefix = reverse( + get_view_name_for_model(Artifact, "list") + ) # todo: reverse() + namespacing issues, print PRN instead? if options["tabular"]: table = PrettyTable() diff --git a/pulpcore/app/models/repository.py b/pulpcore/app/models/repository.py index 7818009562..c53a047e67 100644 --- a/pulpcore/app/models/repository.py +++ b/pulpcore/app/models/repository.py @@ -1422,7 +1422,7 @@ def get_content_href(self, request=None): ctype_model = ctypes[self.content_type] ctype_view = get_view_name_for_model(ctype_model, "list") try: - ctype_url = reverse(ctype_view, request=request) + ctype_url = reverse(ctype_view, request=request) # TODO: reverse() + namespacing issues except django.urls.exceptions.NoReverseMatch: # We've hit a content type for which there is no viewset. # There's nothing we can do here, except to skip it. diff --git a/pulpcore/app/response.py b/pulpcore/app/response.py index 7e78310afa..d6535c2fe0 100644 --- a/pulpcore/app/response.py +++ b/pulpcore/app/response.py @@ -23,7 +23,9 @@ def __init__(self, task, request): request (rest_framework.request.Request): Request used to generate the pulp_href urls """ kwargs = {"pk": task.pk} - resp = {"task": reverse("tasks-detail", kwargs=kwargs, request=request)} + resp = { + "task": reverse("tasks-detail", kwargs=kwargs, request=request) + } # reverse() + namespacing issues super().__init__(data=resp, status=202) @@ -47,5 +49,7 @@ def __init__(self, task_group, request): request (rest_framework.request.Request): Request used to generate the pulp_href urls """ kwargs = {"pk": task_group.pk} - resp = {"task_group": reverse("task-groups-detail", kwargs=kwargs, request=request)} + resp = { + "task_group": reverse("task-groups-detail", kwargs=kwargs, request=request) + } # reverse() + namespacing issues super().__init__(data=resp, status=202) diff --git a/pulpcore/app/serializers/base.py b/pulpcore/app/serializers/base.py index 2076068705..ceb5028c0c 100644 --- a/pulpcore/app/serializers/base.py +++ b/pulpcore/app/serializers/base.py @@ -68,7 +68,7 @@ class HrefPrnFieldMixin: def get_url(self, obj, view_name, request, *args, **kwargs): # Use the Pulp reverse method to display relative hrefs. - self.reverse = _reverse(obj) + self.reverse = _reverse(obj) # TODO: reverse() + namespacing issues return super().get_url(obj, view_name, request, *args, **kwargs) def to_internal_value(self, data): @@ -456,6 +456,30 @@ class Meta: read_only=True, ) + # def __init__(self, *args, **kwargs): + # super().__init__(*args, **kwargs) + + # # The context kwarg is passed by the ViewSet + # context = kwargs.get("context", {}) + # request = context.get("request") + + # # If we are not in a context with a request, or if the namespace is v4, + # # remove the 'pulp_href' field. + # if request and request.resolver_match.namespace == "v4": + # self.fields.pop("pulp_href", None) + + def to_representation(self, instance): + """Overridden to drop the pulp_href field from responses""" + representation = super().to_representation(instance) + + if request := self.context.get("request"): + if request.version == "v4": + # TODO: this feels hacky, but apparently this code is being used on serializers + # w/o pulp_href + if "pulp_href" in representation: + representation.pop("pulp_href") + return representation + def _validate_relative_path(self, path): """ Validate a relative path (eg from a url) to ensure it forms a valid url and does not begin diff --git a/pulpcore/app/serializers/fields.py b/pulpcore/app/serializers/fields.py index dbb2261239..0889ad7272 100644 --- a/pulpcore/app/serializers/fields.py +++ b/pulpcore/app/serializers/fields.py @@ -229,7 +229,9 @@ def to_representation(self, value): if content_artifact.artifact_id: kwargs["pk"] = content_artifact.artifact_id request = self.context.get("request") - url = reverse("artifacts-detail", kwargs=kwargs, request=request) + url = reverse( + "artifacts-detail", kwargs=kwargs, request=request + ) # TODO: reverse() + namespacing issues else: url = None ret[content_artifact.relative_path] = url diff --git a/pulpcore/app/serializers/task.py b/pulpcore/app/serializers/task.py index 998a872744..c2961a9be5 100644 --- a/pulpcore/app/serializers/task.py +++ b/pulpcore/app/serializers/task.py @@ -100,7 +100,9 @@ def get_created_by(self, obj) -> t.Optional[OpenApiTypes.URI]: if user_id := task_user_map.get(str(obj.pk)): kwargs = {"pk": user_id} request = self.context.get("request") - return reverse("users-detail", kwargs=kwargs, request=request) + return reverse( + "users-detail", kwargs=kwargs, request=request + ) # TODO: reverse() + namespacing issues return None class Meta: diff --git a/pulpcore/app/settings.py b/pulpcore/app/settings.py index 6d1161ac28..28fa2339d0 100644 --- a/pulpcore/app/settings.py +++ b/pulpcore/app/settings.py @@ -99,6 +99,9 @@ API_ROOT = "/pulp/" API_ROOT_REWRITE_HEADER = None +# Enable Pulp v4 API namespace +ENABLE_V4_API = False + # Application definition INSTALLED_APPS = [ @@ -192,7 +195,9 @@ "rest_framework.authentication.SessionAuthentication", ), "UPLOADED_FILES_USE_URL": False, - "DEFAULT_VERSIONING_CLASS": "rest_framework.versioning.URLPathVersioning", + "DEFAULT_VERSIONING_CLASS": "rest_framework.versioning.NamespaceVersioning", + "DEFAULT_VERSION": "v3", + "ALLOWED_VERSIONS": ["v3", "v4"], "DEFAULT_SCHEMA_CLASS": "pulpcore.openapi.PulpAutoSchema", } @@ -639,3 +644,8 @@ def otel_middleware_hook(settings): settings.set("V3_DOMAIN_API_ROOT", api_root + "/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("/")) + +settings.set("V4_API_ROOT", api_root + "api/v4/") # Not user configurable +settings.set("V4_DOMAIN_API_ROOT", api_root + "/api/v4/") +settings.set("V4_API_ROOT_NO_FRONT_SLASH", settings.V4_API_ROOT.lstrip("/")) +settings.set("V4_DOMAIN_API_ROOT_NO_FRONT_SLASH", settings.V4_DOMAIN_API_ROOT.lstrip("/")) diff --git a/pulpcore/app/urls.py b/pulpcore/app/urls.py index da966a60f0..01d2a9598c 100644 --- a/pulpcore/app/urls.py +++ b/pulpcore/app/urls.py @@ -32,10 +32,13 @@ 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) + V4_API_ROOT = settings.V4_API_ROOT.replace("//", settings.API_ROOT) else: V3_API_ROOT = settings.V3_API_ROOT + V4_API_ROOT = settings.V4_API_ROOT class ViewSetNode: @@ -153,70 +156,83 @@ class PulpDefaultRouter(routers.DefaultRouter): vs_tree.add_decendent(ViewSetNode(viewset)) special_views = [ - path("login/", LoginViewSet.as_view()), - path("repair/", RepairView.as_view()), + path("login/", LoginViewSet.as_view(), name="login"), + path("repair/", RepairView.as_view(), name="repair"), path( "orphans/cleanup/", OrphansCleanupViewset.as_view(actions={"post": "cleanup"}), + name="orphan-cleanup", ), - path("orphans/", OrphansView.as_view()), + path("orphans/", OrphansView.as_view(), name="orphans"), path( "repository_versions/", ListRepositoryVersionViewSet.as_view(actions={"get": "list"}), + name="repository-versions", ), path( "repositories/reclaim_space/", ReclaimSpaceViewSet.as_view(actions={"post": "reclaim"}), + name="reclaim", ), path( "importers/core/pulp/import-check/", PulpImporterImportCheckView.as_view(), + name="pulp-importer-import-check", ), ] -docs_and_status = [ - path("livez/", LivezView.as_view()), - path("status/", StatusView.as_view()), - path( - "docs/api.json", - SpectacularJSONAPIView.as_view(authentication_classes=[], permission_classes=[]), - name="schema", - ), - path( - "docs/api.yaml", - SpectacularYAMLAPIView.as_view(authentication_classes=[], permission_classes=[]), - name="schema-yaml", - ), - path( - "docs/", - SpectacularRedocView.as_view( - authentication_classes=[], - permission_classes=[], - url=f"{V3_API_ROOT}docs/api.json?include_html=1&pk_path=1", + +def _docs_and_status(_api_root): + paths = [ + path( + "docs/api.json", + SpectacularJSONAPIView.as_view(authentication_classes=[], permission_classes=[]), + name="schema", ), - name="schema-redoc", - ), - path( - "swagger/", - SpectacularSwaggerView.as_view( - authentication_classes=[], - permission_classes=[], - url=f"{V3_API_ROOT}docs/api.json?include_html=1&pk_path=1", + path( + "docs/api.yaml", + SpectacularYAMLAPIView.as_view(authentication_classes=[], permission_classes=[]), + name="schema-yaml", ), - name="schema-swagger", - ), -] + path( + "docs/", + SpectacularRedocView.as_view( + authentication_classes=[], + permission_classes=[], + url=f"{_api_root}docs/api.json?include_html=1&pk_path=1", + ), + name="schema-redoc", + ), + path( + "swagger/", + SpectacularSwaggerView.as_view( + authentication_classes=[], + permission_classes=[], + url=f"{_api_root}docs/api.json?include_html=1&pk_path=1", + ), + name="schema-swagger", + ), + path("livez/", LivezView.as_view(), name="livez"), + path("status/", StatusView.as_view(), name="status"), + ] + + return paths + + +v3_docs_and_status = _docs_and_status(V3_API_ROOT) +v4_docs_and_status = _docs_and_status(V4_API_ROOT) urlpatterns = [ - path(API_ROOT, include(special_views)), path("auth/", include("rest_framework.urls")), - path(settings.V3_API_ROOT_NO_FRONT_SLASH, include(docs_and_status)), + path(API_ROOT, include(special_views)), + path(settings.V3_API_ROOT_NO_FRONT_SLASH, include(v3_docs_and_status)), ] + 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: + for p in v3_docs_and_status: @extend_schema(exclude=True) class NoSchema(p.callback.cls): @@ -227,6 +243,34 @@ class NoSchema(p.callback.cls): 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 settings.ENABLE_V4_API: + urlpatterns.extend( + [ + path(V4_API_ROOT, include((special_views, "core"), namespace="v4")), + path( + settings.V4_API_ROOT_NO_FRONT_SLASH, + include((v4_docs_and_status, "core"), namespace="v4"), + ), + ] + ) + + +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 v4_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(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( path("", include("social_django.urls", namespace=settings.SOCIAL_AUTH_URL_NAMESPACE)) @@ -239,6 +283,12 @@ class NoSchema(p.callback.cls): for router in all_routers: urlpatterns.append(path(API_ROOT, include(router.urls))) +if settings.ENABLE_V4_API: + for router in all_routers: + urlpatterns.append( + path(V4_API_ROOT.lstrip("/"), include((router.urls, "core"), namespace="v4")) + ) + # If plugins define a urls.py, include them into the root namespace. for plugin_pattern in plugin_patterns: urlpatterns.append(path("", include(plugin_pattern))) diff --git a/pulpcore/app/util.py b/pulpcore/app/util.py index f9fb4375c3..785d7bd6ca 100644 --- a/pulpcore/app/util.py +++ b/pulpcore/app/util.py @@ -80,7 +80,9 @@ def get_url(model, domain=None, request=None): else: view_action = "list" - return reverse(get_view_name_for_model(model, view_action), kwargs=kwargs, request=request) + return reverse( + get_view_name_for_model(model, view_action), kwargs=kwargs, request=request + ) # TODO: reverse() + namespacing issues def get_prn(instance=None, uri=None): diff --git a/pulpcore/app/viewsets/base.py b/pulpcore/app/viewsets/base.py index 900066a926..c376c655de 100644 --- a/pulpcore/app/viewsets/base.py +++ b/pulpcore/app/viewsets/base.py @@ -182,7 +182,7 @@ def get_resource(uri, model=None): found_kwargs["pk"] = pk else: try: - match = resolve(urlparse(uri).path) + match = resolve(urlparse(uri).path) # TODO: resolve() + namespacing issues except Resolver404: raise DRFValidationError(detail=_("URI not valid: {u}").format(u=uri)) else: diff --git a/pulpcore/tests/unit/serializers/test_domain.py b/pulpcore/tests/unit/serializers/test_domain.py index c1bcb43787..a05946185c 100644 --- a/pulpcore/tests/unit/serializers/test_domain.py +++ b/pulpcore/tests/unit/serializers/test_domain.py @@ -177,7 +177,7 @@ def test_hidden_settings(storage_class, serializer_class, all_settings): storage_settings=all_settings, ) serializer = DomainSerializer(domain) - serializer.fields.pop("pulp_href") + serializer.fields.pop("pulp_href", None) serializer.fields.pop("prn") assert "hidden_fields" in serializer.data["storage_settings"] hidden_fields = serializer.data["storage_settings"]["hidden_fields"] From 5e60dd59a3528b5194b898f8f0570d85d8987171 Mon Sep 17 00:00:00 2001 From: Daniel Alley Date: Tue, 15 Jul 2025 16:16:51 -0400 Subject: [PATCH 2/2] temp --- pulpcore/app/settings.py | 10 ++++++-- pulpcore/middleware.py | 54 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 60 insertions(+), 4 deletions(-) diff --git a/pulpcore/app/settings.py b/pulpcore/app/settings.py index 28fa2339d0..f860595c4d 100644 --- a/pulpcore/app/settings.py +++ b/pulpcore/app/settings.py @@ -100,7 +100,7 @@ API_ROOT_REWRITE_HEADER = None # Enable Pulp v4 API namespace -ENABLE_V4_API = False +ENABLE_V4_API = True # Application definition @@ -195,7 +195,7 @@ "rest_framework.authentication.SessionAuthentication", ), "UPLOADED_FILES_USE_URL": False, - "DEFAULT_VERSIONING_CLASS": "rest_framework.versioning.NamespaceVersioning", + "DEFAULT_VERSIONING_CLASS": "pulpcore.middleware.NamespaceVersioning", "DEFAULT_VERSION": "v3", "ALLOWED_VERSIONS": ["v3", "v4"], "DEFAULT_SCHEMA_CLASS": "pulpcore.openapi.PulpAutoSchema", @@ -640,6 +640,7 @@ def otel_middleware_hook(settings): 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 + "/api/v3/") settings.set("V3_API_ROOT_NO_FRONT_SLASH", settings.V3_API_ROOT.lstrip("/")) @@ -649,3 +650,8 @@ def otel_middleware_hook(settings): settings.set("V4_DOMAIN_API_ROOT", api_root + "/api/v4/") settings.set("V4_API_ROOT_NO_FRONT_SLASH", settings.V4_API_ROOT.lstrip("/")) settings.set("V4_DOMAIN_API_ROOT_NO_FRONT_SLASH", settings.V4_DOMAIN_API_ROOT.lstrip("/")) + +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 diff --git a/pulpcore/middleware.py b/pulpcore/middleware.py index c01a00e16b..c699bbed4f 100644 --- a/pulpcore/middleware.py +++ b/pulpcore/middleware.py @@ -6,7 +6,9 @@ from django.http.response import Http404 from django.conf import settings -from django.core.exceptions import MiddlewareNotUsed +from django.core import exceptions as django_exceptions + +from rest_framework.versioning import BaseVersioning from pulpcore.metrics import init_otel_meter from pulpcore.app.models import Domain @@ -72,7 +74,7 @@ class APIRootRewriteMiddleware: def __init__(self, get_response): if not settings.API_ROOT_REWRITE_HEADER: - raise MiddlewareNotUsed() + raise django_exceptions.MiddlewareNotUsed() self.get_response = get_response def __call__(self, request): @@ -195,3 +197,51 @@ def __call__(self, request): x_task_diagnostics_var.reset(ctx_token) else: return self.get_response(request) + + +class NamespaceVersioning(BaseVersioning): + """ + To the client this is the same style as `URLPathVersioning`. + The difference is in the backend - this implementation uses + Django's URL namespaces to determine the version. + + An example URL conf that is namespaced into two separate versions + + # users/urls.py + urlpatterns = [ + path('/users/', users_list, name='users-list'), + path('/users//', users_detail, name='users-detail') + ] + + # urls.py + urlpatterns = [ + path('v1/', include('users.urls', namespace='v1')), + path('v2/', include('users.urls', namespace='v2')) + ] + + GET /1.0/something/ HTTP/1.1 + Host: example.com + Accept: application/json + """ + + invalid_version_message = "Invalid version in URL path. Does not match any version namespace." + + def determine_version(self, request, *args, **kwargs): + resolver_match = getattr(request, "resolver_match", None) + if resolver_match is None or not resolver_match.namespace: + return self.default_version + + # Allow for possibly nested namespaces. + possible_versions = resolver_match.namespace.split(":") + for version in possible_versions: + if self.is_allowed_version(version): + return version + raise django_exceptions.NotFound(self.invalid_version_message) + + def reverse(self, viewname, args=None, kwargs=None, request=None, format=None, **extra): + if request.version is not None: + viewname = self.get_versioned_viewname(viewname, request) + return super().reverse(viewname, args, kwargs, request, format, **extra) + + def get_versioned_viewname(self, viewname, request): + return request.version + ":" + viewname