Skip to content

fix(computeplane): do not grant tenants the cluster admin kubeconfig - #4722

Merged
IvanHunters merged 1 commit into
mainfrom
fix/computeplane-admin-kubeconfig-rbac
Oct 2, 2026
Merged

IvanHunters merged 1 commit into
mainfrom
fix/computeplane-admin-kubeconfig-rbac

Conversation

@lexfrei

@lexfrei Aleksei Sviridkin (lexfrei) commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

What this PR does

A tenant user could read the admin kubeconfig of their own ComputePlane cluster, which the module promises never to hand out. The module deploys the kubernetes app into the tenant namespace as computeplane-cluster. The secrets allowlist kept the Secret out of tenantsecrets, but the chart's dashboard Role still gave the tenant's use subjects get/list/watch on computeplane-cluster-admin-kubeconfig. So a plain kubectl get secret worked.

The fix drops that one rule for the ComputePlane cluster only. The kubernetes chart now skips it when the internal value withholdAdminKubeconfig is set, and the computeplane module sets that value inline in the cluster HelmRelease. The tenant can't reach those values, because the module reads only nodeGroups and passes an allowlisted subset of it to the pool releases. The value is kept out of values.yaml and the schema, same as clusterReleaseName in kubernetes-nodes.

A regular Kubernetes app keeps the rule, so the dashboard still offers its kubeconfig. Only the v1.7.0 alphas ship the module. After this lands, the kubectl auth can-i check can go back into the e2e suite in #4713.

Screenshots

No UI changes.

Downstream repositories

Release note

fix(computeplane): tenant users no longer get read access to the ComputePlane cluster's admin kubeconfig Secret through the kubernetes app's dashboard Role.

Summary by CodeRabbit

  • Security
    • Computeplane dashboards no longer grant access to the cluster’s admin kubeconfig Secret. Other dashboard resource permissions remain available. This limits access to sensitive cluster credentials through the dashboard while preserving the service-related permissions needed for dashboard resources.

The computeplane module deploys the kubernetes chart into the tenant
namespace, and that chart's dashboard Role lets the tenant's "use"
subjects read <release>-admin-kubeconfig. Withholding the Secret from
the tenant secrets view does not help when RBAC grants it directly, so
any tenant user at "use" level or above could fetch the cluster-admin
credential of a cluster the module promises to keep operator-owned.

