chore(ci): detect breakage in Backstage bump and Yarn patch PRs [release-1.9] - #5523
Open
gustavolira wants to merge 3 commits into
Open
gustavolira wants to merge 3 commits into
gustavolira wants to merge 3 commits into
Conversation
…eveloper#5517) * ci: detect breakage in Backstage bump PRs before E2E A Backstage bump that breaks config schemas or plugin APIs was only caught at E2E time. PR CI now recognises bump PRs (backstage.json changed, or yarn.lock re-resolves any @backstage/* package) and for them: - builds (incl. tsc) and tests every package instead of --affected; - runs a new "Backstage bump checks" job that snapshots head and base and compares them: - `config:check --strict` with the image ENTRYPOINT configs. The current config already fails strict validation (dynamic plugin keys have no schema here), so only newly introduced error lines fail. - a diff of the published .d.ts of every directly declared @backstage/* package, reported in the job summary and as an artifact. Report only. The base snapshot is a plain worktree of the merge commit's first parent, installed with its own lockfile. That keeps it working when a PR adds or removes workspace packages, and keeps base packages out of the node_modules cache saved for the PR lockfile. `backstage-cli repo build --api-reports`, named in the issue, does not exist; API reports would need @backstage/repo-tools and would only cover this repo's private packages, which rarely change on a bump. Resolves: RHIDP-13523 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Gustavo Lira e Silva <guga.java@gmail.com> * ci: harden Backstage bump checks after review - Compare config:check errors by params and config path instead of the full message. A @backstage/config-loader bump that rewords ajv messages would otherwise make every pre-existing error look new and fail the job on exactly the major bumps this check targets. - Move the per-package API classification into classifyApiChange in lib.mjs, with shared STATUS constants, so every outcome is unit tested instead of living untested inside compare(). - Surface packages resolving several versions on either side; a consolidation on the head side was dropped from the report. - listWorkspaceDirs now throws on workspace patterns other than "<dir>/*" instead of guessing; the repo uses no other form. - The detect action exposes affected_flag, so build and test no longer duplicate the expression, and its output is renamed run_bump_checks since it is also true when only the checks change. - grep -xF for the literal backstage.json match; test temp dirs are cleaned up; README documents the error identity and the scopes and [skip-build] cases the detection does not cover. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Gustavo Lira e Silva <guga.java@gmail.com> * fix(ci): address SonarCloud findings in Backstage bump checks - Install the base worktree with --mode=skip-build (githubactions:S6505). The snapshot only reads .d.ts files and runs config:check, so no dependency lifecycle script is needed. - Run git from fixed system directories instead of PATH (javascript:S4036). - Sort with localeCompare (S2871), parse the config error key with indexOf instead of a backtracking regex (S8786), and give missing prereleases an explicit "" default in compareVersions (S3403). - Extract snapshotDeclarations() from snapshot() to bring its cognitive complexity under the limit (S3776), and build the report header in one array literal (S7778). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Gustavo Lira e Silva <guga.java@gmail.com> * fix(ci): detect yarn.lock-only Backstage bumps under pipefail GitHub runs `shell: bash` steps with `-eo pipefail`. In `git diff -- yarn.lock | grep -q ...`, grep exits on the first match, git diff dies with SIGPIPE (141) on any real bump-sized lockfile diff, and the `elif` evaluates false. So a PR that only re-resolves @backstage/* packages (e.g. a Renovate security bump) was never detected; full bumps were only caught because backstage.json changed too. Reproduced 3/3 on redhat-developer#5357 with the runner's flags. Use `git diff --quiet -G` instead, which needs no pipe; checked against Also, from the same review: - compareVersions orders prereleases numerically (next.10 > next.9), so the newest resolved version is picked for the API diff. - isBreakingRange treats 0.0.x patch changes as breaking, matching caret semantics. - README records that the params+path error key can merge two rules reporting the same params at one path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Gustavo Lira e Silva <guga.java@gmail.com> * fix(ci): diff every resolution a workspace moved between The API diff only compared the newest resolved version on each side, so a lower resolution moving while the newest stayed put was reported as unchanged: [1.2.0, 2.0.0] -> [1.3.0, 2.0.0] compared 2.0.0 -> 2.0.0 and hid the 1.2.0 -> 1.3.0 migration. Snapshots now record the version each workspace resolves, and resolutionPairs() yields every (base, head) pair a workspace moved between; classifyApiChange sums the diffs over those pairs. When no workspace moved (e.g. only a new workspace uses the new version) it falls back to newest vs newest, which also keeps older snapshots without resolutions working. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Gustavo Lira e Silva <guga.java@gmail.com> --------- Signed-off-by: Gustavo Lira e Silva <guga.java@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 55c6def)
CVE fixes shipped through the patch-backstage workflow land as Yarn patches. Turbo does not see a patch as a change to the packages that consume it, so those PRs only built and tested `--affected` packages. detect-backstage-bump now also flags a PR when it changes a file under .yarn/patches/, or when yarn.lock changes which patches are applied. Such PRs build and test every package and run the Backstage bump checks job (config:check and the API surface diff). Patches of @backstage/* packages were already caught, because the patch hash is part of their yarn.lock resolution. This adds patches of other packages, such as the @janus-idp/cli patch from redhat-developer#4929, which the old detection missed. Refs: RHIDP-13524 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 8af5279)
For a Yarn patch, the full test run only reaches the patched code where existing unit tests already do, and the API surface diff only covers @backstage/* packages. The previous wording called any new failure "a regression from the patch", which overstated the gate. E2E remains the integration gate. On release branches, patches under dynamic-plugins/.yarn/patches/ are not detected, because the checks only cover the root Yarn project. Say so instead of implying coverage. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Contributor
|
The container image build workflow finished with status: |
Contributor
|
The container image build workflow finished with status: |
This branch has not been deployed
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.
Description
Backport of #5517 and #5521 to
release-1.9.Fixes shipped through the
patch-backstageworkflow, including CVE backports, land on release branches as Yarn patches. This branch builds and tests only--affectedpackages on those PRs, and turbo does not see a patch as a change to its consumers. With this backport, a PR that bumps Backstage or changes.yarn/patches/builds and tests every package and runs theBackstage bump checksjob.Conflict resolution in
.github/workflows/pr.yaml:backstage-bump-checksjob is appended after the branch's existingbuildsteps, which are kept.patch-backstageskill does not exist on this branch, so its doc change is dropped.checkout/setup-nodev4,yarn-installv0.6.17,upload-artifactv4).yarn run test, without the jest-junit reporters thatmainadds.Not covered on this branch. Patches under
dynamic-plugins/.yarn/patches/are not detected. The checks only cover the root Yarn project, so detecting them would buy a full root run that never touches the patched wrappers. TheTest the dynamic plugin wrappersandExport the dynamic plugin wrapperssteps run at the repo root: the precedingrun: cd ./dynamic-pluginsstep does not carry over to later steps. They re-run the root tests with--affected, andexport-dynamicat the root only prints a notice. That predates this backport and needs its own fix.Which issue(s) does this PR fix
PR acceptance criteria
node --test scripts/backstage-bump-check/lib.test.mjs, 19 tests)How to test changes / Special notes to the reviewer
This PR changes the checks, so the full build and test run and the
Backstage bump checksjob run on this PR itself againstrelease-1.9. That run is the validation that the scripts work on this branch's dependency tree.Merge after #5521 lands on
main.🤖 Generated with Claude Code