Gate the RBAC management UI on a per-cluster ConfigMap by dimitri-nicolo · Pull Request #5114 · tigera/operator

dimitri-nicolo · 2026-07-27T23:09:08Z

Description

Replaces the Manager.spec.rbacUI.state gate for the RBAC management UI with a per-cluster rbac-ui-config ConfigMap in calico-system, keyed rbac-ui-enabled. The admin owns the ConfigMap outright; the operator, ui-apis and rbacsync only read it, so a toggle needs no restart or rollout.

The consuming side is on calico-private branch https://github.com/tigera/calico-private/pull/12954 — that PR will be merged alongside this one.

Changes

API — Manager.spec.rbacUI removed, CRD regenerated, along with the Manager CR reads and watches in the installation and apiserver controllers.
Read — the installation, apiserver and manager controllers each read and watch the ConfigMap. A missing ConfigMap, missing key or unparsable value reads as disabled. The operator never writes it.
Render — the access the feature depends on is gated on the value: the tigera-network-admin rolebinding rules, the calico-manager cluster rules, the namespaced Role scoping ui-apis to the IdP resources, the LDAP/AD egress, and the rbacsync controller plus its Role. Multi-tenant renders none of it.
Enablement — tigera-network-admin gets write access to rbac-ui-config.

Notes

Fail-closed — a missing ConfigMap reads as disabled, and the operator does not re-create it, so deleting it switches the feature off and keeps it off. An unreadable ConfigMap degrades the reconcile rather than rendering as disabled.
create on ConfigMaps is namespace-wide — RBAC cannot restrict create by resource name, so the write grant admits creating any ConfigMap in the namespace. A namespaced Role would narrow it but tigera-network-admin is a ClusterRole the customer binds themselves.

Release Note

NONE

For PR author

Tests for change.
If changing pkg/apis/, run make gen-files
If changing versions, run make gen-versions

For PR reviewers

A note for code reviewers - all pull requests must have the following:

Milestone set according to targeted release.
Appropriate labels:
- kind/bug if this is a bugfix.
- kind/enhancement if this is a a new feature.
- enterprise if this PR applies to Calico Enterprise only.

pasanw

The TL:DR of most of my comments: we've moved the enable knob outside of the operator API, but the operator should still consume it

pasanw · 2026-08-04T16:47:43Z

