feat(ci): the chart bump follows the app bump #130

Merged

jklein merged 1 commits from feat/chart-bump-follows-app-bump into main 2026-10-13 21:06:21 +02:00

Conversation 1 Commits 1 Files Changed 6

+548 -55

jlab-bot commented 2026-10-13 20:48:41 +02:00

Member

Copy link

Copy source

The chart jump follows the app jump: major → major, minor → minor, patch/digest → patch. Not a new rule but a finished one — ci/renovate-bump-chart.sh has documented "app major changed -> chart major" in its header all along. The middle was missing, and that is where the incident of #128 sat: an app minor collapsed onto a chart patch, which is the class the conventions tell the reviewer there is nothing to read in.

No chart directory is touched, so nothing is published and nothing gets a version step. The rule takes effect on the next run of the shared Renovate instance.

What changed

• ci/renovate-bump-chart.sh takes the bump level from Renovate's update type, passed as an optional second argument. Four properties, all decided on the issue:
  • The level comes from Renovate, not from comparing appVersion. An appVersion comparison is blind to the update that prompted the rule: freesailarr keeps a hand-maintained appVersion: "1.0.0" for ten upstreams, and prowlarr's 2.5.2 → 2.6.5 moved an image tag in values.yaml while appVersion stood still.
  • 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 chart major under an app patch. It is also the mechanically necessary half: the hook runs once per update and recomputes from the same base commit every time, so without it the last call wins and freesailarr's grouped PR (three minors among five patches, measured on 2026-10-12) would take whatever level the hook happened to end on. It only ever keeps a higher version, so repeated runs stay idempotent.
  • Non-semver app versions stay at patch for a minor — the seven on main: 2026-09-30, 2026.8.3, 2026.9.13-3b8009065, 2026.8.2-java25, 3.3.1.0-full, 6.0.0-beta13.1, 9.1.2-alpine. In CalVer and date tags the second position is a month, so following it would mint a signal-free chart minor every month; in a four-part or suffixed version the positions are not semver positions at all. An app major is not subject to the fallback, so a year rolling over in a date tag stays a chart major exactly as before.
  • A missing or unknown update type falls back to the pre-#128 behaviour (app major → major, otherwise patch) — see the risk section below.
• ci/run-bump-level-test.sh, new, wired up as the bump-level job in validate-charts.yml. 49 cases, no Helm, no cluster, no pyyaml/yq/Docker, about a second. Each case is a throwaway git init with a committed base Chart.yaml and a dirty one over it, because the script reads the base commit and the working tree — the state the hook really runs in.
• renovate.json5: {{{updateType}}} as the second argument, with the reasoning and the measurements in the comment; the helmv3 (subchart) rule and the freesailarr group rule corrected, since both described the old "bumps the chart patch".
• Convention texts: CLAUDE.md and README.md — the "Patch: … plain app-version updates" line, the wiki exception (where "plain" is now mechanically defined rather than undefined), the recorded open point at README 270–272 (now a decision; the rejected variant and its price are kept, not replaced), and both places that called the subchart safety net a chart patch.
• Wiki: Versioning, Renovate, Workflow, CI-CD, Lessons-Learned, plus Chart-Style-Guide, ytdlp-web-player, bulwark and stalwart, which each carried a sentence this change falsifies.

That {{{updateType}}} reaches the hook is measured, not assumed

Renovate 44.155.7, keycloak pinned one minor back. Two runs, because the obvious one cannot answer the question:

1. --platform=local --dry-run=full against a throwaway checkout reading this branch's config classifies the update ("updateType": "minor", newValue 26.8.1, plus a separate patch bucket for 26.7.5) and resolves the new command — but it never reaches the branch worker. There is not one Processing branch line in the whole log, so the hook never runs under the local platform and that run can prove the lookup and the config, never the command. Saying so is part of the result.
2. Driving Renovate's own post-upgrade executor (dist/workers/repository/update/branch/execute-post-upgrade-commands) against that checkout, with allowedCommands set to the pattern the instance uses:

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 ("One of digest, pin, rollback, patch, minor, major, replacement, pinDigest") and the command is compiled per update, which is what executionMode: "update" means.

The risk this adds, and only the maintainer can close it

postUpgradeTasks matches the allow-list against the compiled command (allowedCommands.some((p) => regEx(p).test(compiledCmd))). The documented pattern is front-anchored and open at the end, so the appended argument is fine:

allowedCommands: ["^ci/renovate-bump-chart\\.sh"]

