diff --git a/.github/workflows/scripts/install.sh b/.github/workflows/scripts/install.sh index 127c907262..663a8e4a56 100755 --- a/.github/workflows/scripts/install.sh +++ b/.github/workflows/scripts/install.sh @@ -126,7 +126,7 @@ if [ "$TEST" = "azure" ]; then - ./azurite:/etc/pulp\ command: "azurite-blob --blobHost 0.0.0.0 --cert /etc/pulp/azcert.pem --key /etc/pulp/azkey.pem"' vars/main.yaml sed -i -e '$a azure_test: true\ -pulp_scenario_settings: {"domain_enabled": true}\ +pulp_scenario_settings: {"api_root_rewrite_header": "X-API-Root", "domain_enabled": true}\ pulp_scenario_env: {"otel_bsp_max_export_batch_size": 1, "otel_bsp_max_queue_size": 1, "otel_exporter_otlp_endpoint": "http://localhost:4318", "otel_exporter_otlp_protocol": "http/protobuf", "pulp_otel_enabled": "true"}\ ' vars/main.yaml fi diff --git a/.github/workflows/scripts/pre_before_script.sh b/.github/workflows/scripts/pre_before_script.sh index 143f743961..0c1f8cb2cc 100644 --- a/.github/workflows/scripts/pre_before_script.sh +++ b/.github/workflows/scripts/pre_before_script.sh @@ -1,8 +1,9 @@ set -euv - - if [ "$TEST" = "azure" ]; then + cmd_stdin_prefix bash -c "cat > /etc/nginx/pulp/api_root_rewrite.conf" < pulpcore/tests/functional/assets/api_root_rewrite.conf + cmd_prefix bash -c "s6-rc -d change nginx" + cmd_prefix bash -c "s6-rc -u change nginx" cmd_stdin_prefix bash -c "cat > /var/lib/pulp/scripts/otel_server.py" < pulpcore/tests/functional/assets/otel_server.py cmd_user_prefix nohup python3 /var/lib/pulp/scripts/otel_server.py & fi diff --git a/CHANGES/4207.feature b/CHANGES/4207.feature new file mode 100644 index 0000000000..293fd0da65 --- /dev/null +++ b/CHANGES/4207.feature @@ -0,0 +1,2 @@ +Added new setting ``API_ROOT_REWRITE_HEADER`` that when specified allows the API_ROOT to be rewritten +per request based on the header's value. diff --git a/CHANGES/plugin_api/4207.feature b/CHANGES/plugin_api/4207.feature new file mode 100644 index 0000000000..950dc94b96 --- /dev/null +++ b/CHANGES/plugin_api/4207.feature @@ -0,0 +1,2 @@ +Added new ``reverse`` method that handles Pulp specific url formatting. Plugins should update +instances of ``django.urls.reverse`` and ``rest_framework.reverse`` to this new Pulp one. diff --git a/pulpcore/app/models/repository.py b/pulpcore/app/models/repository.py index da21cbe985..6f96ec04d5 100644 --- a/pulpcore/app/models/repository.py +++ b/pulpcore/app/models/repository.py @@ -14,7 +14,6 @@ from django.core.validators import MinValueValidator from django.db import models, transaction from django.db.models import F, Func, Q, Value -from django.urls import reverse from django_lifecycle import AFTER_UPDATE, BEFORE_DELETE, hook from rest_framework.exceptions import APIException @@ -22,9 +21,9 @@ batch_qs, get_url, get_view_name_for_model, - get_domain, get_domain_pk, cache_key, + reverse, ) from pulpcore.constants import ALL_KNOWN_CONTENT_CHECKSUMS, PROTECTED_REPO_VERSION_MESSAGE from pulpcore.download.factory import DownloaderFactory @@ -1254,8 +1253,7 @@ class RepositoryVersionContentDetails(models.Model): ) count = models.IntegerField() - @property - def content_href(self): + def get_content_href(self, request=None): """ Generate URLs for the content types added, removed, or present in the RepositoryVersion. @@ -1274,11 +1272,8 @@ def content_href(self): ctypes = {c.get_pulp_type(): c for c in repository_model.CONTENT_TYPES} ctype_model = ctypes[self.content_type] ctype_view = get_view_name_for_model(ctype_model, "list") - kwargs = {} - if settings.DOMAIN_ENABLED: - kwargs["pulp_domain"] = get_domain().name try: - ctype_url = reverse(ctype_view, kwargs=kwargs) + ctype_url = reverse(ctype_view, request=request) 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. @@ -1286,7 +1281,7 @@ def content_href(self): repository_view = get_view_name_for_model(repository_model, "list") - repository_url = reverse(repository_view, kwargs=kwargs) + repository_url = reverse(repository_view, request=request) rv_href = ( repository_url + str(self.repository_version.repository_id) @@ -1300,3 +1295,5 @@ def content_href(self): partial_url_str = "{base}?repository_version_removed={rv_href}" full_url = partial_url_str.format(base=ctype_url, rv_href=rv_href) return full_url + + content_href = property(get_content_href) diff --git a/pulpcore/app/response.py b/pulpcore/app/response.py index 7023e3c26f..4aa9b7ad2f 100644 --- a/pulpcore/app/response.py +++ b/pulpcore/app/response.py @@ -1,6 +1,5 @@ -from django.conf import settings from rest_framework.response import Response -from rest_framework.reverse import reverse +from pulpcore.app.util import reverse class OperationPostponedResponse(Response): @@ -24,9 +23,7 @@ def __init__(self, task, request): request (rest_framework.request.Request): Request used to generate the pulp_href urls """ kwargs = {"pk": task.pk} - if settings.DOMAIN_ENABLED: - kwargs["pulp_domain"] = request.pulp_domain.name - resp = {"task": reverse("tasks-detail", kwargs=kwargs, request=None)} + resp = {"task": reverse("tasks-detail", kwargs=kwargs, request=request)} super().__init__(data=resp, status=202) @@ -50,7 +47,5 @@ def __init__(self, task_group, request): request (rest_framework.request.Request): Request used to generate the pulp_href urls """ kwargs = {"pk": task_group.pk} - if settings.DOMAIN_ENABLED: - kwargs["pulp_domain"] = request.pulp_domain.name - resp = {"task_group": reverse("task-groups-detail", kwargs=kwargs, request=None)} + resp = {"task_group": reverse("task-groups-detail", kwargs=kwargs, request=request)} super().__init__(data=resp, status=202) diff --git a/pulpcore/app/serializers/base.py b/pulpcore/app/serializers/base.py index 3192769812..3b43c64856 100644 --- a/pulpcore/app/serializers/base.py +++ b/pulpcore/app/serializers/base.py @@ -35,6 +35,7 @@ get_viewset_for_model, get_request_without_query_params, get_domain, + reverse, ) @@ -44,17 +45,17 @@ # Field mixins -def _reverse(reverse, request, obj): +def _reverse(obj): """Include domain-path in reverse call if DOMAIN_ENABLED.""" if settings.DOMAIN_ENABLED: @functools.wraps(reverse) - def _patched_reverse(viewname, args=None, kwargs=None, **extra): + 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, args=args, kwargs=kwargs, **extra) + return reverse(viewname, request=request, args=args, kwargs=kwargs, **extra) return _patched_reverse @@ -65,9 +66,9 @@ class HrefFieldMixin: """A mixin to configure related fields to generate relative hrefs.""" def get_url(self, obj, view_name, request, *args, **kwargs): - # Removes the request from the arguments to display relative hrefs. - self.reverse = _reverse(self.reverse, request, obj) - return super().get_url(obj, view_name, None, *args, **kwargs) + # Use the Pulp reverse method to display relative hrefs. + self.reverse = _reverse(obj) + return super().get_url(obj, view_name, request, *args, **kwargs) class _MatchingRegexViewName(object): @@ -165,14 +166,19 @@ class RelatedResourceField(RelatedField): Specific implementation requires the model to be defined in the Meta:. """ - def repo_ver_url(self, repo_ver): + def repo_ver_url(self, repo_ver, request=None): repo_model = Repository.get_model_for_pulp_type(repo_ver.repository.pulp_type) view_name = get_view_name_for_model(repo_model, "detail") obj = PKDomainObject(pk=repo_ver.repository.pk, pulp_domain=self.context["pulp_domain"]) - repo_url = self.get_url(obj, view_name, request=None, format=None) + repo_url = self.get_url(obj, view_name, request=request, format=None) return f"{repo_url}versions/{repo_ver.number}/" def to_representation(self, data): + # query parameters can be ignored because we are looking just for 'pulp_href'; still, + # we need to use the request object due to contextual references required by some + # serializers + request = get_request_without_query_params(self.context) + # Try to use optimized lookup to avoid DB if content_type has been already been fetched if GenericRelationModel.content_type.is_cached(data): model = data.content_type.model_class() @@ -181,7 +187,7 @@ def to_representation(self, data): repo_ver_mapping = self.context.get("repo_ver_mapping") if repo_ver_mapping is not None: if repo_ver := repo_ver_mapping.get(data.object_id): - return self.repo_ver_url(repo_ver) + return self.repo_ver_url(repo_ver, request=request) return None else: try: @@ -196,7 +202,7 @@ def to_representation(self, data): else: obj = PKObject(pk=data.object_id) try: - return self.get_url(obj, view_name, request=None, format=None) + return self.get_url(obj, view_name, request=request, format=None) except NoReverseMatch: pass @@ -210,11 +216,6 @@ def to_representation(self, data): except AttributeError: pass - # query parameters can be ignored because we are looking just for 'pulp_href'; still, - # we need to use the request object due to contextual references required by some - # serializers - request = get_request_without_query_params(self.context) - viewset = get_viewset_for_model(data.content_object) serializer = viewset.serializer_class(data.content_object, context={"request": request}) return serializer.data.get("pulp_href") diff --git a/pulpcore/app/serializers/fields.py b/pulpcore/app/serializers/fields.py index 4400656ff3..4594509ba8 100644 --- a/pulpcore/app/serializers/fields.py +++ b/pulpcore/app/serializers/fields.py @@ -6,11 +6,10 @@ from django.conf import settings from rest_framework import serializers from rest_framework.fields import empty -from rest_framework.reverse import reverse from pulpcore.app import models from pulpcore.app.serializers import DetailIdentityField, IdentityField, RelatedField -from pulpcore.app.util import get_domain +from pulpcore.app.util import reverse def relative_path_validator(relative_path): @@ -179,13 +178,11 @@ def to_representation(self, value): """ ret = {} kwargs = {} - if settings.DOMAIN_ENABLED: - domain = get_domain() - kwargs["pulp_domain"] = domain.name for content_artifact in value: if content_artifact.artifact_id: kwargs["pk"] = content_artifact.artifact_id - url = reverse("artifacts-detail", kwargs=kwargs, request=None) + request = self.context.get("request") + url = reverse("artifacts-detail", kwargs=kwargs, request=request) else: url = None ret[content_artifact.relative_path] = url diff --git a/pulpcore/app/serializers/repository.py b/pulpcore/app/serializers/repository.py index 6d9d2c6f5b..668dd56ac9 100644 --- a/pulpcore/app/serializers/repository.py +++ b/pulpcore/app/serializers/repository.py @@ -375,9 +375,13 @@ def to_representation(self, obj): """ to_return = {"added": {}, "removed": {}, "present": {}} + request = self.context.get("request") for count_detail in obj.counts.all(): count_type = count_detail.get_count_type_display() - item_dict = {"count": count_detail.count, "href": count_detail.content_href} + item_dict = { + "count": count_detail.count, + "href": count_detail.get_content_href(request=request), + } to_return[count_type][count_detail.content_type] = item_dict return to_return diff --git a/pulpcore/app/serializers/task.py b/pulpcore/app/serializers/task.py index 88661b2461..4ede7ea2af 100755 --- a/pulpcore/app/serializers/task.py +++ b/pulpcore/app/serializers/task.py @@ -1,7 +1,5 @@ from gettext import gettext as _ -from django.conf import settings -from django.urls import reverse from rest_framework import serializers from pulpcore.app import models @@ -15,7 +13,7 @@ TaskGroupStatusCountField, ) from pulpcore.constants import TASK_STATES -from pulpcore.app.util import get_domain +from pulpcore.app.util import reverse class CreatedResourceSerializer(RelatedResourceField): @@ -95,9 +93,8 @@ def get_created_by(self, obj): if task_user_map := self.context.get("task_user_mapping"): if user_id := task_user_map.get(str(obj.pk)): kwargs = {"pk": user_id} - if settings.DOMAIN_ENABLED: - kwargs["pulp_domain"] = get_domain().name - return reverse("users-detail", kwargs=kwargs) + request = self.context.get("request") + return reverse("users-detail", kwargs=kwargs, request=request) return None class Meta: diff --git a/pulpcore/app/settings.py b/pulpcore/app/settings.py index d4220a2d36..8414095ab1 100644 --- a/pulpcore/app/settings.py +++ b/pulpcore/app/settings.py @@ -60,6 +60,7 @@ # API Root API_ROOT = "/pulp/" +API_ROOT_REWRITE_HEADER = None # Application definition @@ -113,6 +114,7 @@ "django.contrib.messages.middleware.MessageMiddleware", "django.middleware.clickjacking.XFrameOptionsMiddleware", "pulpcore.middleware.DomainMiddleware", + "pulpcore.middleware.APIRootRewriteMiddleware", ] AUTHENTICATION_BACKENDS = [ @@ -509,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 + "/api/v3/") +if settings.API_ROOT_REWRITE_HEADER: + 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("/")) settings.set("V3_DOMAIN_API_ROOT_NO_FRONT_SLASH", settings.V3_DOMAIN_API_ROOT.lstrip("/")) diff --git a/pulpcore/app/urls.py b/pulpcore/app/urls.py index 2830e135c9..992660de49 100644 --- a/pulpcore/app/urls.py +++ b/pulpcore/app/urls.py @@ -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("//", settings.API_ROOT) +else: + V3_API_ROOT = settings.V3_API_ROOT class ViewSetNode: @@ -112,6 +116,13 @@ class PulpAPIRootView(APIRootView): authentication_classes = [] permission_classes = [] + def get(self, request, *args, **kwargs): + if settings.DOMAIN_ENABLED: + kwargs["pulp_domain"] = request.pulp_domain.name + if api_root := getattr(request, "api_root", None): + kwargs["api_root"] = api_root + return super().get(request, *args, **kwargs) + class PulpDefaultRouter(routers.DefaultRouter): """A DefaultRouter class that benefits from the customized PulpAPIRootView class.""" @@ -133,26 +144,25 @@ class PulpDefaultRouter(routers.DefaultRouter): for viewset in sorted_by_depth: vs_tree.add_decendent(ViewSetNode(viewset)) -urlpatterns = [ - path(f"{API_ROOT}repair/", RepairView.as_view()), +special_views = [ + path("repair/", RepairView.as_view()), path( - f"{API_ROOT}orphans/cleanup/", - OrphansCleanupViewset.as_view({"post": "cleanup"}), + "orphans/cleanup/", + OrphansCleanupViewset.as_view(actions={"post": "cleanup"}), ), - path(f"{API_ROOT}orphans/", OrphansView.as_view()), + path("orphans/", OrphansView.as_view()), path( - f"{API_ROOT}repository_versions/", - ListRepositoryVersionViewSet.as_view({"get": "list"}), + "repository_versions/", + ListRepositoryVersionViewSet.as_view(actions={"get": "list"}), ), path( - f"{API_ROOT}repositories/reclaim_space/", - ReclaimSpaceViewSet.as_view({"post": "reclaim"}), + "repositories/reclaim_space/", + ReclaimSpaceViewSet.as_view(actions={"post": "reclaim"}), ), path( - f"{API_ROOT}importers/core/pulp/import-check/", + "importers/core/pulp/import-check/", PulpImporterImportCheckView.as_view(), ), - path("auth/", include("rest_framework.urls")), ] docs_and_status = [ @@ -172,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", ), @@ -181,13 +191,17 @@ 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.append(path(settings.V3_API_ROOT_NO_FRONT_SLASH, include(docs_and_status))) +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)), +] if settings.DOMAIN_ENABLED: # Ensure Docs and Status endpoints are available within domains, but are not shown in API schema @@ -201,7 +215,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.append(path(API_ROOT, include(docs_and_status_no_schema))) + urlpatterns.insert(-1, path(API_ROOT, include(docs_and_status_no_schema))) if "social_django" in settings.INSTALLED_APPS: urlpatterns.append( diff --git a/pulpcore/app/util.py b/pulpcore/app/util.py index 00729a6f71..aad77a0d44 100644 --- a/pulpcore/app/util.py +++ b/pulpcore/app/util.py @@ -14,10 +14,11 @@ from django.conf import settings from django.db.models import Model, Sum -from django.urls import Resolver404, resolve, reverse +from django.urls import Resolver404, resolve from opentelemetry import metrics from rest_framework.serializers import ValidationError +from rest_framework.reverse import reverse as drf_reverse from pulpcore.app.loggers import deprecation_logger from pulpcore.app.apps import pulp_plugin_configs @@ -27,9 +28,27 @@ # 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 get_url(model, domain=None): +def reverse(viewname, args=None, kwargs=None, request=None, relative_url=True, **extra): + """ + Customized reverse to handle Pulp specific parameters like domains and API_ROOT rewrite. + + Calls DRF's reverse, but with request as None if relative_url is True (default) so that the + returned url is always relative. + """ + kwargs = kwargs or {} + 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)) + if relative_url: + request = None + return drf_reverse(viewname, args=args, kwargs=kwargs, request=request, **extra) + + +def get_url(model, domain=None, request=None): """ Get a resource url for the specified model instance or class. This returns the path component of the resource URI. @@ -38,6 +57,7 @@ def get_url(model, domain=None): model (django.models.Model): A model instance or class. domain Optional(str or Domain): The domain the url should be in if DOMAIN_ENABLED is set and domain can not be gathered from the model. Defaults to 'default'. + request Optional(django.http.HttpRequest): The request object this url is being created for. Returns: str: The path component of the resource url @@ -56,7 +76,7 @@ def get_url(model, domain=None): view_action = "detail" kwargs["pk"] = model.pk - return reverse(get_view_name_for_model(model, view_action), kwargs=kwargs) + return reverse(get_view_name_for_model(model, view_action), kwargs=kwargs, request=request) def get_prn(instance=None, uri=None): diff --git a/pulpcore/app/viewsets/base.py b/pulpcore/app/viewsets/base.py index 28e688bb2b..589c8a82e1 100644 --- a/pulpcore/app/viewsets/base.py +++ b/pulpcore/app/viewsets/base.py @@ -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 diff --git a/pulpcore/middleware.py b/pulpcore/middleware.py index e27b47f440..58ed59e68a 100644 --- a/pulpcore/middleware.py +++ b/pulpcore/middleware.py @@ -1,6 +1,8 @@ from pulpcore.app.models import Domain from pulpcore.app.util import set_current_user_lazy, set_domain from django.http.response import Http404 +from django.conf import settings +from django.core.exceptions import MiddlewareNotUsed class DomainMiddleware: @@ -38,3 +40,47 @@ def process_view(self, request, view_func, view_args, view_kwargs): set_domain(domain) setattr(request, "pulp_domain", domain) return None + + +class APIRootRewriteMiddleware: + """ + A middleware class to support API_ROOT_REWRITE_HEADER setting. + + 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 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. + """ + + def __init__(self, get_response): + if not settings.API_ROOT_REWRITE_HEADER: + raise MiddlewareNotUsed() + self.get_response = get_response + + def __call__(self, request): + # Code to be executed for each request before + # the view (and later middleware) are called. + 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) + + # Code to be executed for each request/response after + # the view is called. + # Should we add a header to the response to indicate the API_ROOT has been rewritten? + + return response + + def process_view(self, request, view_func, view_args, view_kwargs): + # 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 diff --git a/pulpcore/openapi/__init__.py b/pulpcore/openapi/__init__.py index 780637f7ef..db9665a3d2 100644 --- a/pulpcore/openapi/__init__.py +++ b/pulpcore/openapi/__init__.py @@ -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( + "", 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) @@ -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): """ @@ -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 @@ -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) diff --git a/pulpcore/plugin/util.py b/pulpcore/plugin/util.py index 38bb21b6fb..da755b7a4a 100644 --- a/pulpcore/plugin/util.py +++ b/pulpcore/plugin/util.py @@ -25,5 +25,6 @@ set_domain, get_current_user, get_current_authenticated_user, + reverse, set_current_user, ) diff --git a/pulpcore/tests/functional/__init__.py b/pulpcore/tests/functional/__init__.py index 0f2960e9aa..a460872ba9 100644 --- a/pulpcore/tests/functional/__init__.py +++ b/pulpcore/tests/functional/__init__.py @@ -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("", pulp_settings.API_ROOT.strip("/")) + return v3_api_root diff --git a/pulpcore/tests/functional/api/test_api_root_rewrite.py b/pulpcore/tests/functional/api/test_api_root_rewrite.py new file mode 100644 index 0000000000..d2da6a85d0 --- /dev/null +++ b/pulpcore/tests/functional/api/test_api_root_rewrite.py @@ -0,0 +1,141 @@ +import pytest +import uuid +import json + + +""" +To run these tests: +1. Copy ../assets/api_root_rewrite.conf to /etc/nginx/pulp in your Pulp container +2. Set API_ROOT_REWRITE_HEADER to "X-API-Root" +3. Restart Pulp and nginx +""" + + +@pytest.fixture(scope="session") +def proxy_rewrite_url(pulp_api_v3_url, pulp_settings): + return pulp_api_v3_url.replace(pulp_settings.API_ROOT, "/proxy/rewrite/") + + +@pytest.fixture(scope="session", autouse=True) +def proxy_rewrite_set(pulpcore_bindings, pulp_settings, proxy_rewrite_url): + if pulp_settings.API_ROOT_REWRITE_HEADER != "X-API-Root": + pytest.skip("API_ROOT_REWRITE_HEADER not set", allow_module_level=True) + response = pulpcore_bindings.client.request("GET", proxy_rewrite_url) + host = pulpcore_bindings.client.configuration.host + if response.status == 200: + body = json.loads(response.data) + for key, value in body.items(): + if "proxy/rewrite" not in value: + break + if host not in value: + # Hack for the CI + [_, api_root, endpoint] = value.partition("/proxy/rewrite/") + body[key] = f"{host}{api_root}{endpoint}" + else: + return body + + pytest.skip("Proxy rewrite path was not set up.", allow_module_level=True) + + +def auth_headers(bindings): + headers = {} + bindings.client.update_params_for_auth(headers, {}, ["basicAuth"]) + return headers + + +@pytest.mark.parallel +def test_list_endpoints(file_bindings, proxy_rewrite_set, pulp_api_v3_path): + """Check that ALL rewritten API_ROOT endpoints are accessible.""" + API_ROOT = pulp_api_v3_path.encode("utf-8") + for endpoint, url in proxy_rewrite_set.items(): + headers = auth_headers(file_bindings) + response = file_bindings.client.request("GET", url, headers=headers) + assert response.status == 200 + + if endpoint != "tasks": + # Tasks reserved resources can have original API_ROOT + assert API_ROOT not in response.data, f"failed on {endpoint}:{url}" + + +@pytest.mark.parallel +def test_full_workflow( + file_bindings, + basic_manifest_path, + file_fixture_server, + proxy_rewrite_url, + monitor_task, + add_to_cleanup, +): + """Test that normal sync/publish/distribute workflow works.""" + name = str(uuid.uuid4()) + # Step 1: Create Remote + remote_url = file_fixture_server.make_url(basic_manifest_path) + body = {"name": name, "url": remote_url, "policy": "on_demand"} + url = f"{proxy_rewrite_url}remotes/file/file/" + response = file_bindings.client.request( + "POST", url, body=body, headers=auth_headers(file_bindings) + ) + assert response.status == 201 + remote = json.loads(response.data) + add_to_cleanup(file_bindings.RemotesFileApi, remote["pulp_href"]) + assert remote["pulp_href"].startswith("/proxy/rewrite/") + # Step 2: Create Repository + body = {"name": name} + url = f"{proxy_rewrite_url}repositories/file/file/" + response = file_bindings.client.request( + "POST", url, body=body, headers=auth_headers(file_bindings) + ) + assert response.status == 201 + repository = json.loads(response.data) + add_to_cleanup(file_bindings.RepositoriesFileApi, repository["pulp_href"]) + assert repository["pulp_href"].startswith("/proxy/rewrite/") + assert repository["versions_href"].startswith("/proxy/rewrite/") + assert repository["latest_version_href"].startswith("/proxy/rewrite/") + # Step 3: Sync Repository w/ Remote + body = {"remote": remote["pulp_href"]} + url = f"{file_bindings.client.configuration.host}{repository['pulp_href']}sync/" + response = file_bindings.client.request( + "POST", url, body=body, headers=auth_headers(file_bindings) + ) + assert response.status == 202 + task_response = json.loads(response.data) + assert task_response["task"].startswith("/proxy/rewrite/") + task = monitor_task(task_response["task"]) + assert len(task.created_resources) == 1 + repo_ver = task.created_resources[0] + assert repo_ver.startswith("/proxy/rewrite/") + repo_ver = file_bindings.RepositoriesFileVersionsApi.read(repo_ver) + assert repo_ver.pulp_href.startswith("/proxy/rewrite/") + added_href = repo_ver.content_summary.added["file.file"]["href"] + assert added_href.count("/proxy/rewrite/") == 2 # start of href & one for repo-ver query param + # Step 4: Publish Repository + body = {"repository_version": repo_ver.pulp_href} + url = f"{proxy_rewrite_url}publications/file/file/" + response = file_bindings.client.request( + "POST", url, body=body, headers=auth_headers(file_bindings) + ) + assert response.status == 202 + task = monitor_task(json.loads(response.data)["task"]) + assert len(task.created_resources) == 1 + publication = task.created_resources[0] + add_to_cleanup(file_bindings.PublicationsFileApi, publication) + assert publication.startswith("/proxy/rewrite/") + publication = file_bindings.PublicationsFileApi.read(publication) + assert publication.pulp_href.startswith("/proxy/rewrite/") + assert publication.repository == repository["pulp_href"] + assert publication.repository_version == repo_ver.pulp_href + # Step 5: Distribute Publication + body = {"name": name, "base_path": name, "publication": publication.pulp_href} + url = f"{proxy_rewrite_url}distributions/file/file/" + response = file_bindings.client.request( + "POST", url, body=body, headers=auth_headers(file_bindings) + ) + assert response.status == 202 + task = monitor_task(json.loads(response.data)["task"]) + assert len(task.created_resources) == 1 + distribution = task.created_resources[0] + add_to_cleanup(file_bindings.DistributionsFileApi, distribution) + assert distribution.startswith("/proxy/rewrite/") + distribution = file_bindings.DistributionsFileApi.read(distribution) + assert distribution.pulp_href.startswith("/proxy/rewrite") + assert distribution.publication == publication.pulp_href diff --git a/pulpcore/tests/functional/assets/api_root_rewrite.conf b/pulpcore/tests/functional/assets/api_root_rewrite.conf new file mode 100644 index 0000000000..b2ec3538c9 --- /dev/null +++ b/pulpcore/tests/functional/assets/api_root_rewrite.conf @@ -0,0 +1,10 @@ +location /proxy/rewrite/ { + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Proto $scheme; + proxy_set_header X-API-Root proxy/rewrite; + proxy_set_header Host $http_host; + # we don't want nginx trying to do something clever with + # redirects, we set the Host: header above already. + proxy_redirect off; + proxy_pass http://pulp-api; +} diff --git a/pulpcore/tests/unit/test_viewsets.py b/pulpcore/tests/unit/test_viewsets.py index c9f7cbd5be..dee4e986f3 100644 --- a/pulpcore/tests/unit/test_viewsets.py +++ b/pulpcore/tests/unit/test_viewsets.py @@ -11,6 +11,8 @@ 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("/")) class TestGetResource(TestCase): diff --git a/pyproject.toml b/pyproject.toml index aff98b8836..73b9fcae8a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -96,4 +96,5 @@ ignore = [ ".github/**", "lint_requirements.txt", ".flake8", + "pulpcore/tests/functional/assets/**", ] diff --git a/template_config.yml b/template_config.yml index 2e4e69f79e..f7e8f81790 100644 --- a/template_config.yml +++ b/template_config.yml @@ -65,6 +65,7 @@ pulp_settings: - /tmp orphan_protection_time: 0 pulp_settings_azure: + api_root_rewrite_header: X-API-Root domain_enabled: true pulp_settings_gcp: null pulp_settings_s3: