Add the updater service beside Home Assistant #51

Merged
Claude merged 2 commits from feat/updater-service into feat/firmware-updates 2026-09-19 07:38:42 +00:00
Collaborator

CCS-UHA-1. A Python service that will own the Pi's radio, the firmware cache and the over-the-air conversation with the nodes. This is the skeleton only: it starts, logs what it was configured with, and waits to be stopped. The features are CCS-UHA-2 onwards.

The compose entry takes host networking for can0 and the later hotspot address, /run/dbus read-only for NetworkManager, a bind mount for the firmware cache, and FIRMWARE_REGISTRY_TOKEN defaulting to empty. It has no privileged flag and no added capability, unlike the Home Assistant entry above it — NetworkManager does the privileged work in its own process, and a CAN_RAW socket on an interface that is already up needs none. The container does still run as root, which is what keeps polkit happy when a later ticket starts changing connections.

Tested with the suite plus a real docker build and run: it logs its start line, redacts the token, and stops on SIGTERM in under a second. No compose-parsing test — 7412537 dropped that deliberately. The Dockerfile's Python is pinned to target-version by a check in tests/test_manifest.py, and CI's mypy step now covers updater.

CCS-UHA-1. A Python service that will own the Pi's radio, the firmware cache and the over-the-air conversation with the nodes. This is the skeleton only: it starts, logs what it was configured with, and waits to be stopped. The features are CCS-UHA-2 onwards. The compose entry takes host networking for `can0` and the later hotspot address, `/run/dbus` read-only for NetworkManager, a bind mount for the firmware cache, and `FIRMWARE_REGISTRY_TOKEN` defaulting to empty. It has no `privileged` flag and no added capability, unlike the Home Assistant entry above it — NetworkManager does the privileged work in its own process, and a `CAN_RAW` socket on an interface that is already up needs none. The container does still run as root, which is what keeps polkit happy when a later ticket starts changing connections. Tested with the suite plus a real `docker build` and run: it logs its start line, redacts the token, and stops on SIGTERM in under a second. No compose-parsing test — 7412537 dropped that deliberately. The Dockerfile's Python is pinned to `target-version` by a check in `tests/test_manifest.py`, and CI's mypy step now covers `updater`.
Add the updater service beside Home Assistant
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 19s
Lint, type check and test / quality (pull_request) Successful in 1m45s
Lint, type check and test / release (pull_request) Has been skipped
44b6fc2e49
It starts, logs and waits; the radio, the cache and the OTA conversation
land in CCS-UHA-2 onwards. No privileged flag and no capability: the
privileged work is NetworkManager's, over D-Bus.
Claude left a comment

Service starts, logs, stays up and exits 0 on SIGTERM in ~100ms — verified against a built image. Two test defects and a documentation overclaim below.

Service starts, logs, stays up and exits 0 on SIGTERM in ~100ms — verified against a built image. Two test defects and a documentation overclaim below.
README.md Outdated
@ -161,1 +161,4 @@
integration wants `127.0.0.1` on port `1883` with no credentials.
- The `updater` service is built from `updater/` and will own the Pi's radio,
the firmware cache and the over-the-air conversation with the nodes. It runs
with no privileged flag and no added capability: NetworkManager does the
Author
Collaborator

This reads as a security claim and it doesn't hold. The container is root in the host network namespace with the system bus socket, so it can drive systemd over D-Bus (StartTransientUnit) and that is host root — polkit sees uid 0. :ro doesn't limit it either: a read-only mount only blocks writes to regular files, directories and symlinks, so connect() on the socket still works. Keep the mount as the ticket specifies, but state it as "the privileged work is NetworkManager's" without implying the absence of privileged bounds what the container can do.

This reads as a security claim and it doesn't hold. The container is root in the host network namespace with the system bus socket, so it can drive systemd over D-Bus (`StartTransientUnit`) and that is host root — polkit sees uid 0. `:ro` doesn't limit it either: a read-only mount only blocks writes to regular files, directories and symlinks, so `connect()` on the socket still works. Keep the mount as the ticket specifies, but state it as "the privileged work is NetworkManager's" without implying the absence of `privileged` bounds what the container can do.
compose.yaml Outdated
@ -38,0 +42,4 @@
volumes:
- ./updater-data/firmware:/var/lib/campervan-updater/firmware
- /etc/localtime:/etc/localtime:ro
# NetworkManager does the privileged work in its own process; this only
Author
Collaborator

This comment explains the absence of privileged and cap_add, but sits inside the volumes: list where neither appears. Move it above the service's keys.

This comment explains the absence of `privileged` and `cap_add`, but sits inside the `volumes:` list where neither appears. Move it above the service's keys.
@ -38,0 +49,4 @@
- /run/dbus:/run/dbus:ro
restart: unless-stopped
# Long enough to put the host's network back the way it was found.
stop_grace_period: 30s
Author
Collaborator

"put the host's network back the way it was found" describes work that doesn't exist yet — the container stops in about 100ms today. Say it's headroom for the radio teardown landing in CCS-UHA-2 onwards, or leave the number bare.

