opcua: make read-only a property of the build, not a setting

.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

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 $<TARGET_OBJECTS:...>, 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"
+            "$<TARGET_FILE:ros2_medkit_opcua_plugin>"
+            --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.