Skip to content

Commit fad959c

Browse files
IvanHuntersIvanHunters
committed
fix(apps/kubernetes): always emit KCP extraArgs/extraVolumeMounts + rm -f for BusyBox
Two real bugs caught by the dev3 e2e test — both would have shipped without live-cluster validation: - `rm --force` in the bootstrap and cleanup Job scripts is rejected by BusyBox `rm` (the alpine/k8s image uses BusyBox coreutils, not GNU). The bootstrap Job on dev3 died at the prune step with `rm: unrecognized option: force`. Switch to `rm -f` (works on both GNU and BusyBox). Also swap `grep --line-regexp --quiet --fixed-strings` for `grep -x -q -F` for the same reason. - Kamaji's `KamajiControlPlane` is a CRD, and Helm applies JSON-merge-patch to CRDs. That patch does not remove array-typed fields the new manifest simply omits — it leaves the live array untouched. Toggling `oidc.mode: System` → `None` therefore left the tenant kube-apiserver with a dangling `--authentication-config` flag pointing at a Secret the chart had already reaped, and the apiserver would refuse to start on the next kubelet restart. Fix: always render `extraArgs` and `extraVolumeMounts` on the KamajiControlPlane, emitting `[]` on the off-path so the patch REPLACES the stale array. Verified end-to-end on dev3. Adds two helm-unittest cases: the mode=None regression, and an adjustment of the baseline default-values test. Signed-off-by: Ivan Okhotnikov <[email protected]>
1 parent 2b46087 commit fad959c

4 files changed

Lines changed: 78 additions & 11 deletions

File tree

‎packages/apps/kubernetes/templates/cluster.yaml‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -309,16 +309,26 @@ metadata:
309309
spec:
310310
apiServer:
311311
resources: {{- include "cozy-lib.resources.defaultingSanitize" (list .Values.controlPlane.apiServer.resourcesPreset .Values.controlPlane.apiServer.resources $) | nindent 6 }}
312-
{{- if or .Values.controlPlane.apiServer.extraArgs $oidcEnabled }}
312+
{{- /*
313+
extraArgs and extraVolumeMounts are ALWAYS emitted (empty array when
314+
nothing to inject). Kamaji's KamajiControlPlane is a CRD and Helm
315+
applies JSON-merge-patch to CRDs, which for arrays REPLACES the whole
316+
value. If we omitted the field for `oidc.mode: None` after previously
317+
rendering it under `System`, JSON-merge-patch would leave the live
318+
array untouched — the tenant apiserver would keep the stale
319+
--authentication-config flag pointing at a Secret that no longer
320+
exists, and would refuse to start on the next kubelet restart.
321+
Rendering the empty array on the "off" path forces Helm to send a
322+
REPLACE patch that clears the stale value.
323+
*/}}
313324
extraArgs:
314325
{{- with .Values.controlPlane.apiServer.extraArgs }}
315326
{{- toYaml . | nindent 6 }}
316327
{{- end }}
317328
{{- if $oidcEnabled }}
318329
- --authentication-config=/etc/kubernetes/authentication-config/config.yaml
319330
{{- end }}
320-
{{- end }}
321-
{{- if or .Values.controlPlane.apiServer.extraVolumeMounts $oidcEnabled }}
331+
{{- if not (or .Values.controlPlane.apiServer.extraArgs $oidcEnabled) }} []{{ end }}
322332
extraVolumeMounts:
323333
{{- with .Values.controlPlane.apiServer.extraVolumeMounts }}
324334
{{- toYaml . | nindent 6 }}
@@ -328,7 +338,7 @@ spec:
328338
mountPath: /etc/kubernetes/authentication-config
329339
readOnly: true
330340
{{- end }}
331-
{{- end }}
341+
{{- if not (or .Values.controlPlane.apiServer.extraVolumeMounts $oidcEnabled) }} []{{ end }}
332342
controllerManager:
333343
resources: {{- include "cozy-lib.resources.defaultingSanitize" (list .Values.controlPlane.controllerManager.resourcesPreset .Values.controlPlane.controllerManager.resources $) | nindent 6 }}
334344
scheduler:

‎packages/apps/kubernetes/templates/oidc-rbac-job.yaml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -188,12 +188,12 @@ spec:
188188
--selector=app.kubernetes.io/managed-by=cozystack-oidc,app.kubernetes.io/instance=$RELEASE \
189189
--output=jsonpath='{.items[*].metadata.name}')
190190
for crb in $EXISTING; do
191-
if ! grep --line-regexp --quiet --fixed-strings "$crb" "$DESIRED_FILE"; then
191+
if ! grep -x -q -F "$crb" "$DESIRED_FILE"; then
192192
echo " removing $crb"
193193
kubectl --kubeconfig "$KUBECONFIG_TENANT" delete clusterrolebinding "$crb" --ignore-not-found
194194
fi
195195
done
196-
rm --force "$DESIRED_FILE"
196+
rm -f "$DESIRED_FILE"
197197
198198
if [ "$OIDC_MODE" = "System" ]; then
199199
echo "Patching the OIDC kubeconfig Secret on the management cluster..."
@@ -239,7 +239,7 @@ spec:
239239
"$RELEASE-oidc-kubeconfig" \
240240
--type=merge \
241241
--patch "$PATCH"
242-
rm --force "$KUBECONFIG_FILE"
242+
rm -f "$KUBECONFIG_FILE"
243243
fi
244244
fi
245245

‎packages/apps/kubernetes/tests/apiserver_authentication_test.yaml‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,19 +19,24 @@ tests:
1919
# default: opt-in fields render nothing #
2020
###############################################
2121

22-
- it: omits extraArgs and extraVolumeMounts on apiServer by default
22+
- it: emits empty extraArgs and extraVolumeMounts arrays by default
23+
# The chart ALWAYS emits both fields (empty arrays when nothing to inject)
24+
# so a mode=System → mode=None transition sends a JSON-merge-patch that
25+
# REPLACES the live array — otherwise the CRD keeps the stale flag.
2326
release:
2427
name: test-k8s
2528
namespace: tenant-test
2629
asserts:
2730
- isKind:
2831
of: KamajiControlPlane
2932
documentIndex: 1
30-
- notExists:
33+
- equal:
3134
path: spec.apiServer.extraArgs
35+
value: []
3236
documentIndex: 1
33-
- notExists:
37+
- equal:
3438
path: spec.apiServer.extraVolumeMounts
39+
value: []
3540
documentIndex: 1
3641

3742
- it: keeps only the two talos control-plane volumes by default

‎packages/apps/kubernetes/tests/oidc_test.yaml‎

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,17 @@ tests:
1414
###############################################
1515

1616
- it: emits no OIDC apiserver flag when oidc.mode is None
17+
# extraArgs is always rendered (see apiserver_authentication_test — the
18+
# explicit `[]` is required so a mode=System → mode=None upgrade REPLACES
19+
# the previously-set `--authentication-config` flag on the CRD).
1720
release:
1821
name: test
1922
namespace: tenant-test
2023
template: templates/cluster.yaml
2124
asserts:
22-
- notExists:
25+
- equal:
2326
path: spec.apiServer.extraArgs
27+
value: []
2428
documentIndex: 1
2529

2630
- it: emits no chart-managed OIDC volume on the KamajiControlPlane when oidc.mode is None
@@ -70,6 +74,54 @@ tests:
7074
- hasDocuments:
7175
count: 0
7276

77+
###############################################
78+
# existing customer upgrade — vestigial fields #
79+
###############################################
80+
81+
- it: mode=None emits empty extraArgs so a System→None downgrade replaces the live array
82+
# Guards against the Helm+CRD JSON-merge-patch pitfall — see cluster.yaml.
83+
# If a customer previously set oidc.mode=System, the KCP got
84+
# `--authentication-config` in extraArgs. Toggling mode=None must emit an
85+
# empty `extraArgs: []` so the patch REPLACES the stale value. Silently
86+
# omitting extraArgs would leave the live CRD unchanged and the tenant
87+
# apiserver would keep the dangling flag.
88+
release:
89+
name: test
90+
namespace: tenant-test
91+
template: templates/cluster.yaml
92+
set:
93+
oidc.mode: None
94+
asserts:
95+
- equal:
96+
path: spec.apiServer.extraArgs
97+
value: []
98+
documentIndex: 1
99+
- equal:
100+
path: spec.apiServer.extraVolumeMounts
101+
value: []
102+
documentIndex: 1
103+
# deployment.extraVolumes always has the two chart-owned Talos volumes;
104+
# the OIDC volume must NOT be there in mode=None.
105+
- lengthEqual:
106+
path: spec.deployment.extraVolumes
107+
count: 2
108+
documentIndex: 1
109+
110+
- it: unknown fields under oidc are silently ignored (existing customer upgrade path)
111+
release:
112+
name: test-legacy
113+
namespace: tenant-test
114+
template: templates/oidc-rbac-job.yaml
115+
set:
116+
# An existing CR carried over from an earlier OIDC API surface may still
117+
# have the old `oidc.enabled: bool` field. The Phase-1 chart only reads
118+
# `oidc.mode`; the vestigial field is silently ignored and no OIDC
119+
# objects render (mode defaults to None).
120+
oidc.enabled: true
121+
asserts:
122+
- hasDocuments:
123+
count: 0
124+
73125
###############################################
74126
# mode: System — happy path #
75127
###############################################

0 commit comments

Comments
 (0)