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 a7ae4a4469557c45392b60486c56e3d57105b002 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/config.py | 1 + src/blueapi/service/authorization.py | 45 ++++++++++++++++++++++++++++ tests/unit_tests/test_config.py | 1 + 3 files changed, 47 insertions(+) create mode 100644 src/blueapi/service/authorization.py diff --git a/src/blueapi/config.py b/src/blueapi/config.py index 4c2431cf1e..538d141589 100644 --- a/src/blueapi/config.py +++ b/src/blueapi/config.py @@ -298,6 +298,7 @@ class Tag(StrEnum): class OpaConfig(BlueapiBaseModel): root: HttpUrl = HttpUrl("http://localhost:8181") + audience: str = "account" class ApplicationConfig(BlueapiBaseModel): diff --git a/src/blueapi/service/authorization.py b/src/blueapi/service/authorization.py new file mode 100644 index 0000000000..151e60a0cb --- /dev/null +++ b/src/blueapi/service/authorization.py @@ -0,0 +1,45 @@ +import logging +from collections.abc import Mapping +from contextlib import AbstractAsyncContextManager, aclosing, nullcontext +from typing import Any, Self + +from aiohttp import ClientSession + +from blueapi.config import OpaConfig + +LOGGER = logging.getLogger(__name__) + + +class OpaClient: + def __init__(self, instrument: str, config: OpaConfig): + LOGGER.info("Creating OpaClient for %s with config %s", instrument, config) + self._instrument = instrument + self._config = config + self._session = ClientSession(base_url=config.root.encoded_string()) + self._audience = config.audience + + async def aclose(self): + LOGGER.info("Closing OPA session") + await self._session.close() + + async def _call_opa(self, endpoint: str, data: Mapping[str, Any]) -> bool: + resp = await self._session.post( + endpoint, + json={ + "input": { + "beamline": self._instrument, + "audience": self._audience, + **data, + } + }, + ) + return (await resp.json())["result"] + + @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() diff --git a/tests/unit_tests/test_config.py b/tests/unit_tests/test_config.py index 7ce98f6fe6..97a4ce3806 100644 --- a/tests/unit_tests/test_config.py +++ b/tests/unit_tests/test_config.py @@ -339,6 +339,7 @@ def test_config_yaml_parsed(temp_yaml_config_file): }, "opa": { "root": "http://opa.example.com/", + "audience": "account", }, }, { From a0def5fab4d80ebb2c5dae6d5ab24c70a6c31f60 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 +++++++- tests/unit_tests/service/test_main.py | 18 +++++++++++++++++- 2 files changed, 24 insertions(+), 2 deletions(-) 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 diff --git a/tests/unit_tests/service/test_main.py b/tests/unit_tests/service/test_main.py index a7e04105c2..1801b8756b 100644 --- a/tests/unit_tests/service/test_main.py +++ b/tests/unit_tests/service/test_main.py @@ -1,5 +1,5 @@ from unittest import mock -from unittest.mock import Mock, call +from unittest.mock import Mock, call, patch import pytest from fastapi import FastAPI, Request @@ -10,6 +10,7 @@ from blueapi.service.main import ( add_version_headers, get_passthrough_headers, + lifespan, log_request_details, ) @@ -79,3 +80,18 @@ def test_get_passthrough_headers( request = Mock(spec=Request) request.headers = headers assert get_passthrough_headers(request) == expected_headers + + +@patch("blueapi.service.main.teardown_runner") +@patch("blueapi.service.main.setup_runner") +async def test_lifespan(setup: Mock, teardown: Mock): + conf = ApplicationConfig() + lifespan_fn = lifespan(conf) + + app = Mock() + + async with lifespan_fn(app): + setup.assert_called_once_with(conf) + teardown.assert_not_called() + + teardown.assert_called_once() From 9a381285a52569204e90c6a97f418c8b60a15ac8 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 151e60a0cb..aabd4a692a 100644 --- a/src/blueapi/service/authorization.py +++ b/src/blueapi/service/authorization.py @@ -37,9 +37,11 @@ async def _call_opa(self, endpoint: str, 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 5e8af1812b2f863c6dea90291b5d75ce20d8c82e 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 | 62 +++++++++++++++++++ 1 file changed, 62 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..608269d20b --- /dev/null +++ b/tests/unit_tests/service/test_authorization.py @@ -0,0 +1,62 @@ +from unittest.mock import AsyncMock, 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) + + +@patch_client_session +async def test_opa_adds_input_fields(session: MagicMock, opa_config: OpaConfig): + session.return_value.post = AsyncMock() + async with OpaClient.for_config("p45", opa_config) as opa: + assert opa is not None + await opa._call_opa("foo/bar", data={"foo": "bar"}) + + session.assert_called_once() + session().post.assert_called_once_with( + "foo/bar", + json={"input": {"beamline": "p45", "audience": "account", "foo": "bar"}}, + )