A chart pin can be changed to any value and CI will stay green, because nothing renders an addon and the tests that look like they check pins do not.
The index tests are tautological
tests/unit/addons_index_test.bats:58
cilium_v=$(yq -r '.version' "${_PROJECT_ROOT}/.lok8s/addons/cilium/chart.yaml")
out=$(bash "${INDEX_SH}")
assert_equal "$(jq -r '.addons[] | select(.name == "cilium") | .chartVersion' <<<"${out}")" "${cilium_v}"tests/unit/inventory_test.bats:74 does the same thing against the same file.
${INDEX_SH} derives chartVersion by reading chart.yaml. The expected value is read from chart.yaml. Both sides of the assertion come from one source, so the test proves only that yq is deterministic. Its comment says the pins are read "so the test survives deliberate bumps" — which is true, and is also precisely why a wrong pin survives too.
Mutation-proved: setting the loki pin to 6.55.0 — a version from a different chart repository's lineage — renders clean and leaves the suite at 1327/1327 green. There is no pin value that fails.
Nothing renders addons in CI
No workflow runs kustomize build over .lok8s/addons/. A pin bump therefore lands with zero machine-checked evidence that the render is unchanged, that the values we set still exist in the new chart, or that no object appeared or disappeared. All of that is currently done by hand and pasted into the PR body, where it is only as good as the author's diligence and the reviewer's willingness to re-run it.
Proposal: a render-and-diff gate
For each addon whose chart.yaml or values.yaml changed in the PR:
- Render at the merge base and at HEAD (
kustomize build --enable-alpha-plugins --enable-exec, KHELM_TRUST_ANY_REPO=true).
- Diff, normalising the two labels that move on every bump (
helm.sh/chart, app.kubernetes.io/version) and the image tag.
- Post the diff as a PR comment, and fail on a structural change — an added or removed object, or an added RBAC rule — unless the PR carries an ack label.
Sequencing note: this needs network access to the chart repositories from CI. If that is not acceptable, the weaker version still worth having is asserting the pinned version exists in the declared repository's index.yaml and is not marked deprecated — which would have flagged the tempo situation in #146 on its own.
Why it is worth it
#144 widens the cluster-autoscaler ClusterRole: a new podtemplates grant (get,list,watch,create,update,patch), deployments added to the existing apps rule, and three autoscaling.x-k8s.io resource groups. It also silently starts rendering rbac.additionalRules into the namespaced Role as well as the ClusterRole, because 9.59.0 moved an {{- end }} in role.yaml.
Every one of those was found by a human rendering the chart twice and reading the diff. None of them would have failed a single check. A privilege expansion arriving inside a batch labelled low-blast-radius is exactly the thing that should be caught by a gate rather than by attention.
Raised from review of #144.
A chart pin can be changed to any value and CI will stay green, because nothing renders an addon and the tests that look like they check pins do not.
The index tests are tautological
tests/unit/addons_index_test.bats:58tests/unit/inventory_test.bats:74does the same thing against the same file.${INDEX_SH}deriveschartVersionby readingchart.yaml. The expected value is read fromchart.yaml. Both sides of the assertion come from one source, so the test proves only thatyqis deterministic. Its comment says the pins are read "so the test survives deliberate bumps" — which is true, and is also precisely why a wrong pin survives too.Mutation-proved: setting the loki pin to
6.55.0— a version from a different chart repository's lineage — renders clean and leaves the suite at 1327/1327 green. There is no pin value that fails.Nothing renders addons in CI
No workflow runs
kustomize buildover.lok8s/addons/. A pin bump therefore lands with zero machine-checked evidence that the render is unchanged, that the values we set still exist in the new chart, or that no object appeared or disappeared. All of that is currently done by hand and pasted into the PR body, where it is only as good as the author's diligence and the reviewer's willingness to re-run it.Proposal: a render-and-diff gate
For each addon whose
chart.yamlorvalues.yamlchanged in the PR:kustomize build --enable-alpha-plugins --enable-exec,KHELM_TRUST_ANY_REPO=true).helm.sh/chart,app.kubernetes.io/version) and the image tag.Sequencing note: this needs network access to the chart repositories from CI. If that is not acceptable, the weaker version still worth having is asserting the pinned
versionexists in the declaredrepository'sindex.yamland is not markeddeprecated— which would have flagged the tempo situation in #146 on its own.Why it is worth it
#144 widens the cluster-autoscaler ClusterRole: a new
podtemplatesgrant (get,list,watch,create,update,patch),deploymentsadded to the existingappsrule, and threeautoscaling.x-k8s.ioresource groups. It also silently starts renderingrbac.additionalRulesinto the namespaced Role as well as the ClusterRole, because 9.59.0 moved an{{- end }}inrole.yaml.Every one of those was found by a human rendering the chart twice and reading the diff. None of them would have failed a single check. A privilege expansion arriving inside a batch labelled low-blast-radius is exactly the thing that should be caught by a gate rather than by attention.
Raised from review of #144.