+	// The admin owns this ConfigMap; the operator only reads it, and an absent one reads
+	// as disabled.
+	var rbacManagementEnabled bool
+	if instance.Spec.Variant.IsEnterprise() {


Should there be a multi-tenancy check here? Wondering if the setting of rbacManagementEnabled should be gated on the three conditions (enterprise, multi-tenancy, config map), and then the component itself just uses this boolean rather than performing the same checks again

pasanw · 2026-08-04T16:59:13Z


+// RBACManagementEnabled reports whether the RBAC management UI is switched on for this
+// cluster. A missing ConfigMap, missing key or unparsable value reads as disabled.
+func RBACManagementEnabled(cm *corev1.ConfigMap) bool {


Consider migrating this to render/common

pasanw · 2026-08-04T17:02:20Z

 		Manager:                    instance,
 		Authentication:             authenticationCR,
 		KibanaEnabled:              kibanaEnabled,
+		RBACManagementEnabled:      render.RBACManagementEnabled(rbacGate),


Similar comment as the one I left on the core controller: consider setting this based on the full check with tenancy so we don't need to repeat the same checks inside the render component

pasanw · 2026-08-04T18:03:50Z

 		Cloud:                        r.opts.Cloud,
 		RequiresAggregationServer:    !r.opts.UseV3CRDs,
-		RBACManagementEnabled:        managerCR.RBACManagementEnabled(),
+		RBACManagementEnabled:        rbacManagementEnabled,


Same thought here as the one I left in the core controller

pasanw · 2026-08-04T18:08:40Z


+	// The admin-owned switch for the RBAC management UI, read by the operator, ui-apis
+	// and rbacsync. Keep in sync with ui-apis rbacmanagement/gate.
+	RBACManagementConfigMapName = "rbac-ui-config"


Consider moving these to render/common

The gate parser sat in pkg/render/manager.go but has nothing to do with the manager component: the installation, manager and apiserver controllers all call it. Give it a leaf package under render/common instead. pkg/render/common/rbacmanagement imports nothing from the operator, so it is reachable from both the controllers and the render packages without a cycle. The ConfigMap name and key move with the function that reads them; render keeps aliases so existing call sites are untouched. Addresses a review comment on tigera#5114. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The parser already lived in render/common/rbacmanagement, but the names it works with were still in pkg/render/manager.go, so the apiserver, kube-controllers and the three controllers reached into the manager component for constants unrelated to it. Move the ConfigMap name and key and the LDAP config Secret name next to the parser, and drop the aliases render was re-exporting. tigera-idp-groups comes along as GroupsConfigMapName: a bare literal repeated in manager.go and kube-controllers.go, which is the duplication these constants exist to prevent. Names only -- no rendered resource changes. Addresses a review comment on tigera#5114. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

RBACManagementEnabled carried only "the admin's gate is on", so each render component re-derived the rest for itself: the manager re-checked tenancy, kube-controllers re-checked the variant and tenancy, and the apiserver checked neither. utils.RBACManagementEnabled now owns all three conditions -- variant, tenancy and the gate -- and the components render the boolean as-is. It lives in controller/utils because it takes a client, and the dependency runs controller -> render, not the reverse. Two of the render-side tenancy checks could never have fired. kube-controllers reads cfg.Tenant, which the core controller never sets and the ES path sets to nil. The apiserver's gated rules ride on tigera-network-admin, which is only rendered on zero/single-tenant clusters; a new test pins that. Both controllers now pass their real tenancy anyway, so the flag no longer depends on those guarantees holding elsewhere. Two render tests that set the gate alongside a controller-owned condition move up to the controller level; the Calico-variant case was already covered there. Also fixes the apiserver test build: ControllerOptions.EnterpriseCRDExists became Variant in 989cbc0, which left the suite uncompilable and static-checks red. Addresses review comments on tigera#5114. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The RBAC management UI is gated per cluster by the rbac-ui-config ConfigMap in calico-system, keyed rbac-ui-enabled. The operator seeds it disabled and an admin with access to that cluster edits the value to toggle the feature there. ui-apis and rbacsync each read it live for the cluster they are acting on, so a toggle takes effect without restarting or rolling anything. Keep the constants in sync with ui-apis rbacmanagement/gate. The installation controller owns the seed: it runs on management, managed and standalone clusters alike, needs no Manager CR (a managed cluster has none, yet rbacsync still reads that cluster's copy over the tunnel), and is a single reconciler per cluster, so there is no second writer to race. The seed runs right after the namespace it lives in and is gated on the Enterprise variant. The admin's value is protected two ways. The seed is create-only, so an existing ConfigMap is never written and a concurrent create cannot lose the toggle. And it carries no OwnerReference, so deleting the Installation does not garbage-collect the switch and a reinstall does not silently turn the feature off. Deletion is fail-closed: both consumers read a missing ConfigMap as disabled, and the next reconcile re-seeds it disabled rather than restoring a value the operator does not track. The seed is logged, since the feature going quiet is otherwise invisible. Since activation is decided at runtime, the access the feature needs is rendered on every non-multi-tenant Enterprise cluster: the manager cluster and namespaced roles, the rbacsync controller and its roles, the tigera-network-admin rules, and the LDAP egress (gated on LDAP being configured on Authentication). ui-apis and rbacsync get read-only access to the gate, the value being the admin's to set. Multi-tenant management clusters render none of it, matching the force-disable on the ui-apis side. Manager.spec.rbacUI, the RBACManagementEnabled helper, and the Manager CR reads and watches the installation and apiserver controllers held for it are removed, since nothing derives the feature's state from a CR. EV-6816

Adds RBACManagementEnabled, parsed with strconv.ParseBool so True or 1 also work; a missing ConfigMap, missing key or unparsable value reads as disabled. The installation, apiserver and manager controllers each read the ConfigMap and watch it, so a toggle re-runs the reconcile. Nothing is gated on the value yet. EV-6816

tigera-network-admin carries create/update/delete on clusterrolebindings and rolebindings for the RBAC management UI. Rendering that on every Enterprise cluster grants escalation-capable permissions to clusters that never switch the feature on, so add the rules only while rbac-ui-config enables it. ui-apis still writes these impersonating the caller, so the apiserver enforces escalation against the user's own permissions. EV-6816

The calico-manager cluster rules, the namespaced Role scoping ui-apis to the IdP resources, and the LDAP/AD egress all exist to serve the RBAC management UI, so render them only while rbac-ui-config enables the feature. rbacManagementUIActive folds the gate together with the multi-tenant exclusion so the cluster rules and the namespaced grant cannot drift apart. ui-apis keeps read access to the gate itself, which is how it observes the feature being switched off. EV-6816

The rbacsync controller and its namespaced Role are rendered only while rbac-ui-config enables the RBAC management UI. Also restores the multi-tenant exclusion. That was previously an accident of how the value was read: the installation controller looked the Manager CR up by a cluster-scoped key, which never resolves on a multi-tenant management cluster, so the feature always read as disabled there. Reading the ConfigMap removed that side effect, so rbacSyncEnabled now checks tenancy explicitly. ENABLED_CONTROLLERS is a container env var, so toggling the gate restarts calico-kube-controllers. EV-6816

Review feedback: the operator should have a read-only relationship with rbac-ui-config, leaving the user in control of whether it exists. Drops the seeding. The installation controller no longer creates the ConfigMap and render.RBACManagementConfigMap goes with it, so the operator's only contact with the gate is the three controllers reading it. The read path was already fail-closed on a missing ConfigMap, so the default is unchanged. What changes is that deleting it now stays deleted rather than reappearing disabled on the next reconcile. EV-6816

The manager and apiserver reads had no test: the render tests set RBACManagementEnabled by hand, so nothing asserted either controller plumbs the ConfigMap through to its render config. Adds specs per controller that reconcile and assert on the rendered output -- the gated tigera-network-admin rules, and the namespaced IdP Role -- for the ConfigMap being absent, enabled and explicitly false. Also covers an unreadable ConfigMap degrading rather than rendering as disabled, and the gate being read on a managed cluster. EV-6816

Nothing granted write access to rbac-ui-config, so enabling the RBAC management UI needed cluster-admin. Since the operator no longer creates the ConfigMap, an admin with only tigera-network-admin could not switch the feature on at all. Adds the write access to tigera-network-admin, ungated: a rule rendered only while the feature is on could never be used to turn it on. create cannot be restricted by resource name, so this grant admits creating any ConfigMap in the namespace. Narrowing it would need a namespaced Role, which is not available here -- tigera-network-admin is a ClusterRole the customer binds themselves, so an operator-rendered Role would have no subject to bind to. EV-6816

The gate parser sat in pkg/render/manager.go but has nothing to do with the manager component: the installation, manager and apiserver controllers all call it. Give it a leaf package under render/common instead. pkg/render/common/rbacmanagement imports nothing from the operator, so it is reachable from both the controllers and the render packages without a cycle. The ConfigMap name and key move with the function that reads them; render keeps aliases so existing call sites are untouched. Addresses a review comment on tigera#5114. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The parser already lived in render/common/rbacmanagement, but the names it works with were still in pkg/render/manager.go, so the apiserver, kube-controllers and the three controllers reached into the manager component for constants unrelated to it. Move the ConfigMap name and key and the LDAP config Secret name next to the parser, and drop the aliases render was re-exporting. tigera-idp-groups comes along as GroupsConfigMapName: a bare literal repeated in manager.go and kube-controllers.go, which is the duplication these constants exist to prevent. Names only -- no rendered resource changes. Addresses a review comment on tigera#5114. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

RBACManagementEnabled carried only "the admin's gate is on", so each render component re-derived the rest for itself: the manager re-checked tenancy, kube-controllers re-checked the variant and tenancy, and the apiserver checked neither. utils.RBACManagementEnabled now owns all three conditions -- variant, tenancy and the gate -- and the components render the boolean as-is. It lives in controller/utils because it takes a client, and the dependency runs controller -> render, not the reverse. Two of the render-side tenancy checks could never have fired. kube-controllers reads cfg.Tenant, which the core controller never sets and the ES path sets to nil. The apiserver's gated rules ride on tigera-network-admin, which is only rendered on zero/single-tenant clusters; a new test pins that. Both controllers now pass their real tenancy anyway, so the flag no longer depends on those guarantees holding elsewhere. Two render tests that set the gate alongside a controller-owned condition move up to the controller level; the Calico-variant case was already covered there. Also fixes the apiserver test build: ControllerOptions.EnterpriseCRDExists became Variant in 989cbc0, which left the suite uncompilable and static-checks red. Addresses review comments on tigera#5114. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The gate parser and its names sat in pkg/render/manager.go, and each render component re-derived when the feature is on: the manager re-checked tenancy, kube-controllers the variant and tenancy, the apiserver neither. Move the names and the parser to pkg/render/common/rbacmanagement, and give utils.RBACManagementEnabled the whole question -- variant, tenancy and the admin's switch -- so the components render the boolean as-is. The reader lives in controller/utils because it takes a client, and controller/utils already imports render/common. tigera-idp-groups comes along as a named constant; it was a bare literal in both manager.go and kube-controllers.go. Two render tests that set the gate alongside a controller-owned condition move up to the controller level. Also fixes the test build: ControllerOptions.EnterpriseCRDExists became Variant in 989cbc0. Addresses review comments on tigera#5114. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

pasanw · 2026-08-05T04:18:25Z

-			return reconcile.Result{}, err
-		}
-		rbacManagementEnabled = render.RBACManagementEnabled(gate)
+	rbacManagementEnabled, err := utils.RBACManagementEnabled(ctx, r.client, instance.Spec.Variant, false)


