Add opt-in RBAC management UI via rbacManagement flag

[dimitri-nicolo]

Description

Type: New feature (Calico Enterprise only). Addresses PMREQ-824.

Adds an opt-in RBAC management UI feature, toggled by a new Manager.spec.rbac.ui field (Enabled | Disabled, defaults to Disabled). When enabled, the operator renders the additional RBAC and network access the UI needs to let an administrator manage role/group assignments from the Manager: maintaining a catalog of managed ClusterRoles, binding IdP groups to roles via the rbacsync controller, and resolving groups from an external LDAP/AD directory.

This should be merged because it is the operator-side enablement for the RBAC management UI; without it the UI's backend has no RBAC or egress to function. Gating on spec.rbac.ui keeps the feature — and its escalation-capable permissions — entirely dormant on clusters that don't opt in.

What the flag turns on, by layer

• API (api/v1/manager_types.go) — new RBAC spec struct with a UI RBACUI enum field and a nil-safe Manager.RBACManagementEnabled() helper (returns false for a nil Manager or any value other than Enabled). Regenerated deepcopy and the Manager CRD.
• Render
  • calico-manager role (pkg/render/manager.go): adds the cluster-scoped RBAC management UI rules (full CRUD on managed RBAC objects + escalation-prevention coverage for tier/resource roles), plus a namespaced Role+RoleBinding in calico-system that scopes the create/named-resource access to the IdP ConfigMap/Secrets (tigera-idp-groups, tigera-idp-ldap-config) instead of granting it cluster-wide. Sets RBAC_UI_ENABLED on the ui-apis container and opens LDAP/AD egress (389/636) when OIDC authentication is configured.
  • calico-kube-controllers (pkg/render/kubecontrollers/kube-controllers.go): enables the rbacsync controller and its RBAC only when the flag is set (dormant otherwise), and adds a namespaced Role+RoleBinding granting rbacsync read on the tigera-idp-groups ConfigMap in calico-system.
  • tigera-network-admin (pkg/render/apiserver.go): grants the bind/escalate-capable rule a network-admin needs to assign managed roles via the UI, also gated on the flag.
  • Shared escalation-prevention rules live in a new pkg/render/rbac_management.go so the manager and kube-controllers roles stay aligned (the SA writing a managed role must already hold every permission it grants, or K8s rejects the write as privilege escalation).

Why the installation and apiserver controllers read the Manager CR

The flag conceptually belongs to the Manager, but two of the resources it gates are not rendered by the manager controller:

• The rbacsync controller runs inside calico-kube-controllers, rendered by the installation controller.
• The tigera-network-admin ClusterRole is rendered by the apiserver controller.

So each of those controllers reads the Manager CR itself and passes a RBACManagementEnabled bool into its render config. To support this, GetManager was moved out of the manager controller into pkg/controller/utils so the apiserver and installation controllers can reuse it without importing the manager controller package. Both callers treat a missing Manager as not-found (feature off) rather than an error, and the installation controller only reads it for the enterprise variant.

Both controllers also add a watch on the Manager CR. Without it, toggling spec.rbac.ui would not re-trigger an installation/apiserver reconcile, so the rbacsync controller and the network-admin rule wouldn't appear or disappear until some unrelated event re-ran those controllers.

Per-cluster behavior

The feature is keyed off each cluster's own Manager.spec.rbac.ui, so a management cluster and a managed cluster opt in independently. Calico Enterprise evaluates a user's managed-cluster actions against that managed cluster's own RBAC (Voltron proxies the request to the managed cluster's apiserver), so the network-admin bind/escalate grant and the manager-SA rules are intentionally rendered on managed clusters too — that is where a user managing per-cluster RBAC needs them. In multi-tenant management clusters the manager-side rules and namespaced Role are not rendered (gated on !MultiTenant()).

Testing: unit tests added for all three render surfaces (manager, apiserver, kube-controllers) covering the enabled/disabled gate and the multi-tenant exclusion; make gen-files produces no drift; go build ./... and the affected pkg/render* / pkg/controller/apiserver suites pass.

Components affected: apiserver (tigera-network-admin), manager (calico-manager role + namespaced Role + network policy), kube-controllers (rbacsync), Manager CRD, installation & apiserver controllers, pkg/controller/utils.

Release Note

Added an opt-in `Manager.spec.rbacManagement.enabled` flag that enables the RBAC management UI. When enabled, the operator grants the additional RBAC and LDAP egress the UI requires; it defaults to disabled and is supported in zero-tenant management clusters only. Disabling the flag does not remove RBAC objects already created while it was enabled.

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.

[rene-dekker]

Can't we move the GetManager func from manager_controller to the utils package and re-use it? There is no reason that apiserver should throw an error if manager is not present. We should however only fetch it when enterprise CRDs exist and we are not running in cloud.

[rene-dekker]

I see that you moved the func to here, but I also meant to delete this func entirely as well.

[rene-dekker]

