feat(ci): the chart bump follows the app bump
Validate Helm Charts / Detect changed charts (pull_request) Successful in 5s
Validate Helm Charts / Build ${{ matrix.chart }} (pull_request) Skipped
Validate Helm Charts / Install-test (pull_request) Skipped
Validate Helm Charts / Resource quantities (pull_request) Successful in 18s
Validate Helm Charts / Selector match (pull_request) Successful in 14s
Validate Helm Charts / HTTPRoute match (pull_request) Successful in 14s
Validate Helm Charts / Bump level (pull_request) Successful in 7s
Validate Helm Charts / Trivy config cyberchef (pull_request) Successful in 20s
Validate Helm Charts / Trivy config matrix-synapse (pull_request) Successful in 22s
Validate Helm Charts / Trivy config samba (pull_request) Successful in 21s
SonarQube Analysis / SonarQube Analysis (push) Successful in 48s

The version-bump hook classified every app update that was not a major
as a chart patch, so an app minor arrived as a patch - the mechanism
behind the sonarr incident, where a security default was switched off
inside a bump the conventions told the reviewer not to read (Gitea #128).

ci/renovate-bump-chart.sh now takes the level from Renovate's update
type, passed in as a second argument: major -> major, minor -> MINOR,
patch/digest/pin -> patch. The level comes from Renovate rather than
from comparing appVersion because an appVersion comparison is blind to
the case that prompted the rule - freesailarr keeps a hand-maintained
appVersion for ten upstreams, and prowlarr's 2.5.2 -> 2.6.5 moved an
image tag in values.yaml while appVersion stood still.

Three properties come with it:

- Maximum, not replacement. The chart jump is the greater of the
  chart's own classification and the app jump, so a value that becomes
  required stays a major under an app patch. Mechanically it is also
  what makes a grouped branch deterministic: the hook runs once per
  update from the same base commit, so without it the last call would
  win.
- Non-semver app versions stay at patch for a minor (seven charts). In
  CalVer and date tags the second position is a month; in a four-part
  or suffixed version the positions are not semver positions. An app
  major is not subject to it.
- A missing or unknown update type falls back to the pre-#128
  behaviour. postUpgradeTasks only runs where the command is
  allow-listed on the shared instance, which is not visible from here
  and fails silently, so the worst case is the old classification
  rather than every chart PR red on the version check.

ci/run-bump-level-test.sh holds 49 cases against the version string the
script writes, wired up as the bump-level job: a wrong level is a valid
version string, so the PR would go green and the only symptom would be
a number that understates what it publishes.

The convention texts follow in CLAUDE.md and README, including the
limit the decision deliberately keeps: the reading duty was rejected,
so the incident stays repeatable and the honest number is the whole of
the remedy.

Fixes #128
This commit was merged in pull request #130.
This commit is contained in:
jklein committed 2026-10-13 18:47:44 +00:00
1 parent 978e6fbaa8
commit 59d2614384
6 files changed
+548 -55

No files matched your search

+60 -4
View File
@@ -88,8 +88,55 @@
// line can only come from this hook. Nothing has to be configured for the
// move - but the allow-list lives on that instance, not in this repo, so it
// is also not visible from here if it is ever removed.
//
// THE SECOND ARGUMENT IS THE UPDATE TYPE (Gitea #128), and it is what lets
// the chart version follow the app version: major -> major, minor -> MINOR,
// patch/digest -> patch. It is passed rather than derived because Renovate
// has already classified the jump and the script cannot: `freesailarr` keeps
// a hand-maintained `appVersion: "1.0.0"` for ten upstreams, so prowlarr's
// 2.5.2 -> 2.6.5 moved an image tag in values.yaml while appVersion stood
// still - an appVersion comparison is blind to exactly the case the rule was
// written for (README, "The version-bump hook").
//
// VERIFIED, NOT ASSUMED, and it took two runs because one of them cannot
// answer the question. Renovate 44.155.7, keycloak pinned one minor back:
//
// 1. --platform=local --dry-run=full reads THIS file and classifies the
// update ("updateType": "minor", newValue 26.8.1, alongside a separate
// "patch" bucket for 26.7.5). What it does NOT do is reach the branch
// worker at all - there is not one "Processing branch" line in the log -
// so the hook never runs under the local platform and that run can
// prove the lookup and the config, never the command.
// 2. Renovate's own post-upgrade executor
// (dist/workers/repository/update/branch/execute-post-upgrade-commands)
// driven against a throwaway checkout with allowedCommands set to the
// pattern above:
// DEBUG: Executed post-upgrade task
// "cmd": "ci/renovate-bump-chart.sh keycloak minor"
// "stdout": "renovate-bump: keycloak/Chart.yaml 1.3.0+up26.7.0 ->
// 1.4.0+up26.8.1 (update type minor)"
// So the field arrives populated, the front-anchored allow-list accepts
// the command WITH the argument, and the result is a chart minor.
//
// `updateType` is in Renovate's template allowedFields (util/template:
// "One of digest, pin, rollback, patch, minor, major, replacement,
// pinDigest") and the command is compiled per UPDATE here, which is what
// executionMode: "update" below means.
//
// THE RISK THIS ADDS, and it cannot be settled from inside this repo: the
// allow-list is matched against the COMPILED command
// (`allowedCommands.some((p) => regEx(p).test(compiledCmd))`), so the
// front-anchored pattern above still matches with an argument appended. A
// pattern anchored at the END on the instance (`...\.sh$`) would stop
// matching and the task would be skipped with every chart PR red on the
// version check - the same silent failure a missing entry produces. Which
// form the instance uses is not visible from here. The script therefore
// treats a MISSING second argument as the pre-#128 behaviour instead of an
// error, so the worst case is the old classification rather than an outage.
postUpgradeTasks: {
commands: ["ci/renovate-bump-chart.sh {{{packageFileDir}}}"],
commands: [
"ci/renovate-bump-chart.sh {{{packageFileDir}}} {{{updateType}}}",
],
fileFilters: ["**/Chart.yaml"],
// "update" runs the script once per dependency update with the package
// file's directory, which is what the script needs to find the chart. The
@@ -169,9 +216,12 @@
"its size suggests: a subchart is invisible to everything downstream that watches",
"Helm release versions, so this PR is the ONLY way a new proxy version - a security",
"fix included - ever reaches a cluster. Automerge carries that PR to main once it is",
"reviewed and the check is green, and ci/renovate-bump-chart.sh bumps the chart patch",
"that publishes it (appVersion is unchanged, so it takes the patch branch). A major",
"goes the same way - it is the review, not a separate policy, that gives it a look.",
"reviewed and the check is green, and ci/renovate-bump-chart.sh bumps the chart version",
"that publishes it - since #128 at the SUBCHART'S level, so an oauth2-proxy minor is a",
"chart minor even though our appVersion did not move. That is the point of taking the",
"level from Renovate rather than from appVersion: the one number anything downstream",
"sees now reports the size of what is inside the package. A major goes the same way -",
"it is the review, not a separate policy, that gives it a look.",
],
matchManagers: ["helmv3"],
addLabels: ["subchart"],
@@ -262,6 +312,12 @@
"publish, one install-test run for the whole stack. Nothing is left to serialize, so",
"nothing is left to rebase.",
"",
"WHAT LEVEL A MIXED GROUP GETS, since #128 made that a question: the hook still runs once",
"per update and still recomputes from the same base, so the group takes the HIGHEST level",
"in it - three minors among five patches is a chart minor, whatever order the updates are",
"processed in. That is the maximum rule in ci/renovate-bump-chart.sh, and this branch is",
"the reason it is not optional: without it the last call would win.",
"",
"MATCHED BY FILE, NOT BY A LIST OF IMAGE NAMES. What creates the collision is 'touches",
"freesailarr/Chart.yaml', and that is true of every dependency Renovate finds in that",
"chart's values.yaml, whatever its manager, datasource or role in the pod. A",