Repository navigation
[Backport release-1.6] fix(cozy-lib): make the cozyconfig helpers survive uninjected values - #5141
Merged
Merged
Conversation
`_cluster` and `_namespace` reach a chart through valuesFrom on the
cozystack-values Secret, so they are absent whenever a chart is rendered
on its own: `helm lint`, a plain `helm template`, and any helm-unittest
suite that does not set them. Twelve of the helpers in _cozyconfig.tpl
read straight through the missing map and abort the render rather than
returning their documented empty default.
Two spellings, one cause. `index .Values._cluster "root-host"` raises
`index of untyped nil`, and `.Values._namespace.host` raises
`nil pointer evaluating interface {}.host`. In both cases the trailing
`| default ""` never runs: it applies to the result, and there is no
result, only an error. That default covered a key missing from a map
that exists, never a missing map.
Guard the map instead, which is what `cozy-lib.images-registry` in this
same file already did:
(index (.Values._cluster | default dict) "root-host") | default ""
`index` on an empty map returns nil harmlessly, so the trailing default
applies as it always read. With the map present, `| default dict`
returns it untouched.
This changes no rendered output. Every one of the 34 charts that symlink
cozy-lib was rendered before and after, both with the injected values
populated and with them set to empty maps, and every result is
byte-identical. Eight charts differ between any two runs because they
generate passwords; rendering the unchanged tree twice reproduces
exactly those eight, which is how they are excluded.
What this does not do is fix a chart that fails today. The charts that
cannot render standalone read the injected values directly in their own
templates rather than through these helpers, so they are unaffected by a
change to the library and are being addressed where they live.
Twelve of the thirteen helpers here have no callers anywhere in
packages/ at present; only `cozy-lib.images-registry` is reached, from
`cozy-lib.image`, and it was already guarded. The guard is still worth
having on the rest: they are the library's public surface, they are what
a chart author copies when reaching for cluster configuration, and the
broken spelling propagates by being read. `cozy-lib.image` is called
from four templates, so these helpers do get adopted once they exist.
Add a suite in cozy-lib-tests, which is where library behaviour is
pinned because a library chart renders nothing of its own. One case
covers the uninjected state, which needs no mocking since neither key is
in values.yaml, and one covers the injected state so the guard cannot
quietly change what a populated map returns. The injected case includes
`scheduling` and `branding` specifically: those two are the helpers
whose body was restructured rather than wrapped, so they are the ones
whose present-map behaviour most needs pinning. Against the unguarded
helpers the first case fails with `index of untyped nil`.
(cherry picked from commit 93ffb38)
On release-1.6 one of these helpers does have a caller that fails
today, so the two paragraphs above about callers do not hold here:
tenant.ciliumPolicies calls cozy-lib.bundle-name since the isp-hosted
Cilium guard was backported in 2f820f7. Every tenant suite that
renders templates/networkpolicy.yaml without setting _cluster aborts
with `index of untyped nil` in the helper, which is how the
vm-exportproxy suite has failed on this branch since then. With the map
guarded, bundle-name returns "" there and the policies render as on
every non-isp-hosted bundle. The patch applies unchanged.
Assisted-by: LLM
Signed-off-by: Myasnikov Daniil <[email protected]>
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
|
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
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.
What this PR does
Backport of #3647 to release-1.6.
Since #5080 (backport of #4710)
tenant.ciliumPoliciescallscozy-lib.bundle-name, and on release-1.6 that helper still doesindex .Values._cluster "bundle-name", which fails withindex of untyped nilwhen_clusteris not set. Tenant export-proxy suite (tests/networkpolicy_vm_exportproxy_test.yaml) sets no_cluster, so all four cases fail and unit tests are red on every PR against release-1.6, including batch #5089. #5080 set_cluster: {}only in its own suite, that's why it passed there.#3647 guards the map in every cozyconfig helper and adds cozy-lib-tests suite for injected and uninjected state. It applies cleanly, diff is identical to main.
Checked locally (helm-unittest is not installed here, CI runs it):
_cluster/_namespace(apart from generated passwords), only tenant chart with nothing injected changes, it doesn't fail innetworkpolicy.yamlanymore.Plain
helm templateof tenant chart without_clusterstill fails inkeycloakgroups.yaml, main fixed it separately in #3636. Unit tests are not affected, suite renders only templates it names.Screenshots
Not a UI change.
Downstream repositories
Release note