feat(computeplane): provision a ComputePlane tenant module for untrusted-code apps - #3280

Blocking on process first: this cannot merge until cozystack/community#33 is approved and merged. The proposal is approved but still open, and this PR implements its surface directly — the module contract (release name, kubeconfig secret, tenant-visible knobs) is exactly what the proposal fixes. Merging the implementation while the design can still move means any late revision lands as a breaking change to an already-shipped module kind. Please land #33 first, then rebase this.

The module itself is in good shape. Wrapping apps/kubernetes with operator-fixed inline spec.values is the right lever — Flux merges spec.values after valuesFrom, so the addon pins genuinely cannot be disabled through cozystack-values. Reserving the canonical release name for the cluster HelmRelease so Kamaji writes computeplane-admin-kubeconfig, and excluding that Secret from tenant-visible secrets, is a clean way to hold the credential. The source-only re-declaration of apps/kubernetes is safe as built: the package reconciler skips components without an install: block, so the duplicate component yields an artifact and never a colliding HelmRelease. And the cozystack-engine edge is correct — it really does gate the computeplane-rd release that would otherwise fail to render against a missing ApplicationDefinition CRD.

Findings below, ranked.

1. packages/apps/kubernetes/templates/ingress.yaml — the Proxied Service now renders on every cluster, including those with no ingress-nginx

The guard went from and (eq exposeMethod "Proxied") .hosts to if eq exposeMethod "Proxied", with a new end closing the hosts condition above the Service. That's the intended change for ComputePlane, but it drops the only thing that was standing in for "the operator actually wants ingress here": exposeMethod defaults to Proxied while addons.ingressNginx.enabled defaults to false.

Rendering the chart against its own tests/values-ci.yaml (which configures no ingressNginx at all) emits nothing before this change and emits <release>-ingress-nginx after it. So on upgrade, every existing kind: Kubernetes application gains a Service in its tenant namespace whose selector (node-role.kubernetes.io/ingress-nginx: "") matches no pods. It is not invisible either — packages/system/kubernetes-rd/cozyrds/kubernetes.yaml lists kubernetes-{{ .name }}-ingress-nginx under services.include, so it surfaces in the tenant dashboard advertising ports 80/443 that route nowhere.

Suggested fix — gate on the addon rather than on hosts, which still satisfies ComputePlane since it sets enabled: true explicitly:

{{- if and .Values.addons.ingressNginx.enabled (eq .Values.addons.ingressNginx.exposeMethod "Proxied") }}

2. packages/core/platform/templates/bundles/iaas.yaml — the toggle ships in system, the backing package ships in iaas

Every other tenant module toggle (etcd, monitoring, ingress, gateway, seaweedfs) is backed by a package in the system bundle, so the toggle always resolves. ComputePlane is the first whose package lives in iaas, while the computeplane field itself arrives via tenant-rd in the system bundle. Bundles are independent booleans with no cross-validation.

Failure: on a system + paas install, a tenant sets computeplane: true and gets a HelmRelease pointing at a cozystack-computeplane-application-kubevirt-computeplane artifact that does not exist. With install.remediation.retries: -1 it retries forever, with no diagnostic and no dashboard entry (the ComputePlane kind is not registered either, since computeplane-rd is in the same absent package). Either move the package to the system bundle, or have the tenant chart fail loudly when the module is toggled on without its package.

3. packages/extra/computeplane/values.schema.json — nodeGroups is unvalidated, and can defeat the pinned ingress addon

The module declares nodeGroups as a bare type: object with x-kubernetes-preserve-unknown-fields: true. The wrapped kubernetes app validates the same knob against a typed schema (diskSize, gpus, instanceType, kubelet, maxReplicas, minReplicas, resources, roles, storageClass) with per-field enums and patterns. Two consequences:

