Skip to content

Commit ec22f5b

Browse files
kvapskvaps
committed
fix(computeplane): let Flux derive both release names
The aggregated API cannot represent spec.releaseName: the conversion rebuilds HelmRelease specs without it, so the first tenant edit through the API dropped the module's explicit computeplane-module release name, let it default to the object name computeplane, and collided with the Helm storage record the cluster release owned — with the tenant chart writing the old name back every interval. Drop releaseName from both HelmReleases and derive names from object names, like every other tenant module. Kamaji now writes the admin kubeconfig to computeplane-cluster-admin-kubeconfig; the computeplane-rd secrets.exclude entry follows in the same change, and a bats test pins the exclusion to the name the cluster release actually produces so a future rename cannot silently re-expose the credential. Also add the check-release-name guard the other extra modules carry, and pin the fluxcd addon's distribution artifact to the flux-operator version vendored in system/fluxcd-operator instead of :latest. Assisted-By: Claude <[email protected]> Signed-off-by: Andrei Kvapil <[email protected]>
1 parent 5cde79f commit ec22f5b

8 files changed

Lines changed: 123 additions & 34 deletions

File tree

‎hack/check-computeplane-kubeconfig-exclusion.bats‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
#!/usr/bin/env bats
2+
# The ComputePlane admin-kubeconfig Secret name derives from the cluster
3+
# HelmRelease object name (extra/computeplane/templates/cluster.yaml sets no
4+
# spec.releaseName), while computeplane-rd withholds that Secret from
5+
# tenant-visible secrets by literal name under secrets.exclude. Rename the
6+
# object without the exclusion following and the tenant regains the
7+
# cluster-admin credential the whole design exists to withhold. This test
8+
# pins the pair so a future rename fails here instead of silently re-exposing
9+
# the credential.
10+
11+
REPO_ROOT="$(cd "$(dirname "${BATS_TEST_FILENAME:-$0}")/.." && pwd)"
12+
13+
@test "computeplane-rd excludes the admin kubeconfig Secret the cluster release actually produces" {
14+
command -v helm >/dev/null || { echo "helm is required" >&2; exit 1; }
15+
command -v yq >/dev/null || { echo "yq (mikefarah v4+) is required" >&2; exit 1; }
16+
17+
rendered="$(helm template computeplane "$REPO_ROOT/packages/extra/computeplane")"
18+
19+
# Release name of the cluster HelmRelease: spec.releaseName when set, else
20+
# the object name (Flux's default with no targetNamespace).
21+
release="$(printf '%s' "$rendered" | yq eval-all 'select(.kind == "HelmRelease") | .spec.releaseName // .metadata.name' -)"
22+
[ -n "$release" ]
23+
[ "$release" != "null" ]
24+
25+
expected="${release}-admin-kubeconfig"
26+
excluded="$(yq '.spec.secrets.exclude[].resourceNames[]' "$REPO_ROOT/packages/system/computeplane-rd/cozyrds/computeplane.yaml")"
27+
28+
echo "expected exclusion: $expected"
29+
echo "declared exclusions: $excluded"
30+
printf '%s\n' "$excluded" | grep -qx "$expected"
31+
}

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,12 @@ metadata:
3838
apps.cozystack.io/application.group: apps.cozystack.io
3939
apps.cozystack.io/application.name: computeplane
4040
spec:
41-
# The module's own Helm release takes the -module suffix so the canonical
42-
# release name "computeplane" — and with it the Kamaji admin-kubeconfig
43-
# Secret "computeplane-admin-kubeconfig" consumed by placement: ComputePlane
44-
# applications — stays reserved for the cluster HelmRelease this module
45-
# renders (see packages/extra/computeplane/templates/cluster.yaml).
46-
releaseName: computeplane-module
41+
# No spec.releaseName — the aggregated API rebuilds HelmRelease specs
42+
# without it, so the release name must equal the object name (as in every
43+
# other tenant module). The cluster HelmRelease this module renders is the
44+
# separately-named computeplane-cluster, whose derived release name makes
45+
# Kamaji write the admin kubeconfig to computeplane-cluster-admin-kubeconfig
46+
# (see packages/extra/computeplane/templates/cluster.yaml).
4747
chartRef:
4848
kind: ExternalArtifact
4949
name: cozystack-computeplane-application-kubevirt-computeplane

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

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@ suite: tenant computeplane module toggle
33
# cluster for untrusted-code apps (design-proposals/compute-plane in
44
# cozystack/community). Contract pinned here:
55
# - off by default — no HelmRelease;
6-
# - the module's own Helm release takes the -module suffix so the canonical
7-
# release name "computeplane" (and the computeplane-admin-kubeconfig
8-
# Secret consumed by placement: ComputePlane apps) stays reserved for the
9-
# cluster HelmRelease the module chart renders.
6+
# - NO spec.releaseName on the module HelmRelease: the aggregated API
7+
# rebuilds HelmRelease specs without releaseName, so the release name
8+
# must always be the object name or the first tenant edit through the
9+
# API would rename the release out from under Helm.
1010
templates:
1111
- templates/computeplane.yaml
1212
release:
@@ -27,9 +27,8 @@ tests:
2727
- equal:
2828
path: metadata.name
2929
value: computeplane
30-
- equal:
30+
- notExists:
3131
path: spec.releaseName
32-
value: computeplane-module
3332
- equal:
3433
path: spec.chartRef.name
3534
value: cozystack-computeplane-application-kubevirt-computeplane

