diff --git a/api/v1/manager_types.go b/api/v1/manager_types.go index f338ce0d99..d6e7223844 100644 --- a/api/v1/manager_types.go +++ b/api/v1/manager_types.go @@ -26,37 +26,6 @@ type ManagerSpec struct { // ManagerDeployment configures the Manager Deployment. // +optional ManagerDeployment *ManagerDeployment `json:"managerDeployment,omitempty"` - - // RBACUI configures the RBAC management UI feature. - // +optional - RBACUI *RBACUI `json:"rbacUI,omitempty"` -} - -// +kubebuilder:validation:Enum=Enabled;Disabled -type RBACUIStatusType string - -const ( - RBACUIDisabled RBACUIStatusType = "Disabled" - RBACUIEnabled RBACUIStatusType = "Enabled" -) - -// RBACUI configures the RBAC management UI. This is a separate control plane -// for Calico Enterprise RBAC that lives alongside, and does not replace, the -// user's ability to configure RBAC themselves. -type RBACUI struct { - // State turns the RBAC management UI on or off. Defaults to Disabled. - // +optional - State *RBACUIStatusType `json:"state,omitempty"` -} - -// RBACManagementEnabled returns true when the Manager CR enables the RBAC -// management UI. Safe to call on a nil receiver; returns false for a nil -// Manager, an unset RBACUI, or any state other than Enabled. -func (m *Manager) RBACManagementEnabled() bool { - if m == nil || m.Spec.RBACUI == nil || m.Spec.RBACUI.State == nil { - return false - } - return *m.Spec.RBACUI.State == RBACUIEnabled } // ManagerDeployment is the configuration for the Manager Deployment. diff --git a/api/v1/manager_types_test.go b/api/v1/manager_types_test.go deleted file mode 100644 index 9a59442dff..0000000000 --- a/api/v1/manager_types_test.go +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright (c) 2026 Tigera, Inc. All rights reserved. - -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package v1 - -import "testing" - -func TestRBACManagementEnabled(t *testing.T) { - state := func(s RBACUIStatusType) *RBACUIStatusType { return &s } - - for _, tc := range []struct { - name string - m *Manager - want bool - }{ - // Nil paths: RBACManagementEnabled is called as managerCR.RBACManagementEnabled() - // where managerCR is nil when no Manager CR exists, so a nil receiver (and each - // nil field below) must return false rather than panic. - {name: "nil Manager", m: nil, want: false}, - {name: "nil RBACUI", m: &Manager{}, want: false}, - {name: "nil State", m: &Manager{Spec: ManagerSpec{RBACUI: &RBACUI{}}}, want: false}, - - {name: "State Enabled", m: &Manager{Spec: ManagerSpec{RBACUI: &RBACUI{State: state(RBACUIEnabled)}}}, want: true}, - {name: "State Disabled", m: &Manager{Spec: ManagerSpec{RBACUI: &RBACUI{State: state(RBACUIDisabled)}}}, want: false}, - // Any value other than Enabled is off (the Enum marker rejects this at the - // apiserver, but the helper must not treat a non-empty value as enabled). - {name: "State unrecognized value", m: &Manager{Spec: ManagerSpec{RBACUI: &RBACUI{State: state("SomethingElse")}}}, want: false}, - } { - t.Run(tc.name, func(t *testing.T) { - defer func() { - if r := recover(); r != nil { - t.Fatalf("RBACManagementEnabled panicked on %s: %v", tc.name, r) - } - }() - if got := tc.m.RBACManagementEnabled(); got != tc.want { - t.Errorf("RBACManagementEnabled() = %v, want %v", got, tc.want) - } - }) - } -} diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index 30e6ba5f6c..8548208f0e 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -7927,11 +7927,6 @@ func (in *ManagerSpec) DeepCopyInto(out *ManagerSpec) { *out = new(ManagerDeployment) (*in).DeepCopyInto(*out) } - if in.RBACUI != nil { - in, out := &in.RBACUI, &out.RBACUI - *out = new(RBACUI) - (*in).DeepCopyInto(*out) - } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ManagerSpec. @@ -9026,26 +9021,6 @@ func (in *QueryServerLogging) DeepCopy() *QueryServerLogging { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *RBACUI) DeepCopyInto(out *RBACUI) { - *out = *in - if in.State != nil { - in, out := &in.State, &out.State - *out = new(RBACUIStatusType) - **out = **in - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RBACUI. -func (in *RBACUI) DeepCopy() *RBACUI { - if in == nil { - return nil - } - out := new(RBACUI) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Retention) DeepCopyInto(out *Retention) { *out = *in diff --git a/pkg/controller/apiserver/apiserver_controller.go b/pkg/controller/apiserver/apiserver_controller.go index 700a958891..1301adafa7 100644 --- a/pkg/controller/apiserver/apiserver_controller.go +++ b/pkg/controller/apiserver/apiserver_controller.go @@ -104,6 +104,11 @@ func Add(mgr manager.Manager, opts options.ControllerOptions) error { } if opts.EnterpriseCRDExists { + // Watched so a toggle re-renders the rules gated on it. + if err = utils.AddConfigMapWatch(c, render.RBACManagementConfigMapName, common.CalicoNamespace, &handler.EnqueueRequestForObject{}); err != nil { + return fmt.Errorf("apiserver-controller failed to watch ConfigMap %s: %w", render.RBACManagementConfigMapName, err) + } + // Watch for changes to ApplicationLayer err = c.WatchObject(&operatorv1.ApplicationLayer{ObjectMeta: metav1.ObjectMeta{Name: utils.DefaultEnterpriseInstanceKey.Name}}, &handler.EnqueueRequestForObject{}) if err != nil { @@ -122,13 +127,6 @@ func Add(mgr manager.Manager, opts options.ControllerOptions) error { return fmt.Errorf("apiserver-controller failed to watch primary resource: %v", err) } - // Watch the Manager CR so toggling spec.rbac re-runs the apiserver - // reconcile (the tigera-network-admin RBAC is gated on it). - err = c.WatchObject(&operatorv1.Manager{}, &handler.EnqueueRequestForObject{}) - if err != nil { - return fmt.Errorf("apiserver-controller failed to watch Manager: %v", err) - } - for _, namespace := range []string{common.OperatorNamespace(), render.APIServerNamespace} { for _, secretName := range []string{render.VoltronTunnelSecretName, render.ManagerTLSSecretName} { if err = utils.AddSecretsWatch(c, secretName, namespace); err != nil { @@ -361,7 +359,7 @@ func (r *ReconcileAPIServer) Reconcile(ctx context.Context, request reconcile.Re var managementCluster *operatorv1.ManagementCluster var managementClusterConnection *operatorv1.ManagementClusterConnection var keyValidatorConfig authentication.KeyValidatorConfig - var managerCR *operatorv1.Manager + var rbacManagementEnabled bool includeV3NetworkPolicy := false if installationSpec.Variant.IsEnterprise() { @@ -372,6 +370,17 @@ func (r *ReconcileAPIServer) Reconcile(ctx context.Context, request reconcile.Re return reconcile.Result{}, err } + // The admin owns this ConfigMap; the operator only reads it, and an absent one + // reads as disabled. + gate, err := utils.GetIfExists[corev1.ConfigMap](ctx, client.ObjectKey{ + Name: render.RBACManagementConfigMapName, Namespace: common.CalicoNamespace, + }, r.client) + if err != nil { + r.status.SetDegraded(operatorv1.ResourceReadError, "Error reading the RBAC management UI ConfigMap", err, reqLogger) + return reconcile.Result{}, err + } + rbacManagementEnabled = render.RBACManagementEnabled(gate) + applicationLayer, err = utils.GetApplicationLayer(ctx, r.client) if err != nil { r.status.SetDegraded(operatorv1.ResourceReadError, "Error reading ApplicationLayer", err, reqLogger) @@ -390,12 +399,6 @@ func (r *ReconcileAPIServer) Reconcile(ctx context.Context, request reconcile.Re return reconcile.Result{}, err } - managerCR, err = utils.GetManager(ctx, r.client, false, "") - if err != nil { - r.status.SetDegraded(operatorv1.ResourceReadError, "Error reading Manager", err, reqLogger) - return reconcile.Result{}, err - } - if managementClusterConnection != nil && managementCluster != nil { err = fmt.Errorf("having both a ManagementCluster and a ManagementClusterConnection is not supported") r.status.SetDegraded(operatorv1.ResourceValidationError, "", err, reqLogger) @@ -538,7 +541,7 @@ func (r *ReconcileAPIServer) Reconcile(ctx context.Context, request reconcile.Re ClusterDomain: r.opts.ClusterDomain, Cloud: r.opts.Cloud, RequiresAggregationServer: !r.opts.UseV3CRDs, - RBACManagementEnabled: managerCR.RBACManagementEnabled(), + RBACManagementEnabled: rbacManagementEnabled, QueryServerTLSKeyPairCertificateManagementOnly: queryServerTLSSecretCertificateManagementOnly, } diff --git a/pkg/controller/apiserver/apiserver_controller_test.go b/pkg/controller/apiserver/apiserver_controller_test.go index d192103173..472d7fc99c 100644 --- a/pkg/controller/apiserver/apiserver_controller_test.go +++ b/pkg/controller/apiserver/apiserver_controller_test.go @@ -33,6 +33,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" "sigs.k8s.io/controller-runtime/pkg/reconcile" v3 "github.com/tigera/api/pkg/apis/projectcalico/v3" @@ -491,6 +492,126 @@ var _ = Describe("apiserver controller tests", func() { }) }) + // These cover the controller's half: reading the ConfigMap and handing the value to + // the renderer. + Context("RBAC management UI feature gate", func() { + // gatedRule is where the gate's value is observable in the rendered output. + gatedRule := rbacv1.PolicyRule{ + APIGroups: []string{"rbac.authorization.k8s.io"}, + Resources: []string{"clusterrolebindings", "rolebindings"}, + Verbs: []string{"get", "list", "watch", "create", "update", "delete"}, + } + + writeGate := func(value string) { + Expect(cli.Create(ctx, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: render.RBACManagementConfigMapName, + Namespace: common.CalicoNamespace, + }, + Data: map[string]string{render.RBACManagementConfigMapKey: value}, + })).NotTo(HaveOccurred()) + } + + // networkAdminRules reconciles and returns the rendered rules. + networkAdminRules := func() []rbacv1.PolicyRule { + r := ReconcileAPIServer{ + client: cli, + scheme: scheme, + status: mockStatus, + tierWatchReady: ready, + migrationWatchReady: &utils.ReadyFlag{}, + opts: options.ControllerOptions{ + EnterpriseCRDExists: true, + DetectedProvider: operatorv1.ProviderNone, + }, + } + _, err := r.Reconcile(ctx, reconcile.Request{}) + Expect(err).ShouldNot(HaveOccurred()) + + cr := rbacv1.ClusterRole{} + Expect(cli.Get(ctx, client.ObjectKey{Name: "tigera-network-admin"}, &cr)).NotTo(HaveOccurred()) + return cr.Rules + } + + BeforeEach(func() { + Expect(cli.Create(ctx, installation)).NotTo(HaveOccurred()) + }) + + It("withholds the rules when the admin has not created the ConfigMap", func() { + Expect(networkAdminRules()).NotTo(ContainElement(gatedRule)) + }) + + It("adds the rules once the admin enables the feature", func() { + writeGate("true") + Expect(networkAdminRules()).To(ContainElement(gatedRule)) + }) + + It("withholds the rules when the admin sets the value to false", func() { + writeGate("false") + Expect(networkAdminRules()).NotTo(ContainElement(gatedRule)) + }) + + // A managed cluster carries tigera-network-admin too, so the read must not be + // skipped there. + It("reads the gate on a managed cluster", func() { + Expect(cli.Create(ctx, &operatorv1.ManagementClusterConnection{ + ObjectMeta: metav1.ObjectMeta{Name: utils.DefaultEnterpriseInstanceKey.Name}, + })).NotTo(HaveOccurred()) + writeGate("true") + + Expect(networkAdminRules()).To(ContainElement(gatedRule)) + }) + + // An unreadable ConfigMap is unknown state, not absent, so it degrades rather + // than rendering as disabled. + It("degrades and requeues when the ConfigMap cannot be read", func() { + readErr := fmt.Errorf("the API server is having a bad day") + failing := ctrlrfake.DefaultFakeClientBuilder(scheme). + WithInterceptorFuncs(interceptor.Funcs{ + Get: func(ctx context.Context, c client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + if _, ok := obj.(*corev1.ConfigMap); ok && key.Name == render.RBACManagementConfigMapName { + return readErr + } + return c.Get(ctx, key, obj, opts...) + }, + }).Build() + + // Re-plant what the reconcile needs on the new client. + certificateManager, err := certificatemanager.Create(failing, nil, "cluster.local", common.OperatorNamespace(), certificatemanager.AllowCACreation()) + Expect(err).NotTo(HaveOccurred()) + Expect(failing.Create(ctx, certificateManager.KeyPair().Secret(common.OperatorNamespace()))).NotTo(HaveOccurred()) + Expect(failing.Create(ctx, &operatorv1.APIServer{ObjectMeta: metav1.ObjectMeta{Name: "tigera-secure"}})).NotTo(HaveOccurred()) + Expect(failing.Create(ctx, &v3.Tier{ObjectMeta: metav1.ObjectMeta{Name: "calico-system"}})).NotTo(HaveOccurred()) + // The shared installation carries a resourceVersion that Create would reject. + freshInstallation := installation.DeepCopy() + freshInstallation.ResourceVersion = "" + Expect(failing.Create(ctx, freshInstallation)).NotTo(HaveOccurred()) + + degraded := &status.MockStatus{} + degraded.On("OnCRFound").Return() + degraded.On("SetMetaData", mock.Anything).Return() + degraded.On("AddCertificateSigningRequests", mock.Anything).Return().Maybe() + degraded.On("RemoveCertificateSigningRequests", mock.Anything).Return().Maybe() + degraded.On("SetDegraded", operatorv1.ResourceReadError, + "Error reading the RBAC management UI ConfigMap", readErr.Error(), mock.Anything).Return().Once() + + r := ReconcileAPIServer{ + client: failing, + scheme: scheme, + status: degraded, + tierWatchReady: ready, + migrationWatchReady: &utils.ReadyFlag{}, + opts: options.ControllerOptions{ + EnterpriseCRDExists: true, + DetectedProvider: operatorv1.ProviderNone, + }, + } + _, err = r.Reconcile(ctx, reconcile.Request{}) + Expect(err).To(MatchError(readErr)) + degraded.AssertExpectations(GinkgoT()) + }) + }) + Context("Reconcile for Condition status", func() { generation := int64(2) BeforeEach(func() { diff --git a/pkg/controller/installation/core_controller.go b/pkg/controller/installation/core_controller.go index 6c3d08279f..6f53bbbc7c 100644 --- a/pkg/controller/installation/core_controller.go +++ b/pkg/controller/installation/core_controller.go @@ -204,6 +204,11 @@ func Add(mgr manager.Manager, opts options.ControllerOptions) error { return fmt.Errorf("tigera-installation-controller failed to watch ConfigMap %s: %w", active.ActiveConfigMapName, err) } + // Watched so a toggle re-renders the access gated on it. + if err = utils.AddConfigMapWatch(c, render.RBACManagementConfigMapName, common.CalicoNamespace, &handler.EnqueueRequestForObject{}); err != nil { + return fmt.Errorf("tigera-installation-controller failed to watch ConfigMap %s: %w", render.RBACManagementConfigMapName, err) + } + if err = imageset.AddImageSetWatch(c); err != nil { return fmt.Errorf("tigera-installation-controller failed to watch ImageSet: %w", err) } @@ -252,14 +257,6 @@ func Add(mgr manager.Manager, opts options.ControllerOptions) error { return fmt.Errorf("tigera-installation-controller failed to watch primary resource: %v", err) } - // Watch the Manager CR so changes to spec.rbac re-run the installation - // reconcile (the rbacsync controller in calico-kube-controllers is - // gated on it). - err = c.WatchObject(&operatorv1.Manager{}, &handler.EnqueueRequestForObject{}) - if err != nil { - return fmt.Errorf("tigera-installation-controller failed to watch Manager: %v", err) - } - // watch for change to primary resource LogCollector err = c.WatchObject(&operatorv1.LogCollector{}, &handler.EnqueueRequestForObject{}) if err != nil { @@ -1068,7 +1065,6 @@ func (r *ReconcileInstallation) Reconcile(ctx context.Context, request reconcile var managementCluster *operatorv1.ManagementCluster var managementClusterConnection *operatorv1.ManagementClusterConnection - var managerCR *operatorv1.Manager var logCollector *operatorv1.LogCollector if r.enterpriseCRDsExist { logCollector, err = utils.GetLogCollector(ctx, r.client) @@ -1091,12 +1087,6 @@ func (r *ReconcileInstallation) Reconcile(ctx context.Context, request reconcile return reconcile.Result{}, err } - managerCR, err = utils.GetManager(ctx, r.client, false, "") - if err != nil { - r.status.SetDegraded(operatorv1.ResourceReadError, "Error reading Manager", err, reqLogger) - return reconcile.Result{}, err - } - if managementClusterConnection != nil && managementCluster != nil { err = fmt.Errorf("having both a managementCluster and a managementClusterConnection is not supported") r.status.SetDegraded(operatorv1.ResourceValidationError, "", err, reqLogger) @@ -1355,6 +1345,20 @@ func (r *ReconcileInstallation) Reconcile(ctx context.Context, request reconcile return reconcile.Result{}, err } + // 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() { + gate, err := utils.GetIfExists[corev1.ConfigMap](ctx, client.ObjectKey{ + Name: render.RBACManagementConfigMapName, Namespace: common.CalicoNamespace, + }, r.client) + if err != nil { + r.status.SetDegraded(operatorv1.ResourceReadError, "Error reading the RBAC management UI ConfigMap", err, reqLogger) + return reconcile.Result{}, err + } + rbacManagementEnabled = render.RBACManagementEnabled(gate) + } + // Build the list of components to render, in rendering order. components := []render.Component{} if newActiveCM != nil && !installationMarkedForDeletion { @@ -1741,8 +1745,8 @@ func (r *ReconcileInstallation) Reconcile(ctx context.Context, request reconcile // disabled); the caBundle is the operator CA that issued the serving // cert above. WAFWebhookCABundle: certificateManager.KeyPair().GetCertificatePEM(), - RBACManagementEnabled: managerCR.RBACManagementEnabled(), Cloud: r.cloud, + RBACManagementEnabled: rbacManagementEnabled, } components = append(components, kubecontrollers.NewCalicoKubeControllers(&kubeControllersCfg)) diff --git a/pkg/controller/installation/core_controller_test.go b/pkg/controller/installation/core_controller_test.go index bda4f6065b..80c354c664 100644 --- a/pkg/controller/installation/core_controller_test.go +++ b/pkg/controller/installation/core_controller_test.go @@ -36,6 +36,7 @@ import ( rbacv1 "k8s.io/api/rbac/v1" schedv1 "k8s.io/api/scheduling/v1" storagev1 "k8s.io/api/storage/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -1381,6 +1382,33 @@ var _ = Describe("Testing core-controller installation", func() { )) }) + // The feature is Enterprise only, so the gate is not read on Calico. + It("should ignore the RBAC management UI feature gate for Calico", func() { + cr.Spec.Variant = operator.Calico + Expect(c.Create(ctx, cr)).NotTo(HaveOccurred()) + Expect(c.Create(ctx, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: render.RBACManagementConfigMapName, + Namespace: common.CalicoNamespace, + }, + Data: map[string]string{render.RBACManagementConfigMapKey: "true"}, + })).NotTo(HaveOccurred()) + + _, err := r.Reconcile(ctx, reconcile.Request{}) + Expect(err).ShouldNot(HaveOccurred()) + + d := &appsv1.Deployment{} + Expect(c.Get(ctx, client.ObjectKey{ + Name: "calico-kube-controllers", Namespace: common.CalicoNamespace, + }, d)).ShouldNot(HaveOccurred()) + container := test.GetContainer(d.Spec.Template.Spec.Containers, "calico-kube-controllers") + Expect(container).NotTo(BeNil()) + Expect(container.Env).NotTo(ContainElement(WithTransform( + func(env corev1.EnvVar) string { return env.Value }, + ContainSubstring("rbacsync"), + ))) + }) + It("should Reconcile with default config", func() { Expect(c.Create(ctx, cr)).NotTo(HaveOccurred()) _, err := r.Reconcile(ctx, reconcile.Request{}) @@ -2401,6 +2429,109 @@ var _ = Describe("Testing core-controller installation", func() { Expect(c.Get(ctx, client.ObjectKey{Name: render.TyphaTLSSecretName, Namespace: common.OperatorNamespace()}, secret)).ShouldNot(HaveOccurred()) Expect(secret.GetOwnerReferences()).To(HaveLen(1)) }) + + // The admin owns whether the ConfigMap exists and what it says; the operator only + // reads it. + Context("RBAC management UI feature gate", func() { + gateKey := client.ObjectKey{Name: render.RBACManagementConfigMapName, Namespace: common.CalicoNamespace} + + // enabledControllers is where the gate's value is observable. + enabledControllers := func() string { + d := &appsv1.Deployment{} + Expect(c.Get(ctx, client.ObjectKey{ + Name: "calico-kube-controllers", Namespace: common.CalicoNamespace, + }, d)).ShouldNot(HaveOccurred()) + + container := test.GetContainer(d.Spec.Template.Spec.Containers, "calico-kube-controllers") + Expect(container).NotTo(BeNil()) + for _, env := range container.Env { + if env.Name == "ENABLED_CONTROLLERS" { + return env.Value + } + } + Fail("calico-kube-controllers has no ENABLED_CONTROLLERS env var") + return "" + } + + writeGate := func(value string) { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: gateKey.Name, Namespace: gateKey.Namespace}, + Data: map[string]string{render.RBACManagementConfigMapKey: value}, + } + Expect(c.Create(ctx, cm)).ShouldNot(HaveOccurred()) + } + + It("does not create the ConfigMap", func() { + _, err := r.Reconcile(ctx, reconcile.Request{}) + Expect(err).ShouldNot(HaveOccurred()) + + err = c.Get(ctx, gateKey, &corev1.ConfigMap{}) + Expect(apierrors.IsNotFound(err)).To(BeTrue(), "expected the operator not to create rbac-ui-config") + }) + + It("reads a missing ConfigMap as disabled", func() { + _, err := r.Reconcile(ctx, reconcile.Request{}) + Expect(err).ShouldNot(HaveOccurred()) + + Expect(enabledControllers()).NotTo(ContainSubstring("rbacsync")) + }) + + It("follows the admin's value once they create the ConfigMap", func() { + writeGate("true") + + _, err := r.Reconcile(ctx, reconcile.Request{}) + Expect(err).ShouldNot(HaveOccurred()) + + Expect(enabledControllers()).To(ContainSubstring("rbacsync")) + }) + + It("leaves the admin's value untouched across reconciles", func() { + writeGate("true") + + _, err := r.Reconcile(ctx, reconcile.Request{}) + Expect(err).ShouldNot(HaveOccurred()) + + cm := &corev1.ConfigMap{} + Expect(c.Get(ctx, gateKey, cm)).ShouldNot(HaveOccurred()) + Expect(cm.Data).To(HaveKeyWithValue(render.RBACManagementConfigMapKey, "true")) + // Deleting the Installation must not take the admin's toggle with it. + Expect(cm.GetOwnerReferences()).To(BeEmpty()) + }) + + It("switches the feature back off when the admin deletes the ConfigMap", func() { + writeGate("true") + + _, err := r.Reconcile(ctx, reconcile.Request{}) + Expect(err).ShouldNot(HaveOccurred()) + Expect(enabledControllers()).To(ContainSubstring("rbacsync")) + + cm := &corev1.ConfigMap{} + Expect(c.Get(ctx, gateKey, cm)).ShouldNot(HaveOccurred()) + Expect(c.Delete(ctx, cm)).ShouldNot(HaveOccurred()) + + // Fail-closed, and the operator does not put the ConfigMap back. + _, err = r.Reconcile(ctx, reconcile.Request{}) + Expect(err).ShouldNot(HaveOccurred()) + Expect(enabledControllers()).NotTo(ContainSubstring("rbacsync")) + Expect(apierrors.IsNotFound(c.Get(ctx, gateKey, cm))).To(BeTrue()) + }) + + // An unreadable ConfigMap is unknown state, not absent, so it degrades rather + // than rendering as disabled. + It("degrades and requeues when the ConfigMap cannot be read", func() { + readErr := fmt.Errorf("the API server is having a bad day") + r.client = failingGateReadClient{Client: c, err: readErr} + mockStatus.On("SetDegraded", operator.ResourceReadError, + "Error reading the RBAC management UI ConfigMap", readErr.Error(), mock.Anything).Return().Once() + + _, err := r.Reconcile(ctx, reconcile.Request{}) + Expect(err).To(MatchError(readErr)) + // The shared mockStatus expects a full reconcile, which this returns early + // from, so assert the one call. + mockStatus.AssertCalled(GinkgoT(), "SetDegraded", operator.ResourceReadError, + "Error reading the RBAC management UI ConfigMap", readErr.Error(), mock.Anything) + }) + }) }) Context("with a fake component handler", func() { @@ -3080,3 +3211,17 @@ var _ = Describe("updateValidatingAdmissionPolicies", func() { Expect(componentHandler.objectsToCreate).To(HaveLen(2)) }) }) + +// failingGateReadClient fails the read of the gate ConfigMap and passes everything else +// through, to distinguish an unreadable ConfigMap from an absent one. +type failingGateReadClient struct { + client.Client + err error +} + +func (f failingGateReadClient) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + if _, ok := obj.(*corev1.ConfigMap); ok && key.Name == render.RBACManagementConfigMapName { + return f.err + } + return f.Client.Get(ctx, key, obj, opts...) +} diff --git a/pkg/controller/manager/manager_controller.go b/pkg/controller/manager/manager_controller.go index 993e229821..cc2d2d0061 100644 --- a/pkg/controller/manager/manager_controller.go +++ b/pkg/controller/manager/manager_controller.go @@ -188,6 +188,11 @@ func Add(mgr manager.Manager, opts options.ControllerOptions) error { return fmt.Errorf("manager-controller failed to watch ConfigMap resource %s: %w", tigerakvc.StaticWellKnownJWKSConfigMapName, err) } + // Watched so that toggling the RBAC management UI re-renders the access gated on it. + if err = utils.AddConfigMapWatch(c, render.RBACManagementConfigMapName, common.CalicoNamespace, eventHandler); err != nil { + return fmt.Errorf("manager-controller failed to watch ConfigMap resource %s: %w", render.RBACManagementConfigMapName, err) + } + if err = utils.AddConfigMapWatch(c, relasticsearch.ClusterConfigConfigMapName, common.OperatorNamespace(), eventHandler); err != nil { return fmt.Errorf("compliance-controller failed to watch the ConfigMap resource: %w", err) } @@ -706,6 +711,16 @@ func (r *ReconcileManager) Reconcile(ctx context.Context, request reconcile.Requ } } + // The admin owns this ConfigMap; the operator only reads it, and an absent one reads + // as disabled. + rbacGate, err := utils.GetIfExists[corev1.ConfigMap](ctx, client.ObjectKey{ + Name: render.RBACManagementConfigMapName, Namespace: common.CalicoNamespace, + }, r.client) + if err != nil { + r.status.SetDegraded(operatorv1.ResourceReadError, "Error reading the RBAC management UI ConfigMap", err, logc) + return reconcile.Result{}, err + } + managerCfg := &render.ManagerConfiguration{ VoltronRouteConfig: routeConfig, KeyValidatorConfig: keyValidatorConfig, @@ -735,6 +750,7 @@ func (r *ReconcileManager) Reconcile(ctx context.Context, request reconcile.Requ Manager: instance, Authentication: authenticationCR, KibanaEnabled: kibanaEnabled, + RBACManagementEnabled: render.RBACManagementEnabled(rbacGate), CACertCommonName: certificateManager.CACertCommonName(), Cloud: r.opts.Cloud, CloudResources: mcr, diff --git a/pkg/controller/manager/manager_controller_test.go b/pkg/controller/manager/manager_controller_test.go index b3960fa4c7..ba8a82fb2b 100644 --- a/pkg/controller/manager/manager_controller_test.go +++ b/pkg/controller/manager/manager_controller_test.go @@ -584,6 +584,64 @@ var _ = Describe("Manager controller tests", func() { r.tierWatchReady.MarkAsReady() }) + // These cover the controller's half: reading the ConfigMap and handing the + // value to the renderer. The namespaced Role is where it is observable. + Context("RBAC management UI feature gate", func() { + roleKey := client.ObjectKey{Name: render.ManagerClusterRole, Namespace: common.CalicoNamespace} + + writeGate := func(value string) { + Expect(c.Create(ctx, &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: render.RBACManagementConfigMapName, + Namespace: common.CalicoNamespace, + }, + Data: map[string]string{render.RBACManagementConfigMapKey: value}, + })).NotTo(HaveOccurred()) + } + + // idpRoleExists reconciles and reports whether the gated Role landed. + idpRoleExists := func() bool { + _, err := r.Reconcile(ctx, reconcile.Request{}) + Expect(err).NotTo(HaveOccurred()) + + err = c.Get(ctx, roleKey, &rbacv1.Role{}) + if err != nil && !kerror.IsNotFound(err) { + Expect(err).NotTo(HaveOccurred()) + } + return err == nil + } + + It("withholds the namespaced Role when the admin has not created the ConfigMap", func() { + Expect(idpRoleExists()).To(BeFalse()) + }) + + It("renders the namespaced Role once the admin enables the feature", func() { + writeGate("true") + Expect(idpRoleExists()).To(BeTrue()) + }) + + It("withholds the namespaced Role when the admin sets the value to false", func() { + writeGate("false") + Expect(idpRoleExists()).To(BeFalse()) + }) + + // An unreadable ConfigMap is unknown state, not absent, so it degrades + // rather than rendering as disabled. + It("degrades and requeues when the ConfigMap cannot be read", func() { + readErr := fmt.Errorf("the API server is having a bad day") + r.client = failingGateReadClient{Client: c, err: readErr} + // The shared mockStatus expects a full reconcile, which this returns + // early from, so assert the one call. + mockStatus.On("SetDegraded", operatorv1.ResourceReadError, + "Error reading the RBAC management UI ConfigMap", readErr.Error(), mock.Anything).Return().Once() + + _, err := r.Reconcile(ctx, reconcile.Request{}) + Expect(err).To(MatchError(readErr)) + mockStatus.AssertCalled(GinkgoT(), "SetDegraded", operatorv1.ResourceReadError, + "Error reading the RBAC management UI ConfigMap", readErr.Error(), mock.Anything) + }) + }) + It("should reconcile legacy manager namespace", func() { result, err := r.Reconcile(ctx, reconcile.Request{}) Expect(err).NotTo(HaveOccurred()) @@ -1479,6 +1537,20 @@ var _ = Describe("Manager controller tests", func() { }) }) +// failingGateReadClient fails the read of the gate ConfigMap and passes everything else +// through, to distinguish an unreadable ConfigMap from an absent one. +type failingGateReadClient struct { + client.Client + err error +} + +func (f failingGateReadClient) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + if _, ok := obj.(*corev1.ConfigMap); ok && key.Name == render.RBACManagementConfigMapName { + return f.err + } + return f.Client.Get(ctx, key, obj, opts...) +} + func assertSANs(secret *corev1.Secret, expectedSAN string) { var cert *x509.Certificate diff --git a/pkg/imports/crds/operator/operator.tigera.io_managers.yaml b/pkg/imports/crds/operator/operator.tigera.io_managers.yaml index 66c1345bf4..5e77333404 100644 --- a/pkg/imports/crds/operator/operator.tigera.io_managers.yaml +++ b/pkg/imports/crds/operator/operator.tigera.io_managers.yaml @@ -298,18 +298,6 @@ spec: type: object type: object type: object - rbacUI: - description: RBACUI configures the RBAC management UI feature. - properties: - state: - description: - State turns the RBAC management UI on or off. Defaults - to Disabled. - enum: - - Enabled - - Disabled - type: string - type: object type: object status: description: Most recently observed state for the Calico Enterprise manager. diff --git a/pkg/render/apiserver.go b/pkg/render/apiserver.go index 2cc42b9148..12ca5bab45 100644 --- a/pkg/render/apiserver.go +++ b/pkg/render/apiserver.go @@ -152,16 +152,15 @@ type APIServerConfiguration struct { KubernetesVersion *common.VersionInfo ClusterDomain string - // RBACManagementEnabled gates the RBAC management UI permissions on - // tigera-network-admin. - RBACManagementEnabled bool - // Cloud indicates the API server is being rendered for a Calico Cloud install. It gates // cloud-specific RBAC in the tigera-ui-user / tigera-network-admin cluster roles (Calico Cloud // exposes only per-user UISettings and grants access to runtime logs). When false the RBAC is // exactly the regular Calico/Calico Enterprise RBAC. Cloud bool + // RBACManagementEnabled is the value of the rbac-ui-config gate for this cluster. + RBACManagementEnabled bool + // Whether or not we should run the aggregation API server for projectcalico.org/v3 APIs // as part of this component. RequiresAggregationServer bool @@ -2192,10 +2191,26 @@ func (c *apiServerComponent) tigeraNetworkAdminClusterRole() *rbacv1.ClusterRole }, }...) - // Role/binding access for the RBAC management UI. ui-apis writes these - // impersonating the caller, so the apiserver enforces escalation against the - // user's own permissions. The UI reads the role catalogue and manages group - // membership through both cluster- and namespace-scoped bindings. + // Write access to the switch, so a network admin can enable the feature without + // cluster-admin. Not gated: 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 it admits + // creating any ConfigMap in the namespace. + rules = append(rules, + rbacv1.PolicyRule{ + APIGroups: []string{""}, + Resources: []string{"configmaps"}, + Verbs: []string{"create"}, + }, + rbacv1.PolicyRule{ + APIGroups: []string{""}, + Resources: []string{"configmaps"}, + ResourceNames: []string{RBACManagementConfigMapName}, + Verbs: []string{"get", "list", "watch", "update", "patch", "delete"}, + }, + ) + + // Role/binding access for the RBAC management UI. ui-apis writes these impersonating + // the caller, so the apiserver enforces escalation against the user's own permissions. if c.cfg.RBACManagementEnabled { rules = append(rules, rbacv1.PolicyRule{ diff --git a/pkg/render/apiserver_test.go b/pkg/render/apiserver_test.go index cd718e50ea..049407fdbb 100644 --- a/pkg/render/apiserver_test.go +++ b/pkg/render/apiserver_test.go @@ -384,9 +384,9 @@ var _ = Describe("API server rendering tests (Calico Enterprise)", func() { Entry("custom cluster domain", "custom-domain.internal"), ) - It("should gate the RBAC management UI rule on RBACManagementEnabled", func() { - // Disabled (default): tigera-network-admin must not carry the - // escalation-capable RBAC management rule. + // The escalation-capable rolebinding rules are the reason this is gated. + It("should gate the RBAC management UI rules on tigera-network-admin", func() { + By("omitting them while the feature gate is off") component, err := render.APIServer(cfg) Expect(err).NotTo(HaveOccurred()) resources, _ := component.Objects() @@ -394,19 +394,46 @@ var _ = Describe("API server rendering tests (Calico Enterprise)", func() { for _, rule := range rbacManagementNetworkAdminRules { Expect(clusterRole.Rules).NotTo(ContainElement(rule)) } + Expect(clusterRole.Rules).To(ConsistOf(networkAdminPolicyRules)) - // Enabled: the rules are appended. + By("adding them once the admin switches the feature on") cfg.RBACManagementEnabled = true component, err = render.APIServer(cfg) Expect(err).NotTo(HaveOccurred()) resources, _ = component.Objects() clusterRole = rtest.GetResource(resources, "tigera-network-admin", "", "rbac.authorization.k8s.io", "v1", "ClusterRole").(*rbacv1.ClusterRole) - for _, rule := range rbacManagementNetworkAdminRules { - Expect(clusterRole.Rules).To(ContainElement(rule)) - } Expect(clusterRole.Rules).To(ConsistOf(append(networkAdminPolicyRules, rbacManagementNetworkAdminRules...))) }) + // Not gated: a rule rendered only while the feature is on could never turn it on. + It("should grant tigera-network-admin write access to the switch regardless of the gate", func() { + gateWriteRules := []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"configmaps"}, + Verbs: []string{"create"}, + }, + { + APIGroups: []string{""}, + Resources: []string{"configmaps"}, + ResourceNames: []string{render.RBACManagementConfigMapName}, + Verbs: []string{"get", "list", "watch", "update", "patch", "delete"}, + }, + } + + for _, enabled := range []bool{false, true} { + cfg.RBACManagementEnabled = enabled + component, err := render.APIServer(cfg) + Expect(err).NotTo(HaveOccurred()) + resources, _ := component.Objects() + clusterRole := rtest.GetResource(resources, "tigera-network-admin", "", "rbac.authorization.k8s.io", "v1", "ClusterRole").(*rbacv1.ClusterRole) + for _, rule := range gateWriteRules { + Expect(clusterRole.Rules).To(ContainElement(rule), + "expected the switch write rules with RBACManagementEnabled=%v", enabled) + } + } + }) + It("should render resources without an aggregation server", func() { cfg.RequiresAggregationServer = false @@ -1976,11 +2003,22 @@ var ( ResourceNames: []string{"webhooks-secret"}, Verbs: []string{"patch"}, }, + // Write access to the switch, ungated so it can be used to turn the feature on. + { + APIGroups: []string{""}, + Resources: []string{"configmaps"}, + Verbs: []string{"create"}, + }, + { + APIGroups: []string{""}, + Resources: []string{"configmaps"}, + ResourceNames: []string{render.RBACManagementConfigMapName}, + Verbs: []string{"get", "list", "watch", "update", "patch", "delete"}, + }, } - // rbacManagementNetworkAdminRules are the extra tigera-network-admin rules - // added when rbac.ui is Enabled. See tigeraNetworkAdminClusterRole for the - // rationale behind the verb set. + // rbacManagementNetworkAdminRules are the extra tigera-network-admin rules added + // while the feature is enabled. rbacManagementNetworkAdminRules = []rbacv1.PolicyRule{ { APIGroups: []string{"rbac.authorization.k8s.io"}, diff --git a/pkg/render/kubecontrollers/kube-controllers.go b/pkg/render/kubecontrollers/kube-controllers.go index bb0fecd785..c299428497 100644 --- a/pkg/render/kubecontrollers/kube-controllers.go +++ b/pkg/render/kubecontrollers/kube-controllers.go @@ -170,8 +170,7 @@ type KubeControllersConfiguration struct { // Only consulted when WAFGatewayExtensionEnabled is true. WAFWebhookCABundle []byte - // RBACManagementEnabled mirrors Manager.spec.rbacUI.state and gates the - // rbacsync controller in calico-kube-controllers. + // RBACManagementEnabled is the value of the rbac-ui-config gate for this cluster. RBACManagementEnabled bool } @@ -228,9 +227,8 @@ func NewCalicoKubeControllers(cfg *KubeControllersConfiguration) *kubeController enabledControllers = append(enabledControllers, "applicationlayer") } - // Runs the rbacsync controller to reconcile managed ClusterRoles and - // bindings against the tigera-idp-groups ConfigMap. - if cfg.RBACManagementEnabled { + // Reconciles ClusterRoles and bindings against the tigera-idp-groups ConfigMap. + if rbacSyncEnabled(cfg) { enabledControllers = append(enabledControllers, "rbacsync") kubeControllerRolePolicyRules = append(kubeControllerRolePolicyRules, rbacSyncControllerRules()...) } @@ -379,8 +377,8 @@ func (c *kubeControllersComponent) Objects() ([]client.Object, []client.Object) c.controllersClusterRoleBinding(), ) objectsToCreate = append(objectsToCreate, c.managedClusterRoleBindings()...) - if c.cfg.RBACManagementEnabled { - objectsToCreate = append(objectsToCreate, c.rbacSyncIDPGroupsRole()...) + if c.kubeControllerName == KubeController && rbacSyncEnabled(c.cfg) { + objectsToCreate = append(objectsToCreate, c.rbacSyncNamespacedRole()...) } if len(c.enabledControllers) > 0 { @@ -714,10 +712,17 @@ func kubeControllersRoleEnterpriseCommonRules(cfg *KubeControllersConfiguration) return rules } -// rbacSyncIDPGroupsRole returns the Role + RoleBinding that grants rbacsync -// read access to the tigera-idp-groups ConfigMap in calico-system, its only -// namespaced dependency. -func (c *kubeControllersComponent) rbacSyncIDPGroupsRole() []client.Object { +// rbacSyncEnabled reports whether the rbacsync controller and its access should be +// rendered. Multi-tenant is excluded: the feature is force-disabled on the ui-apis side. +func rbacSyncEnabled(cfg *KubeControllersConfiguration) bool { + return cfg.Installation.Variant.IsEnterprise() && + cfg.RBACManagementEnabled && + !cfg.Tenant.MultiTenant() +} + +// rbacSyncNamespacedRole returns the Role + RoleBinding granting rbacsync read access to +// the two ConfigMaps in calico-system it depends on: tigera-idp-groups and the gate. +func (c *kubeControllersComponent) rbacSyncNamespacedRole() []client.Object { name := "calico-kube-controllers-rbac-sync" return []client.Object{ &rbacv1.Role{ @@ -730,6 +735,14 @@ func (c *kubeControllersComponent) rbacSyncIDPGroupsRole() []client.Object { ResourceNames: []string{"tigera-idp-groups"}, Verbs: []string{"get", "list", "watch"}, }, + { + // This cluster's copy of the gate; a managed cluster's is read over + // that cluster's own client. + APIGroups: []string{""}, + Resources: []string{"configmaps"}, + ResourceNames: []string{render.RBACManagementConfigMapName}, + Verbs: []string{"get", "list", "watch"}, + }, }, }, &rbacv1.RoleBinding{ diff --git a/pkg/render/kubecontrollers/kube-controllers_test.go b/pkg/render/kubecontrollers/kube-controllers_test.go index e2c484492d..eb52270857 100644 --- a/pkg/render/kubecontrollers/kube-controllers_test.go +++ b/pkg/render/kubecontrollers/kube-controllers_test.go @@ -448,12 +448,12 @@ var _ = Describe("kube-controllers rendering tests", func() { } }) - Context("RBAC management UI gate", func() { + Context("RBAC management UI", func() { BeforeEach(func() { instance.Variant = operatorv1.CalicoEnterprise }) - It("does not enable rbacsync when RBACManagementEnabled is false", func() { + It("does not enable rbacsync or grant its RBAC while the feature gate is off", func() { component := kubecontrollers.NewCalicoKubeControllers(&cfg) Expect(component.ResolveImages(nil)).To(BeNil()) resources, _ := component.Objects() @@ -467,7 +467,28 @@ var _ = Describe("kube-controllers rendering tests", func() { Expect(rtest.GetResource(resources, "calico-kube-controllers-rbac-sync", common.CalicoNamespace, "rbac.authorization.k8s.io", "v1", "Role")).To(BeNil()) }) - It("enables rbacsync and adds the controller's RBAC when RBACManagementEnabled is true", func() { + // Multi-tenant force-disables the feature on the ui-apis side, so the controller + // must not be wired up there even with the gate on. + It("does not enable rbacsync on a multi-tenant management cluster", func() { + cfg.RBACManagementEnabled = true + cfg.Tenant = &operatorv1.Tenant{ + ObjectMeta: metav1.ObjectMeta{Name: "tenantA", Namespace: "tenant-a"}, + Spec: operatorv1.TenantSpec{ID: "tenant-a"}, + } + component := kubecontrollers.NewCalicoKubeControllers(&cfg) + Expect(component.ResolveImages(nil)).To(BeNil()) + resources, _ := component.Objects() + + dp := rtest.GetResource(resources, kubecontrollers.KubeController, common.CalicoNamespace, "apps", "v1", "Deployment").(*appsv1.Deployment) + for _, e := range dp.Spec.Template.Spec.Containers[0].Env { + if e.Name == "ENABLED_CONTROLLERS" { + Expect(e.Value).NotTo(ContainSubstring("rbacsync")) + } + } + Expect(rtest.GetResource(resources, "calico-kube-controllers-rbac-sync", common.CalicoNamespace, "rbac.authorization.k8s.io", "v1", "Role")).To(BeNil()) + }) + + It("enables rbacsync and grants it read access to both ConfigMaps it depends on", func() { cfg.RBACManagementEnabled = true component := kubecontrollers.NewCalicoKubeControllers(&cfg) Expect(component.ResolveImages(nil)).To(BeNil()) @@ -486,6 +507,28 @@ var _ = Describe("kube-controllers rendering tests", func() { ResourceNames: []string{"tigera-idp-groups"}, Verbs: []string{"get", "list", "watch"}, }), "expected read-only access to tigera-idp-groups in calico-system") + Expect(nsRole.Rules).To(ContainElement(rbacv1.PolicyRule{ + APIGroups: []string{""}, + Resources: []string{"configmaps"}, + ResourceNames: []string{render.RBACManagementConfigMapName}, + Verbs: []string{"get", "list", "watch"}, + }), "expected read-only access to the feature gate in calico-system") + }) + + It("does not enable rbacsync or grant its RBAC for Calico", func() { + instance.Variant = operatorv1.Calico + cfg.RBACManagementEnabled = true + component := kubecontrollers.NewCalicoKubeControllers(&cfg) + Expect(component.ResolveImages(nil)).To(BeNil()) + resources, _ := component.Objects() + + dp := rtest.GetResource(resources, kubecontrollers.KubeController, common.CalicoNamespace, "apps", "v1", "Deployment").(*appsv1.Deployment) + envs := dp.Spec.Template.Spec.Containers[0].Env + Expect(envs).To(ContainElement(corev1.EnvVar{ + Name: "ENABLED_CONTROLLERS", Value: "node,loadbalancer", + })) + + Expect(rtest.GetResource(resources, "calico-kube-controllers-rbac-sync", common.CalicoNamespace, "rbac.authorization.k8s.io", "v1", "Role")).To(BeNil()) }) }) diff --git a/pkg/render/manager.go b/pkg/render/manager.go index 06512c4f16..82553ddc6a 100644 --- a/pkg/render/manager.go +++ b/pkg/render/manager.go @@ -83,6 +83,11 @@ const ( // Keep in sync with ui-apis rbacmanagement/idp LDAPConfigSecretName. RBACManagementLDAPConfigSecretName = "tigera-idp-ldap-config" + // 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" + RBACManagementConfigMapKey = "rbac-ui-enabled" + // The name of the TLS certificate used by Voltron to authenticate connections from managed // cluster clients talking to Linseed. VoltronLinseedTLS = "calico-voltron-linseed-tls" @@ -221,6 +226,9 @@ type ManagerConfiguration struct { Authentication *operatorv1.Authentication KibanaEnabled bool + // RBACManagementEnabled is the value of the rbac-ui-config gate for this cluster. + RBACManagementEnabled bool + // CACertCommonName is the CommonName from the CA certificate used for operator-managed certificates. // Passed to Voltron so it can identify the correct CA issuer public key. CACertCommonName string @@ -296,11 +304,11 @@ func (c *managerComponent) Objects() ([]client.Object, []client.Object) { objsToCreate = append(objsToCreate, managerClusterRoleBinding(c.cfg.Tenant, c.cfg.BindingNamespaces, c.cfg.OSSTenantNamespaces), - managerClusterRole(false, c.cfg.Installation.KubernetesProvider, c.cfg.Tenant, c.cfg.Manager.RBACManagementEnabled()), + managerClusterRole(false, c.cfg.Installation.KubernetesProvider, c.cfg.Tenant, c.rbacManagementUIActive()), c.managedClustersWatchRoleBinding(), ) objsToCreate = append(objsToCreate, c.managedClustersUpdateRBAC()...) - if c.cfg.Manager.RBACManagementEnabled() && !c.cfg.Tenant.MultiTenant() { + if c.rbacManagementUIActive() { objsToCreate = append(objsToCreate, c.rbacManagementUINamespacedRole()...) } if c.cfg.Tenant.MultiTenant() { @@ -777,7 +785,6 @@ func (c *managerComponent) managerUIAPIsContainer() corev1.Container { {Name: "LINSEED_CLIENT_KEY", Value: keyPath}, {Name: "ELASTIC_KIBANA_DISABLED", Value: strconv.FormatBool(c.cfg.Tenant.MultiTenant())}, {Name: "VOLTRON_URL", Value: ManagerService(c.cfg.Tenant)}, - {Name: "RBAC_UI_ENABLED", Value: strconv.FormatBool(c.cfg.Manager.RBACManagementEnabled() && !c.cfg.Tenant.MultiTenant())}, } // Determine the Linseed location. Use code default unless in multi-tenant mode, @@ -987,8 +994,8 @@ func (c *managerComponent) managedClustersUpdateRBAC() []client.Object { } // managerClusterRole returns a clusterrole that allows authn/authz review requests. -// When rbacManagementEnabled is true it also carries the RBAC management UI rules. -func managerClusterRole(managedCluster bool, kubernetesProvider operatorv1.Provider, tenant *operatorv1.Tenant, rbacManagementEnabled bool) *rbacv1.ClusterRole { +// When rbacManagementUIActive is true it also carries the RBAC management UI rules. +func managerClusterRole(managedCluster bool, kubernetesProvider operatorv1.Provider, tenant *operatorv1.Tenant, rbacManagementUIActive bool) *rbacv1.ClusterRole { // Different tenant types use different permission sets. name := ManagerClusterRole if tenant.ManagedClusterIsCalico() { @@ -1194,10 +1201,8 @@ func managerClusterRole(managedCluster bool, kubernetesProvider operatorv1.Provi }, } - // Not rendered on multi-tenant management clusters. Keep this condition in - // sync with the rbacManagementUINamespacedRole gate; the cluster rules and - // the namespaced grant are rendered together. - if rbacManagementEnabled && !tenant.MultiTenant() { + // Keep in sync with the rbacManagementUINamespacedRole gate. + if rbacManagementUIActive { cr.Rules = append(cr.Rules, rbacManagementUIRules()...) } @@ -1235,6 +1240,22 @@ func managerClusterRole(managedCluster bool, kubernetesProvider operatorv1.Provi return cr } +// 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 { + if cm == nil { + return false + } + enabled, err := strconv.ParseBool(cm.Data[RBACManagementConfigMapKey]) + return err == nil && enabled +} + +// rbacManagementUIActive reports whether this cluster should carry the RBAC management +// UI access. Multi-tenant is excluded: the feature is force-disabled on the ui-apis side. +func (c *managerComponent) rbacManagementUIActive() bool { + return c.cfg.RBACManagementEnabled && !c.cfg.Tenant.MultiTenant() +} + // rbacManagementUIRules returns the cluster-scoped rules the RBAC management // UI adds to calico-manager-role. Named-resource access is scoped separately // on rbacManagementUINamespacedRole. @@ -1261,9 +1282,8 @@ func (c *managerComponent) rbacManagementUINamespacedRole() []client.Object { ObjectMeta: metav1.ObjectMeta{Name: ManagerClusterRole, Namespace: common.CalicoNamespace}, Rules: []rbacv1.PolicyRule{ { - // create carries the object name in the request body, not the - // URL path, so RBAC cannot restrict it by resource name; it is - // scoped to this namespace instead. + // create cannot be restricted by resource name, so it is scoped to + // this namespace instead. APIGroups: []string{""}, Resources: []string{"configmaps", "secrets"}, Verbs: []string{"create"}, @@ -1280,6 +1300,13 @@ func (c *managerComponent) rbacManagementUINamespacedRole() []client.Object { ResourceNames: []string{"tigera-idp-groups"}, Verbs: []string{"get", "list", "watch", "update", "patch", "delete"}, }, + { + // The gate ui-apis watches; read-only, the value is the admin's. + APIGroups: []string{""}, + Resources: []string{"configmaps"}, + ResourceNames: []string{RBACManagementConfigMapName}, + Verbs: []string{"get", "list", "watch"}, + }, }, }, &rbacv1.RoleBinding{ @@ -1371,7 +1398,7 @@ func (c *managerComponent) managerCalicoSystemNetworkPolicy() *v3.NetworkPolicy }) } - if c.cfg.Manager.RBACManagementEnabled() && !c.cfg.Tenant.MultiTenant() && + if c.rbacManagementUIActive() && c.cfg.Authentication != nil && c.cfg.Authentication.Spec.LDAP != nil { // LDAP/AD egress (389, 636) for the RBAC-UI directory sync, gated on LDAP // being configured on the Authentication CR. The destination is scoped to diff --git a/pkg/render/manager_test.go b/pkg/render/manager_test.go index 5579011ec4..6343b3ebd1 100644 --- a/pkg/render/manager_test.go +++ b/pkg/render/manager_test.go @@ -31,7 +31,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" v3 "github.com/tigera/api/pkg/apis/projectcalico/v3" @@ -162,7 +161,6 @@ var _ = Describe("Tigera Secure Manager rendering tests", func() { {Name: "LINSEED_CLIENT_KEY", Value: "/internal-manager-tls/tls.key"}, {Name: "ELASTIC_KIBANA_DISABLED", Value: "false"}, {Name: "VOLTRON_URL", Value: render.ManagerService(nil)}, - {Name: "RBAC_UI_ENABLED", Value: "false"}, } Expect(uiAPIs.Env).To(Equal(uiAPIsExpectedEnvVars)) @@ -1745,79 +1743,41 @@ var _ = Describe("Tigera Secure Manager rendering tests", func() { } }) - rbacUIEnabledEnv := func(d *appsv1.Deployment) corev1.EnvVar { - for _, c := range d.Spec.Template.Spec.Containers { - if c.Name == render.UIAPIsName { - for _, e := range c.Env { - if e.Name == "RBAC_UI_ENABLED" { - return e - } - } - } - } - return corev1.EnvVar{} - } - - // Namespaced Role carries the create rule on ConfigMaps/Secrets — this - // is the stable presence check for the RBAC management UI gate. + // The create rule is unique to this Role, so it identifies the access without + // matching the whole rule set. nsCreateRule := rbacv1.PolicyRule{ APIGroups: []string{""}, Resources: []string{"configmaps", "secrets"}, Verbs: []string{"create"}, } - It("renders RBAC_UI_ENABLED=false and no namespaced RBAC UI role when rbac is unset", func() { - resources, _ := renderObjects(renderConfig{ - installation: installation, - ns: render.ManagerNamespace, - }) - d := rtest.GetResource(resources, render.ManagerDeploymentName, render.ManagerNamespace, appsv1.GroupName, "v1", "Deployment").(*appsv1.Deployment) - Expect(rbacUIEnabledEnv(d)).To(Equal(corev1.EnvVar{Name: "RBAC_UI_ENABLED", Value: "false"})) - - Expect(rtest.GetResource(resources, render.ManagerClusterRole, render.ManagerNamespace, rbacv1.GroupName, "v1", "Role")).To(BeNil()) - }) + // Read-only access to the gate ui-apis watches. + gateReadRule := rbacv1.PolicyRule{ + APIGroups: []string{""}, + Resources: []string{"configmaps"}, + ResourceNames: []string{render.RBACManagementConfigMapName}, + Verbs: []string{"get", "list", "watch"}, + } - It("renders RBAC_UI_ENABLED=false and no namespaced RBAC UI role when the Manager exists but rbacUI is unset", func() { + It("does not render the namespaced RBAC UI role while the feature gate is off", func() { resources, _ := renderObjects(renderConfig{ installation: installation, ns: render.ManagerNamespace, - manager: &operatorv1.Manager{Spec: operatorv1.ManagerSpec{}}, }) - d := rtest.GetResource(resources, render.ManagerDeploymentName, render.ManagerNamespace, appsv1.GroupName, "v1", "Deployment").(*appsv1.Deployment) - Expect(rbacUIEnabledEnv(d)).To(Equal(corev1.EnvVar{Name: "RBAC_UI_ENABLED", Value: "false"})) - Expect(rtest.GetResource(resources, render.ManagerClusterRole, render.ManagerNamespace, rbacv1.GroupName, "v1", "Role")).To(BeNil()) }) - It("renders RBAC_UI_ENABLED=true and the namespaced RBAC UI role when rbacUI.state is Enabled", func() { + It("renders the namespaced RBAC UI role with read access to the feature gate when enabled", func() { resources, _ := renderObjects(renderConfig{ - installation: installation, - ns: render.ManagerNamespace, - manager: &operatorv1.Manager{ - Spec: operatorv1.ManagerSpec{ - RBACUI: &operatorv1.RBACUI{State: ptr.To(operatorv1.RBACUIEnabled)}, - }, - }, + installation: installation, + ns: render.ManagerNamespace, + rbacManagementEnabled: true, }) - d := rtest.GetResource(resources, render.ManagerDeploymentName, render.ManagerNamespace, appsv1.GroupName, "v1", "Deployment").(*appsv1.Deployment) - Expect(rbacUIEnabledEnv(d)).To(Equal(corev1.EnvVar{Name: "RBAC_UI_ENABLED", Value: "true"})) - role := rtest.GetResource(resources, render.ManagerClusterRole, render.ManagerNamespace, rbacv1.GroupName, "v1", "Role").(*rbacv1.Role) Expect(role.Rules).To(ContainElement(nsCreateRule)) - }) - - It("renders RBAC_UI_ENABLED=false when rbacUI.state is Disabled", func() { - resources, _ := renderObjects(renderConfig{ - installation: installation, - ns: render.ManagerNamespace, - manager: &operatorv1.Manager{ - Spec: operatorv1.ManagerSpec{ - RBACUI: &operatorv1.RBACUI{State: ptr.To(operatorv1.RBACUIDisabled)}, - }, - }, - }) - d := rtest.GetResource(resources, render.ManagerDeploymentName, render.ManagerNamespace, appsv1.GroupName, "v1", "Deployment").(*appsv1.Deployment) - Expect(rbacUIEnabledEnv(d)).To(Equal(corev1.EnvVar{Name: "RBAC_UI_ENABLED", Value: "false"})) + // ui-apis keeps read access to the gate so it can observe the admin + // switching the feature back off. + Expect(role.Rules).To(ContainElement(gateReadRule)) }) It("does not add the manager-side RBAC rules in multi-tenant mode", func() { @@ -1825,6 +1785,8 @@ var _ = Describe("Tigera Secure Manager rendering tests", func() { installation: installation, ns: "tenant-a", bindingNamespaces: []string{"tenant-a"}, + // Enabled, to prove tenancy is what excludes these and not the gate. + rbacManagementEnabled: true, tenant: &operatorv1.Tenant{ ObjectMeta: metav1.ObjectMeta{Name: "tenantA", Namespace: "tenant-a"}, Spec: operatorv1.TenantSpec{ @@ -1832,13 +1794,18 @@ var _ = Describe("Tigera Secure Manager rendering tests", func() { ManagedClusterVariant: &operatorv1.Calico, }, }, - manager: &operatorv1.Manager{ - Spec: operatorv1.ManagerSpec{ - RBACUI: &operatorv1.RBACUI{State: ptr.To(operatorv1.RBACUIEnabled)}, - }, - }, }) Expect(rtest.GetResource(resources, render.ManagerClusterRole, "tenant-a", rbacv1.GroupName, "v1", "Role")).To(BeNil()) + + // The cluster rules and the namespaced grant are gated on the same + // condition and must be dropped together; assert both halves so the + // two gates cannot drift apart. + clusterRole := rtest.GetResource(resources, render.ManagerManagedCalicoClusterRole, "", rbacv1.GroupName, "v1", "ClusterRole").(*rbacv1.ClusterRole) + Expect(clusterRole.Rules).NotTo(ContainElement(rbacv1.PolicyRule{ + APIGroups: []string{"operator.tigera.io"}, + Resources: []string{"compliances"}, + Verbs: []string{"get"}, + })) }) Context("LDAP egress network policy gate", func() { @@ -1853,14 +1820,10 @@ var _ = Describe("Tigera Secure Manager rendering tests", func() { Ports: networkpolicy.Ports(389, 636), }, } - rbacUIManager := &operatorv1.Manager{ - Spec: operatorv1.ManagerSpec{RBACUI: &operatorv1.RBACUI{State: ptr.To(operatorv1.RBACUIEnabled)}}, - } - - It("adds an unscoped LDAP egress when RBAC UI and LDAP auth are configured but no host is set", func() { + It("adds an unscoped LDAP egress when LDAP auth is configured but no host is set", func() { resources, _ := renderObjects(renderConfig{ installation: installation, ns: render.ManagerNamespace, - manager: rbacUIManager, ldapConfigured: true, + rbacManagementEnabled: true, ldapConfigured: true, }) policy := testutils.GetCalicoSystemPolicyFromResources(policyName, resources) Expect(policy.Spec.Egress).To(ContainElement(ldapEgress)) @@ -1869,7 +1832,7 @@ var _ = Describe("Tigera Secure Manager rendering tests", func() { It("scopes LDAP egress to a Domains match when the LDAP host is a hostname", func() { resources, _ := renderObjects(renderConfig{ installation: installation, ns: render.ManagerNamespace, - manager: rbacUIManager, ldapConfigured: true, + rbacManagementEnabled: true, ldapConfigured: true, ldapHost: "ad.example.com:636", }) policy := testutils.GetCalicoSystemPolicyFromResources(policyName, resources) @@ -1888,7 +1851,7 @@ var _ = Describe("Tigera Secure Manager rendering tests", func() { It("scopes LDAP egress to a /32 Nets match when the LDAP host is an IPv4 address", func() { resources, _ := renderObjects(renderConfig{ installation: installation, ns: render.ManagerNamespace, - manager: rbacUIManager, ldapConfigured: true, + rbacManagementEnabled: true, ldapConfigured: true, ldapHost: "10.20.30.40:389", }) policy := testutils.GetCalicoSystemPolicyFromResources(policyName, resources) @@ -1907,7 +1870,7 @@ var _ = Describe("Tigera Secure Manager rendering tests", func() { It("scopes LDAP egress to a /128 Nets match when the LDAP host is an IPv6 address", func() { resources, _ := renderObjects(renderConfig{ installation: installation, ns: render.ManagerNamespace, - manager: rbacUIManager, ldapConfigured: true, + rbacManagementEnabled: true, ldapConfigured: true, ldapHost: "[2001:db8::1]:636", }) policy := testutils.GetCalicoSystemPolicyFromResources(policyName, resources) @@ -1923,16 +1886,16 @@ var _ = Describe("Tigera Secure Manager rendering tests", func() { Expect(policy.Spec.Egress).NotTo(ContainElement(ldapEgress)) }) - It("omits LDAP egress when LDAP auth is not configured, even with RBAC UI enabled", func() { + It("omits LDAP egress when LDAP auth is not configured", func() { resources, _ := renderObjects(renderConfig{ installation: installation, ns: render.ManagerNamespace, - manager: rbacUIManager, ldapConfigured: false, + rbacManagementEnabled: true, ldapConfigured: false, }) policy := testutils.GetCalicoSystemPolicyFromResources(policyName, resources) Expect(policy.Spec.Egress).NotTo(ContainElement(ldapEgress)) }) - It("omits LDAP egress when LDAP auth is configured but RBAC UI is disabled", func() { + It("omits LDAP egress when the feature gate is off, even with LDAP configured", func() { resources, _ := renderObjects(renderConfig{ installation: installation, ns: render.ManagerNamespace, ldapConfigured: true, @@ -1941,7 +1904,7 @@ var _ = Describe("Tigera Secure Manager rendering tests", func() { Expect(policy.Spec.Egress).NotTo(ContainElement(ldapEgress)) }) - It("omits LDAP egress in multi-tenant mode even with RBAC UI enabled and LDAP auth configured", func() { + It("omits LDAP egress in multi-tenant mode even with LDAP auth configured", func() { resources, _ := renderObjects(renderConfig{ installation: installation, ns: "tenant-a", @@ -1953,7 +1916,6 @@ var _ = Describe("Tigera Secure Manager rendering tests", func() { ManagedClusterVariant: &operatorv1.Calico, }, }, - manager: rbacUIManager, ldapConfigured: true, }) policy := testutils.GetCalicoSystemPolicyFromResources( @@ -1980,8 +1942,10 @@ type renderConfig struct { externalElastic bool // ldapConfigured, when true, sets Authentication.spec.ldap (gating the RBAC-UI // LDAP egress rule); ldapHost sets Authentication.spec.ldap.host (scoping it). - ldapConfigured bool - ldapHost string + ldapConfigured bool + ldapHost string + // rbacManagementEnabled mirrors the admin's rbac-ui-config value. + rbacManagementEnabled bool cloud bool voltronMetricsEnabled bool cloudResources render.ManagerCloudResources @@ -2057,6 +2021,7 @@ func renderObjects(roc renderConfig) ([]client.Object, []client.Object) { Manager: roc.manager, ExternalElastic: roc.externalElastic, CACertCommonName: certificateManager.CACertCommonName(), + RBACManagementEnabled: roc.rbacManagementEnabled, Cloud: roc.cloud, CloudResources: roc.cloudResources, } @@ -2079,3 +2044,20 @@ func renderObjects(roc renderConfig) ([]client.Object, []client.Object) { resourcesToCreate, resourcesToDelete := component.Objects() return resourcesToCreate, resourcesToDelete } + +// The gate is hand-edited by an admin, so the parser has to be forgiving about +// spelling and strict about everything else: anything it cannot read as an explicit +// true leaves the feature — and all of its access — switched off. +var _ = DescribeTable("RBACManagementEnabled", + func(cm *corev1.ConfigMap, expected bool) { + Expect(render.RBACManagementEnabled(cm)).To(Equal(expected)) + }, + Entry("nil ConfigMap (never created, or deleted)", nil, false), + Entry("missing key", &corev1.ConfigMap{Data: map[string]string{}}, false), + Entry("explicitly disabled", &corev1.ConfigMap{Data: map[string]string{render.RBACManagementConfigMapKey: "false"}}, false), + Entry("enabled", &corev1.ConfigMap{Data: map[string]string{render.RBACManagementConfigMapKey: "true"}}, true), + Entry("enabled, capitalised", &corev1.ConfigMap{Data: map[string]string{render.RBACManagementConfigMapKey: "True"}}, true), + Entry("enabled as 1", &corev1.ConfigMap{Data: map[string]string{render.RBACManagementConfigMapKey: "1"}}, true), + Entry("unparsable value stays off", &corev1.ConfigMap{Data: map[string]string{render.RBACManagementConfigMapKey: "yes please"}}, false), + Entry("empty value stays off", &corev1.ConfigMap{Data: map[string]string{render.RBACManagementConfigMapKey: ""}}, false), +)