If the pattern on the instance is anchored at the end (…\.sh$), this change switches the hook off silently — the task is skipped with no error and every chart PR goes red on the version-bump check with no explanation. The allow-list is not readable from this repository; only a look at the instance settles which form is in use.

That is why the script keeps working without the second argument and falls back to today's behaviour. A missing update type — narrower allow-list, older Renovate, unpopulated template field — is then the status quo rather than an outage.

What this does not do, and it is part of the decision

The variant that would have caught the #128 incident at its origin — a chart minor or major obliges reading the upstream release notes, result in the PR even when it is "nothing found" — was rejected. The consequence is written down rather than left implicit, here and in Lessons-Learned:

The classification stops understating the jump; it creates no duty to look behind it. A patch-sized app update that inverts a default can still reach main with nobody reading a release note, exactly as it did for sonarr.

What the honest number does buy is indirect and worth naming precisely: anything watching Helm release versions — Fleet's fleet.yaml in cluster-config included — sees only our chart version, so a minor is a value other gates can act on where a patch is invisible to them. Whether cluster-config actually distinguishes update types is not verifiable from this repository and is not claimed; it is the mechanism that would make the number act.

Evidence

• ci/run-bump-level-test.sh: 49 cases passed locally under dash (/bin/sh here, as on the runner). Cases include every update type, the freesailarr shape, all seven non-semver app versions plus the majors that must not fall back, the maximum rule in both directions including a stale branch version that must not be kept, missing/empty/unknown second argument, and the inputs the script must leave alone (new chart on the branch, template-chart, .github//ci/ arguments, no argument).
• renovate.json5 validates: renovate-config-validator clean, and a full --platform=local --dry-run=full run exits 0 with the new command in the resolved config.
• CI result for the bump-level job is on this PR.

Fixes #128

The chart jump follows the app jump: `major → major`, **`minor → minor`**, `patch`/`digest → patch`. Not a new rule but a finished one — `ci/renovate-bump-chart.sh` has documented "app major changed -> chart major" in its header all along. The middle was missing, and that is where the incident of #128 sat: an app minor collapsed onto a chart patch, which is the class the conventions tell the reviewer there is nothing to read in. No chart directory is touched, so nothing is published and nothing gets a version step. The rule takes effect on the next run of the shared Renovate instance. ## What changed - **`ci/renovate-bump-chart.sh`** takes the bump level from Renovate's update type, passed as an optional second argument. Four properties, all decided on the issue: - **The level comes from Renovate, not from comparing `appVersion`.** An `appVersion` comparison is blind to the update that prompted the rule: `freesailarr` keeps a hand-maintained `appVersion: "1.0.0"` for ten upstreams, and prowlarr's `2.5.2 → 2.6.5` moved an image tag in `values.yaml` while `appVersion` stood still. - **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 chart major under an app patch. It is also the mechanically necessary half: the hook runs once per update and recomputes from the same base commit every time, so without it the **last call wins** and `freesailarr`'s grouped PR (three minors among five patches, measured on 2026-10-12) would take whatever level the hook happened to end on. It only ever keeps a *higher* version, so repeated runs stay idempotent. - **Non-semver app versions stay at patch for a minor** — the seven on `main`: `2026-09-30`, `2026.8.3`, `2026.9.13-3b8009065`, `2026.8.2-java25`, `3.3.1.0-full`, `6.0.0-beta13.1`, `9.1.2-alpine`. In CalVer and date tags the second position is a *month*, so following it would mint a signal-free chart minor every month; in a four-part or suffixed version the positions are not semver positions at all. An app **major** is *not* subject to the fallback, so a year rolling over in a date tag stays a chart major exactly as before. - **A missing or unknown update type falls back to the pre-#128 behaviour** (app major → major, otherwise patch) — see the risk section below. - **`ci/run-bump-level-test.sh`**, new, wired up as the `bump-level` job in `validate-charts.yml`. 49 cases, no Helm, no cluster, no `pyyaml`/`yq`/Docker, about a second. Each case is a throwaway `git init` with a committed base `Chart.yaml` and a dirty one over it, because the script reads the base commit *and* the working tree — the state the hook really runs in. - **`renovate.json5`**: `{{{updateType}}}` as the second argument, with the reasoning and the measurements in the comment; the `helmv3` (subchart) rule and the `freesailarr` group rule corrected, since both described the old "bumps the chart patch". - **Convention texts**: `CLAUDE.md` and `README.md` — the "**Patch**: … plain app-version updates" line, the wiki exception (where "plain" is now *mechanically defined* rather than undefined), the recorded open point at README 270–272 (now a decision; the rejected variant and its price are kept, not replaced), and both places that called the subchart safety net a chart patch. - **Wiki**: `Versioning`, `Renovate`, `Workflow`, `CI-CD`, `Lessons-Learned`, plus `Chart-Style-Guide`, `ytdlp-web-player`, `bulwark` and `stalwart`, which each carried a sentence this change falsifies. ## That `{{{updateType}}}` reaches the hook is measured, not assumed Renovate 44.155.7, `keycloak` pinned one minor back. Two runs, because the obvious one cannot answer the question: 1. `--platform=local --dry-run=full` against a throwaway checkout reading this branch's config **classifies** the update (`"updateType": "minor"`, `newValue 26.8.1`, plus a separate `patch` bucket for `26.7.5`) and resolves the new command — but it never reaches the branch worker. There is not one `Processing branch` line in the whole log, so **the hook never runs under the local platform** and that run can prove the lookup and the config, never the command. Saying so is part of the result. 2. Driving Renovate's own post-upgrade executor (`dist/workers/repository/update/branch/execute-post-upgrade-commands`) against that checkout, with `allowedCommands` set to the pattern the instance uses: ``` 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` ("One of digest, pin, rollback, patch, minor, major, replacement, pinDigest") and the command is compiled per *update*, which is what `executionMode: "update"` means. ## The risk this adds, and only the maintainer can close it `postUpgradeTasks` matches the allow-list against the **compiled** command (`allowedCommands.some((p) => regEx(p).test(compiledCmd))`). The documented pattern is front-anchored and open at the end, so the appended argument is fine: ```yaml allowedCommands: ["^ci/renovate-bump-chart\\.sh"] ``` **If the pattern on the instance is anchored at the end** (`…\.sh$`), this change switches the hook off silently — the task is skipped with no error and every chart PR goes red on the version-bump check with no explanation. The allow-list is not readable from this repository; only a look at the instance settles which form is in use. That is why the script **keeps working without the second argument** and falls back to today's behaviour. A missing update type — narrower allow-list, older Renovate, unpopulated template field — is then the status quo rather than an outage. ## What this does not do, and it is part of the decision The variant that would have caught the #128 incident *at its origin* — a chart minor or major obliges reading the upstream release notes, result in the PR even when it is "nothing found" — was **rejected**. The consequence is written down rather than left implicit, here and in `Lessons-Learned`: > The classification stops understating the jump; it creates no duty to look behind it. A patch-sized app update that inverts a default can still reach `main` with nobody reading a release note, exactly as it did for `sonarr`. What the honest number does buy is indirect and worth naming precisely: anything watching Helm release versions — Fleet's `fleet.yaml` in `cluster-config` included — sees only *our* chart version, so a minor is a value other gates can act on where a patch is invisible to them. **Whether `cluster-config` actually distinguishes update types is not verifiable from this repository** and is not claimed; it is the mechanism that would make the number act. ## Evidence - `ci/run-bump-level-test.sh`: **49 cases passed** locally under `dash` (`/bin/sh` here, as on the runner). Cases include every update type, the `freesailarr` shape, all seven non-semver app versions plus the majors that must not fall back, the maximum rule in both directions including a stale branch version that must *not* be kept, missing/empty/unknown second argument, and the inputs the script must leave alone (new chart on the branch, `template-chart`, `.github/`/`ci/` arguments, no argument). - `renovate.json5` validates: `renovate-config-validator` clean, and a full `--platform=local --dry-run=full` run exits 0 with the new command in the resolved config. - CI result for the `bump-level` job is on this PR. Fixes #128

jlab-bot added 1 commit 2026-10-13 20:48:41 +02:00

feat(ci): the chart bump follows the app bump ... 59d2614384

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

jklein approved these changes 2026-10-13 21:06:16 +02:00

jklein merged commit 59d2614384 into main 2026-10-13 21:06:21 +02:00

jklein deleted branch feat/chart-bump-follows-app-bump 2026-10-13 21:06:21 +02:00

Sign in to join this conversation.

Reviewers

No Reviewers

jklein

Labels

Clear labels

inhouse-chart

Inhouse-Chart aus der JLab-Git-Paketregistry

renovate

Von Renovate angelegt

No labels

Milestone

No items

No Milestone

Assignees

Clear assignees

jklein (Justin Klein) jlab-bot (jlab-git-bot) jlab-renovate-bot (jlab-renovate-bot)

No Assignees

2 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: jlab-cluster/helm-charts#130