• A bad instanceType or a misspelled field draws no admission error. It lands in the cluster HelmRelease and fails deep in the wrapped chart's render, surfacing as a stuck release rather than a rejected write.
• More importantly: user-supplied nodeGroups are authoritative and are not merged with the default md0 (per the kubernetes.nodeGroups helper), and the ingress-nginx controller carries nodeSelector: node-role.kubernetes.io/ingress-nginx: "". A tenant who supplies any nodeGroups without roles: [ingress-nginx] leaves the controller Pending indefinitely and the ComputePlane's ingress never comes up.

The README states "the security posture is fixed by the module and cannot be overridden", but ingress availability is in practice reachable through the one knob the module does expose. Reusing the kubernetes app's nodeGroups schema addresses the validation half; the roles requirement wants either defaulting in the module or an explicit warning in the parameter docs.

4. packages/core/platform/sources/computeplane-application.yaml — the comment overstates what the kubernetes-application edge does

The comment explains the edge as needed "because the wrapped apps/kubernetes chart renders child HelmReleases whose chartRefs point at that package's addon artifacts". Variant dependsOn only gates HelmRelease creation for components carrying an install: block — here that is computeplane-rd alone. The cluster HelmRelease is rendered by the tenant chart and is not ordered by this edge, and the addon artifacts it references are produced unconditionally by the PackageSource reconciler regardless of dependency readiness. The edge is harmless and the artifact naming is right; the stated reason just isn't the operative one.

Checked and fine

• Re-running hack/update-crd.sh in packages/extra/computeplane reproduces the committed ApplicationDefinition byte-for-byte, so the generated RD is in sync with the generator.
• The releaseName: computeplane-module / object-name computeplane split does not break application lookup, which keys off the HelmRelease object name rather than spec.releaseName.
• The victoria-metrics-operator gate that other tenant modules carry is correctly absent — apps/kubernetes renders no VM* resources on the management side.
• All touched helm-unittest suites pass.

Not blocking

No e2e coverage is added. Standing up a full Kamaji + KubeVirt cluster in CI is a real cost and the unit suites do pin the module contract meaningfully, so this reads as a reasonable follow-up rather than a merge condition — but it is worth an issue so it does not get lost behind the placement work.

Code Review

This pull request introduces the 'ComputePlane' feature, a single-tenant, Cozystack-managed cluster for running untrusted-code applications. It adds the computeplane configuration option to the tenant API, creates the computeplane extra package and its resource definitions, wires it into the IaaS bundle, and updates the ingress configuration in the kubernetes application to support proxied routing. The review feedback suggests improving Helm template safety in ingress.yaml by using a nil-safe field walking pattern to prevent potential nil pointer dereference panics, and pinning the Flux operator manifests artifact in cluster.yaml to a specific stable version instead of using the :latest tag to ensure reproducible deployments.

All four findings from the previous round are addressed on their merits, with tests, and community#33 has merged — the process block is cleared. The addon gate on the Proxied Service is exactly right, the corrected dependsOn comment now describes what variant dependencies actually do, and the nodeGroups typedefs are a real improvement. Requesting changes on one blocking issue plus a few smaller items.

Blocking: the release-name divergence is unrepresentable in the aggregated API

packages/apps/tenant/templates/computeplane.yaml sets spec.releaseName: computeplane-module, and packages/extra/computeplane/templates/cluster.yaml pins spec.releaseName: computeplane on the inner cluster HelmRelease. That split cannot survive a write through the aggregated API.

computeplane-rd declares release.prefix: "", so a ComputePlane object named computeplane maps onto the same HelmRelease the tenant chart renders. ApplicationDefinitionRelease has exactly three fields — ChartRef, Labels, Prefix — with no way to express a release name, and convertApplicationToHelmRelease never assigns Spec.ReleaseName. REST.Update replaces the live spec with that rebuilt one, carrying over only labels and the shard key, and Flux does not pin releaseName, so the field is silently dropped.

The first edit through kubectl or the dashboard therefore blanks releaseName, it defaults to the object name computeplane, and that is the exact Helm release computeplane-cluster owns — two HelmReleases driving one Helm storage record with two different charts. The tenant chart then reconciles at its 5m interval and writes computeplane-module back, so the name oscillates. Blast radius is the Kamaji control plane, the worker MachineDeployments and the admin kubeconfig Secret.

