From 0e68df413864361a92d45490d4cba7d537829062 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Thu, 21 May 2026 11:42:59 +0100 Subject: [PATCH 1/5] feat: Add OpaConfig for authorization configuration --- helm/blueapi/config_schema.json | 27 +++++++++++++++++++++++++++ helm/blueapi/values.schema.json | 26 ++++++++++++++++++++++++++ pyproject.toml | 1 + src/blueapi/config.py | 6 ++++++ tests/unit_tests/test_config.py | 4 ++++ uv.lock | 2 ++ 6 files changed, 66 insertions(+) diff --git a/helm/blueapi/config_schema.json b/helm/blueapi/config_schema.json index b5d0c9bf3d..1ad4e82bfe 100644 --- a/helm/blueapi/config_schema.json +++ b/helm/blueapi/config_schema.json @@ -330,6 +330,22 @@ "type": "object", "$id": "OIDCConfig" }, + "OpaConfig": { + "additionalProperties": false, + "properties": { + "root": { + "default": "http://localhost:8181/", + "format": "uri", + "maxLength": 2083, + "minLength": 1, + "title": "Root", + "type": "string" + } + }, + "title": "OpaConfig", + "type": "object", + "$id": "OpaConfig" + }, "PlanSource": { "additionalProperties": false, "properties": { @@ -612,6 +628,17 @@ } ], "default": null + }, + "opa": { + "anyOf": [ + { + "$ref": "OpaConfig" + }, + { + "type": "null" + } + ], + "default": null } }, "title": "ApplicationConfig", diff --git a/helm/blueapi/values.schema.json b/helm/blueapi/values.schema.json index 74deedadb2..6de532cc88 100644 --- a/helm/blueapi/values.schema.json +++ b/helm/blueapi/values.schema.json @@ -751,6 +751,22 @@ }, "additionalProperties": false }, + "OpaConfig": { + "$id": "OpaConfig", + "title": "OpaConfig", + "type": "object", + "properties": { + "root": { + "title": "Root", + "default": "http://localhost:8181/", + "type": "string", + "format": "uri", + "maxLength": 2083, + "minLength": 1 + } + }, + "additionalProperties": false + }, "PlanSource": { "$id": "PlanSource", "title": "PlanSource", @@ -1011,6 +1027,16 @@ } ] }, + "opa": { + "anyOf": [ + { + "$ref": "OpaConfig" + }, + { + "type": "null" + } + ] + }, "scratch": { "anyOf": [ { diff --git a/pyproject.toml b/pyproject.toml index 659779994a..9f4231c76c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -37,6 +37,7 @@ dependencies = [ "tomlkit", "graypy>=2.1.0", "httpx>=0.28.1", + "aiohttp>=3.13.5", ] dynamic = ["version"] license.file = "LICENSE" diff --git a/src/blueapi/config.py b/src/blueapi/config.py index 83d6d70211..4c2431cf1e 100644 --- a/src/blueapi/config.py +++ b/src/blueapi/config.py @@ -296,6 +296,10 @@ class Tag(StrEnum): META = "Meta" +class OpaConfig(BlueapiBaseModel): + root: HttpUrl = HttpUrl("http://localhost:8181") + + class ApplicationConfig(BlueapiBaseModel): """ Config for the worker application as a whole. Root of @@ -335,6 +339,7 @@ class ApplicationConfig(BlueapiBaseModel): oidc: OIDCConfig | None = None auth_token_path: Path | None = None numtracker: NumtrackerConfig | None = None + opa: OpaConfig | None = None def __eq__(self, other: object) -> bool: if isinstance(other, ApplicationConfig): @@ -343,6 +348,7 @@ def __eq__(self, other: object) -> bool: & (self.env == other.env) & (self.logging == other.logging) & (self.api == other.api) + & (self.opa == other.opa) ) return False diff --git a/tests/unit_tests/test_config.py b/tests/unit_tests/test_config.py index 30fe551c4c..7ce98f6fe6 100644 --- a/tests/unit_tests/test_config.py +++ b/tests/unit_tests/test_config.py @@ -337,6 +337,9 @@ def test_config_yaml_parsed(temp_yaml_config_file): } ], }, + "opa": { + "root": "http://opa.example.com/", + }, }, { "stomp": { @@ -392,6 +395,7 @@ def test_config_yaml_parsed(temp_yaml_config_file): } ], }, + "opa": None, }, ], indirect=True, diff --git a/uv.lock b/uv.lock index b2d10b5bd5..eab94a8670 100644 --- a/uv.lock +++ b/uv.lock @@ -420,6 +420,7 @@ name = "blueapi" source = { editable = "." } dependencies = [ { name = "aioca" }, + { name = "aiohttp" }, { name = "bluesky", extra = ["plotting"] }, { name = "bluesky-stomp" }, { name = "click" }, @@ -481,6 +482,7 @@ dev = [ [package.metadata] requires-dist = [ { name = "aioca" }, + { name = "aiohttp", specifier = ">=3.13.5" }, { name = "bluesky", extras = ["plotting"], specifier = ">=1.14.0" }, { name = "bluesky-stomp", specifier = ">=0.1.6" }, { name = "click", specifier = ">=8.2.0" }, From 9a9b9b8c558d496e0a526534f276a097a803b73e Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Thu, 21 May 2026 11:52:04 +0100 Subject: [PATCH 2/5] Add OpaClient to wrap OPA interactions --- src/blueapi/service/authorization.py | 61 ++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 src/blueapi/service/authorization.py diff --git a/src/blueapi/service/authorization.py b/src/blueapi/service/authorization.py new file mode 100644 index 0000000000..8e260cbd53 --- /dev/null +++ b/src/blueapi/service/authorization.py @@ -0,0 +1,61 @@ +import logging +from collections.abc import Mapping +from contextlib import AbstractAsyncContextManager, aclosing, nullcontext +from typing import Any, Self + +import aiohttp +from aiohttp import ClientSession + +from blueapi.config import OpaConfig + +LOGGER = logging.getLogger(__name__) + + +class OpaClient: + client: aiohttp.ClientSession + + def __init__(self, instrument: str, config: OpaConfig): + LOGGER.info("Creating OpaClient for %s with config %s", instrument, config) + self._instrument = instrument + self._conf = config + self._session = ClientSession(base_url=config.root.encoded_string()) + + async def aclose(self): + LOGGER.info("Closing OPA session") + await self._session.close() + + async def _call_opa(self, endpoint, data: Mapping[str, Any]) -> bool: + try: + resp = await self._session.post( + endpoint, + json={ + "input": { + "beamline": self._instrument, + "audience": "account", + **data, + } + }, + ) + return (await resp.json())["result"] + except Exception: + LOGGER.exception("Failed to run check", exc_info=True) + raise + + @classmethod + def for_config( + cls, instrument: str, config: OpaConfig | None + ) -> AbstractAsyncContextManager[Self | None]: + if config: + return aclosing(cls(instrument, config)) + LOGGER.info("No OPA config provided - not creating OpaClient") + return nullcontext() + + + +class OpaUserClient: + client: OpaClient + token: str + + def __init__(self, client: OpaClient, token: str): + self.client = client + self.token = token From f12bd2a1d2891ae51345f5df4bda21791b1722a6 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Thu, 21 May 2026 13:32:18 +0100 Subject: [PATCH 3/5] Create OpaClient as part of server lifecycle --- src/blueapi/service/main.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index 546155f5c2..23baa6f13e 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -40,6 +40,7 @@ from blueapi.worker import TrackableTask, WorkerState from blueapi.worker.event import TaskStatusEnum +from .authorization import OpaClient from .model import ( DeviceModel, DeviceResponse, @@ -93,8 +94,13 @@ def teardown_runner(): def lifespan(config: ApplicationConfig): @asynccontextmanager async def inner(app: FastAPI): + if not (meta := config.env.metadata): + raise ValueError("Instrument name is required in metadata") + setup_runner(config) - yield + async with OpaClient.for_config(meta.instrument, config.opa) as opa: + app.state.authz = opa + yield teardown_runner() return inner From b2d09df81074f92e920fdc7eafb0fd38b0905131 Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Wed, 27 May 2026 14:54:30 +0100 Subject: [PATCH 4/5] Move instrument requirement into OpaClient init --- src/blueapi/service/authorization.py | 4 +++- src/blueapi/service/main.py | 6 ++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/blueapi/service/authorization.py b/src/blueapi/service/authorization.py index 8e260cbd53..b27c304c97 100644 --- a/src/blueapi/service/authorization.py +++ b/src/blueapi/service/authorization.py @@ -43,9 +43,11 @@ async def _call_opa(self, endpoint, data: Mapping[str, Any]) -> bool: @classmethod def for_config( - cls, instrument: str, config: OpaConfig | None + cls, instrument: str | None, config: OpaConfig | None ) -> AbstractAsyncContextManager[Self | None]: if config: + if not instrument: + raise ValueError("Instrument name is required for OPA client") return aclosing(cls(instrument, config)) LOGGER.info("No OPA config provided - not creating OpaClient") return nullcontext() diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index 23baa6f13e..462c9318f1 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -94,11 +94,9 @@ def teardown_runner(): def lifespan(config: ApplicationConfig): @asynccontextmanager async def inner(app: FastAPI): - if not (meta := config.env.metadata): - raise ValueError("Instrument name is required in metadata") - + meta = config.env.metadata setup_runner(config) - async with OpaClient.for_config(meta.instrument, config.opa) as opa: + async with OpaClient.for_config(meta and meta.instrument, config.opa) as opa: app.state.authz = opa yield teardown_runner() From 5c3dba8f54ab5dbfa01877021fad0cc40aef401d Mon Sep 17 00:00:00 2001 From: Peter Holloway Date: Thu, 28 May 2026 17:21:00 +0100 Subject: [PATCH 5/5] Add tests for OpaClient --- .../unit_tests/service/test_authorization.py | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 tests/unit_tests/service/test_authorization.py diff --git a/tests/unit_tests/service/test_authorization.py b/tests/unit_tests/service/test_authorization.py new file mode 100644 index 0000000000..aa4417d500 --- /dev/null +++ b/tests/unit_tests/service/test_authorization.py @@ -0,0 +1,48 @@ +from unittest.mock import MagicMock, patch + +import pytest +from pydantic import HttpUrl + +from blueapi.config import OpaConfig +from blueapi.service.authorization import ( + OpaClient, +) + +# Reusable client patch decorator +patch_client_session = patch( + "blueapi.service.authorization.ClientSession", + name="mock_client_session", + spec=True, +) + + +@pytest.fixture(scope="module") +def opa_config() -> OpaConfig: + return OpaConfig( + root=HttpUrl("http://auth.example.com"), + ) + + +@patch_client_session +async def test_session_closed(session: MagicMock, opa_config: OpaConfig): + async with OpaClient.for_config("p45", opa_config): + pass + session().close.assert_called_once() + + +@patch_client_session +async def test_opa_client_for_config(session: MagicMock, opa_config: OpaConfig): + async with OpaClient.for_config("p45", opa_config) as opa: + assert opa is not None + session.assert_called_once_with(base_url="http://auth.example.com/") + + +@pytest.mark.parametrize("instrument", [None, "p99"]) +async def test_opa_client_without_config(instrument: str | None): + async with OpaClient.for_config(instrument, None) as opa: + assert opa is None + + +async def test_opa_fails_without_instrument(opa_config: OpaConfig): + with pytest.raises(ValueError, match="Instrument name is required"): + OpaClient.for_config(None, opa_config)