Repository navigation
ci: run the store suites on PostgreSQL, MySQL, MariaDB and Redis - #122
Merged
Merged
Conversation
The legacy-payload tests of oauth2/store/redis listed the real Redis only when OAUTH2_TEST_REDIS_ADDR was set, and otherwise left its cases out without a trace. A renamed variable would then drop, unnoticed, the only real-Redis evidence for cjson's truthy userdata null, even from a run that refuses skipped tests. The real server is now always listed and its cases skip while the variable is unset, like the conformance test, which reads the same constant. A case that forgets the skip fails on its nil client instead of passing. TestContextBackendsListTheRealServer pins the listing. Audit: STORE-DL.
…them The shared-store tests left Redis, PostgreSQL and MySQL out of a run whose variable was unset, without a trace: a renamed variable would silently drop a real server even from a run that refuses skipped tests. The five opt-in backends are now always listed. Their subtests skip while their variable is unset, as in the other real-backend suites, and their namespace opener skips too, so that no client is ever built on an empty address (go-redis would take it for localhost:6379). TestStoreBackendsListTheOptInServers pins the listing. Audit: STORE-DL.
CI ran the SQL and Redis stores on SQLite and miniredis only; their real-server suites were opt-in and ran locally. A second job of the Go workflow, "Store suites (real backends)", now runs them with the race detector on every pull request and push to master. It uses service containers of the images the compose files pin by digest: PostgreSQL 16, MySQL 8.0 and 8.4, MariaDB 11.4 and Redis 7.4. make test-backends (scripts/test-backends.sh) runs these suites: - internal/interop/sqlstores; - the root package of internal/interop/oidcstack; - internal/interop/saml, except its Keycloak and browser-helper tests; - oauth2/store/redis. It fails when one of its eleven variables is missing, when a test fails, when a suite runs no test, and when any test is skipped. A real-backend test skips while its variable is unset, so a skip means a server the run does not provide. -count=1 keeps a cached result from standing in for a run. The services hold no credentials: PostgreSQL trusts every connection, the MySQL and MariaDB root has no password, and Redis has no auth. Their ports are published on the runner's 127.0.0.1 only, so nothing outside the disposable VM can reach them, and every process inside it is the job's own code. A generated password would not stay secret anyway: the runner prints service environments in the log. The job has a cache key of its own for its race-built packages. Audit: STORE-DL.
LIMITATIONS, the security considerations, the OpenID Connect and SAML guides, the CHANGELOG and the package documentation of the three SQL stores said that PostgreSQL and MySQL were not exercised in CI, and that their suites ran locally only. The "Store suites (real backends)" job now runs them, so they say what CI runs and on which servers: PostgreSQL 16.15, MySQL 8.0.46 and 8.4.11, MariaDB 11.4.13, Redis 7.4.11. They also note that MySQL 8.0 reached its end of life in April 2026, and that other versions, compatible engines, replicas and proxies stay untested. docs/releasing.md now covers both jobs of the Go workflow in the release checklist. It names the new job as a required check once proven stable, and warns that renaming a job breaks the required check. It also notes that Dependabot updates neither the service images nor the compose files, which pin the same digests and are bumped together. README and MIGRATION list `make test-backends`. Audit: STORE-DL.
CLAUDE.md still called the real-backend store runs opt-in only. It now lists `make test-backends`, describes the "Store suites (real backends)" job of go.yml (PostgreSQL 16, MySQL 8.0/8.4, MariaDB 11.4 and Redis 7.4 service containers, pinned by digest, loopback only, no credentials), and states the rule that keeps that job meaningful: a real-backend test skips while its variable is unset, the job fails on any skip, so a new real-backend test adds its variable to scripts/test-backends.sh and to the job. The interop table and the oidcstack and SAML bullets say that CI runs these suites, opt-in locally. Audit: STORE-DL.
A real-backend test in a package that scripts/test-backends.sh does not list never runs in CI, and the job stays green: the rule now says such a package also gets a `suite` line in the script, as well as its variable. Audit: STORE-DL.
The header of scripts/test-backends.sh told a new real-backend test to skip while its variable is unset, and to add the variable to the script and to the job. That is not enough for a test in a package the script does not run: it never runs in CI, and the job stays green. Such a package also gets a `suite` line. A new opt-in test of another kind, in a listed package, goes into that suite's skip pattern, or the run fails on its skip. One go test invocation replaces the two that avoided an empty -skip: an empty pattern skips nothing. A comment now says why -count=1 is required. The DSNs are fixed strings, so a cached result would stand in for a run against the servers and pass every guard: with the servers stopped, a run without the flag passed every suite from the cache in 2 s. Audit: STORE-DL.
euskadi31
force-pushed
the
ci/real-db-stores
branch
from
October 13, 2026 21:18
aa33b82 to
141e607
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follows up a finding of the internal security audit of 2026-10-12 (commit c5c2ac3): STORE-DL. PostgreSQL and MySQL were not exercised in CI.
Goal
The guarantees the SQL and Redis stores give on real servers were only checked by opt-in suites that ran on a developer's machine:
Migrateunder PostgreSQL's advisory lock;nullon Redis.PR J (#121) documented this gap. This PR closes it: CI runs every environment-gated real-backend store suite on every pull request and every push to master, and fails when any of them is skipped.
Design
The job
Store suites (real backends)is a second job of.github/workflows/go.yml. It runs in parallel withBuild & test (workspace), with the same triggers, the read-only token and the same concurrency group.postgres:16@sha256:ca0bd484…127.0.0.1:35432mysql:8.0@sha256:7dcddc01…127.0.0.1:33306mysql:8.4@sha256:24989179…127.0.0.1:33307mariadb:11.4@sha256:12928441…127.0.0.1:33308redis:7.4@sha256:4fa24486…127.0.0.1:36379Images. Each digest is an OCI image index with a linux/amd64 manifest.
Health checks run over TCP:
pg_isready -h 127.0.0.1,mysqladmin ping -h 127.0.0.1, and MariaDB'shealthcheck.sh --connect. The server that initializes a database listens on its socket only, so a check passes only once the final server is up.Redis keeps the image defaults:
maxmemory 0andnoeviction, as the stores require.Steps:
build's: checkout v7.0.1 and setup-go v7.0.0, withpersist-credentials: false.timeout-minutes: 30, while go test bounds each suite at 300 s.make test-backends(scripts/test-backends.sh)The script runs the suites one after another, with
-race -count=1 -timeout 300s -json:internal/interop/sqlstoresinternal/interop/oidcstackinternal/interop/saml-skip; they are opt-in on their own)oauth2/store/redisIt fails when:
Every suite runs and reports before the exit.
-count=1is required. The DSNs are fixed strings and setup-go restores the test cache, so a cached replay would pass every guard without reaching any server (shown below).The script's header gives a local recipe (the sqlstores compose file, plus the oidcstack compose file's Redis). It also gives the rule for a new real-backend test:
suiteline if the script does not run it yet.Unset backends now skip instead of vanishing
Two suites left a backend out of the run when its variable was unset. A skip guard cannot see that.
internal/interop/saml(TestSharedStore*).localhost:6379.oauth2/store/redis. The real-Redis cases of the two legacy-payload tests now skip like the conformance test, through oneenvRedisAddrconstant. They are the only real-Redis evidence for cjson's truthy userdatanull.Routine
make testnow reports these cases as skipped (15 + 3), as it already did for sqlstores and oidcstack.Docs
make test-backendsin README and MIGRATION.docs/releasing.md:Decisions (Axel)
trust, MySQL and MariaDB have a root without password, and Redis has no auth. Ports are published on the runner's 127.0.0.1 only.docker create … -e) in the log.Tests
Red first.
TestContextBackendsListTheRealServer(oauth2/store/redis) failed withexpected [miniredis redis],actual [miniredis].TestStoreBackendsListTheOptInServers(saml) failed withexpected [sqlite-file miniredis redis postgres mysql …],actual [sqlite-file miniredis].After the change, with the variables unset there are 15 skip events (saml) and 4 (oauth2/store/redis). Each says "set X to run against a real Y (opt-in)".
Mutants of the test changes:
skipIfUnsetremovedruntime.Goexiton the parent)Script guards. The independent review ran its fake-go harness under dash: 17 mutants plus real runs, and every non-equivalent mutant is killed. After the review fixes, the status-handling mutants of the changed line were run again and are still killed.
Real servers, locally:
-count=1, servers stopped(cached)CI, on this PR:
Every green run of the job printed
314 / 156 / 286 / 104 tests passed, none skipped. Passes per backend were the same in every run, counted from the job logs:startup_failure; no job started (see the notes)aa33b82renamesSAMLSP_POSTGRES_DSNin the test onlyinternal/interop/saml: FAILED: the tests above were skipped, a server this run does not provide. The other suites still ran and passed (314 / 156 / 104).Build & testpassed.141e607The job runs in parallel with
Build & test (workspace), which takes about 5 to 7 min. So it adds no wall time: about 3 min on a warm cache, 5 min cold.Checks
make generate sync build test lint check-modulespasses: lint 0 issues, check-modules ok, no sync churn;make vuln: no vulnerabilities in 26 modules.make test) of the store packages the job exercises:oauth2/store/redis(test file changed)oauth2/store/sql(doc.go only)oidc/client/store/sql(doc.go only)oidc/client/store/redissaml/sp/store/sql(doc.go only)saml/sp/store/redisThe only production changes are package doc comments. There is no go.mod, go.sum or go.work change.
Follow-ups (out of scope)
Notes for the maintainer
Store suites (real backends)to the master ruleset's required status checks once it has proven stable: this PR's run, the 3 re-runs above, and the first push to master, all green.buildandgovulncheckboth key setup-go's cache onhashFiles("**/go.sum").buildnever saves its compiled packages ("Unable to reserve cache … another job may be creating this cache").build(or govulncheck) a distinct key is a separate change.startup_failure. On run 38463962065, the second re-run of the job never started. The page said "An unexpected error has occurred and we've been automatically notified. Errors are sometimes temporary, so please try again", and requesting the run again failed the same way twice.TestRealBackendReplicaRace/mysqlinreadHandshakePacket. A run opens about 565 connections, through the macOS port forwarder.