diff --git a/docs/reference/openapi.yaml b/docs/reference/openapi.yaml index 2ba917804..28239ffe6 100644 --- a/docs/reference/openapi.yaml +++ b/docs/reference/openapi.yaml @@ -1,5 +1,22 @@ components: schemas: + BasicAuthentication: + additionalProperties: false + description: User credentials for basic authentication + properties: + password: + description: Password to verify user's identity + title: Password + type: string + username: + description: Unique identifier for user + title: Username + type: string + required: + - username + - password + title: BasicAuthentication + type: object DeviceModel: additionalProperties: false description: Representation of a device @@ -224,6 +241,28 @@ components: - new_state title: StateChangeRequest type: object + StompConfig: + additionalProperties: false + description: Config for connecting to stomp broker + properties: + auth: + $ref: '#/components/schemas/BasicAuthentication' + description: Auth information for communicating with STOMP broker, if required + title: Auth + enabled: + default: false + description: True if blueapi should connect to stomp for asynchronous event + publishing + title: Enabled + type: boolean + url: + default: tcp://localhost:61613 + format: uri + minLength: 1 + title: Url + type: string + title: StompConfig + type: object Task: additionalProperties: false description: Task that will run a plan @@ -382,7 +421,7 @@ info: name: Apache 2.0 url: https://www.apache.org/licenses/LICENSE-2.0.html title: BlueAPI Control - version: 1.1.3 + version: 1.2.0 openapi: 3.1.0 paths: /config/oidc: @@ -401,6 +440,22 @@ paths: summary: Get Oidc Config tags: - Meta + /config/stomp: + get: + description: Retrieve the stomp configuration for the server. + operationId: get_stomp_config_config_stomp_get + responses: + '200': + content: + application/json: + schema: + $ref: '#/components/schemas/StompConfig' + description: Successful Response + '204': + description: No Stomp configured + summary: Get Stomp Config + tags: + - Meta /devices: get: description: Retrieve information about all available devices. diff --git a/helm/blueapi/config_schema.json b/helm/blueapi/config_schema.json index d54bc5357..6a4171ada 100644 --- a/helm/blueapi/config_schema.json +++ b/helm/blueapi/config_schema.json @@ -453,16 +453,10 @@ "type": "string" }, "auth": { - "anyOf": [ - { - "$ref": "BasicAuthentication" - }, - { - "type": "null" - } - ], + "$ref": "BasicAuthentication", "default": null, - "description": "Auth information for communicating with STOMP broker, if required" + "description": "Auth information for communicating with STOMP broker, if required", + "title": "Auth" } }, "title": "StompConfig", diff --git a/helm/blueapi/values.schema.json b/helm/blueapi/values.schema.json index 1578c0dad..567abcfa5 100644 --- a/helm/blueapi/values.schema.json +++ b/helm/blueapi/values.schema.json @@ -862,15 +862,9 @@ "type": "object", "properties": { "auth": { + "title": "Auth", "description": "Auth information for communicating with STOMP broker, if required", - "anyOf": [ - { - "$ref": "BasicAuthentication" - }, - { - "type": "null" - } - ] + "$ref": "BasicAuthentication" }, "enabled": { "title": "Enabled", diff --git a/src/blueapi/cli/cli.py b/src/blueapi/cli/cli.py index 4d974d35e..e6072eb60 100644 --- a/src/blueapi/cli/cli.py +++ b/src/blueapi/cli/cli.py @@ -49,30 +49,30 @@ invoke_without_command=True, context_settings={"auto_envvar_prefix": "BLUEAPI"} ) @click.version_option(version=__version__, prog_name="blueapi") +@click.option("-H", "--host", type=str) @click.option( "-c", "--config", type=Path, help="Path to configuration YAML file", multiple=True ) @click.pass_context -def main(ctx: click.Context, config: Path | None | tuple[Path, ...]) -> None: +def main(ctx: click.Context, config: tuple[Path, ...], host: str | None): # if no command is supplied, run with the options passed # Set umask to DLS standard os.umask(stat.S_IWOTH) config_loader = ConfigLoader(ApplicationConfig) - if config is not None: - configs = (config,) if isinstance(config, Path) else config - for path in configs: - if path.exists(): - config_loader.use_values_from_yaml(path) - else: - raise FileNotFoundError(f"Cannot find file: {path}") + try: + config_loader.use_values_from_yaml(*config) + except FileNotFoundError as fnfe: + raise ClickException(f"Config file not found: {fnfe.filename}") from fnfe + if host: + config_loader.use_values({"api": {"url": host}}) - ctx.ensure_object(dict) loaded_config: ApplicationConfig = config_loader.load() set_up_logging(loaded_config.logging) + ctx.ensure_object(dict) ctx.obj["config"] = loaded_config if ctx.invoked_subcommand is None: diff --git a/src/blueapi/client/client.py b/src/blueapi/client/client.py index 0930e240a..a7de43ace 100644 --- a/src/blueapi/client/client.py +++ b/src/blueapi/client/client.py @@ -1,12 +1,8 @@ import time from concurrent.futures import Future -from bluesky_stomp.messaging import MessageContext, StompClient -from bluesky_stomp.models import Broker -from observability_utils.tracing import ( - get_tracer, - start_as_current_span, -) +from bluesky_stomp.messaging import MessageContext +from observability_utils.tracing import get_tracer, start_as_current_span from blueapi.config import ApplicationConfig, MissingStompConfigurationError from blueapi.core.bluesky_types import DataEvent @@ -38,7 +34,7 @@ class BlueapiClient: """Unified client for controlling blueapi""" _rest: BlueapiRestClient - _events: EventBusClient | None + _event_bus_client: EventBusClient | None def __init__( self, @@ -46,7 +42,7 @@ def __init__( events: EventBusClient | None = None, ): self._rest = rest - self._events = events + self._event_bus_client = events @classmethod def from_config(cls, config: ApplicationConfig) -> "BlueapiClient": @@ -56,20 +52,8 @@ def from_config(cls, config: ApplicationConfig) -> "BlueapiClient": except Exception: ... # Swallow exceptions rest = BlueapiRestClient(config.api, session_manager=session_manager) - if config.stomp.enabled: - assert config.stomp.url.host is not None, "Stomp URL missing host" - assert config.stomp.url.port is not None, "Stomp URL missing port" - client = StompClient.for_broker( - broker=Broker( - host=config.stomp.url.host, - port=config.stomp.url.port, - auth=config.stomp.auth, - ) - ) - events = EventBusClient(client) - return cls(rest, events) - else: - return cls(rest) + event_bus = EventBusClient.from_stomp_config(config.stomp) + return cls(rest, event_bus) @start_as_current_span(TRACER) def get_plans(self) -> PlanResponse: @@ -216,7 +200,7 @@ def run_task( of task execution. """ - if self._events is None: + if (event_bus := self._event_bus()) is None: raise MissingStompConfigurationError( "Stomp configuration required to run plans is missing or disabled" ) @@ -253,8 +237,8 @@ def inner_on_event(event: AnyEvent, ctx: MessageContext) -> None: else: complete.set_result(event) - with self._events: - self._events.subscribe_to_all_events(inner_on_event) + with event_bus: + event_bus.subscribe_to_all_events(inner_on_event) self.start_task(WorkerTask(task_id=task_id)) return complete.result(timeout=timeout) @@ -457,3 +441,10 @@ def get_python_env( """ return self._rest.get_python_environment(name=name, source=source) + + def _event_bus(self) -> EventBusClient | None: + if not self._event_bus_client: + if stomp_config := self._rest.get_stomp_config(): + self._event_bus_client = EventBusClient.from_stomp_config(stomp_config) + + return self._event_bus_client diff --git a/src/blueapi/client/event_bus.py b/src/blueapi/client/event_bus.py index cb807f24d..2f344a081 100644 --- a/src/blueapi/client/event_bus.py +++ b/src/blueapi/client/event_bus.py @@ -1,8 +1,10 @@ from collections.abc import Callable +from typing import Self -from bluesky_stomp.messaging import MessageContext, StompClient +from bluesky_stomp.messaging import Broker, MessageContext, StompClient from bluesky_stomp.models import MessageTopic +from blueapi.config import StompConfig from blueapi.core import DataEvent from blueapi.worker import ProgressEvent, WorkerEvent @@ -45,3 +47,15 @@ def subscribe_to_all_events( raise BlueskyStreamingError( "Unable to subscribe to messages from blueapi" ) from err + + @classmethod + def from_stomp_config(cls, config: StompConfig) -> Self | None: + if config.enabled: + assert config.url.host is not None, "Stomp URL missing host" + assert config.url.port is not None, "Stomp URL missing port" + client = StompClient.for_broker( + broker=Broker( + host=config.url.host, port=config.url.port, auth=config.auth + ) + ) + return cls(client) diff --git a/src/blueapi/client/rest.py b/src/blueapi/client/rest.py index 3ff119449..9f14251eb 100644 --- a/src/blueapi/client/rest.py +++ b/src/blueapi/client/rest.py @@ -10,7 +10,7 @@ ) from pydantic import BaseModel, TypeAdapter, ValidationError -from blueapi.config import RestConfig +from blueapi.config import RestConfig, StompConfig from blueapi.service.authentication import JWTAuth, SessionManager from blueapi.service.model import ( DeviceModel, @@ -230,6 +230,14 @@ def get_oidc_config(self) -> OIDCConfig | None: # Server is not using authentication return None + def get_stomp_config(self) -> StompConfig | None: + try: + return self._request_and_deserialize("/config/stomp", StompConfig) + except (NoContentError, KeyError): + # Older versions of the server may not have the endpoint implemented so + # treat 404s as no configuration. + return None + def get_python_environment( self, name: str | None = None, source: SourceInfo | None = None ) -> PythonEnvironmentResponse: diff --git a/src/blueapi/config.py b/src/blueapi/config.py index 214016860..1601fd336 100644 --- a/src/blueapi/config.py +++ b/src/blueapi/config.py @@ -21,6 +21,7 @@ ValidationError, field_validator, ) +from pydantic.json_schema import SkipJsonSchema from blueapi.utils import BlueapiBaseModel, InvalidConfigError @@ -100,7 +101,7 @@ class StompConfig(BlueapiBaseModel): default=False, ) url: TcpUrl = TcpUrl("tcp://localhost:61613") - auth: BasicAuthentication | None = Field( + auth: BasicAuthentication | SkipJsonSchema[None] = Field( description="Auth information for communicating with STOMP broker, if required", default=None, ) @@ -275,7 +276,7 @@ class ApplicationConfig(BlueapiBaseModel): """ #: API version to publish in OpenAPI schema - REST_API_VERSION: ClassVar[str] = "1.1.3" + REST_API_VERSION: ClassVar[str] = "1.2.0" LICENSE_INFO: ClassVar[dict[str, str]] = { "name": "Apache 2.0", @@ -358,9 +359,9 @@ def recursively_update_map(old: dict[str, Any], new: Mapping[str, Any]) -> None: recursively_update_map(self._values, values) - def use_values_from_yaml(self, path: Path) -> None: + def use_values_from_yaml(self, *paths: Path) -> None: """ - Use all values provided in a YAML/JSON file in the + Use all values provided in a YAML/JSON files in the config, override any defaults and values set by previous calls into this class. @@ -368,9 +369,15 @@ def use_values_from_yaml(self, path: Path) -> None: path (Path): Path to YAML/JSON file """ - with path.open("r") as stream: - values = yaml.load(stream, yaml.Loader) - self.use_values(values) + # Split reading and loading so that a missing file does not leave + # config in partially loaded state + configs = [] + for path in paths: + with path.open("r") as stream: + configs.append(yaml.load(stream, yaml.Loader)) + + for values in configs: + self.use_values(values) def load(self) -> C: """ diff --git a/src/blueapi/service/interface.py b/src/blueapi/service/interface.py index 9bc8bcef8..4c4c2633b 100644 --- a/src/blueapi/service/interface.py +++ b/src/blueapi/service/interface.py @@ -260,6 +260,10 @@ def get_oidc_config() -> OIDCConfig | None: return config().oidc +def get_stomp_config() -> StompConfig | None: + return config().stomp + + def get_python_env( name: str | None = None, source: SourceInfo | None = None ) -> PythonEnvironmentResponse: diff --git a/src/blueapi/service/main.py b/src/blueapi/service/main.py index 0e6faf9e6..0f545f06e 100644 --- a/src/blueapi/service/main.py +++ b/src/blueapi/service/main.py @@ -33,7 +33,7 @@ from starlette.responses import JSONResponse from super_state_machine.errors import TransitionError -from blueapi.config import ApplicationConfig, OIDCConfig, Tag +from blueapi.config import ApplicationConfig, OIDCConfig, StompConfig, Tag from blueapi.service import interface from blueapi.worker import TrackableTask, WorkerState from blueapi.worker.event import TaskStatusEnum @@ -224,6 +224,22 @@ def get_oidc_config( return config +@open_router.get( + "/config/stomp", + tags=[Tag.META], + responses={status.HTTP_204_NO_CONTENT: {"description": "No Stomp configured"}}, +) +@start_as_current_span(TRACER) +def get_stomp_config( + runner: Annotated[WorkerDispatcher, Depends(_runner)], +) -> StompConfig: + """Retrieve the stomp configuration for the server.""" + config = runner.run(interface.get_stomp_config) + if config is None: + raise HTTPException(status_code=status.HTTP_204_NO_CONTENT) + return config + + @secure_router.get("/plans", tags=[Tag.PLAN]) @start_as_current_span(TRACER) def get_plans(runner: Annotated[WorkerDispatcher, Depends(_runner)]) -> PlanResponse: diff --git a/tests/system_tests/test_blueapi_system.py b/tests/system_tests/test_blueapi_system.py index e4ce504fa..35372ac2d 100644 --- a/tests/system_tests/test_blueapi_system.py +++ b/tests/system_tests/test_blueapi_system.py @@ -162,12 +162,11 @@ def expected_devices() -> DeviceResponse: ) -@pytest.fixture -def blueapi_client_get_methods() -> list[str]: - # Get a list of methods that take only one argument (self) - # This will currently return +def authenticated_get_methods() -> list[str]: + # Get a list of methods that take only one argument (self) and require + # authentication. This will currently return # ['get_plans', 'get_devices', 'get_state', 'get_all_tasks', - # 'get_active_task','get_environment','resume', 'stop','get_oidc_config'] + # 'get_active_task','get_environment','resume', 'stop'] return [ method for method in BlueapiClient.__dict__ @@ -175,6 +174,9 @@ def blueapi_client_get_methods() -> list[str]: and not method.startswith("__") and len(inspect.signature(getattr(BlueapiClient, method)).parameters) == 1 and "self" in inspect.signature(getattr(BlueapiClient, method)).parameters + # oidc_config and stomp config can be accessed without auth + and method != "get_oidc_config" + and method != "_event_bus" ] @@ -212,15 +214,10 @@ def reset_numtracker(server_config: ApplicationConfig): yield -def test_cannot_access_endpoints( - client_without_auth: BlueapiClient, blueapi_client_get_methods: list[str] -): - blueapi_client_get_methods.remove( - "get_oidc_config" - ) # get_oidc_config can be accessed without auth - for get_method in blueapi_client_get_methods: - with pytest.raises(BlueskyRemoteControlError, match=r""): - getattr(client_without_auth, get_method)() +@pytest.mark.parametrize("method_name", authenticated_get_methods()) +def test_cannot_access_endpoints(client_without_auth: BlueapiClient, method_name: str): + with pytest.raises(BlueskyRemoteControlError, match=r""): + getattr(client_without_auth, method_name)() def test_can_get_oidc_config_without_auth(client_without_auth: BlueapiClient): diff --git a/tests/unit_tests/client/test_client.py b/tests/unit_tests/client/test_client.py index d6d2e1f22..73f9ba18f 100644 --- a/tests/unit_tests/client/test_client.py +++ b/tests/unit_tests/client/test_client.py @@ -392,7 +392,8 @@ def test_resume( ) -def test_cannot_run_task_without_message_bus(client: BlueapiClient): +def test_cannot_run_task_without_message_bus(client: BlueapiClient, mock_rest: Mock): + mock_rest.get_stomp_config.return_value = None with pytest.raises( MissingStompConfigurationError, match="Stomp configuration required to run plans is missing or disabled", @@ -660,8 +661,11 @@ def test_resume_span_ok( def test_cannot_run_task_span_ok( - exporter: JsonObjectSpanExporter, client: BlueapiClient + exporter: JsonObjectSpanExporter, + client: BlueapiClient, + mock_rest: Mock, ): + mock_rest.get_stomp_config.return_value = None with pytest.raises( MissingStompConfigurationError, match="Stomp configuration required to run plans is missing or disabled", diff --git a/tests/unit_tests/test_cli.py b/tests/unit_tests/test_cli.py index 2a49a1fe8..7ec530a2c 100644 --- a/tests/unit_tests/test_cli.py +++ b/tests/unit_tests/test_cli.py @@ -249,19 +249,22 @@ def test_submit_plan(runner: CliRunner): @responses.activate def test_submit_plan_without_stomp(runner: CliRunner): config_path = "tests/unit_tests/example_yaml/rest_config.yaml" - result = runner.invoke( - main, - [ - "-c", - config_path, - "controller", - "run", - "-i", - "cm12345-1", - "sleep", - '{"time": 5}', - ], - ) + with patch( + "blueapi.client.rest.BlueapiRestClient.get_stomp_config", return_value=None + ): + result = runner.invoke( + main, + [ + "-c", + config_path, + "controller", + "run", + "-i", + "cm12345-1", + "sleep", + '{"time": 5}', + ], + ) assert ( result.stderr @@ -269,7 +272,7 @@ def test_submit_plan_without_stomp(runner: CliRunner): ) -@patch("blueapi.client.client.StompClient") +@patch("blueapi.client.event_bus.StompClient") @responses.activate def test_run_plan(stomp_client: StompClient, runner: CliRunner): task_id = "abcd-1234" @@ -402,17 +405,20 @@ def test_invalid_stomp_config_for_listener(runner: CliRunner): def test_cannot_run_plans_without_stomp_config(runner: CliRunner): - result = runner.invoke( - main, - [ - "controller", - "run", - "-i", - "cm12345-1", - "sleep", - '{"time": 5}', - ], - ) + with patch( + "blueapi.client.rest.BlueapiRestClient.get_stomp_config", return_value=None + ): + result = runner.invoke( + main, + [ + "controller", + "run", + "-i", + "cm12345-1", + "sleep", + '{"time": 5}', + ], + ) assert result.exit_code == 1 assert ( result.stderr