diff --git a/.github/workflows/opcua-plugin.yml b/.github/workflows/opcua-plugin.yml index 43cf4a39d..8455705a4 100644 --- a/.github/workflows/opcua-plugin.yml +++ b/.github/workflows/opcua-plugin.yml @@ -125,9 +125,21 @@ jobs: run: colcon test-result --verbose integration: - name: Integration (OpenPLC) + name: Integration (OpenPLC, ${{ matrix.variant }}) + # Both write surfaces are exercised against the same real PLC. The + # read-only leg is the shipped image: it proves the refusal, its vendor + # code, and that the tag on the PLC did not move. The write-capable leg + # keeps proving that a write reaches OpenPLC and reads back. runs-on: ubuntu-latest timeout-minutes: 30 + strategy: + fail-fast: false + matrix: + include: + - variant: read-only + read_only: 'ON' + - variant: write-capable + read_only: 'OFF' steps: - name: Checkout repository uses: actions/checkout@v4 @@ -141,9 +153,26 @@ jobs: - name: Build gateway + OPC-UA plugin image run: | docker build \ + --build-arg MEDKIT_OPCUA_READ_ONLY=${{ matrix.read_only }} \ -f src/ros2_medkit_plugins/ros2_medkit_opcua/docker/Dockerfile.gateway \ -t gateway-opcua . + - name: Inspect the plugin object inside the built image + # The image is what ships, and Dockerfile.gateway builds it with + # BUILD_TESTING=OFF, so the inspection ctest does not exist inside the + # container. Pull the object out and run the same check on the runner: + # without this the image is only proven by its behaviour, and a + # build-arg or Dockerfile drift that shipped the wrong variant would be + # caught by the integration assertions rather than by the inspection. + run: | + cid=$(docker create gateway-opcua) + docker cp "$cid:$(docker run --rm --entrypoint sh gateway-opcua -c \ + "find /root/ws/install -name libros2_medkit_opcua_plugin.so | head -1")" \ + ./plugin-from-image.so + docker rm -v "$cid" >/dev/null + python3 src/ros2_medkit_plugins/ros2_medkit_opcua/test/inspect_build_variant.py \ + ./plugin-from-image.so --expect ${{ matrix.variant }} + - name: Start OpenPLC timeout-minutes: 3 run: | @@ -212,6 +241,8 @@ jobs: docker logs gateway 2>&1 | tail -10 - name: Run integration tests + env: + MEDKIT_OPCUA_VARIANT: ${{ matrix.variant }} run: bash src/ros2_medkit_plugins/ros2_medkit_opcua/docker/scripts/run_integration_tests.sh - name: Dump gateway logs on failure @@ -229,13 +260,24 @@ jobs: docker network rm plc-demo 2>/dev/null || true integration-alarms: - name: Integration (AlarmConditionType) + name: Integration (AlarmConditionType, ${{ matrix.variant }}) # Issue #386: tests the native OPC-UA AlarmCondition subscription bridge # against the test_alarm_server fixture (open62541 with FULL ns0 + alarms # ON). Independent of the OpenPLC threshold-mode integration above; runs # in parallel. + # + # Both write surfaces, like the OpenPLC job: acknowledging a condition + # changes its state on the controller, so the read-only leg asserts the + # refusal and that the condition stays unacknowledged on the server, and the + # write-capable leg asserts the acknowledge reaches it. The script builds the + # gateway image itself and derives the build arg from the same variable, so + # the image and the expectations cannot drift apart. runs-on: ubuntu-latest timeout-minutes: 30 + strategy: + fail-fast: false + matrix: + variant: [read-only, write-capable] steps: - name: Checkout repository uses: actions/checkout@v4 @@ -247,8 +289,22 @@ jobs: pip3 install --break-system-packages asyncua - name: Run alarm integration suite + env: + MEDKIT_OPCUA_VARIANT: ${{ matrix.variant }} run: bash src/ros2_medkit_plugins/ros2_medkit_opcua/docker/scripts/run_alarm_tests.sh + - name: Inspect the plugin object inside the built image + # Same reason as the OpenPLC job. This suite builds its own image, so + # the step runs after it; the tag is the one run_alarm_tests.sh builds. + run: | + cid=$(docker create gateway-opcua:alarm-test) + docker cp "$cid:$(docker run --rm --entrypoint sh gateway-opcua:alarm-test -c \ + "find /root/ws/install -name libros2_medkit_opcua_plugin.so | head -1")" \ + ./plugin-from-image.so + docker rm -v "$cid" >/dev/null + python3 src/ros2_medkit_plugins/ros2_medkit_opcua/test/inspect_build_variant.py \ + ./plugin-from-image.so --expect ${{ matrix.variant }} + - name: Dump container logs on failure if: failure() run: | @@ -342,3 +398,91 @@ jobs: - name: Show test results if: always() run: colcon test-result --verbose + + write-capable-build: + # The shipped plugin is read-only; MEDKIT_OPCUA_READ_ONLY=OFF is the + # development variant. Nothing else in CI configures it, so without this job + # the write path and its tests would rot unnoticed behind an #if until + # someone needed them. It is also the control for the build-inspection test: + # test_opcua_build_variant asserts the write symbols are PRESENT here, which + # is what keeps its symbol list from decaying into one that matches nothing + # and passes everywhere. + name: Write-capable build (jazzy) + runs-on: ubuntu-latest + container: + image: ubuntu:noble + timeout-minutes: 60 + defaults: + run: + shell: bash + steps: + - name: Install Git + run: | + apt-get update + apt-get install -y git + + - name: Checkout repository + uses: actions/checkout@v4 + + - name: Pre-install ROS 2 apt source + uses: ./.github/actions/ros-apt-source + + - name: Set up ROS 2 Jazzy + uses: ros-tooling/setup-ros@v0.7 + with: + required-ros-distributions: jazzy + + - name: Install ccache + run: apt-get install -y ccache + + - name: Cache ccache + uses: actions/cache@v4 + with: + path: /root/.cache/ccache + key: ccache-opcua-write-capable-${{ github.sha }} + restore-keys: | + ccache-opcua-write-capable- + + - name: Install dependencies + run: | + apt-get update + apt-get install -y ros-jazzy-test-msgs libyaml-cpp-dev libssl-dev + source /opt/ros/jazzy/setup.bash + rosdep update + rosdep install --from-paths src --ignore-src -y \ + --skip-keys='nav2_msgs ament_cmake_clang_format ament_cmake_clang_tidy' + + - name: Build ros2_medkit_opcua write-capable + env: + CCACHE_DIR: /root/.cache/ccache + CCACHE_MAXSIZE: 500M + CCACHE_SLOPPINESS: pch_defines,time_macros + run: | + source /opt/ros/jazzy/setup.bash + # Two passes so MEDKIT_OPCUA_READ_ONLY reaches only the package that + # defines it. Passed to the whole chain instead, CMake reports it as a + # manually-specified variable nobody used in each of the other ten + # packages, and the resulting stderr would hide any real one. + colcon build --symlink-install \ + --packages-up-to ros2_medkit_opcua --packages-skip ros2_medkit_opcua \ + --cmake-args -DCMAKE_BUILD_TYPE=Release \ + --event-handlers console_direct+ + colcon build --symlink-install \ + --packages-select ros2_medkit_opcua \ + --cmake-args -DCMAKE_BUILD_TYPE=Release -DMEDKIT_OPCUA_READ_ONLY=OFF \ + --event-handlers console_direct+ + ccache -s + + - name: Run tests + timeout-minutes: 20 + run: | + source /opt/ros/jazzy/setup.bash + source install/setup.bash + colcon test --return-code-on-test-failure \ + --packages-select ros2_medkit_opcua \ + --ctest-args -LE linter \ + --event-handlers console_direct+ + + - name: Show test results + if: always() + run: colcon test-result --verbose diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/CMakeLists.txt b/src/ros2_medkit_plugins/ros2_medkit_opcua/CMakeLists.txt index 26af9f4a6..698e41733 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/CMakeLists.txt +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/CMakeLists.txt @@ -94,6 +94,54 @@ foreach(_op62_target open62541pp open62541) endif() endforeach() +# One section per function and per object in the vendored library, so +# --gc-sections on the module below can drop the ones nothing here reaches. +# Without it the library arrives as a handful of large sections the collector +# cannot take apart, and the read-only object keeps carrying open62541's own +# Write service primitives - __UA_Client_writeAttribute and the rest - even +# though no route in the plugin reaches them. +# +# The list names the OBJECT libraries, not just `open62541`: upstream compiles +# its C sources in open62541-object and open62541-plugins and assembles +# `open62541` from $, so options set on `open62541` reach no +# compilation at all. Upstream does add the same two flags itself, but only for +# Release and MinSizeRel (its CMakeLists.txt, "Strip release builds"), so a +# default-type build got neither - measurably: twelve UA_*_write* symbols in the +# read-only object there against none in Release. Setting them here makes the +# property independent of CMAKE_BUILD_TYPE; where upstream already set them the +# repetition is harmless. +foreach(_op62_target open62541pp open62541 open62541-object open62541-plugins) + if(TARGET ${_op62_target}) + target_compile_options(${_op62_target} PRIVATE -ffunction-sections -fdata-sections) + endif() +endforeach() + +# ---- Read-only build ---- +# Whether this box can change a controller is a property of the binary, not of +# a configuration file: ON compiles the OPC-UA value-write path out entirely, +# so no symbol able to put a value on the wire is emitted and no data point can +# be marked writable. A write-capable build exists for development and is +# selected explicitly. +option(MEDKIT_OPCUA_READ_ONLY + "Build the OPC-UA plugin without any controller write path" ON) +if(MEDKIT_OPCUA_READ_ONLY) + set(_medkit_opcua_read_only 1) + set(_medkit_opcua_variant "read-only") +else() + set(_medkit_opcua_read_only 0) + set(_medkit_opcua_variant "write-capable") +endif() +message(STATUS "ros2_medkit_opcua: ${_medkit_opcua_variant} build " + "(MEDKIT_OPCUA_READ_ONLY=${MEDKIT_OPCUA_READ_ONLY})") + +# Directory scope, and placed after the FetchContent block above: the macro +# guards headers shared by the plugin and every test target, so all of them +# must compile against one value or the one-definition rule is broken, while +# open62541pp - added as a subdirectory before this point - must not see it. +# Always defined to 0 or 1 so `#if MEDKIT_OPCUA_READ_ONLY` cannot silently read +# as false because of a misspelt macro name. +add_compile_definitions(MEDKIT_OPCUA_READ_ONLY=${_medkit_opcua_read_only}) + # ---- MODULE target: loaded via dlopen at runtime by PluginManager ---- # Symbols from ros2_medkit_gateway are resolved from the host process at runtime. add_library(ros2_medkit_opcua_plugin MODULE @@ -122,6 +170,10 @@ target_include_directories(ros2_medkit_opcua_plugin PRIVATE target_compile_options(ros2_medkit_opcua_plugin PRIVATE -fvisibility=hidden -fvisibility-inlines-hidden + # Paired with --gc-sections on the link, so an unreachable function is not + # merely hidden but absent. + -ffunction-sections + -fdata-sections ) medkit_target_dependencies(ros2_medkit_opcua_plugin @@ -132,9 +184,22 @@ medkit_target_dependencies(ros2_medkit_opcua_plugin std_msgs ) -# Allow unresolved symbols - they resolve from the host process at runtime target_link_options(ros2_medkit_opcua_plugin PRIVATE + # Allow unresolved symbols - they resolve from the host process at runtime. -Wl,--unresolved-symbols=ignore-all + # -fvisibility=hidden covers the sources compiled into this module, but not + # the static archives it links. Without this, every global in the vendored + # open62541 lands in the module's dynamic symbol table, so the read-only + # object exported the library's own Write primitives and dlsym reached them + # even though no route did. ALL, not a list: the same holds for every archive + # the module happens to pull in. The plugin's extern "C" entry points come + # from object files, not archives, and keep their GATEWAY_PLUGIN_EXPORT + # visibility. + -Wl,--exclude-libs,ALL + # Then actually drop what nothing reaches. Retention starts from the dynamic + # symbol table, so this only becomes effective once --exclude-libs has emptied + # it of the library's globals - the two belong together. + -Wl,--gc-sections ) target_link_libraries(ros2_medkit_opcua_plugin @@ -180,6 +245,36 @@ if(BUILD_TESTING) include(ROS2MedkitTestDomain) + # ---- Build inspection: does the object match the variant it declares? ---- + # The acceptance for a read-only build is the built object, not a setting, so + # this reads the object with nm. It runs in BOTH variants with opposite + # expectations: asserting only "no write symbols" would also pass for a + # symbol list that matches nothing at all, and the write-capable direction is + # what catches that. The .so comes from a generator expression - a hard-coded + # build path would let the check pass against a stale file. + find_package(Python3 REQUIRED COMPONENTS Interpreter) + add_test(NAME test_opcua_build_variant + COMMAND "${Python3_EXECUTABLE}" + "${CMAKE_CURRENT_SOURCE_DIR}/test/inspect_build_variant.py" + "$" + --expect "${_medkit_opcua_variant}") + set_tests_properties(test_opcua_build_variant PROPERTIES TIMEOUT 60) + # Reads a file with nm; starts no ROS node. + medkit_test_needs_no_domain(test_opcua_build_variant) + + # A test of that test: it links an object exporting + # __UA_Client_writeAttribute alongside the plugin entry points - what a + # version script or a visibility slip on the vendored library produces - and + # requires the export rule to reject it. The leading underscores are what makes + # it worth asserting: a rule matching the prefix "UA_" lets that spelling + # through. + add_test(NAME test_opcua_build_variant_self_check + COMMAND "${Python3_EXECUTABLE}" + "${CMAKE_CURRENT_SOURCE_DIR}/test/inspect_build_variant.py" + --self-check) + set_tests_properties(test_opcua_build_variant_self_check PROPERTIES TIMEOUT 60) + medkit_test_needs_no_domain(test_opcua_build_variant_self_check) + # Each test connects to a non-existent OPC UA host and waits ~3.8s for the # DNS / TCP failure path; with 13 tests in the suite the run requires ~90s. # The default ament_add_gtest timeout (60s) is too tight, causing the runner @@ -455,7 +550,25 @@ if(BUILD_TESTING) install(PROGRAMS test/integration/gen_test_certs.sh test/integration/test_opcua_secured.test.py + test/integration/test_opcua_read_only.test.py DESTINATION lib/${PROJECT_NAME}) + + # ---- Read-only / write-capable behaviour over real HTTP ---------------- + # The build-inspection test proves the write symbols are gone; this proves + # what the running system then does with a node that the fixture server + # genuinely permits writing. The variant is passed from CMake so the test + # cannot be told at run time which build it is looking at. + medkit_add_wrapped_test(test_opcua_read_only + COMMAND "${Python3_EXECUTABLE}" + "${CMAKE_CURRENT_SOURCE_DIR}/test/integration/test_opcua_read_only.test.py" + "${CMAKE_BINARY_DIR}/test_alarm_server" + "${_medkit_opcua_variant}") + # Four gateways, each waiting on an OPC-UA connect, plus a server restart; + # the timeout has to exceed the sum of the script's own wait deadlines so a + # slow failing run still reaches its own diagnostics. + set_tests_properties(test_opcua_read_only PROPERTIES + LABELS "integration" + TIMEOUT 900) # This one boots a real gateway and fault manager, so it needs a domain, and # it builds its own command line - hence the generic wrapper rather than an # ament test runner. diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/README.md b/src/ros2_medkit_plugins/ros2_medkit_opcua/README.md index 0381ef765..a66b58490 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/README.md +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/README.md @@ -11,7 +11,11 @@ Follows the same plugin pattern as `ros2_medkit_graph_provider`: implements `Gat discovers them automatically by browsing the live address space (`auto_browse`, see below) - zero node-map config required - Exposes PLC values as the `x-plc-data` vendor collection -- Allows writing setpoints via `x-plc-operations` with type-aware coercion and range validation +- Ships read-only by default: the shipped binary contains no compiled path that + writes a value or acknowledges a condition, and exports nothing from the OPC UA + stack (see [Read-only and write-capable builds](#read-only-and-write-capable-builds)) +- In a write-capable build, allows writing setpoints via `x-plc-operations` with + type-aware coercion and range validation - Reports the connection state and poll metrics via `x-plc-status` - Maps threshold-based PLC alarms to SOVD faults on the owning entity - Optionally publishes numeric PLC values to ROS 2 `std_msgs/Float32` topics @@ -100,6 +104,85 @@ to `false`) like every other discovery resource - it is a device fingerprint, useful for reconnaissance. Enable gateway authentication (`auth.enabled: true`) or front the API with an authenticating proxy to gate access to it. +## Read-only and write-capable builds + +Whether this box can change a controller is a property of the binary, not of a +configuration file. `MEDKIT_OPCUA_READ_ONLY` is a CMake cache option and +defaults to **ON**: + +```bash +# The default: read-only. No OPC-UA write path in the object. +colcon build --packages-up-to ros2_medkit_opcua + +# Write-capable, for development against a PLC you are allowed to drive. +colcon build --packages-select ros2_medkit_opcua \ + --cmake-args -DMEDKIT_OPCUA_READ_ONLY=OFF +``` + +In the default read-only build: + +- **What is absent from the object**, each name asserted by `nm` in + `test_opcua_build_variant`: + - every C++ function that composes an OPC UA Write - `OpcuaClient::write_value`, + the vendor route handler `OpcuaPlugin::handle_plc_operations`, the value + coercion they share, and the `open62541pp` templates and service functions + they reach (`Node::writeValueScalar`, `Node::writeValue`, + `services::write`, `services::writeAttribute`); + - the entry point that issues a Part 9 condition method, + `OpcuaClient::call_condition_method`; + - open62541's own `UA_Client_write*` / `UA_Server_write*` primitives, dropped + by `-Wl,--exclude-libs,ALL` and `-Wl,--gc-sections` over + `-ffunction-sections -fdata-sections` because nothing references them once + the C++ write path is gone; + - every OPC UA symbol in the dynamic symbol table. The module exports the six + plugin entry points and C++ vague-linkage symbols, and nothing matching + `UA_*` or `opcua::*`, so `dlsym` reaches none of the machinery below. + +- **What remains inside the object, and why no route reaches it.** open62541 is + one static library, so removing the write path does not remove the transport + it shared. Still present: the generic request dispatcher + `__UA_Client_Service`, which the read, browse and ConditionRefresh paths all + use; the binary encoders (23 `*_encodeBinary` symbols), which serialize every + message type including the ones below; and the generated `UA_TYPES` + descriptors, which the table references as a whole so the linker cannot drop + individual entries - among them `WriteRequest`, `WriteValue`, `WriteResponse`, + `AddNodes`, `DeleteNodes`, `AddReferences`, `SetMonitoringMode`, + `SetPublishingMode` and `TransferSubscriptions` (`HistoryUpdate`: absent). + Descriptors are data, not a code path: no function in the object builds any of + those requests, none of these symbols is exported, and the plugin registers + three GET routes in a read-only build, so nothing in the REST contract + supplies a NodeId, a method id or an attribute id to reach them. + +- Two OPC UA calls that do change server-side state are deliberately **kept** in + both variants, because they change no controller data: `ConditionRefresh`, + which asks the server to replay conditions it already holds, and subscription + and monitored-item creation, which the read path needs to receive values and + alarms at all. +- No data point is ever `writable`. A node-map entry that says `writable: true` + is ignored with one startup warning naming the build property, and the + address-space walk never consults the server's `CurrentWrite` bit whatever + `infer_writable` says. +- Nothing advertises a write: no `x-plc-operations` capability on any entity, no + `set_` entry in `/operations`, and the `POST .../x-plc-operations/...` + route is not registered (it answers 404). +- **Alarm `acknowledge_fault` and `confirm_fault` are gone too.** A method call + that changes alarm state on the server is a write to the controller, and the + read-only build forbids every write path, not only value writes. The + `OpcuaClient` entry point that issues the Part 9 Acknowledge / Confirm calls + is not compiled, neither operation is listed on an event-alarm entity, and a + client that posts one anyway is refused. `ConditionRefresh` is unaffected and + stays in both variants: it asks the server to replay conditions it already + holds and changes nothing. +- `PUT /{type}/{id}/data/{name}` and + `POST /{type}/{id}/operations/{name}/executions` answer **501** before any + node lookup, condition lookup or client call, with vendor code + `x-medkit-plugin-error` and a message naming `MEDKIT_OPCUA_READ_ONLY`. 501 is + the status SOVD uses for an operation the entity does not support; 403 is + reserved for a valid token with insufficient permissions, and no credential + reaches a write path that is not in the binary. + +A write-capable build restores everything above; nothing else differs. + ## REST API ### Vendor Endpoints @@ -108,7 +191,7 @@ or front the API with an authenticating proxy to gate access to it. |--------|------|-------------| | GET | `/apps/{id}/x-plc-data` | All OPC-UA values for entity (with units, types, timestamps) | | GET | `/apps/{id}/x-plc-data/{name}` | Single data point value | -| POST | `/apps/{id}/x-plc-operations/set_{name}` | Write value to PLC (`{"value": 75.0}`) | +| POST | `/apps/{id}/x-plc-operations/set_{name}` | Write value to PLC (`{"value": 75.0}`) - write-capable build only; not registered otherwise | | GET | `/components/{id}/x-plc-status` | Connection state, poll stats, active alarms | ### Standard SOVD (provided by gateway) @@ -138,6 +221,9 @@ GET /api/v1/apps/tank_process/x-plc-data } ``` +`writable` is `false` on every item in the default read-only build, whatever the +node map or the server says. + **Write to PLC:** ```json POST /api/v1/apps/fill_pump/x-plc-operations/set_pump_speed @@ -210,7 +296,9 @@ nodes: display_name: Tank Level unit: mm data_type: float - writable: true # Allow writes via x-plc-operations + writable: true # Allow writes via x-plc-operations. Ignored in + # the default read-only build (one startup + # warning, the point stays read-only). min_value: 0.0 # Optional: range validation for writes max_value: 100.0 alarm: # Optional: numeric threshold -> SOVD fault @@ -342,8 +430,11 @@ that catch-all code is never raised for it (see "System messages" under `auto_alarms` below). A mapping-level `severity_override` / `message` overrides the source-level one; otherwise the source-level value is inherited. -The plugin auto-registers `acknowledge_fault` and `confirm_fault` operations -on every entity that has at least one `event_alarms` entry. Invoke them with: +In a write-capable build the plugin auto-registers `acknowledge_fault` and +`confirm_fault` operations on every entity that has at least one `event_alarms` +entry. The default read-only build offers neither and refuses both - see +[Read-only and write-capable builds](#read-only-and-write-capable-builds). +Invoke them with: ```bash curl -X POST http://localhost:8080/api/v1/apps/tank_process/operations/acknowledge_fault/executions \ @@ -384,10 +475,13 @@ along hierarchical references: `Objects` are walked normally. A depth limit and a total-node budget bound the walk against a pathological or very large address space; hitting either logs an operator warning that the resulting tree may be incomplete. -- **Read-only.** auto_browse never writes to the server, and every - auto-discovered data point loads with `writable: false` - promoting a - specific point to writable is a deliberate, reviewed decision made via an - explicit `nodes:` entry. +- **The walk itself never writes to the server.** Whether a discovered point is + exposed as writable depends on the build and on `infer_writable`: in the + default read-only build every auto-discovered point loads with + `writable: false` and the server's `CurrentWrite` bit is never read. In a + write-capable build `infer_writable` (default `true`) marks a point writable + exactly when the server says this session may write it; set it to `false` to + require an explicit `nodes:` entry instead. - **Explicit config always wins.** An auto-browsed entry for a NodeId that already has a hand-written `nodes:` entry is dropped; auto_browse only fills in what the node map does not already cover (or the whole tree, when there @@ -416,9 +510,17 @@ discovered endpoint go straight to a populated tree with no node-map file: ```yaml plugins.opcua.endpoint_url: "opc.tcp://192.168.1.10:4840" -plugins.opcua.auto_browse: true # or the same map form as above +plugins.opcua.auto_browse.enabled: true +plugins.opcua.auto_browse.infer_writable: true # write-capable builds only ``` +`infer_writable` is accepted in both forms - the node-map YAML's `auto_browse:` +block and the ROS param above. It defaults to `true`, and in a read-only build +it has no effect: every discovered point stays read-only. Setting it explicitly +there logs one startup warning naming the setting, so it can be found and +removed; leaving the key alone logs nothing, because the default is not a +request anybody made. + The JSON/ROS-param form takes precedence over whatever the node-map YAML's `auto_browse:` block set, mirroring how environment variables override the rest of the plugin's YAML config. @@ -805,23 +907,41 @@ Any PLC with an OPC-UA server works out of the box: source /opt/ros/jazzy/setup.bash colcon build --packages-select ros2_medkit_opcua colcon test --packages-select ros2_medkit_opcua + +# Write-capable variant (default is read-only) +colcon build --packages-select ros2_medkit_opcua \ + --cmake-args -DMEDKIT_OPCUA_READ_ONLY=OFF +colcon test --packages-select ros2_medkit_opcua ``` +`test_opcua_build_variant` inspects the built `.so` with `nm` and asserts it +matches the variant it was configured for, so the same `colcon test` command +checks the opposite property in each build. + ### Docker Integration Tests The plugin ships a self-contained OpenPLC tank demo in `docker/` that exercises the full stack end-to-end. CI runs this suite on every PR that touches the plugin; it is also runnable locally from any developer laptop. +`MEDKIT_OPCUA_VARIANT` selects the write surface of the image under test and +the expectations applied to it, so the two cannot drift. CI runs both legs. + ```bash cd src/ros2_medkit_plugins/ros2_medkit_opcua/docker -# Start OpenPLC + gateway (builds everything) +# Start OpenPLC + gateway (builds everything). Default: the read-only image. bash scripts/start.sh +MEDKIT_OPCUA_VARIANT=write-capable bash scripts/start.sh # Manual testing curl -s http://localhost:8080/api/v1/apps/tank_process/x-plc-data | jq . -# Automated tests (16 assertions) +# Automated tests against a running pair bash scripts/run_integration_tests.sh +MEDKIT_OPCUA_VARIANT=write-capable bash scripts/run_integration_tests.sh + +# Build, start, test and clean up in one go +bash scripts/test_all.sh +MEDKIT_OPCUA_VARIANT=write-capable bash scripts/test_all.sh # Stop bash scripts/stop.sh @@ -829,14 +949,16 @@ bash scripts/stop.sh ### Test Coverage -| Category | Tests | What it validates | -|----------|-------|-------------------| -| Entity discovery | 5 | Areas, components, apps from PLC node map | -| PLC connection | 2 | OPC-UA connected, zero errors | -| Live data | 3 | Tank level, temperature, pressure have values | -| Write control | 2 | Pump speed, valve position written to PLC | -| Error handling | 3 | Unknown entity, unknown operation, invalid JSON | -| **Total** | **16** | | +| Category | read-only | write-capable | What it validates | +|----------|-----------|---------------|-------------------| +| Entity discovery | 5 | 5 | Areas, components, apps from PLC node map | +| PLC connection | 2 | 2 | OPC-UA connected, zero errors | +| Live data | 3 | 3 | Tank level, temperature, pressure have values | +| Advertised write surface | 2 | 2 | x-plc-operations capability and the set_* operation, absent / present | +| Write control | 6 | 3 | read-only: the vendor route 404s, the SOVD write is refused with the vendor code and a message naming the build property, and both tags read back unchanged on the PLC. write-capable: pump speed and valve position written, pump speed read back | +| Error handling | 3 | 3 | Unknown entity, unknown operation, invalid JSON | +| SOVD /data | 2 | 2 | The standard data collection serves the same points | +| **Total** | **23** | **20** | | ## Security @@ -883,6 +1005,7 @@ When the value returns below threshold, the fault is automatically cleared. - **Type-aware writes** - Plugin reads the OPC-UA node's data type before writing to avoid type mismatches (e.g., writing float32 to a REAL node, not float64). - **Node map driven** - All entity mapping is in YAML config, not code. Same plugin binary works with any PLC by changing the config file. - **Env var overrides** - `OPCUA_ENDPOINT_URL` and `OPCUA_NODE_MAP_PATH` override YAML config for Docker deployment flexibility. +- **Read-only is a build property, not a setting** - the shipped binary contains no OPC-UA write path, and CI proves it by inspecting the object rather than by reading a configuration value. A setting can be flipped on a running box; an absent symbol cannot. ## License diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/design/index.rst b/src/ros2_medkit_plugins/ros2_medkit_opcua/design/index.rst index 8ee173e2e..8be7812c3 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/design/index.rst +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/design/index.rst @@ -57,7 +57,8 @@ All mapping lives in YAML (``config/tank_demo_nodes.yaml`` is the reference exam - ``node_id`` - OPC-UA node identifier (e.g. ``"ns=2;i=1"``) - ``entity_id`` - SOVD app the value belongs to - ``data_name`` - short name used in REST URLs - - ``display_name``, ``unit``, ``data_type``, ``writable`` - metadata + - ``display_name``, ``unit``, ``data_type``, ``writable`` - metadata. + ``writable`` is ignored in a read-only build (see below) - ``min_value`` / ``max_value`` - optional write range check - ``alarm`` - optional fault definition (``fault_code``, ``severity``, ``threshold``, ``above_threshold`` direction) @@ -96,8 +97,103 @@ edge-triggered callbacks: The plugin keeps per-fault state only long enough to detect edges; the fault manager owns persistence and fault lifecycle. -Type-aware writes -================= +Read-only is a property of the build +==================================== + +Four of the five protocol plugins cannot write to their device at all. OPC UA can, +and config-less discovery would mark a point writable straight from the server's +``CurrentWrite`` bit. A plant asking "can this box change a controller" cannot be +answered by a configuration value, because a configuration value can be changed +without rebuilding or shipping anything. So the answer is carried by the +binary. + +``MEDKIT_OPCUA_READ_ONLY`` is a CMake cache option, default ``ON``. With it on: + +- ``OpcuaClient::write_value``, the ``open62541pp`` Write service functions and + templates it reaches, the vendor route handler + ``OpcuaPlugin::handle_plc_operations`` and the value-coercion helper are all + outside ``#if`` and are never compiled. +- The link removes what the compiler alone cannot. ``-fvisibility=hidden`` covers + the sources compiled into the module but not the static archives it links, so + without further flags open62541's own ``UA_Client_write*`` primitives sit in the + object *and* in its dynamic symbol table, where a caller holding the ``.so`` + can ``dlsym`` one and drive a controller with it even though no route reaches + it. ``-Wl,--exclude-libs,ALL`` takes the archives out of the export table, and + ``-ffunction-sections -fdata-sections`` plus ``-Wl,--gc-sections`` then let the + linker drop them from the object entirely, because nothing references them once + the C++ write path is gone. The read-only object exports the six plugin entry + points and nothing from the OPC-UA stack. +- The section flags are set on ``open62541-object`` and ``open62541-plugins``, + which is where upstream compiles its C sources - ``open62541`` itself is + assembled from ``$`` and compiles nothing. Upstream adds the + same two flags, but only under ``Release`` and ``MinSizeRel``, so a default-type + build previously kept twelve ``UA_*_write*`` symbols in the read-only object as + local, unexported code. Setting them here makes the property independent of + ``CMAKE_BUILD_TYPE``, and ``test_opcua_build_variant`` asserts the absence of + that whole family so the difference cannot come back unnoticed. +- What is left of open62541 inside the object is one shared transport, not a + write path. open62541 is a single static library, so removing the write code + does not remove what it shared with the read code: the generic dispatcher + ``__UA_Client_Service`` (used by read, browse and ConditionRefresh), the binary + encoders (23 ``*_encodeBinary`` symbols), and the generated ``UA_TYPES`` + descriptors, which the table references as a whole so the linker cannot drop + individual entries. Measured on the read-only object, those descriptors include + ``WriteRequest``, ``WriteValue``, ``WriteResponse``, ``AddNodes``, + ``DeleteNodes``, ``AddReferences``, ``SetMonitoringMode``, + ``SetPublishingMode`` and ``TransferSubscriptions``; ``HistoryUpdate`` is + absent. + + Descriptors are data. What makes them unreachable is that no function in the + object composes any of those requests, none of these symbols is exported, and a + read-only build registers three GET routes - no route, node-map key or config + key supplies a NodeId, a method id or an attribute id. So the claim the package + makes is the exact one: every C++ path that composes a Write or a condition + method call is absent, and no OPC UA symbol is exported. Not "no OPC UA + machinery at all", which would be false. + +- Subscription and monitored-item creation stay in both variants. They change + server-side session state, which is not controller data, and the read path + cannot receive values or alarms without them. +- ``NodeMap::load`` forces ``writable`` to false and warns once when the file asked + otherwise; ``AutoBrowser`` does not compile the ``infer_writable`` inference, so + the server's ``CurrentWrite`` bit is never read. +- No entity registers the ``x-plc-operations`` capability, ``list_operations`` + emits no ``set_`` entry, and ``get_routes`` does not register the write + route. +- ``write_data`` and the value-write half of ``execute_operation`` return 501 as + their first statement, before any node lookup, with a message naming the build + property. SOVD spells "the entity does not support this" as 501 (fault + deletion, data lists, subscriptions and triggers all use it); 403 is defined + once, for a valid token with insufficient permissions, which is the one reading + that is wrong here - no credential reaches a path that is not in the binary. They stay declared because the gateway reaches the plugin through the + ``DataProvider`` / ``OperationProvider`` interfaces; the refusal reaches a client + as SOVD vendor code ``x-medkit-plugin-error``, which is the code the gateway + assigns to every plugin provider error. +- ``acknowledge_fault`` / ``confirm_fault`` go the same way. A method call that + changes alarm state on the server is a write to the controller, and the rule is + that a read-only build carries no write path at all, not merely no value writes. + ``OpcuaClient::call_condition_method`` - the entry point that issues the Part 9 + Acknowledge (i=9111) and Confirm (i=9113) calls - is compiled only in a + write-capable build, ``list_operations`` offers neither operation on an + event-alarm entity, and ``execute_operation`` refuses both by name before any + condition lookup. +- ``ConditionRefresh`` stays, and so does the generic ``OpcuaClient::call_method`` + it rides on. Part 9 5.5.7 makes it a request for the server to replay the + conditions it already holds to the calling subscription; it changes nothing on + the controller, and without it a restart loses the active fault set. That is why + the guard sits on the condition-method entry point rather than on ``call_method``, + and why neither ``call_method`` nor ``opcua::services::call`` is a write marker - + measured, they are present in both variants. + +The acceptance is an inspection of the built object, not a reading of the +configuration: ``test_opcua_build_variant`` runs ``nm`` over the plugin ``.so`` and +asserts the write symbols are absent, the read symbols present, the dynamic symbol +table free of OPC-UA machinery, and the six plugin entry points still exported. It +runs in both variants with opposite expectations, so a symbol list that stopped +matching anything fails in the write-capable build instead of passing everywhere. + +Type-aware writes (write-capable build) +======================================= ``POST /apps/{id}/x-plc-operations/{op}`` accepts a JSON body ``{"value": ...}``. The handler: @@ -366,7 +462,9 @@ Acknowledge / Confirm round-trip -------------------------------- Two SOVD operations appear on every entity that has at least one event-mode -alarm declared: +alarm declared, in a write-capable build. Both change condition state on the +controller, so the default read-only build neither lists nor performs them (see +"Read-only is a property of the build" above): - ``POST /apps/{entity}/operations/acknowledge_fault/executions`` - ``POST /apps/{entity}/operations/confirm_fault/executions`` diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/Dockerfile.gateway b/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/Dockerfile.gateway index 2887336b1..83cd2aa91 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/Dockerfile.gateway +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/Dockerfile.gateway @@ -7,8 +7,16 @@ FROM ros:jazzy-ros-base AS builder ENV DEBIAN_FRONTEND=noninteractive ENV COLCON_WS=/root/ws +# apt-get update fetches package indexes from the distribution mirrors and +# rosdep update fetches the distribution index from raw.githubusercontent.com. +# Both are network reads that a reset connection fails outright, and both run +# before anything of ours compiles, so a blip fails the whole build for a reason +# that has nothing to do with the change under test. Three attempts, five +# seconds apart; each attempt's own error text goes to stderr, so the last +# failure is what a reader sees. # hadolint ignore=DL3008 -RUN apt-get update && apt-get install -y --no-install-recommends \ +RUN retry() { n=1; while :; do "$@" && return 0; [ "$n" -ge 3 ] && return 1; echo "attempt $n of 3 failed: $*" >&2; n=$((n+1)); sleep 5; done; }; \ + retry apt-get update && apt-get install -y --no-install-recommends \ python3-colcon-common-extensions nlohmann-json3-dev libcpp-httplib-dev \ sqlite3 libsqlite3-dev libsystemd-dev libssl-dev libyaml-cpp-dev \ pkg-config git ros-jazzy-ament-cmake-gtest ros-jazzy-yaml-cpp-vendor \ @@ -29,14 +37,24 @@ WORKDIR ${COLCON_WS} # test_depend and which rosdep tries to install even with BUILD_TESTING=OFF). # This was previously masked by Docker layer cache hits on CI; cold builds # always failed. -RUN bash -c "source /opt/ros/jazzy/setup.bash && \ - apt-get update && \ - rosdep update && \ +# Same retry as above, for the same two commands. This is the RUN where a +# reset connection to raw.githubusercontent.com aborts the build. +RUN bash -c 'retry() { n=1; while :; do "$@" && return 0; [ "$n" -ge 3 ] && return 1; \ + echo "attempt $n of 3 failed: $*" >&2; n=$((n+1)); sleep 5; done; }; \ + source /opt/ros/jazzy/setup.bash && \ + retry apt-get update && \ + retry rosdep update && \ rosdep install --from-paths src --ignore-src -r -y \ - --skip-keys='ament_cmake_clang_format ament_cmake_clang_tidy test_msgs sqlite3 ros2_medkit_graph_provider python3-requests launch_testing_ament_cmake launch_testing launch_ros ament_index_python ros2_medkit_param_beacon nav2_msgs' && \ + --skip-keys="ament_cmake_clang_format ament_cmake_clang_tidy test_msgs sqlite3 ros2_medkit_graph_provider python3-requests launch_testing_ament_cmake launch_testing launch_ros ament_index_python ros2_medkit_param_beacon nav2_msgs" && \ rm -rf /var/lib/apt/lists/* && \ colcon build --cmake-args -DBUILD_TESTING=OFF \ - --packages-skip vda5050_agent ros2_medkit_vda5050_msgs ros2_medkit_opcua" + --packages-skip vda5050_agent ros2_medkit_vda5050_msgs ros2_medkit_opcua' + +# Which write surface the image ships. ON is the default the plugin itself +# defaults to: no controller write path in the object. OFF builds the +# write-capable image the OpenPLC suite uses to prove the write side still +# works; it is not a deployment configuration. +ARG MEDKIT_OPCUA_READ_ONLY=ON # Build the plugin separately so it picks up headers from the installed # ros2_medkit_gateway from the previous step. BUILD_TESTING=OFF avoids @@ -45,7 +63,8 @@ RUN bash -c "source /opt/ros/jazzy/setup.bash && \ RUN bash -c "source /opt/ros/jazzy/setup.bash && \ source ${COLCON_WS}/install/setup.bash && \ colcon build --packages-select ros2_medkit_opcua \ - --cmake-args -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTING=OFF" + --cmake-args -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTING=OFF \ + -DMEDKIT_OPCUA_READ_ONLY=${MEDKIT_OPCUA_READ_ONLY}" # Stage 2: Runtime FROM ros:jazzy-ros-base @@ -54,7 +73,8 @@ ENV DEBIAN_FRONTEND=noninteractive ENV COLCON_WS=/root/ws # hadolint ignore=DL3008 -RUN apt-get update && apt-get install -y --no-install-recommends \ +RUN retry() { n=1; while :; do "$@" && return 0; [ "$n" -ge 3 ] && return 1; echo "attempt $n of 3 failed: $*" >&2; n=$((n+1)); sleep 5; done; }; \ + retry apt-get update && apt-get install -y --no-install-recommends \ ros-jazzy-yaml-cpp-vendor ros-jazzy-example-interfaces \ nlohmann-json3-dev libcpp-httplib-dev sqlite3 libsqlite3-dev curl jq \ && rm -rf /var/lib/apt/lists/* diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/openplc/Dockerfile b/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/openplc/Dockerfile index 89b3fc8ed..b4d41d55c 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/openplc/Dockerfile +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/openplc/Dockerfile @@ -4,7 +4,13 @@ FROM debian:bookworm-slim AS matiec-builder -RUN apt-get update && apt-get install -y \ +# apt-get update fetches package indexes from the distribution mirrors, a +# network read that a reset connection fails outright and that runs before +# anything of ours is built. Three attempts, five seconds apart; each +# attempt's own error text goes to stderr, so the last failure is what a +# reader sees. +RUN retry() { n=1; while :; do "$@" && return 0; [ "$n" -ge 3 ] && return 1; echo "attempt $n of 3 failed: $*" >&2; n=$((n+1)); sleep 5; done; }; \ + retry apt-get update && apt-get install -y \ git build-essential flex bison autoconf automake \ && rm -rf /var/lib/apt/lists/* @@ -19,7 +25,9 @@ WORKDIR /openplc # Pin to specific commit for reproducible builds ENV OPENPLC_COMMIT=8a22c81ed6ddaba13225caec4a3ff15fd2c92909 -RUN apt-get update && apt-get install -y git ca-certificates curl jq && \ +# Same retry as above, same reason. +RUN retry() { n=1; while :; do "$@" && return 0; [ "$n" -ge 3 ] && return 1; echo "attempt $n of 3 failed: $*" >&2; n=$((n+1)); sleep 5; done; }; \ + retry apt-get update && apt-get install -y git ca-certificates curl jq && \ git clone https://github.com/Autonomy-Logic/openplc-runtime.git /openplc && \ cd /openplc && git checkout ${OPENPLC_COMMIT} && \ rm -rf /var/lib/apt/lists/* diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/scripts/run_alarm_tests.sh b/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/scripts/run_alarm_tests.sh index e8f09b6d1..b99a24b13 100755 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/scripts/run_alarm_tests.sh +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/scripts/run_alarm_tests.sh @@ -8,12 +8,35 @@ # # Acknowledge / Confirm round-trips go through the SOVD HTTP path # (POST /apps/{entity}/operations/{op}/executions) so that the medkit -# implementation - lookup_condition + EventId tracking + call_method on the -# inherited AcknowledgeableConditionType methods - is exercised end-to-end, -# not bypassed via the server stdin shortcuts. +# implementation - lookup_condition + EventId tracking + the condition-method +# call on the inherited AcknowledgeableConditionType methods - is exercised +# end-to-end, not bypassed via the server stdin shortcuts. +# +# MEDKIT_OPCUA_VARIANT selects the write surface of the image under test and +# has to match how it was built (docker build --build-arg +# MEDKIT_OPCUA_READ_ONLY=ON|OFF); this script builds the image itself, so it +# derives the build arg from the variable and the two cannot drift. It defaults +# to read-only, which is what the plugin and the Dockerfile default to. +# Acknowledging a condition changes its state on the controller, so a read-only +# image refuses it: that leg asserts the refusal and that the condition is still +# unacknowledged on the server, then drives the same lifecycle through the +# server's own CLI so every downstream expectation still runs. set -euo pipefail +VARIANT="${MEDKIT_OPCUA_VARIANT:-read-only}" +# SOVD spells "the entity does not support this" as 501; 403 is reserved for a +# valid token with insufficient permissions, which no credential can fix here. +REFUSAL_STATUS=501 +if [[ "${VARIANT}" == "read-only" ]]; then + READ_ONLY=ON +elif [[ "${VARIANT}" == "write-capable" ]]; then + READ_ONLY=OFF +else + echo "MEDKIT_OPCUA_VARIANT must be read-only or write-capable (got '${VARIANT}')" >&2 + exit 2 +fi + REPO_ROOT="$(cd "$(dirname "$0")/../../../../.." && pwd)" NET_NAME=alarm-test-net SERVER_NAME=alarm-test-server @@ -52,7 +75,7 @@ trap cleanup EXIT # existing run_integration_tests.sh convention; never sleeps blindly. wait_for() { local url="$1" expr="$2" deadline="${3:-60}" - for i in $(seq 1 "${deadline}"); do + for _ in $(seq 1 "${deadline}"); do if curl -sf "${url}" 2>/dev/null | jq -e "${expr}" >/dev/null 2>&1; then return 0 fi @@ -71,7 +94,7 @@ wait_for() { wait_no_fault() { local fault_code="$1" deadline="${2:-30}" local url="http://localhost:${GATEWAY_PORT}/api/v1/faults" - for i in $(seq 1 "${deadline}"); do + for _ in $(seq 1 "${deadline}"); do local status status=$(curl -sf "${url}" 2>/dev/null \ | jq -r --arg code "${fault_code}" \ @@ -101,12 +124,26 @@ assert_status() { echo " OK ${fault_code}: ${actual}" } +assert_fault_present() { + local fault_code="$1" + local status + status=$(curl -sf "http://localhost:${GATEWAY_PORT}/api/v1/faults" \ + | jq -r --arg code "${fault_code}" \ + '.items[] | select(.fault_code == $code) | .status' \ + | head -1) + if [[ -z "${status}" || "${status}" == "CLEARED" ]]; then + echo "ASSERT FAILED: ${fault_code} expected still raised, got '${status:-absent}'" >&2 + return 1 + fi + echo " OK ${fault_code} still raised (${status})" +} + # Poll the global ``/api/v1/faults`` list until the named fault has the # expected status. Mirrors ``wait_for`` but specialized for the fault list # shape so callers do not need to construct jq filters per scenario. wait_until_status() { local fault_code="$1" expected="$2" deadline="${3:-30}" - for i in $(seq 1 "${deadline}"); do + for _ in $(seq 1 "${deadline}"); do local actual actual=$(curl -sf "http://localhost:${GATEWAY_PORT}/api/v1/faults" 2>/dev/null \ | jq -r --arg code "${fault_code}" \ @@ -129,7 +166,7 @@ wait_until_status() { # corresponding state on the OPC-UA server. assert_server_state() { local condition="$1" key="$2" expected="$3" deadline="${4:-30}" - for i in $(seq 1 "${deadline}"); do + for _ in $(seq 1 "${deadline}"); do local line line=$(docker logs "${SERVER_NAME}" 2>&1 | grep -E "^STATE ${condition} " | tail -1 || true) if [[ "${line}" == *"${key}=${expected}"* ]]; then @@ -157,6 +194,56 @@ sovd_post_op() { echo " OK POST ${op} -> ${code}" } +# The read-only counterpart: the same POST has to come back as the plugin's +# refusal, not as a transport error or a 404, and the body has to name the build +# property so an operator reading it knows no credential will help. +sovd_post_op_refused() { + local op="$1" body="$2" + local url="http://localhost:${GATEWAY_PORT}/api/v1/apps/tank_process/operations/${op}/executions" + local code + code=$(curl -s -o /tmp/alarm_test_resp.json -w '%{http_code}' \ + -X POST -H 'Content-Type: application/json' -d "${body}" "${url}") + if [[ "${code}" != "${REFUSAL_STATUS}" ]]; then + echo "ASSERT FAILED: POST ${op} expected HTTP ${REFUSAL_STATUS}, got ${code}" >&2 + cat /tmp/alarm_test_resp.json >&2 || true + return 1 + fi + if ! jq -e '.vendor_code == "x-medkit-plugin-error"' /tmp/alarm_test_resp.json >/dev/null; then + echo "ASSERT FAILED: POST ${op} refusal carries no x-medkit-plugin-error vendor code" >&2 + cat /tmp/alarm_test_resp.json >&2 || true + return 1 + fi + if ! jq -e '.message | test("MEDKIT_OPCUA_READ_ONLY")' /tmp/alarm_test_resp.json >/dev/null; then + echo "ASSERT FAILED: POST ${op} refusal does not name MEDKIT_OPCUA_READ_ONLY" >&2 + cat /tmp/alarm_test_resp.json >&2 || true + return 1 + fi + echo " OK POST ${op} refused ${REFUSAL_STATUS} x-medkit-plugin-error naming MEDKIT_OPCUA_READ_ONLY" +} + +# What the tree advertises: a read-only image must not offer an operation it +# will refuse, and a write-capable one must offer both. +assert_operations_offered() { + local url="http://localhost:${GATEWAY_PORT}/api/v1/apps/tank_process/operations" + local ids + ids=$(curl -s "${url}" | jq -c '[.items[].id]') + for op in acknowledge_fault confirm_fault; do + if [[ "${VARIANT}" == "read-only" ]]; then + if jq -e --arg op "${op}" 'contains([$op])' <<<"${ids}" >/dev/null; then + echo "ASSERT FAILED: read-only image advertises ${op} (${ids})" >&2 + return 1 + fi + echo " OK ${op} not advertised" + else + if ! jq -e --arg op "${op}" 'contains([$op])' <<<"${ids}" >/dev/null; then + echo "ASSERT FAILED: write-capable image does not advertise ${op} (${ids})" >&2 + return 1 + fi + echo " OK ${op} advertised" + fi + done +} + # Poll the gateway's docker logs until appears. Required because the # AlarmConditionType subscription has a 500 ms server-side publishing interval - # new events from method calls or stdin commands take up to that long to arrive @@ -165,7 +252,7 @@ sovd_post_op() { # rejects stale IDs with BadEventIdUnknown). wait_gateway_log() { local pattern="$1" deadline="${2:-30}" - for i in $(seq 1 "${deadline}"); do + for _ in $(seq 1 "${deadline}"); do if docker logs "${GATEWAY_NAME}" 2>&1 | grep -q -- "${pattern}"; then return 0 fi @@ -190,8 +277,9 @@ docker build --network=host \ -f src/ros2_medkit_plugins/ros2_medkit_opcua/docker/test_alarm_server/Dockerfile \ -t ros2_medkit_alarm_test_server:dev . >/dev/null -echo "[2/5] Build gateway-opcua image (re-uses existing Dockerfile.gateway)" +echo "[2/5] Build gateway-opcua image (${VARIANT}, re-uses existing Dockerfile.gateway)" docker build --network=host \ + --build-arg "MEDKIT_OPCUA_READ_ONLY=${READ_ONLY}" \ -f src/ros2_medkit_plugins/ros2_medkit_opcua/docker/Dockerfile.gateway \ -t gateway-opcua:alarm-test . >/dev/null @@ -215,7 +303,7 @@ docker run --rm --name "${SERVER_NAME}" --network "${NET_NAME}" \ <&3 >/dev/null 2>&1 & SERVER_DOCKER_PID=$! # Wait for server to bind; the binary prints "READY ..." after listen. -for i in $(seq 1 30); do +for _ in $(seq 1 30); do if docker logs "${SERVER_NAME}" 2>&1 | grep -q '^READY '; then break fi @@ -299,7 +387,7 @@ docker run -d --name "${GATEWAY_NAME}" --network "${NET_NAME}" \ # sometimes too short, leaving the gateway to come up before the # service was discoverable. ``ros2 service list`` is the cheapest # ROS-native availability signal. - for i in $(seq 1 30); do + for _ in $(seq 1 30); do if ros2 service list 2>/dev/null | grep -q "/fault_manager/report_fault"; then break fi @@ -320,7 +408,8 @@ wait_for "http://localhost:${GATEWAY_PORT}/api/v1/apps" \ echo "[5/5] Run alarm scenarios" -echo " [scenario] fire / SOVD ack / latch / SOVD confirm / clear lifecycle" +echo " [scenario] fire / ack / latch / confirm / clear lifecycle (${VARIANT})" +assert_operations_offered echo "fire Overpressure 750" >&3 wait_until_status PLC_OVERPRESSURE CONFIRMED 30 @@ -332,31 +421,56 @@ wait_until_status PLC_OVERPRESSURE CONFIRMED 30 # HEALED verb - we deliberately do not flip fault_manager into PASSED-debounce # territory). The lifecycle proof is therefore ``wait_no_fault`` after the # follow-up SOVD confirm + the OPC-UA event with all three states cleared. -sovd_post_op acknowledge_fault \ - '{"fault_code":"PLC_OVERPRESSURE","comment":"e2e ack via SOVD"}' - -# Latch flips ActiveState=false on the server. Combined with the AckedState= -# true set by the SOVD ack above, the next AlarmCondition event payload has -# active=false, acked=true, confirmed=false -> SovdAlarmStatus::Healed -# (state machine internal), action=ReportHealed (no-op for fault_manager). -# /faults still shows CONFIRMED here, by design. -echo "latch Overpressure" >&3 - -# Wait for the gateway to actually receive and process the latch event before -# issuing SOVD confirm. Without this, the gateway still has the EventId from -# the original fire payload and the OPC-UA Confirm method on the server -# returns BadEventIdUnknown (the server's branch->lastEventId has been -# superseded by the Acknowledge auto-emit and the latch trigger). -wait_gateway_log "AlarmCondition HEALED.*PLC_OVERPRESSURE" 20 - -# Real SOVD confirm - exercises call_method(i=9113) + EventId. After this -# ConfirmedState=true on the server; the resulting event has all three of -# Active=false, Acked=true, Confirmed=true and the state machine emits -# ClearFault, removing the entry from /faults. -sovd_post_op confirm_fault \ - '{"fault_code":"PLC_OVERPRESSURE","comment":"e2e confirm via SOVD"}' -wait_no_fault PLC_OVERPRESSURE 30 -echo " OK PLC_OVERPRESSURE cleared after SOVD ack + latch + SOVD confirm" +if [[ "${VARIANT}" == "read-only" ]]; then + # The image carries no way to acknowledge or confirm, so both calls are + # refused. Proving they never reached the server needs a consequence rather + # than a STATE line: the fixture prints those only from its own stdin CLI, so + # a condition acknowledged over OPC-UA looks identical to one that was not. + # The consequence used here is that the fault does not clear - had both calls + # landed, the latch below would have driven it to cleared - and the CLI-driven + # ack + confirm afterwards does clear it, which is what makes the negative + # meaningful instead of a symptom of a stalled pipeline. + sovd_post_op_refused acknowledge_fault \ + '{"fault_code":"PLC_OVERPRESSURE","comment":"e2e ack via SOVD"}' + sovd_post_op_refused confirm_fault \ + '{"fault_code":"PLC_OVERPRESSURE","comment":"e2e confirm via SOVD"}' + + echo "latch Overpressure" >&3 + wait_gateway_log "AlarmCondition HEALED.*PLC_OVERPRESSURE" 20 + assert_fault_present PLC_OVERPRESSURE + echo " OK the refused acknowledge never reached the server" + + echo "ack Overpressure" >&3 + echo "confirm Overpressure" >&3 + wait_no_fault PLC_OVERPRESSURE 30 + echo " OK PLC_OVERPRESSURE cleared once the server itself acked and confirmed" +else + sovd_post_op acknowledge_fault \ + '{"fault_code":"PLC_OVERPRESSURE","comment":"e2e ack via SOVD"}' + + # Latch flips ActiveState=false on the server. Combined with the AckedState= + # true set by the SOVD ack above, the next AlarmCondition event payload has + # active=false, acked=true, confirmed=false -> SovdAlarmStatus::Healed + # (state machine internal), action=ReportHealed (no-op for fault_manager). + # /faults still shows CONFIRMED here, by design. + echo "latch Overpressure" >&3 + + # Wait for the gateway to actually receive and process the latch event before + # issuing SOVD confirm. Without this, the gateway still has the EventId from + # the original fire payload and the OPC-UA Confirm method on the server + # returns BadEventIdUnknown (the server's branch->lastEventId has been + # superseded by the Acknowledge auto-emit and the latch trigger). + wait_gateway_log "AlarmCondition HEALED.*PLC_OVERPRESSURE" 20 + + # Real SOVD confirm - exercises call_method(i=9113) + EventId. After this + # ConfirmedState=true on the server; the resulting event has all three of + # Active=false, Acked=true, Confirmed=true and the state machine emits + # ClearFault, removing the entry from /faults. + sovd_post_op confirm_fault \ + '{"fault_code":"PLC_OVERPRESSURE","comment":"e2e confirm via SOVD"}' + wait_no_fault PLC_OVERPRESSURE 30 + echo " OK PLC_OVERPRESSURE cleared after SOVD ack + latch + SOVD confirm" +fi echo " [scenario] shelving suppression" echo "fire Overheat 600" >&3 @@ -434,7 +548,7 @@ docker run --rm --name "${SERVER_NAME}" --network "${NET_NAME}" \ -i ros2_medkit_alarm_test_server:dev --port "${SERVER_PORT}" \ <&3 >/dev/null 2>&1 & SERVER_DOCKER_PID=$! -for i in $(seq 1 30); do +for _ in $(seq 1 30); do if docker logs "${SERVER_NAME}" 2>&1 | grep -q '^READY '; then break fi diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/scripts/run_integration_tests.sh b/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/scripts/run_integration_tests.sh index f48a5d2e5..c11f7c87d 100755 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/scripts/run_integration_tests.sh +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/scripts/run_integration_tests.sh @@ -1,12 +1,26 @@ #!/usr/bin/env bash # OpenPLC Tank Demo - Integration Tests -# Validates: entity discovery, live data, control writes, error handling +# Validates: entity discovery, live data, the write surface the image was built +# with, and error handling. +# +# MEDKIT_OPCUA_VARIANT selects which write surface is expected, and it has to +# match how the gateway image was built (docker build --build-arg +# MEDKIT_OPCUA_READ_ONLY=ON|OFF). It defaults to read-only because that is what +# the plugin and the Dockerfile default to; against a default image the suite +# proves the read-only contract on a real PLC - the refusal, its vendor code, +# and that the tag did not move - rather than skipping the write section. set -o pipefail API="${GATEWAY_URL:-http://localhost:8080}/api/v1" +VARIANT="${MEDKIT_OPCUA_VARIANT:-read-only}" PASS=0 FAIL=0 +if [ "$VARIANT" != "read-only" ] && [ "$VARIANT" != "write-capable" ]; then + echo "MEDKIT_OPCUA_VARIANT must be read-only or write-capable (got '$VARIANT')" >&2 + exit 2 +fi + RED='\033[0;31m' GREEN='\033[0;32m' YELLOW='\033[1;33m' @@ -23,7 +37,7 @@ assert() { fi } -echo -e "${YELLOW}=== OpenPLC Tank Demo Integration Tests ===${NC}\n" +echo -e "${YELLOW}=== OpenPLC Tank Demo Integration Tests (${VARIANT} image) ===${NC}\n" # 1. Wait for gateway + PLC entities echo -e "${YELLOW}1. Wait for gateway + PLC entities${NC}" @@ -65,23 +79,77 @@ assert "tank_temperature has value" "$([ -n "$TEMP" ] && [ "$TEMP" != "null" ] & assert "tank_pressure has value" "$([ -n "$PRESS" ] && [ "$PRESS" != "null" ] && echo true || echo false)" echo " Level=$LEVEL mm, Temp=$TEMP C, Pressure=$PRESS bar" -# 5. Write - Pump Speed -echo -e "\n${YELLOW}5. Write Pump Speed${NC}" -WRITE=$(curl -s -X POST "$API/apps/fill_pump/x-plc-operations/set_pump_speed" \ - -H "Content-Type: application/json" -d '{"value": 75.0}') -assert "Write pump speed OK" "$(echo "$WRITE" | jq '.status == "ok"' 2>/dev/null)" -sleep 5 -PUMP=$(curl -s "$API/apps/fill_pump/x-plc-data" | jq '.items[] | select(.name == "pump_speed") | .value' 2>/dev/null) -assert "Pump speed ~= 75" "$(echo "$PUMP" | jq '. >= 74 and . <= 76' 2>/dev/null)" - -# 6. Write - Valve Position -echo -e "\n${YELLOW}6. Write Valve Position${NC}" -WRITE=$(curl -s -X POST "$API/apps/drain_valve/x-plc-operations/set_valve_position" \ - -H "Content-Type: application/json" -d '{"value": 50.0}') -assert "Write valve position OK" "$(echo "$WRITE" | jq '.status == "ok"' 2>/dev/null)" - -# 7. Error Handling -echo -e "\n${YELLOW}7. Error Handling${NC}" +# 5. What the tree advertises about writing +echo -e "\n${YELLOW}5. Advertised write surface${NC}" +CAPS=$(curl -s "$API/apps/fill_pump" | jq '[.capabilities[].name]' 2>/dev/null) +OPS=$(curl -s "$API/apps/fill_pump/operations" | jq '[.items[].id]' 2>/dev/null) +if [ "$VARIANT" = "read-only" ]; then + assert "no x-plc-operations capability" \ + "$(echo "$CAPS" | jq 'contains(["x-plc-operations"]) | not' 2>/dev/null)" + assert "no set_pump_speed operation" \ + "$(echo "$OPS" | jq 'contains(["set_pump_speed"]) | not' 2>/dev/null)" +else + assert "x-plc-operations capability" \ + "$(echo "$CAPS" | jq 'contains(["x-plc-operations"])' 2>/dev/null)" + assert "set_pump_speed operation" \ + "$(echo "$OPS" | jq 'contains(["set_pump_speed"])' 2>/dev/null)" +fi + +# 6. Write - Pump Speed +echo -e "\n${YELLOW}6. Write Pump Speed${NC}" +PUMP_BEFORE=$(curl -s "$API/apps/fill_pump/x-plc-data" | jq '.items[] | select(.name == "pump_speed") | .value' 2>/dev/null) +if [ "$VARIANT" = "read-only" ]; then + # The vendor route is not registered at all, so this is the gateway's own + # 404 rather than a plugin refusal. + WRITE=$(curl -s -X POST "$API/apps/fill_pump/x-plc-operations/set_pump_speed" \ + -H "Content-Type: application/json" -d '{"value": 75.0}') + assert "x-plc-operations set_pump_speed not routed" \ + "$(echo "$WRITE" | jq '.error_code == "resource-not-found"' 2>/dev/null)" + # The SOVD write endpoint does reach the plugin, and the plugin refuses. + PUT=$(curl -s -X PUT "$API/apps/fill_pump/data/pump_speed" \ + -H "Content-Type: application/json" -d '{"value": 75.0}') + assert "PUT pump_speed refused with the vendor code" \ + "$(echo "$PUT" | jq '.vendor_code == "x-medkit-plugin-error"' 2>/dev/null)" + assert "refusal names MEDKIT_OPCUA_READ_ONLY" \ + "$(echo "$PUT" | jq '.message | test("MEDKIT_OPCUA_READ_ONLY")' 2>/dev/null)" + sleep 5 + PUMP=$(curl -s "$API/apps/fill_pump/x-plc-data" | jq '.items[] | select(.name == "pump_speed") | .value' 2>/dev/null) + # The tag did not move. Tolerance rather than equality because the value is + # a float coming back through the PLC; a write of 75 from rest is far larger + # than any representation jitter. + assert "pump_speed unchanged on the PLC" \ + "$(jq -n --argjson a "${PUMP_BEFORE:-null}" --argjson b "${PUMP:-null}" \ + '($a != null) and ($b != null) and (($a - $b) | fabs < 0.5)' 2>/dev/null)" +else + WRITE=$(curl -s -X POST "$API/apps/fill_pump/x-plc-operations/set_pump_speed" \ + -H "Content-Type: application/json" -d '{"value": 75.0}') + assert "Write pump speed OK" "$(echo "$WRITE" | jq '.status == "ok"' 2>/dev/null)" + sleep 5 + PUMP=$(curl -s "$API/apps/fill_pump/x-plc-data" | jq '.items[] | select(.name == "pump_speed") | .value' 2>/dev/null) + assert "Pump speed ~= 75" "$(echo "$PUMP" | jq '. >= 74 and . <= 76' 2>/dev/null)" +fi + +# 7. Write - Valve Position +echo -e "\n${YELLOW}7. Write Valve Position${NC}" +VALVE_BEFORE=$(curl -s "$API/apps/drain_valve/x-plc-data" | jq '.items[] | select(.name == "valve_position") | .value' 2>/dev/null) +if [ "$VARIANT" = "read-only" ]; then + PUT=$(curl -s -X PUT "$API/apps/drain_valve/data/valve_position" \ + -H "Content-Type: application/json" -d '{"value": 50.0}') + assert "PUT valve_position refused with the vendor code" \ + "$(echo "$PUT" | jq '.vendor_code == "x-medkit-plugin-error"' 2>/dev/null)" + sleep 5 + VALVE=$(curl -s "$API/apps/drain_valve/x-plc-data" | jq '.items[] | select(.name == "valve_position") | .value' 2>/dev/null) + assert "valve_position unchanged on the PLC" \ + "$(jq -n --argjson a "${VALVE_BEFORE:-null}" --argjson b "${VALVE:-null}" \ + '($a != null) and ($b != null) and (($a - $b) | fabs < 0.5)' 2>/dev/null)" +else + WRITE=$(curl -s -X POST "$API/apps/drain_valve/x-plc-operations/set_valve_position" \ + -H "Content-Type: application/json" -d '{"value": 50.0}') + assert "Write valve position OK" "$(echo "$WRITE" | jq '.status == "ok"' 2>/dev/null)" +fi + +# 8. Error Handling +echo -e "\n${YELLOW}8. Error Handling${NC}" assert "404 unknown entity" "$(curl -s "$API/apps/nonexistent/x-plc-data" | jq 'has("error_code")' 2>/dev/null)" assert "404 unknown operation" "$(curl -s -X POST "$API/apps/tank_process/x-plc-operations/nonexistent" -H "Content-Type: application/json" -d '{"value":1}' | jq 'has("error_code")' 2>/dev/null)" assert "400 invalid JSON" "$(curl -s -X POST "$API/apps/fill_pump/x-plc-operations/set_pump_speed" -H "Content-Type: application/json" -d 'bad' | jq 'has("error_code")' 2>/dev/null)" @@ -98,9 +166,11 @@ SOVD_DATA=$(curl -s "$API/apps/tank_process/data" 2>/dev/null) assert "SOVD /data returns items" "$(echo "$SOVD_DATA" | jq 'has("items")' 2>/dev/null)" assert "SOVD /data has tank_level" "$(echo "$SOVD_DATA" | jq '[.items[].id] | contains(["tank_level"])' 2>/dev/null)" -# Cleanup - stop pump -curl -s -X POST "$API/apps/fill_pump/x-plc-operations/set_pump_speed" \ - -H "Content-Type: application/json" -d '{"value": 0}' >/dev/null 2>&1 +# Cleanup - stop pump. Only the write-capable image can, and only it moved it. +if [ "$VARIANT" = "write-capable" ]; then + curl -s -X POST "$API/apps/fill_pump/x-plc-operations/set_pump_speed" \ + -H "Content-Type: application/json" -d '{"value": 0}' >/dev/null 2>&1 +fi echo -e "\n${YELLOW}===== Test Summary =====${NC}" TOTAL=$((PASS + FAIL)) diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/scripts/start.sh b/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/scripts/start.sh index f80d00fbc..1c942a3c5 100755 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/scripts/start.sh +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/scripts/start.sh @@ -2,8 +2,21 @@ # Start OpenPLC + medkit gateway for manual testing. # Usage: from the ros2_medkit repo root, run # bash src/ros2_medkit_plugins/ros2_medkit_opcua/docker/scripts/start.sh +# +# MEDKIT_OPCUA_VARIANT=write-capable brings up an image with the OPC-UA write +# path compiled in. The default, read-only, is what ships. set -eo pipefail +VARIANT="${MEDKIT_OPCUA_VARIANT:-read-only}" +if [ "$VARIANT" = "read-only" ]; then + READ_ONLY=ON +elif [ "$VARIANT" = "write-capable" ]; then + READ_ONLY=OFF +else + echo "MEDKIT_OPCUA_VARIANT must be read-only or write-capable (got '$VARIANT')" >&2 + exit 2 +fi + SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" DOCKER_DIR="$(dirname "$SCRIPT_DIR")" PLUGIN_DIR="$(dirname "$DOCKER_DIR")" @@ -15,9 +28,10 @@ echo "=== Building OpenPLC ===" docker build -t openplc-tank "$DOCKER_DIR/openplc" 2>&1 | tail -3 echo "" -echo "=== Building gateway + OPC-UA plugin ===" +echo "=== Building gateway + OPC-UA plugin ($VARIANT) ===" cd "$REPO_ROOT" -docker build -f "$DOCKER_DIR/Dockerfile.gateway" -t gateway-opcua . 2>&1 | tail -5 +docker build --build-arg "MEDKIT_OPCUA_READ_ONLY=$READ_ONLY" \ + -f "$DOCKER_DIR/Dockerfile.gateway" -t gateway-opcua . 2>&1 | tail -5 echo "" echo "=== Starting containers ===" diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/scripts/test_all.sh b/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/scripts/test_all.sh index a8520c931..121938630 100755 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/scripts/test_all.sh +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/scripts/test_all.sh @@ -3,8 +3,23 @@ # assertions against the OpenPLC tank demo, then clean up. # Usage: from the ros2_medkit repo root, run # bash src/ros2_medkit_plugins/ros2_medkit_opcua/docker/scripts/test_all.sh +# +# MEDKIT_OPCUA_VARIANT selects the write surface of the image under test +# (read-only, the default and what ships, or write-capable). It drives both the +# image build and the expectations the suite applies, so the two cannot drift. set -eo pipefail +VARIANT="${MEDKIT_OPCUA_VARIANT:-read-only}" +if [ "$VARIANT" = "read-only" ]; then + READ_ONLY=ON +elif [ "$VARIANT" = "write-capable" ]; then + READ_ONLY=OFF +else + echo "MEDKIT_OPCUA_VARIANT must be read-only or write-capable (got '$VARIANT')" >&2 + exit 2 +fi +export MEDKIT_OPCUA_VARIANT="$VARIANT" + SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" DOCKER_DIR="$(dirname "$SCRIPT_DIR")" PLUGIN_DIR="$(dirname "$DOCKER_DIR")" @@ -27,9 +42,10 @@ echo -e "${YELLOW}Step 1: Build OpenPLC container${NC}" docker build -t openplc-tank "$DOCKER_DIR/openplc" 2>&1 | tail -3 # 2. Build gateway image (includes ros2_medkit_opcua plugin) -echo -e "\n${YELLOW}Step 2: Build gateway + OPC-UA plugin image${NC}" +echo -e "\n${YELLOW}Step 2: Build gateway + OPC-UA plugin image (${VARIANT})${NC}" cd "$REPO_ROOT" -docker build -f "$DOCKER_DIR/Dockerfile.gateway" -t gateway-opcua . 2>&1 | tail -5 +docker build --build-arg "MEDKIT_OPCUA_READ_ONLY=$READ_ONLY" \ + -f "$DOCKER_DIR/Dockerfile.gateway" -t gateway-opcua . 2>&1 | tail -5 # 3. Start containers on isolated network echo -e "\n${YELLOW}Step 3: Start containers${NC}" diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/test_alarm_server/Dockerfile b/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/test_alarm_server/Dockerfile index e21c6a5ce..e07993407 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/test_alarm_server/Dockerfile +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/docker/test_alarm_server/Dockerfile @@ -12,7 +12,13 @@ FROM ubuntu:24.04 AS builder ENV DEBIAN_FRONTEND=noninteractive -RUN apt-get update && apt-get install -y --no-install-recommends \ +# apt-get update fetches package indexes from the distribution mirrors, a +# network read that a reset connection fails outright and that runs before +# anything of ours is built. Three attempts, five seconds apart; each +# attempt's own error text goes to stderr, so the last failure is what a +# reader sees. +RUN retry() { n=1; while :; do "$@" && return 0; [ "$n" -ge 3 ] && return 1; echo "attempt $n of 3 failed: $*" >&2; n=$((n+1)); sleep 5; done; }; \ + retry apt-get update && apt-get install -y --no-install-recommends \ build-essential cmake git python3 ca-certificates libssl-dev \ && rm -rf /var/lib/apt/lists/* @@ -51,7 +57,9 @@ RUN g++ -O2 -std=c++17 -w \ -lssl -lcrypto -lpthread -o /opt/test_alarm_server FROM ubuntu:24.04 -RUN apt-get update && apt-get install -y --no-install-recommends \ +# Same retry as above, same reason. +RUN retry() { n=1; while :; do "$@" && return 0; [ "$n" -ge 3 ] && return 1; echo "attempt $n of 3 failed: $*" >&2; n=$((n+1)); sleep 5; done; }; \ + retry apt-get update && apt-get install -y --no-install-recommends \ libstdc++6 ca-certificates libssl3 openssl \ && rm -rf /var/lib/apt/lists/* COPY --from=builder /opt/test_alarm_server /usr/local/bin/test_alarm_server diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/node_map.hpp b/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/node_map.hpp index 0686b19fd..6e2ef92ba 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/node_map.hpp +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/node_map.hpp @@ -263,6 +263,12 @@ struct AutoBrowseConfig { /// permits it. When false, every auto-browsed point stays read-only and only /// an explicit node_map ``nodes:`` entry can promote it (legacy behaviour). bool infer_writable{true}; + + /// Where an explicit ``infer_writable`` came from, for a message that can + /// point at the setting to change. Empty when nobody wrote the key, which is + /// what separates "the operator asked for this" from "this is the default" - + /// a read-only build says nothing about a key that was never set. + std::string infer_writable_source; }; /// SOVD entity definition derived from the node map diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_client.hpp b/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_client.hpp index d617108dc..9888798ca 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_client.hpp +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_client.hpp @@ -174,6 +174,13 @@ class OpcuaClient { /// Read multiple values std::vector read_values(const std::vector & node_ids); +#if !MEDKIT_OPCUA_READ_ONLY + /// The value-write surface, present only in a write-capable build + /// (``-DMEDKIT_OPCUA_READ_ONLY=OFF``). A read-only build has no declaration + /// and no definition, so nothing in the plugin can reach an OPC-UA Write + /// service call - see the package README on why that is a build property and + /// not a setting. + /// OPC-UA write error classification enum class WriteError { NotConnected, TypeMismatch, AccessDenied, NodeNotFound, TransportError }; @@ -190,6 +197,7 @@ class OpcuaClient { /// @return void on success, WriteErrorInfo on failure with specific error code tl::expected write_value(const opcua::NodeId & node_id, const OpcuaValue & value, const std::string & data_type_hint = ""); +#endif /// The AccessLevel / UserAccessLevel bits of a Variable node, read straight /// from the server. ``ok`` is false when not connected or the attribute read @@ -295,13 +303,34 @@ class OpcuaClient { }; /// Synchronously call an OPC-UA Method on a target object. - /// Used by ConditionRefresh, Acknowledge, and Confirm operations on - /// AlarmConditionType nodes (issue #386). + /// The read path uses it for ConditionRefresh, which asks the server to + /// replay active conditions to this subscription and changes nothing on the + /// controller (Part 9 5.5.7). /// @return Output arguments on success, MethodErrorInfo on failure. tl::expected, MethodErrorInfo> call_method(const opcua::NodeId & object_id, const opcua::NodeId & method_id, const std::vector & input_args); +#if !MEDKIT_OPCUA_READ_ONLY + /// Acknowledge or Confirm a live condition (Part 9 Acknowledge i=9111, + /// Confirm i=9113), present only in a write-capable build. + /// + /// Separate from ``call_method`` because it is a different kind of call: it + /// changes alarm state on the server, and a read-only build forbids every + /// write path, not only value writes. ``call_method`` itself stays, because + /// ConditionRefresh rides on it and asks the server for a replay rather than + /// changing anything. + /// + /// @param condition_id ConditionId of the live instance to act on. + /// @param method_id Acknowledge or Confirm on AcknowledgeableConditionType. + /// @param event_id EventId of the event being acknowledged. + /// @param comment Operator comment, forwarded as a LocalizedText. + tl::expected, MethodErrorInfo> call_condition_method(const opcua::NodeId & condition_id, + const opcua::NodeId & method_id, + const opcua::ByteString & event_id, + const std::string & comment); +#endif + /// Map an OPC-UA StatusCode (from an attempted method call or a /// per-argument validation result) to a ``MethodError`` category. /// Exposed as a public static helper so the classification table is diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_plugin.hpp b/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_plugin.hpp index e77a3a519..a547740e8 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_plugin.hpp +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/include/ros2_medkit_opcua/opcua_plugin.hpp @@ -69,6 +69,14 @@ namespace ros2_medkit_gateway { /// GET /apps/{id}/x-plc-data/{node} - Single node value /// POST /apps/{id}/x-plc-operations/{op} - Write value to PLC /// GET /components/{id}/x-plc-status - Connection state and stats +/// +/// Whether a value can be written at all is a property of the build, not of a +/// setting. ``MEDKIT_OPCUA_READ_ONLY`` defaults to ON: the OPC-UA write path is +/// then absent from the object, no data point is ever marked writable, neither +/// the x-plc-operations capability nor its POST route is registered, and +/// write_data / the value-write half of execute_operation refuse with 501 +/// before reaching the client. ``-DMEDKIT_OPCUA_READ_ONLY=OFF`` builds the +/// write-capable plugin. class OpcuaPlugin : public ros2_medkit_gateway::GatewayPlugin, public ros2_medkit_gateway::IntrospectionProvider, public ros2_medkit_gateway::DataProvider, @@ -115,6 +123,13 @@ class OpcuaPlugin : public ros2_medkit_gateway::GatewayPlugin, // false for the Component itself, which has no entity_defs entry. bool has_operations(const std::string & entity_id) const override; + /// The address-space walk configuration after configure() has merged the + /// node map and the ROS parameters. Read by tests that need to see which + /// source supplied a setting, which no REST response exposes. + const AutoBrowseConfig & auto_browse_config_for_test() const { + return node_map_.auto_browse_config(); + } + // -- FaultProvider interface -- tl::expected list_faults(const std::string & entity_id) override; tl::expected get_fault(const std::string & entity_id, @@ -146,7 +161,11 @@ class OpcuaPlugin : public ros2_medkit_gateway::GatewayPlugin, void handle_plc_data(const ros2_medkit_gateway::PluginRequest & req, ros2_medkit_gateway::PluginResponse & res); void handle_plc_data_single(const ros2_medkit_gateway::PluginRequest & req, ros2_medkit_gateway::PluginResponse & res); +#if !MEDKIT_OPCUA_READ_ONLY + /// Route handler for the vendor write endpoint. Declared and registered only + /// in a write-capable build; a read-only build serves no such route. void handle_plc_operations(const ros2_medkit_gateway::PluginRequest & req, ros2_medkit_gateway::PluginResponse & res); +#endif void handle_plc_status(const ros2_medkit_gateway::PluginRequest & req, ros2_medkit_gateway::PluginResponse & res); // Fault-detection signal (threshold / status-bit / enum) -> Fault bridge diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/src/address_space_browser.cpp b/src/ros2_medkit_plugins/ros2_medkit_opcua/src/address_space_browser.cpp index b523d0025..53967d85b 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/src/address_space_browser.cpp +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/src/address_space_browser.cpp @@ -229,7 +229,14 @@ void AutoBrowser::visit_object(const opcua::NodeId & node, const std::vector writable = source_.read_writable(child.node_id); if (writable.has_value()) { @@ -238,6 +245,7 @@ void AutoBrowser::visit_object(const opcua::NodeId & node, const std::vector(); @@ -392,6 +397,9 @@ bool NodeMap::load(const std::string & yaml_path) { } } +#if MEDKIT_OPCUA_READ_ONLY + size_t writable_ignored = 0; +#endif for (size_t i = 0; has_nodes && i < nodes.size(); ++i) { const auto & n = nodes[i]; @@ -449,7 +457,17 @@ bool NodeMap::load(const std::string & yaml_path) { continue; } +#if MEDKIT_OPCUA_READ_ONLY + // A read-only build has no write path in the binary, so a map entry + // claiming a point is writable would advertise a surface that does not + // exist. The request is counted and reported once after the loop. + if (n["writable"].as(false)) { + ++writable_ignored; + } + entry.writable = false; +#else entry.writable = n["writable"].as(false); +#endif if (n["min_value"]) { entry.min_value = n["min_value"].as(); } @@ -663,6 +681,17 @@ bool NodeMap::load(const std::string & yaml_path) { entries_.push_back(std::move(entry)); } +#if MEDKIT_OPCUA_READ_ONLY + if (writable_ignored > 0) { + RCLCPP_WARN(rclcpp::get_logger("opcua.node_map"), + "%zu node map entries request writable: true - ignored, every point stays " + "read-only. This plugin was built with MEDKIT_OPCUA_READ_ONLY=ON and carries " + "no write path; rebuild with -DMEDKIT_OPCUA_READ_ONLY=OFF for a write-capable " + "plugin.", + writable_ignored); + } +#endif + // Issue #386: native AlarmConditionType event subscriptions. Loaded from // top-level ``event_alarms:`` (sibling of ``nodes:``). Each entry must // declare its own entity_id; the entity will be merged into entity_defs_ diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_client.cpp b/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_client.cpp index 95513dad6..219b6b071 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_client.cpp +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_client.cpp @@ -196,6 +196,9 @@ opcua::ByteString read_file_bytes(const std::string & path) { return opcua::ByteString(std::string_view(data)); } +#if !MEDKIT_OPCUA_READ_ONLY +// Only the value-write path builds an outgoing Variant; a read-only build +// decodes incoming ones and never encodes one. opcua::Variant value_to_variant(const OpcuaValue & val) { return std::visit( [](auto && v) -> opcua::Variant { @@ -218,6 +221,7 @@ opcua::Variant value_to_variant(const OpcuaValue & val) { }, val); } +#endif // !MEDKIT_OPCUA_READ_ONLY } // namespace @@ -812,6 +816,10 @@ std::vector OpcuaClient::read_values(const std::vector OpcuaClient::write_value(const opcua::NodeId & node_id, const OpcuaValue & value, const std::string & data_type_hint) { std::lock_guard lock(impl_->client_mutex); @@ -987,6 +995,7 @@ OpcuaClient::write_value(const opcua::NodeId & node_id, const OpcuaValue & value return tl::make_unexpected(WriteErrorInfo{WriteError::TransportError, e.what()}); } } +#endif // !MEDKIT_OPCUA_READ_ONLY uint32_t OpcuaClient::create_subscription(double publish_interval_ms, DataChangeCallback callback) { std::lock_guard lock(impl_->client_mutex); @@ -1818,6 +1827,23 @@ OpcuaClient::classify_call_result(uint32_t overall_status_code, const std::vecto return {}; } +#if !MEDKIT_OPCUA_READ_ONLY +// The Part 9 Acknowledge / Confirm entry point. It builds the two arguments +// those methods take and hands them to the generic Call service. A read-only +// build does not compile it: acknowledging an alarm changes condition state on +// the controller, which is a write, and the object must carry no way to make +// one. ConditionRefresh keeps using call_method below - it asks the server to +// replay what it already holds. +tl::expected, OpcuaClient::MethodErrorInfo> +OpcuaClient::call_condition_method(const opcua::NodeId & condition_id, const opcua::NodeId & method_id, + const opcua::ByteString & event_id, const std::string & comment) { + std::vector args; + args.push_back(opcua::Variant::fromScalar(event_id)); + args.push_back(opcua::Variant::fromScalar(opcua::LocalizedText("", comment))); + return call_method(condition_id, method_id, args); +} +#endif // !MEDKIT_OPCUA_READ_ONLY + tl::expected, OpcuaClient::MethodErrorInfo> OpcuaClient::call_method(const opcua::NodeId & object_id, const opcua::NodeId & method_id, const std::vector & input_args) { diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_plugin.cpp b/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_plugin.cpp index aebb164f9..4c58b1ce8 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_plugin.cpp +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/src/opcua_plugin.cpp @@ -94,6 +94,22 @@ UserAuthMode require_user_auth_mode(const std::string & value) { return parsed; } +#if MEDKIT_OPCUA_READ_ONLY +/// One sentence for every refusal on the value-write path. It names the build +/// property rather than a permission, because that is the difference an +/// operator reading the HTTP body needs: no credential and no configuration +/// change makes this binary write to a controller. +constexpr const char * kReadOnlyBuildRefusal = + "This OPC UA plugin was built read-only (MEDKIT_OPCUA_READ_ONLY=ON) and contains no controller write path; " + "rebuild with -DMEDKIT_OPCUA_READ_ONLY=OFF for a write-capable plugin"; + +/// SOVD reserves 403 for a valid token with insufficient permissions, which is +/// the one thing this is not: no credential and no role reaches a write path +/// that is not in the binary. 501 is the status the SOVD catalogue uses for an +/// operation the entity does not support (fault deletion, data lists, +/// subscriptions and triggers all spell it that way), and that is what this is. +constexpr int kReadOnlyBuildStatus = 501; +#else /// Parse a JSON "value" field, coerce to the node's declared data_type, and /// validate against the optional min/max range. Shared by handle_plc_operations, /// DataProvider::write_data, and OperationProvider::execute_operation to keep @@ -133,6 +149,7 @@ tl::expected parse_coerce_validate(const nlohmann::json return val; } +#endif // MEDKIT_OPCUA_READ_ONLY bool is_valid_path_segment(const std::string & s) { if (s.empty() || s.size() > 256) { @@ -515,6 +532,7 @@ void OpcuaPlugin::configure(const nlohmann::json & config) { if (ab.contains("infer_writable")) { if (ab["infer_writable"].is_boolean()) { ab_cfg.infer_writable = ab["infer_writable"].get(); + ab_cfg.infer_writable_source = "plugins.opcua.auto_browse.infer_writable"; } else { warn("plugins.opcua.auto_browse.infer_writable must be a boolean - keeping current value"); } @@ -582,6 +600,20 @@ void OpcuaPlugin::set_context(PluginContext & context) { log_security_profile(); +#if MEDKIT_OPCUA_READ_ONLY + // One line, once, and only when someone asked for the inference: the setting + // defaults to true, so warning on the default would tell every auto_browse + // deployment about a key nobody wrote. An explicit request is different - it + // has no effect here, and the message names the setting so it can be found. + const auto & ab_cfg = node_map_.auto_browse_config(); + if (ab_cfg.enabled && ab_cfg.infer_writable && !ab_cfg.infer_writable_source.empty()) { + log_warn(ab_cfg.infer_writable_source + + " is ignored: this plugin was built with MEDKIT_OPCUA_READ_ONLY=ON and carries no write path, so " + "every discovered data point stays read-only. Rebuild with -DMEDKIT_OPCUA_READ_ONLY=OFF for a " + "write-capable plugin."); + } +#endif + const bool connected = client_->connect(client_config_); if (connected) { log_info("Connected to OPC-UA server: " + client_config_.endpoint_url); @@ -675,10 +707,12 @@ std::vector OpcuaPlugin::get_routes() { [this](const PluginRequest & req, PluginResponse & res) { handle_plc_data_single(req, res); }}, +#if !MEDKIT_OPCUA_READ_ONLY {"POST", R"(apps/([^/]+)/x-plc-operations/([^/]+))", [this](const PluginRequest & req, PluginResponse & res) { handle_plc_operations(req, res); }}, +#endif {"GET", R"(components/([^/]+)/x-plc-status)", [this](const PluginRequest & req, PluginResponse & res) { handle_plc_status(req, res); @@ -798,9 +832,11 @@ IntrospectionResult OpcuaPlugin::introspect(const IntrospectionInput & /*input*/ if (!def.data_names.empty()) { ctx_->register_entity_capability(def.id, "x-plc-data"); } +#if !MEDKIT_OPCUA_READ_ONLY if (!def.writable_names.empty()) { ctx_->register_entity_capability(def.id, "x-plc-operations"); } +#endif } } @@ -887,6 +923,7 @@ void OpcuaPlugin::handle_plc_data_single(const PluginRequest & req, PluginRespon res.send_json(j); } +#if !MEDKIT_OPCUA_READ_ONLY void OpcuaPlugin::handle_plc_operations(const PluginRequest & req, PluginResponse & res) { if (!ctx_ || !poller_ || shutdown_requested_.load()) { res.send_error(503, ERR_SERVICE_UNAVAILABLE, "OPC-UA plugin not initialized"); @@ -967,6 +1004,7 @@ void OpcuaPlugin::handle_plc_operations(const PluginRequest & req, PluginRespons res.send_json(response); } +#endif // !MEDKIT_OPCUA_READ_ONLY void OpcuaPlugin::handle_plc_status(const PluginRequest & req, PluginResponse & res) { if (!ctx_ || !poller_ || shutdown_requested_.load()) { @@ -1678,6 +1716,16 @@ tl::expected OpcuaPlugin::read_data(const tl::expected OpcuaPlugin::write_data(const std::string & entity_id, const std::string & resource_name, const nlohmann::json & value) { +#if MEDKIT_OPCUA_READ_ONLY + // First statement in the function: the refusal precedes every lookup, so no + // request reaches the OPC-UA client. The gateway renders provider errors as + // the x-medkit-plugin-error vendor code with this message verbatim. + (void)entity_id; + (void)resource_name; + (void)value; + return tl::make_unexpected( + DataProviderErrorInfo{DataProviderError::ReadOnly, kReadOnlyBuildRefusal, kReadOnlyBuildStatus}); +#else if (!ctx_ || !poller_) { return tl::make_unexpected(DataProviderErrorInfo{DataProviderError::Internal, "plugin not initialized", 503}); } @@ -1728,6 +1776,7 @@ tl::expected OpcuaPlugin::write_dat }, *parsed); return dto::DataWriteResult{std::move(result)}; +#endif // MEDKIT_OPCUA_READ_ONLY } bool OpcuaPlugin::has_data(const std::string & entity_id) const { @@ -1760,10 +1809,15 @@ OpcuaPlugin::list_operations(const std::string & entity_id) { collection.items.push_back(std::move(item)); } +#if !MEDKIT_OPCUA_READ_ONLY // Issue #386: emit acknowledge_fault / confirm_fault when the entity // has at least one native AlarmConditionType event subscription. The // fault_code parameter (passed in operation execution body) discriminates // which condition the operator is acting on. + // + // Not offered by a read-only build. Acknowledging or confirming a condition + // changes alarm state on the controller, so it is a write like any other, + // and the tree must not advertise what the binary cannot do. bool has_event_alarms = std::any_of(node_map_.event_alarms().begin(), node_map_.event_alarms().end(), [&entity_id](const AlarmEventConfig & cfg) { return cfg.entity_id == entity_id; @@ -1783,6 +1837,7 @@ OpcuaPlugin::list_operations(const std::string & entity_id) { confirm.asynchronous_execution = false; collection.items.push_back(std::move(confirm)); } +#endif return collection; } @@ -1799,6 +1854,22 @@ OpcuaPlugin::execute_operation(const std::string & entity_id, const std::string OperationProviderErrorInfo{OperationProviderError::Internal, "plugin not initialized", 503}); } +#if MEDKIT_OPCUA_READ_ONLY + // Neither branch below reads these: every operation is refused on its name + // alone, before any entity lookup or parameter parsing. + (void)entity_id; + (void)parameters; + + // Acknowledge and Confirm change alarm state on the controller, so they are + // write paths and this build carries neither. Refused before any condition + // lookup or client call, with the same answer a value write gets - nothing + // advertises them either, so this is the backstop for a client that posts the + // operation id directly. + if (operation_name == "acknowledge_fault" || operation_name == "confirm_fault") { + return tl::make_unexpected( + OperationProviderErrorInfo{OperationProviderError::Rejected, kReadOnlyBuildRefusal, kReadOnlyBuildStatus}); + } +#else // Issue #386: acknowledge_fault and confirm_fault dispatch to OPC-UA // Method calls on the live ConditionId. AcknowledgeableConditionType // declares Acknowledge as method i=9111 and Confirm as method i=9113; @@ -1843,10 +1914,6 @@ OpcuaPlugin::execute_operation(const std::string & entity_id, const std::string constexpr uint32_t kConfirmMethodId = 9113; opcua::NodeId method_id(0, operation_name == "acknowledge_fault" ? kAcknowledgeMethodId : kConfirmMethodId); - std::vector args; - args.push_back(opcua::Variant::fromScalar(runtime->latest_event_id)); - args.push_back(opcua::Variant::fromScalar(opcua::LocalizedText("", comment))); - if (plugin_debug_enabled()) { std::ostringstream hex_oss; const auto * bytes = runtime->latest_event_id.data(); @@ -1860,7 +1927,7 @@ OpcuaPlugin::execute_operation(const std::string & entity_id, const std::string << " conditionId=" << runtime->condition_id.toString()); } - auto result = client_->call_method(runtime->condition_id, method_id, args); + auto result = client_->call_condition_method(runtime->condition_id, method_id, runtime->latest_event_id, comment); if (!result.has_value()) { auto code = result.error().code; int http = 502; @@ -1889,7 +1956,17 @@ OpcuaPlugin::execute_operation(const std::string & entity_id, const std::string out["condition_id"] = runtime->condition_id.toString(); return dto::OperationExecutionResult{std::move(out)}; } +#endif // MEDKIT_OPCUA_READ_ONLY +#if MEDKIT_OPCUA_READ_ONLY + // Everything past the branch above is the value-write path, and this build + // does not contain it either. Nothing advertised reaches here - a read-only + // build marks no point writable, so list_operations emits no set_* entry - + // and a request aimed straight at one is refused before any node lookup or + // client call. + return tl::make_unexpected( + OperationProviderErrorInfo{OperationProviderError::Rejected, kReadOnlyBuildRefusal, kReadOnlyBuildStatus}); +#else std::string data_name; if (operation_name.substr(0, 4) == "set_") { data_name = operation_name.substr(4); @@ -1945,6 +2022,7 @@ OpcuaPlugin::execute_operation(const std::string & entity_id, const std::string }, *parsed); return dto::OperationExecutionResult{std::move(result)}; +#endif // MEDKIT_OPCUA_READ_ONLY } bool OpcuaPlugin::has_operations(const std::string & entity_id) const { diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/test/inspect_build_variant.py b/src/ros2_medkit_plugins/ros2_medkit_opcua/test/inspect_build_variant.py new file mode 100644 index 000000000..442b4cd04 --- /dev/null +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/test/inspect_build_variant.py @@ -0,0 +1,360 @@ +#!/usr/bin/env python3 +# Copyright 2026 bburda +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +Inspect the built OPC-UA plugin object for the write surface it declares. + +The check reads the object itself rather than any configuration. + +A read-only build is a property of the binary: `MEDKIT_OPCUA_READ_ONLY=ON` +compiles the OPC-UA value-write path out, so no symbol that can put a value on +the wire is emitted. A runtime flag could be flipped; an absent symbol cannot. + +The check runs in BOTH variants with opposite expectations, and that is what +makes it meaningful: + + read-only every WRITE_MARKER absent, every READ_MARKER present + write-capable every WRITE_MARKER present, every READ_MARKER present + +Asserting only the read-only direction would pass for a marker list that +matches nothing at all - a renamed or fully inlined symbol would read as +"the write path is gone". The write-capable direction is the control that keeps +the marker list honest, and READ_MARKER is the control that fails on an empty, +stripped or wrong object. + +Usage: inspect_build_variant.py --expect {read-only,write-capable} +""" + +import argparse +from pathlib import Path +import re +import shutil +import subprocess +import sys +import tempfile + +# Symbols that exist only when the value-write path is compiled in. Each is on +# the road from the provider entry points to the wire: +# - OpcuaClient::write_value is the plugin's own single write entry point; +# - OpcuaPlugin::handle_plc_operations is the vendor route that reaches it; +# - parse_coerce_validate is the value coercion the three write paths share; +# - Node::writeValueScalar / writeValue are the open62541pp templates +# where the value is encoded for the Write service; +# - services::write / services::writeAttribute are the layer beneath +# them, the last C++ frame before open62541's own client machinery; +# - call_condition_method issues the Part 9 Acknowledge / Confirm calls, which +# change alarm state on the controller and are a write like any other. +# +# call_method and opcua::services::call are deliberately NOT markers even though +# the acknowledge path runs through them: ConditionRefresh runs through them +# too, asking the server to replay conditions it already holds, and that is a +# read the shipped build keeps. Measured 3/3 and 1/1 across the two variants - +# they discriminate nothing. +# +# Every marker is verified to discriminate in an OPTIMIZED build, which is what +# CI and every release produce. That rules out opcua::services::writeValue and +# writeAttributeImpl: they are fully inlined at -O2 and +# emit no symbol in either variant, so they would read as "the write path is +# gone" in both builds and prove nothing. +# +# open62541's own primitives - __UA_Client_writeAttribute and the rest of the +# UA_Client_write* family - are absent from BOTH objects, so they discriminate +# nothing between variants and are not markers. Nothing in the plugin references +# them (open62541pp reaches the Write service through services::write), and the +# link drops them: -Wl,--exclude-libs,ALL keeps a static archive's globals out of +# the module's dynamic symbol table, which -fvisibility=hidden does not reach, +# and -ffunction-sections/-fdata-sections with -Wl,--gc-sections then discard the +# unreferenced sections. WRITE_PRIMITIVES below asserts their absence as an +# invariant, because an object that carries them is one where the archive member +# was pulled back in. EXPORT_DENY covers the rest: what remains of open62541 in a +# read-only object - the generic __UA_Client_Service dispatcher the read path +# needs, and the generated type descriptors the UA_TYPES table pins - is +# unreachable because the module exports none of it. +WRITE_MARKERS = ( + 'ros2_medkit_gateway::OpcuaClient::write_value(', + 'ros2_medkit_gateway::OpcuaPlugin::handle_plc_operations(', + 'parse_coerce_validate(', + 'opcua::Node::writeValueScalar<', + 'opcua::Node::writeValue(opcua::Variant const&)', + 'opcua::services::write(opcua::Client&, opcua::WriteRequest const&)', + 'opcua::services::writeAttribute', + 'ros2_medkit_gateway::OpcuaClient::call_condition_method(', +) + +# The read path the plugin needs in every variant. Present in both builds, so a +# stripped, truncated or simply wrong object fails here instead of silently +# satisfying the "no write symbols" half. These are the plugin's own out-of-line +# definitions rather than open62541pp templates, for the same optimization +# reason as above: services::detail::readAttributeImpl is inlined away at -O2. +READ_MARKERS = ( + 'ros2_medkit_gateway::OpcuaClient::read_value(', + 'ros2_medkit_gateway::OpcuaClient::read_values(', + 'ros2_medkit_gateway::OpcuaClient::read_access_level(', + 'ros2_medkit_gateway::OpcuaClient::browse_detailed(', + 'ros2_medkit_gateway::OpcuaClient::call_method(', +) + +# Nothing from the OPC-UA stack may appear in the module's dynamic symbol table, +# in either variant. The gateway dlopens the plugin and needs its extern "C" +# entry points and nothing else; anything else exported is a dlsym handle on +# machinery no route exposes. +# +# A regex, not a prefix match on "UA_": open62541's internal entry points are +# spelled with leading underscores (__UA_Client_writeAttribute, +# __UA_Client_Service), so a startswith check waves through exactly the symbol +# that started this - a version script exporting it alongside the six entry +# points passed a prefix check while handing dlsym a working write primitive. +# Leading underscores are optional in the match for that reason. +EXPORT_DENY = re.compile(r'^_*UA_|^opcua::') + +# open62541's own Write service primitives, which the plugin never calls: the +# C++ wrapper reaches the service through opcua::services::write. They must be +# absent from a read-only object even as LOCAL symbols, not merely unexported - +# a symbol nothing exports is still a gadget for anything running in the same +# process, and their presence means the archive member was pulled in, which is +# the state a link or optimisation change can silently restore. Checked as an +# absence invariant rather than a variant marker because they are absent from +# the write-capable object too, so they discriminate nothing between builds. +WRITE_PRIMITIVES = re.compile(r'_*UA_(Client|Server)_write') + +# Symbols the gateway resolves out of the plugin. If a link-time change ever +# hides these, the plugin still builds and still passes every symbol check above +# while failing to load at runtime, so they are asserted here rather than left +# to an integration test to discover. +REQUIRED_EXPORTS = ( + 'create_plugin', + 'plugin_api_version', + 'get_introspection_provider', + 'get_data_provider', + 'get_operation_provider', + 'get_fault_provider', +) + +# An optimized plugin object still carries a few thousand symbols. Anything near +# zero means nm read something that is not the plugin, or an object whose symbol +# table was stripped, and every "absent" verdict below would then be vacuous. +MIN_SYMBOLS = 1000 + + +def nm(args, path): + """Run nm with the given flags and return its stdout lines.""" + out = subprocess.run( + ['nm', *args, path], capture_output=True, text=True, check=False) + if out.returncode != 0: + print(f'FAIL: nm {" ".join(args)} {path} exited {out.returncode}\n{out.stderr}', + file=sys.stderr) + sys.exit(1) + return out.stdout.splitlines() + + +def count(lines, marker): + """Return the number of symbol lines containing the marker substring.""" + return sum(1 for line in lines if marker in line) + + +def symbol_name(line): + """Return the demangled name from one nm line, without the address and type.""" + return line.split(' ', 2)[-1] + + +def check_object(plugin, expect, report): + """Check one object against a variant and return the list of failures.""" + defined = nm(['-C'], plugin) + exported = nm(['-DC', '--defined-only'], plugin) + report(f'{plugin}: {len(defined)} symbols, {len(exported)} dynamic exports, ' + f'expecting {expect}') + + failures = [] + if len(defined) < MIN_SYMBOLS: + failures.append(f'only {len(defined)} symbols (< {MIN_SYMBOLS}) - ' + 'object is stripped, truncated or not the plugin') + + for marker in READ_MARKERS: + n = count(defined, marker) + report(f' read {n:>4} {marker}') + if n == 0: + failures.append(f'read path symbol missing: {marker}') + + want_writes = expect == 'write-capable' + for marker in WRITE_MARKERS: + n = count(defined, marker) + report(f' write {n:>4} {marker}') + if want_writes and n == 0: + failures.append(f'write-capable build is missing: {marker}') + if not want_writes and n != 0: + failures.append(f'read-only build still contains {n} x: {marker}') + + # The export table is an invariant, not a variant property: neither build + # may hand dlsym a way into the OPC-UA stack. + leaked = [symbol_name(line) for line in exported + if EXPORT_DENY.search(symbol_name(line))] + report(f' export {len(leaked):>4} OPC-UA symbols in the dynamic symbol table') + for name in leaked[:10]: + failures.append(f'dynamic symbol table exports OPC-UA machinery: {name}') + + export_names = {symbol_name(line) for line in exported} + missing = [name for name in REQUIRED_EXPORTS if name not in export_names] + report(f' export {len(REQUIRED_EXPORTS) - len(missing):>4}' + f'/{len(REQUIRED_EXPORTS)} plugin entry points') + for name in missing: + failures.append(f'plugin entry point not exported: {name}') + + # And the library's Write primitives must not be in the read-only object at + # all, exported or not. Independent of build type: upstream only compiles + # with -ffunction-sections for Release and MinSizeRel, so without the flags + # this package sets on the object libraries a default-type build kept twelve + # of them as local symbols while every other check here still passed. + if not want_writes: + primitives = [symbol_name(line) for line in defined + if WRITE_PRIMITIVES.search(symbol_name(line))] + report(f' absent {len(primitives):>4} open62541 UA_*_write* primitives') + for name in primitives[:10]: + failures.append(f'read-only object still carries a write primitive: {name}') + + return failures + + +# A test of the export rule: a module that exports __UA_Client_writeAttribute +# next to the six entry points, which is what a version script or an -fvisibility +# slip on the vendored library produces. The leading underscores are the point - +# a rule that matched the prefix "UA_" would let this spelling through and hand +# dlsym a working write primitive. The object is otherwise a plausible plugin +# (the read markers and entry points are there), so a rejection can only come +# from the export rule under test. +SELF_CHECK_SOURCE = r""" +#include +#define EXPORT __attribute__((visibility("default"))) +EXPORT int __UA_Client_writeAttribute(void) { return 0; } +EXPORT void *create_plugin(void) { return 0; } +EXPORT int plugin_api_version(void) { return 1; } +EXPORT void *get_introspection_provider(void *p) { return p; } +EXPORT void *get_data_provider(void *p) { return p; } +EXPORT void *get_operation_provider(void *p) { return p; } +EXPORT void *get_fault_provider(void *p) { return p; } +""" + + +def build_self_check_object(workdir): + """Compile the reproducer module; return its path, or None with a reason.""" + compiler = shutil.which('cc') or shutil.which('gcc') + if compiler is None: + return None, 'no C compiler on PATH' + src = workdir / 'leaky_plugin.c' + obj = workdir / 'leaky_plugin.so' + src.write_text(SELF_CHECK_SOURCE) + out = subprocess.run( + [compiler, '-shared', '-fPIC', '-fvisibility=hidden', '-o', str(obj), str(src)], + capture_output=True, text=True, check=False) + if out.returncode != 0: + return None, f'compile failed: {out.stderr.strip()}' + return obj, '' + + +def self_check(): + """Prove the rules reject what they are meant to reject. Returns an exit code.""" + quiet = (lambda *a, **k: None) + problems = [] + + # The name rules, on symbol lines rather than on a whole object, so the + # spellings that matter are pinned one by one. + must_deny = ('__UA_Client_writeAttribute', 'UA_Client_writeValueAttribute', + '_UA_Server_write', '__UA_Client_Service', + 'opcua::services::write(opcua::Client&, opcua::WriteRequest const&)') + must_allow = ('create_plugin', 'plugin_api_version', 'medkit_UA_helper', + 'std::__cxx11::basic_string::~basic_string()') + for name in must_deny: + if not EXPORT_DENY.search(name): + problems.append(f'EXPORT_DENY fails to match {name}') + for name in must_allow: + if EXPORT_DENY.search(name): + problems.append(f'EXPORT_DENY wrongly matches {name}') + + must_be_primitives = ('__UA_Client_writeAttribute', 'UA_Client_writeArrayDimensionsAttribute', + 'UA_Server_writeValue', '__UA_Server_write') + must_not_be_primitives = ('__UA_Client_readAttribute', 'UA_Client_Service_read', + 'UA_WriteRequest_init') + for name in must_be_primitives: + if not WRITE_PRIMITIVES.search(name): + problems.append(f'WRITE_PRIMITIVES fails to match {name}') + for name in must_not_be_primitives: + if WRITE_PRIMITIVES.search(name): + problems.append(f'WRITE_PRIMITIVES wrongly matches {name}') + + # And the same rule end to end, on a real linked object rather than on names + # alone. A compiler is present wherever this test runs, because the package + # it inspects was just compiled. + workdir = Path(tempfile.mkdtemp(prefix='opcua_self_check_')) + try: + obj, reason = build_self_check_object(workdir) + if obj is None: + print(f'FAIL: cannot build the reproducer object - {reason}', file=sys.stderr) + return 1 + failures = check_object(str(obj), 'read-only', quiet) + leak = [f for f in failures if '__UA_Client_writeAttribute' in f + and 'exports OPC-UA machinery' in f] + if not leak: + problems.append('an object exporting __UA_Client_writeAttribute was not ' + 'rejected by the export rule') + entry_points = [f for f in failures if 'entry point not exported' in f] + if entry_points: + problems.append('the reproducer object is not shaped like a plugin, so its ' + f'rejection proves nothing: {entry_points}') + finally: + shutil.rmtree(workdir, ignore_errors=True) + + if problems: + print('FAIL (self-check):', file=sys.stderr) + for p in problems: + print(f' - {p}', file=sys.stderr) + return 1 + print(f'PASS: self-check - {len(must_deny) + len(must_allow)} export-rule cases, ' + f'{len(must_be_primitives) + len(must_not_be_primitives)} write-primitive cases, ' + 'and a linked object exporting __UA_Client_writeAttribute is rejected') + return 0 + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument('plugin', nargs='?', + help='path to libros2_medkit_opcua_plugin.so') + parser.add_argument('--expect', choices=('read-only', 'write-capable'), + help='the write surface this build declares') + parser.add_argument('--self-check', action='store_true', + help='check the rules against objects and names built to break them') + args = parser.parse_args() + + if args.self_check: + return self_check() + if not args.plugin or not args.expect: + parser.error('a plugin path and --expect are required unless --self-check is given') + + if shutil.which('nm') is None: + print('FAIL: nm (binutils) not on PATH - the build inspection cannot run', + file=sys.stderr) + return 1 + + failures = check_object(args.plugin, args.expect, print) + if failures: + print(f'FAIL ({args.expect}):', file=sys.stderr) + for f in failures: + print(f' - {f}', file=sys.stderr) + return 1 + + print(f'PASS: object matches the {args.expect} build variant') + return 0 + + +if __name__ == '__main__': + sys.exit(main()) diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/test/integration/test_opcua_read_only.test.py b/src/ros2_medkit_plugins/ros2_medkit_opcua/test/integration/test_opcua_read_only.test.py new file mode 100644 index 000000000..75902dac9 --- /dev/null +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/test/integration/test_opcua_read_only.test.py @@ -0,0 +1,604 @@ +#!/usr/bin/env python3 +# Copyright 2026 bburda +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +""" +Drive the read-only / write-capable OPC-UA plugin over real HTTP. + +A live gateway loads the plugin against the in-tree OPC-UA fixture server and +every claim is checked at the wire, not in a mock: + + * a node map asking for ``writable: true`` on a node the server really does + let us write; + * the ``infer_writable`` sweep (true / false / absent) over the address-space + walk, against the same node; + * the SOVD write endpoints and the vendor write route; + * the Part 9 alarm acknowledge, against a condition the fixture really + raises - acknowledging changes condition state on the controller, so it is + a write and a read-only build neither offers nor performs it; + * what the entity tree advertises; + * and a reconnect, so the answer does not change when the address space is + walked a second time. + +The expected variant is passed in from CMake, never read from a runtime +parameter: the whole point is that a read-only build is a property of the +binary, and a test that could be told otherwise at run time would not be +testing that. + +Usage: test_opcua_read_only.test.py +""" + +import json +import os +from pathlib import Path +import shutil +import signal +import socket +import subprocess +import sys +import tempfile +import time +import urllib.error +import urllib.request + +# The fixture registers these under the Objects folder in namespace 2. The two +# Int32 registers carry AccessLevel READ|WRITE, so the server would let a +# write-capable client change them; Tank.Level is READ only. +WRITABLE_NODE = 'ns=2;s=StatusWord' +READ_ONLY_NODE = 'ns=2;s=Tank.Level' +SECOND_WRITABLE_NODE = 'ns=2;s=FaultCode' + +ENTITY = 'plc_app' +ALARM_CODE = 'PLC_OVERPRESSURE' +COMPONENT = 'read_only_runtime' + +# The vendor code the gateway puts on the wire for any plugin provider refusal +# (primitives.cpp maps every x-medkit-* code into error_code "vendor-specific" +# plus this vendor_code). The build property is named in the message. +PLUGIN_VENDOR_CODE = 'x-medkit-plugin-error' +# SOVD spells "the entity does not support this" as 501; 403 is reserved for a +# valid token with insufficient permissions, which no credential can fix here. +REFUSAL_STATUS = 501 +BUILD_PROPERTY = 'MEDKIT_OPCUA_READ_ONLY' + +failures = [] + + +def check(condition, message): + """Record a failed expectation and keep going, so one run reports them all.""" + if condition: + print(f' OK {message}') + else: + print(f' FAIL {message}', file=sys.stderr) + failures.append(message) + return bool(condition) + + +def free_port(): + """Grab an OS-assigned free TCP port on the loopback and release it.""" + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: + s.bind(('127.0.0.1', 0)) + return s.getsockname()[1] + + +def find_plugin(): + """Locate libros2_medkit_opcua_plugin.so under AMENT_PREFIX_PATH.""" + for prefix in os.environ.get('AMENT_PREFIX_PATH', '').split(os.pathsep): + if not prefix: + continue + for root, _dirs, files in os.walk(prefix): + if 'libros2_medkit_opcua_plugin.so' in files: + return os.path.join(root, 'libros2_medkit_opcua_plugin.so') + return None + + +def http(url, method='GET', body=None, timeout=5): + """Return (status, parsed_json_or_raw_text) for one request; (0, None) on transport failure.""" + data = json.dumps(body).encode() if body is not None else None + req = urllib.request.Request(url, data=data, method=method) + if data is not None: + req.add_header('Content-Type', 'application/json') + try: + with urllib.request.urlopen(req, timeout=timeout) as resp: + raw = resp.read().decode() + status = resp.status + except urllib.error.HTTPError as e: + raw = e.read().decode() + status = e.code + except (urllib.error.URLError, OSError): + return 0, None + try: + return status, json.loads(raw) + except ValueError: + return status, raw + + +def wait_json(url, predicate, deadline=90, period=2.0): + """Poll until predicate(json) holds; return the final json or None.""" + last = None + for _ in range(int(deadline / period) + 1): + _status, last = http(url) + if last is not None: + try: + if predicate(last): + return last + except (KeyError, TypeError, ValueError): + pass + time.sleep(period) + return last + + +def wait_log(path, needle, deadline=30, period=0.5): + """Poll a log file until appears; return True/False.""" + for _ in range(int(deadline / period) + 1): + try: + if needle in Path(path).read_text(errors='replace'): + return True + except OSError: + pass + time.sleep(period) + return False + + +def terminate(proc): + """SIGTERM then SIGKILL a child process group started with start_new_session.""" + if proc is None: + return + if proc.returncode is not None: + pgid = None + else: + try: + pgid = os.getpgid(proc.pid) + except ProcessLookupError: + pgid = None + + def signal_group(sig): + if pgid is None: + return + try: + os.killpg(pgid, sig) + except ProcessLookupError: + pass + + signal_group(signal.SIGTERM) + proc.terminate() + try: + proc.wait(timeout=8) + except subprocess.TimeoutExpired: + signal_group(signal.SIGKILL) + proc.kill() + proc.wait() + signal_group(signal.SIGKILL) + log = getattr(proc, '_log', None) + if log is not None: + log.close() + + +def start_server(server_bin, port, log_path): + """Start the plain (unsecured) fixture server and wait for its READY line.""" + log = open(log_path, 'w') + proc = subprocess.Popen( + [str(server_bin), '--port', str(port)], + stdin=subprocess.PIPE, stdout=log, stderr=subprocess.STDOUT, + text=True, start_new_session=True, + ) + proc._log = log + if not wait_log(log_path, 'READY ', deadline=25): + terminate(proc) + return None + return proc + + +def send_cmd(server, cmd): + """Write one alarm-CLI command to the fixture; False when it is already gone.""" + if server.poll() is not None or server.stdin is None: + return False + try: + server.stdin.write(cmd + '\n') + server.stdin.flush() + except (BrokenPipeError, ValueError): + return False + return True + + +def write_params(path, *, port, plugin, server_port, node_map, manifest, auto_browse): + """Render a gateway params file for one leg of the sweep.""" + lines = [ + 'ros2_medkit_gateway:', + ' ros__parameters:', + ' server:', + ' host: "127.0.0.1"', + f' port: {port}', + ' plugins: ["opcua"]', + f' plugins.opcua.path: "{plugin}"', + f' plugins.opcua.endpoint_url: "opc.tcp://127.0.0.1:{server_port}"', + f' plugins.opcua.node_map_path: "{node_map}"', + ' plugins.opcua.poll_interval_ms: 500', + ' discovery.mode: "hybrid"', + f' discovery.manifest_path: "{manifest}"', + ' discovery.manifest_strict_validation: false', + ] + # `absent` leaves infer_writable to its own default, which is the shape most + # deployments actually run; the other two pin the documented endpoints. + if auto_browse is not None: + lines.append(' plugins.opcua.auto_browse.enabled: true') + if auto_browse != 'absent': + lines.append(f' plugins.opcua.auto_browse.infer_writable: {auto_browse}') + path.write_text('\n'.join(lines) + '\n') + + +def start_gateway(params_file, log_path, env): + """Launch a gateway_node bound to the given params file.""" + log = open(log_path, 'w') + proc = subprocess.Popen( + ['ros2', 'run', 'ros2_medkit_gateway', 'gateway_node', + '--ros-args', '--params-file', str(params_file)], + stdout=log, stderr=subprocess.STDOUT, env=env, start_new_session=True, + ) + proc._log = log + return proc + + +def item_by_name(payload, name): + """Pick one entry out of an x-plc-data / data collection response.""" + for item in (payload or {}).get('items', []): + if item.get('name') == name or item.get('id') == name: + return item + return None + + +def refusal_is_the_build(status, payload, what): + """Assert one HTTP answer is the read-only build's refusal, naming the build.""" + ok = check(status == REFUSAL_STATUS, f'{what}: {REFUSAL_STATUS} (got {status})') + if not isinstance(payload, dict): + check(False, f'{what}: JSON error body (got {payload!r})') + return + ok &= check(payload.get('vendor_code') == PLUGIN_VENDOR_CODE, + f'{what}: vendor_code {PLUGIN_VENDOR_CODE} (got {payload.get("vendor_code")!r})') + ok &= check(BUILD_PROPERTY in str(payload.get('message', '')), + f'{what}: message names {BUILD_PROPERTY} (got {payload.get("message")!r})') + return ok + + +def node_map_text(): + """Return a map sweeping writable true / false / absent over three real server nodes.""" + return ( + 'area_id: plc_systems\n' + f'component_id: {COMPONENT}\n' + 'nodes:\n' + f' - node_id: "{WRITABLE_NODE}"\n' + f' entity_id: {ENTITY}\n' + ' data_name: status_word\n' + ' data_type: int\n' + ' writable: true\n' + f' - node_id: "{SECOND_WRITABLE_NODE}"\n' + f' entity_id: {ENTITY}\n' + ' data_name: fault_code\n' + ' data_type: int\n' + ' writable: false\n' + f' - node_id: "{READ_ONLY_NODE}"\n' + f' entity_id: {ENTITY}\n' + ' data_name: tank_level\n' + ' data_type: float\n' + ) + + +def auto_browse_map_text(): + """Return a map that only turns the address-space walk on, with no hand-written nodes.""" + return ( + 'area_id: plc_systems\n' + f'component_id: {COMPONENT}\n' + 'auto_browse: true\n' + ) + + +def run_node_map_leg(workdir, env, plugin, server_port, manifest, read_only): + """Run the hand-written node map leg: what the tree advertises, and what a write does.""" + print('--- node map leg (writable: true / false / absent) ---') + node_map = workdir / 'nodes.yaml' + node_map.write_text(node_map_text()) + port = free_port() + params = workdir / 'gateway_nodemap.yaml' + write_params(params, port=port, plugin=plugin, server_port=server_port, + node_map=node_map, manifest=manifest, auto_browse=None) + log = workdir / 'gateway_nodemap.log' + gw = start_gateway(params, log, env) + try: + base = f'http://127.0.0.1:{port}/api/v1' + data = wait_json(f'{base}/apps/{ENTITY}/x-plc-data', + lambda j: item_by_name(j, 'status_word') is not None) + if not check(item_by_name(data, 'status_word') is not None, + 'x-plc-data serves the mapped points'): + print(log.read_text(errors='replace')[-3000:], file=sys.stderr) + return + + # 1. The writable flag on the wire. + for name, mapped in (('status_word', True), ('fault_code', False), ('tank_level', None)): + item = item_by_name(data, name) + expected = bool(mapped) and not read_only + check(item is not None and item.get('writable') is expected, + f'{name} (map writable={mapped}) reports writable={expected}') + + # 2. What the entity advertises. + _status, detail = http(f'{base}/apps/{ENTITY}') + names = {c.get('name') for c in (detail or {}).get('capabilities', [])} + check(('x-plc-operations' in names) is not read_only, + f'x-plc-operations capability advertised: {not read_only}') + + # 3. The SOVD operations collection. + _status, ops = http(f'{base}/apps/{ENTITY}/operations') + op_ids = {o.get('id') for o in (ops or {}).get('items', [])} + check(('set_status_word' in op_ids) is not read_only, + f'set_status_word offered: {not read_only} (got {sorted(op_ids)})') + + # 4. PUT /data/{id} on the point the map and the server both allow. + status, body = http(f'{base}/apps/{ENTITY}/data/status_word', 'PUT', {'value': 7}) + if read_only: + refusal_is_the_build(status, body, 'PUT data on a map-writable point') + else: + check(status == 200, f'PUT data succeeds (got {status}: {body!r})') + got = wait_json(f'{base}/apps/{ENTITY}/x-plc-data', + lambda j: (item_by_name(j, 'status_word') or {}).get('value') == 7, + deadline=20) + check((item_by_name(got, 'status_word') or {}).get('value') == 7, + 'the written value reached the server') + + # 5. POST an execution on the operation the map asks for. + exec_url = f'{base}/apps/{ENTITY}/operations/set_status_word/executions' + status, body = http(exec_url, 'POST', {'value': 9}) + if read_only: + refusal_is_the_build(status, body, 'POST execution on set_status_word') + else: + check(status in (200, 202), f'POST execution succeeds (got {status}: {body!r})') + got = wait_json(f'{base}/apps/{ENTITY}/x-plc-data', + lambda j: (item_by_name(j, 'status_word') or {}).get('value') == 9, + deadline=20) + check((item_by_name(got, 'status_word') or {}).get('value') == 9, + 'the executed operation reached the server') + + # 6. The vendor route is not registered at all in a read-only build. + status, body = http(f'{base}/apps/{ENTITY}/x-plc-operations/set_status_word', + 'POST', {'value': 11}) + if read_only: + check(status == 404, + f'POST x-plc-operations is not routed (got {status}: {body!r})') + else: + check(status == 200, f'POST x-plc-operations succeeds (got {status}: {body!r})') + + # 7. Points the map did NOT mark writable. + for name in ('fault_code', 'tank_level'): + status, body = http(f'{base}/apps/{ENTITY}/data/{name}', 'PUT', {'value': 3}) + if read_only: + refusal_is_the_build(status, body, f'PUT data on {name}') + else: + check(status == 400, + f'PUT data on the read-only point {name} is 400 (got {status})') + + # 8. The startup warning naming the build property. + if read_only: + check(wait_log(log, BUILD_PROPERTY, deadline=5), + f'the gateway log names {BUILD_PROPERTY} for the ignored writable: true') + finally: + terminate(gw) + + +def alarm_map_text(): + """Map one of the fixture's AlarmConditionType sources onto an entity.""" + return ( + 'area_id: plc_systems\n' + f'component_id: {COMPONENT}\n' + 'nodes:\n' + f' - node_id: "{READ_ONLY_NODE}"\n' + f' entity_id: {ENTITY}\n' + ' data_name: tank_level\n' + ' data_type: float\n' + 'event_alarms:\n' + ' - alarm_source: "ns=2;s=Alarms.Overpressure"\n' + f' entity_id: {ENTITY}\n' + f' fault_code: {ALARM_CODE}\n' + ) + + +def run_alarm_leg(workdir, env, plugin, server, server_port, manifest, read_only): + """Acknowledge is a controller state change, so it follows the same rule.""" + print('--- alarm acknowledge / confirm leg ---') + node_map = workdir / 'alarm_nodes.yaml' + node_map.write_text(alarm_map_text()) + port = free_port() + params = workdir / 'gateway_alarm.yaml' + write_params(params, port=port, plugin=plugin, server_port=server_port, + node_map=node_map, manifest=manifest, auto_browse=None) + log = workdir / 'gateway_alarm.log' + gw = start_gateway(params, log, env) + try: + base = f'http://127.0.0.1:{port}/api/v1' + status = wait_json(f'{base}/components/{COMPONENT}/x-plc-status', + lambda j: j.get('connected') is True, deadline=90) + if not check(bool(status) and status.get('connected') is True, + 'the gateway connected for the alarm leg'): + print(log.read_text(errors='replace')[-3000:], file=sys.stderr) + return + + # 1. What the entity offers. + _s, ops = http(f'{base}/apps/{ENTITY}/operations') + op_ids = {o.get('id') for o in (ops or {}).get('items', [])} + for op in ('acknowledge_fault', 'confirm_fault'): + check((op in op_ids) is not read_only, + f'{op} offered: {not read_only} (got {sorted(op_ids)})') + + # 2. Raise a real condition, so the write-capable call has something + # live to act on and the read-only refusal is not just "not found". + if not check(send_cmd(server[0], 'fire Overpressure 750'), + 'fired an alarm on the fixture server'): + return + exec_url = f'{base}/apps/{ENTITY}/operations/acknowledge_fault/executions' + body = {'fault_code': ALARM_CODE, 'comment': 'acked by the integration test'} + if read_only: + status, payload = http(exec_url, 'POST', body) + refusal_is_the_build(status, payload, 'POST acknowledge_fault') + else: + # The condition has to reach the poller's registry first; the + # subscription delivers it a moment after the server emits it. + got = None + for _ in range(30): + status, payload = http(exec_url, 'POST', body) + if status in (200, 202): + got = payload + break + time.sleep(2) + check(got is not None, + f'POST acknowledge_fault succeeds (last: {status} {payload!r})') + if got is not None: + check(got.get('status') == 'ok' and got.get('fault_code') == ALARM_CODE, + f'the acknowledge reached the server ({got!r})') + finally: + terminate(gw) + + +def auto_browse_writable(base, deadline=90): + """Return the writable flag of the auto-browsed StatusWord point, or None.""" + payload = wait_json(f'{base}/apps', lambda j: j.get('items'), deadline=deadline) + for app in (payload or {}).get('items', []): + _status, data = http(f'{base}/apps/{app.get("id")}/x-plc-data') + item = item_by_name(data, 'statusword') or item_by_name(data, 'StatusWord') + if item is not None: + return item.get('writable') + return None + + +def run_auto_browse_leg(workdir, env, plugin, server_bin, server, server_port, + manifest, read_only, infer_writable, rebrowse): + """One infer_writable setting: the walk must never mark a point writable here.""" + label = f'infer_writable={infer_writable}' + print(f'--- auto_browse leg ({label}) ---') + node_map = workdir / f'auto_{infer_writable}.yaml' + node_map.write_text(auto_browse_map_text()) + port = free_port() + params = workdir / f'gateway_auto_{infer_writable}.yaml' + write_params(params, port=port, plugin=plugin, server_port=server_port, + node_map=node_map, manifest=manifest, auto_browse=infer_writable) + log = workdir / f'gateway_auto_{infer_writable}.log' + gw = start_gateway(params, log, env) + try: + base = f'http://127.0.0.1:{port}/api/v1' + writable = auto_browse_writable(base) + # The server sets CurrentWrite on StatusWord, so a write-capable build + # infers writable unless the setting says otherwise. A read-only build + # never consults the bit. + expected = (not read_only) and infer_writable != 'false' + check(writable is expected, + f'{label}: auto-browsed StatusWord reports writable={expected} (got {writable!r})') + + # The startup warning belongs to a key someone set, not to the default. + # infer_writable defaults to true, so warning on the value would warn on + # every auto_browse deployment; the log must name the setting when it is + # written and say nothing when it is not. + warned = wait_log(log, 'plugins.opcua.auto_browse.infer_writable is ignored', deadline=3) + want_warning = read_only and infer_writable == 'true' + check(warned is want_warning, + f'{label}: startup warning about the ignored setting: {want_warning} (got {warned})') + + if rebrowse: + # CHANGE: the plugin re-walks the address space on a fresh session + # (maybe_rebrowse_on_reconnect). Restarting the server is the only + # runtime path that redoes the walk - the node map itself is read + # once at configure() and has no reload. + print(f'--- auto_browse leg ({label}) after a reconnect ---') + terminate(server[0]) + server[0] = start_server(server_bin, server_port, + workdir / 'alarm_server_restarted.log') + if not check(server[0] is not None, 'fixture server restarted on the same port'): + return + reconnected = wait_json(f'{base}/components/{COMPONENT}/x-plc-status', + lambda j: j.get('connected') is True, deadline=120) + if not check(bool(reconnected) and reconnected.get('connected') is True, + 'the plugin reconnected after the server restart'): + print(log.read_text(errors='replace')[-3000:], file=sys.stderr) + return + writable = auto_browse_writable(base, deadline=60) + check(writable is expected, + f'{label}: after the re-walk, writable={expected} (got {writable!r})') + finally: + terminate(gw) + + +def main(): + if len(sys.argv) < 3: + print('usage: test_opcua_read_only.test.py ', + file=sys.stderr) + return 2 + server_bin = Path(sys.argv[1]).resolve() + variant = sys.argv[2] + if variant not in ('read-only', 'write-capable'): + print(f'unknown build variant {variant!r}', file=sys.stderr) + return 2 + read_only = variant == 'read-only' + print(f'build variant under test: {variant}') + + # ROS_DOMAIN_ID comes from the domain wrapper this test is registered + # behind; a missing value is a wiring bug, not something to guess around. + ros_domain_id = os.environ.get('ROS_DOMAIN_ID') + if not ros_domain_id: + print('ROS_DOMAIN_ID is not set: this test must be launched by CTest, which ' + 'runs it behind medkit_run_with_domain.py', file=sys.stderr) + return 1 + + # Every prerequisite below is produced by the same build that produces this + # test, so a missing one is a broken build rather than an environment this + # run should tiptoe around. + for tool in ('ros2', 'nm'): + if shutil.which(tool) is None: + print(f'FAIL: {tool} not on PATH', file=sys.stderr) + return 1 + if not (server_bin.is_file() and os.access(server_bin, os.X_OK)): + print(f'FAIL: fixture server missing: {server_bin}', file=sys.stderr) + return 1 + plugin = find_plugin() + if plugin is None: + print('FAIL: libros2_medkit_opcua_plugin.so not found under AMENT_PREFIX_PATH', + file=sys.stderr) + return 1 + + workdir = Path(tempfile.mkdtemp(prefix='opcua_read_only_')) + env = dict(os.environ, ROS_DOMAIN_ID=ros_domain_id) + manifest = workdir / 'manifest.yaml' + manifest.write_text('manifest_version: "1.0"\n') + server_port = free_port() + server = [start_server(server_bin, server_port, workdir / 'alarm_server.log')] + try: + if server[0] is None: + print('FAIL: fixture server did not become READY', file=sys.stderr) + print((workdir / 'alarm_server.log').read_text(errors='replace'), file=sys.stderr) + return 1 + + run_node_map_leg(workdir, env, plugin, server_port, manifest, read_only) + run_alarm_leg(workdir, env, plugin, server, server_port, manifest, read_only) + for infer_writable, rebrowse in (('absent', True), ('true', False), ('false', False)): + run_auto_browse_leg(workdir, env, plugin, server_bin, server, server_port, + manifest, read_only, infer_writable, rebrowse) + + if failures: + print(f'FAIL: {len(failures)} expectation(s) not met:', file=sys.stderr) + for f in failures: + print(f' - {f}', file=sys.stderr) + return 1 + print(f'PASS: the {variant} plugin behaves as its build declares') + return 0 + finally: + terminate(server[0]) + shutil.rmtree(workdir, ignore_errors=True) + + +if __name__ == '__main__': + sys.exit(main()) diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_address_space_browser.cpp b/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_address_space_browser.cpp index b54bb95f9..9cf19c983 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_address_space_browser.cpp +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_address_space_browser.cpp @@ -183,7 +183,14 @@ TEST(AutoBrowserTest, MapsObjectHierarchyToEntityAndVariablesToDataPoints) { EXPECT_EQ(running_entry->entity_id, "plc_1_db_test"); EXPECT_EQ(running_entry->data_name, "running"); EXPECT_EQ(running_entry->data_type, "bool"); +#if MEDKIT_OPCUA_READ_ONLY + // A read-only build never consults the server's CurrentWrite bit: the + // inference is not compiled in, so a point the server would let us write + // still comes back read-only. + EXPECT_FALSE(running_entry->writable); +#else EXPECT_TRUE(running_entry->writable); // server: CurrentWrite set +#endif EXPECT_FALSE(result.node_cap_hit); EXPECT_FALSE(result.depth_cap_hit); @@ -212,12 +219,62 @@ TEST(AutoBrowserTest, WritableInferredFromServerAccessLevel) { auto result = browser.browse(); ASSERT_EQ(result.entries.size(), 3u); +#if MEDKIT_OPCUA_READ_ONLY + EXPECT_FALSE(find_entry(result.entries, rw.toString())->writable) + << "infer_writable must not produce a writable point in a read-only build"; +#else EXPECT_TRUE(find_entry(result.entries, rw.toString())->writable); +#endif EXPECT_FALSE(find_entry(result.entries, ro.toString())->writable); // AccessLevel read failed -> safe read-only default, never a false-positive. EXPECT_FALSE(find_entry(result.entries, unk.toString())->writable); } +// Whether infer_writable was ASKED FOR is separate from what it evaluates to: +// it defaults to true, so a read-only build that warned on the value would warn +// on every auto_browse deployment. The source string is what tells the two +// apart, and it has to come from either spelling of the key. +TEST(NodeMapAutoBrowseConfigTest, InferWritableSourceRecordsTheNodeMapSpelling) { + NodeMap node_map; + const TempYamlFile yaml_file(R"( +component_id: test_plc +auto_browse: + enabled: true + infer_writable: true +)"); + ASSERT_TRUE(node_map.load(yaml_file.path())); + EXPECT_TRUE(node_map.auto_browse_config().infer_writable); + EXPECT_EQ(node_map.auto_browse_config().infer_writable_source, "the node map's auto_browse.infer_writable"); +} + +TEST(NodeMapAutoBrowseConfigTest, InferWritableSourceStaysEmptyWhenTheKeyIsAbsent) { + NodeMap node_map; + const TempYamlFile yaml_file(R"( +component_id: test_plc +auto_browse: + enabled: true +)"); + ASSERT_TRUE(node_map.load(yaml_file.path())); + // The default is still true - only nobody asked for it. + EXPECT_TRUE(node_map.auto_browse_config().infer_writable); + EXPECT_TRUE(node_map.auto_browse_config().infer_writable_source.empty()); +} + +// The node-map spelling is parsed rather than dropped as an unknown key, so a +// value written there takes effect on a write-capable build. +TEST(NodeMapAutoBrowseConfigTest, InferWritableFalseFromTheNodeMapIsHonoured) { + NodeMap node_map; + const TempYamlFile yaml_file(R"( +component_id: test_plc +auto_browse: + enabled: true + infer_writable: false +)"); + ASSERT_TRUE(node_map.load(yaml_file.path())); + EXPECT_FALSE(node_map.auto_browse_config().infer_writable); + EXPECT_EQ(node_map.auto_browse_config().infer_writable_source, "the node map's auto_browse.infer_writable"); +} + TEST(AutoBrowserTest, InferWritableDisabledKeepsEverythingReadOnly) { FakeAutoBrowseSource source; const auto root = NodeMap::parse_node_id("i=85"); @@ -619,7 +676,11 @@ auto_browse: true ASSERT_NE(level, nullptr); EXPECT_EQ(level->entity_id, "hand_authored_entity"); EXPECT_EQ(level->data_name, "hand_authored_name"); +#if MEDKIT_OPCUA_READ_ONLY + EXPECT_FALSE(level->writable); +#else EXPECT_TRUE(level->writable); +#endif const auto * running = node_map.find_by_node_id("ns=2;s=DB_Test.Running"); ASSERT_NE(running, nullptr); diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_node_map.cpp b/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_node_map.cpp index 08ba58402..2d2389844 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_node_map.cpp +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_node_map.cpp @@ -115,8 +115,15 @@ TEST_F(NodeMapTest, WritableEntries) { ASSERT_TRUE(map.load(yaml_path_)); auto writable = map.writable_entries_for_entity("fill_pump"); +#if MEDKIT_OPCUA_READ_ONLY + // The fixture map declares pump_speed as writable: true. A read-only build + // has no write path, so the request is ignored and no entity has a writable + // entry, whatever the file says. + EXPECT_TRUE(writable.empty()); +#else EXPECT_EQ(writable.size(), 1u); EXPECT_EQ(writable[0]->data_name, "pump_speed"); +#endif auto tank_writable = map.writable_entries_for_entity("tank_process"); EXPECT_TRUE(tank_writable.empty()); @@ -142,7 +149,11 @@ TEST_F(NodeMapTest, FindByNodeId) { auto * entry = map.find_by_node_id("ns=1;s=PumpSpeed"); ASSERT_NE(entry, nullptr); EXPECT_EQ(entry->entity_id, "fill_pump"); +#if MEDKIT_OPCUA_READ_ONLY + EXPECT_FALSE(entry->writable) << "writable: true in the file must not survive a read-only build"; +#else EXPECT_TRUE(entry->writable); +#endif } TEST_F(NodeMapTest, AlarmEntries) { @@ -199,7 +210,13 @@ TEST_F(NodeMapTest, EntityDefs) { ASSERT_NE(pump_def, nullptr); EXPECT_EQ(pump_def->data_names.size(), 2u); +#if MEDKIT_OPCUA_READ_ONLY + // No writable entry means no set_* operation and no x-plc-operations + // capability: the tree cannot advertise a write this build cannot perform. + EXPECT_TRUE(pump_def->writable_names.empty()); +#else EXPECT_EQ(pump_def->writable_names.size(), 1u); +#endif EXPECT_FALSE(pump_def->has_faults); } diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_opcua_client.cpp b/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_opcua_client.cpp index d015014d7..5b0860ff8 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_opcua_client.cpp +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_opcua_client.cpp @@ -77,11 +77,6 @@ TEST(OpcuaClientTest, ReadSourceConditionsReportsScanFailureWhenDisconnected) { EXPECT_FALSE(scan_ok); } -TEST(OpcuaClientTest, WriteWhenDisconnected) { - OpcuaClient client; - EXPECT_FALSE(client.write_value({1, "SomeNode"}, 42.0)); -} - TEST(OpcuaClientTest, CreateSubscriptionWhenDisconnected) { OpcuaClient client; auto id = client.create_subscription(500.0, [](const std::string &, const OpcuaValue &) {}); @@ -94,6 +89,16 @@ TEST(OpcuaClientTest, RemoveSubscriptionsWhenEmpty) { client.remove_subscriptions(); } +#if !MEDKIT_OPCUA_READ_ONLY +// OpcuaClient::write_value exists only in a write-capable build +// (-DMEDKIT_OPCUA_READ_ONLY=OFF). A read-only build has no declaration for +// these three to call, which is the source-level half of the property +// test_opcua_build_variant asserts on the built object. +TEST(OpcuaClientTest, WriteWhenDisconnected) { + OpcuaClient client; + EXPECT_FALSE(client.write_value({1, "SomeNode"}, 42.0)); +} + TEST(OpcuaClientTest, WriteValueReturnsNotConnected) { OpcuaClient client; // Client never connected - write should return NotConnected error @@ -109,6 +114,7 @@ TEST(OpcuaClientTest, WriteValueWithTypeHintDisconnected) { EXPECT_FALSE(result.has_value()); EXPECT_EQ(result.error().code, OpcuaClient::WriteError::NotConnected); } +#endif // !MEDKIT_OPCUA_READ_ONLY // --------------------------------------------------------------------------- // Issue #389: OPC-UA client security config parsing (pure helpers, no server). @@ -300,6 +306,8 @@ TEST(OpcuaClientTest, RemoveEventMonitoredItemUnknownIdDoesNotBumpGeneration) { EXPECT_EQ(client.current_generation(), before); } +// call_method itself is in both builds: ConditionRefresh rides on it and asks +// the server to replay conditions rather than changing any. TEST(OpcuaClientTest, CallMethodWhenDisconnected) { OpcuaClient client; auto result = client.call_method(opcua::NodeId(0, UA_NS0ID_SERVER), opcua::NodeId(0, 11489), {}); @@ -307,6 +315,21 @@ TEST(OpcuaClientTest, CallMethodWhenDisconnected) { EXPECT_EQ(result.error().code, OpcuaClient::MethodError::NotConnected); } +#if !MEDKIT_OPCUA_READ_ONLY +// Acknowledge / Confirm change condition state on the controller, so the entry +// point that issues them exists only in a write-capable build - the same rule +// write_value follows, and what test_opcua_build_variant checks on the object. +TEST(OpcuaClientTest, CallConditionMethodWhenDisconnected) { + OpcuaClient client; + constexpr uint32_t kAcknowledgeMethodId = 9111; + auto result = + client.call_condition_method(opcua::NodeId(2, "Alarms.Overpressure"), opcua::NodeId(0, kAcknowledgeMethodId), + opcua::ByteString("event-id"), "comment"); + ASSERT_FALSE(result.has_value()); + EXPECT_EQ(result.error().code, OpcuaClient::MethodError::NotConnected); +} +#endif // !MEDKIT_OPCUA_READ_ONLY + TEST(OpcuaClientTest, GenerationBumpsOnDisconnect) { OpcuaClient client; // The disconnect-without-connect path is a no-op - generation should not diff --git a/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_opcua_plugin.cpp b/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_opcua_plugin.cpp index 4bfdf4bc6..0c6b58b64 100644 --- a/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_opcua_plugin.cpp +++ b/src/ros2_medkit_plugins/ros2_medkit_opcua/test/test_opcua_plugin.cpp @@ -290,7 +290,13 @@ TEST_F(OpcuaPluginTest, ReadDataReturnsValue) { EXPECT_EQ(result->content["id"], "level"); EXPECT_EQ(result->content["unit"], "mm"); EXPECT_EQ(result->content["data_type"], "float"); +#if MEDKIT_OPCUA_READ_ONLY + // The fixture map says writable: true. A read-only build ignores that, and + // the value it reports has to match what it will actually do. + EXPECT_EQ(result->content["writable"], false); +#else EXPECT_EQ(result->content["writable"], true); +#endif } TEST_F(OpcuaPluginTest, ReadDataNotFound) { @@ -299,6 +305,35 @@ TEST_F(OpcuaPluginTest, ReadDataNotFound) { EXPECT_EQ(result.error().code, DataProviderError::ResourceNotFound); } +#if MEDKIT_OPCUA_READ_ONLY +// A read-only build refuses every write before it looks anything up, so the +// point being writable, read-only or absent, and the body being well formed or +// not, all reach the same answer: 501 and a message naming the build property. +TEST_F(OpcuaPluginTest, WriteDataRefusedOnAReadOnlyBuild) { + const std::vector resources{"level", "pressure", "nonexistent"}; + const std::vector bodies{nlohmann::json{{"value", 5.0}}, nlohmann::json{{"not_value", 42}}, + nlohmann::json::object()}; + for (const auto & resource : resources) { + for (const auto & body : bodies) { + auto result = plugin_.write_data("tank", resource, body); + ASSERT_FALSE(result.has_value()) << resource << " / " << body.dump(); + EXPECT_EQ(result.error().code, DataProviderError::ReadOnly); + EXPECT_EQ(result.error().http_status, 501); + EXPECT_NE(result.error().message.find("MEDKIT_OPCUA_READ_ONLY"), std::string::npos) + << "the refusal must name the build property, got: " << result.error().message; + } + } +} + +// The refusal is not entity-scoped either: an entity this plugin does not own +// is refused the same way rather than answering 404, which would suggest some +// other entity could be written. +TEST_F(OpcuaPluginTest, WriteDataRefusedForUnknownEntityOnAReadOnlyBuild) { + auto result = plugin_.write_data("nonexistent", "level", nlohmann::json{{"value", 5.0}}); + ASSERT_FALSE(result.has_value()); + EXPECT_EQ(result.error().http_status, 501); +} +#else TEST_F(OpcuaPluginTest, WriteDataReadOnly) { nlohmann::json body = {{"value", 5.0}}; auto result = plugin_.write_data("tank", "pressure", body); @@ -313,6 +348,7 @@ TEST_F(OpcuaPluginTest, WriteDataMissingValue) { EXPECT_FALSE(result.has_value()); EXPECT_EQ(result.error().code, DataProviderError::InvalidValue); } +#endif // has_data gates PluginManager::get_data_provider_for_entity (and, through // it, the gateway's `data` capability advertisement) at entity granularity - @@ -334,6 +370,15 @@ TEST_F(OpcuaPluginTest, HasDataFalseForUnknownEntity) { // -- OperationProvider tests -- +#if MEDKIT_OPCUA_READ_ONLY +// The tree must not advertise what the box cannot do: with no writable point +// there is no set_* operation to offer, even though the map asks for one. +TEST_F(OpcuaPluginTest, ListOperationsOffersNoWriteOnAReadOnlyBuild) { + auto result = plugin_.list_operations("tank"); + ASSERT_TRUE(result.has_value()); + EXPECT_TRUE(result->items.empty()); +} +#else TEST_F(OpcuaPluginTest, ListOperationsOnlyWritable) { auto result = plugin_.list_operations("tank"); ASSERT_TRUE(result.has_value()); @@ -341,6 +386,7 @@ TEST_F(OpcuaPluginTest, ListOperationsOnlyWritable) { EXPECT_EQ(result->items.size(), 1u); EXPECT_EQ(result->items[0].id, "set_level"); } +#endif TEST_F(OpcuaPluginTest, ListOperationsEntityNotFound) { auto result = plugin_.list_operations("nonexistent"); @@ -366,6 +412,27 @@ TEST_F(OpcuaPluginTest, HasOperationsFalseForUnknownEntity) { EXPECT_FALSE(plugin_.has_operations("nonexistent")); } +#if MEDKIT_OPCUA_READ_ONLY +// Nothing routes to the value-write branch on a read-only build, but a client +// that posts the operation id directly still has to be refused, and refused for +// the reason that is true. +TEST_F(OpcuaPluginTest, ExecuteOperationRefusedOnAReadOnlyBuild) { + const std::vector ops{"set_level", "set_pressure", "set_nonexistent", "acknowledge_fault", + "confirm_fault"}; + const std::vector params_list{nlohmann::json{{"value", 5.0}}, nlohmann::json{{"not_value", 42}}, + nlohmann::json{{"fault_code", "PLC_OVERPRESSURE"}}}; + for (const auto & op : ops) { + for (const auto & params : params_list) { + auto result = plugin_.execute_operation("tank", op, params); + ASSERT_FALSE(result.has_value()) << op << " / " << params.dump(); + EXPECT_EQ(result.error().code, OperationProviderError::Rejected); + EXPECT_EQ(result.error().http_status, 501); + EXPECT_NE(result.error().message.find("MEDKIT_OPCUA_READ_ONLY"), std::string::npos) + << "the refusal must name the build property, got: " << result.error().message; + } + } +} +#else TEST_F(OpcuaPluginTest, ExecuteOperationMissingValue) { nlohmann::json params = {{"not_value", 42}}; auto result = plugin_.execute_operation("tank", "set_level", params); @@ -379,6 +446,101 @@ TEST_F(OpcuaPluginTest, ExecuteOperationReadOnly) { EXPECT_FALSE(result.has_value()); EXPECT_EQ(result.error().code, OperationProviderError::Rejected); } +#endif + +// -- infer_writable: an explicit request is distinguishable from the default -- + +TEST(OpcuaPluginAutoBrowseConfig, InferWritableSourceRecordsTheParameterSpelling) { + OpcuaPlugin plugin; + nlohmann::json config; + config["endpoint_url"] = "opc.tcp://nonexistent:4840"; + config["auto_browse"] = nlohmann::json{{"enabled", true}, {"infer_writable", true}}; + plugin.configure(config); + EXPECT_EQ(plugin.auto_browse_config_for_test().infer_writable_source, "plugins.opcua.auto_browse.infer_writable"); +} + +TEST(OpcuaPluginAutoBrowseConfig, InferWritableSourceStaysEmptyWhenTheParameterIsAbsent) { + OpcuaPlugin plugin; + nlohmann::json config; + config["endpoint_url"] = "opc.tcp://nonexistent:4840"; + config["auto_browse"] = nlohmann::json{{"enabled", true}}; + plugin.configure(config); + // Enabled, and infer_writable still defaults to true - but nobody asked, so a + // read-only build has nothing to warn about. + EXPECT_TRUE(plugin.auto_browse_config_for_test().enabled); + EXPECT_TRUE(plugin.auto_browse_config_for_test().infer_writable); + EXPECT_TRUE(plugin.auto_browse_config_for_test().infer_writable_source.empty()); +} + +// -- Condition operations (acknowledge / confirm) per build variant -------- +// +// Acknowledging an alarm changes condition state on the controller, so a +// read-only build neither offers nor performs it. An entity with an +// event_alarms source is the only one those operations are ever offered on. + +class OpcuaPluginEventAlarmsTest : public ::testing::Test { + protected: + void SetUp() override { + yaml_path_ = "/tmp/test_opcua_plugin_event_alarms_nodemap.yaml"; + std::ofstream f(yaml_path_); + f << R"( +area_id: test_plc +component_id: test_runtime +nodes: + - node_id: "ns=2;i=1" + entity_id: tank + data_name: level + data_type: float +event_alarms: + - alarm_source: "ns=2;s=Alarms.Overpressure" + entity_id: tank + fault_code: PLC_OVERPRESSURE +)"; + f.close(); + + nlohmann::json config; + config["node_map_path"] = yaml_path_; + config["endpoint_url"] = "opc.tcp://nonexistent:4840"; + plugin_.configure(config); + ctx_.entities["tank"] = {SovdEntityType::APP, "tank", "/test_plc", "/test_plc/test_runtime/tank"}; + plugin_.set_context(ctx_); + } + + std::string yaml_path_; + OpcuaPlugin plugin_; + FakePluginContext ctx_; +}; + +TEST_F(OpcuaPluginEventAlarmsTest, ConditionOperationsOfferedOnlyWhenTheBuildCanPerformThem) { + auto result = plugin_.list_operations("tank"); + ASSERT_TRUE(result.has_value()); + std::vector ids; + for (const auto & item : result->items) { + ids.push_back(item.id); + } + const bool has_ack = std::find(ids.begin(), ids.end(), "acknowledge_fault") != ids.end(); + const bool has_confirm = std::find(ids.begin(), ids.end(), "confirm_fault") != ids.end(); +#if MEDKIT_OPCUA_READ_ONLY + EXPECT_FALSE(has_ack) << "a read-only build must not advertise an alarm acknowledge"; + EXPECT_FALSE(has_confirm) << "a read-only build must not advertise an alarm confirm"; +#else + EXPECT_TRUE(has_ack); + EXPECT_TRUE(has_confirm); +#endif +} + +#if MEDKIT_OPCUA_READ_ONLY +TEST_F(OpcuaPluginEventAlarmsTest, ConditionOperationsRefusedOnAReadOnlyBuild) { + for (const auto & op : {std::string("acknowledge_fault"), std::string("confirm_fault")}) { + auto result = plugin_.execute_operation("tank", op, nlohmann::json{{"fault_code", "PLC_OVERPRESSURE"}}); + ASSERT_FALSE(result.has_value()) << op; + EXPECT_EQ(result.error().code, OperationProviderError::Rejected); + EXPECT_EQ(result.error().http_status, 501); + EXPECT_NE(result.error().message.find("MEDKIT_OPCUA_READ_ONLY"), std::string::npos) + << "the refusal must name the build property, got: " << result.error().message; + } +} +#endif // -- auto_alarms fallback entity: has data/operations fitness + introspect // -- capability registration --