I think the code is more clear if we delete this method, remove any notion of "zero tenant" from the code including comments (it's not yargon that is used today) and use the multitenant booleans explicitly at the caller. I think that will be more understandable to the average reader.

[rene-dekker]

I don't think we should leak cloud (internal) implications into the API comments. Our users don't need to be aware/think about those.

[rene-dekker]

What do you think of the following field names?

• Management: Enabled|Disabled
• UI: Enabled|Disabled

or values: Visible | Hidden

[rene-dekker]

nit: This number is not likely to hold up over time, best to keep the comment succinct.

[rene-dekker]

General comment: the AI generated comments in this code are a bit excessive. An extreme example may be the permissions. The func explains in comments twice which permissions are added + the code to add them. They are making the code less readable. And now we need to also keep the comments up to date when we add more. The operator code is not super complicated, I'd much rather have comments be only used if it adds critical information you couldn't get from simply reading the code (or have your ai read it for you).

[rene-dekker]

Could we be more specific with secrets and configmaps and bind them to the namespaces we actually need?

[dimitri-nicolo]

The named-resource access moved to a new Role + RoleBinding in calico-system, where tigera-idp-groups and tigera-idp-ldap-config actually live. The broad create is bound to that one namespace now.

Dropped the tigera-known-oidc-users rule (it lives in tigera-elasticsearch and is already granted by logstorage.go), and tightened the activation gate to !Tenant.MultiTenant() since the IdP resources are pinned to calico-system and the RBAC UI is zero-tenant-only anyway.

Is that what you meant by "bind them to the namespaces we actually need" — or were you pointing at something narrower?

[rene-dekker]

This one should be in a role.

[pasanw]

Wondering if you found the variant check to be necessary here?

[pasanw]

Is it possible to avoid giving escalate and bind privileges? It essentially means that this controller can become a superuser if it chooses to be (i.e. is compromised)

[pasanw]

Similar to previous comment - can we avoid bind and escalate? This one seems a bit more serious since it is given to users

[pasanw]

Does this controller access this config map? I thought only the ui-apis endpoint would consult this config map when listing users

[pasanw]

Does the manager use its own SA when creating RBAC via the UI? Or is it impersonating the user?

[pasanw]

Why do we gate on OIDC being enabled? I'm wondering what that has do to with the user deciding they want to do an LDAP sync for RBAC UI.

To me I would have expected that this block conditionally renders depending on whether the LDAP secret is placed by the user, and that we could parse the LDAP URL to scope the network policy (we do a similar thing for guardian policy)

[pasanw]

Suggested change

	Enabled RBACUIStatus `json:"enabled,omitempty"`
	Type *RBACUIType `json:"rbacUI,omitempty"`

[dimitri-nicolo]

What about the following options:

// RBACUI configures the RBAC management UI.
type RBACUI struct {
	// Type selects the RBAC management UI mode. Unmanaged leaves Calico
	// Enterprise RBAC to be configured manually; ManagedCalicoEnterpriseRBAC
	// enables the RBAC management UI for Calico Enterprise RBAC. Defaults to
	// Unmanaged.
	// +optional
	// +kubebuilder:validation:Enum=Unmanaged;ManagedCalicoEnterpriseRBAC
	Type *RBACUIType `json:"type,omitempty"`
}

// RBACUIType selects the RBAC management UI mode.
type RBACUIType string

const (
	// RBACUIUnmanaged leaves RBAC to be configured manually.
	RBACUIUnmanaged RBACUIType = "Unmanaged"
	// RBACUIManagedCalicoEnterpriseRBAC enables the RBAC management UI for
	// Calico Enterprise RBAC.
	RBACUIManagedCalicoEnterpriseRBAC RBACUIType = "ManagedCalicoEnterpriseRBAC"
)

[pasanw]

nit: this comment serves as storage of rationale/history that is useful for the author, rather than context that helps the reader - would just remove (personally)

[pasanw]

nit: this comment serves as storage of rationale/history that is useful for the author, rather than context that helps the reader - would just remove (personally)

[pasanw]

What I meant was that it would be nice to have comments that store the context behind why we need these specific rules - my understanding is that they map to a specific permission needed from a specific managed role - can we reference the name of that managed role here?

[pasanw]

Sorry for catching this just now and not as part of my previous feedback, but if we have realized now that escalation rules only apply to the rbacsync controller and not ui-apis, I guess this no longer needs to be shared code? Everything should just be defined within rbacSyncControllerRules and everything there is an escalation rule? (and as per my other comment, each rule can have a comment explaining which managed role it covers)

[pasanw]

nit: I would remove this comment, feels like a comment written for the person steering Claude rather than the eventual reader of the code

[pasanw]

Should this also gate on multi-tenancy?

[pasanw]

LGTM 🎉 As discussed, let's see what the other maintainers think about the API changes before we merge

[Brian-McM]

We have an updated api spec in the docs that specifies these toggles,

Use a named string type with Enabled/Disabled constants and an Enum marker — not a *bool. This is the established repo idiom:

type NotificationMode string

const (
    Disabled NotificationMode = "Disabled"
    Enabled  NotificationMode = "Enabled"
)
// +kubebuilder:validation:Enum=Enabled;Disabled

[dimitri-nicolo]

@Brian-McM Thanks, I had considered raising the boolean vs. enum question with the maintainers before merging anything, but I'm happy to follow the idiom. Done in the latest commit: replaced the *bool with a named RBACUIStatusType enum (Enabled/Disabled + Enum marker), exposed as a State field.

cc: @pasanw

[Brian-McM]

Since utils.GetManager returns nil, nil on not found won't this now panic?

[dimitri-nicolo]

@Brian-McM It shouldn't panic, RBACManagementEnabled() guards m == nil before touching the pointer. The latest commit adds a table test (api/v1/manager_types_test.go) covering the nil-managerCR, nil-RBACUI, and nil-State cases.

[rene-dekker]

The host should be taken from Authentication.spec.ldap.host.

[rene-dekker]

I reviewed only the ldap changes, which look good now.