From 8ebe39096c4dc6c993f0116b066f70cec7e7d29d Mon Sep 17 00:00:00 2001 From: vara Date: Tue, 1 Sep 2026 11:30:38 -0700 Subject: [PATCH 1/4] PMREQ-821: Hold gateway access grants with a finalizer during teardown The access Role/RoleBinding were protected only by delete ordering: emitted last so the operator kept its own write grant while deleting the gateway resources. A failed or reordered delete could strip the grant first and orphan them. Both grants now carry operator.tigera.io/gateway-rbac-finalizer. Teardown deletes only mark them; a sweep in the uigateway helper clears the finalizer once the Gateway, HTTPRoute, Backend and ReferenceGrant in that namespace are gone. The sweep lists grants by the gateway label, so a grant that outlives its Gateway is still found and never wedges. Also adds BackendName, ReferenceGrantName and ListenerName helpers so generated names have one definition each. Follow-on from #5146 review. --- pkg/render/gateway/component.go | 55 +++++++++++------ pkg/render/gateway/component_test.go | 11 ++++ pkg/uigateway/uigateway.go | 91 +++++++++++++++++++++++++++- pkg/uigateway/uigateway_test.go | 60 ++++++++++++++++++ 4 files changed, 198 insertions(+), 19 deletions(-) diff --git a/pkg/render/gateway/component.go b/pkg/render/gateway/component.go index 0e53ccb55b..48705dc1fa 100644 --- a/pkg/render/gateway/component.go +++ b/pkg/render/gateway/component.go @@ -49,6 +49,12 @@ const ( // once no labeled Gateway from any component remains, so components that // share a namespace never delete it out from under each other. GatewayNamespaceLabel = "operator.tigera.io/gateway-namespace" + + // RBACFinalizer holds a gateway access Role and RoleBinding until the + // gateway resources they authorize deleting are gone, so teardown cannot + // strip the operator's own write grant first and orphan them. The + // uigateway helper removes it once those resources no longer exist. + RBACFinalizer = "operator.tigera.io/gateway-rbac-finalizer" ) // Configuration holds everything the shared gateway component needs to render @@ -162,6 +168,16 @@ func GatewayName(prefix string) string { return prefix + "-gateway" } // RouteName is the HTTPRoute object name for a component's resource prefix. func RouteName(prefix string) string { return prefix + "-route" } +// BackendName is the Envoy Gateway Backend object name for a component's resource prefix. +func BackendName(prefix string) string { return prefix + "-backend" } + +// ReferenceGrantName is the ReferenceGrant object name for a component's resource prefix. +func ReferenceGrantName(prefix string) string { return prefix + "-allow-gateway" } + +// ListenerName is the Gateway's HTTPS listener name for a component's resource +// prefix. The HTTPRoute's parentRef sectionName must match it to attach. +func ListenerName(prefix string) string { return prefix + "-https" } + // gatewayAccess grants the operator the write permissions needed in the gateway namespace; the // cluster-wide ClusterRole keeps the reads. func (c *gatewayComponent) gatewayAccess() (*rbacv1.Role, *rbacv1.RoleBinding) { @@ -191,22 +207,25 @@ func (c *gatewayComponent) backendAccess() (*rbacv1.Role, *rbacv1.RoleBinding) { } // access builds a Role with rules and a RoleBinding tying it to the operator's -// own ServiceAccount, the identity that renders the gateway resources. +// own ServiceAccount, the identity that renders the gateway resources. Both +// carry RBACFinalizer so the grant outlives the resources it covers. func (c *gatewayComponent) access(name, namespace string, rules []rbacv1.PolicyRule) (*rbacv1.Role, *rbacv1.RoleBinding) { return &rbacv1.Role{ TypeMeta: metav1.TypeMeta{Kind: "Role", APIVersion: "rbac.authorization.k8s.io/v1"}, ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: namespace, - Labels: map[string]string{GatewayLabel: c.cfg.ResourcePrefix}, + Name: name, + Namespace: namespace, + Labels: map[string]string{GatewayLabel: c.cfg.ResourcePrefix}, + Finalizers: []string{RBACFinalizer}, }, Rules: rules, }, &rbacv1.RoleBinding{ TypeMeta: metav1.TypeMeta{Kind: "RoleBinding", APIVersion: "rbac.authorization.k8s.io/v1"}, ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: namespace, - Labels: map[string]string{GatewayLabel: c.cfg.ResourcePrefix}, + Name: name, + Namespace: namespace, + Labels: map[string]string{GatewayLabel: c.cfg.ResourcePrefix}, + Finalizers: []string{RBACFinalizer}, }, RoleRef: rbacv1.RoleRef{ APIGroup: "rbac.authorization.k8s.io", @@ -230,7 +249,7 @@ func (c *gatewayComponent) tlsSecret() *corev1.Secret { } func (c *gatewayComponent) gateway() *gapi.Gateway { - listenerName := gapi.SectionName(c.cfg.ResourcePrefix + "-https") + listenerName := gapi.SectionName(ListenerName(c.cfg.ResourcePrefix)) hostname := gapi.Hostname(c.cfg.Hostname) tlsSecretName := c.cfg.TLSKeyPair.GetName() @@ -272,8 +291,8 @@ func (c *gatewayComponent) gateway() *gapi.Gateway { func (c *gatewayComponent) httpRoute() *gapi.HTTPRoute { gatewayName := gapi.ObjectName(GatewayName(c.cfg.ResourcePrefix)) - sectionName := gapi.SectionName(c.cfg.ResourcePrefix + "-https") - backendName := gapi.ObjectName(c.cfg.ResourcePrefix + "-backend") + sectionName := gapi.SectionName(ListenerName(c.cfg.ResourcePrefix)) + backendName := gapi.ObjectName(BackendName(c.cfg.ResourcePrefix)) backendNS := gapi.Namespace(c.cfg.BackendNamespace) group := gapi.Group(EnvoyGatewayGroup) @@ -325,7 +344,7 @@ func (c *gatewayComponent) backend() *envoyapi.Backend { return &envoyapi.Backend{ TypeMeta: metav1.TypeMeta{Kind: BackendKind, APIVersion: "gateway.envoyproxy.io/v1alpha1"}, ObjectMeta: metav1.ObjectMeta{ - Name: c.cfg.ResourcePrefix + "-backend", + Name: BackendName(c.cfg.ResourcePrefix), Namespace: c.cfg.BackendNamespace, }, Spec: envoyapi.BackendSpec{ @@ -356,12 +375,12 @@ func (c *gatewayComponent) backend() *envoyapi.Backend { // and CRDManagementPreferExisting leaves it alone). v1beta1 is still the // storage version as of Gateway API v1.6. func (c *gatewayComponent) referenceGrant() *gapiv1b1.ReferenceGrant { - backendName := gapi.ObjectName(c.cfg.ResourcePrefix + "-backend") + backendName := gapi.ObjectName(BackendName(c.cfg.ResourcePrefix)) return &gapiv1b1.ReferenceGrant{ TypeMeta: metav1.TypeMeta{Kind: "ReferenceGrant", APIVersion: "gateway.networking.k8s.io/v1beta1"}, ObjectMeta: metav1.ObjectMeta{ - Name: c.cfg.ResourcePrefix + "-allow-gateway", + Name: ReferenceGrantName(c.cfg.ResourcePrefix), Namespace: c.cfg.BackendNamespace, }, Spec: gapiv1b1.ReferenceGrantSpec{ @@ -506,11 +525,11 @@ func (c *gatewayDeletionComponent) Objects() (objsToCreate, objsToDelete []clien objs = append(objs, &envoyapi.Backend{ TypeMeta: metav1.TypeMeta{Kind: BackendKind, APIVersion: "gateway.envoyproxy.io/v1alpha1"}, - ObjectMeta: metav1.ObjectMeta{Name: prefix + "-backend", Namespace: bkNS}, + ObjectMeta: metav1.ObjectMeta{Name: BackendName(prefix), Namespace: bkNS}, }, &gapiv1b1.ReferenceGrant{ TypeMeta: metav1.TypeMeta{Kind: "ReferenceGrant", APIVersion: "gateway.networking.k8s.io/v1beta1"}, - ObjectMeta: metav1.ObjectMeta{Name: prefix + "-allow-gateway", Namespace: bkNS}, + ObjectMeta: metav1.ObjectMeta{Name: ReferenceGrantName(prefix), Namespace: bkNS}, }, ) } @@ -536,8 +555,10 @@ func (c *gatewayDeletionComponent) Objects() (objsToCreate, objsToDelete []clien ObjectMeta: metav1.ObjectMeta{Name: GatewayName(prefix), Namespace: staleNS}, }) - // The grants go after the resources they permit deleting. The backend grant - // is dropped only by the backend namespace's own component. + // The grants carry RBACFinalizer, so these deletes only mark them; they + // hold the operator's write access until the uigateway helper clears the + // finalizer once the resources above are gone. The backend grant is + // dropped only by the backend namespace's own component. objs = append(objs, c.roleBinding(staleNS, gatewayAccessSuffix), c.role(staleNS, gatewayAccessSuffix)) if staleNS == bkNS && c.cfg.TargetNamespace == "" { objs = append(objs, c.roleBinding(bkNS, backendAccessSuffix), c.role(bkNS, backendAccessSuffix)) diff --git a/pkg/render/gateway/component_test.go b/pkg/render/gateway/component_test.go index b1c2b8d977..8be7335d16 100644 --- a/pkg/render/gateway/component_test.go +++ b/pkg/render/gateway/component_test.go @@ -110,6 +110,17 @@ var _ = Describe("Gateway component render", func() { Expect(gw.Labels).To(HaveKeyWithValue(gateway.GatewayLabel, prefix)) }) + It("stamps the access grants with the cleanup finalizer", func() { + for _, name := range []string{prefix + "-ingressgateway-access", prefix + "-ingressgateway-backend-access"} { + role := findObject[*rbacv1.Role](toCreate, name, bkNS) + Expect(role).NotTo(BeNil(), "expected access Role "+name) + Expect(role.Finalizers).To(ConsistOf(gateway.RBACFinalizer)) + binding := findObject[*rbacv1.RoleBinding](toCreate, name, bkNS) + Expect(binding).NotTo(BeNil(), "expected access RoleBinding "+name) + Expect(binding.Finalizers).To(ConsistOf(gateway.RBACFinalizer)) + } + }) + It("renders a Gateway with the correct listener", func() { gw := findObject[*gapi.Gateway](toCreate, prefix+"-gateway", gwNS) Expect(gw).NotTo(BeNil()) diff --git a/pkg/uigateway/uigateway.go b/pkg/uigateway/uigateway.go index 90e0af1f40..21cba1c7ea 100644 --- a/pkg/uigateway/uigateway.go +++ b/pkg/uigateway/uigateway.go @@ -25,8 +25,10 @@ import ( "fmt" "slices" + envoyapi "github.com/envoyproxy/gateway/api/v1alpha1" "github.com/go-logr/logr" corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" "k8s.io/apimachinery/pkg/api/errors" apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -36,6 +38,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/predicate" gapi "sigs.k8s.io/gateway-api/apis/v1" + gapiv1b1 "sigs.k8s.io/gateway-api/apis/v1beta1" operatorv1 "github.com/tigera/operator/api/v1" "github.com/tigera/operator/pkg/common" @@ -205,7 +208,12 @@ func (h *Helper) Namespaces(ctx context.Context) ([]string, error) { // StaleComponents returns deletion components for every labeled Gateway // outside the desired namespace — leftovers of a gatewayNamespace change. +// It also clears access finalizers whose resources are gone, so a stale +// namespace's grant finishes deleting. func (h *Helper) StaleComponents(ctx context.Context, desiredNS string) ([]render.Component, error) { + if err := h.clearRBACFinalizers(ctx); err != nil { + return nil, err + } strays, err := h.Namespaces(ctx) if err != nil { return nil, err @@ -234,9 +242,14 @@ func (h *Helper) StaleComponents(ctx context.Context, desiredNS string) ([]rende // Teardown returns deletion components for every labeled Gateway namespace, // plus the backend namespace, which holds the Backend and ReferenceGrant. -// No labeled Gateway means nothing to do: the Gateway is rendered first, so -// nothing else can exist without one. +// It first clears access finalizers whose resources are gone — a grant can +// outlive its Gateway, so this runs even when no labeled Gateway remains. +// After that, no labeled Gateway means nothing to do: the Gateway is +// rendered first, so nothing else can exist without one. func (h *Helper) Teardown(ctx context.Context) ([]render.Component, error) { + if err := h.clearRBACFinalizers(ctx); err != nil { + return nil, err + } namespaces, err := h.Namespaces(ctx) if err != nil { return nil, err @@ -265,6 +278,80 @@ func (h *Helper) Teardown(ctx context.Context) ([]render.Component, error) { return components, nil } +// clearRBACFinalizers removes RBACFinalizer from every labeled access +// Role and RoleBinding that is marked for deletion, once the gateway +// resources it covers are gone. The finalizer keeps the operator's write +// grant in place until then, so teardown does not depend on delete order. +func (h *Helper) clearRBACFinalizers(ctx context.Context) error { + byLabel := client.MatchingLabels{rgateway.GatewayLabel: h.cfg.ResourcePrefix} + roles := &rbacv1.RoleList{} + if err := h.cli.List(ctx, roles, byLabel); err != nil { + return err + } + bindings := &rbacv1.RoleBindingList{} + if err := h.cli.List(ctx, bindings, byLabel); err != nil { + return err + } + var grants []client.Object + for i := range roles.Items { + grants = append(grants, &roles.Items[i]) + } + for i := range bindings.Items { + grants = append(grants, &bindings.Items[i]) + } + + resourcesGone := map[string]bool{} + for _, grant := range grants { + if grant.GetDeletionTimestamp().IsZero() || !slices.Contains(grant.GetFinalizers(), rgateway.RBACFinalizer) { + continue + } + ns := grant.GetNamespace() + gone, checked := resourcesGone[ns] + if !checked { + var err error + if gone, err = h.accessResourcesGone(ctx, ns); err != nil { + return err + } + resourcesGone[ns] = gone + } + if !gone { + continue + } + patchFrom := client.MergeFrom(grant.DeepCopyObject().(client.Object)) + grant.SetFinalizers(slices.DeleteFunc(grant.GetFinalizers(), func(f string) bool { + return f == rgateway.RBACFinalizer + })) + if err := h.cli.Patch(ctx, grant, patchFrom); err != nil && !errors.IsNotFound(err) { + return err + } + } + return nil +} + +// accessResourcesGone reports whether none of the component's gateway +// resources remain in the namespace — the point at which an access grant +// there has nothing left to cover. A kind the cluster does not serve counts +// as gone. +func (h *Helper) accessResourcesGone(ctx context.Context, namespace string) (bool, error) { + prefix := h.cfg.ResourcePrefix + for name, obj := range map[string]client.Object{ + rgateway.GatewayName(prefix): &gapi.Gateway{}, + rgateway.RouteName(prefix): &gapi.HTTPRoute{}, + rgateway.BackendName(prefix): &envoyapi.Backend{}, + rgateway.ReferenceGrantName(prefix): &gapiv1b1.ReferenceGrant{}, + } { + err := h.cli.Get(ctx, types.NamespacedName{Name: name, Namespace: namespace}, obj) + if err == nil { + return false, nil + } + var noMatch *apimeta.NoKindMatchError + if !errors.IsNotFound(err) && !stderrors.As(err, &noMatch) { + return false, err + } + } + return true, nil +} + // UnhealthyReason returns why the Gateway or HTTPRoute is not ready, or "" // when both are healthy. The caller degrades and requeues; nothing is torn // down. NotFound counts too: the cache may not yet hold what this reconcile diff --git a/pkg/uigateway/uigateway_test.go b/pkg/uigateway/uigateway_test.go index 8dcce09fab..362b8d0fb2 100644 --- a/pkg/uigateway/uigateway_test.go +++ b/pkg/uigateway/uigateway_test.go @@ -22,6 +22,8 @@ import ( envoyapi "github.com/envoyproxy/gateway/api/v1alpha1" corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + kerrors "k8s.io/apimachinery/pkg/api/errors" apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -256,6 +258,64 @@ var _ = Describe("Cleanup helpers", func() { }) }) + Describe("access grant finalizers", func() { + accessName := prefix + "-ingressgateway-access" + + accessGrant := func(ns string) (*rbacv1.Role, *rbacv1.RoleBinding) { + objMeta := func() metav1.ObjectMeta { + return metav1.ObjectMeta{ + Name: accessName, + Namespace: ns, + Labels: map[string]string{rgateway.GatewayLabel: prefix}, + Finalizers: []string{rgateway.RBACFinalizer}, + } + } + return &rbacv1.Role{ObjectMeta: objMeta()}, &rbacv1.RoleBinding{ObjectMeta: objMeta()} + } + + It("completes a marked grant's deletion once the gateway resources are gone", func() { + role, binding := accessGrant("ns-a") + build(role, binding) + Expect(cli.Delete(ctx, role)).To(Succeed()) + Expect(cli.Delete(ctx, binding)).To(Succeed()) + + _, err := h.Teardown(ctx) + Expect(err).NotTo(HaveOccurred()) + + Expect(kerrors.IsNotFound(cli.Get(ctx, types.NamespacedName{Name: accessName, Namespace: "ns-a"}, &rbacv1.Role{}))).To(BeTrue(), + "the finalizer should be cleared so the pending delete finishes") + Expect(kerrors.IsNotFound(cli.Get(ctx, types.NamespacedName{Name: accessName, Namespace: "ns-a"}, &rbacv1.RoleBinding{}))).To(BeTrue()) + }) + + It("holds a marked grant while its Gateway remains", func() { + role, binding := accessGrant("ns-a") + build(role, binding, labeledGateway(rgateway.GatewayName(prefix), "ns-a")) + Expect(cli.Delete(ctx, role)).To(Succeed()) + Expect(cli.Delete(ctx, binding)).To(Succeed()) + + _, err := h.Teardown(ctx) + Expect(err).NotTo(HaveOccurred()) + + got := &rbacv1.Role{} + Expect(cli.Get(ctx, types.NamespacedName{Name: accessName, Namespace: "ns-a"}, got)).To(Succeed()) + Expect(got.Finalizers).To(ContainElement(rgateway.RBACFinalizer), + "the grant must keep the operator's write access until the Gateway is gone") + }) + + It("leaves a grant that is not marked for deletion alone", func() { + role, binding := accessGrant("ns-a") + build(role, binding) + + _, err := h.Teardown(ctx) + Expect(err).NotTo(HaveOccurred()) + + got := &rbacv1.Role{} + Expect(cli.Get(ctx, types.NamespacedName{Name: accessName, Namespace: "ns-a"}, got)).To(Succeed()) + Expect(got.DeletionTimestamp.IsZero()).To(BeTrue()) + Expect(got.Finalizers).To(ContainElement(rgateway.RBACFinalizer)) + }) + }) + Describe("Teardown", func() { It("tears down every labeled Gateway's namespace plus the backend namespace", func() { build(labeledGateway(prefix+"-gateway", "ns-a")) From 7356544fbe55dbb392455e1748caf2c91c031991 Mon Sep 17 00:00:00 2001 From: vara Date: Tue, 1 Sep 2026 12:32:40 -0700 Subject: [PATCH 2/4] PMREQ-821: Supply Enterprise gateway objects through a UIGateway extension The WAF filter ServiceAccount/RoleBinding reached the gateway render as a raw ExtraProxyObjects pass-through: the manager controller built them by hand and the create and delete paths each carried the field. A UIGatewayExtension in the variant extension set replaces that. The Enterprise implementation returns the WAF objects for the Manager prefix and nothing for other components, so behavior is unchanged on every variant. The uigateway Helper takes the extension and feeds one proxyObjects() result to the render and both deletion paths, removing the mirrored wiring. ExtraProxyObjects is gone from the shared Config. Follow-on from #5146 review. --- pkg/controller/whisker/controller.go | 6 ++- .../controller/manager/manager_controller.go | 5 +- pkg/enterprise/register.go | 2 + pkg/enterprise/uigateway/uigateway.go | 27 ++++++++-- pkg/extensions/extensions.go | 8 +++ pkg/extensions/uigateway.go | 33 +++++++++++++ pkg/uigateway/uigateway.go | 26 ++++++---- pkg/uigateway/uigateway_test.go | 49 ++++++++++++++++--- 8 files changed, 133 insertions(+), 23 deletions(-) create mode 100644 pkg/extensions/uigateway.go diff --git a/pkg/controller/whisker/controller.go b/pkg/controller/whisker/controller.go index c2f6268354..d970d33e23 100644 --- a/pkg/controller/whisker/controller.go +++ b/pkg/controller/whisker/controller.go @@ -142,6 +142,7 @@ func newReconciler( clusterDomain: opts.ClusterDomain, variant: opts.Variant, ext: opts.Extensions.Whisker(), + gwExt: opts.Extensions.UIGateway(), } c.status.Run(opts.ShutdownContext) return c @@ -158,6 +159,7 @@ type Reconciler struct { clusterDomain string variant operatorv1.ProductVariant ext extensions.WhiskerExtension + gwExt extensions.UIGatewayExtension } // Reconcile reads that state of the cluster for a Whisker object and makes changes based on the @@ -177,7 +179,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, request reconcile.Request) ( // Gateway objects are garbage-collected with the CR, but a namespace the // operator created for them has no owner reference and must be torn down here. - gwHelper := uigateway.NewHelper(r.cli, uigateway.Config{ + gwHelper := uigateway.NewHelper(r.cli, r.gwExt, uigateway.Config{ ResourcePrefix: whisker.GatewayResourcePrefix, TLSSecretName: whisker.GatewayTLSSecretName, BackendNamespace: whisker.WhiskerNamespace, @@ -305,7 +307,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, request reconcile.Request) ( cfg.ClusterID = clusterInfo.Spec.ClusterGUID } - gwHelper := uigateway.NewHelper(r.cli, uigateway.Config{ + gwHelper := uigateway.NewHelper(r.cli, r.gwExt, uigateway.Config{ ResourcePrefix: whisker.GatewayResourcePrefix, TLSSecretName: whisker.GatewayTLSSecretName, BackendNamespace: whisker.WhiskerNamespace, diff --git a/pkg/enterprise/controller/manager/manager_controller.go b/pkg/enterprise/controller/manager/manager_controller.go index 92d2b0610d..0a19d0fe18 100644 --- a/pkg/enterprise/controller/manager/manager_controller.go +++ b/pkg/enterprise/controller/manager/manager_controller.go @@ -753,14 +753,13 @@ func (r *ReconcileManager) Reconcile(ctx context.Context, request reconcile.Requ // observed on the cluster. var gatewayComponents []render.Component var gatewayTLSKeyPair certificatemanagement.KeyPairInterface - gwHelper := uigateway.NewHelper(r.client, uigateway.Config{ + gwHelper := uigateway.NewHelper(r.client, r.opts.Extensions.UIGateway(), uigateway.Config{ ResourcePrefix: ManagerGatewayResourcePrefix, TLSSecretName: ManagerGatewayTLSSecretName, BackendNamespace: helper.InstallNamespace(), BackendServiceName: render.ManagerServiceName, BackendPort: render.ManagerPort, BackendCABundleConfigMapName: certificatemanagement.TrustedCertConfigMapName, - ExtraProxyObjects: euigateway.ProxyObjects(helper.InstallNamespace()), Provider: r.opts.DetectedProvider, Azure: installationSpec.Azure, }) @@ -941,7 +940,7 @@ func (r *ReconcileManager) resolveAdditionalTunnelCert( const ( ManagerGatewayTLSSecretName = "calico-manager-gateway-tls" - ManagerGatewayResourcePrefix = "calico-manager" + ManagerGatewayResourcePrefix = euigateway.ManagerGatewayResourcePrefix ) // resolveGateway validates the Manager spec.ingressGateway configuration, resolves the diff --git a/pkg/enterprise/register.go b/pkg/enterprise/register.go index c7679cd3b9..15f882d01d 100644 --- a/pkg/enterprise/register.go +++ b/pkg/enterprise/register.go @@ -32,6 +32,7 @@ import ( "github.com/tigera/operator/pkg/enterprise/istio" eoptions "github.com/tigera/operator/pkg/enterprise/options" "github.com/tigera/operator/pkg/enterprise/tiers" + "github.com/tigera/operator/pkg/enterprise/uigateway" "github.com/tigera/operator/pkg/enterprise/whisker" "github.com/tigera/operator/pkg/enterprise/windows" "github.com/tigera/operator/pkg/extensions" @@ -85,6 +86,7 @@ func New(variant operatorv1.ProductVariant, o eoptions.Options) extensions.Exten set.Goldmane = goldmane.New(variant) set.Whisker = whisker.New(variant) set.GatewayAPI = gatewayapi.New(variant) + set.UIGateway = uigateway.New() case variant == operatorv1.Calico: // Clean up what a prior Enterprise installation left behind. set.APIServer = apiserver.CalicoCleanup{} diff --git a/pkg/enterprise/uigateway/uigateway.go b/pkg/enterprise/uigateway/uigateway.go index b1878c1d74..01bc492567 100644 --- a/pkg/enterprise/uigateway/uigateway.go +++ b/pkg/enterprise/uigateway/uigateway.go @@ -13,17 +13,38 @@ // limitations under the License. // Package uigateway layers the Calico Enterprise pieces onto a UI component's -// ingress gateway. The common gateway code carries no variant knowledge; an -// Enterprise controller passes what this package builds, and an OSS controller -// passes nothing. +// ingress gateway. The common gateway code carries no variant knowledge; it +// reaches this package only through the registered UIGateway extension. package uigateway import ( "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/tigera/operator/pkg/extensions" rgatewayapi "github.com/tigera/operator/pkg/render/gatewayapi" ) +// ManagerGatewayResourcePrefix names the Manager's CIG resources; the OSS +// counterpart is whisker.GatewayResourcePrefix. +const ManagerGatewayResourcePrefix = "calico-manager" + +// Extension is the Calico Enterprise behavior for UI ingress gateways. +type Extension struct{} + +var _ extensions.UIGatewayExtension = Extension{} + +// New returns the UI gateway extension for Calico Enterprise. +func New() Extension { return Extension{} } + +// ProxyObjects returns the WAF filter objects for the Manager gateway; other +// components get nothing. +func (Extension) ProxyObjects(resourcePrefix, namespace string) []client.Object { + if resourcePrefix != ManagerGatewayResourcePrefix { + return nil + } + return ProxyObjects(namespace) +} + // ProxyObjects returns the Enterprise-only objects that run beside a UI // gateway's Envoy proxy in the backend namespace: the WAF HTTP filter's // ServiceAccount and the RoleBinding giving it Gateway API reads. diff --git a/pkg/extensions/extensions.go b/pkg/extensions/extensions.go index 32be064a75..8e1fe266e9 100644 --- a/pkg/extensions/extensions.go +++ b/pkg/extensions/extensions.go @@ -27,6 +27,7 @@ type Set struct { Goldmane GoldmaneExtension Whisker WhiskerExtension GatewayAPI GatewayAPIExtension + UIGateway UIGatewayExtension // Startup is the variant's hook into operator startup rather than into a controller. Startup StartupExtension @@ -114,6 +115,13 @@ func (e Extensions) GatewayAPI() GatewayAPIExtension { return e.set.GatewayAPI } +func (e Extensions) UIGateway() UIGatewayExtension { + if e.set.UIGateway == nil { + return noopUIGateway{} + } + return e.set.UIGateway +} + func (e Extensions) Startup() StartupExtension { if e.set.Startup == nil { return noopStartup{} diff --git a/pkg/extensions/uigateway.go b/pkg/extensions/uigateway.go new file mode 100644 index 0000000000..8a24df08db --- /dev/null +++ b/pkg/extensions/uigateway.go @@ -0,0 +1,33 @@ +// 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 extensions + +import ( + "sigs.k8s.io/controller-runtime/pkg/client" +) + +// UIGatewayExtension is the variant's hook into the ingress gateway a UI +// component (Manager, Whisker) exposes itself through. +type UIGatewayExtension interface { + // ProxyObjects returns the variant's objects rendered beside the named + // component's gateway proxy in the backend namespace, or nil when the + // variant adds none. + ProxyObjects(resourcePrefix, namespace string) []client.Object +} + +// noopUIGateway runs the core operator's behavior unchanged. +type noopUIGateway struct{} + +func (noopUIGateway) ProxyObjects(string, string) []client.Object { return nil } diff --git a/pkg/uigateway/uigateway.go b/pkg/uigateway/uigateway.go index 21cba1c7ea..082540c8b6 100644 --- a/pkg/uigateway/uigateway.go +++ b/pkg/uigateway/uigateway.go @@ -45,6 +45,7 @@ import ( "github.com/tigera/operator/pkg/controller/gatewayapi" "github.com/tigera/operator/pkg/controller/utils" "github.com/tigera/operator/pkg/ctrlruntime" + "github.com/tigera/operator/pkg/extensions" "github.com/tigera/operator/pkg/render" rgateway "github.com/tigera/operator/pkg/render/gateway" "github.com/tigera/operator/pkg/tls/certificatemanagement" @@ -74,9 +75,6 @@ type Config struct { // it for a component that streams, where the default would cut the stream. RouteRequestTimeout *string - // ExtraProxyObjects are the variant's additions beside the proxy, or nil. - ExtraProxyObjects []client.Object - Provider operatorv1.Provider // Azure carries Installation.Azure so the gateway namespace gets the same @@ -87,12 +85,22 @@ type Config struct { // Helper renders and cleans up one UI component's gateway resources. type Helper struct { cli client.Client + ext extensions.UIGatewayExtension cfg Config } -// NewHelper returns the helper for one UI component's gateway. -func NewHelper(cli client.Client, cfg Config) *Helper { - return &Helper{cli: cli, cfg: cfg} +// NewHelper returns the helper for one UI component's gateway. ext supplies +// the variant's additions beside the proxy; nil adds none. +func NewHelper(cli client.Client, ext extensions.UIGatewayExtension, cfg Config) *Helper { + return &Helper{cli: cli, ext: ext, cfg: cfg} +} + +// proxyObjects returns the variant's additions beside the proxy. +func (h *Helper) proxyObjects() []client.Object { + if h.ext == nil { + return nil + } + return h.ext.ProxyObjects(h.cfg.ResourcePrefix, h.cfg.BackendNamespace) } // Components renders the component's gateway resources, plus deletion @@ -158,7 +166,7 @@ func (h *Helper) Components( BackendCABundleConfigMapName: h.cfg.BackendCABundleConfigMapName, TLSKeyPair: keyPair, ResourcePrefix: h.cfg.ResourcePrefix, - ExtraProxyObjects: h.cfg.ExtraProxyObjects, + ExtraProxyObjects: h.proxyObjects(), OpenShift: h.cfg.Provider.IsOpenShift(), RouteRequestTimeout: h.cfg.RouteRequestTimeout, })), nil @@ -232,7 +240,7 @@ func (h *Helper) StaleComponents(ctx context.Context, desiredNS string) ([]rende StaleNamespace: ns, BackendNamespace: h.cfg.BackendNamespace, TLSSecretName: h.cfg.TLSSecretName, - ExtraProxyObjects: h.cfg.ExtraProxyObjects, + ExtraProxyObjects: h.proxyObjects(), DeleteNamespace: deletable, TargetNamespace: desiredNS, })) @@ -271,7 +279,7 @@ func (h *Helper) Teardown(ctx context.Context) ([]render.Component, error) { StaleNamespace: ns, BackendNamespace: h.cfg.BackendNamespace, TLSSecretName: h.cfg.TLSSecretName, - ExtraProxyObjects: h.cfg.ExtraProxyObjects, + ExtraProxyObjects: h.proxyObjects(), DeleteNamespace: deletable, })) } diff --git a/pkg/uigateway/uigateway_test.go b/pkg/uigateway/uigateway_test.go index 362b8d0fb2..dcd9ccd478 100644 --- a/pkg/uigateway/uigateway_test.go +++ b/pkg/uigateway/uigateway_test.go @@ -82,7 +82,7 @@ var _ = Describe("UnhealthyReason", func() { scheme := runtime.NewScheme() Expect(apis.AddToScheme(scheme, false)).NotTo(HaveOccurred()) cli := ctrlrfake.DefaultFakeClientBuilder(scheme).WithObjects(objs...).Build() - h = uigateway.NewHelper(cli, uigateway.Config{ResourcePrefix: "calico-manager"}) + h = uigateway.NewHelper(cli, nil, uigateway.Config{ResourcePrefix: "calico-manager"}) } BeforeEach(func() { @@ -177,7 +177,7 @@ var _ = Describe("Cleanup helpers", func() { TLSSecretName: prefix + "-gateway-tls", BackendNamespace: backendNS, } - h = uigateway.NewHelper(cli, cfg) + h = uigateway.NewHelper(cli, nil, cfg) } // deletionNamespaces collects, per object type, the namespaces the given @@ -226,7 +226,7 @@ var _ = Describe("Cleanup helpers", func() { It("returns empty, not an error, when the Gateway kind is not served", func() { build() - h = uigateway.NewHelper(noGatewayKindClient{cli}, cfg) + h = uigateway.NewHelper(noGatewayKindClient{cli}, nil, cfg) namespaces, err := h.Namespaces(ctx) Expect(err).NotTo(HaveOccurred()) Expect(namespaces).To(BeEmpty()) @@ -258,6 +258,32 @@ var _ = Describe("Cleanup helpers", func() { }) }) + Describe("UIGateway extension", func() { + It("feeds the extension's proxy objects into the backend namespace's teardown", func() { + build(labeledGateway(prefix+"-gateway", backendNS)) + ext := &fakeUIGatewayExt{objs: []client.Object{ + &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{Name: "waf-http-filter", Namespace: backendNS}}, + }} + h = uigateway.NewHelper(cli, ext, cfg) + + components, err := h.Teardown(ctx) + Expect(err).NotTo(HaveOccurred()) + Expect(ext.seenPrefix).To(Equal(prefix)) + + var deleted []string + for _, c := range components { + _, toDelete := c.Objects() + for _, obj := range toDelete { + if sa, ok := obj.(*corev1.ServiceAccount); ok { + deleted = append(deleted, sa.Namespace+"/"+sa.Name) + } + } + } + Expect(deleted).To(ConsistOf(backendNS+"/waf-http-filter"), + "only the backend namespace's component deletes the extension objects") + }) + }) + Describe("access grant finalizers", func() { accessName := prefix + "-ingressgateway-access" @@ -342,7 +368,7 @@ var _ = Describe("Cleanup helpers", func() { It("returns nothing when the gateway CRDs are absent", func() { build() - h = uigateway.NewHelper(noGatewayKindClient{cli}, cfg) + h = uigateway.NewHelper(noGatewayKindClient{cli}, nil, cfg) components, err := h.Teardown(ctx) Expect(err).NotTo(HaveOccurred()) Expect(components).To(BeEmpty()) @@ -443,7 +469,7 @@ var _ = Describe("Cleanup helpers", func() { build(gatewayAPI("tigera-gateway-class")) cfgOCP := cfg cfgOCP.Provider = operatorv1.ProviderOpenShift - h = uigateway.NewHelper(cli, cfgOCP) + h = uigateway.NewHelper(cli, nil, cfgOCP) components, err := h.Components(ctx, spec("ns-a"), keyPair()) Expect(err).NotTo(HaveOccurred()) @@ -458,7 +484,7 @@ var _ = Describe("Cleanup helpers", func() { build(gatewayAPI("tigera-gateway-class")) cfgAKS := cfg cfgAKS.Provider = operatorv1.ProviderAKS - h = uigateway.NewHelper(cli, cfgAKS) + h = uigateway.NewHelper(cli, nil, cfgAKS) components, err := h.Components(ctx, spec("ns-a"), keyPair()) Expect(err).NotTo(HaveOccurred()) @@ -556,3 +582,14 @@ func (c noGatewayKindClient) List(ctx context.Context, list client.ObjectList, o } return c.Client.List(ctx, list, opts...) } + +// fakeUIGatewayExt supplies fixed proxy objects and records the prefix asked for. +type fakeUIGatewayExt struct { + objs []client.Object + seenPrefix string +} + +func (f *fakeUIGatewayExt) ProxyObjects(resourcePrefix, _ string) []client.Object { + f.seenPrefix = resourcePrefix + return f.objs +} From 15d20816384f004bf77e5ae230b63204995d2967 Mon Sep 17 00:00:00 2001 From: vara Date: Tue, 1 Sep 2026 13:23:55 -0700 Subject: [PATCH 3/4] PMREQ-821: Clear the grant finalizer with Update, not Patch The operator's ClusterRole grants update on roles and rolebindings but not patch, so the finalizer sweep's Patch was forbidden and marked grants never finished deleting. Unit tests missed it: the fake client enforces no RBAC. Verified on a GKE Enterprise cluster: the fixed sweep cleared grants wedged by the old code, and a fresh enable/teardown cycle left nothing behind. --- pkg/uigateway/uigateway.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/uigateway/uigateway.go b/pkg/uigateway/uigateway.go index 082540c8b6..393496b355 100644 --- a/pkg/uigateway/uigateway.go +++ b/pkg/uigateway/uigateway.go @@ -325,11 +325,12 @@ func (h *Helper) clearRBACFinalizers(ctx context.Context) error { if !gone { continue } - patchFrom := client.MergeFrom(grant.DeepCopyObject().(client.Object)) grant.SetFinalizers(slices.DeleteFunc(grant.GetFinalizers(), func(f string) bool { return f == rgateway.RBACFinalizer })) - if err := h.cli.Patch(ctx, grant, patchFrom); err != nil && !errors.IsNotFound(err) { + // Update, not Patch: the operator's ClusterRole grants update on + // roles and rolebindings but not patch. + if err := h.cli.Update(ctx, grant); err != nil && !errors.IsNotFound(err) { return err } } From e644cc31f7868d135efa975dac2c1257a66e95bf Mon Sep 17 00:00:00 2001 From: vara Date: Tue, 1 Sep 2026 13:58:32 -0700 Subject: [PATCH 4/4] PMREQ-821: Treat every no-match form as kind-not-served in the sweep accessResourcesGone suppressed only NoKindMatchError, so a NoResourceMatchError from the mapper would block finalizer clearing. apimeta.IsNoMatchError covers both. Review follow-up on #5293. --- pkg/uigateway/uigateway.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/uigateway/uigateway.go b/pkg/uigateway/uigateway.go index 393496b355..7dbea14e11 100644 --- a/pkg/uigateway/uigateway.go +++ b/pkg/uigateway/uigateway.go @@ -353,8 +353,7 @@ func (h *Helper) accessResourcesGone(ctx context.Context, namespace string) (boo if err == nil { return false, nil } - var noMatch *apimeta.NoKindMatchError - if !errors.IsNotFound(err) && !stderrors.As(err, &noMatch) { + if !errors.IsNotFound(err) && !apimeta.IsNoMatchError(err) { return false, err } }