Skip to content

Commit bd3f3b9

Browse files
lexfreilexfrei
committed
fix(tenant): render the chart without the platform's injected values
`helm lint packages/apps/tenant` fails, and two of its three errors are the chart's own. `templates/keycloakgroups.yaml` reads `index .Values._cluster "oidc-enabled"`. `_cluster` is injected by the platform, so it is absent whenever the chart is rendered on its own, and `index` on a nil map is a hard error: `index of untyped nil`. The chart cannot be rendered or linted outside a cluster at all. The idiom the neighbouring charts use for the same lookup does not rescue this on its own, which is worth stating because it looks like it does. `(index .Values._cluster "x") | default "y"` still errors: `index` evaluates first and fails on the nil map, so `default` never sees a result to substitute; it only ever covered a key missing from a map that exists. The spelling alone does not decide whether a chart breaks, though. `packages/system/cozystack-basics` uses it and lints clean, because line 1 of its values.yaml is `_cluster: {}`, which makes the map non-nil before `index` runs. So a chart is broken only when it neither declares the empty map nor guards at the use site. Guarding at the use site is preferred here because it makes the template correct whatever values.yaml contains, which is what lets the chart render standalone; and `_cluster` is platform-internal, while this chart's values.yaml, schema and README are generated together from api/apps/v1alpha1/tenant/types.go as user-facing output. The guard has to sit on the map rather than on the value it returns, hence `index (.Values._cluster | default dict) "oidc-enabled"`. That is behaviour-preserving rather than merely quieter. With `_cluster` present, `| default dict` returns it untouched and the lookup is what it was. With it absent the lookup is nil, the `eq` below is false, and the block stays unrendered, which is what an unconfigured tenant should produce. Rendered both ways before and after, with `--api-versions v1.edp.epam.com/v1` so the capability-gated block actually executes, the output is identical byte for byte: four KeycloakRealmGroups when `oidc-enabled` is `"true"`, none when it is `"false"`. Rendering with no values at all goes from a hard error to a clean render with no groups. `Chart.yaml` also does not declare `cozy-lib`, which is present as a symlink under `charts/`, so lint reports missing chart metadata. Declare it the way `packages/apps/qdrant` does, that being the one chart in the tree that declares it and does not carry this error. Rendering is unchanged by the declaration, checked the same way. `tests/keycloakgroups_oidc_test.yaml` pins all three branches of the gate: four realm groups when `oidc-enabled` is `"true"`, none when it is `"false"`, and none when the platform injected no `_cluster` at all. That third case is the one the guard exists for, and it needs no mock, because `_cluster` is absent from values.yaml and so "not injected" is simply the chart's own default state. Against the unguarded template the suite fails with `index of untyped nil`; the chart's other seven suites pass identically either way, which is what left the change uncovered. The third error, `invalid icon URL '/logos/tenant.svg'`, is deliberately left. Lint wants a URL with a scheme, and all 31 charts in the tree that carry an icon use a relative path, none an absolute one. Changing this one would make it the only chart with a different convention, and choosing the URL a relative path should become is a decision about where those assets are served from, not about lint. Assisted-By: Claude <[email protected]> Signed-off-by: Aleksei Sviridkin <[email protected]>
1 parent 605030b commit bd3f3b9

3 files changed

Lines changed: 65 additions & 1 deletion

File tree

‎packages/apps/tenant/Chart.yaml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,7 @@ description: Separated tenant namespace
44
icon: /logos/tenant.svg
55
type: application
66
version: 0.0.0 # Placeholder, the actual version will be automatically set during the build process
7+
dependencies:
8+
- name: cozy-lib
9+
version: "*"
10+
repository: "file://charts/cozy-lib"

‎packages/apps/tenant/templates/keycloakgroups.yaml‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,11 @@
1-
{{- $oidcEnabled := index .Values._cluster "oidc-enabled" }}
1+
{{- /* _cluster is injected by the platform, so it is absent whenever the chart
2+
is rendered on its own, as `helm lint` and `helm template` do. `index`
3+
errors on a nil map before any `| default` on its result can apply, so
4+
the guard has to sit on the map rather than on the looked-up value. With
5+
_cluster present this yields exactly what indexing it directly yielded;
6+
with it absent the lookup is nil and the block below stays unrendered,
7+
which is what an unconfigured tenant should produce. */ -}}
8+
{{- $oidcEnabled := index (.Values._cluster | default dict) "oidc-enabled" }}
29
{{- if eq $oidcEnabled "true" }}
310
{{- if .Capabilities.APIVersions.Has "v1.edp.epam.com/v1" }}
411
apiVersion: v1.edp.epam.com/v1

‎packages/apps/tenant/tests/keycloakgroups_oidc_test.yaml‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
suite: tenant keycloak realm groups — the oidc gate survives an uninjected _cluster
2+
templates:
3+
- templates/keycloakgroups.yaml
4+
5+
# `_cluster` is injected by the platform, so it is simply absent whenever the
6+
# chart is rendered on its own: `helm lint`, `helm template`, and this suite.
7+
# The template reads it with `index`, and `index` on a nil map is a hard error
8+
# rather than an empty result, so before the guard the chart could not be
9+
# rendered outside a cluster at all.
10+
#
11+
# The guard has to sit on the map, not on the value: the spelling used
12+
# elsewhere in the tree, `(index .Values._cluster "x") | default "y"`, does not
13+
# help, because `index` is evaluated first and fails before `default` ever sees
14+
# a result to substitute.
15+
#
16+
# The third case below is the one that pins the fix. It carries no `set:` at
17+
# all, because `_cluster` is absent from values.yaml, so "the platform injected
18+
# nothing" needs no mocking — it is just the chart's own default state.
19+
20+
release:
21+
name: tenant-alice
22+
namespace: tenant-alice
23+
24+
capabilities:
25+
apiVersions:
26+
- v1.edp.epam.com/v1
27+
28+
tests:
29+
- it: renders the four realm groups when oidc is enabled
30+
set:
31+
_cluster:
32+
oidc-enabled: "true"
33+
asserts:
34+
- hasDocuments:
35+
count: 4
36+
37+
- it: renders nothing when oidc is explicitly disabled
38+
# The gate is an equality against the string "true", so anything else is
39+
# off. Pinned alongside the enabled case so a guard that accidentally
40+
# rendered unconditionally would be caught.
41+
set:
42+
_cluster:
43+
oidc-enabled: "false"
44+
asserts:
45+
- hasDocuments:
46+
count: 0
47+
48+
- it: renders nothing when the platform injected no _cluster at all
49+
# Fails with `index of untyped nil` without the guard. This is the case the
50+
# fix exists for, and the reason the chart can now be rendered standalone.
51+
asserts:
52+
- hasDocuments:
53+
count: 0

0 commit comments

Comments
 (0)