The kubernetes chart now drops that rule when the internal value
withholdAdminKubeconfig is set, and the module sets it in the cluster
HelmRelease's inline values. The tenant cannot reach those values: the
module reads only nodeGroups from them and splices an allowlisted
subset into the pool releases, and Flux applies spec.values over
valuesFrom. A regular kubernetes app keeps the rule, so its dashboard
can still offer the kubeconfig.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <[email protected]>
@lexfrei Aleksei Sviridkin (lexfrei) added this to the v1.7.0 milestone Oct 2, 2026
@github-actions github-actions Bot added area/uncategorized PR auto-labeler could not map title scope to a known area/*; please review kind/bug Categorizes issue or PR as related to a bug size/M This PR changes 30-99 lines, ignoring generated files labels Oct 2, 2026
@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: cozystack/cozystack/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 188a88fa-494b-4b4a-98ce-0c409697a736

📥 Commits

Reviewing files that changed from the base of the PR and between 9c9a8c2 and 64f43f4.

📒 Files selected for processing (4)
  • packages/apps/kubernetes/templates/dashboard-resourcemap.yaml
  • packages/apps/kubernetes/tests/dashboard_resourcemap_test.yaml
  • packages/extra/computeplane/templates/cluster.yaml
  • packages/extra/computeplane/tests/cluster_test.yaml

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

The dashboard Role conditionally includes a rule to read the release’s admin-kubeconfig Secret. The computeplane HelmRelease sets the value that omits this rule.

Changes

Dashboard admin kubeconfig access

Layer / File(s) Summary
Conditional dashboard Role rule
packages/apps/kubernetes/templates/dashboard-resourcemap.yaml, packages/apps/kubernetes/tests/dashboard_resourcemap_test.yaml
The Role omits the admin-kubeconfig Secret rule when withholdAdminKubeconfig is true. Tests check the Role’s rules when the Secret is available and when it is withheld.
Enable withholding for computeplane
packages/extra/computeplane/templates/cluster.yaml, packages/extra/computeplane/tests/cluster_test.yaml
The computeplane HelmRelease sets withholdAdminKubeconfig to true. A test asserts this value.

Priority: ⬆️ High

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 64f43

Computeplane withholds dashboard access to its admin kubeconfig, while regular Kubernetes apps retain their existing access. No merge-blocking issue was identified.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 64f43

The change narrows credential access, and the inspected configuration prevents tenant inputs from disabling that restriction. No introduced or worsened security issue was established. Successful rollout and recovery from any earlier credential disclosure remain unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The affected credential provides administrative access to the tenant's ComputePlane cluster. The restriction targets that release's named Secret, not unrelated Secrets or platform-wide privileges. Cross-tenant or management-cluster compromise through this credential was not established.

Security Findings and Attack Paths

  • inferred — The pre-existing path was a bound tenant identity reading the admin-kubeconfig Secret and obtaining cluster-admin credentials. The PR removes this specific grant after convergence. Removing read permission does not itself invalidate credentials copied earlier; recovery from prior disclosure is not demonstrated and is not an introduced or worsened condition.

Trust Boundaries and Controls

  • inferred — Authorization policy remains operator-owned: inline cluster values take precedence over imported values, and tenant pool shape is allowlisted. The supplied helper-and-rule proof gap is resolved for this RoleBinding, but independently created runtime grants were not verified.

Resilience and Maintainability Implications

  • inferred — The stable Role identity avoids a second authorization object that would require separate cleanup. Existing disableWait and retry settings do not prove effective denial during partial failure, interruption, or recovery; rollout completion must be distinguished from desired configuration.

Hardening Proposals

  • proposed — Validate denial using representative tenant identities after upgrading an existing release, including recovery from interruption. For clusters where credentials may already have been disclosed, assess credential revocation separately and ensure rollback does not silently restore the old grant.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: preventing tenant users from receiving the cluster admin kubeconfig.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@lexfrei Aleksei Sviridkin (lexfrei) added area/extra Issues or PRs related to tenant-specific modules (packages/extra/) area/kubernetes Issues or PRs related to the tenant Kubernetes app and removed area/uncategorized PR auto-labeler could not map title scope to a known area/*; please review labels Oct 2, 2026

@IvanHunters IvanHunters left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

This closes the tenant's read path to the ComputePlane cluster-admin kubeconfig, and the removal is scoped the way it should be.

What I checked:

  • The withholdAdminKubeconfig guard only drops the secrets/<release>-admin-kubeconfig rule from the dashboard Role. With the value absent (the default) the rule stays, so a normal kubernetes app doesn't change at all: rendering the merge base and HEAD for a plain cluster gives byte-identical output.
  • The invariant itself, not just the guard. A tenant "use" subject has no other way to that Secret. None of the tenant tiers (cozy:tenant, :view, :use, :admin, :super-admin) grant secrets at all, by any name, so the per-app dashboard Role was the only RBAC grant and it's now gated. The aggregated-API path is closed independently by computeplane-rd with secrets.include: [], and a tenant can't squat the name through TenantSecret since there's no write grant on tenantsecrets.
  • The pin can't be reopened from tenant input. withholdAdminKubeconfig: true is a literal in the wrapper's spec.values, which Flux merges over valuesFrom (cozystack-values), so nothing injected there wins.
  • The tests actually hold the behavior. Inverting the guard reddens both assertions in dashboard_resourcemap_test.yaml (default-on and withheld), and the computeplane test pins that the wrapper passes the flag through. Both touched charts pass against current main.

Keeping the toggle out of values.yaml and the schema, with the reasoning written inline, is the right call.

@IvanHunters
IvanHunters enabled auto-merge October 2, 2026 12:13
@IvanHunters
IvanHunters merged commit dad5924 into main Oct 2, 2026
41 of 42 checks passed
@IvanHunters
IvanHunters deleted the fix/computeplane-admin-kubeconfig-rbac branch October 2, 2026 15:11
IvanHunters added a commit that referenced this pull request Oct 6, 2026
## What this PR does

This adds an e2e suite for the ComputePlane tenant module, which had
only render tests so far. The suite lives in
`hack/e2e-chainsaw/computeplane/`. It turns the module on in its own
tenant, `tenant-cplane`, so the shared `tenant-test` is never toggled.
That tenant is a child of `tenant-root`, not of `tenant-test`, because a
tenant two levels down cannot reach the etcd it inherits from root
(#4726).

What it checks on a real cluster:

- the module release installs, and the cluster release it renders keeps
the pinned addons and the `internal.cozystack.io/computeplane` label;
- the pool release gets only the keys from the module's per-pool
allowlist (the patch sends a `nameOverride` through the ComputePlane API
and checks it is dropped);
- Kamaji writes `computeplane-cluster-admin-kubeconfig`, and the tenant
can't get at it: the Secret is not labelled `tenantresource=true`,
`tenantsecrets` does not list it to the tenant's super-admin group, and
`kubectl auth can-i get` answers `no` for that group (the read was
possible until #4722);
- cert-manager and ingress-nginx come up inside the cluster, and the
Proxied ingress Service gets a ready endpoint;
- the module and cluster releases have no remediation cycle in
`status.history`;
- on teardown, the in-cluster addon releases are gone before the
KamajiControlPlane starts deleting;
- turning the module off removes all of it: releases, in-cluster addons,
control plane, Secret, worker VMs and disks, and the LINSTOR pools go
back to their pre-suite level.

The issue names the Secret `computeplane-admin-kubeconfig`. The module
writes `computeplane-cluster-admin-kubeconfig`, so the suite checks that
name.

The issue also lists fluxcd as a pinned addon. That is out of date. The
module runs no in-cluster Flux on purpose
(`packages/extra/computeplane/templates/cluster.yaml:108-110`), so the
suite checks cert-manager and ingress-nginx only.

One part of the issue is not covered here. The `placement: ComputePlane`
routing in cozystack-api doesn't exist yet, so there is no catalog app
to run on the cluster. The issue ties that to "together with, or right
after" this test, so I left it for the routing work, tracked in #4715.

Things the review found outside this diff, each filed on its own: the
module release's 10m install timeout against a slow cluster bringup
(#4714), the module docs not matching the per-pool allowlist (#4716),
and the LINSTOR helpers in `_lib` running `kubectl exec` without a
timeout (#4111).

The suite brings its own pool: `md0`, one worker, same sizing as the
kubernetes suites. The default `md0` that the module renders for empty
`nodeGroups` asks for the `replicated` StorageClass. The container lane
doesn't have it, so on this lane the default pool stays covered by the
render tests only.

ComputePlane workers can't use the in-sandbox Talos image cache. The
pool allowlist drops `talos`, so the worker pulls its disk from the
public Image Factory (#4700). If the worker join or an addon release
times out, the catch block prints the worker image source and points at
that issue, so a red run caused by the factory is easy to tell apart.

The suite itself needs no entry in `hack/select-e2e.sh`, because
`computeplane-application` maps to it by name. That source depends on
`kubernetes-application` and re-sources the kubernetes charts. So PRs
that touch the kubernetes charts or ingress-nginx now select
`computeplane` too, which I estimate adds 12-15 minutes to a green run
(kubernetes-previous takes about 8 minutes with two workers, plus
teardown). A red run can take much longer, because the waits sit above
the release install timeouts. The suite header has the numbers. It is
the third suite that brings up a tenant cluster, and the 215m E2E job
cap is sized for the red path of the kubernetes pair only. If worker
join breaks everywhere, all three go red and can run past the cap, which
cancels the job and loses its log. I left that as an accepted risk.
`hack/select-e2e_test.bats` now expects `computeplane` in the
ingress-nginx selection.

The computeplane source also owns `apps/kubernetes-nodes`. Before this
PR a pool chart edit reached no suite and ran everything. With the new
suite alone it would have selected only `computeplane`, which never
creates `KubernetesNodes` natively. So `kubernetes-nodes-application`
now maps to `kubernetes-latest kubernetes-previous computeplane` in
`hack/select-e2e.sh`, and two new tests pin that for a pool chart edit
and a `kubernetes-nodes-rd` edit.

The suite is not in the leak-guard allowlist. Its teardown already fails
when a TenantControlPlane sticks on its finalizer (#4489), because the
admin kubeconfig Secret it owns stays behind, so an exemption would only
hide other leaks. The README suite list now names the suite.

JFYI, not a defect claim: the Proxied ingress Service from the
kubernetes chart has `internal.cozystack.io/tenantresource: "true"`. So
the tenant sees it, even though `computeplane-rd` has `services.include:
[]`. The module doesn't promise to hide it.

How I checked it. `chainsaw lint` passes at v0.2.15, the version the
sandbox pins. I also ran the suite against an envtest apiserver, with
fake controllers applying the real `helm template` output of the tenant
and computeplane charts. The intact run is green. Each of these
breakages turns it red at the step meant to catch it: label removed,
addon `exposeMethod` changed, a key outside the pool allowlist, the
allowlist `pick` replaced by the whole group, stale Ready from an older
generation, wrong Secret name, Secret labelled `tenantresource`, Secret
listed in `tenantsecrets`, a direct RBAC grant on the Secret,
ingress-nginx addon not Ready, no ready ingress endpoint, an
`uninstalled` history entry, the control plane deleted before the addon
releases, an addon release or worker VM left after teardown.

Closes #3369
Closes #4747

### Screenshots

Not a UI change.

### Downstream repositories

- [x] No downstream repository is affected by this change
- [ ] [cozystack/website](https://github.com/cozystack/website) -
follow-up:
- [ ]
[cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack)
- follow-up:
- [ ]
[cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack)
- follow-up:
- [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up:
- [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up:
- [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) -
follow-up:
- [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) -
follow-up:
- [ ]
[cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server)
- follow-up:
- [ ]
[cozystack/external-apps-example](https://github.com/cozystack/external-apps-example)
- follow-up:
- [ ] [cozystack/examples](https://github.com/cozystack/examples) -
follow-up:
- [ ] [cozystack/community](https://github.com/cozystack/community) -
follow-up:

### Release note

```release-note
test(e2e): add an end-to-end suite for the ComputePlane tenant module
```


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Tests**
* Added end-to-end coverage for enabling and disabling the ComputePlane
tenant module, including generated releases, pool value filtering, admin
credential access, addon and cluster readiness, and resource cleanup.
* Kubernetes nodes changes now select the ComputePlane suite alongside
the latest and previous Kubernetes suites.

* **Documentation**
* Added the ComputePlane suite to the documented end-to-end test suite
list.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/extra Issues or PRs related to tenant-specific modules (packages/extra/) area/kubernetes Issues or PRs related to the tenant Kubernetes app kind/bug Categorizes issue or PR as related to a bug size/M This PR changes 30-99 lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants