diff --git a/CHANGELOG.md b/CHANGELOG.md index d4e6c3b6..db246985 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ ### Changed +- The contract declares four things the service already sent (#994, API contract 1.80.0, description only). `X-Provider-Key` is a header parameter of the five operations that call a provider (`previewBuild`, `createBuild`, `submitBuild`, `getProviderStatus`, `testProviderConnection`) — it was mentioned only in `info.description`, so a client's drift test had nothing to check it against. The 429 `auth_throttled` and the overload 503 are shared responses any operation can answer (`components.responses.AuthThrottled`, `ServerOverloaded`), and `X-Request-ID` is `components.headers.RequestId`. `tests/unit/test_contract_shared_declarations.py` compares each with what the code sends. Generating the route table from the route adapter, the remaining part of #994, is not done. - The bearer token Builder accepts is documented as what Studio sends: the IdP's access token (kpubdata-studio#722). `service/auth.py`, `docs/deploy.md` and ADR 0015 said "ID token", while Studio has always sent the Keycloak access token and Builder never looked at the token's kind — only its issuer, an `aud` that includes `OIDC_AUDIENCE`, expiry, subject and `email_verified`. The access token is now the stated contract. `docs/deploy.md` gains the two realm settings that make a Keycloak access token acceptable — an audience mapper for the Builder audience and the `email` client scope — and tests pin that a default realm's token (audience `account`) is refused, one with the mapper is accepted, and one without `email_verified` is refused. No verification code changed. - Four documents describe the service as it is (#1001). `BUILD_STATE.md` §8 no longer calls the async job state machine a future plan outside the contract; the contract's `info.description` no longer says async and publish endpoints are excluded; `API_CONTRACT.md` lists the preflight methods and headers the server sends (`GET, POST, PUT, DELETE, OPTIONS`; `X-Provider-Key` and `X-Publish-Credential` among the headers); and `API_CONTRACT.md` states by deployment mode whether a provider key is stored — not stored in a multi-user deployment, stored encrypted in a single-user one. No behaviour or schema changed, so the contract version stays. - The package metadata names its repository (kpubdata#791): `[project.urls]` gives the homepage, documentation, repository, issue tracker and changelog under `kpubdata-lab/kpubdata-builder`. There was no `[project.urls]`. diff --git a/contract/builder-api.yaml b/contract/builder-api.yaml index 79d6bda4..62757f7c 100644 --- a/contract/builder-api.yaml +++ b/contract/builder-api.yaml @@ -1,7 +1,7 @@ openapi: 3.1.0 info: title: KPubData Builder Service API - version: "1.79.0" + version: "1.80.0" description: >- KPubData Builder(패키지 `kpubdata-builder`) 서비스 계약. BuildSpec 검증, preview, build 실행, artifact 조회, 계약 버전 조회를 제공한다. CLI와 HTTP service mode(#36)가 동일한 도메인 계약을 @@ -20,6 +20,7 @@ info: 추가한다(#504, additive). v1.8.0은 stable principal별 encrypted Provider credential CRUD와 Provider status/test API를 추가한다(#492, additive). v1.9.0은 `/query` 응답에 child startup과 Polars engine 실행 시간을 추가한다(#523, additive). + v1.80.0 declares what the service already sends (#994, description only — the wire is unchanged): the `X-Provider-Key` request header as a parameter of the five operations that call a provider (`previewBuild`, `createBuild`, `submitBuild`, `getProviderStatus`, `testProviderConnection`), the 429 `auth_throttled` and the overload 503 as shared responses any operation can answer (`components.responses.AuthThrottled`, `ServerOverloaded`), and the `X-Request-ID` response header (`components.headers.RequestId`). v1.79.0 gives four failures a stable `code` (#1000, additive): the full async build queue's 429 is `build_queue_full` (it was told from 429 `auth_throttled` only by its sentence), an authentication failure is `unauthorized`, `token_expired` (the bearer token's `exp` has passed — get a new token) or, with 503, `auth_unavailable` (the JWKS could not be fetched), and the overload 503 written before a request is read is `server_overloaded`. Each body keeps its `error` text. v1.78.0 lets the owner read a run a restart interrupted (#996, additive): `GET /builds/{run_id}` answers such a run as `failed` with the new optional `BuildJob.code` `credentials_required` and the reason in `error`, and `GET /builds/{run_id}/events` shows its `run_failed` event, where both answered 404 before; another user still gets what a missing run gets. The submitter of an async run is now recorded with its events, so this holds for runs submitted from this version on. v1.77.0 stops a snapshot profile's range from disclosing a single record's value (#903, additive): `ColumnRange.min`/`max` are read after removing the `range_trim` lowest and the `range_trim` highest values, the range's `status` is `trimmed` instead of `exact`, and it carries `trimmed_count`. `SnapshotProfile` gains `range_trim`, and `min_range_values` is at least `2 * range_trim + 1`. A client that showed only `exact` ranges shows none until it reads `trimmed`. @@ -288,6 +289,7 @@ paths: operationId: getProviderStatus summary: 현재 principal credential로 Provider 연결 상태 확인 parameters: + - $ref: "#/components/parameters/ProviderKey" - $ref: "#/components/parameters/ProviderName" responses: "200": @@ -336,6 +338,7 @@ paths: operationId: testProviderConnection summary: 현재 principal credential로 lightweight connection test 실행 parameters: + - $ref: "#/components/parameters/ProviderKey" - $ref: "#/components/parameters/ProviderName" responses: "200": @@ -682,6 +685,8 @@ paths: post: operationId: previewBuild summary: 각 소스의 스키마와 샘플 행 산출 (파일 미기록, 동기식) + parameters: + - $ref: "#/components/parameters/ProviderKey" requestBody: required: true content: @@ -805,6 +810,8 @@ paths: post: operationId: createBuild summary: 파이프라인 실행 및 결과 반환 (동기식) + parameters: + - $ref: "#/components/parameters/ProviderKey" requestBody: required: true content: @@ -1376,6 +1383,8 @@ paths: 제출한 principal 의 업로드를 읽고, 다른 소유자의 `upload_id` 나 안정적인 소유자가 없는 요청은 job 이 `failed` 로 끝나며 그 source 의 `error` 에 사유가 적힌다. 제출 시점(202)에는 업로드를 확인하지 않는다. + parameters: + - $ref: "#/components/parameters/ProviderKey" requestBody: required: true content: @@ -4162,7 +4171,57 @@ components: each operation: an operation has one 403 response, and many already use it for their own refusal, whose `Error.code` tells the two apart. + headers: + RequestId: + description: >- + The id this request is logged under (#994). Sent on every response a handler + writes — success or error — so it can be quoted when reporting a problem; + absent only from the overload 503, which is written before a request is read. + Readable from a cross-origin page (`Access-Control-Expose-Headers`, #995). + schema: + type: string + responses: + AuthThrottled: + x-status: 429 + description: >- + Too many failed authentication attempts from this client address (#994) — any + operation can answer this before it runs. `retry_after_seconds` says how long + the window has left. `x-status` names the status code, since no single + operation references this response. + content: + application/json: + schema: + $ref: "#/components/schemas/Error" + examples: + AuthThrottled: + summary: The failure limit for this client address was reached + value: + error: too many failed authentication attempts + code: auth_throttled + retry_after_seconds: 42 + ServerOverloaded: + x-status: 503 + description: >- + Too many requests are in flight (#994, #1000). The connection is answered + before the request is read and closed; `Retry-After` says when to try again. + Any operation can answer this. `x-status` names the status code, since no + single operation references this response. + headers: + Retry-After: + description: Seconds to wait before sending the request again. + schema: + type: string + content: + application/json: + schema: + $ref: "#/components/schemas/Error" + examples: + ServerOverloaded: + summary: The request was refused before it was read + value: + error: server overloaded + code: server_overloaded SignupNotApproved: x-status: 403 description: >- @@ -4247,6 +4306,20 @@ components: reason: "the dataset declares redistribution: forbidden" parameters: + ProviderKey: + name: X-Provider-Key + in: header + required: false + description: >- + Multi-user deployment only (#683): the requester's own provider key for this + request or the job it submits, `=`, repeated or comma-separated. + Held in memory until the request ends, or the async job ends, is cancelled or + its TTL passes; never stored, logged or returned. A malformed header answers + 400 `invalid_provider_key` on any route. A single-user deployment ignores it + and keeps its stored and server keys. Declared on the operations that call a + provider (#994). + schema: + type: string PublishCredential: name: X-Publish-Credential in: header diff --git a/contract/fixtures/responses.json b/contract/fixtures/responses.json index 2ff29d91..adc490c1 100644 --- a/contract/fixtures/responses.json +++ b/contract/fixtures/responses.json @@ -1,6 +1,6 @@ { "fixture_format": 1, - "contract_version": "1.79.0", + "contract_version": "1.80.0", "probe_field": "future_optional_field", "rules": "A response may gain optional fields in any minor version; a client ignores fields it does not know. A required field keeps its name and type until the next major version; a client rejects a body whose required field is missing or mistyped.", "fixtures": [ @@ -5669,6 +5669,53 @@ }, "broken_path": "$.error" }, + { + "response": "AuthThrottled", + "status": 429, + "example": "AuthThrottled", + "current": { + "error": "too many failed authentication attempts", + "code": "auth_throttled", + "retry_after_seconds": 42 + }, + "with_additive_fields": { + "error": "too many failed authentication attempts", + "code": "auth_throttled", + "retry_after_seconds": 42, + "future_optional_field": "added by a later minor contract version" + }, + "additive_paths": [ + "$" + ], + "required_type_broken": { + "error": 12345, + "code": "auth_throttled", + "retry_after_seconds": 42 + }, + "broken_path": "$.error" + }, + { + "response": "ServerOverloaded", + "status": 503, + "example": "ServerOverloaded", + "current": { + "error": "server overloaded", + "code": "server_overloaded" + }, + "with_additive_fields": { + "error": "server overloaded", + "code": "server_overloaded", + "future_optional_field": "added by a later minor contract version" + }, + "additive_paths": [ + "$" + ], + "required_type_broken": { + "error": 12345, + "code": "server_overloaded" + }, + "broken_path": "$.error" + }, { "response": "SignupNotApproved", "status": 403, diff --git a/src/kpubdata_builder/service/app.py b/src/kpubdata_builder/service/app.py index c0b21356..2808af38 100644 --- a/src/kpubdata_builder/service/app.py +++ b/src/kpubdata_builder/service/app.py @@ -402,7 +402,9 @@ def _enforce_ownership() -> bool: # 1.78.0 -> 1.79.0: stable codes for the full build queue (429 build_queue_full), # authentication failures (unauthorized, token_expired, auth_unavailable) and the # overload 503 (server_overloaded) (#1000, additive). -API_CONTRACT_VERSION = "1.79.0" +# 1.79.0 -> 1.80.0: the X-Provider-Key parameter, the shared 429 auth_throttled and +# overload 503 responses and the X-Request-ID header are declared (#994, description). +API_CONTRACT_VERSION = "1.80.0" #: manifest status vocabulary (ok/failed/cancelled) → publish status vocabulary diff --git a/tests/unit/test_contract_shared_declarations.py b/tests/unit/test_contract_shared_declarations.py new file mode 100644 index 00000000..edebc564 --- /dev/null +++ b/tests/unit/test_contract_shared_declarations.py @@ -0,0 +1,125 @@ +"""What the service sends on every route is declared in the contract (#994). + +The ``X-Provider-Key`` request header, the 429 ``auth_throttled``, the overload 503 and +the ``X-Request-ID`` response header existed in the service and only in prose in the +contract. These compare the declarations with what the code actually sends. +""" + +from __future__ import annotations + +import json +from pathlib import Path +from typing import Any + +import pytest +import yaml + +from kpubdata_builder.service.http import _overloaded_response +from kpubdata_builder.service.request_credentials import PROVIDER_KEY_HEADER + +_CONTRACT = Path(__file__).resolve().parents[2] / "contract" / "builder-api.yaml" + +#: The operations that call a provider with the requester's key. +_PROVIDER_OPERATIONS = { + "previewBuild", + "createBuild", + "submitBuild", + "getProviderStatus", + "testProviderConnection", +} + + +@pytest.fixture(scope="module") +def contract() -> dict[str, Any]: + loaded: dict[str, Any] = yaml.safe_load(_CONTRACT.read_text(encoding="utf-8")) + return loaded + + +def _operations(contract: dict[str, Any]) -> dict[str, dict[str, Any]]: + return { + operation["operationId"]: operation + for item in contract["paths"].values() + for method, operation in item.items() + if method in ("get", "post", "put", "delete") and "operationId" in operation + } + + +def test_the_provider_key_header_is_a_declared_parameter(contract: dict[str, Any]) -> None: + parameter = contract["components"]["parameters"]["ProviderKey"] + + assert (parameter["name"], parameter["in"], parameter["required"]) == ( + PROVIDER_KEY_HEADER, + "header", + False, + ) + + +def test_it_is_declared_on_exactly_the_operations_that_call_a_provider( + contract: dict[str, Any], +) -> None: + reference = {"$ref": "#/components/parameters/ProviderKey"} + declaring = { + name + for name, operation in _operations(contract).items() + if reference in operation.get("parameters", []) + } + + assert declaring == _PROVIDER_OPERATIONS + + +def test_the_overload_response_is_the_declared_one(contract: dict[str, Any]) -> None: + declared = contract["components"]["responses"]["ServerOverloaded"] + head, _, body = _overloaded_response().partition(b"\r\n\r\n") + headers = dict(line.split(b": ", 1) for line in head.split(b"\r\n")[1:]) + + assert head.startswith(b"HTTP/1.1 %d " % declared["x-status"]) + example = declared["content"]["application/json"]["examples"]["ServerOverloaded"]["value"] + assert json.loads(body) == example + assert set(declared["headers"]) <= {name.decode() for name in headers} + + +def test_the_auth_throttle_response_is_the_declared_one( + contract: dict[str, Any], tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + from kpubdata_builder.service import BuilderService, ServiceResponse, dispatch + + declared = contract["components"]["responses"]["AuthThrottled"] + # The suite runs in dev mode, which skips authentication; this needs it on. + monkeypatch.delenv("KPUBDATA_BUILDER_DEV_MODE", raising=False) + monkeypatch.setenv("KPUBDATA_BUILDER_API_KEY", "the-right-key") + monkeypatch.setenv("KPUBDATA_BUILDER_AUTH_FAILURE_LIMIT", "1") + service = BuilderService(output_root=tmp_path, client_factory=lambda **_kw: None) + + first = dispatch(service, "GET", "/datasets", None, api_key="wrong", client_id="10.0.0.9") + second = dispatch(service, "GET", "/datasets", None, api_key="wrong", client_id="10.0.0.9") + + assert isinstance(first, ServiceResponse) and first.status_code == 401 + assert isinstance(second, ServiceResponse) + assert second.status_code == declared["x-status"] + example = declared["content"]["application/json"]["examples"]["AuthThrottled"]["value"] + assert set(second.body) == set(example) + assert (second.body["error"], second.body["code"]) == (example["error"], example["code"]) + + +def test_the_request_id_header_is_declared_and_sent( + contract: dict[str, Any], tmp_path: Path +) -> None: + import threading + import urllib.request + from http.server import HTTPServer + + from kpubdata_builder.service import BuilderService + from kpubdata_builder.service.http import make_handler + + assert "RequestId" in contract["components"]["headers"] + service = BuilderService(output_root=tmp_path, client_factory=lambda **_kw: None) + server = HTTPServer(("127.0.0.1", 0), make_handler(service)) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + url = f"http://127.0.0.1:{server.server_address[1]}/healthz" + with urllib.request.urlopen(url, timeout=5.0) as response: + assert response.headers["X-Request-ID"] + finally: + server.shutdown() + server.server_close()