This is on the main path rather than a corner: nodeGroups is the module's only tenant-facing knob, and the aggregated API is the only way to set it, since the tenant chart's module HelmRelease passes valuesFrom and no values: at all.

Why this module and not the others

Worth spelling out, because the fix depends on it. Every other tenant module that renders a nested HelmRelease derives the inner name from the release and lets the inner release name default to the object name: external-dns and monitoring use {{ .Release.Name }}-system, seaweedfs uses {{ .Release.Name }}-db. None of them sets spec.releaseName, and computeplane.yaml is the only template under packages/apps/tenant/templates/ that sets it at all.

computeplane inverts the pattern: it hardcodes the inner object name and claims the canonical release name for the inner release, which pushes the outer module release off onto a suffixed one. It does that for a real reason — Kamaji derives the computeplane-admin-kubeconfig Secret from the Helm release name, so the inner release name is load-bearing for an external contract. No other module has a downstream consumer that reads its inner release name, which is why suffixing the inner one is free for all of them and not for this one.

So the general shape the aggregated API can express is exactly one release name per object, prefix + name. Any module whose outer release name diverges from that is unrepresentable — and on closer look the divergence buys nothing, because the inner release name is only load-bearing in the sense that something downstream has to know it, not that it has to be any particular string.

Fix: leave both releaseName fields blank, as every other module does

Drop spec.releaseName from both HelmReleases and let Flux derive them. Flux defaults releaseName to [TargetNamespace-]Name, and neither HelmRelease sets targetNamespace, so the outer module release becomes computeplane from its object name and the inner cluster release becomes computeplane-cluster from its own. Nothing collides, nothing diverges, and the aggregated API round-trips cleanly because prefix + name is then exactly what both already are — an Update that rebuilds the spec without releaseName reproduces the same release name rather than changing it.

This also puts the module back on the same pattern as external-dns, monitoring and seaweedfs, which is worth something on its own.

The consequence is that Kamaji writes the admin kubeconfig to computeplane-cluster-admin-kubeconfig instead of computeplane-admin-kubeconfig, which is fine — nothing consumes it yet, since placement: ComputePlane is a deferred follow-up — but it must be changed everywhere in the same commit.

The security-critical one is packages/system/computeplane-rd/cozyrds/computeplane.yaml, whose secrets.exclude withholds the credential by literal name under resourceNames. Rename the release without updating that entry and the exclusion silently stops matching, so the cluster admin kubeconfig becomes visible to the tenant — precisely the credential the whole design exists to withhold. The remaining references are prose and test comments: packages/extra/computeplane/templates/cluster.yaml, packages/extra/computeplane/README.md, packages/apps/tenant/templates/computeplane.yaml, packages/extra/computeplane/tests/cluster_test.yaml and packages/apps/tenant/tests/computeplane_module_test.yaml.

Since the merged proposal names computeplane-admin-kubeconfig when describing the deferred placement injection, that line wants a follow-up amendment so the future kubeConfig.secretRef is written against the derived name.

This wants two tests: a helm-unittest case asserting the tenant module HelmRelease carries no spec.releaseName, and one asserting the excluded secret name in computeplane-rd matches the name the inner release actually produces — the second is the one that would catch a future rename re-exposing the credential.

Add the release-name guard, and file the gap for the rest

packages/extra/computeplane has no check-release-name.yaml. Four of the nine extra charts ship one — bootbox, etcd, ingress, monitoring — so it is an established pattern rather than a new demand, and it is cheap here. It also fails closed on the issue above: a dropped releaseName yields computeplane, which the guard rejects loudly instead of silently colliding two releases.

The other four charts without a guard (external-dns, gateway, info, seaweedfs) have the same latent exposure and are out of scope for this PR — worth a tracking issue rather than scope creep here. Note that community#39 proposes cardinality as a declarative capability on ApplicationDefinition, which would replace all of these hand-rolled per-chart guards; the issue should reference it so the guards are understood as a stopgap.

