test(core): Add a repeatable upgrade-cycle test for the encryption key rollout (no-changelog) - #37885
Draft
sychovSaveliy wants to merge 1 commit into
Draft
Conversation
…y rollout (no-changelog) One command (pnpm test:upgrade) drives a sqlite data folder through seed-on-old-release, upgrade with the rotation flag off, downgrade-read on the old release, and write-on with rotation enabled, asserting the stored format and decrypt round-trips at every step. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
PR review overviewBased on ownership of the 3 changed files in this PR:
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 3 files
Confidence score: 3/5
scripts/upgrade-test/upgrade-cycle.shuses a changinglatestsource binary by default, so repeatable upgrade results can vary over time; pin the default FROM image to a known release.package.jsononly exposespnpm test:upgradelocally, leaving the encryption-key rollout acceptance gate out of continuous integration; wire the suite into CI so every release is covered.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/upgrade-test/upgrade-cycle.sh">
<violation number="1" location="scripts/upgrade-test/upgrade-cycle.sh:24">
P2: According to linked Linear issue IAM-1292, this is the standing repeatable upgrade suite, but the default source binary is not repeatable because `latest` changes over time. Pin the default FROM image to a known release digest or tag, and update that baseline deliberately when the compatibility target changes.</violation>
</file>
<file name="package.json">
<violation number="1" location="package.json:81">
P2: Per linked Linear issue IAM-1292, this test suite must "run in continuous integration" — it is the acceptance gate for every release of the encryption-key rollout. This PR only wires up a local `pnpm test:upgrade` command; no CI workflow calls the script, so regressions will only be caught when a developer runs it manually. Add a workflow (for example a nightly job using the PR-built image as TO) to satisfy the AC rather than deferring it.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Fix all with cubic | Re-trigger cubic
| SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" | ||
| N8N_REPO="${N8N_REPO:-$(cd "$SCRIPT_DIR/../.." && pwd)}" | ||
| N8N_PORT="${N8N_PORT:-5714}" | ||
| FROM_IMAGE="${FROM_IMAGE:-n8nio/n8n:latest}" |
Contributor
There was a problem hiding this comment.
P2: According to linked Linear issue IAM-1292, this is the standing repeatable upgrade suite, but the default source binary is not repeatable because latest changes over time. Pin the default FROM image to a known release digest or tag, and update that baseline deliberately when the compatibility target changes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/upgrade-test/upgrade-cycle.sh, line 24:
<comment>According to linked Linear issue IAM-1292, this is the standing repeatable upgrade suite, but the default source binary is not repeatable because `latest` changes over time. Pin the default FROM image to a known release digest or tag, and update that baseline deliberately when the compatibility target changes.</comment>
<file context>
@@ -0,0 +1,350 @@
+SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
+N8N_REPO="${N8N_REPO:-$(cd "$SCRIPT_DIR/../.." && pwd)}"
+N8N_PORT="${N8N_PORT:-5714}"
+FROM_IMAGE="${FROM_IMAGE:-n8nio/n8n:latest}"
+WORK_ROOT="${WORK_ROOT:-$(mktemp -d "${TMPDIR:-/tmp}/${SPEC_NAME}.XXXXXX")}"
+
</file context>
| "test:ci:backend:integration": "turbo run test:integration --continue --concurrency=1 --filter='!./packages/frontend/**' --filter='!./packages/modules/**'", | ||
| "test:ci:backend:integration:changed": "turbo run test:integration:changed --continue --concurrency=1 --filter='!./packages/frontend/**' --filter='!./packages/modules/**'", | ||
| "test:affected": "turbo run test --affected --concurrency=1", | ||
| "test:upgrade": "./scripts/upgrade-test/upgrade-cycle.sh", |
Contributor
There was a problem hiding this comment.
P2: Per linked Linear issue IAM-1292, this test suite must "run in continuous integration" — it is the acceptance gate for every release of the encryption-key rollout. This PR only wires up a local pnpm test:upgrade command; no CI workflow calls the script, so regressions will only be caught when a developer runs it manually. Add a workflow (for example a nightly job using the PR-built image as TO) to satisfy the AC rather than deferring it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At package.json, line 81:
<comment>Per linked Linear issue IAM-1292, this test suite must "run in continuous integration" — it is the acceptance gate for every release of the encryption-key rollout. This PR only wires up a local `pnpm test:upgrade` command; no CI workflow calls the script, so regressions will only be caught when a developer runs it manually. Add a workflow (for example a nightly job using the PR-built image as TO) to satisfy the AC rather than deferring it.</comment>
<file context>
@@ -78,6 +78,7 @@
"test:ci:backend:integration": "turbo run test:integration --continue --concurrency=1 --filter='!./packages/frontend/**' --filter='!./packages/modules/**'",
"test:ci:backend:integration:changed": "turbo run test:integration:changed --continue --concurrency=1 --filter='!./packages/frontend/**' --filter='!./packages/modules/**'",
"test:affected": "turbo run test --affected --concurrency=1",
+ "test:upgrade": "./scripts/upgrade-test/upgrade-cycle.sh",
"db:schema:docs": "turbo run build --filter=@n8n/db && pnpm --filter=@n8n/db schema:docs",
"db:schema:check": "turbo run build --filter=@n8n/db && pnpm --filter=@n8n/db schema:check",
</file context>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
1 participant
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.
Summary
A one-command, repeatable upgrade test for the encryption-key rollout (v1). It proves the three guarantees every release of the rollout must keep:
<keyId>:<ciphertext>, rotation moves the write key immediately, and all data generations keep decrypting side by side.What it runs
One sqlite data folder travels through four phases (
FROM_IMAGEdefaults ton8nio/n8n:latest, the "new" version is this built checkout):U2FsdGVkX1…); key store seeded with exactly 2deployment_keyrows<activeKeyId>:…;POST /rest/encryption/keysrotates and the very next write uses the new key id; all four generations decryptDecryption is proven through the REST API: the unique seeded secret lives in a non-password credential field, so
GET /rest/credentials/:id?includeData=truereturning it verbatim means the stored blob decrypted (the password-typed field coming back redacted is further evidence — redaction only runs on successfully decrypted data).Every step is asserted against the raw
credentials_entity.datacolumn as well, so the stored format is checked, not just the round-trip.Metrics collected
Each run prints per-phase duration, the number of decrypt round-trips, and their average/max end-to-end latency (raw samples land in
<work root>/metrics.csv) — the seed for the decrypt-regression go/no-go metric from the rollout plan:Console output from a full run (FROM = 2.37.10, TO = this branch stack)
A nice side-finding from P3: the old release boots cleanly on a database already migrated by this checkout (TypeORM ignores applied migrations it does not know), so the downgrade check runs against the real old binary rather than an offline decrypt.
Design notes
mktempfolder, the container is removed by atrap, unique ports; failure prints the failing check plus the instance log tail.How to test
pnpm build # once pnpm test:upgradeOptional knobs:
FROM_IMAGE=n8nio/n8n:2.35.0 pnpm test:upgradeto start from an older release;N8N_PORT,WORK_ROOTas needed. Needs docker,sqlite3,python3(exit 77 = skipped when docker is down).Related Linear tickets, Github issues, and Community forum posts
https://linear.app/n8n/issue/IAM-1292
Review / Merge checklist
🤖 Generated with Claude Code