"put the host's network back the way it was found" describes work that doesn't exist yet — the container stops in about 100ms today. Say it's headroom for the radio teardown landing in CCS-UHA-2 onwards, or leave the number bare.
pyproject.toml Outdated
Author
Collaborator

Coverage still measures only custom_components/campervan, so the new package reports nothing while the rest of the repo sits at 99%. Add --cov=updater if that bar is meant to apply here too.

Coverage still measures only `custom_components/campervan`, so the new package reports nothing while the rest of the repo sits at 99%. Add `--cov=updater` if that bar is meant to apply here too.
@ -0,0 +40,4 @@
"""The startup line goes to the container log, which is not a secret store."""
summary = Settings(Path("/cache"), "secret").describe()
assert "secret" not in summary
assert "set" in summary
Author
Collaborator

"set" in summary is satisfied by the substring inside "unset", so both summary tests pass with a describe() that always reports the token missing — I mutated it to a literal "registry token unset" and all 9 tests still passed. Assert "registry token set" in summary.

`"set" in summary` is satisfied by the substring inside `"unset"`, so both summary tests pass with a `describe()` that always reports the token missing — I mutated it to a literal `"registry token unset"` and all 9 tests still passed. Assert `"registry token set" in summary`.
@ -0,0 +89,4 @@
running = asyncio.create_task(run(Settings(tmp_path, "")))
await asyncio.sleep(0)
signal.raise_signal(signal.SIGTERM)
Author
Collaborator

If signal registration ever regresses, this raises SIGTERM with no handler installed and kills the pytest process mid-run: no failure report, and every test after it is lost. Verified by stubbing out add_signal_handler — the run died after 8 dots with no summary. Assert the handler is in place first (signal.getsignal(signal.SIGTERM) is not signal.SIG_DFL) so the regression fails as an assertion.

If signal registration ever regresses, this raises SIGTERM with no handler installed and kills the pytest process mid-run: no failure report, and every test after it is lost. Verified by stubbing out `add_signal_handler` — the run died after 8 dots with no summary. Assert the handler is in place first (`signal.getsignal(signal.SIGTERM) is not signal.SIG_DFL`) so the regression fails as an assertion.
@ -0,0 +8,4 @@
COPY requirements.txt ./updater/
RUN pip install --no-cache-dir -r updater/requirements.txt
COPY . ./updater/
Author
Collaborator

No .dockerignore, so COPY . ./updater/ bakes the build machine's __pycache__ and the Dockerfile itself into the image — confirmed present in the built image at /app/updater/__pycache__. COPY preserves mtimes, so a stale .pyc can validate and win over the source beside it. Add updater/.dockerignore with __pycache__/, *.py[cod] and Dockerfile.

No `.dockerignore`, so `COPY . ./updater/` bakes the build machine's `__pycache__` and the Dockerfile itself into the image — confirmed present in the built image at `/app/updater/__pycache__`. COPY preserves mtimes, so a stale `.pyc` can validate and win over the source beside it. Add `updater/.dockerignore` with `__pycache__/`, `*.py[cod]` and `Dockerfile`.
@ -0,0 +25,4 @@
leaving it out, so an empty string has to mean the default.
"""
source = os.environ if environment is None else environment
cache = source.get("FIRMWARE_CACHE") or DEFAULT_FIRMWARE_CACHE
Author
Collaborator

FIRMWARE_CACHE is a knob nothing sets — the compose entry only passes TZ and the token, and the ticket scoped it to a volume at a fixed path. Either drop it and use the constant, or set it in compose beside the mount so the two can't drift. Two of the five settings tests exist only for this path.

`FIRMWARE_CACHE` is a knob nothing sets — the compose entry only passes `TZ` and the token, and the ticket scoped it to a volume at a fixed path. Either drop it and use the constant, or set it in compose beside the mount so the two can't drift. Two of the five settings tests exist only for this path.
Take the review: pin the token line, and stop overclaiming
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 12s
Lint, type check and test / quality (pull_request) Successful in 1m55s
Lint, type check and test / release (pull_request) Has been skipped
0815f0202c
Claude left a comment

All seven addressed, nothing further. Re-ran both mutations against 0815f02: forcing describe() to a literal "registry token unset" now fails, and stubbing out add_signal_handler fails as an assertion with the run's report intact. Rebuilt image carries only the source files. Leaving main() and __main__.py uncovered is the right call — __main__.py exits on import and a test for main() would only assert asyncio.run was called.

All seven addressed, nothing further. Re-ran both mutations against 0815f02: forcing `describe()` to a literal `"registry token unset"` now fails, and stubbing out `add_signal_handler` fails as an assertion with the run's report intact. Rebuilt image carries only the source files. Leaving `main()` and `__main__.py` uncovered is the right call — `__main__.py` exits on import and a test for `main()` would only assert `asyncio.run` was called.
Claude merged commit 1c823e8d5c into feat/firmware-updates 2026-09-19 07:38:42 +00:00
Claude deleted branch feat/updater-service 2026-09-19 07:38:42 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
rob/CampervanHomeAssistant!51
No description provided.