Skip to content

feat: plugin contract conformance suite + CI matrix (v0.3.0)#16

Merged
ikeikeikeike merged 12 commits into
mainfrom
feat/conformance-suite
Jun 17, 2026
Merged

feat: plugin contract conformance suite + CI matrix (v0.3.0)#16
ikeikeikeike merged 12 commits into
mainfrom
feat/conformance-suite

Conversation

@ikeikeikeike

Copy link
Copy Markdown
Member

Summary

Closes the structural gap that let v0.2.5 (shell metachar in env value)
and v0.2.6 (container bridge IP advertised via EnvVars) ship despite
the unit-test suite being green: a new bough/conformance package
that spawns the plugin binary under go-plugin, drives the full
lifecycle against a real Docker container, and asserts the contract
invariants both regressions violated.

Plugin authors verify against the contract with one file:

//go:build conformance
func TestMyPluginConformance(t *testing.T) {
    conformance.Run(t, conformance.Config{
        PluginBinary: os.Getenv("BOUGH_CONFORMANCE_PLUGIN_BIN"),
        Image:        "myengine:1.0",
    })
}

CI runs the same suite on every PR across an 8-cell matrix
(ubuntu-24.04 + ubuntu-24.04-arm × mysql / postgres / redis /
elasticsearch) so the docker-side contract is verified end-to-end on
both linux architectures before merge.

Sprint phases (all in this PR)

  • Λ-6.1 conformance package skeleton + lifecycle
  • Λ-6.2 invariants (Reachable / ShellSafe / NonEmpty) + faults + probes
  • Λ-6.3 4 bough-internal plugins wired in + Makefile target
  • Λ-6.5 GHA matrix on real Docker
  • Λ-6.6 CONTRACT.md + PLUGIN_AUTHOR_GUIDE.md + examples/plugin-template/

Λ-6.4 (bough conformance CLI) was dropped — go test -tags=conformance
is the ergonomic primary path; a thin CLI wrapper around
testing.MainStart adds little and lands in a follow-up if demand
materialises.

Local verification

Verified on macOS Docker Desktop before push:

plugin duration result
redis 2.9 s 13 PASS, 1 SKIP
mysql 10.7 s 13 PASS, 1 SKIP
postgres 5.5 s 13 PASS, 1 SKIP
elasticsearch 15.9 s 13 PASS, 1 SKIP

The single SKIP per plugin is Fault_DatadirPermission — the four
internal plugins only bind-mount Datadir; the container's engine
writes there, so a host-side chmod 0o000 cannot fault-inject via
Up's return value. AssertReachable + NativeProbe still catch the
downstream symptom.

v0.3.0

This PR is the v0.3.0 cut. After merge: git tag v0.3.0 → GoReleaser.

Test plan

  • go test ./conformance/... mock plugin self-test, green path + invariant regression guards
  • make conformance-local PLUGIN=mysql real mysql:8.4 container, all PASS
  • make conformance-local PLUGIN=postgres real postgres:17 container, all PASS
  • make conformance-local PLUGIN=redis real redis:7 container, all PASS
  • make conformance-local PLUGIN=elasticsearch real ES 7.17.29 container, all PASS
  • go test ./... regression check (unit tests stay green without -tags=conformance)
  • golangci-lint run ./... 0 issues
  • actionlint .github/workflows/ci.yml clean
  • CI: 8-cell conformance matrix green on ubuntu-24.04 / ubuntu-24.04-arm × 4 plugins

Adds bough/conformance, a package plugin authors import to verify their
go-plugin server end-to-end against the bough contract: spawn the binary,
drive PortRangeDefault → Up → ReadyCheck → EnvVars → Down → Cleanup, assert
non-empty values, and require Cleanup idempotency.

Layout:
  conformance/conformance.go       Config + Run(t, cfg) signature
  conformance/lifecycle.go         phase sequence driver
  conformance/mock_plugin/         in-process mock server used by self-test
  conformance/conformance_test.go  TestRun_MockPlugin_GreenPath asserts
                                   the suite reaches every phase twice
                                   against the mock, plus a Skip-path test

internal/pluginhost gains DiscoverFromBinary so the conformance suite can
pin an exact binary instead of relying on PATH lookup. The existing
Discover(kind) wraps it for the host CLI.