Close the nodeGroup object in the schema

The typedefs landed, but values.schema.json still has no additionalProperties: false, so unlisted nodeGroup keys pass through to the wrapped chart unvalidated — maxUnhealthy and nodeStartupTimeout among them. For a module whose security argument is that the chart is the profile, the README's "only the cluster shape the module exposes" is not yet true.

Close it through the generator by adjusting the NodeGroup typedef in values.yaml and re-running make -C packages/extra/computeplane generate, rather than hand-editing the schema or the embedded openAPISchema. If the generator cannot emit additionalProperties: false, drop the claim from the README and values.yaml and state plainly that unlisted fields pass through. A helm-unittest case setting an unlisted nodeGroup field and asserting rejection pins whichever way it goes.

Two notes, not change requests

The etcd dependency is a precedent worth recording, not a defect. A ComputePlane with no tenant etcd gets no cluster-autoscaler, so the default nodeGroups: {} scale-from-zero md0 never receives its trigger and the cluster provisions no workers. That is not specific to the pinned addons — none of the tenant Kubernetes machinery works without etcd, and that is the intended design. What is new is that this makes one tenant module depend on another, which the module system has no way to express or surface today. A tenant enabling computeplane without etcd gets silence rather than a diagnostic. Worth deciding deliberately whether to document the prerequisite in the module README and the tenant README, or to surface it, and worth capturing the module-depends-on-module precedent somewhere durable — plausibly as input to community#39, which is already reshaping how modules declare themselves.

The package guard's blast radius is a machinery-wide deficiency. Helm's fail aborts the whole chart, so a tenant flipping computeplane: true on an install without the iaas bundle freezes reconciliation of everything else the tenant chart owns — namespace labels, quotas, network policy, ingress, monitoring — behind remediation.retries: -1. The guard itself is correct and the mechanism checks out; the problem is that a tenant-settable boolean can wedge the tenant's entire configuration surface, and that is true of any fail in the tenant chart rather than anything this PR introduced. community#39's move away from the special-cased extra bucket is the right place to fix the class. No change requested here beyond awareness.

Smaller

packages/apps/kubernetes/tests/ingress_proxied_gate_test.yaml covers enabled/disabled and hosts/no-hosts, but not enabled: false with exposeMethod: Proxied and a non-empty hosts list. That combination used to render an Ingress and now renders nothing. It is almost certainly the intended reading of "the addon is off", but it is a real behaviour change for any existing cluster in that state — worth a case pinning it deliberately so it reads as a decision.

packages/system/computeplane-rd/values.yaml contains a literal {}; the sibling seaweedfs-rd ships no values.yaml at all. Drop it for consistency.

The release-name fix is right, and deriving both names from object names is the change that makes this representable through the aggregated API. check-release-name, the pinned flux-operator version instead of :latest, and the two pushbacks — the -rd values.yaml convention and the generator's inability to emit additionalProperties: false — are all correct; thanks for checking those rather than just applying them.

Most of what remains is drift between what the code does and what its comments, docs and PR body say. One item is functional, and one is a correction to something the previous round asked for.

Retracting the secrets.exclude framing — the previous round had the mechanism wrong

The last review called the computeplane-admin-kubeconfig entry security-critical and asked for a test pinning it to the derived release name. That was wrong, and the resulting hack/check-computeplane-kubeconfig-exclusion.bats should come back out.

matchResourceToExcludeInclude in internal/lineagecontrollerwebhook/matcher.go returns the value of matchResourceToSelectorArray(..., resources.Include), and that function returns false on an empty slice. With include: [], no secret in a ComputePlane's lineage is ever labelled internal.cozystack.io/tenantresource: "true", and pkg/registry/core/tenantsecret/rest.go surfaces only labelled secrets. It is an allowlist, not a denylist — the exclude branch never becomes load-bearing, so renaming the release could not have exposed the credential.

