diff --git a/pkg-py/pyproject.toml b/pkg-py/pyproject.toml index 5d178596..54112a1e 100644 --- a/pkg-py/pyproject.toml +++ b/pkg-py/pyproject.toml @@ -18,6 +18,7 @@ dependencies = [ "jinja2>=3", "pyarrow>=17", "sqlglot>=25", + "httpx>=0.27", ] [project.urls] @@ -28,7 +29,6 @@ Issues = "https://github.com/posit-dev/commons/issues" tracing = [ "opentelemetry-sdk>=1.39", "opentelemetry-exporter-otlp-json-file", - "httpx>=0.27", ] [dependency-groups] diff --git a/pkg-py/src/commons/_connect.py b/pkg-py/src/commons/_connect.py new file mode 100644 index 00000000..2800ff86 --- /dev/null +++ b/pkg-py/src/commons/_connect.py @@ -0,0 +1,96 @@ +"""A Posit Connect API client, covering what commons needs from Connect. + +Connect gives running content an ephemeral owner-scoped ``CONNECT_API_KEY`` +(``Applications.DefaultAPIKeyEnv``, on by default), so content can act on its +own behalf without a publisher configuring anything. +``pkg-r/R/connect.R`` is its counterpart. +""" + +from __future__ import annotations + +import os +from typing import Any + +import httpx + +__all__ = [ + "ConnectClient", + "ConnectError", + "connect_content_guid", + "is_connect_runtime", +] + + +def is_connect_runtime() -> bool: + """Is this process running as content on Posit Connect?""" + return os.environ.get("POSIT_PRODUCT") == "CONNECT" or bool( + os.environ.get("CONNECT_CONTENT_GUID") + ) + + +def connect_content_guid() -> str: + """This content's GUID, or an empty string when it has none.""" + return os.environ.get("CONNECT_CONTENT_GUID", "") + + +class ConnectError(RuntimeError): + """A Posit Connect API request failed.""" + + def __init__(self, method: str, url: httpx.URL, status_code: int) -> None: + # Named by path rather than by response: the request carries the API + # key in a header, and this message can reach a log or a bug report. + super().__init__( + f"Posit Connect returned {status_code} for {method} {url.path}" + ) + self.status_code = status_code + + +class ConnectClient: + """Authenticated access to one Posit Connect server's v1 API.""" + + def __init__( + self, server: str, api_key: str, *, http: httpx.Client | None = None + ) -> None: + self.server = normalize_server(server) + self.api_key = api_key + self._http = httpx.Client() if http is None else http + + @classmethod + def from_env(cls, *, http: httpx.Client | None = None) -> ConnectClient: + server = os.environ.get("CONNECT_SERVER", "") + api_key = os.environ.get("CONNECT_API_KEY", "") + if not server: + raise ValueError( + "Set the CONNECT_SERVER environment variable to your Posit " + "Connect server URL." + ) + if not api_key: + raise ValueError( + "Set the CONNECT_API_KEY environment variable to a Posit " + "Connect API key." + ) + return cls(server, api_key, http=http) + + def request( + self, + method: str, + *path: str, + params: dict[str, Any] | None = None, + json: Any = None, + ) -> httpx.Response: + """Perform a request against ``/__api__/v1/``.""" + response = self._http.request( + method, + "/".join((self.server, "__api__", "v1", *path)), + params=params, + json=json, + headers={"Authorization": f"Key {self.api_key}"}, + ) + if response.is_error: + raise ConnectError(method, response.request.url, response.status_code) + return response + + +def normalize_server(server: str) -> str: + """Reduce the forms a publisher might paste to one base URL.""" + return server.rstrip("/").removesuffix("/__api__").rstrip("/") diff --git a/pkg-py/tests/test_connect.py b/pkg-py/tests/test_connect.py new file mode 100644 index 00000000..0cb23fae --- /dev/null +++ b/pkg-py/tests/test_connect.py @@ -0,0 +1,203 @@ +"""The Posit Connect API client: runtime detection, credentials, requests. + +commons needs Connect for two things on the write side, turning on content +observability and granting collaborators access to traces. Both go through +this client. ``pkg-r/R/connect.R`` is its counterpart, and the behaviour a +publisher can observe, the variables read, the server forms accepted and the +URLs requests land on, is pinned for both in ``tests/shared/connect.json``. +""" + +from __future__ import annotations + +import json +from typing import Any + +import httpx +import pytest + +from commons._connect import ( + ConnectClient, + ConnectError, + connect_content_guid, + is_connect_runtime, +) + +from ._shared import load_shared_fixture + +SPEC = load_shared_fixture("connect") +ENVIRONMENT: dict[str, str] = SPEC["environment"] +NORMALIZATION: list[dict[str, Any]] = SPEC["server_normalization"]["cases"] +DETECTION: list[dict[str, Any]] = SPEC["runtime_detection"]["cases"] +API_REQUEST: dict[str, Any] = SPEC["api_request"] + +CONNECT_ENV = ( + ENVIRONMENT["product"], + ENVIRONMENT["content_guid"], + ENVIRONMENT["server"], + ENVIRONMENT["api_key"], +) +GUID = "01234567-89ab-cdef-0123-456789abcdef" + + +@pytest.fixture(autouse=True) +def _clean_env(monkeypatch: pytest.MonkeyPatch) -> None: + for name in CONNECT_ENV: + monkeypatch.delenv(name, raising=False) + + +def client_recording( + *, + server: str = "https://connect.example.com", + status: int = 200, + json: object = None, +) -> tuple[ConnectClient, list[httpx.Request]]: + """A client whose requests are captured instead of sent.""" + seen: list[httpx.Request] = [] + + def record(request: httpx.Request) -> httpx.Response: + seen.append(request) + return httpx.Response(status, json=json) + + http = httpx.Client(transport=httpx.MockTransport(record)) + return ConnectClient(server, "the-api-key", http=http), seen + + +def test_the_shared_fixture_carries_the_cases_it_promises() -> None: + # A truncated fixture would collect fewer parametrized cases and the + # suite would still pass, so pin what the tables have to contain. + assert {case["expected"] for case in DETECTION} == {True, False} + assert len(NORMALIZATION) >= 2 + assert len(API_REQUEST["cases"]) >= 1 + + +@pytest.mark.parametrize("case", DETECTION, ids=lambda case: case["name"]) +def test_connect_runtime_detection_matches_the_shared_fixture( + monkeypatch: pytest.MonkeyPatch, case: dict[str, Any] +) -> None: + for name, value in case["env"].items(): + if value is not None: + monkeypatch.setenv(name, value) + + assert is_connect_runtime() is case["expected"] + + +def test_content_guid_is_empty_off_connect() -> None: + assert connect_content_guid() == "" + + +def test_content_guid_comes_from_the_environment( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv(ENVIRONMENT["content_guid"], GUID) + + assert connect_content_guid() == GUID + + +def test_credentials_are_read_from_the_environment( + monkeypatch: pytest.MonkeyPatch, +) -> None: + # Connect gives running content an ephemeral owner-scoped key, which is + # what lets commons act on the content's behalf without configuration. + monkeypatch.setenv(ENVIRONMENT["server"], "https://connect.example.com") + monkeypatch.setenv(ENVIRONMENT["api_key"], "the-api-key") + + client = ConnectClient.from_env() + + assert client.server == "https://connect.example.com" + assert client.api_key == "the-api-key" + + +@pytest.mark.parametrize("case", NORMALIZATION, ids=lambda case: case["name"]) +def test_server_normalization_matches_the_shared_fixture( + monkeypatch: pytest.MonkeyPatch, case: dict[str, Any] +) -> None: + monkeypatch.setenv(ENVIRONMENT["server"], case["configured"]) + monkeypatch.setenv(ENVIRONMENT["api_key"], "the-api-key") + + assert ConnectClient.from_env().server == case["expected"] + + +def test_a_missing_server_names_the_variable_to_set( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv(ENVIRONMENT["api_key"], "the-api-key") + + with pytest.raises(ValueError, match=ENVIRONMENT["server"]): + ConnectClient.from_env() + + +def test_a_missing_api_key_names_the_variable_to_set( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv(ENVIRONMENT["server"], "https://connect.example.com") + + with pytest.raises(ValueError, match=ENVIRONMENT["api_key"]): + ConnectClient.from_env() + + +@pytest.mark.parametrize("case", API_REQUEST["cases"], ids=lambda case: case["name"]) +def test_request_urls_match_the_shared_fixture(case: dict[str, Any]) -> None: + client, seen = client_recording(server=API_REQUEST["server"]) + + client.request("GET", *case["path"]) + + (request,) = seen + assert str(request.url) == case["expected"] + + +def test_requests_carry_the_api_key() -> None: + client, seen = client_recording() + + client.request("GET", "content", GUID) + + (request,) = seen + assert request.headers["Authorization"] == "Key the-api-key" + + +def test_query_parameters_are_sent() -> None: + client, seen = client_recording() + + client.request("GET", "users", params={"prefix": "ada", "page_number": 2}) + + (request,) = seen + assert request.url.params["prefix"] == "ada" + assert request.url.params["page_number"] == "2" + + +def test_a_json_body_is_sent() -> None: + client, seen = client_recording() + + client.request("PATCH", "content", GUID, json={"otel_enabled": True}) + + (request,) = seen + assert json.loads(request.read()) == {"otel_enabled": True} + assert request.headers["Content-Type"] == "application/json" + + +def test_the_decoded_body_is_returned() -> None: + client, _ = client_recording(json={"otel_enabled": False}) + + assert client.request("GET", "content", GUID).json() == {"otel_enabled": False} + + +def test_a_failed_request_raises_with_the_status_and_the_url() -> None: + client, _ = client_recording(status=403, json={"error": "not authorized"}) + + with pytest.raises(ConnectError) as raised: + client.request("GET", "content", GUID) + + assert raised.value.status_code == 403 + assert "403" in str(raised.value) + assert f"/content/{GUID}" in str(raised.value) + + +def test_a_failure_does_not_disclose_the_api_key() -> None: + # The key is an ephemeral owner-scoped credential; a traceback that + # reaches a log or an issue report must not carry it. + client, _ = client_recording(status=500) + + with pytest.raises(ConnectError) as raised: + client.request("GET", "content", GUID) + + assert "the-api-key" not in str(raised.value) + assert "the-api-key" not in repr(raised.value) diff --git a/pkg-py/uv.lock b/pkg-py/uv.lock index 11381217..16ee1ab5 100644 --- a/pkg-py/uv.lock +++ b/pkg-py/uv.lock @@ -205,6 +205,7 @@ source = { editable = "." } dependencies = [ { name = "chatlas" }, { name = "duckdb" }, + { name = "httpx" }, { name = "jinja2" }, { name = "pyarrow" }, { name = "pydantic" }, @@ -216,7 +217,6 @@ dependencies = [ [package.optional-dependencies] tracing = [ - { name = "httpx" }, { name = "opentelemetry-exporter-otlp-json-file" }, { name = "opentelemetry-sdk" }, ] @@ -235,7 +235,7 @@ dev = [ requires-dist = [ { name = "chatlas", specifier = ">=0.22.0" }, { name = "duckdb", specifier = ">=1.0" }, - { name = "httpx", marker = "extra == 'tracing'", specifier = ">=0.27" }, + { name = "httpx", specifier = ">=0.27" }, { name = "jinja2", specifier = ">=3" }, { name = "opentelemetry-exporter-otlp-json-file", marker = "extra == 'tracing'" }, { name = "opentelemetry-sdk", marker = "extra == 'tracing'", specifier = ">=1.39" }, diff --git a/pkg-r/tests/testthat/fixtures/shared/connect.json b/pkg-r/tests/testthat/fixtures/shared/connect.json new file mode 100644 index 00000000..c66bdc35 --- /dev/null +++ b/pkg-r/tests/testthat/fixtures/shared/connect.json @@ -0,0 +1,97 @@ +{ + "description": "Posit Connect client contract shared by pkg-r and pkg-py. The source is tests/shared/connect.json; the copy under pkg-r/tests/testthat/fixtures/shared/ is generated by scripts/sync-shared-fixtures.sh. Edit the source and re-run that script.", + "environment": { + "description": "Environment variables the client reads. A publisher or Connect itself sets these, so both packages must read the same names.", + "server": "CONNECT_SERVER", + "api_key": "CONNECT_API_KEY", + "content_guid": "CONNECT_CONTENT_GUID", + "product": "POSIT_PRODUCT", + "product_value": "CONNECT" + }, + "server_normalization": { + "description": "Server URLs a publisher might configure, and the base each reduces to. Publishers paste whatever the dashboard shows them, so a form one package accepts and the other does not sends requests to the wrong place.", + "cases": [ + { + "name": "an already normalized url is unchanged", + "configured": "https://connect.example.com", + "expected": "https://connect.example.com" + }, + { + "name": "a trailing slash is dropped", + "configured": "https://connect.example.com/", + "expected": "https://connect.example.com" + }, + { + "name": "repeated trailing slashes are dropped", + "configured": "https://connect.example.com///", + "expected": "https://connect.example.com" + }, + { + "name": "the api root is dropped", + "configured": "https://connect.example.com/__api__", + "expected": "https://connect.example.com" + }, + { + "name": "the api root with a trailing slash is dropped", + "configured": "https://connect.example.com/__api__/", + "expected": "https://connect.example.com" + }, + { + "name": "a server hosted under a path keeps the path", + "configured": "https://example.com/connect/__api__/", + "expected": "https://example.com/connect" + } + ] + }, + "runtime_detection": { + "description": "Whether the process is running as content on Connect. Connect sets POSIT_PRODUCT, and some processes carry only the content guid. `env` maps a variable to its value, or to null when it is unset.", + "cases": [ + { + "name": "a local session is not Connect", + "env": { "POSIT_PRODUCT": null, "CONNECT_CONTENT_GUID": null }, + "expected": false + }, + { + "name": "the product variable identifies Connect", + "env": { "POSIT_PRODUCT": "CONNECT", "CONNECT_CONTENT_GUID": null }, + "expected": true + }, + { + "name": "another Posit product is not Connect", + "env": { "POSIT_PRODUCT": "WORKBENCH", "CONNECT_CONTENT_GUID": null }, + "expected": false + }, + { + "name": "a content guid alone identifies Connect", + "env": { "POSIT_PRODUCT": null, "CONNECT_CONTENT_GUID": "01234567-89ab-cdef-0123-456789abcdef" }, + "expected": true + }, + { + "name": "an empty content guid is not Connect", + "env": { "POSIT_PRODUCT": null, "CONNECT_CONTENT_GUID": "" }, + "expected": false + } + ] + }, + "api_request": { + "description": "Where a request lands, given the normalized server and the path segments the caller passes. Both packages address Connect's versioned API root.", + "server": "https://connect.example.com", + "cases": [ + { + "name": "a content item", + "path": ["content", "01234567-89ab-cdef-0123-456789abcdef"], + "expected": "https://connect.example.com/__api__/v1/content/01234567-89ab-cdef-0123-456789abcdef" + }, + { + "name": "a content item's permissions", + "path": ["content", "01234567-89ab-cdef-0123-456789abcdef", "permissions"], + "expected": "https://connect.example.com/__api__/v1/content/01234567-89ab-cdef-0123-456789abcdef/permissions" + }, + { + "name": "the users collection", + "path": ["users"], + "expected": "https://connect.example.com/__api__/v1/users" + } + ] + } +} diff --git a/pkg-r/tests/testthat/test-connect.R b/pkg-r/tests/testthat/test-connect.R index ea8da7ff..5e0e819e 100644 --- a/pkg-r/tests/testthat/test-connect.R +++ b/pkg-r/tests/testthat/test-connect.R @@ -1,11 +1,26 @@ -test_that("connect_client normalizes the server URL", { - withr::local_envvar(CONNECT_API_KEY = "key") +# The variables read, the server forms accepted and the URLs requests land on +# are shared with pkg-py; see tests/shared/connect.json. +connect_spec <- shared_fixture("connect") + +test_that("the shared fixture carries the cases it promises", { + detection <- vapply( + connect_spec$runtime_detection$cases, + function(case) case$expected, + logical(1) + ) + expect_setequal(detection, c(TRUE, FALSE)) + expect_gte(length(connect_spec$server_normalization$cases), 2) + expect_gte(length(connect_spec$api_request$cases), 1) +}) - client <- connect_client(server = "https://connect.example.com/") - expect_equal(client$server, "https://connect.example.com") +test_that("connect_client normalizes the server URL", { + env <- connect_spec$environment + withr::local_envvar(structure(list("key"), names = env$api_key)) - client <- connect_client(server = "https://connect.example.com/__api__") - expect_equal(client$server, "https://connect.example.com") + for (case in connect_spec$server_normalization$cases) { + client <- connect_client(server = case$configured) + expect_equal(client$server, case$expected, info = case$name) + } }) test_that("connect_client errors without credentials", { @@ -18,14 +33,24 @@ test_that("connect_client errors without credentials", { }) test_that("is_connect_runtime detects Connect env vars", { - withr::local_envvar(POSIT_PRODUCT = NA, CONNECT_CONTENT_GUID = NA) - expect_false(is_connect_runtime()) + for (case in connect_spec$runtime_detection$cases) { + values <- lapply(case$env, function(value) if (is.null(value)) NA else value) + withr::with_envvar(values, { + expect_equal(is_connect_runtime(), case$expected, info = case$name) + }) + } +}) - withr::local_envvar(POSIT_PRODUCT = "CONNECT") - expect_true(is_connect_runtime()) +test_that("connect_req addresses the versioned API root", { + client <- connect_client( + server = connect_spec$api_request$server, + api_key = "key" + ) - withr::local_envvar(POSIT_PRODUCT = NA, CONNECT_CONTENT_GUID = "guid") - expect_true(is_connect_runtime()) + for (case in connect_spec$api_request$cases) { + req <- do.call(connect_req, c(list(client), case$path)) + expect_equal(req$url, case$expected, info = case$name) + } }) test_that("connect_trace_lines pages until the total is exhausted", { diff --git a/tests/shared/connect.json b/tests/shared/connect.json new file mode 100644 index 00000000..c66bdc35 --- /dev/null +++ b/tests/shared/connect.json @@ -0,0 +1,97 @@ +{ + "description": "Posit Connect client contract shared by pkg-r and pkg-py. The source is tests/shared/connect.json; the copy under pkg-r/tests/testthat/fixtures/shared/ is generated by scripts/sync-shared-fixtures.sh. Edit the source and re-run that script.", + "environment": { + "description": "Environment variables the client reads. A publisher or Connect itself sets these, so both packages must read the same names.", + "server": "CONNECT_SERVER", + "api_key": "CONNECT_API_KEY", + "content_guid": "CONNECT_CONTENT_GUID", + "product": "POSIT_PRODUCT", + "product_value": "CONNECT" + }, + "server_normalization": { + "description": "Server URLs a publisher might configure, and the base each reduces to. Publishers paste whatever the dashboard shows them, so a form one package accepts and the other does not sends requests to the wrong place.", + "cases": [ + { + "name": "an already normalized url is unchanged", + "configured": "https://connect.example.com", + "expected": "https://connect.example.com" + }, + { + "name": "a trailing slash is dropped", + "configured": "https://connect.example.com/", + "expected": "https://connect.example.com" + }, + { + "name": "repeated trailing slashes are dropped", + "configured": "https://connect.example.com///", + "expected": "https://connect.example.com" + }, + { + "name": "the api root is dropped", + "configured": "https://connect.example.com/__api__", + "expected": "https://connect.example.com" + }, + { + "name": "the api root with a trailing slash is dropped", + "configured": "https://connect.example.com/__api__/", + "expected": "https://connect.example.com" + }, + { + "name": "a server hosted under a path keeps the path", + "configured": "https://example.com/connect/__api__/", + "expected": "https://example.com/connect" + } + ] + }, + "runtime_detection": { + "description": "Whether the process is running as content on Connect. Connect sets POSIT_PRODUCT, and some processes carry only the content guid. `env` maps a variable to its value, or to null when it is unset.", + "cases": [ + { + "name": "a local session is not Connect", + "env": { "POSIT_PRODUCT": null, "CONNECT_CONTENT_GUID": null }, + "expected": false + }, + { + "name": "the product variable identifies Connect", + "env": { "POSIT_PRODUCT": "CONNECT", "CONNECT_CONTENT_GUID": null }, + "expected": true + }, + { + "name": "another Posit product is not Connect", + "env": { "POSIT_PRODUCT": "WORKBENCH", "CONNECT_CONTENT_GUID": null }, + "expected": false + }, + { + "name": "a content guid alone identifies Connect", + "env": { "POSIT_PRODUCT": null, "CONNECT_CONTENT_GUID": "01234567-89ab-cdef-0123-456789abcdef" }, + "expected": true + }, + { + "name": "an empty content guid is not Connect", + "env": { "POSIT_PRODUCT": null, "CONNECT_CONTENT_GUID": "" }, + "expected": false + } + ] + }, + "api_request": { + "description": "Where a request lands, given the normalized server and the path segments the caller passes. Both packages address Connect's versioned API root.", + "server": "https://connect.example.com", + "cases": [ + { + "name": "a content item", + "path": ["content", "01234567-89ab-cdef-0123-456789abcdef"], + "expected": "https://connect.example.com/__api__/v1/content/01234567-89ab-cdef-0123-456789abcdef" + }, + { + "name": "a content item's permissions", + "path": ["content", "01234567-89ab-cdef-0123-456789abcdef", "permissions"], + "expected": "https://connect.example.com/__api__/v1/content/01234567-89ab-cdef-0123-456789abcdef/permissions" + }, + { + "name": "the users collection", + "path": ["users"], + "expected": "https://connect.example.com/__api__/v1/users" + } + ] + } +}