From 857f2858c5574f1c4d53a550e1daa505bb39354f Mon Sep 17 00:00:00 2001 From: Matthias Dellweg Date: Tue, 5 Nov 2024 12:10:18 +0100 Subject: [PATCH 1/5] Test downloading profile artifacts --- .ci/settings/settings.py | 1 + pulp-glue/docs/dev/learn/architecture.md | 2 +- pulpcore/cli/core/task.py | 9 ++++++--- pytest_pulp_cli/__init__.py | 7 ++++--- tests/scripts/pulpcore/test_task.sh | 12 +++++++++++- 5 files changed, 23 insertions(+), 8 deletions(-) diff --git a/.ci/settings/settings.py b/.ci/settings/settings.py index ed037be22..362d83f73 100644 --- a/.ci/settings/settings.py +++ b/.ci/settings/settings.py @@ -3,6 +3,7 @@ ALLOWED_EXPORT_PATHS = ["/tmp"] ANALYTICS = False ALLOWED_CONTENT_CHECKSUMS = ["sha1", "sha256", "sha512"] +TASK_DIAGNOSTICS = ["memory"] if os.environ.get("PULP_HTTPS", "false").lower() == "true": AUTHENTICATION_BACKENDS = "@merge django.contrib.auth.backends.RemoteUserBackend" diff --git a/pulp-glue/docs/dev/learn/architecture.md b/pulp-glue/docs/dev/learn/architecture.md index 132b43fd4..afe5aadc9 100644 --- a/pulp-glue/docs/dev/learn/architecture.md +++ b/pulp-glue/docs/dev/learn/architecture.md @@ -8,7 +8,7 @@ To this end, `pulp-glue` is the go-to place for all known version-dependent Pulp ## OpenAPI -This is the part in `pulp_glue` that uses [`requests`](https://requests.readthedocs.io/) to perform low level communication with an `OpenAPI 3` compatible server. +This is the part in `pulp_glue` that uses http to perform low level communication with an `OpenAPI 3` compatible server. It is not anticipated that users of Pulp Glue need to interact with this abstraction layer. ## Contexts diff --git a/pulpcore/cli/core/task.py b/pulpcore/cli/core/task.py index c2e1fac5a..6ece44767 100644 --- a/pulpcore/cli/core/task.py +++ b/pulpcore/cli/core/task.py @@ -4,9 +4,12 @@ from pathlib import Path import click -import requests -from pulp_glue.common.context import DATETIME_FORMATS, PluginRequirement, PulpEntityContext +from pulp_glue.common.context import ( + DATETIME_FORMATS, + PluginRequirement, + PulpEntityContext, +) from pulp_glue.common.exceptions import PulpException from pulp_glue.common.i18n import get_translation from pulp_glue.core.context import PulpTaskContext @@ -194,7 +197,7 @@ def profile_artifact_urls( uuid = uuid_match.group("uuid") profile_artifact_dir = Path(".") / f"task_profile-{task_name}-{uuid}" profile_artifact_dir.mkdir(exist_ok=True) - with requests.session() as session: + with pulp_ctx.api._session as session: for name, url in urls.items(): profile_artifact_path = profile_artifact_dir / name click.echo(_("Downloading {path}").format(path=profile_artifact_path)) diff --git a/pytest_pulp_cli/__init__.py b/pytest_pulp_cli/__init__.py index 0f549e9ae..7e0d6f7c8 100644 --- a/pytest_pulp_cli/__init__.py +++ b/pytest_pulp_cli/__init__.py @@ -3,10 +3,10 @@ import subprocess import sys import typing as t +import urllib.request import gnupg import pytest -import requests import tomli_w if sys.version_info >= (3, 11): @@ -149,8 +149,9 @@ def pulp_cli_gnupghome(tmp_path_factory: pytest.TempPathFactory) -> pathlib.Path private_key_data = key_file.read_text() else: private_key_url = "https://github.com/pulp/pulp-fixtures/raw/master/common/GPG-PRIVATE-KEY-fixture-signing" # noqa: E501 - private_key_data = requests.get(private_key_url).text - key_file.write_text(private_key_data) + with urllib.request.urlopen(private_key_url) as response: + private_key_data = response.read() + key_file.write_bytes(private_key_data) import_result = gpg.import_keys(private_key_data) gpg.trust_keys(import_result.fingerprints[0], "TRUST_ULTIMATE") diff --git a/tests/scripts/pulpcore/test_task.sh b/tests/scripts/pulpcore/test_task.sh index 3ff85c102..6bfc2f63a 100755 --- a/tests/scripts/pulpcore/test_task.sh +++ b/tests/scripts/pulpcore/test_task.sh @@ -51,7 +51,7 @@ fi expect_fail pulp --dry-run task cancel --all # Test waiting for a task -expect_succ pulp --background file repository sync --name "cli_test_core_task_repository" +expect_succ pulp --header X-Task-Diagnostics:memory --background file repository sync --name "cli_test_core_task_repository" task=$(echo "$ERROUTPUT" | grep -E -o "${PULP_API_ROOT}([-_a-zA-Z0-9]+/)?api/v3/tasks/[-[:xdigit:]]*/") task_uuid="${task%/}" task_uuid="${task_uuid##*/}" @@ -66,6 +66,16 @@ expect_succ pulp task list --started-before "21/01/12" --started-after "22/01/06 expect_succ pulp task list --finished-before "2021-12-01" --finished-after "2022-06-01 00:00:00" expect_succ pulp task list --created-resource "$created_resource" +if pulp debug has-plugin --name "core" --specifier ">=3.57.0" +then + # Downloading profile artifacts. + expect_succ pulp task profile-artifact-urls --uuid "${task_uuid}" + expect_succ test "$(echo "$OUTPUT" | jq -r 'keys[]')" = "memory_profile" + + expect_succ pulp task profile-artifact-urls --uuid "${task_uuid}" --download + expect_succ test -f "task_profile-pulp_file.app.tasks.synchronizing.synchronize-${task_uuid}/memory_profile" +fi + if pulp debug has-plugin --name "core" --specifier ">=3.22.0" then # New style task resource filters From 0c90c31491946610082b1d596105dd0e77612fb3 Mon Sep 17 00:00:00 2001 From: Matthias Dellweg Date: Tue, 5 Nov 2024 12:10:18 +0100 Subject: [PATCH 2/5] Properly encode boolean for querystrings --- pulp-glue/pulp_glue/common/schema.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pulp-glue/pulp_glue/common/schema.py b/pulp-glue/pulp_glue/common/schema.py index 72b576bba..be54c15a9 100644 --- a/pulp-glue/pulp_glue/common/schema.py +++ b/pulp-glue/pulp_glue/common/schema.py @@ -42,6 +42,8 @@ def encode_param(value: t.Any) -> t.Any: return value.strftime(ISO_DATETIME_FORMAT) elif isinstance(value, datetime.date): return value.strftime(ISO_DATE_FORMAT) + elif isinstance(value, bool): + return "true" if value else "false" else: return value From 5dbfb07b9d28b4ea5e751188b2844fb47501f1ff Mon Sep 17 00:00:00 2001 From: Matthias Dellweg Date: Tue, 20 Jan 2026 16:23:26 +0100 Subject: [PATCH 3/5] Add request & response abstraction --- pulp-glue/pulp_glue/common/openapi.py | 199 ++++++++++++++---------- pulp-glue/tests/test_openapi_logging.py | 129 +++++++++------ 2 files changed, 204 insertions(+), 124 deletions(-) diff --git a/pulp-glue/pulp_glue/common/openapi.py b/pulp-glue/pulp_glue/common/openapi.py index ffc908c27..ce7e32e99 100644 --- a/pulp-glue/pulp_glue/common/openapi.py +++ b/pulp-glue/pulp_glue/common/openapi.py @@ -1,6 +1,3 @@ -# copyright (c) 2020, Matthias Dellweg -# GNU General Public License v3.0+ (see LICENSE or https://www.gnu.org/licenses/gpl-3.0.txt) - import json import logging import os @@ -13,13 +10,12 @@ import requests import urllib3 -from multidict import CIMultiDict, MutableMultiMapping +from multidict import CIMultiDict, CIMultiDictProxy, MutableMultiMapping from pulp_glue.common import __version__ from pulp_glue.common.exceptions import ( OpenAPIError, PulpAuthenticationFailed, - PulpException, PulpHTTPError, PulpNotAutorized, UnsafeCallError, @@ -37,10 +33,22 @@ SAFE_METHODS = ["GET", "HEAD", "OPTIONS"] +@dataclass +class _Request: + operation_id: str + method: str + url: str + headers: MutableMultiMapping[str] | CIMultiDictProxy[str] | t.MutableMapping[str, str] + params: dict[str, str] | None = None + data: dict[str, t.Any] | str | None = None + files: dict[str, tuple[str, UploadType, str]] | None = None + security: list[dict[str, list[str]]] | None = None + + @dataclass class _Response: status_code: int - headers: MutableMultiMapping[str] | t.MutableMapping[str, str] + headers: MutableMultiMapping[str] | CIMultiDictProxy[str] | t.MutableMapping[str, str] body: bytes @@ -149,7 +157,7 @@ class OpenAPI: auth_provider: Object that returns requests auth objects according to the api spec. cert: Client certificate used for auth. key: Matching key for `cert` if not already included. - verify_ssl: Whether to check server TLS certificates agains a CA (requests semantic). + verify_ssl: Whether to check server TLS certificates agains a CA. refresh_cache: Whether to fetch the api doc regardless. dry_run: Flag to disallow issuing POST, PUT, PATCH or DELETE calls. debug_callback: Callback that will be called with strings useful for logging or debugging. @@ -163,7 +171,7 @@ def __init__( self, base_url: str, doc_path: str, - headers: CIMultiDict[str] | None = None, + headers: CIMultiDict[str] | CIMultiDictProxy[str] | None = None, auth_provider: AuthProviderBase | None = None, cert: str | None = None, key: str | None = None, @@ -178,11 +186,15 @@ def __init__( ): if validate_certs is not None: warnings.warn( - "validate_certs is deprecated; use verify_ssl instead.", DeprecationWarning + "validate_certs is deprecated; use verify_ssl instead.", + DeprecationWarning, ) verify_ssl = validate_certs if safe_calls_only is not None: - warnings.warn("safe_calls_only is deprecated; use dry_run instead.", DeprecationWarning) + warnings.warn( + "safe_calls_only is deprecated; use dry_run instead.", + DeprecationWarning, + ) dry_run = safe_calls_only if debug_callback is not None: warnings.warn( @@ -281,7 +293,7 @@ def _parse_api(self, data: bytes) -> None: raise OpenAPIError(_("Unknown schema version")) self.operations: dict[str, t.Any] = { method_entry["operationId"]: (method, path) - for path, path_entry in self.api_spec["paths"].items() + for path, path_entry in self.api_spec.get("paths", {}).items() for method, method_entry in path_entry.items() if method in {"get", "put", "post", "delete", "options", "head", "patch", "trace"} } @@ -398,7 +410,7 @@ def _render_request_body( ) -> tuple[ str | None, dict[str, t.Any] | str | None, - list[tuple[str, tuple[str, UploadType, str]]] | None, + dict[str, tuple[str, UploadType, str]] | None, ]: content_types: list[str] = [] try: @@ -417,7 +429,7 @@ def _render_request_body( content_type: str | None = None data: dict[str, t.Any] | str | None = None - files: list[tuple[str, tuple[str, UploadType, str]]] | None = None + files: dict[str, tuple[str, UploadType, str]] | None = None candidate_content_types = [ "multipart/form-data", @@ -455,21 +467,20 @@ def _render_request_body( elif content_type.startswith("application/x-www-form-urlencoded"): data = body elif content_type.startswith("multipart/form-data"): - uploads: dict[str, tuple[str, UploadType, str]] = {} data = {} + files = {} # Extract and prepare the files to upload if body: for key, value in body.items(): if isinstance(value, (bytes, BufferedReader)): - uploads[key] = ( - getattr(value, "name", key), + # If available, use the filename. + files[key] = ( + getattr(value, "name", key).split("/")[-1], value, "application/octet-stream", ) else: data[key] = value - if uploads: - files = [(key, upload_data) for key, upload_data in uploads.items()] break else: # No known content-type left @@ -485,7 +496,7 @@ def _render_request_body( return content_type, data, files - def _send_request( + def _render_request( self, path_spec: dict[str, t.Any], method: str, @@ -494,83 +505,118 @@ def _send_request( headers: dict[str, str], body: dict[str, t.Any] | None = None, validate_body: bool = True, - ) -> _Response: + ) -> _Request: method_spec = path_spec[method] + _headers = CIMultiDict(self._headers) + _headers.update(headers) + + security: list[dict[str, list[str]]] | None + if self._auth_provider and "Authorization" not in self._headers: + security = method_spec.get("security", self.api_spec.get("security")) + else: + # No auth required? Don't provide it. + # No auth_provider available? Hope for the best (should do the trick for cert auth). + # Authorization header present? You wanted it that way... + security = None + content_type, data, files = self._render_request_body(method_spec, body, validate_body) - security: list[dict[str, list[str]]] | None = method_spec.get( - "security", self.api_spec.get("security") + # For we encode the json on our side. + # Somehow this does not work properly for multipart... + if content_type is not None and content_type.startswith("application/json"): + _headers["Content-Type"] = content_type + + return _Request( + operation_id=method_spec["operationId"], + method=method, + url=url, + headers=_headers, + params=params, + data=data, + files=files, + security=security, ) - if security and self._auth_provider: + + def _log_request(self, request: _Request) -> None: + if request.params: + qs = urlencode(request.params) + self._debug_callback(1, f"{request.operation_id} : {request.method} {request.url}?{qs}") + self._debug_callback( + 2, + "\n".join([f" {key}=={value}" for key, value in request.params.items()]), + ) + else: + self._debug_callback(1, f"{request.operation_id} : {request.method} {request.url}") + for key, value in request.headers.items(): + self._debug_callback(2, f" {key}: {value}") + if request.data is not None: + self._debug_callback(3, f"{request.data!r}") + if request.files is not None: + for key, (name, _dummy, content_type) in request.files.items(): + self._debug_callback(3, f"{key} <- {name} [{content_type}]") + + def _send_request( + self, + request: _Request, + ) -> _Response: + # This function uses requests to translate the _Request into a _Response. + if request.security and self._auth_provider: if "Authorization" in self._session.headers: # Bad idea, but you wanted it that way. auth = None else: - auth = self._auth_provider(security, self.api_spec["components"]["securitySchemes"]) + auth = self._auth_provider( + request.security, self.api_spec["components"]["securitySchemes"] + ) else: # No auth required? Don't provide it. # No auth_provider available? Hope for the best (should do the trick for cert auth). auth = None - # For we encode the json on our side. - # Somehow this does not work properly for multipart... - if content_type is not None and content_type.startswith("application/json"): - headers["content-type"] = content_type - request = self._session.prepare_request( - requests.Request( - method, - url, + try: + r = self._session.request( + request.method, + request.url, + params=request.params, + headers=request.headers, + data=request.data, + files=request.files, auth=auth, - params=params, - headers=headers, - data=data, - files=files, - ) - ) - if content_type: - assert request.headers["content-type"].startswith(content_type), ( - f"{request.headers['content-type']} != {content_type}" ) - for key, value in request.headers.items(): - self._debug_callback(2, f" {key}: {value}") - if request.body is not None: - self._debug_callback(3, f"{request.body!r}") - if self._dry_run and method.upper() not in SAFE_METHODS: - raise UnsafeCallError(_("Call aborted due to safe mode")) - try: - response = self._session.send(request) + response = _Response(status_code=r.status_code, headers=r.headers, body=r.content) except requests.TooManyRedirects as e: assert e.response is not None raise OpenAPIError( - _("Received redirect to '{url}'. Please check your CLI configuration.").format( - url=e.response.headers["location"] + _( + "Received redirect to '{new_url} from {old_url}'." + " Please check your configuration." + ).format( + new_url=e.response.headers["location"], + old_url=request.url, ) ) except requests.RequestException as e: raise OpenAPIError(str(e)) + + return response + + def _log_response(self, response: _Response) -> None: self._debug_callback( 1, _("Response: {status_code}").format(status_code=response.status_code) ) for key, value in response.headers.items(): self._debug_callback(2, f" {key}: {value}") - if response.text: - self._debug_callback(3, f"{response.text}") + if response.body: + self._debug_callback(3, f"{response.body!r}") + + def _parse_response(self, method_spec: dict[str, t.Any], response: _Response) -> t.Any: if "Correlation-Id" in response.headers: self._set_correlation_id(response.headers["Correlation-Id"]) if response.status_code == 401: raise PulpAuthenticationFailed(method_spec["operationId"]) - if response.status_code == 403: + elif response.status_code == 403: raise PulpNotAutorized(method_spec["operationId"]) - try: - response.raise_for_status() - except requests.HTTPError as e: - if e.response is not None: - raise PulpHTTPError(str(e.response.text), e.response.status_code) - else: - raise PulpException(str(e)) - return _Response( - status_code=response.status_code, headers=response.headers, body=response.content - ) + elif response.status_code >= 300: + raise PulpHTTPError(response.body.decode(), response.status_code) - def _parse_response(self, method_spec: dict[str, t.Any], response: _Response) -> t.Any: if response.status_code == 204: return {} @@ -613,8 +659,8 @@ def call( The JSON decoded server response if any. Raises: - OpenAPIValidationError: on failed input validation (no request was sent to the server). - requests.HTTPError: on failures related to the HTTP call made. + ValidationError: on failed input validation (no request was sent to the server). + OpenAPIError: on failures related to the HTTP call made. """ method, path = self.operations[operation_id] path_spec = self.api_spec["paths"][path] @@ -643,17 +689,7 @@ def call( ) url = urljoin(self._base_url, path) - if query_params: - qs = urlencode(query_params) - log_msg = f"{operation_id} : {method} {url}?{qs}" - else: - log_msg = f"{operation_id} : {method} {url}" - self._debug_callback(1, log_msg) - self._debug_callback( - 2, "\n".join([f" {key}=={value}" for key, value in query_params.items()]) - ) - - response = self._send_request( + request = self._render_request( path_spec, method, url, @@ -662,5 +698,12 @@ def call( body, validate_body=validate_body, ) + self._log_request(request) + + if self._dry_run and request.method.upper() not in SAFE_METHODS: + raise UnsafeCallError(_("Call aborted due to safe mode")) + + response = self._send_request(request) + self._log_response(response) return self._parse_response(method_spec, response) diff --git a/pulp-glue/tests/test_openapi_logging.py b/pulp-glue/tests/test_openapi_logging.py index 6f5cb2ca8..91200fb23 100644 --- a/pulp-glue/tests/test_openapi_logging.py +++ b/pulp-glue/tests/test_openapi_logging.py @@ -1,67 +1,104 @@ import json import logging -import typing as t import pytest -from pulp_glue.common.openapi import OpenAPI +from pulp_glue.common.openapi import OpenAPI, _Request, _Response + +pytestmark = pytest.mark.glue TEST_SCHEMA = json.dumps( { "openapi": "3.0.3", - "paths": {"test/": {"get": {"operationId": "test_id", "responses": {200: {}}}}}, - "components": {"schemas": {}}, + "paths": { + "test/": { + "get": {"operationId": "get_test_id", "responses": {200: {}}}, + "post": { + "operationId": "post_test_id", + "requestBody": { + "required": True, + "content": { + "application/json": { + "schema": {"$ref": "#/components/schemas/testBody"} + } + }, + }, + "responses": {200: {}}, + }, + } + }, + "components": { + "schemas": { + "testBody": { + "type": "object", + "properties": {"text": {"type": "string"}}, + "required": ["text"], + } + } + }, } ).encode() -class MockRequest: - headers: dict[str, str] = {} - body: dict[str, t.Any] = {} - - -class MockResponse: - status_code = 200 - headers: dict[str, str] = {} - text = "{}" - content: dict[str, t.Any] = {} - - def raise_for_status(self) -> None: - pass - - -class MockSession: - def prepare_request(self, *args: t.Any, **kwargs: t.Any) -> MockRequest: - return MockRequest() - - def send(self, request: MockRequest) -> MockResponse: - return MockResponse() +def mock_send_request(request: _Request) -> _Response: + return _Response(status_code=200, headers={}, body=b"{}") @pytest.fixture -def openapi(monkeypatch: pytest.MonkeyPatch) -> OpenAPI: +def mock_openapi(monkeypatch: pytest.MonkeyPatch) -> OpenAPI: monkeypatch.setattr(OpenAPI, "load_api", lambda self, refresh_cache: TEST_SCHEMA) - openapi = OpenAPI("base_url", "doc_path") + openapi = OpenAPI("base_url", "doc_path", user_agent="test agent") openapi._parse_api(TEST_SCHEMA) - monkeypatch.setattr(openapi, "_session", MockSession()) + monkeypatch.setattr( + openapi, + "_send_request", + mock_send_request, + ) return openapi -def test_openapi_logs_nothing_from_info(openapi: OpenAPI, caplog: pytest.LogCaptureFixture) -> None: - caplog.set_level(logging.INFO) - openapi.call("test_id") - assert caplog.record_tuples == [] - - -def test_openapi_logs_operation_info_to_debug( - openapi: OpenAPI, caplog: pytest.LogCaptureFixture -) -> None: - caplog.set_level(logging.DEBUG) - openapi.call("test_id") - assert caplog.record_tuples == [ - ("pulp_glue.openapi", logging.DEBUG + 3, "test_id : get test/"), - ("pulp_glue.openapi", logging.DEBUG + 2, ""), - ("pulp_glue.openapi", logging.DEBUG + 1, "{}"), - ("pulp_glue.openapi", logging.DEBUG + 3, "Response: 200"), - ("pulp_glue.openapi", logging.DEBUG + 1, "{}"), - ] +class TestOpenAPILogs: + def test_nothing_with_level_info( + self, + mock_openapi: OpenAPI, + caplog: pytest.LogCaptureFixture, + ) -> None: + caplog.set_level(logging.INFO) + mock_openapi.call("get_test_id") + assert caplog.record_tuples == [] + + def test_get_operation_to_debug( + self, + mock_openapi: OpenAPI, + caplog: pytest.LogCaptureFixture, + ) -> None: + caplog.set_level(logging.DEBUG, logger="pulp_glue.openapi") + mock_openapi.call("get_test_id") + assert caplog.record_tuples == [ + ("pulp_glue.openapi", logging.DEBUG + 3, "get_test_id : get test/"), + ("pulp_glue.openapi", logging.DEBUG + 2, " User-Agent: test agent"), + ("pulp_glue.openapi", logging.DEBUG + 2, " Accept: application/json"), + ("pulp_glue.openapi", logging.DEBUG + 3, "Response: 200"), + ("pulp_glue.openapi", logging.DEBUG + 1, "b'{}'"), + ] + + def test_post_operation_to_debug( + self, + mock_openapi: OpenAPI, + caplog: pytest.LogCaptureFixture, + ) -> None: + caplog.set_level(logging.DEBUG, logger="pulp_glue.openapi") + mock_openapi.call("post_test_id", body={"text": "Trace"}) + assert caplog.record_tuples == [ + ("pulp_glue.openapi", logging.DEBUG + 3, "post_test_id : post test/"), + ("pulp_glue.openapi", logging.DEBUG + 2, " User-Agent: test agent"), + ("pulp_glue.openapi", logging.DEBUG + 2, " Accept: application/json"), + ( + "pulp_glue.openapi", + logging.DEBUG + 2, + " Content-Type: application/json", + ), + ("pulp_glue.openapi", logging.DEBUG + 1, '\'{"text": "Trace"}\''), + ("pulp_glue.openapi", logging.DEBUG + 3, "Response: 200"), + ("pulp_glue.openapi", logging.DEBUG + 1, "b'{}'"), + ] From 2a0f1d66897ecce9dff13577114459d09ce70497 Mon Sep 17 00:00:00 2001 From: Matthias Dellweg Date: Thu, 22 Jan 2026 15:26:54 +0100 Subject: [PATCH 4/5] Restructure openapi unittests --- pulp-glue/tests/test_openapi.py | 190 +++++++++++++++++------- pulp-glue/tests/test_openapi_logging.py | 104 ------------- 2 files changed, 136 insertions(+), 158 deletions(-) delete mode 100644 pulp-glue/tests/test_openapi_logging.py diff --git a/pulp-glue/tests/test_openapi.py b/pulp-glue/tests/test_openapi.py index 41b53a346..afa871c68 100644 --- a/pulp-glue/tests/test_openapi.py +++ b/pulp-glue/tests/test_openapi.py @@ -1,81 +1,163 @@ import datetime import json +import logging import pytest -from pulp_glue.common.openapi import OpenAPI, _Response +from pulp_glue.common.openapi import OpenAPI, _Request, _Response pytestmark = pytest.mark.glue TEST_SCHEMA = json.dumps( { "openapi": "3.0.3", - "paths": {"test/": {"get": {"operationId": "test_id"}}}, - "components": {"schemas": {}}, + "paths": { + "test/": { + "get": {"operationId": "get_test_id", "responses": {200: {}}}, + "post": { + "operationId": "post_test_id", + "requestBody": { + "required": True, + "content": { + "application/json": { + "schema": {"$ref": "#/components/schemas/testBody"} + } + }, + }, + "responses": {200: {}}, + }, + } + }, + "components": { + "schemas": { + "testBody": { + "type": "object", + "properties": {"text": {"type": "string"}}, + "required": ["text"], + } + } + }, } ).encode() +def mock_send_request(request: _Request) -> _Response: + return _Response(status_code=200, headers={}, body=b"{}") + + @pytest.fixture -def openapi(monkeypatch: pytest.MonkeyPatch) -> OpenAPI: +def mock_openapi(monkeypatch: pytest.MonkeyPatch) -> OpenAPI: monkeypatch.setattr(OpenAPI, "load_api", lambda self, refresh_cache: TEST_SCHEMA) - openapi = OpenAPI("base_url", "doc_path") + openapi = OpenAPI("base_url", "doc_path", user_agent="test agent") openapi._parse_api(TEST_SCHEMA) + monkeypatch.setattr(openapi, "_send_request", mock_send_request) return openapi -def test_parse_response_returns_dict_for_no_content(openapi: OpenAPI) -> None: - response = _Response(204, {}, b"") - result = openapi._parse_response({}, response) - assert result == {} - - -def test_parse_response_decodes_json(openapi: OpenAPI) -> None: - response = _Response(200, {"content-type": "application/json"}, b'{"a": 1, "b": "Hallo!"}') - result = openapi._parse_response( - {"responses": {"200": {"content": {"application/json": {}}}}}, response - ) - assert result == {"a": 1, "b": "Hallo!"} - - -def test_extract_params_with_no_match(openapi: OpenAPI) -> None: - parameters = {"a": 1, "b": "C"} - query_params = openapi._extract_params("query", {}, {}, parameters) - assert query_params == {} - assert parameters == {"a": 1, "b": "C"} +class TestParseResponse: + def test_returns_dict_for_no_content(self, mock_openapi: OpenAPI) -> None: + response = _Response(204, {}, b"") + result = mock_openapi._parse_response({}, response) + assert result == {} - -def test_extract_params_raises_for_missing_required_parameter(openapi: OpenAPI) -> None: - parameters = {"a": 1, "b": "C"} - with pytest.raises(RuntimeError, match=r"Required parameters \[c\] missing"): - openapi._extract_params( + def test_decodes_json(self, mock_openapi: OpenAPI) -> None: + response = _Response(200, {"content-type": "application/json"}, b'{"a": 1, "b": "Hallo!"}') + result = mock_openapi._parse_response( + {"responses": {"200": {"content": {"application/json": {}}}}}, response + ) + assert result == {"a": 1, "b": "Hallo!"} + + +class TestExtractParams: + def test_with_no_match(self, mock_openapi: OpenAPI) -> None: + parameters = {"a": 1, "b": "C"} + query_params = mock_openapi._extract_params("query", {}, {}, parameters) + assert query_params == {} + assert parameters == {"a": 1, "b": "C"} + + def test_raises_for_missing_required_parameter(self, mock_openapi: OpenAPI) -> None: + parameters = {"a": 1, "b": "C"} + with pytest.raises(RuntimeError, match=r"Required parameters \[c\] missing"): + mock_openapi._extract_params( + "query", + {"parameters": [{"name": "c", "in": "query", "required": True}]}, + {}, + parameters, + ) + assert parameters == {"a": 1, "b": "C"} + + def test_removes_matches(self, mock_openapi: OpenAPI) -> None: + parameters = {"a": 1, "c": "C"} + query_params = mock_openapi._extract_params( "query", {"parameters": [{"name": "c", "in": "query", "required": True}]}, {}, parameters, ) - assert parameters == {"a": 1, "b": "C"} - - -def test_extract_params_removes_matches(openapi: OpenAPI) -> None: - parameters = {"a": 1, "c": "C"} - query_params = openapi._extract_params( - "query", {"parameters": [{"name": "c", "in": "query", "required": True}]}, {}, parameters - ) - assert query_params == {"c": "C"} - assert parameters == {"a": 1} - - -def test_extract_params_encodes_date(openapi: OpenAPI) -> None: - parameters = {"a": datetime.date(2000, 1, 1)} - query_params = openapi._extract_params( - "query", - { - "parameters": [ - {"name": "a", "in": "query", "schema": {"type": "string", "format": "date"}} - ] - }, - {}, - parameters, - ) - assert query_params == {"a": "2000-01-01"} + assert query_params == {"c": "C"} + assert parameters == {"a": 1} + + def test_encodes_date(self, mock_openapi: OpenAPI) -> None: + parameters = {"a": datetime.date(2000, 1, 1)} + query_params = mock_openapi._extract_params( + "query", + { + "parameters": [ + { + "name": "a", + "in": "query", + "schema": {"type": "string", "format": "date"}, + } + ] + }, + {}, + parameters, + ) + assert query_params == {"a": "2000-01-01"} + + +class TestOpenAPILogs: + def test_nothing_with_level_info( + self, + mock_openapi: OpenAPI, + caplog: pytest.LogCaptureFixture, + ) -> None: + caplog.set_level(logging.INFO) + mock_openapi.call("get_test_id") + assert caplog.record_tuples == [] + + def test_get_operation_to_debug( + self, + mock_openapi: OpenAPI, + caplog: pytest.LogCaptureFixture, + ) -> None: + caplog.set_level(logging.DEBUG, logger="pulp_glue.openapi") + mock_openapi.call("get_test_id") + assert caplog.record_tuples == [ + ("pulp_glue.openapi", logging.DEBUG + 3, "get_test_id : get test/"), + ("pulp_glue.openapi", logging.DEBUG + 2, " User-Agent: test agent"), + ("pulp_glue.openapi", logging.DEBUG + 2, " Accept: application/json"), + ("pulp_glue.openapi", logging.DEBUG + 3, "Response: 200"), + ("pulp_glue.openapi", logging.DEBUG + 1, "b'{}'"), + ] + + def test_post_operation_to_debug( + self, + mock_openapi: OpenAPI, + caplog: pytest.LogCaptureFixture, + ) -> None: + caplog.set_level(logging.DEBUG, logger="pulp_glue.openapi") + mock_openapi.call("post_test_id", body={"text": "Trace"}) + assert caplog.record_tuples == [ + ("pulp_glue.openapi", logging.DEBUG + 3, "post_test_id : post test/"), + ("pulp_glue.openapi", logging.DEBUG + 2, " User-Agent: test agent"), + ("pulp_glue.openapi", logging.DEBUG + 2, " Accept: application/json"), + ( + "pulp_glue.openapi", + logging.DEBUG + 2, + " Content-Type: application/json", + ), + ("pulp_glue.openapi", logging.DEBUG + 1, '\'{"text": "Trace"}\''), + ("pulp_glue.openapi", logging.DEBUG + 3, "Response: 200"), + ("pulp_glue.openapi", logging.DEBUG + 1, "b'{}'"), + ] diff --git a/pulp-glue/tests/test_openapi_logging.py b/pulp-glue/tests/test_openapi_logging.py deleted file mode 100644 index 91200fb23..000000000 --- a/pulp-glue/tests/test_openapi_logging.py +++ /dev/null @@ -1,104 +0,0 @@ -import json -import logging - -import pytest - -from pulp_glue.common.openapi import OpenAPI, _Request, _Response - -pytestmark = pytest.mark.glue - -TEST_SCHEMA = json.dumps( - { - "openapi": "3.0.3", - "paths": { - "test/": { - "get": {"operationId": "get_test_id", "responses": {200: {}}}, - "post": { - "operationId": "post_test_id", - "requestBody": { - "required": True, - "content": { - "application/json": { - "schema": {"$ref": "#/components/schemas/testBody"} - } - }, - }, - "responses": {200: {}}, - }, - } - }, - "components": { - "schemas": { - "testBody": { - "type": "object", - "properties": {"text": {"type": "string"}}, - "required": ["text"], - } - } - }, - } -).encode() - - -def mock_send_request(request: _Request) -> _Response: - return _Response(status_code=200, headers={}, body=b"{}") - - -@pytest.fixture -def mock_openapi(monkeypatch: pytest.MonkeyPatch) -> OpenAPI: - monkeypatch.setattr(OpenAPI, "load_api", lambda self, refresh_cache: TEST_SCHEMA) - openapi = OpenAPI("base_url", "doc_path", user_agent="test agent") - openapi._parse_api(TEST_SCHEMA) - monkeypatch.setattr( - openapi, - "_send_request", - mock_send_request, - ) - return openapi - - -class TestOpenAPILogs: - def test_nothing_with_level_info( - self, - mock_openapi: OpenAPI, - caplog: pytest.LogCaptureFixture, - ) -> None: - caplog.set_level(logging.INFO) - mock_openapi.call("get_test_id") - assert caplog.record_tuples == [] - - def test_get_operation_to_debug( - self, - mock_openapi: OpenAPI, - caplog: pytest.LogCaptureFixture, - ) -> None: - caplog.set_level(logging.DEBUG, logger="pulp_glue.openapi") - mock_openapi.call("get_test_id") - assert caplog.record_tuples == [ - ("pulp_glue.openapi", logging.DEBUG + 3, "get_test_id : get test/"), - ("pulp_glue.openapi", logging.DEBUG + 2, " User-Agent: test agent"), - ("pulp_glue.openapi", logging.DEBUG + 2, " Accept: application/json"), - ("pulp_glue.openapi", logging.DEBUG + 3, "Response: 200"), - ("pulp_glue.openapi", logging.DEBUG + 1, "b'{}'"), - ] - - def test_post_operation_to_debug( - self, - mock_openapi: OpenAPI, - caplog: pytest.LogCaptureFixture, - ) -> None: - caplog.set_level(logging.DEBUG, logger="pulp_glue.openapi") - mock_openapi.call("post_test_id", body={"text": "Trace"}) - assert caplog.record_tuples == [ - ("pulp_glue.openapi", logging.DEBUG + 3, "post_test_id : post test/"), - ("pulp_glue.openapi", logging.DEBUG + 2, " User-Agent: test agent"), - ("pulp_glue.openapi", logging.DEBUG + 2, " Accept: application/json"), - ( - "pulp_glue.openapi", - logging.DEBUG + 2, - " Content-Type: application/json", - ), - ("pulp_glue.openapi", logging.DEBUG + 1, '\'{"text": "Trace"}\''), - ("pulp_glue.openapi", logging.DEBUG + 3, "Response: 200"), - ("pulp_glue.openapi", logging.DEBUG + 1, "b'{}'"), - ] From 1a541742d47f29f508e2fa4954d7466377af4bae Mon Sep 17 00:00:00 2001 From: Matthias Dellweg Date: Tue, 27 Jan 2026 16:48:43 +0100 Subject: [PATCH 5/5] Fix query parameter test to run in parallel --- pulpcore/cli/file/repository.py | 6 +++++- tests/scripts/pulpcore/test_query_params.sh | 20 ++++++++++---------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/pulpcore/cli/file/repository.py b/pulpcore/cli/file/repository.py index 114de753f..4d412d9e7 100644 --- a/pulpcore/cli/file/repository.py +++ b/pulpcore/cli/file/repository.py @@ -153,7 +153,11 @@ def repository(ctx: click.Context, pulp_ctx: PulpCLIContext, /, repo_type: str) ), ] -repository.add_command(list_command(decorators=[label_select_option])) +repository.add_command( + list_command( + decorators=[label_select_option, click.option("--name-startswith", "name__startswith")] + ) +) repository.add_command(show_command(decorators=lookup_options)) repository.add_command(create_command(decorators=create_options)) repository.add_command(update_command(decorators=lookup_options + update_options)) diff --git a/tests/scripts/pulpcore/test_query_params.sh b/tests/scripts/pulpcore/test_query_params.sh index 41f22272c..cb8463dbe 100755 --- a/tests/scripts/pulpcore/test_query_params.sh +++ b/tests/scripts/pulpcore/test_query_params.sh @@ -7,29 +7,29 @@ set -eu pulp debug has-plugin --name "file" || exit 23 cleanup() { - pulp file repository destroy --name aaaaaaaaaa || true - pulp file repository destroy --name bbbbbbbbbb || true - pulp file repository destroy --name cccccccccc || true + pulp file repository destroy --name query_params_aaaaaaaaaa || true + pulp file repository destroy --name query_params_bbbbbbbbbb || true + pulp file repository destroy --name query_params_cccccccccc || true } trap cleanup EXIT -expect_succ pulp file repository create --name aaaaaaaaaa -expect_succ pulp file repository create --name bbbbbbbbbb -expect_succ pulp file repository create --name cccccccccc +expect_succ pulp file repository create --name query_params_aaaaaaaaaa +expect_succ pulp file repository create --name query_params_bbbbbbbbbb +expect_succ pulp file repository create --name query_params_cccccccccc -expect_succ pulp file repository list --ordering -name +expect_succ pulp file repository list --ordering -name --name-startswith query_params if ! (echo "$OUTPUT" | jq -r .[].name | sort -r -C -); then echo -e "Ordered results are not in a reverse alphabetical order.\n$(echo "$OUTPUT" | jq -r .[].name)" exit 1 fi -expect_succ pulp file repository list --ordering name +expect_succ pulp file repository list --ordering name --name-startswith query_params if ! (echo "$OUTPUT" | jq -r .[].name | sort -C -); then echo -e "Ordered results are not in an alphabetical order.\n$(echo "$OUTPUT" | jq -r .[].name)" exit 1 fi -expect_succ pulp file repository list --field name --field remote +expect_succ pulp file repository list --field name --field remote --name-startswith query_params SELECTED_FIELDS=$(echo "$OUTPUT" | jq -r ".[] | keys[]" | sort -u | tr "\n" " " | xargs) EXPECTED_FIELDS="name remote" if [[ "$SELECTED_FIELDS" != "$EXPECTED_FIELDS" ]]; then @@ -37,7 +37,7 @@ if [[ "$SELECTED_FIELDS" != "$EXPECTED_FIELDS" ]]; then exit 1 fi -expect_succ pulp file repository list --exclude-field name --exclude-field remote +expect_succ pulp file repository list --exclude-field name --exclude-field remote --name-startswith query_params EXCLUDED_FIELDS=$(echo "$OUTPUT" | jq -r ".[] | keys[]" | tr "\n" " " ) if [[ "$EXCLUDED_FIELDS" == *"name"* ]]; then