Repository navigation
docs(compute-plane): derive release names — the kubeconfig Secret is computeplane-cluster-admin-kubeconfig - #40
Andrei Kvapil (kvaps) wants to merge 2 commits into
Conversation
…computeplane-cluster-admin-kubeconfig Follow-up from the cozystack/cozystack#3280 review: the aggregated API rebuilds HelmRelease specs without spec.releaseName, so neither the module nor the cluster HelmRelease may set it — release names derive from object names. The stable external contract is therefore the cluster HelmRelease object name computeplane-cluster, and the Kamaji admin-kubeconfig Secret the placement: ComputePlane injection consumes is computeplane-cluster-admin-kubeconfig. Assisted-By: Claude Signed-off-by: Andrei Kvapil <[email protected]>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the compute-plane design proposal to reflect that spec.releaseName is no longer set on HelmRelease objects, meaning release names are now derived from object names. This changes the cluster release name to computeplane-cluster and its admin kubeconfig secret to computeplane-cluster-admin-kubeconfig. Feedback was provided to correct an obsolete sentence in the document that still references the old -module suffix and the reservation of the computeplane name.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
|
||
| 1. When a tenant has the module enabled, the **tenant chart** renders the module `HelmRelease` (name `computeplane`, in the tenant namespace on the management cluster) whose `chartRef` points at the **`extra/computeplane`** chart's `ExternalArtifact`. Its own Helm release takes the `-module` suffix so the canonical release name `computeplane` stays reserved for the cluster release below. | ||
| 2. The **`extra/computeplane` chart** in turn renders the cluster `HelmRelease` (object name `computeplane-cluster`, `spec.releaseName: computeplane`) whose `chartRef` points at the **source-only re-sourced `apps/kubernetes`** `ExternalArtifact`, carrying the operator-fixed values. The release name `computeplane` is what makes Kamaji write the admin kubeconfig to the `computeplane-admin-kubeconfig` Secret — the contract `placement: ComputePlane` apps consume. | ||
| 2. The **`extra/computeplane` chart** in turn renders the cluster `HelmRelease` (object name `computeplane-cluster`) whose `chartRef` points at the **source-only re-sourced `apps/kubernetes`** `ExternalArtifact`, carrying the operator-fixed values. Neither `HelmRelease` sets `spec.releaseName` — the aggregated API rebuilds `HelmRelease` specs without that field, so both releases must derive their names from object names. The derived cluster release name `computeplane-cluster` is what makes Kamaji write the admin kubeconfig to the `computeplane-cluster-admin-kubeconfig` Secret — the contract `placement: ComputePlane` apps consume. |
There was a problem hiding this comment.
The changes in this PR remove spec.releaseName from both HelmRelease objects, meaning their release names are now derived directly from their object names (computeplane for the module, and computeplane-cluster for the cluster).\n\nHowever, the preceding sentence in item 1 (line 81) still states:\n> "Its own Helm release takes the -module suffix so the canonical release name computeplane stays reserved for the cluster release below."\n\nThis is now obsolete and incorrect because:\n1. The module's Helm release name no longer takes the -module suffix (it is now computeplane).\n2. The canonical name computeplane is no longer reserved for the cluster release (which is now computeplane-cluster).\n\nPlease update line 81 to remove or correct this sentence to match the new design.
The gap list still counted extra/computeplane and its computeplane-application PackageSource as unwritten, three weeks after cozystack#3280 merged them. Two of its five items were stale, which is the same class of drift this branch exists to fix. The remaining three are genuinely absent, verified against main: no placement field on ApplicationDefinition, no scoped ComputePlane-to-tenant-service egress policy, and no cozystack-api visibility or mutation control. Assisted-By: Claude Signed-off-by: Andrei Kvapil <[email protected]>
Aleksei Sviridkin (lexfrei)
left a comment
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
Andrei Kvapil (@kvaps) NOT LGTM. The new Secret name matches main, but the contract paragraph describes a secrets exclusion that main no longer has, one sentence next to it still says the opposite of the change, and both commits name a model in the trailer.
The secrets part of the contract paragraph (line 120) is false at main 52f542c4. It says the module's ApplicationDefinition withholds computeplane-cluster-admin-kubeconfig by literal name, that a rename would re-expose the credential, and that a bats cross-check pins the exclusion. On main, packages/system/computeplane-rd/cozyrds/computeplane.yaml has secrets.exclude: [] and include: []. Tenant-visible secrets are an allowlist there, so the kubeconfig is withheld whatever the release is called. The literal-name exclusion and hack/check-computeplane-kubeconfig-exclusion.bats were both removed in 8fc8d23d. A reader of the proposal would believe a rename leaks admin credentials and would look for a test that does not exist. The rename risks that are still real are the dangling secretRef and Helm treating the cluster as a new release, so the credential part should go.
Line 81 still says the module's own Helm release "takes the -module suffix so the canonical release name computeplane stays reserved". With no spec.releaseName on either HelmRelease, the module release is simply computeplane. The tenant chart names the object computeplane, and check-release-name.yaml fails the render unless the release name equals the chart name. So the sentence contradicts line 82 and the YAML below it, which already dropped releaseName: computeplane-module. The bot thread on this spot is still open for the same reason.
Both commits end with Assisted-By: Claude. The trailer takes only LLM, and this repo merges by rebase, so the commits land in main as they are. The body of 609c219 also opens with "Follow-up from the cozystack/cozystack#3280 review", which is review history and not a reason; the rest of that paragraph already gives the reason.
2 participants
Follow-up amendment from the cozystack/cozystack#3280 review: the aggregated API rebuilds HelmRelease specs without
spec.releaseName, so neither the module nor the cluster HelmRelease may set it — release names derive from object names, like every other tenant module. The stable external contract becomes the cluster HelmRelease object namecomputeplane-cluster, and the Secret the deferredplacement: ComputePlaneinjection consumes iscomputeplane-cluster-admin-kubeconfig. The §1 invariant is restated accordingly (including why an explicit release name is unrepresentable), and all Secret references are updated.