Skip to content

Commit b51b1d3

Browse files
authored
Add query-contraction check against the previous release (#2085)
## Description Adds a static check that a migration on this branch doesn't break the queries a prior release still runs — the backward-compatibility property that makes a one-step rollback (and a rolling deploy) safe. It compiles a prior release's sqlc queries against the current schema (derived from the migration files) and fails if a migration dropped, renamed, or retyped a column or table an older query still references. Runs in normal CI on every PR — no database, no cluster. ## Targets Checks two prior versions (deduplicated when they coincide): - the latest release reachable from HEAD — the adjacent previous release, and - the previous stable line's latest patch (the `release/vX.Y.x` tip) — the rollback-window floor. ## Running ``` make -C go check-query-contraction ``` Override the targets with `TARGET_VERSIONS` (space-separated, no leading `v`). ## Notes - Verified it passes today and fails on a simulated column drop. - Catches column/table/type-shape contractions; it does not catch data rewrites (no DDL to inspect) or semantic changes a query still compiles against. - Complements the contraction-declaration check in #2067: that one requires destructive DDL to be acknowledged; this one verifies a prior release's queries actually survive the new schema. - Also documents the existing `sqlc generate` gate (schema derived from migrations) as the forward-direction equivalent for current-release queries. - This branch carries its own copies of the helper scripts, which are shared with #2084, so that will require a trivial merge conflict resolution. --------- Signed-off-by: Jeremy Alvis <jeremy.alvis@solo.io>
1 parent f9fcea2 commit b51b1d3

6 files changed

Lines changed: 232 additions & 6 deletions

File tree

.claude/skills/kagent-dev/references/database-migrations.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -205,11 +205,11 @@ These tests catch policy violations at PR time without needing a running databas
205205

206206
## Upgrade and rollback testing
207207

208-
Static analysis covers file *content*; round-trip tests cover *behavior* against a real Postgres. Beyond `runner_test.go` (rollback and concurrency), two release-to-release tests make the rollback promise real. Both are *Target — not yet enforced*.
208+
Static analysis covers file *content*; round-trip tests cover *behavior* against a real Postgres. Beyond `runner_test.go` (rollback and concurrency), release-to-release coverage makes the rollback promise real.
209209

210-
**Previous-minor round-trip.** Seed a database at the previous minor's latest release with representative data, apply migrations up to `HEAD`, and assert the schema matches a clean `HEAD` install and the data survives; then reverse to the previous minor and assert the schema matches a clean previous-minor install and the data survives. This exercises every changed down file rather than only reviewing it.
210+
**Previous-minor round-trip** (*Target — not yet enforced*). Seed a database at the previous minor's latest release with representative data, apply migrations up to `HEAD`, and assert the schema matches a clean `HEAD` install and the data survives; then reverse to the previous minor and assert the schema matches a clean previous-minor install and the data survives. This exercises every changed down file rather than only reviewing it.
211211

212-
**Query-level backward compatibility.** Run the previous minor's database test suite against a `HEAD`-migrated schema, proving old code's queries run against the newer schema — the exact property [ahead-schema tolerance](#rollback-and-ahead-schema-tolerance) relies on.
212+
**Query-level backward compatibility.** A static check — `scripts/check-query-contraction.sh`, run by the `query-contraction-check` CI job — compiles a previous release's sqlc queries against the `HEAD` schema and fails if a migration dropped, renamed, or retyped a column or table an older query still reads. It catches column/table/type-shape contraction with no database, against two prior versions: the latest release reachable from `HEAD` and the previous stable line's latest patch (the `release/vX.Y.x` tip, via `scripts/prev-stable-version.sh`). The fuller property — running the previous minor's whole database *test suite* against a `HEAD`-migrated schema, which also covers semantic breaks a query still compiles against — remains a *Target — not yet enforced*.
213213

214214
## Downstream Extension Model
215215

.github/workflows/ci.yaml

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,25 @@ jobs:
148148
echo "::error::Kubectl logs -n kagent deployment/kagent-controller"
149149
kubectl logs -n kagent deployment/kagent-controller
150150
151+
query-contraction-check:
152+
name: Query Contraction Check
153+
runs-on: ubuntu-latest
154+
steps:
155+
- name: Checkout repository
156+
uses: actions/checkout@v6
157+
with:
158+
# Full history + tags so the previous release's queries can be read.
159+
fetch-depth: 0
160+
fetch-tags: true
161+
- name: Set up Go
162+
uses: actions/setup-go@v6
163+
with:
164+
go-version-file: go/go.mod
165+
cache: true
166+
cache-dependency-path: go/go.sum
167+
- name: Previous-release queries vs current schema
168+
run: make -C go check-query-contraction
169+
151170
go-unit-tests:
152171
runs-on: ubuntu-latest
153172
steps:
@@ -157,7 +176,7 @@ jobs:
157176
- name: Set up Go
158177
uses: actions/setup-go@v6
159178
with:
160-
go-version: "1.26"
179+
go-version-file: go/go.mod
161180
cache: true
162181
cache-dependency-path: go/go.sum
163182

@@ -292,7 +311,7 @@ jobs:
292311
- name: Set up Go
293312
uses: actions/setup-go@v6
294313
with:
295-
go-version: "1.26"
314+
go-version-file: go/go.mod
296315
cache: true
297316
cache-dependency-path: go/go.sum
298317
- name: golangci-lint
@@ -365,7 +384,7 @@ jobs:
365384
- name: Set up Go
366385
uses: actions/setup-go@v6
367386
with:
368-
go-version: "1.26"
387+
go-version-file: go/go.mod
369388
cache: true
370389
cache-dependency-path: go/go.sum
371390

go/Makefile

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,14 @@ generate: controller-gen sqlc-generate ## Generate DeepCopy methods and sqlc que
3333
sqlc-generate: sqlc ## Generate type-safe Go code from SQL queries.
3434
cd core/internal/database && $(SQLC) generate
3535

36+
# Compile the previous release's sqlc queries against the current schema (the
37+
# migration files) and fail if a migration removed/renamed/retyped a column or
38+
# table an old query still uses. Static (no database/cluster). Needs git tags
39+
# fetched. TARGET_VERSIONS (space-separated) overrides the auto-derived release(s).
40+
.PHONY: check-query-contraction
41+
check-query-contraction: sqlc ## Verify the previous release's queries still type-check against the current schema
42+
SQLC=$(SQLC) ../scripts/check-query-contraction.sh
43+
3644
##@ Development
3745

3846
.PHONY: fmt

go/core/internal/database/sqlc.yaml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,11 @@
1+
# `schema` points at the migration files, so `sqlc generate` validates every
2+
# query against the schema those migrations produce. This is a load-bearing
3+
# migration-safety gate: a migration that drops/renames/retypes a column
4+
# a current query uses makes codegen fail, and the manifests-check CI job catches
5+
# the resulting drift. Do not "fix" such a failure by deleting the query — it is
6+
# reporting a real schema/query incompatibility. The previous-release direction
7+
# (old queries vs the new schema = the windowed-contraction invariant) is checked
8+
# by scripts/check-query-contraction.sh.
19
version: "2"
210
sql:
311
- schema: ["../../pkg/migrations/core", "../../pkg/migrations/vector"]

scripts/check-query-contraction.sh

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
#!/usr/bin/env bash
2+
# check-query-contraction.sh — query-contraction check.
3+
#
4+
# Compiles a PREVIOUS release's sqlc queries against the CURRENT schema (the
5+
# migration files under go/core/pkg/migrations) and fails if a migration on this
6+
# branch removed, renamed, or retyped a column or table that an older query still
7+
# references — a change that would break that release's code against the new
8+
# schema (the windowed-contraction invariant).
9+
#
10+
# It checks two targets, deduplicated:
11+
# A) the latest released tag reachable from HEAD — the in-line previous release;
12+
# catches a contraction introduced during the current line's development.
13+
# B) the previous stable line's latest patch (release/vX.Y.x tip, via
14+
# prev-stable-version.sh) — the supported rollback-window floor.
15+
# Today these usually resolve to the same tag (one compile); they diverge once a
16+
# new minor releases or the stable line gets a backport patch.
17+
#
18+
# Static: no database and no cluster. sqlc derives the schema from the migration
19+
# files (see go/core/internal/database/sqlc.yaml), so "does every old query still
20+
# type-check against the new schema" is answerable offline. It catches
21+
# column/table/type-shape contraction; semantic breaks (a new NOT NULL, a
22+
# tightened constraint, an index/ordering change) are out of scope for a static
23+
# check and belong to a runtime regression suite.
24+
#
25+
# Inputs (env):
26+
# TARGET_VERSIONS space-separated versions without leading 'v' to check
27+
# instead of the auto-derived A/B (for local runs).
28+
# SQLC sqlc binary to use (default: sqlc on PATH).
29+
# REMOTE git remote for target B (default: origin).
30+
set -euo pipefail
31+
32+
here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
33+
repo_root="$(git -C "$here" rev-parse --show-toplevel)"
34+
sqlc_bin="${SQLC:-sqlc}"
35+
queries_path="go/core/internal/database/queries"
36+
core_migrations="$repo_root/go/core/pkg/migrations/core"
37+
vector_migrations="$repo_root/go/core/pkg/migrations/vector"
38+
39+
# Resolve the target versions.
40+
targets=()
41+
if [ -n "${TARGET_VERSIONS:-}" ]; then
42+
read -ra targets <<<"${TARGET_VERSIONS}"
43+
else
44+
a="$(git -C "$repo_root" describe --tags --abbrev=0 2>/dev/null | sed 's/^v//' || true)"
45+
b="$("$here/prev-stable-version.sh" 2>/dev/null || true)"
46+
[ -n "${a}" ] && targets+=("${a}")
47+
[ -n "${b}" ] && targets+=("${b}")
48+
fi
49+
if [ "${#targets[@]}" -eq 0 ]; then
50+
echo "ERROR: no contraction target versions resolved; ensure tags are fetched and a release branch exists, or set TARGET_VERSIONS." >&2
51+
exit 1
52+
fi
53+
# Deduplicate, preserving order (version strings are space- and glob-free).
54+
targets=($(printf '%s\n' "${targets[@]}" | awk 'NF && !seen[$0]++'))
55+
56+
workroot="$(mktemp -d)"
57+
trap 'rm -rf "$workroot"' EXIT
58+
59+
# A target resolved (non-empty version) but whose tag is absent locally means
60+
# the checkout didn't fetch tags — a misconfiguration that would otherwise let
61+
# the whole check pass having compiled nothing. Track the two outcomes so the
62+
# post-loop guard can fail on that case while still allowing a legitimately
63+
# empty run (every resolved target predates the sqlc query set).
64+
compiled=0
65+
missing_tag=0
66+
67+
check_target() {
68+
local prev="$1"
69+
local prev_tag="v${prev}"
70+
71+
if ! git -C "$repo_root" rev-parse -q --verify "refs/tags/${prev_tag}" >/dev/null; then
72+
echo "NOTE: tag ${prev_tag} not present locally; skipping (fetch tags to include it)."
73+
missing_tag=$((missing_tag + 1))
74+
return 0
75+
fi
76+
if [ -z "$(git -C "$repo_root" ls-tree "$prev_tag" -- "$queries_path" 2>/dev/null)" ]; then
77+
echo "NOTE: ${prev_tag} has no ${queries_path}; skipping (predates the sqlc query set)."
78+
return 0
79+
fi
80+
81+
# Self-contained sqlc project: sqlc resolves schema/queries relative to the
82+
# config file, so stage everything under a per-target dir. Current migrations
83+
# supply the schema; the previous release supplies the queries.
84+
local wd="$workroot/$prev"
85+
mkdir -p "$wd/schema/core" "$wd/schema/vector" "$wd/queries" "$wd/gen" "$wd/prev"
86+
cp "$core_migrations"/*.sql "$wd/schema/core/"
87+
cp "$vector_migrations"/*.sql "$wd/schema/vector/"
88+
git -C "$repo_root" archive "$prev_tag" "$queries_path" | tar -x -C "$wd/prev"
89+
cp "$wd/prev/$queries_path"/*.sql "$wd/queries/"
90+
91+
# Minimal config: the go_type overrides in the real sqlc.yaml only affect the
92+
# generated Go types, not whether a query type-checks against the schema.
93+
cat >"$wd/sqlc.yaml" <<'EOF'
94+
version: "2"
95+
sql:
96+
- engine: "postgresql"
97+
schema: ["schema/core", "schema/vector"]
98+
queries: "queries"
99+
gen:
100+
go:
101+
package: "dbgen"
102+
out: "gen"
103+
EOF
104+
105+
echo "=== Contraction check: queries@${prev_tag} vs current schema ==="
106+
( cd "$wd" && "$sqlc_bin" compile -f sqlc.yaml )
107+
echo "OK: ${prev_tag} queries still type-check against the current schema."
108+
compiled=$((compiled + 1))
109+
}
110+
111+
for t in "${targets[@]}"; do
112+
check_target "$t"
113+
done
114+
115+
# Guard against a vacuous green: if nothing compiled because resolved targets had
116+
# no local tag, the checkout almost certainly didn't fetch tags. Fail loudly
117+
# rather than report success on an empty run. An all-predate run (no missing
118+
# tags) is legitimately empty and stays green.
119+
if [ "$compiled" -eq 0 ]; then
120+
if [ "$missing_tag" -gt 0 ]; then
121+
echo "ERROR: no targets compiled — ${missing_tag} resolved version(s) had no local tag; fetch tags (fetch-depth: 0, fetch-tags: true) so the contraction check actually runs." >&2
122+
exit 1
123+
fi
124+
echo "NOTE: no targets compiled; all resolved versions predate the sqlc query set."
125+
fi

scripts/prev-stable-version.sh

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
#!/usr/bin/env bash
2+
# prev-stable-version.sh — prints the latest released patch of the stable line
3+
# immediately BELOW the line currently being built: the highest
4+
# vMAJOR.MINOR.PATCH tag on the newest release/vMAJOR.MINOR.x branch whose
5+
# MAJOR.MINOR is strictly less than the current line. This is the rollback-window
6+
# floor a contraction must stay compatible with.
7+
#
8+
# The current line comes from the base/target branch:
9+
# - release/vX.Y.x -> current line is X.Y, so this resolves to the newest
10+
# release line below X.Y (release/v0.9.x -> 0.8.x).
11+
# - main (or any non-release branch) -> the unreleased next minor, which sorts
12+
# above every release line, so this resolves to the newest
13+
# release line overall (-> 0.9.x).
14+
# Source order for the current ref: CURRENT_REF override, GITHUB_BASE_REF (PR
15+
# target), GITHUB_REF_NAME (push), then the checked-out branch.
16+
#
17+
# Prints nothing and exits 0 when no stable line exists below the current one
18+
# (e.g. building the oldest release line), so the caller can skip that target.
19+
# Uses `git ls-remote`, so it needs network to the remote but not the branch
20+
# checked out locally. Output has no leading 'v'. Override the remote with REMOTE.
21+
set -euo pipefail
22+
23+
remote="${REMOTE:-origin}"
24+
25+
# Current line MAJOR.MINOR, or empty for main / any non-release line.
26+
current_ref="${CURRENT_REF:-${GITHUB_BASE_REF:-${GITHUB_REF_NAME:-$(git rev-parse --abbrev-ref HEAD 2>/dev/null || true)}}}"
27+
current_minor=""
28+
if [[ "${current_ref}" =~ ^release/v([0-9]+\.[0-9]+)\.x$ ]]; then
29+
current_minor="${BASH_REMATCH[1]}"
30+
fi
31+
32+
# All release lines on the remote, ascending by version (MAJOR.MINOR only).
33+
lines="$(git ls-remote --heads "$remote" 'refs/heads/release/v*' 2>/dev/null \
34+
| sed -nE 's#.*refs/heads/release/v([0-9]+\.[0-9]+)\.x$#\1#p' \
35+
| sort -V)"
36+
if [ -z "${lines}" ]; then
37+
echo "ERROR: no release/vMAJOR.MINOR.x branch found on ${remote}" >&2
38+
exit 1
39+
fi
40+
41+
# ver_lt A B -> success when A < B by version sort.
42+
ver_lt() { [ "$1" != "$2" ] && [ "$(printf '%s\n%s\n' "$1" "$2" | sort -V | head -1)" = "$1" ]; }
43+
44+
# Highest line strictly below the current line. With no current_minor (main /
45+
# next), every line qualifies, so this lands on the newest line overall.
46+
prev_minor=""
47+
for l in ${lines}; do
48+
if [ -z "${current_minor}" ] || ver_lt "$l" "${current_minor}"; then
49+
prev_minor="$l"
50+
fi
51+
done
52+
if [ -z "${prev_minor}" ]; then
53+
# No stable line below the current one; let the caller skip that target.
54+
exit 0
55+
fi
56+
57+
esc="${prev_minor//./\\.}"
58+
latest="$(git ls-remote --tags "$remote" 2>/dev/null \
59+
| grep -oE "refs/tags/v${esc}\.[0-9]+$" \
60+
| sed 's#refs/tags/v##' | sort -V | tail -1)"
61+
if [ -z "${latest}" ]; then
62+
echo "ERROR: no v${prev_minor}.PATCH release tags found on ${remote} (fetch tags?)" >&2
63+
exit 1
64+
fi
65+
66+
echo "${latest}"

0 commit comments

Comments
 (0)