I can't say I'm very knowledgeable on multi-tenancy, but Claude is saying that just because kube-controllers is always a cluster-scoped deployment, it doesn't mean that it is never run on a multi-tenant management cluster. It's suggesting that we wire through the multi-tenancy signal from opts.MultiTenant.

Happy to leave this up to you to evaluate and determine the correct approach

marvin-tigera added this to the v1.44.0 milestone Jul 27, 2026

marvin-tigera added docs-pr-required release-note-required labels Jul 27, 2026

dimitri-nicolo force-pushed the dimitri-EV-6816-rbac-ui-enabled branch from 397e263 to 4bfcc19 Compare July 28, 2026 23:34

dimitri-nicolo added release-note-not-required and removed release-note-required labels Jul 28, 2026

dimitri-nicolo force-pushed the dimitri-EV-6816-rbac-ui-enabled branch 2 times, most recently from 1ada653 to b72d4e6 Compare July 29, 2026 17:39

dimitri-nicolo marked this pull request as ready for review July 29, 2026 17:39

dimitri-nicolo requested review from a team and marvin-tigera as code owners July 29, 2026 17:39

pasanw reviewed Jul 29, 2026

View reviewed changes

dimitri-nicolo force-pushed the dimitri-EV-6816-rbac-ui-enabled branch from b72d4e6 to 570c7e4 Compare July 30, 2026 22:29

