From 0bf2deb393ddcb704bc1b199b2a7efe3d48884b8 Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Tue, 30 Jan 2024 00:48:25 -0500 Subject: [PATCH 1/2] Add ability to have proxy rewrite API_ROOT w/ header fixes: #4207 --- .github/workflows/scripts/install.sh | 2 +- .../workflows/scripts/pre_before_script.sh | 5 +- CHANGES/4207.feature | 2 + CHANGES/plugin_api/4207.feature | 2 + pulpcore/app/models/repository.py | 14 +- pulpcore/app/path_api_urls.py | 12 ++ pulpcore/app/response.py | 11 +- pulpcore/app/serializers/base.py | 45 +++--- pulpcore/app/serializers/fields.py | 9 +- pulpcore/app/serializers/repository.py | 5 +- pulpcore/app/serializers/task.py | 8 +- pulpcore/app/settings.py | 4 +- pulpcore/app/urls.py | 53 ++++--- pulpcore/app/util.py | 27 +++- pulpcore/middleware.py | 48 +++++++ pulpcore/plugin/util.py | 1 + pulpcore/tasking/tasks.py | 5 + .../functional/api/test_api_root_rewrite.py | 129 ++++++++++++++++++ .../functional/assets/api_root_rewrite.conf | 10 ++ template_config.yml | 1 + 20 files changed, 310 insertions(+), 83 deletions(-) create mode 100644 CHANGES/4207.feature create mode 100644 CHANGES/plugin_api/4207.feature create mode 100644 pulpcore/app/path_api_urls.py create mode 100644 pulpcore/tests/functional/api/test_api_root_rewrite.py create mode 100644 pulpcore/tests/functional/assets/api_root_rewrite.conf 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..4bfc4a914f 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 @@ -25,6 +24,7 @@ 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 +1254,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 +1273,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 +1282,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 +1296,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/path_api_urls.py b/pulpcore/app/path_api_urls.py new file mode 100644 index 0000000000..5e4969dd13 --- /dev/null +++ b/pulpcore/app/path_api_urls.py @@ -0,0 +1,12 @@ +from django.conf import settings +from django.urls import path, include +from pulpcore.app.urls import API_ROOT, docs_and_status, special_views, all_routers + +# Add all the Pulp API endpoints again, but with the API_ROOT as a path parameter +PATH_API_ROOT = "/" + API_ROOT.split(settings.API_ROOT[1:])[1] +dup_urls = special_views + docs_and_status +for router in all_routers: + dup_urls.extend(router.urls) + +# dups_no_schema = [no_schema_view(p, name=p.name) for p in dup_urls] +urlpatterns = [path(PATH_API_ROOT, include(dup_urls))] 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..82ecf34733 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,30 +45,16 @@ # Field mixins -def _reverse(reverse, request, 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): - 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 _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): - # 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 + if settings.DOMAIN_ENABLED: + domain_name = obj.pulp_domain.name if hasattr(obj, "pulp_domain") else "default" + kwargs["pulp_domain"] = domain_name + return super().get_url(obj, view_name, request, *args, **kwargs) class _MatchingRegexViewName(object): @@ -165,14 +152,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 +173,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 +188,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 +202,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..9d62cd40a9 100644 --- a/pulpcore/app/serializers/repository.py +++ b/pulpcore/app/serializers/repository.py @@ -375,9 +375,12 @@ 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..316f166124 100755 --- a/pulpcore/app/serializers/task.py +++ b/pulpcore/app/serializers/task.py @@ -1,7 +1,6 @@ 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 +14,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 +94,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..9e950fd62f 100644 --- a/pulpcore/app/settings.py +++ b/pulpcore/app/settings.py @@ -60,6 +60,7 @@ # API Root API_ROOT = "/pulp/" +API_ROOT_REWRITE_HEADER = 'X-API-Root' # Application definition @@ -113,6 +114,7 @@ "django.contrib.messages.middleware.MessageMiddleware", "django.middleware.clickjacking.XFrameOptionsMiddleware", "pulpcore.middleware.DomainMiddleware", + "pulpcore.middleware.APIRootRewriteMiddleware", ] AUTHENTICATION_BACKENDS = [ @@ -417,7 +419,7 @@ ENVVAR_FOR_DYNACONF="PULP_SETTINGS", load_dotenv=False, validators=[ - api_root_validator, + # api_root_validator, #running into a bug with this when running tests, ask bruno/pedro cache_validator, content_origin_validator, sha256_validator, diff --git a/pulpcore/app/urls.py b/pulpcore/app/urls.py index 2830e135c9..aa6f382aaa 100644 --- a/pulpcore/app/urls.py +++ b/pulpcore/app/urls.py @@ -112,6 +112,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 +140,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 = [ @@ -187,20 +193,29 @@ class PulpDefaultRouter(routers.DefaultRouter): ), ] -urlpatterns.append(path(settings.V3_API_ROOT_NO_FRONT_SLASH, include(docs_and_status))) +urlpatterns = [ + path(f"{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(path(str(p.pattern), view, name=name)) + docs_and_status_no_schema.append(no_schema_view(p, name=name)) urlpatterns.append(path(API_ROOT, include(docs_and_status_no_schema))) if "social_django" in settings.INSTALLED_APPS: diff --git a/pulpcore/app/util.py b/pulpcore/app/util.py index 00729a6f71..19cd8c75a4 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, get_urlconf 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,28 @@ # 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) + 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) + 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 +58,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 +77,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/middleware.py b/pulpcore/middleware.py index e27b47f440..53db3e280e 100644 --- a/pulpcore/middleware.py +++ b/pulpcore/middleware.py @@ -1,6 +1,9 @@ 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 +from django.urls import set_urlconf class DomainMiddleware: @@ -38,3 +41,48 @@ 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 does + nothing. + + 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 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') + + 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): + 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() + + return None 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/tasking/tasks.py b/pulpcore/tasking/tasks.py index ca19becf9c..f30e2ff454 100644 --- a/pulpcore/tasking/tasks.py +++ b/pulpcore/tasking/tasks.py @@ -10,6 +10,7 @@ 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 @@ -67,6 +68,7 @@ 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) @@ -158,6 +160,9 @@ 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 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..a35ce0953f --- /dev/null +++ b/pulpcore/tests/functional/api/test_api_root_rewrite.py @@ -0,0 +1,129 @@ +import pytest +import uuid +import json + +from pulpcore.app import settings + +""" +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 +""" + +if settings.API_ROOT_REWRITE_HEADER != 'X-API-Root': + pytest.skip("API_ROOT_REWRITE_HEADER not set", allow_module_level=True) + + +@pytest.fixture(scope="session") +def proxy_rewrite_url(bindings_cfg): + return f"{bindings_cfg.host}/proxy/rewrite/api/v3/" + + +@pytest.fixture(scope="session", autouse=True) +def proxy_rewrite_set(pulpcore_bindings, proxy_rewrite_url): + response = pulpcore_bindings.client.request("GET", proxy_rewrite_url) + if response.status == 200: + body = json.loads(response.data) + for value in body.values(): + if "proxy/rewrite" not in value: + break + else: + return body + + pytest.skip("Proxy rewrite path was not set up.", allow_module_level=True) + + +@pytest.fixture(scope="session") +def auth_headers(pulpcore_bindings): + headers = {} + pulpcore_bindings.client.update_params_for_auth(headers, {}, ["basicAuth"]) + return headers + + +@pytest.mark.parallel +def test_list_endpoints(pulpcore_bindings, proxy_rewrite_set, auth_headers): + """Check that ALL rewritten API_ROOT endpoints are accessible.""" + API_ROOT = settings.API_ROOT.encode("utf-8") + for endpoint, url in proxy_rewrite_set.items(): + response = pulpcore_bindings.client.request("GET", url, headers=auth_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, + auth_headers, + 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) + 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) + 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) + 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) + 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) + 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..78345fa120 --- /dev/null +++ b/pulpcore/tests/functional/assets/api_root_rewrite.conf @@ -0,0 +1,10 @@ +location /proxy/rewrite/api/v3/ { + 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; +} \ No newline at end of file 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: From bb71385f753640351ea902d5f319b0554f2a8e94 Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Mon, 18 Mar 2024 09:35:34 -0400 Subject: [PATCH 2/2] rework url routing [noissue] --- pulpcore/app/models/repository.py | 3 +- pulpcore/app/path_api_urls.py | 12 ----- pulpcore/app/serializers/base.py | 22 ++++++-- pulpcore/app/serializers/repository.py | 3 +- pulpcore/app/serializers/task.py | 1 - pulpcore/app/settings.py | 12 +++-- pulpcore/app/urls.py | 31 ++++++----- pulpcore/app/util.py | 9 ++-- pulpcore/app/viewsets/base.py | 2 + pulpcore/middleware.py | 24 ++++----- pulpcore/openapi/__init__.py | 11 +++- pulpcore/tasking/tasks.py | 5 -- pulpcore/tests/functional/__init__.py | 4 ++ .../functional/api/test_api_root_rewrite.py | 52 ++++++++++++------- .../functional/assets/api_root_rewrite.conf | 4 +- pulpcore/tests/unit/test_viewsets.py | 2 + pyproject.toml | 1 + 17 files changed, 111 insertions(+), 87 deletions(-) delete mode 100644 pulpcore/app/path_api_urls.py diff --git a/pulpcore/app/models/repository.py b/pulpcore/app/models/repository.py index 4bfc4a914f..6f96ec04d5 100644 --- a/pulpcore/app/models/repository.py +++ b/pulpcore/app/models/repository.py @@ -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 diff --git a/pulpcore/app/path_api_urls.py b/pulpcore/app/path_api_urls.py deleted file mode 100644 index 5e4969dd13..0000000000 --- a/pulpcore/app/path_api_urls.py +++ /dev/null @@ -1,12 +0,0 @@ -from django.conf import settings -from django.urls import path, include -from pulpcore.app.urls import API_ROOT, docs_and_status, special_views, all_routers - -# Add all the Pulp API endpoints again, but with the API_ROOT as a path parameter -PATH_API_ROOT = "/" + API_ROOT.split(settings.API_ROOT[1:])[1] -dup_urls = special_views + docs_and_status -for router in all_routers: - dup_urls.extend(router.urls) - -# dups_no_schema = [no_schema_view(p, name=p.name) for p in dup_urls] -urlpatterns = [path(PATH_API_ROOT, include(dup_urls))] diff --git a/pulpcore/app/serializers/base.py b/pulpcore/app/serializers/base.py index 82ecf34733..3b43c64856 100644 --- a/pulpcore/app/serializers/base.py +++ b/pulpcore/app/serializers/base.py @@ -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) diff --git a/pulpcore/app/serializers/repository.py b/pulpcore/app/serializers/repository.py index 9d62cd40a9..668dd56ac9 100644 --- a/pulpcore/app/serializers/repository.py +++ b/pulpcore/app/serializers/repository.py @@ -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 diff --git a/pulpcore/app/serializers/task.py b/pulpcore/app/serializers/task.py index 316f166124..4ede7ea2af 100755 --- a/pulpcore/app/serializers/task.py +++ b/pulpcore/app/serializers/task.py @@ -1,6 +1,5 @@ from gettext import gettext as _ -from django.conf import settings from rest_framework import serializers from pulpcore.app import models diff --git a/pulpcore/app/settings.py b/pulpcore/app/settings.py index 9e950fd62f..8414095ab1 100644 --- a/pulpcore/app/settings.py +++ b/pulpcore/app/settings.py @@ -60,7 +60,7 @@ # API Root API_ROOT = "/pulp/" -API_ROOT_REWRITE_HEADER = 'X-API-Root' +API_ROOT_REWRITE_HEADER = None # Application definition @@ -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, @@ -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 + "/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 aa6f382aaa..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: @@ -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", ), @@ -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( diff --git a/pulpcore/app/util.py b/pulpcore/app/util.py index 19cd8c75a4..aad77a0d44 100644 --- a/pulpcore/app/util.py +++ b/pulpcore/app/util.py @@ -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 @@ -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) 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 53db3e280e..58ed59e68a 100644 --- a/pulpcore/middleware.py +++ b/pulpcore/middleware.py @@ -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: @@ -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. """ @@ -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) @@ -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 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/tasking/tasks.py b/pulpcore/tasking/tasks.py index f30e2ff454..ca19becf9c 100644 --- a/pulpcore/tasking/tasks.py +++ b/pulpcore/tasking/tasks.py @@ -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 @@ -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) @@ -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 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 index a35ce0953f..d2da6a85d0 100644 --- a/pulpcore/tests/functional/api/test_api_root_rewrite.py +++ b/pulpcore/tests/functional/api/test_api_root_rewrite.py @@ -2,7 +2,6 @@ import uuid import json -from pulpcore.app import settings """ To run these tests: @@ -11,42 +10,46 @@ 3. Restart Pulp and nginx """ -if settings.API_ROOT_REWRITE_HEADER != 'X-API-Root': - pytest.skip("API_ROOT_REWRITE_HEADER not set", allow_module_level=True) - @pytest.fixture(scope="session") -def proxy_rewrite_url(bindings_cfg): - return f"{bindings_cfg.host}/proxy/rewrite/api/v3/" +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, proxy_rewrite_url): +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 value in body.values(): + 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) -@pytest.fixture(scope="session") -def auth_headers(pulpcore_bindings): +def auth_headers(bindings): headers = {} - pulpcore_bindings.client.update_params_for_auth(headers, {}, ["basicAuth"]) + bindings.client.update_params_for_auth(headers, {}, ["basicAuth"]) return headers @pytest.mark.parallel -def test_list_endpoints(pulpcore_bindings, proxy_rewrite_set, auth_headers): +def test_list_endpoints(file_bindings, proxy_rewrite_set, pulp_api_v3_path): """Check that ALL rewritten API_ROOT endpoints are accessible.""" - API_ROOT = settings.API_ROOT.encode("utf-8") + API_ROOT = pulp_api_v3_path.encode("utf-8") for endpoint, url in proxy_rewrite_set.items(): - response = pulpcore_bindings.client.request("GET", url, headers=auth_headers) + headers = auth_headers(file_bindings) + response = file_bindings.client.request("GET", url, headers=headers) assert response.status == 200 if endpoint != "tasks": @@ -60,7 +63,6 @@ def test_full_workflow( basic_manifest_path, file_fixture_server, proxy_rewrite_url, - auth_headers, monitor_task, add_to_cleanup, ): @@ -70,7 +72,9 @@ def test_full_workflow( 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) + 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"]) @@ -78,7 +82,9 @@ def test_full_workflow( # 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) + 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"]) @@ -88,7 +94,9 @@ def test_full_workflow( # 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) + 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/") @@ -103,7 +111,9 @@ def test_full_workflow( # 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) + 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 @@ -117,7 +127,9 @@ def test_full_workflow( # 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) + 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 diff --git a/pulpcore/tests/functional/assets/api_root_rewrite.conf b/pulpcore/tests/functional/assets/api_root_rewrite.conf index 78345fa120..b2ec3538c9 100644 --- a/pulpcore/tests/functional/assets/api_root_rewrite.conf +++ b/pulpcore/tests/functional/assets/api_root_rewrite.conf @@ -1,4 +1,4 @@ -location /proxy/rewrite/api/v3/ { +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; @@ -7,4 +7,4 @@ location /proxy/rewrite/api/v3/ { # redirects, we set the Host: header above already. proxy_redirect off; proxy_pass http://pulp-api; -} \ No newline at end of file +} 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/**", ]