The clearest evidence is in this PR already: packages/apps/kubernetes/templates/helmreleases/fluxcd.yaml consumes two Kamaji credentials, {{ .Release.Name }}-admin-kubeconfig and {{ .Release.Name }}-kubeconfig. The exclude list names only the first. If that list were the guard the comments claim, the second cluster-admin credential would already be exposed — it is not, because the empty include list withholds everything.

So the withholding is rename-proof by construction, which is a stronger property than the one the previous round asked to pin. The suggested resolution is to drop the resourceNames entry and keep exclude: [] / include: [] as seaweedfs-rd does, rewrite the three comment blocks that assert the rename coupling — in computeplane-rd/cozyrds/computeplane.yaml, extra/computeplane/templates/cluster.yaml and extra/computeplane/README.md — to state the real mechanism, and delete the bats file. Keeping the literal entry as defence-in-depth is also defensible, but then it should say so and should list both secrets, so it is not mistaken for the mechanism again.

No replacement static test. The property is "a tenant-scoped client cannot read the ComputePlane's kubeconfig Secrets", which lives between the lineage webhook, the aggregated API and RBAC at runtime; a file-comparison test cannot observe it and gives false assurance about the most important security claim in the PR. That belongs in the ComputePlane e2e already filed.

The Proxied Service escapes the etcd gate

packages/apps/kubernetes/templates/ingress.yaml gates the Service on and .Values.addons.ingressNginx.enabled (eq ... "Proxied"), but the ingress-nginx addon HelmRelease that backs it is gated on and .Values.addons.ingressNginx.enabled .Values._namespace.etcd. ComputePlane pins the addon on, so on an etcd-less tenant the chart emits computeplane-cluster-ingress-nginx with no controller behind it — confirmed by rendering packages/apps/kubernetes against tests/values-ci-no-etcd.yaml with the addon enabled and exposeMethod: Proxied.

That is the same selector-less-Service outcome the addon gate was added to prevent, reached by a different route, and it contradicts the comment now sitting above the Service. It also breaks the etcd-less contract the rest of the chart maintains, where the render collapses to the awaiting-etcd beacon and nothing else.

Adding .Values._namespace.etcd to the Service condition fixes it. A case in ingress_proxied_gate_test.yaml with the addon enabled and _namespace.etcd empty, asserting nothing renders, pins it — that one is a genuine render-behaviour assertion rather than a drift guard, so it is worth having.

The etcd prerequisite: two details wrong in the README

Both are worth correcting whether or not the guard becomes a hard fail.

"the tenant must have the etcd module enabled" is stricter than reality — packages/apps/tenant/templates/namespace.yaml inherits _namespace.etcd from the parent tenant, so an ancestor's etcd satisfies the wrapped chart, and a tenant following this literally provisions a redundant one. "gives no diagnostic" is also not quite right: packages/apps/kubernetes/templates/cluster.yaml emits the awaiting-etcd ConfigMap as a status beacon. It is a weak diagnostic — both HelmReleases still report Ready over an empty shell — but it exists and should be named.

Whether to escalate this from documentation to a render-time fail alongside the package guard is your call, and there is a reasonable argument either way now that the beacon is in the picture. If it stays documentation, the two corrections above are enough.

placement: ComputePlane is advertised but not shipped

The Tenant field description in api/apps/v1alpha1/tenant/types.go says "Applications with placement: ComputePlane are routed onto it." That field does not exist anywhere in Go, charts or CRDs — it is the deferred follow-up. The text is baked into the Tenant openAPISchema in tenant-rd, so it renders in the dashboard, and it is repeated in packages/apps/tenant/values.yaml, values.schema.json and README.md.

An operator enabling computeplane: true today therefore provisions a Kamaji control plane and KubeVirt workers that nothing can be scheduled onto, with the field description implying otherwise. Reword to state what ships now and note the routing is not yet available, then regenerate through make generate so all four copies follow rather than hand-editing them.