dimitri-nicolo requested a review from pasanw July 31, 2026 16:57

dimitri-nicolo added docs-not-required and removed docs-pr-required labels Jul 31, 2026

pasanw reviewed Aug 4, 2026

View reviewed changes

dimitri-nicolo force-pushed the dimitri-EV-6816-rbac-ui-enabled branch from 570c7e4 to 1692936 Compare August 4, 2026 23:11

dimitri-nicolo added 8 commits August 4, 2026 16:11

dimitri-nicolo force-pushed the dimitri-EV-6816-rbac-ui-enabled branch from 1692936 to c6eceb4 Compare August 4, 2026 23:17

dimitri-nicolo requested a review from pasanw August 4, 2026 23:27

dimitri-nicolo force-pushed the dimitri-EV-6816-rbac-ui-enabled branch from c6eceb4 to 2eeb425 Compare August 5, 2026 00:07

pasanw approved these changes Aug 5, 2026

View reviewed changes

Navigation Menu

Uh oh!

Gate the RBAC management UI on a per-cluster ConfigMap - #5114

Gate the RBAC management UI on a per-cluster ConfigMap#5114
dimitri-nicolo wants to merge 9 commits into
tigera:masterfrom
dimitri-nicolo:dimitri-EV-6816-rbac-ui-enabled

dimitri-nicolo commented Jul 27, 2026 •

edited

Loading

Uh oh!

pasanw left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pasanw Aug 4, 2026

Uh oh!

pasanw Aug 4, 2026

Uh oh!

pasanw Aug 4, 2026

Uh oh!

pasanw Aug 4, 2026

Uh oh!

pasanw Aug 4, 2026

Uh oh!

pasanw Aug 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

dimitri-nicolo commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Notes

Release Note

For PR author

For PR reviewers

Uh oh!

pasanw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pasanw Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

pasanw Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

pasanw Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

pasanw Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

pasanw Aug 4, 2026

Choose a reason for hiding this comment

Uh oh!

pasanw Aug 5, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dimitri-nicolo commented Jul 27, 2026 •

edited

Loading