diff --git a/.changeset/check-driver-versions-across-pins.md b/.changeset/check-driver-versions-across-pins.md new file mode 100644 index 00000000..6de8f34d --- /dev/null +++ b/.changeset/check-driver-versions-across-pins.md @@ -0,0 +1,5 @@ +--- +"ftw": patch +--- + +Require a SemVer bump from any bundled driver whose bytes change between the old pin and the new one, asked of the pinned snapshots rather than of this repository's Git history. diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9d3b7642..52c6715b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -248,6 +248,13 @@ jobs: go run ./cmd/ftw-driver-repository check-versions -repo-root .. -base "${{ needs.changes.outputs.base_sha }}" + + # The check above diffs this repository's history. What reaches a + # gateway is decided by where the pin points, so this asks the same + # question of the pins -- and keeps asking it once the .lua files stop + # being committed here. Quiet on a pull request that does not move it. + - name: Require version bumps across the pin + run: bash scripts/check-driver-versions.sh "${{ needs.changes.outputs.base_sha }}" -head "${{ needs.changes.outputs.head_sha }}" device-support-contract: diff --git a/Makefile b/Makefile index e1e8d87f..6407fbda 100644 --- a/Makefile +++ b/Makefile @@ -14,7 +14,7 @@ .PHONY: help test optimizer-install optimizer-test compose-migration-test container-boundary-test build build-arm64 build-amd64 build-windows-amd64 release \ run-sim dev fmt vet clean e2e ci ci-ui ci-hw-pi docs \ verify verify-all install-hooks driver-repository-validate driver-versions \ - drivers drivers-present + drivers drivers-present driver-versions-across-pin VERSION ?= $(shell git describe --tags --always --dirty 2>/dev/null || echo dev) LDFLAGS := -s -w -X main.Version=$(VERSION) @@ -39,6 +39,7 @@ help: @echo " install-hooks install git pre-commit + pre-push hooks (opt-in)" @echo " driver-repository-validate build and validate unsigned driver release artifacts" @echo " driver-versions require changed Lua drivers to increase SemVer" + @echo " driver-versions-across-pin same rule, asked of the pinned snapshots" @echo " ci run local CI incl. browser smoke" @echo " ci-ui browser smoke against FTW_BASE_URL" @echo " ci-hw-pi deploy candidate to Pi CI slot + browser smoke" @@ -86,7 +87,7 @@ optimizer-test: optimizer/.venv/.installed optimizer/.venv/bin/pytest -q optimizer/tests compose-migration-test: - bash -n scripts/enable-modular-stack.sh scripts/migrate-legacy-compose.sh scripts/install-macos.sh scripts/deploy-home-link-web.sh scripts/sync-bundled-drivers.sh + bash -n scripts/enable-modular-stack.sh scripts/migrate-legacy-compose.sh scripts/install-macos.sh scripts/deploy-home-link-web.sh scripts/sync-bundled-drivers.sh scripts/check-driver-versions.sh bash scripts/test-modular-compose.sh container-boundary-test: @@ -105,6 +106,12 @@ DRIVER_BASE ?= origin/master driver-versions: cd go && go run ./cmd/ftw-driver-repository check-versions -repo-root .. -base $(DRIVER_BASE) -head WORKTREE +# The same rule asked of the pins rather than of this repository's history: +# a bundled driver whose bytes moved between the old pin and the new one must +# have moved its version too. Quiet when the pin has not moved. +driver-versions-across-pin: + bash scripts/check-driver-versions.sh $(DRIVER_BASE) + ci: ./scripts/ci-local.sh diff --git a/README.md b/README.md index e812b511..7ecd35e4 100644 --- a/README.md +++ b/README.md @@ -59,6 +59,12 @@ throwing the committed copies away and rebuilding from the pin alone. That is the groundwork for dropping them from git entirely, so this repository holds no driver source at all. +Moving the pin is a driver release. `make driver-versions-across-pin` compares +the drivers at the old pin against the drivers at the new one and fails when a +file changed without its `DRIVER` version moving — the signed channel refuses to +publish changed bytes under a version it already published, and a gateway +offered the same version twice cannot tell them apart. + ## Install on Linux The installer supports Raspberry Pi OS, Debian and Ubuntu: diff --git a/go/cmd/ftw-driver-repository/main.go b/go/cmd/ftw-driver-repository/main.go index cfa361fe..99419a36 100644 --- a/go/cmd/ftw-driver-repository/main.go +++ b/go/cmd/ftw-driver-repository/main.go @@ -154,14 +154,73 @@ func keygen(args []string) error { return nil } +// checkVersionsAcrossDirs compares two materialised driver snapshots rather +// than two Git revisions. +// +// The bundled drivers are generated from srcfl/device-drivers at a pinned +// commit, so what actually reaches a gateway is decided by where that pin +// points, not by what this repository's history happens to show. Diffing the +// pins asks the question that matters -- did a driver's bytes change without +// its version moving -- and keeps asking it once the files stop being +// committed here at all. +// +// A driver present in new but not in old is newly bundled: nothing to compare, +// and ValidateDriverVersionChange treats an empty previous as fine. One present +// in old but not in new was dropped from the pin, which is not a version +// question. +func checkVersionsAcrossDirs(oldDir, newDir string) error { + entries, err := os.ReadDir(newDir) + if err != nil { + return fmt.Errorf("read new snapshot: %w", err) + } + var failures []error + checked := 0 + for _, entry := range entries { + name := entry.Name() + if entry.IsDir() || filepath.Ext(name) != ".lua" { + continue + } + current, err := os.ReadFile(filepath.Join(newDir, name)) + if err != nil { + failures = append(failures, fmt.Errorf("%s: read new snapshot: %w", name, err)) + continue + } + previous, err := os.ReadFile(filepath.Join(oldDir, name)) + if err != nil { + if !os.IsNotExist(err) { + failures = append(failures, fmt.Errorf("%s: read old snapshot: %w", name, err)) + continue + } + previous = nil // newly bundled + } + checked++ + if err := driverrepo.ValidateDriverVersionChange(name, previous, current); err != nil { + failures = append(failures, err) + } + } + if err := errors.Join(failures...); err != nil { + return err + } + fmt.Printf("driver version changes verified across %d bundled drivers\n", checked) + return nil +} + func checkVersions(args []string) error { fs := flag.NewFlagSet("check-versions", flag.ContinueOnError) repoRoot := fs.String("repo-root", "..", "Git repository root") base := fs.String("base", "", "base Git revision") head := fs.String("head", "HEAD", "head Git revision or WORKTREE") + oldDir := fs.String("old-dir", "", "previous driver snapshot; compares snapshots instead of Git revisions") + newDir := fs.String("new-dir", "", "current driver snapshot; requires -old-dir") if err := fs.Parse(args); err != nil { return err } + if (*oldDir == "") != (*newDir == "") { + return errors.New("-old-dir and -new-dir must be given together") + } + if *oldDir != "" { + return checkVersionsAcrossDirs(*oldDir, *newDir) + } if *base == "" { return errors.New("base revision is required") } diff --git a/go/cmd/ftw-driver-repository/main_test.go b/go/cmd/ftw-driver-repository/main_test.go new file mode 100644 index 00000000..80870df5 --- /dev/null +++ b/go/cmd/ftw-driver-repository/main_test.go @@ -0,0 +1,118 @@ +package main + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// driverFile writes a minimal driver whose DRIVER table carries a version and +// whose body can be varied to change the file's bytes. +func driverFile(t *testing.T, dir, name, version, body string) { + t.Helper() + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + source := "DRIVER = {\n id = \"" + strings.TrimSuffix(name, ".lua") + "\",\n" + + " version = \"" + version + "\",\n}\n\nfunction driver_poll()\n" + body + "\nend\n" + if err := os.WriteFile(filepath.Join(dir, name), []byte(source), 0o644); err != nil { + t.Fatal(err) + } +} + +func snapshots(t *testing.T) (string, string) { + t.Helper() + root := t.TempDir() + oldDir := filepath.Join(root, "old") + newDir := filepath.Join(root, "new") + if err := os.MkdirAll(oldDir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(newDir, 0o755); err != nil { + t.Fatal(err) + } + return oldDir, newDir +} + +// The failure this gate exists for: a driver's bytes move between two pins +// while its version stands still. A gateway offered that driver cannot tell +// the two apart, and the signed channel refuses to publish changed bytes +// under a version it already published. +func TestChangedBytesWithoutAVersionBumpFail(t *testing.T) { + oldDir, newDir := snapshots(t) + driverFile(t, oldDir, "pixii.lua", "2.1.0", " return 5000") + driverFile(t, newDir, "pixii.lua", "2.1.0", " return 1000") + + err := checkVersionsAcrossDirs(oldDir, newDir) + if err == nil { + t.Fatal("changed bytes under an unchanged version were accepted") + } + if !strings.Contains(err.Error(), "pixii.lua") { + t.Errorf("error does not name the driver: %v", err) + } +} + +func TestChangedBytesWithABumpPass(t *testing.T) { + oldDir, newDir := snapshots(t) + driverFile(t, oldDir, "pixii.lua", "2.1.0", " return 5000") + driverFile(t, newDir, "pixii.lua", "2.1.1", " return 1000") + + if err := checkVersionsAcrossDirs(oldDir, newDir); err != nil { + t.Fatalf("a bumped driver was rejected: %v", err) + } +} + +func TestUnchangedDriversPass(t *testing.T) { + oldDir, newDir := snapshots(t) + driverFile(t, oldDir, "pixii.lua", "2.1.0", " return 5000") + driverFile(t, newDir, "pixii.lua", "2.1.0", " return 5000") + + if err := checkVersionsAcrossDirs(oldDir, newDir); err != nil { + t.Fatalf("an untouched driver was rejected: %v", err) + } +} + +// A driver added to the pin's bundle list has no previous bytes to compare +// against. That is not a version question. +func TestNewlyBundledDriverPasses(t *testing.T) { + oldDir, newDir := snapshots(t) + driverFile(t, newDir, "atmoce.lua", "1.0.0", " return 5000") + + if err := checkVersionsAcrossDirs(oldDir, newDir); err != nil { + t.Fatalf("a newly bundled driver was rejected: %v", err) + } +} + +// Dropping a driver from the bundle list is a coverage decision, not a +// version one, so a file present only in the old snapshot is ignored. +func TestDroppedDriverIsIgnored(t *testing.T) { + oldDir, newDir := snapshots(t) + driverFile(t, oldDir, "zap.lua", "1.0.0", " return 5000") + driverFile(t, newDir, "pixii.lua", "2.1.0", " return 5000") + driverFile(t, oldDir, "pixii.lua", "2.1.0", " return 5000") + + if err := checkVersionsAcrossDirs(oldDir, newDir); err != nil { + t.Fatalf("a dropped driver was treated as a failure: %v", err) + } +} + +// Every driver is reported, not just the first bad one, so a pin move that +// missed several bumps is fixed in one pass rather than one per run. +func TestAllOffendersAreReported(t *testing.T) { + oldDir, newDir := snapshots(t) + for _, name := range []string{"pixii.lua", "sungrow.lua", "kostal.lua"} { + driverFile(t, oldDir, name, "1.0.0", " return 5000") + driverFile(t, newDir, name, "1.0.0", " return 1000") + } + + err := checkVersionsAcrossDirs(oldDir, newDir) + if err == nil { + t.Fatal("expected failures") + } + for _, name := range []string{"pixii.lua", "sungrow.lua", "kostal.lua"} { + if !strings.Contains(err.Error(), name) { + t.Errorf("%s missing from the report: %v", name, err) + } + } +} diff --git a/scripts/check-driver-versions.sh b/scripts/check-driver-versions.sh new file mode 100755 index 00000000..e93bc3fa --- /dev/null +++ b/scripts/check-driver-versions.sh @@ -0,0 +1,77 @@ +#!/usr/bin/env bash +# Require a SemVer bump from any bundled driver whose bytes changed. +# +# The drivers here are generated from srcfl/device-drivers at the commit +# pinned in drivers/BUNDLED_SOURCE.json, so what reaches a gateway is decided +# by where that pin points -- not by what this repository's history shows. +# This compares the drivers at the previous pin against the drivers at the +# current one and fails when a file changed without its DRIVER version moving. +# +# Asking the pins rather than the Git log keeps working once the .lua files +# stop being committed here, and is the more honest question even now: it +# measures what ships. +# +# Usage: +# scripts/check-driver-versions.sh +# +# The base revision is only used to read the previous drivers/BUNDLED_SOURCE.json. +# A pull request that does not move the pin has nothing to check. + +set -euo pipefail + +BASE="${1:-}" +[ -n "$BASE" ] || { echo "usage: $0 " >&2; exit 2; } + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +PIN="drivers/BUNDLED_SOURCE.json" + +command -v jq >/dev/null || { echo "jq is required" >&2; exit 2; } + +NEW_PIN_JSON="$(cat "${ROOT}/${PIN}")" +# A pull request that adds the pin for the first time has no previous one. +OLD_PIN_JSON="$(git -C "$ROOT" show "${BASE}:${PIN}" 2>/dev/null || echo '')" + +if [ -z "$OLD_PIN_JSON" ]; then + echo "no ${PIN} at ${BASE}; nothing to compare" + exit 0 +fi + +REPOSITORY="$(printf '%s' "$NEW_PIN_JSON" | jq -r .repository)" +SOURCE_DIR="$(printf '%s' "$NEW_PIN_JSON" | jq -r .source_dir)" +NEW_COMMIT="$(printf '%s' "$NEW_PIN_JSON" | jq -r .commit)" +OLD_COMMIT="$(printf '%s' "$OLD_PIN_JSON" | jq -r .commit)" + +if [ "$OLD_COMMIT" = "$NEW_COMMIT" ]; then + echo "pin unchanged at ${NEW_COMMIT:0:12}; no driver bytes can have moved" + exit 0 +fi + +WORK="$(mktemp -d)" +trap 'rm -rf "${WORK}"' EXIT + +# Materialise one commit's drivers into a flat directory, limited to the +# drivers the current pin actually bundles. A driver added to the list has no +# counterpart in the old snapshot, which reads as "newly bundled". +materialise() { + local commit="$1" dest="$2" + mkdir -p "$dest" + local extract="${WORK}/extract-${commit:0:12}" + mkdir -p "$extract" + curl -fsSL "https://codeload.github.com/${REPOSITORY}/tar.gz/${commit}" \ + | tar -xz -C "$extract" + local src + src="$(find "$extract" -maxdepth 1 -mindepth 1 -type d | head -1)/${SOURCE_DIR}" + [ -d "$src" ] || { echo "no ${SOURCE_DIR} at ${commit}" >&2; exit 1; } + while IFS= read -r id; do + [ -n "$id" ] || continue + [ -f "${src}/${id}.lua" ] && cp "${src}/${id}.lua" "${dest}/${id}.lua" + done < <(printf '%s' "$NEW_PIN_JSON" | jq -r '.drivers[]') +} + +echo "comparing ${REPOSITORY} ${OLD_COMMIT:0:12} -> ${NEW_COMMIT:0:12}" +materialise "$OLD_COMMIT" "${WORK}/old" +materialise "$NEW_COMMIT" "${WORK}/new" + +cd "${ROOT}/go" +go run ./cmd/ftw-driver-repository check-versions \ + -old-dir "${WORK}/old" -new-dir "${WORK}/new"