‎packages/extra/computeplane/templates/check-release-name.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{{- if ne .Release.Name .Chart.Name }}
2+
{{- fail (printf "The name of the release MUST BE %s" .Chart.Name) }}
3+
{{- end -}}

‎packages/extra/computeplane/templates/cluster.yaml‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,17 @@
88
packages/core/platform/sources/computeplane-application.yaml), so the module
99
is self-contained.
1010

11-
The Helm release is named "computeplane" (spec.releaseName below) so Kamaji
12-
writes the cluster's admin kubeconfig to the Secret
13-
`computeplane-admin-kubeconfig` (key super-admin.svc) — the fixed contract
14-
consumed by the HelmReleases of `placement: ComputePlane` applications. The
15-
HelmRelease object itself is named computeplane-cluster because the module's
16-
own HelmRelease already occupies the object name "computeplane" in the
17-
tenant namespace.
11+
The HelmRelease is named computeplane-cluster (the module's own HelmRelease
12+
occupies the object name "computeplane" in the tenant namespace) and sets no
13+
spec.releaseName: the aggregated API cannot represent releaseName — the
14+
conversion rebuilds HelmRelease specs without it — so both releases must let
15+
Flux derive the name from the object name, like every other tenant module.
16+
Kamaji therefore writes the cluster's admin kubeconfig to the Secret
17+
`computeplane-cluster-admin-kubeconfig` (key super-admin.svc) — the derived
18+
contract consumed by the HelmReleases of `placement: ComputePlane`
19+
applications, and the exact name computeplane-rd withholds from
20+
tenant-visible secrets. Renaming this object renames the Secret and breaks
21+
both; don't.
1822

1923
Everything security-relevant is fixed here and unreachable by the tenant:
2024
the tenant holds no admin kubeconfig for this cluster and the module exposes
@@ -53,7 +57,6 @@ metadata:
5357
sharding.fluxcd.io/key: tenants
5458
internal.cozystack.io/computeplane: "true"
5559
spec:
56-
releaseName: computeplane
5760
chartRef:
5861
kind: ExternalArtifact
5962
name: cozystack-computeplane-application-kubevirt-kubernetes
@@ -83,14 +86,16 @@ spec:
8386
# The kubernetes-app fluxcd addon (cozy-fluxcd chart) defaults the
8487
# FluxInstance spec.distribution.artifact to "", which fails CRD
8588
# validation ("must be of type string"). ComputePlane is the first real
86-
# consumer of the fluxcd addon, so set the canonical distribution
87-
# artifact here so the in-cluster Flux installs and can reconcile the
88-
# child HelmReleases that placement: ComputePlane apps render.
89+
# consumer of the fluxcd addon, so set the distribution artifact here
90+
# so the in-cluster Flux installs and can reconcile the child
91+
# HelmReleases that placement: ComputePlane apps render. Pinned to the
92+
# flux-operator version vendored in system/fluxcd-operator — bump the
93+
# two together.
8994
valuesOverride:
9095
flux-instance:
9196
instance:
9297
distribution:
93-
artifact: "oci://ghcr.io/controlplaneio-fluxcd/flux-operator-manifests:latest"
98+
artifact: "oci://ghcr.io/controlplaneio-fluxcd/flux-operator-manifests:v0.50.0"
9499
certManager:
95100
enabled: true
96101
ingressNginx:

