Skip to content

Commit cdbeb0d

Browse files
docs(skill): catch hardcoded chart version stragglers in upgrade protocol
The four-place consistency test does not cover tests that hardcode a version string, so they go stale silently and only surface in CI. Add a straggler grep step, document the known cert-manager load_test.go case, and add a matching post-upgrade checklist item.
1 parent 5976685 commit cdbeb0d

1 file changed

Lines changed: 25 additions & 4 deletions

File tree

  • .claude/skills/educates-upgrade-cluster-services

.claude/skills/educates-upgrade-cluster-services/SKILL.md

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ all four for newer upstream versions and report before changing anything.
2727
2828
## Where each piece of version state lives
2929

30-
A version is pinned in **four** places that must stay in lockstep (a
30+
A version is pinned in **four** canonical places that must stay in lockstep (a
3131
consistency test enforces it — see Verification):
3232

3333
1. `installer/operator/Makefile``VENDORED_CHARTS` list: `name=version=url`.
@@ -38,6 +38,15 @@ consistency test enforces it — see Verification):
3838
`CertManagerVersion` (its chart version equals its appVersion).
3939
4. The tarball file itself on disk.
4040

41+
Beyond these four, some tests **hardcode a version string** rather than
42+
referencing the `embed.go` constant (an import cycle prevents the `helm` package's
43+
tests from importing `vendoredcharts`). The `DirectoryConsistent` test does **not**
44+
cover these, so they go stale silently and only surface as a CI test failure. The
45+
known one is `installer/operator/internal/helm/load_test.go`
46+
(`TestLoadArchive_VendoredCertManager`), which hardcodes the **cert-manager**
47+
version twice (the `.tgz` filename and the `AppVersion` assertion). Always finish a
48+
bump with the straggler grep below — do not rely on the four-place list alone.
49+
4150
## Upstream sources and version semantics
4251

4352
| Service | Upstream releases | Chart URL pattern | App version |
@@ -120,14 +129,25 @@ cert-manager has no separate `AppVersion` constant.
120129
tar -xzOf installer/operator/vendored-charts/contour-<VER>.tgz contour/Chart.yaml | grep -E '^(version|appVersion):'
121130
```
122131

123-
6. **Re-verify reconciler assumptions** (the load-bearing step — see next
132+
6. **Grep the repo for stragglers — hardcoded references to the *old* version**
133+
that live outside the four canonical places (notably tests). Run this with the
134+
OLD version string before you forget it, and update every hit:
135+
```bash
136+
# e.g. old cert-manager version v1.20.2 → catches internal/helm/load_test.go
137+
git grep -n "<old-version>" -- installer/operator
138+
```
139+
For cert-manager this always includes `internal/helm/load_test.go` (filename +
140+
`AppVersion` assertion). Skip the `vendored-charts/` line for the tarball you
141+
just `git rm`'d.
142+
143+
7. **Re-verify reconciler assumptions** (the load-bearing step — see next
124144
section). The per-service reconciler gates readiness on specific workload
125145
names / webhook behaviour verified against a chart version.
126146

127-
7. **`installer/operator/vendored-charts/README.md`** — update the "Current
147+
8. **`installer/operator/vendored-charts/README.md`** — update the "Current
128148
contents" version table.
129149

130-
8. **Kyverno only — also re-vendor the bundled policies.** See "Kyverno
150+
9. **Kyverno only — also re-vendor the bundled policies.** See "Kyverno
131151
special case" below.
132152

133153
## Re-verify reconciler assumptions
@@ -204,6 +224,7 @@ Ready and its reconciler condition flips (`CertificatesReady`, `IngressReady`,
204224
- [ ] `VENDORED_CHARTS` (Makefile), `SHA256SUMS`, and `embed.go` all updated to the new version
205225
- [ ] New tarball downloaded via `make vendor-charts` (hash-verified); old tarball `git rm`'d
206226
- [ ] `embed.go` `<X>AppVersion` read from the new tarball's `Chart.yaml`
227+
- [ ] `git grep "<old-version>"` run; straggler hardcoded refs updated (cert-manager: `internal/helm/load_test.go`)
207228
- [ ] Reconciler workload-name / values / CRD assumptions re-verified; constants + comments updated if changed
208229
- [ ] Kyverno: bundled policies re-vendored + session-manager subchart repackaged (if appVersion changed)
209230
- [ ] README "Current contents" table updated

0 commit comments

Comments
 (0)