The larger question is whether the toggle should ship before its consumer exists at all. Gating it until the placement work lands would avoid advertising an inert capability, and is worth a deliberate decision rather than defaulting to shipping.

Drift

packages/extra/computeplane/templates/cluster.yaml still names the Service computeplane-ingress-nginx; after the release rename it renders as computeplane-cluster-ingress-nginx. The per-app management Ingress follow-up will be written against whatever this comment says, so it is worth fixing now.

The PR body still describes the pre-rename scheme — module release taking the -module suffix, canonical computeplane reserved for the cluster, Secret computeplane-admin-kubeconfig. The shipped code is the inverse. Since the release note is drafted from this text, it should be updated before merge.

None of the items in this section need tests; the codegen check already covers the generated copies.

Recommended, not blocking

The ingress-role guard requires some group to carry roles: [ingress-nginx] but not that the group can ever run a pod — a matching group with maxReplicas: 0 reproduces the Pending-forever failure the guard exists to prevent. Requiring maxReplicas >= 1 on the matching group would close it.

internal.cozystack.io/computeplane: "true" on the cluster HelmRelease has no reader anywhere in the tree. If it is a hook for the placement follow-up, a comment saying so would stop the next person removing it as dead.

The package guard fails closed on a nil lookup, where the comparable guard in the platform chart fails only when the lookup returns something. The Capabilities gate covers offline helm template, but a client-side render against a live cluster's discovery would trip it and fail the whole tenant render. Worth confirming no such path exists in the packaging pipeline.

Finally, worth confirming against the design that module self-provisioning is acceptable: with release.prefix: "", a tenant can create a ComputePlane object named computeplane through the aggregated API in a tenant where the operator left the toggle off, and check-release-name blocks every name except that one. This is true of every dashboard.module: true RD and is not introduced here — but ComputePlane is the first module where the unauthorised object is a VM-backed Kubernetes cluster, which changes the cost of the gap.

Actionable comments posted: 1

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/apps/tenant/README.md`:
- Line 85: The computeplane README entry contradicts itself about tenant
kubeconfig access. Update the row to state that
computeplane-cluster-admin-kubeconfig is platform-internal and unavailable to
tenant administrators, or remove the external-catalog workaround if the include
allowlist prevents its use; keep the no-admin-kubeconfig tenant behavior
explicit.

LGTM. Round three is closed and I verified the fixes rather than reading the summary — rendering apps/kubernetes against the no-etcd values with the addon enabled and exposeMethod: Proxied now produces nothing, which was the one functional bug left; the secrets framing is corrected to the allowlist mechanism with the bats cross-check removed; and re-running the generators for both computeplane and tenant produces zero drift, so the reworded toggle description is genuinely generated across all four copies. Suites green at 9 / 26 / 90 / 186.

Taking exclude: [] / include: [] over defence-in-depth was the right call — a literal name list would have invited the same misreading a second time, and the empty include list withholds by construction regardless of naming.

Two notes, neither blocking and neither needing a change here.

The etcd prerequisite staying documentation rather than a render-time fail is a reasonable call, and the rewritten README is accurate now that ancestor etcd counts and the awaiting-etcd beacon is named. The stated reason does not hold, though: packages/apps/tenant/templates/namespace.yaml computes the effective etcd as $parentNamespace.etcd overridden by .Values.etcd, so enabling etcd and computeplane in a single edit would satisfy a guard in that same render — the enable-both-in-one-edit flow would not have broken. The decision stands on the beacon plus the docs being proportionate; it just should not be cited later as a precedent for skipping a guard that would have worked.

On the maxReplicas >= 1 guard: I misread the template on first pass and thought the fail sat inside the range, which would have made it order-dependent across node groups. It does not — the range closes before the check. Flagging only so the observation is not mistaken for an unreported concern.

Shipping the toggle ahead of in-tree placement routing is fine given cozyllm targets the kubeconfig Secret directly; the capability is not inert, which was the actual worry. Module self-provisioning belonging with community#39's protection capability rather than a special case here is the right home for it.