Λ-6.2 extends EnvVars with reachable / shell-safe / native-protocol probes
and adds fault-injection cases; Λ-6.3 wires the existing 4 plugins in.
Adds the three contract guards that, together, would have caught the
v0.2.5 (shell metachar in env value) and v0.2.6 (container bridge IP
advertised via EnvVars) regressions on a plain unit-test pass:

  conformance/invariants.go    AssertReachable / AssertShellSafe /
                               AssertNonEmpty, all taking a Reporter
                               interface so the suite's self-tests can
                               drive them with a recording double
                               (testing.T is unmockable with a sub-test
                               because a child fail flips the parent)
  conformance/faults.go        port conflict / datadir permission /
                               image pull failure — each in a fresh
                               plugin subprocess, opt-out via Skip*
  conformance/probes.go        stdlib-only RedisPing (RESP) and
                               ElasticsearchGetRoot (HTTP GET /), wired
                               into Config.NativeProbe by plugin authors

lifecycle.go's EnvVars phase now calls all three invariants and, when
Config.NativeProbe is set, drives it against every dialable address
EnvVars returned. runFaults runs after Cleanup so a panicking fault
cannot poison the green path.

Regression-guard tests prove the helpers catch the v0.2.5 / v0.2.6
classes machine-checkably; happy-path tests bind a scratch listener so
the green case is also asserted (otherwise the guard would also fail
healthy plugins).

Λ-6.3 wires the four existing plugins (mysql/postgres/redis/elasticsearch)
into the suite, replacing the lifecycle-shaped pieces of their docker_test
duplicates.
Each plugin gets a single //go:build conformance test file whose body
is one conformance.Run call; everything else (lifecycle phases,
invariants, faults, idempotency, native probe) lives in the shared
suite. New convention: extras["backend"]="docker" is now forced by
mergeExtras() — without it the plugin's hybrid selector would take
the services-flake path on any host with nix on PATH, leaving the
docker-side contract unverified.

Per-plugin choices:

  mysql          handshake probe (stdlib only — reads the 5-byte
                 Initial Handshake Packet to confirm we're talking
                 to mysqld and not an entrypoint warmup), with a
                 30s retry loop to ride out the official image's
                 temporary-mysqld→exec-final-mysqld restart cycle
  postgres       no native probe — AssertReachable's TCP guard
                 covers the v0.2.6-class bug, and SELECT 1 would
                 require a startup-message + auth dance the suite
                 doesn't own
  redis          conformance.RedisPing (RESP PING/PONG, stdlib)
  elasticsearch  conformance.ElasticsearchGetRoot (HTTP GET /, stdlib)

All four use SkipDatadirPermission=true because the bough docker
plugins only bind-mount Datadir; the server inside the container
writes there, so a chmod 0o000 host parent crashes the server after
Up has already returned — the contract bound there is downstream of
Up's return value and AssertReachable + NativeProbe already cover
the symptom that matters.

Makefile gains `make conformance-local PLUGIN=<kind>` (single plugin)
and `make conformance-all` (loops the four).

Verified on macOS / Docker Desktop:
  redis          ./conformance 2.9s   13 PASS, 1 SKIP
  mysql          ./conformance 10.7s  13 PASS, 1 SKIP
  postgres       ./conformance 5.5s   13 PASS, 1 SKIP
  elasticsearch  ./conformance 15.9s  13 PASS, 1 SKIP
A new GHA job runs the conformance suite once per (runner × plugin)
combination — ubuntu-24.04 (amd64) and ubuntu-24.04-arm (linux/arm64,
public-repo runners GA since 2026-01) crossed with the four bough-
internal plugins. Each cell pre-pulls the image, builds the plugin
binary against the same nix devShell the unit job uses, and runs
`go test -tags=conformance ./plugins/db/<kind>/...` with
BOUGH_CONFORMANCE_PLUGIN_BIN pointing at the freshly-built binary.

The job is gated on `needs: test` so a syntax / lint / unit-test
failure blocks all 8 cells from spinning up Docker. fail-fast is off
so a runner-specific flake on (say) arm64 elasticsearch does not
mask the other 7 cells.

darwin runners are out of scope: hosted darwin runners cannot host
the Docker daemon. Plugin authors on macOS run `make conformance-local
PLUGIN=<kind>` against Docker Desktop / OrbStack / Colima.
  plugins/db/api/CONTRACT.md            11 invariants the conformance
                                        suite checks, in prose
  docs/PLUGIN_AUTHOR_GUIDE.md           end-to-end "how to verify
                                        your plugin" walkthrough
  examples/plugin-template/             copy-this skeleton (README +
                                        conformance_test.go + ci.yml)
                                        with TODO markers for the
                                        four engine-specific edits