‎packages/extra/computeplane/tests/cluster_test.yaml‎

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,27 +2,29 @@ suite: computeplane module renders the operator-owned cluster HelmRelease
22
# The module wraps the ordinary apps/kubernetes chart with operator-fixed
33
# values (design-proposals/compute-plane in cozystack/community). These pins
44
# are the contract:
5-
# - the Helm release is named "computeplane" so Kamaji writes the admin
6-
# kubeconfig to the Secret computeplane-admin-kubeconfig, which the
7-
# HelmReleases of placement: ComputePlane applications consume;
5+
# - the cluster HelmRelease is named computeplane-cluster and sets NO
6+
# spec.releaseName (the aggregated API cannot represent releaseName, so
7+
# both the module and cluster releases must derive from object names);
8+
# the derived release name makes Kamaji write the admin kubeconfig to
9+
# computeplane-cluster-admin-kubeconfig, which placement: ComputePlane
10+
# applications consume and computeplane-rd withholds from tenants;
811
# - the Cozystack-enablement addons (fluxcd/certManager/ingressNginx) are
912
# inline spec.values, so nothing injected via valuesFrom can disable them;
1013
# - the chart comes from this package's own source-only ExternalArtifact,
1114
# keeping the module self-contained.
1215
templates:
1316
- templates/cluster.yaml
1417
release:
15-
name: computeplane-module
18+
name: computeplane
1619
namespace: tenant-alice
1720
tests:
18-
- it: reserves the canonical release name for the cluster (kubeconfig contract)
21+
- it: derives the cluster release name from the object name (kubeconfig contract)
1922
asserts:
2023
- equal:
2124
path: metadata.name
2225
value: computeplane-cluster
23-
- equal:
26+
- notExists:
2427
path: spec.releaseName
25-
value: computeplane
2628
- equal:
2729
path: spec.chartRef.name
2830
value: cozystack-computeplane-application-kubevirt-kubernetes
@@ -111,3 +113,23 @@ tests:
111113
- equal:
112114
path: spec.values.nodeGroups.edge.roles[0]
113115
value: ingress-nginx
116+
117+
# The generator does not emit additionalProperties: false, so unlisted
118+
# NodeGroup fields are NOT rejected by the schema — they pass through to the
119+
# wrapped chart unvalidated. Pinned deliberately (and stated in the README);
120+
# if the generator ever learns to close the object, this test should flip to
121+
# asserting rejection.
122+
- it: passes unlisted nodeGroup fields through to the wrapped chart
123+
set:
124+
nodeGroups:
125+
md0:
126+
minReplicas: 1
127+
maxReplicas: 3
128+
instanceType: u1.large
129+
diskSize: 32Gi
130+
roles: ["ingress-nginx"]
131+
maxUnhealthy: 40%
132+
asserts:
133+
- equal:
134+
path: spec.values.nodeGroups.md0.maxUnhealthy
135+
value: 40%

‎packages/extra/computeplane/tests/release_name_test.yaml‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
suite: computeplane module refuses a non-canonical release name
2+
# The module release name must equal the chart name: the aggregated API
3+
# rebuilds HelmRelease specs without spec.releaseName, so the release name is
4+
# always the object name — and the object name is what the ComputePlane
5+
# ApplicationDefinition (release.prefix "") maps onto. A stray release name
6+
# (e.g. the historical computeplane-module) would collide with that mapping,
7+
# so the guard fails the render loudly.
8+
templates:
9+
- templates/check-release-name.yaml
10+
tests:
11+
- it: rejects a suffixed release name
12+
release:
13+
name: computeplane-module
14+
namespace: tenant-alice
15+
asserts:
16+
- failedTemplate:
17+
errorPattern: "The name of the release MUST BE computeplane"
18+
19+
- it: accepts the canonical release name
20+
release:
21+
name: computeplane
22+
namespace: tenant-alice
23+
asserts:
24+
- hasDocuments:
25+
count: 0

‎packages/system/computeplane-rd/cozyrds/computeplane.yaml‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ spec:
88
singular: computeplane
99
plural: computeplanes
1010
openAPISchema: |-
11-
{"title":"Chart Values","type":"object","properties":{"nodeGroups":{"description":"Worker node groups of the ComputePlane cluster (cluster shape only — the security posture is fixed by the module and cannot be overridden). Same structure as the `kubernetes` application's `nodeGroups`. When left empty, the wrapped `kubernetes` chart emits its default scale-from-zero `md0` group with `roles: [ingress-nginx]`, so the cluster sits idle until a pending Pod (e.g. the ingress-nginx controller) triggers the cluster-autoscaler. Supplied groups are authoritative (not merged with the default), so at least one of them must carry `roles: [ingress-nginx]` — enforced at render time.","type":"object","default":{},"additionalProperties":{"type":"object","required":["diskSize","instanceType","maxReplicas","minReplicas"],"properties":{"diskSize":{"description":"System disk size for the worker VM. Carries the Talos OS image, kubelet state, containerd image cache, and any local-path PVCs.","default":"20Gi","pattern":"^(\\+|-)?(([0-9]+(\\.[0-9]*)?)|(\\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\\+|-)?(([0-9]+(\\.[0-9]*)?)|(\\.[0-9]+))))?$","anyOf":[{"type":"integer"},{"type":"string"}],"x-kubernetes-int-or-string":true},"gpus":{"description":"List of GPUs to attach (NVIDIA driver requires at least 4 GiB RAM).","type":"array","items":{"type":"object","required":["name"],"properties":{"name":{"description":"Name of GPU, such as \"nvidia.com/AD102GL_L40S\".","type":"string","x-cozystack-options":{"source":"gpu"}}}}},"instanceType":{"description":"Virtual machine instance type.","type":"string","default":"u1.medium","x-cozystack-options":{"source":"instancetype"}},"kubelet":{"description":"Kubelet resource reservations for this node group.","type":"object","properties":{"evictionHardMemory":{"description":"Hard eviction threshold for memory (absolute like 200Mi or percentage like 7%).","type":"string","default":"7%"},"evictionSoftMemory":{"description":"Soft eviction threshold for memory (absolute like 1Gi or percentage like 10%).","type":"string","default":"10%"},"kubeReservedCpu":{"description":"CPU reserved for kubelet and container runtime. Auto-computed from instanceType if empty.","type":"string"},"kubeReservedMemory":{"description":"Memory reserved for kubelet and container runtime. Auto-computed from instanceType if empty.","type":"string"},"systemReservedCpu":{"description":"CPU reserved for host OS. Auto-computed from instanceType if empty.","type":"string"},"systemReservedMemory":{"description":"Memory reserved for host OS. Auto-computed from instanceType if empty.","type":"string"}}},"maxReplicas":{"description":"Maximum number of replicas.","type":"integer","default":10},"minReplicas":{"description":"Minimum number of replicas.","type":"integer","default":0},"resources":{"description":"Explicit CPU and memory for each worker node, as an alternative to `instanceType` sizing. Set both `cpu` and `memory` together or neither.","type":"object","properties":{"cpu":{"description":"CPU available.","pattern":"^(\\+|-)?(([0-9]+(\\.[0-9]*)?)|(\\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\\+|-)?(([0-9]+(\\.[0-9]*)?)|(\\.[0-9]+))))?$","anyOf":[{"type":"integer"},{"type":"string"}],"x-kubernetes-int-or-string":true},"memory":{"description":"Memory (RAM) available.","pattern":"^(\\+|-)?(([0-9]+(\\.[0-9]*)?)|(\\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\\+|-)?(([0-9]+(\\.[0-9]*)?)|(\\.[0-9]+))))?$","anyOf":[{"type":"integer"},{"type":"string"}],"x-kubernetes-int-or-string":true}}},"roles":{"description":"List of node roles. At least one group must carry `ingress-nginx` — the module pins the ingress-nginx addon on, and its controller only schedules onto nodes with that role (enforced at render time).","type":"array","items":{"type":"string"}},"storageClass":{"description":"StorageClass for worker node persistent disks. When empty, falls back to the application-level storageClass of the wrapped kubernetes app.","type":"string","x-cozystack-options":{"source":"storageclass"}}}}}}}
11+
{"title":"Chart Values","type":"object","properties":{"nodeGroups":{"description":"Worker node groups of the ComputePlane cluster (cluster shape only — the security posture is fixed by the module and cannot be overridden through this knob; note that unlisted `NodeGroup` fields pass through to the wrapped chart unvalidated). Same structure as the `kubernetes` application's `nodeGroups`. When left empty, the wrapped `kubernetes` chart emits its default scale-from-zero `md0` group with `roles: [ingress-nginx]`, so the cluster sits idle until a pending Pod (e.g. the ingress-nginx controller) triggers the cluster-autoscaler. Supplied groups are authoritative (not merged with the default), so at least one of them must carry `roles: [ingress-nginx]` — enforced at render time.","type":"object","default":{},"additionalProperties":{"type":"object","required":["diskSize","instanceType","maxReplicas","minReplicas"],"properties":{"diskSize":{"description":"System disk size for the worker VM. Carries the Talos OS image, kubelet state, containerd image cache, and any local-path PVCs.","default":"20Gi","pattern":"^(\\+|-)?(([0-9]+(\\.[0-9]*)?)|(\\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\\+|-)?(([0-9]+(\\.[0-9]*)?)|(\\.[0-9]+))))?$","anyOf":[{"type":"integer"},{"type":"string"}],"x-kubernetes-int-or-string":true},"gpus":{"description":"List of GPUs to attach (NVIDIA driver requires at least 4 GiB RAM).","type":"array","items":{"type":"object","required":["name"],"properties":{"name":{"description":"Name of GPU, such as \"nvidia.com/AD102GL_L40S\".","type":"string","x-cozystack-options":{"source":"gpu"}}}}},"instanceType":{"description":"Virtual machine instance type.","type":"string","default":"u1.medium","x-cozystack-options":{"source":"instancetype"}},"kubelet":{"description":"Kubelet resource reservations for this node group.","type":"object","properties":{"evictionHardMemory":{"description":"Hard eviction threshold for memory (absolute like 200Mi or percentage like 7%).","type":"string","default":"7%"},"evictionSoftMemory":{"description":"Soft eviction threshold for memory (absolute like 1Gi or percentage like 10%).","type":"string","default":"10%"},"kubeReservedCpu":{"description":"CPU reserved for kubelet and container runtime. Auto-computed from instanceType if empty.","type":"string"},"kubeReservedMemory":{"description":"Memory reserved for kubelet and container runtime. Auto-computed from instanceType if empty.","type":"string"},"systemReservedCpu":{"description":"CPU reserved for host OS. Auto-computed from instanceType if empty.","type":"string"},"systemReservedMemory":{"description":"Memory reserved for host OS. Auto-computed from instanceType if empty.","type":"string"}}},"maxReplicas":{"description":"Maximum number of replicas.","type":"integer","default":10},"minReplicas":{"description":"Minimum number of replicas.","type":"integer","default":0},"resources":{"description":"Explicit CPU and memory for each worker node, as an alternative to `instanceType` sizing. Set both `cpu` and `memory` together or neither.","type":"object","properties":{"cpu":{"description":"CPU available.","pattern":"^(\\+|-)?(([0-9]+(\\.[0-9]*)?)|(\\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\\+|-)?(([0-9]+(\\.[0-9]*)?)|(\\.[0-9]+))))?$","anyOf":[{"type":"integer"},{"type":"string"}],"x-kubernetes-int-or-string":true},"memory":{"description":"Memory (RAM) available.","pattern":"^(\\+|-)?(([0-9]+(\\.[0-9]*)?)|(\\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\\+|-)?(([0-9]+(\\.[0-9]*)?)|(\\.[0-9]+))))?$","anyOf":[{"type":"integer"},{"type":"string"}],"x-kubernetes-int-or-string":true}}},"roles":{"description":"List of node roles. At least one group must carry `ingress-nginx` — the module pins the ingress-nginx addon on, and its controller only schedules onto nodes with that role (enforced at render time).","type":"array","items":{"type":"string"}},"storageClass":{"description":"StorageClass for worker node persistent disks. When empty, falls back to the application-level storageClass of the wrapped kubernetes app.","type":"string","x-cozystack-options":{"source":"storageclass"}}}}}}}
1212
release:
1313
prefix: ""
1414
labels:
@@ -31,9 +31,13 @@ spec:
3131
exclude:
3232
# The cluster admin kubeconfig is the operator-retained credential of the
3333
# ComputePlane: the tenant must not receive it (the hardening is
34-
# tamper-proof precisely because the tenant cannot get admin).
34+
# tamper-proof precisely because the tenant cannot get admin). The name
35+
# derives from the cluster HelmRelease object name (computeplane-cluster,
36+
# see extra/computeplane/templates/cluster.yaml) — if that object is ever
37+
# renamed, this entry must follow or the credential becomes visible;
38+
# hack/check-computeplane-kubeconfig-exclusion.bats pins the pair.
3539
- resourceNames:
36-
- computeplane-admin-kubeconfig
40+
- computeplane-cluster-admin-kubeconfig
3741
include: []
3842
services:
3943
exclude: []

0 commit comments

Comments
 (0)