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/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..7dbea14e11 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,12 +38,14 @@ 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" "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" @@ -71,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 @@ -84,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 @@ -155,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 @@ -205,7 +216,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 @@ -224,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, })) @@ -234,9 +250,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 @@ -258,13 +279,87 @@ 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, })) } 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 + } + grant.SetFinalizers(slices.DeleteFunc(grant.GetFinalizers(), func(f string) bool { + return f == rgateway.RBACFinalizer + })) + // 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 + } + } + 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 + } + if !errors.IsNotFound(err) && !apimeta.IsNoMatchError(err) { + 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..dcd9ccd478 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" @@ -80,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() { @@ -175,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 @@ -224,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()) @@ -256,6 +258,90 @@ 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" + + 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")) @@ -282,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()) @@ -383,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()) @@ -398,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()) @@ -496,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 +}