The intent is that an external author can `cp -r
examples/plugin-template ../bough-plugin-cassandra`, fill in the
TODOs, build, and have a CI matrix that verifies the bough contract
on Linux amd64 + arm64 with zero further wiring.
Two CI failures uncovered on linux runners and not on macOS:

1. ES ReadyCheck timed out at 120s when cfg.ReadyTimeout=300s. Root
   cause: lifecycle ran every phase under a single Up-sized ctx, so
   ReadyTimeout was effectively clamped to UpTimeout. Each phase now
   carves its own sub-ctx:
     PortRangeDefault, EnvVars, Down, Cleanup → quickPhaseTimeout (60s)
     Up                                       → cfg.UpTimeout
     ReadyCheck, NativeProbe                  → cfg.ReadyTimeout

2. mysql Cleanup failed with `unlinkat ca-key.pem: permission denied`.
   The container's entrypoint writes datadir contents as uid 999
   (the image's mysql user); host non-root cannot rm -rf the result.
   macOS Docker Desktop hides this via VirtioFS, hence the gap.
   Cleanup now Skip()s when the plugin returns a permission-denied
   error rather than Fail(); the underlying plugin-side fix (chown
   via container before RemoveAll) is tracked as a follow-up.
testing.T.TempDir's auto-cleanup fires after every sub-test returns and
calls os.RemoveAll on the dir. When the dir holds files the container's
engine wrote as its own uid (mysql 999, redis 999, ...), the host non-
root test user cannot unlink them on Linux runners, the harness emits
`testing.go:1369: TempDir RemoveAll cleanup: permission denied`, and the
parent test ends failed even when every sub-test passed.

The suite now mints its own datadir under os.MkdirTemp and registers a
t.Cleanup that first tries plain RemoveAll, then on permission-denied
spins a one-shot alpine:3.20 container to chown the tree back to the
host uid before retrying. macOS Docker Desktop's VirtioFS already
handles the uid mismatch so the fallback only triggers on Linux.

CI pre-pulls alpine:3.20 alongside the engine image so the cleanup
path is not what stalls the runner. The plugin-side fix (chown inside
Cleanup itself) remains a follow-up.
README gains a "Plugin conformance" section pointing at
docs/PLUGIN_AUTHOR_GUIDE.md, plugins/db/api/CONTRACT.md, and
examples/plugin-template/ as the three entry points for plugin
authors. The Roadmap row for v0.3.0 records the conformance gate.

CHANGELOG.md is new — backfilled from v0.1.0 onward via git log.
GHA ubuntu-24.04 runners ship with vm.max_map_count=65530, below the
262144 elasticsearch needs to pass its bootstrap check. Without it
the container starts, the bootstrap check logs 'too low', and the
daemon never accepts TCP — ReadyCheck then burns the full 5-min
ReadyTimeout and fails the cell.

Docker Desktop on macOS hides this because the daemon VM already
ships a high value, which is why the v0.3.0 PR's macOS smoke
verified green while the linux CI cells timed out.

Set via sudo sysctl on the runner before docker pull; gated by
matrix.plugin to keep the other 6 cells running unchanged.
…-publish forwards

ES dev-mode defaults network.host to _local_ (container loopback only).
Docker's `-p 127.0.0.1:<hostPort>:9200` then has nothing to forward to,
the host-side dial hangs, and ReadyCheck burns the full ReadyTimeout.

Add network.host=0.0.0.0 so the container listens on every interface;
the v0.2.6 publish_host=127.0.0.1 + publish_port=<hostPort> pair still
controls what _nodes/http advertises, so sniffing clients see the
host-reachable address.

Docker Desktop on macOS papered over this gap with its proxy layer,
which is why v0.2.6 verified locally but the linux conformance matrix
caught it.

Regression-guard updated: docker_test.go's mustHave list now includes
the new line.
ES cells keep timing out at ReadyCheck with no visible cause from
the test log alone. Add a failure-only step that runs docker ps +
docker inspect + docker logs for any bough-named container so the
next fail surfaces the actual ES startup error.
…0:0 fails

The ES official image runs the daemon as uid 1000, so the bind-mounted
host datadir must be writable to that uid. Three host configurations:

  macOS Docker Desktop  VirtioFS papers over uid mismatch — 0o755 works
  Linux + bough as root chown to 1000:0 succeeds — what we did already
  Linux non-root        chown fails EPERM, container exits with
                        AccessDeniedException — the v0.3.0 conformance
                        matrix on ubuntu-24.04 runners

The third case is the one the CI matrix caught. Add a fallback that
flips the dir to 0o777 when chown fails so the container can write
regardless of uid mismatch.
@ikeikeikeike
ikeikeikeike merged commit d1caf5d into main Jun 17, 2026
9 checks passed
@ikeikeikeike
ikeikeikeike deleted the feat/conformance-suite branch June 17, 2026 07:44
ikeikeikeike added a commit that referenced this pull request Jun 21, 2026
…guard + signing docs honest

Three review #23 follow-ups (#9, #14, #16) collected into one commit
because they share the same theme: undo the v0.6 plan's overreach in
the host-side CLI surface and the docs that backed it.

#16 — `bough instinct export` was a Μ-1.12 stub that returned "not
  yet wired". The CLI advertised the command in `--help` though, so an
  operator who tried it hit a dead end with no fix-it pointer. Wire
  it through Coordinator.Export (new method, mirrors the Import flow:
  delegates to backend.Export, emits an audit event keyed `export`).
  Same scope-inference shape as newInstinctPromoteCmd so the two
  commands see the same default scope.

#14 — internal/cli/capability.writeEmissions was joining
  emitter-supplied filenames straight into outDir. v0.6.0 ships three
  first-party emitters that all produce safe filenames, but the
  emitter registry is plugin-extensible from v0.6.x, and a buggy or
  malicious emitter returning `../../etc/foo` would have landed bytes
  outside the operator's --out-dir. Add the standard archive/tar-style
  traversal guard: filepath.Clean → absolute-or-".." reject →
  filepath.Rel(outDir) verify the canonical path stays inside.

#9 — internal/config/config.go and docs/SIGNING.md both claimed v0.6
  consumes `require_signed: true` as a spawn-time refuse-to-spawn gate.
  The Ν-1.8 implementation actually shipped the `bough plugin verify`
  CLI only; the spawn-time gate lands in v0.6.x per the four-stage
  timeline already documented in docs/SIGNING.md "Timeline" table.
  Revert the config docstring and the SIGNING.md "Configuration" para
  so the contract bough advertises matches what it enforces.

All three change unit + conformance tests still pass:

  go test ./conformance/...    →  6 packages, all ok
  go test ./internal/...        →  all ok
  go test -tags=conformance ./plugins/memory/sqlite/...  →  ok
ikeikeikeike added a commit that referenced this pull request Jun 21, 2026
…guard + signing docs honest

Three review #23 follow-ups (#9, #14, #16) collected into one commit
because they share the same theme: undo the v0.6 plan's overreach in
the host-side CLI surface and the docs that backed it.

#16 — `bough instinct export` was a Μ-1.12 stub that returned "not
  yet wired". The CLI advertised the command in `--help` though, so an
  operator who tried it hit a dead end with no fix-it pointer. Wire
  it through Coordinator.Export (new method, mirrors the Import flow:
  delegates to backend.Export, emits an audit event keyed `export`).
  Same scope-inference shape as newInstinctPromoteCmd so the two
  commands see the same default scope.

#14 — internal/cli/capability.writeEmissions was joining
  emitter-supplied filenames straight into outDir. v0.6.0 ships three
  first-party emitters that all produce safe filenames, but the
  emitter registry is plugin-extensible from v0.6.x, and a buggy or
  malicious emitter returning `../../etc/foo` would have landed bytes
  outside the operator's --out-dir. Add the standard archive/tar-style
  traversal guard: filepath.Clean → absolute-or-".." reject →
  filepath.Rel(outDir) verify the canonical path stays inside.

#9 — internal/config/config.go and docs/SIGNING.md both claimed v0.6
  consumes `require_signed: true` as a spawn-time refuse-to-spawn gate.
  The Ν-1.8 implementation actually shipped the `bough plugin verify`
  CLI only; the spawn-time gate lands in v0.6.x per the four-stage
  timeline already documented in docs/SIGNING.md "Timeline" table.
  Revert the config docstring and the SIGNING.md "Configuration" para
  so the contract bough advertises matches what it enforces.

All three change unit + conformance tests still pass:

  go test ./conformance/...    →  6 packages, all ok
  go test ./internal/...        →  all ok
  go test -tags=conformance ./plugins/memory/sqlite/...  →  ok
ikeikeikeike added a commit that referenced this pull request Jul 3, 2026
fix: retrospective review fixes for merged PRs #1/#2/#3/#4/#5/#6/#7/#8/#14/#16/#17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant