From 15105dbae0f2623b4878a841a4c89c035131d4a0 Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Fri, 16 Oct 2026 08:36:04 +0800 Subject: [PATCH 1/8] fix: preserve shared GatewayProxy resources when deleting Gateways --- internal/controller/httproute_controller.go | 2 +- internal/controller/utils.go | 9 ++- internal/controller/utils_parentref_test.go | 11 ++-- internal/provider/api7ee/provider.go | 4 +- internal/provider/api7ee/provider_test.go | 62 +++++++++++++++++++++ internal/provider/apisix/provider.go | 4 +- internal/provider/apisix/provider_test.go | 58 +++++++++++++++++++ test/conformance/conformance_test.go | 9 +-- 8 files changed, 137 insertions(+), 22 deletions(-) diff --git a/internal/controller/httproute_controller.go b/internal/controller/httproute_controller.go index d2e507aa..d7b05576 100644 --- a/internal/controller/httproute_controller.go +++ b/internal/controller/httproute_controller.go @@ -200,7 +200,7 @@ func (r *HTTPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( if len(gateways) == 0 { if unresolvedParents { - // A missing Gateway or GatewayClass leaves ownership unknown rather than + // A missing GatewayClass leaves ownership unknown rather than // disproven. GatewayClass is cluster-scoped, so while one is absent every // route under it resolves empty and deleting would drain the data plane. return ctrl.Result{}, nil diff --git a/internal/controller/utils.go b/internal/controller/utils.go index 732a7414..0b961e5a 100644 --- a/internal/controller/utils.go +++ b/internal/controller/utils.go @@ -339,10 +339,9 @@ func parentRefTargetsListenerExplicitly(parentRef gatewayv1.ParentReference) boo // ParseRouteParentRefs resolves the parentRefs of a route to the Gateways this // controller manages. The second return value reports that at least one -// parentRef could not be resolved, because its Gateway or that Gateway's -// GatewayClass does not exist. An empty gateway list then means "ownership -// unknown", not "owned by another controller", and callers must not act on the -// route's data plane configuration. +// parentRef points to an existing Gateway whose GatewayClass does not exist. +// Callers preserve the existing configuration while GatewayClass ownership +// cannot be determined. Missing Gateways do not keep stale routes active. func ParseRouteParentRefs( ctx context.Context, mgrc client.Client, @@ -369,7 +368,7 @@ func ParseRouteParentRefs( Name: name, }, &gateway); err != nil { if client.IgnoreNotFound(err) == nil { - unresolved = true + // A deleted Gateway no longer authorizes its routes to serve traffic. continue } return nil, false, fmt.Errorf("failed to retrieve gateway for route: %w", err) diff --git a/internal/controller/utils_parentref_test.go b/internal/controller/utils_parentref_test.go index f6e1ffa4..9d026aef 100644 --- a/internal/controller/utils_parentref_test.go +++ b/internal/controller/utils_parentref_test.go @@ -233,10 +233,9 @@ func TestParseRouteParentRefs_ConflictingTLSModePort(t *testing.T) { } // TestParseRouteParentRefs_UnresolvedParents verifies the second return value -// distinguishes "no parentRef named this controller" from "a parentRef could not -// be resolved". Callers delete data plane configuration on an empty gateway list, -// which is only correct in the first case: a GatewayClass is cluster-scoped, so -// while one is missing every route under it resolves to an empty list at once. +// distinguishes missing Gateways from unresolved GatewayClass ownership. +// Missing Gateways require route cleanup, while a missing cluster-scoped +// GatewayClass must not drain every route under it. func TestParseRouteParentRefs_UnresolvedParents(t *testing.T) { scheme := parentRefTestScheme(t) @@ -266,10 +265,10 @@ func TestParseRouteParentRefs_UnresolvedParents(t *testing.T) { wantUnresolved bool }{ { - name: "gateway missing", + name: "gateway missing retracts stale routes", objects: []client.Object{newParentRefGatewayClass()}, parentRef: "gw", - wantUnresolved: true, + wantUnresolved: false, }, { name: "gatewayclass missing", diff --git a/internal/provider/api7ee/provider.go b/internal/provider/api7ee/provider.go index 72cdcbea..3ba66db8 100644 --- a/internal/provider/api7ee/provider.go +++ b/internal/provider/api7ee/provider.go @@ -205,7 +205,9 @@ func (d *api7eeProvider) Delete(ctx context.Context, obj client.Object) error { resourceTypes = append(resourceTypes, "service") labels = label.GenLabel(obj) case *gatewayv1.Gateway: - // delete all resources + // Gateways sharing a GatewayProxy only own their own certificates. + resourceTypes = append(resourceTypes, "ssl") + labels = label.GenLabel(obj) case *networkingv1.Ingress: resourceTypes = append(resourceTypes, "service", "ssl") labels = label.GenLabel(obj) diff --git a/internal/provider/api7ee/provider_test.go b/internal/provider/api7ee/provider_test.go index 8ef49365..a63ccf77 100644 --- a/internal/provider/api7ee/provider_test.go +++ b/internal/provider/api7ee/provider_test.go @@ -19,8 +19,12 @@ package api7ee import ( "context" + "encoding/json" + "net/http" + "net/http/httptest" "strings" "testing" + "time" "github.com/go-logr/logr" "github.com/go-logr/logr/funcr" @@ -34,8 +38,10 @@ import ( adctypes "github.com/apache/apisix-ingress-controller/api/adc" "github.com/apache/apisix-ingress-controller/api/v1alpha1" apiv2 "github.com/apache/apisix-ingress-controller/api/v2" + adcclient "github.com/apache/apisix-ingress-controller/internal/adc/client" "github.com/apache/apisix-ingress-controller/internal/controller/label" "github.com/apache/apisix-ingress-controller/internal/provider" + "github.com/apache/apisix-ingress-controller/internal/types" "github.com/apache/apisix-ingress-controller/internal/utils" ) @@ -133,3 +139,59 @@ func TestUpdateKeepsLastKnownGoodStateWhenL4PolicyCannotRender(t *testing.T) { require.Len(t, resources.Services, 1) assert.Equal(t, "last-known-good", resources.Services[0].Name) } + +func TestDeleteGatewayPreservesSharedProxyResources(t *testing.T) { + var received adcclient.ADCServerRequest + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.NoError(t, json.NewDecoder(r.Body).Decode(&received)) + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(adctypes.SyncResult{Status: adctypes.StatusSuccess}) + })) + defer server.Close() + t.Setenv("ADC_SERVER_URL", server.URL) + cli, err := adcclient.New(logr.Discard(), "api7ee", time.Second) + require.NoError(t, err) + d := &api7eeProvider{client: cli, log: logr.Discard()} + gateway := &gatewayv1.Gateway{ + TypeMeta: metav1.TypeMeta{Kind: "Gateway", APIVersion: gatewayv1.GroupVersion.String()}, + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "deleted"}, + } + other := gateway.DeepCopy() + other.Name = "remaining" + route := &gatewayv1.HTTPRoute{ + TypeMeta: metav1.TypeMeta{Kind: "HTTPRoute", APIVersion: gatewayv1.GroupVersion.String()}, + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "route"}, + } + proxy := types.NamespacedNameKind{Kind: "GatewayProxy", Namespace: "default", Name: "proxy"} + configs := map[types.NamespacedNameKind]adctypes.Config{proxy: { + Name: proxy.String(), BackendType: "apisix", ServerAddrs: []string{"http://apisix:9180"}, + }} + for _, gw := range []*gatewayv1.Gateway{gateway, other} { + labels := label.GenLabel(gw) + require.NoError(t, cli.UpdateConfig(context.Background(), adcclient.Task{ + Key: utils.NamespacedNameKind(gw), Configs: configs, Labels: labels, ResourceTypes: []string{adctypes.TypeSSL}, + Resources: &adctypes.Resources{SSLs: []*adctypes.SSL{{Metadata: adctypes.Metadata{ID: gw.Name, Labels: labels}}}}, + })) + } + labels := label.GenLabel(route) + require.NoError(t, cli.UpdateConfig(context.Background(), adcclient.Task{ + Key: utils.NamespacedNameKind(route), Configs: configs, Labels: labels, ResourceTypes: []string{adctypes.TypeService}, + Resources: &adctypes.Resources{Services: []*adctypes.Service{{Metadata: adctypes.Metadata{ID: "route", Labels: labels}}}}, + })) + require.NoError(t, d.Delete(context.Background(), gateway)) + resources, err := cli.GetResources(proxy.String()) + require.NoError(t, err) + require.Len(t, resources.SSLs, 1) + assert.Equal(t, "remaining", resources.SSLs[0].ID) + require.Len(t, resources.Services, 1) + assert.Equal(t, "route", resources.Services[0].ID) + assert.Equal(t, []string{adctypes.TypeSSL}, received.Task.Opts.IncludeResourceType) + assert.Equal(t, map[string]string{label.LabelResourceKey: label.GenLabel(gateway)[label.LabelResourceKey]}, received.Task.Opts.LabelSelector) + _, err = cli.Sync(context.Background()) + require.NoError(t, err) + assert.Equal(t, resources.SSLs, received.Task.Config.SSLs) + assert.Equal(t, resources.Services, received.Task.Config.Services) + assert.Empty(t, cli.ConfigManager.Get(utils.NamespacedNameKind(gateway))) + assert.NotEmpty(t, cli.ConfigManager.Get(utils.NamespacedNameKind(other))) + assert.NotEmpty(t, cli.ConfigManager.Get(utils.NamespacedNameKind(route))) +} diff --git a/internal/provider/apisix/provider.go b/internal/provider/apisix/provider.go index e6b8c04d..addd9f51 100644 --- a/internal/provider/apisix/provider.go +++ b/internal/provider/apisix/provider.go @@ -202,7 +202,9 @@ func (d *apisixProvider) Delete(ctx context.Context, obj client.Object) error { resourceTypes = append(resourceTypes, adctypes.TypeService) labels = label.GenLabel(obj) case *gatewayv1.Gateway: - // delete all resources + // Gateways sharing a GatewayProxy only own their own certificates. + resourceTypes = append(resourceTypes, adctypes.TypeSSL) + labels = label.GenLabel(obj) case *networkingv1.Ingress: resourceTypes = append(resourceTypes, adctypes.TypeService, adctypes.TypeSSL) labels = label.GenLabel(obj) diff --git a/internal/provider/apisix/provider_test.go b/internal/provider/apisix/provider_test.go index 50d9e420..214bdbda 100644 --- a/internal/provider/apisix/provider_test.go +++ b/internal/provider/apisix/provider_test.go @@ -19,6 +19,9 @@ package apisix import ( "context" + "encoding/json" + "net/http" + "net/http/httptest" "strings" "testing" "time" @@ -170,3 +173,58 @@ func TestUpdateKeepsLastKnownGoodStateWhenL4PolicyCannotRender(t *testing.T) { require.Len(t, resources.Services, 1) assert.Equal(t, "last-known-good", resources.Services[0].Name) } + +func TestDeleteGatewayPreservesSharedProxyResources(t *testing.T) { + var received adcclient.ADCServerRequest + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + require.NoError(t, json.NewDecoder(r.Body).Decode(&received)) + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(adctypes.SyncResult{Status: adctypes.StatusSuccess}) + })) + defer server.Close() + t.Setenv("ADC_SERVER_URL", server.URL) + cli, err := adcclient.New(logr.Discard(), ProviderTypeAPISIX, time.Second) + require.NoError(t, err) + d := &apisixProvider{client: cli, syncCh: make(chan struct{}, 1), log: logr.Discard()} + gateway := &gatewayv1.Gateway{ + TypeMeta: metav1.TypeMeta{Kind: "Gateway", APIVersion: gatewayv1.GroupVersion.String()}, + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "deleted"}, + } + other := gateway.DeepCopy() + other.Name = "remaining" + route := &gatewayv1.HTTPRoute{ + TypeMeta: metav1.TypeMeta{Kind: "HTTPRoute", APIVersion: gatewayv1.GroupVersion.String()}, + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "route"}, + } + proxy := types.NamespacedNameKind{Kind: "GatewayProxy", Namespace: "default", Name: "proxy"} + configs := map[types.NamespacedNameKind]adctypes.Config{proxy: { + Name: proxy.String(), BackendType: "apisix", ServerAddrs: []string{"http://apisix:9180"}, + }} + for _, gw := range []*gatewayv1.Gateway{gateway, other} { + labels := label.GenLabel(gw) + require.NoError(t, cli.UpdateConfig(context.Background(), adcclient.Task{ + Key: utils.NamespacedNameKind(gw), Configs: configs, Labels: labels, ResourceTypes: []string{adctypes.TypeSSL}, + Resources: &adctypes.Resources{SSLs: []*adctypes.SSL{{Metadata: adctypes.Metadata{ID: gw.Name, Labels: labels}}}}, + })) + } + labels := label.GenLabel(route) + require.NoError(t, cli.UpdateConfig(context.Background(), adcclient.Task{ + Key: utils.NamespacedNameKind(route), Configs: configs, Labels: labels, ResourceTypes: []string{adctypes.TypeService}, + Resources: &adctypes.Resources{Services: []*adctypes.Service{{Metadata: adctypes.Metadata{ID: "route", Labels: labels}}}}, + })) + require.NoError(t, d.Delete(context.Background(), gateway)) + resources, err := cli.GetResources(proxy.String()) + require.NoError(t, err) + require.Len(t, resources.SSLs, 1) + assert.Equal(t, "remaining", resources.SSLs[0].ID) + require.Len(t, resources.Services, 1) + assert.Equal(t, "route", resources.Services[0].ID) + require.Len(t, d.syncCh, 1, "Gateway deletion must schedule the remaining shared snapshot") + _, err = cli.Sync(context.Background()) + require.NoError(t, err) + assert.Equal(t, resources.SSLs, received.Task.Config.SSLs) + assert.Equal(t, resources.Services, received.Task.Config.Services) + assert.Empty(t, cli.ConfigManager.Get(utils.NamespacedNameKind(gateway))) + assert.NotEmpty(t, cli.ConfigManager.Get(utils.NamespacedNameKind(other))) + assert.NotEmpty(t, cli.ConfigManager.Get(utils.NamespacedNameKind(route))) +} diff --git a/test/conformance/conformance_test.go b/test/conformance/conformance_test.go index 223a0a73..9d794ab9 100644 --- a/test/conformance/conformance_test.go +++ b/test/conformance/conformance_test.go @@ -39,15 +39,8 @@ var skippedTestsForKnownGaps = []string{ tests.HTTPRouteListenerHostnameMatching.ShortName, tests.GRPCRouteListenerHostnameMatching.ShortName, - // A backendRef that cannot be resolved must still produce a route that - // answers 500; today no route is generated at all, so the request 404s. - // These consistently pass in standalone mode and fail against the APISIX - // admin API, and which member of the group trips is not stable between - // runs, so all four are skipped together rather than one at a time. + // Omitted backendRefs do not yet inject the required HTTP 500 response. tests.HTTPRouteNoBackendRefs.ShortName, - tests.HTTPRouteInvalidBackendRefUnknownKind.ShortName, - tests.HTTPRouteInvalidCrossNamespaceBackendRef.ShortName, - tests.HTTPRouteInvalidNonExistentBackendRef.ShortName, // Terminate mode itself is covered by TLSRouteListenerTerminateSupportedKinds // and by the e2e TLSRoute suite. This provisional test additionally requires a // standalone Gateway with no GatewayProxy attached to reach Accepted=True, and From 1b328305aaa4c6104372c77680306427f3b782a5 Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Fri, 16 Oct 2026 09:10:51 +0800 Subject: [PATCH 2/8] fix: reconcile attached routes when a Gateway is deleted --- internal/controller/grpcroute_controller.go | 2 +- internal/controller/httproute_controller.go | 2 +- internal/controller/tcproute_controller.go | 2 +- internal/controller/tlsroute_controller.go | 2 +- internal/controller/udproute_controller.go | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/controller/grpcroute_controller.go b/internal/controller/grpcroute_controller.go index 11a5a2ca..3968f619 100644 --- a/internal/controller/grpcroute_controller.go +++ b/internal/controller/grpcroute_controller.go @@ -94,7 +94,7 @@ func (r *GRPCRouteReconciler) SetupWithManager(mgr ctrl.Manager) error { return false }, DeleteFunc: func(e event.DeleteEvent) bool { - return false + return true }, CreateFunc: func(e event.CreateEvent) bool { return true diff --git a/internal/controller/httproute_controller.go b/internal/controller/httproute_controller.go index d7b05576..200e9f01 100644 --- a/internal/controller/httproute_controller.go +++ b/internal/controller/httproute_controller.go @@ -116,7 +116,7 @@ func (r *HTTPRouteReconciler) SetupWithManager(mgr ctrl.Manager) error { return false }, DeleteFunc: func(e event.DeleteEvent) bool { - return false + return true }, CreateFunc: func(e event.CreateEvent) bool { return true diff --git a/internal/controller/tcproute_controller.go b/internal/controller/tcproute_controller.go index 8428a55e..94350850 100644 --- a/internal/controller/tcproute_controller.go +++ b/internal/controller/tcproute_controller.go @@ -86,7 +86,7 @@ func (r *TCPRouteReconciler) SetupWithManager(mgr ctrl.Manager) error { return false }, DeleteFunc: func(e event.DeleteEvent) bool { - return false + return true }, CreateFunc: func(e event.CreateEvent) bool { return true diff --git a/internal/controller/tlsroute_controller.go b/internal/controller/tlsroute_controller.go index 1346c3ef..81286819 100644 --- a/internal/controller/tlsroute_controller.go +++ b/internal/controller/tlsroute_controller.go @@ -86,7 +86,7 @@ func (r *TLSRouteReconciler) SetupWithManager(mgr ctrl.Manager) error { return false }, DeleteFunc: func(e event.DeleteEvent) bool { - return false + return true }, CreateFunc: func(e event.CreateEvent) bool { return true diff --git a/internal/controller/udproute_controller.go b/internal/controller/udproute_controller.go index a9475941..846ce3c8 100644 --- a/internal/controller/udproute_controller.go +++ b/internal/controller/udproute_controller.go @@ -86,7 +86,7 @@ func (r *UDPRouteReconciler) SetupWithManager(mgr ctrl.Manager) error { return false }, DeleteFunc: func(e event.DeleteEvent) bool { - return false + return true }, CreateFunc: func(e event.CreateEvent) bool { return true From cd3f45a200559f88c034643044a3c6b3ab3b75ff Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Fri, 16 Oct 2026 09:10:51 +0800 Subject: [PATCH 3/8] test: wait for data plane updates and pod termination --- test/e2e/crds/v2/tls.go | 16 +++++++------- test/e2e/ingress/annotations.go | 31 ++++++++++++++++------------ test/e2e/scaffold/apisix_deployer.go | 6 ++++++ 3 files changed, 33 insertions(+), 20 deletions(-) diff --git a/test/e2e/crds/v2/tls.go b/test/e2e/crds/v2/tls.go index 5a64cebc..c12588e1 100644 --- a/test/e2e/crds/v2/tls.go +++ b/test/e2e/crds/v2/tls.go @@ -131,13 +131,15 @@ spec: assert.ElementsMatch(GinkgoT(), []string{host}, tls[0].Snis) By("test HTTPS request to dataplane") - Eventually(func() int { - return s.NewAPISIXHttpsClient("api6.com"). - GET("/get"). - WithHost("api6.com"). - Expect(). - Raw().StatusCode - }).WithTimeout(30 * time.Second).ProbeEvery(1 * time.Second).Should(Equal(http.StatusOK)) + s.RequestAssert(&scaffold.RequestAssert{ + Client: s.NewAPISIXHttpsClient("api6.com"), + Method: "GET", + Path: "/get", + Host: "api6.com", + Check: scaffold.WithExpectedStatus(http.StatusOK), + Timeout: 30 * time.Second, + Interval: time.Second, + }) s.NewAPISIXHttpsClient("api6.com"). GET("/get"). diff --git a/test/e2e/ingress/annotations.go b/test/e2e/ingress/annotations.go index 622d0cb8..2f813c88 100644 --- a/test/e2e/ingress/annotations.go +++ b/test/e2e/ingress/annotations.go @@ -623,19 +623,24 @@ spec: Check: scaffold.WithExpectedStatus(http.StatusPermanentRedirect), }) - _ = s.NewAPISIXClient(). - GET("/get"). - WithHost("httpbin.example"). - Expect(). - Status(http.StatusMovedPermanently). - Header("Location").IsEqual("https://httpbin.example:9443/get") - - _ = s.NewAPISIXClient(). - GET("/ip"). - WithHost("httpbin.example"). - Expect(). - Status(http.StatusPermanentRedirect). - Header("Location").IsEqual("/anything/ip") + s.RequestAssert(&scaffold.RequestAssert{ + Method: "GET", + Path: "/get", + Host: "httpbin.example", + Checks: []scaffold.ResponseCheckFunc{ + scaffold.WithExpectedStatus(http.StatusMovedPermanently), + scaffold.WithExpectedHeader("Location", "https://httpbin.example:9443/get"), + }, + }) + s.RequestAssert(&scaffold.RequestAssert{ + Method: "GET", + Path: "/ip", + Host: "httpbin.example", + Checks: []scaffold.ResponseCheckFunc{ + scaffold.WithExpectedStatus(http.StatusPermanentRedirect), + scaffold.WithExpectedHeader("Location", "/anything/ip"), + }, + }) }) It("csrf", func() { diff --git a/test/e2e/scaffold/apisix_deployer.go b/test/e2e/scaffold/apisix_deployer.go index a08b3885..f841b64d 100644 --- a/test/e2e/scaffold/apisix_deployer.go +++ b/test/e2e/scaffold/apisix_deployer.go @@ -282,6 +282,12 @@ func (s *APISIXDeployer) deployDataplane(opts *APISIXDeployOptions) *corev1.Serv LabelSelector: "app.kubernetes.io/name=apisix", }) Expect(err).ToNot(HaveOccurred(), "waiting for gateway pod ready") + } else { + // A terminating pod can still be Ready and selected by a new port-forward. + err = k8s.WaitUntilNumPodsCreatedE(s.GinkgoT, kubectlOpts, metav1.ListOptions{ + LabelSelector: "app.kubernetes.io/name=apisix", + }, 0, 30, time.Second) + Expect(err).ToNot(HaveOccurred(), "waiting for gateway pods to terminate") } Eventually(func() bool { From 66938e9014dee65ce94c85b4dd7b3c9c311b1c47 Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Mon, 19 Oct 2026 11:28:59 +0800 Subject: [PATCH 4/8] fix: resolve surviving route parents after Gateway deletion --- internal/controller/grpcroute_controller.go | 10 +- internal/controller/httproute_controller.go | 11 +- .../controller/httproute_controller_test.go | 10 +- internal/controller/route_parent_resolver.go | 98 +++++++++ .../controller/route_parent_resolver_test.go | 192 ++++++++++++++++++ internal/controller/tcproute_controller.go | 10 +- internal/controller/tlsroute_controller.go | 10 +- internal/controller/udproute_controller.go | 10 +- internal/controller/utils.go | 29 +-- internal/controller/utils_parentref_test.go | 59 +++--- .../provider/api7ee/gateway_delete_test.go | 102 ++++++++++ .../provider/apisix/gateway_delete_test.go | 102 ++++++++++ 12 files changed, 551 insertions(+), 92 deletions(-) create mode 100644 internal/controller/route_parent_resolver.go create mode 100644 internal/controller/route_parent_resolver_test.go create mode 100644 internal/provider/api7ee/gateway_delete_test.go create mode 100644 internal/provider/apisix/gateway_delete_test.go diff --git a/internal/controller/grpcroute_controller.go b/internal/controller/grpcroute_controller.go index 3968f619..9db9261d 100644 --- a/internal/controller/grpcroute_controller.go +++ b/internal/controller/grpcroute_controller.go @@ -57,6 +57,8 @@ type GRPCRouteReconciler struct { //nolint:revive Provider provider.Provider + parentResolver routeParentResolver + genericEvent chan event.GenericEvent Updater status.Updater @@ -164,6 +166,7 @@ func (r *GRPCRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( gr := new(gatewayv1.GRPCRoute) if err := r.Get(ctx, req.NamespacedName, gr); err != nil { if client.IgnoreNotFound(err) == nil { + r.parentResolver.Forget(req.NamespacedName) gr.Namespace = req.Namespace gr.Name = req.Name @@ -192,17 +195,12 @@ func (r *GRPCRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( msg: "Route is accepted", } - gateways, unresolvedParents, err := ParseRouteParentRefs(ctx, r.Client, r.Log, gr, gr.Spec.ParentRefs) + gateways, err := r.parentResolver.Resolve(ctx, r.Client, r.Log, gr, gr.Spec.ParentRefs) if err != nil { return ctrl.Result{}, err } if len(gateways) == 0 { - if unresolvedParents { - // See the HTTPRoute reconciler: an unresolvable parentRef leaves ownership - // unknown rather than disproven, so the data plane must be left alone. - return ctrl.Result{}, nil - } // See the HTTPRoute reconciler: a route that no longer references a // Gateway managed by this controller must have its previously pushed // configuration removed, or the data plane keeps serving it. diff --git a/internal/controller/httproute_controller.go b/internal/controller/httproute_controller.go index 200e9f01..a556eb7e 100644 --- a/internal/controller/httproute_controller.go +++ b/internal/controller/httproute_controller.go @@ -60,6 +60,8 @@ type HTTPRouteReconciler struct { //nolint:revive Provider provider.Provider + parentResolver routeParentResolver + genericEvent chan event.GenericEvent Updater status.Updater @@ -162,6 +164,7 @@ func (r *HTTPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( hr := new(gatewayv1.HTTPRoute) if err := r.Get(ctx, req.NamespacedName, hr); err != nil { if client.IgnoreNotFound(err) == nil { + r.parentResolver.Forget(req.NamespacedName) if err := r.updateHTTPRoutePolicyStatusOnDeleting(ctx, req.NamespacedName); err != nil { r.Log.Error(err, "failed to update HTTPRoutePolicy status on HTTPRoute deleting", "httproute", req.NamespacedName) } @@ -193,18 +196,12 @@ func (r *HTTPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( msg: "Route is accepted", } - gateways, unresolvedParents, err := ParseRouteParentRefs(ctx, r.Client, r.Log, hr, hr.Spec.ParentRefs) + gateways, err := r.parentResolver.Resolve(ctx, r.Client, r.Log, hr, hr.Spec.ParentRefs) if err != nil { return ctrl.Result{}, err } if len(gateways) == 0 { - if unresolvedParents { - // A missing GatewayClass leaves ownership unknown rather than - // disproven. GatewayClass is cluster-scoped, so while one is absent every - // route under it resolves empty and deleting would drain the data plane. - return ctrl.Result{}, nil - } // The route does not reference any Gateway managed by this controller. // It may have referenced one before, e.g. when its parentRefs are // repointed at a Gateway belonging to another GatewayClass, so the diff --git a/internal/controller/httproute_controller_test.go b/internal/controller/httproute_controller_test.go index 42daff8b..6e9d2f1e 100644 --- a/internal/controller/httproute_controller_test.go +++ b/internal/controller/httproute_controller_test.go @@ -43,11 +43,7 @@ type noopReadier struct { func (noopReadier) Done(client.Object, k8stypes.NamespacedName) {} -// TestHTTPRouteReconcile_EmptyGateways covers the two ways a route resolves to no -// Gateway of this controller. Only a resolved parent naming another controller -// proves the route is not ours; an unresolvable parent must leave the data plane -// alone, since a missing cluster-scoped GatewayClass empties the list for every -// route under it at once. +// Routes with no known managed parents retract any stale configuration. func TestHTTPRouteReconcile_EmptyGateways(t *testing.T) { scheme := parentRefTestScheme(t) @@ -85,9 +81,9 @@ func TestHTTPRouteReconcile_EmptyGateways(t *testing.T) { wantDelete: true, }, { - name: "gatewayclass missing", + name: "gatewayclass never resolved", objects: []client.Object{route, gw("gone")}, - wantDelete: false, + wantDelete: true, }, } { t.Run(tc.name, func(t *testing.T) { diff --git a/internal/controller/route_parent_resolver.go b/internal/controller/route_parent_resolver.go new file mode 100644 index 00000000..e4ff9eb8 --- /dev/null +++ b/internal/controller/route_parent_resolver.go @@ -0,0 +1,98 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you 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 controller + +import ( + "context" + "sync" + + "github.com/go-logr/logr" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + + "github.com/apache/apisix-ingress-controller/internal/controller/config" +) + +type resolvedParentClass struct { + gatewayUID types.UID + class *gatewayv1.GatewayClass +} + +type routeParentOwnership struct { + uid types.UID + parents map[client.ObjectKey]resolvedParentClass +} + +// routeParentResolver remembers ownership only for parents previously resolved +// for this route. A never-resolved sibling must not prevent deletion cleanup. +// Like the provider's desired configuration, this state is rebuilt on startup. +type routeParentResolver struct { + mu sync.Mutex + parents map[client.ObjectKey]routeParentOwnership +} + +func (r *routeParentResolver) Forget(key client.ObjectKey) { + r.mu.Lock() + defer r.mu.Unlock() + delete(r.parents, key) +} + +func (r *routeParentResolver) Resolve(ctx context.Context, cli client.Client, log logr.Logger, route client.Object, refs []gatewayv1.ParentReference) ([]RouteParentRefContext, error) { + r.mu.Lock() + defer r.mu.Unlock() + + routeKey := client.ObjectKeyFromObject(route) + previous := r.parents[routeKey].parents + if r.parents[routeKey].uid != route.GetUID() { + previous = nil + } + next := make(map[client.ObjectKey]resolvedParentClass) + gateways, err := parseRouteParentRefs(ctx, cli, log, route, refs, func(gw *gatewayv1.Gateway) (*gatewayv1.GatewayClass, error) { + key := client.ObjectKeyFromObject(gw) + gc := &gatewayv1.GatewayClass{} + if err := cli.Get(ctx, client.ObjectKey{Name: string(gw.Spec.GatewayClassName)}, gc); err != nil { + if client.IgnoreNotFound(err) != nil { + return nil, err + } + known, ok := previous[key] + if !ok || known.gatewayUID != gw.UID || known.class.Name != string(gw.Spec.GatewayClassName) { + return nil, nil + } + // Keep resolving the surviving Gateway's current listeners without + // retaining configuration belonging to a different, deleted parent. + gc = known.class + } + if string(gc.Spec.ControllerName) == config.ControllerConfig.ControllerName { + next[key] = resolvedParentClass{gatewayUID: gw.UID, class: gc} + } + return gc, nil + }) + if err != nil { + return nil, err + } + if len(next) == 0 { + delete(r.parents, routeKey) + } else { + if r.parents == nil { + r.parents = make(map[client.ObjectKey]routeParentOwnership) + } + r.parents[routeKey] = routeParentOwnership{uid: route.GetUID(), parents: next} + } + return gateways, nil +} diff --git a/internal/controller/route_parent_resolver_test.go b/internal/controller/route_parent_resolver_test.go new file mode 100644 index 00000000..4ab3f505 --- /dev/null +++ b/internal/controller/route_parent_resolver_test.go @@ -0,0 +1,192 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you 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 controller + +import ( + "context" + "errors" + "testing" + + "github.com/go-logr/logr" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" +) + +func TestRouteParentResolverGatewayDeletion(t *testing.T) { + for _, tc := range []struct { + name string + route client.Object + protocol gatewayv1.ProtocolType + }{ + {"HTTPRoute", &gatewayv1.HTTPRoute{}, gatewayv1.HTTPProtocolType}, + {"GRPCRoute", &gatewayv1.GRPCRoute{}, gatewayv1.HTTPProtocolType}, + {"TCPRoute", &gatewayv1.TCPRoute{}, gatewayv1.TCPProtocolType}, + {"UDPRoute", &gatewayv1.UDPRoute{}, gatewayv1.UDPProtocolType}, + {"TLSRoute", &gatewayv1.TLSRoute{}, gatewayv1.TLSProtocolType}, + } { + t.Run(tc.name, func(t *testing.T) { + for _, knownSibling := range []bool{false, true} { + name := "never-resolved sibling" + if knownSibling { + name = "previously-resolved sibling" + } + t.Run(name, func(t *testing.T) { + ctx := context.Background() + route := tc.route.DeepCopyObject().(client.Object) + route.SetNamespace("default") + route.SetName("route") + route.SetUID("route-uid") + classA := newParentRefGatewayClass() + classB := classA.DeepCopy() + classB.Name = "sibling-class" + listener := gatewayv1.Listener{Name: "listener", Port: 9000, Protocol: tc.protocol} + if tc.protocol == gatewayv1.TLSProtocolType { + listener.TLS = &gatewayv1.ListenerTLSConfig{Mode: ptr.To(gatewayv1.TLSModePassthrough)} + } + a := &gatewayv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "a", UID: "a-uid"}, + Spec: gatewayv1.GatewaySpec{GatewayClassName: gatewayv1.ObjectName(classA.Name), Listeners: []gatewayv1.Listener{listener}}, + } + b := a.DeepCopy() + b.Name, b.UID = "b", "b-uid" + b.Spec.GatewayClassName = gatewayv1.ObjectName(classB.Name) + objects := []client.Object{classA, a, b} + if knownSibling { + objects = append(objects, classB) + } + cli := fake.NewClientBuilder().WithScheme(parentRefTestScheme(t)).WithObjects(objects...).Build() + refs := []gatewayv1.ParentReference{{Name: "a"}, {Name: "b"}} + resolver := &routeParentResolver{} + parents, err := resolver.Resolve(ctx, cli, logr.Discard(), route, refs) + require.NoError(t, err) + expected := 1 + if knownSibling { + expected = 2 + } + require.Len(t, parents, expected) + require.True(t, isRouteAccepted(parents)) + if knownSibling { + require.NoError(t, cli.Delete(ctx, classB)) + } + require.NoError(t, cli.Delete(ctx, a)) + parents, err = resolver.Resolve(ctx, cli, logr.Discard(), route, refs) + require.NoError(t, err) + if knownSibling { + require.Len(t, parents, 1) + require.Equal(t, "b", parents[0].Gateway.Name) + require.True(t, isRouteAccepted(parents)) + // Listener changes still apply while its class is absent. + b.Spec.Listeners[0].AllowedRoutes = &gatewayv1.AllowedRoutes{ + Kinds: []gatewayv1.RouteGroupKind{{Kind: "UnsupportedRoute"}}, + } + require.NoError(t, cli.Update(ctx, b)) + parents, err = resolver.Resolve(ctx, cli, logr.Discard(), route, refs) + require.NoError(t, err) + require.False(t, isRouteAccepted(parents)) + require.NoError(t, cli.Delete(ctx, b)) + parents, err = resolver.Resolve(ctx, cli, logr.Discard(), route, refs) + require.NoError(t, err) + } + require.Empty(t, parents) + require.Empty(t, resolver.parents) + }) + } + }) + } +} + +func TestRouteParentResolverDoesNotReuseOwnership(t *testing.T) { + for _, change := range []string{"gateway recreated", "route recreated", "class changed", "route deleted", "parent removed", "class reassigned"} { + t.Run(change, func(t *testing.T) { + ctx := context.Background() + gc := newParentRefGatewayClass() + gw := &gatewayv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "gw", UID: "gateway-uid"}, + Spec: gatewayv1.GatewaySpec{GatewayClassName: gatewayv1.ObjectName(gc.Name), Listeners: []gatewayv1.Listener{{Name: "http", Port: 80, Protocol: gatewayv1.HTTPProtocolType}}}, + } + route := &gatewayv1.HTTPRoute{ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "route", UID: "route-uid"}} + cli := fake.NewClientBuilder().WithScheme(parentRefTestScheme(t)).WithObjects(gc, gw).Build() + resolver := &routeParentResolver{} + refs := []gatewayv1.ParentReference{{Name: "gw"}} + parents, err := resolver.Resolve(ctx, cli, logr.Discard(), route, refs) + require.NoError(t, err) + require.Len(t, parents, 1) + require.NoError(t, cli.Delete(ctx, gc)) + switch change { + case "gateway recreated": + require.NoError(t, cli.Delete(ctx, gw)) + gw.ResourceVersion = "" + gw.UID = "new-gateway-uid" + require.NoError(t, cli.Create(ctx, gw)) + case "route recreated": + route.UID = "new-route-uid" + case "class changed": + gw.Spec.GatewayClassName = "different-class" + require.NoError(t, cli.Update(ctx, gw)) + case "route deleted": + resolver.Forget(client.ObjectKeyFromObject(route)) + case "parent removed": + _, err = resolver.Resolve(ctx, cli, logr.Discard(), route, nil) + require.NoError(t, err) + case "class reassigned": + gc.ResourceVersion = "" + gc.Spec.ControllerName = "other.example/controller" + require.NoError(t, cli.Create(ctx, gc)) + } + parents, err = resolver.Resolve(ctx, cli, logr.Discard(), route, refs) + require.NoError(t, err) + require.Empty(t, parents) + }) + } +} + +func TestRouteParentResolverPreservesOwnershipOnReadError(t *testing.T) { + ctx := context.Background() + gc := newParentRefGatewayClass() + gw := &gatewayv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "gw", UID: "gateway-uid"}, + Spec: gatewayv1.GatewaySpec{GatewayClassName: gatewayv1.ObjectName(gc.Name), Listeners: []gatewayv1.Listener{{Name: "http", Port: 80, Protocol: gatewayv1.HTTPProtocolType}}}, + } + route := &gatewayv1.HTTPRoute{ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "route"}} + fail := false + readErr := errors.New("temporary API failure") + cli := fake.NewClientBuilder().WithScheme(parentRefTestScheme(t)).WithObjects(gc, gw). + WithInterceptorFuncs(interceptor.Funcs{Get: func(ctx context.Context, cli client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + if _, ok := obj.(*gatewayv1.GatewayClass); ok && fail { + return readErr + } + return cli.Get(ctx, key, obj, opts...) + }}).Build() + resolver := &routeParentResolver{} + refs := []gatewayv1.ParentReference{{Name: "gw"}} + _, err := resolver.Resolve(ctx, cli, logr.Discard(), route, refs) + require.NoError(t, err) + fail = true + _, err = resolver.Resolve(ctx, cli, logr.Discard(), route, refs) + require.ErrorIs(t, err, readErr) + fail = false + require.NoError(t, cli.Delete(ctx, gc)) + parents, err := resolver.Resolve(ctx, cli, logr.Discard(), route, refs) + require.NoError(t, err) + require.Len(t, parents, 1) +} diff --git a/internal/controller/tcproute_controller.go b/internal/controller/tcproute_controller.go index 94350850..8116f482 100644 --- a/internal/controller/tcproute_controller.go +++ b/internal/controller/tcproute_controller.go @@ -55,6 +55,8 @@ type TCPRouteReconciler struct { //nolint:revive Provider provider.Provider + parentResolver routeParentResolver + Updater status.Updater Readier readiness.ReadinessManager @@ -253,6 +255,7 @@ func (r *TCPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c tr := new(gatewayv1.TCPRoute) if err := r.Get(ctx, req.NamespacedName, tr); err != nil { if client.IgnoreNotFound(err) == nil { + r.parentResolver.Forget(req.NamespacedName) tr.Namespace = req.Namespace tr.Name = req.Name @@ -283,17 +286,12 @@ func (r *TCPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c msg: "Route is accepted", } - gateways, unresolvedParents, err := ParseRouteParentRefs(ctx, r.Client, r.Log, tr, tr.Spec.ParentRefs) + gateways, err := r.parentResolver.Resolve(ctx, r.Client, r.Log, tr, tr.Spec.ParentRefs) if err != nil { return ctrl.Result{}, err } if len(gateways) == 0 { - if unresolvedParents { - // See the HTTPRoute reconciler: an unresolvable parentRef leaves ownership - // unknown rather than disproven, so the data plane must be left alone. - return ctrl.Result{}, nil - } // See the HTTPRoute reconciler: a route that no longer references a // Gateway managed by this controller must have its previously pushed // configuration removed, or the data plane keeps serving it. diff --git a/internal/controller/tlsroute_controller.go b/internal/controller/tlsroute_controller.go index 81286819..1ba9f4f5 100644 --- a/internal/controller/tlsroute_controller.go +++ b/internal/controller/tlsroute_controller.go @@ -55,6 +55,8 @@ type TLSRouteReconciler struct { //nolint:revive Provider provider.Provider + parentResolver routeParentResolver + Updater status.Updater Readier readiness.ReadinessManager @@ -253,6 +255,7 @@ func (r *TLSRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c tr := new(gatewayv1.TLSRoute) if err := r.Get(ctx, req.NamespacedName, tr); err != nil { if client.IgnoreNotFound(err) == nil { + r.parentResolver.Forget(req.NamespacedName) tr.Namespace = req.Namespace tr.Name = req.Name @@ -283,17 +286,12 @@ func (r *TLSRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c msg: "Route is accepted", } - gateways, unresolvedParents, err := ParseRouteParentRefs(ctx, r.Client, r.Log, tr, tr.Spec.ParentRefs) + gateways, err := r.parentResolver.Resolve(ctx, r.Client, r.Log, tr, tr.Spec.ParentRefs) if err != nil { return ctrl.Result{}, err } if len(gateways) == 0 { - if unresolvedParents { - // See the HTTPRoute reconciler: an unresolvable parentRef leaves ownership - // unknown rather than disproven, so the data plane must be left alone. - return ctrl.Result{}, nil - } // See the HTTPRoute reconciler: a route that no longer references a // Gateway managed by this controller must have its previously pushed // configuration removed, or the data plane keeps serving it. diff --git a/internal/controller/udproute_controller.go b/internal/controller/udproute_controller.go index 846ce3c8..3f3255d4 100644 --- a/internal/controller/udproute_controller.go +++ b/internal/controller/udproute_controller.go @@ -55,6 +55,8 @@ type UDPRouteReconciler struct { //nolint:revive Provider provider.Provider + parentResolver routeParentResolver + Updater status.Updater Readier readiness.ReadinessManager @@ -253,6 +255,7 @@ func (r *UDPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c tr := new(gatewayv1.UDPRoute) if err := r.Get(ctx, req.NamespacedName, tr); err != nil { if client.IgnoreNotFound(err) == nil { + r.parentResolver.Forget(req.NamespacedName) tr.Namespace = req.Namespace tr.Name = req.Name @@ -283,17 +286,12 @@ func (r *UDPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c msg: "Route is accepted", } - gateways, unresolvedParents, err := ParseRouteParentRefs(ctx, r.Client, r.Log, tr, tr.Spec.ParentRefs) + gateways, err := r.parentResolver.Resolve(ctx, r.Client, r.Log, tr, tr.Spec.ParentRefs) if err != nil { return ctrl.Result{}, err } if len(gateways) == 0 { - if unresolvedParents { - // See the HTTPRoute reconciler: an unresolvable parentRef leaves ownership - // unknown rather than disproven, so the data plane must be left alone. - return ctrl.Result{}, nil - } // See the HTTPRoute reconciler: a route that no longer references a // Gateway managed by this controller must have its previously pushed // configuration removed, or the data plane keeps serving it. diff --git a/internal/controller/utils.go b/internal/controller/utils.go index 0b961e5a..fe8b8e92 100644 --- a/internal/controller/utils.go +++ b/internal/controller/utils.go @@ -337,20 +337,15 @@ func parentRefTargetsListenerExplicitly(parentRef gatewayv1.ParentReference) boo return parentRef.Port != nil } -// ParseRouteParentRefs resolves the parentRefs of a route to the Gateways this -// controller manages. The second return value reports that at least one -// parentRef points to an existing Gateway whose GatewayClass does not exist. -// Callers preserve the existing configuration while GatewayClass ownership -// cannot be determined. Missing Gateways do not keep stale routes active. -func ParseRouteParentRefs( +func parseRouteParentRefs( ctx context.Context, mgrc client.Client, log logr.Logger, route client.Object, parentRefs []gatewayv1.ParentReference, -) ([]RouteParentRefContext, bool, error) { + resolveClass func(*gatewayv1.Gateway) (*gatewayv1.GatewayClass, error), +) ([]RouteParentRefContext, error) { gateways := make([]RouteParentRefContext, 0) - unresolved := false for _, parentRef := range parentRefs { namespace := route.GetNamespace() if parentRef.Namespace != nil { @@ -371,21 +366,15 @@ func ParseRouteParentRefs( // A deleted Gateway no longer authorizes its routes to serve traffic. continue } - return nil, false, fmt.Errorf("failed to retrieve gateway for route: %w", err) + return nil, fmt.Errorf("failed to retrieve gateway for route: %w", err) } - gatewayClass := gatewayv1.GatewayClass{} - if err := mgrc.Get(ctx, client.ObjectKey{ - Name: string(gateway.Spec.GatewayClassName), - }, &gatewayClass); err != nil { - if client.IgnoreNotFound(err) == nil { - unresolved = true - continue - } - return nil, false, fmt.Errorf("failed to retrieve gatewayclass for gateway: %w", err) + gatewayClass, err := resolveClass(&gateway) + if err != nil { + return nil, fmt.Errorf("failed to retrieve gatewayclass for gateway: %w", err) } - if string(gatewayClass.Spec.ControllerName) != config.ControllerConfig.ControllerName { + if gatewayClass == nil || string(gatewayClass.Spec.ControllerName) != config.ControllerConfig.ControllerName { continue } @@ -520,7 +509,7 @@ func ParseRouteParentRefs( } } - return gateways, unresolved, nil + return gateways, nil } // reuseUnchangedListenerStatus keeps the previously published status when diff --git a/internal/controller/utils_parentref_test.go b/internal/controller/utils_parentref_test.go index 9d026aef..5624e62c 100644 --- a/internal/controller/utils_parentref_test.go +++ b/internal/controller/utils_parentref_test.go @@ -94,7 +94,7 @@ func TestParseRouteParentRefs_ReasonOrderIndependent(t *testing.T) { cli := fake.NewClientBuilder().WithScheme(scheme). WithObjects(newParentRefGatewayClass(), gw).Build() - got, _, err := ParseRouteParentRefs(context.Background(), cli, logr.Discard(), route, + got, err := (&routeParentResolver{}).Resolve(context.Background(), cli, logr.Discard(), route, []gatewayv1.ParentReference{{Name: "gw"}}) require.NoError(t, err) require.Len(t, got, 1) @@ -145,7 +145,7 @@ func TestParseRouteParentRefs_ExplicitListenerMatch(t *testing.T) { port8080 := gatewayv1.PortNumber(8080) t.Run("invalid explicit ref is not satisfied by another gateway's listener", func(t *testing.T) { - got, _, err := ParseRouteParentRefs(context.Background(), cli, logr.Discard(), route, + got, err := (&routeParentResolver{}).Resolve(context.Background(), cli, logr.Discard(), route, []gatewayv1.ParentReference{ // Explicit sectionName "web" on gw-a, which has no such listener. {Name: "gw-a", SectionName: &webSection}, @@ -171,7 +171,7 @@ func TestParseRouteParentRefs_ExplicitListenerMatch(t *testing.T) { }) t.Run("explicit sectionName on the owning gateway is an explicit match", func(t *testing.T) { - got, _, err := ParseRouteParentRefs(context.Background(), cli, logr.Discard(), route, + got, err := (&routeParentResolver{}).Resolve(context.Background(), cli, logr.Discard(), route, []gatewayv1.ParentReference{{Name: "gw-b", SectionName: &webSection}}) require.NoError(t, err) require.Len(t, got, 1) @@ -180,7 +180,7 @@ func TestParseRouteParentRefs_ExplicitListenerMatch(t *testing.T) { }) t.Run("explicit port on the owning gateway is an explicit match", func(t *testing.T) { - got, _, err := ParseRouteParentRefs(context.Background(), cli, logr.Discard(), route, + got, err := (&routeParentResolver{}).Resolve(context.Background(), cli, logr.Discard(), route, []gatewayv1.ParentReference{{Name: "gw-b", Port: &port8080}}) require.NoError(t, err) require.Len(t, got, 1) @@ -189,7 +189,7 @@ func TestParseRouteParentRefs_ExplicitListenerMatch(t *testing.T) { }) t.Run("implicit ref is not an explicit match", func(t *testing.T) { - got, _, err := ParseRouteParentRefs(context.Background(), cli, logr.Discard(), route, + got, err := (&routeParentResolver{}).Resolve(context.Background(), cli, logr.Discard(), route, []gatewayv1.ParentReference{{Name: "gw-b"}}) require.NoError(t, err) require.Len(t, got, 1) @@ -222,7 +222,7 @@ func TestParseRouteParentRefs_ConflictingTLSModePort(t *testing.T) { cli := fake.NewClientBuilder().WithScheme(scheme). WithObjects(newParentRefGatewayClass(), gw).Build() - got, _, err := ParseRouteParentRefs(context.Background(), cli, logr.Discard(), route, + got, err := (&routeParentResolver{}).Resolve(context.Background(), cli, logr.Discard(), route, []gatewayv1.ParentReference{{Name: "gw"}}) require.NoError(t, err) require.Len(t, got, 1) @@ -232,10 +232,7 @@ func TestParseRouteParentRefs_ConflictingTLSModePort(t *testing.T) { assert.Equal(t, string(gatewayv1.RouteReasonNotAllowedByListeners), cond.Reason) } -// TestParseRouteParentRefs_UnresolvedParents verifies the second return value -// distinguishes missing Gateways from unresolved GatewayClass ownership. -// Missing Gateways require route cleanup, while a missing cluster-scoped -// GatewayClass must not drain every route under it. +// Unknown parents must not prevent cleanup of previously managed configuration. func TestParseRouteParentRefs_UnresolvedParents(t *testing.T) { scheme := parentRefTestScheme(t) @@ -258,46 +255,40 @@ func TestParseRouteParentRefs_UnresolvedParents(t *testing.T) { } for _, tc := range []struct { - name string - objects []client.Object - parentRef string - wantGateways int - wantUnresolved bool + name string + objects []client.Object + parentRef string + wantGateways int }{ { - name: "gateway missing retracts stale routes", - objects: []client.Object{newParentRefGatewayClass()}, - parentRef: "gw", - wantUnresolved: false, + name: "gateway missing retracts stale routes", + objects: []client.Object{newParentRefGatewayClass()}, + parentRef: "gw", }, { - name: "gatewayclass missing", - objects: []client.Object{gw("gw", "apisix")}, - parentRef: "gw", - wantUnresolved: true, + name: "gatewayclass missing", + objects: []client.Object{gw("gw", "apisix")}, + parentRef: "gw", }, { - name: "gateway of another controller", - objects: []client.Object{foreignClass, gw("gw", "other")}, - parentRef: "gw", - wantUnresolved: false, + name: "gateway of another controller", + objects: []client.Object{foreignClass, gw("gw", "other")}, + parentRef: "gw", }, { - name: "gateway of this controller", - objects: []client.Object{newParentRefGatewayClass(), gw("gw", "apisix")}, - parentRef: "gw", - wantGateways: 1, - wantUnresolved: false, + name: "gateway of this controller", + objects: []client.Object{newParentRefGatewayClass(), gw("gw", "apisix")}, + parentRef: "gw", + wantGateways: 1, }, } { t.Run(tc.name, func(t *testing.T) { cli := fake.NewClientBuilder().WithScheme(scheme).WithObjects(tc.objects...).Build() - got, unresolved, err := ParseRouteParentRefs(context.Background(), cli, logr.Discard(), route, + got, err := (&routeParentResolver{}).Resolve(context.Background(), cli, logr.Discard(), route, []gatewayv1.ParentReference{{Name: gatewayv1.ObjectName(tc.parentRef)}}) require.NoError(t, err) assert.Len(t, got, tc.wantGateways) - assert.Equal(t, tc.wantUnresolved, unresolved) }) } } diff --git a/internal/provider/api7ee/gateway_delete_test.go b/internal/provider/api7ee/gateway_delete_test.go new file mode 100644 index 00000000..5ecd3ac9 --- /dev/null +++ b/internal/provider/api7ee/gateway_delete_test.go @@ -0,0 +1,102 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you 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 api7ee + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/go-logr/logr" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + k8stypes "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + + adctypes "github.com/apache/apisix-ingress-controller/api/adc" + adcclient "github.com/apache/apisix-ingress-controller/internal/adc/client" + "github.com/apache/apisix-ingress-controller/internal/controller" + "github.com/apache/apisix-ingress-controller/internal/controller/label" + "github.com/apache/apisix-ingress-controller/internal/manager/readiness" + "github.com/apache/apisix-ingress-controller/internal/types" + "github.com/apache/apisix-ingress-controller/internal/utils" +) + +type gatewayDeleteReadier struct{ readiness.ReadinessManager } + +func (gatewayDeleteReadier) Done(client.Object, k8stypes.NamespacedName) {} + +func TestGatewayDeletionRetractsRouteWithUnknownSibling(t *testing.T) { + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _ = json.NewEncoder(w).Encode(adctypes.SyncResult{Status: adctypes.StatusSuccess}) + })) + defer server.Close() + t.Setenv("ADC_SERVER_URL", server.URL) + adc, err := adcclient.New(logr.Discard(), "api7ee", time.Second) + require.NoError(t, err) + d := &api7eeProvider{client: adc, syncCh: make(chan struct{}, 1), log: logr.Discard()} + + gateway := &gatewayv1.Gateway{ + TypeMeta: metav1.TypeMeta{Kind: "Gateway", APIVersion: gatewayv1.GroupVersion.String()}, + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "deleted"}, + } + route := &gatewayv1.HTTPRoute{ + TypeMeta: metav1.TypeMeta{Kind: "HTTPRoute", APIVersion: gatewayv1.GroupVersion.String()}, + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "route"}, + Spec: gatewayv1.HTTPRouteSpec{CommonRouteSpec: gatewayv1.CommonRouteSpec{ + ParentRefs: []gatewayv1.ParentReference{{Name: "deleted"}, {Name: "unresolved"}}, + }}, + } + sibling := &gatewayv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "unresolved"}, + Spec: gatewayv1.GatewaySpec{GatewayClassName: "missing-class"}, + } + proxy := types.NamespacedNameKind{Kind: "GatewayProxy", Namespace: "default", Name: "proxy"} + configs := map[types.NamespacedNameKind]adctypes.Config{proxy: {Name: proxy.String(), BackendType: "apisix", ServerAddrs: []string{"http://apisix:9180"}}} + + for _, obj := range []client.Object{gateway, route} { + resources := &adctypes.Resources{} + resourceType := adctypes.TypeSSL + if obj == route { + resourceType = adctypes.TypeService + resources.Services = []*adctypes.Service{{Metadata: adctypes.Metadata{ID: "stale-route", Labels: label.GenLabel(route)}}} + } + require.NoError(t, adc.UpdateConfig(context.Background(), adcclient.Task{ + Key: utils.NamespacedNameKind(obj), Configs: configs, ResourceTypes: []string{resourceType}, Resources: resources, Labels: label.GenLabel(obj), + })) + } + + require.NoError(t, d.Delete(context.Background(), gateway)) + scheme := runtime.NewScheme() + require.NoError(t, gatewayv1.Install(scheme)) + cli := fake.NewClientBuilder().WithScheme(scheme).WithObjects(route, sibling).Build() + r := &controller.HTTPRouteReconciler{Client: cli, Scheme: scheme, Log: logr.Discard(), Provider: d, Readier: gatewayDeleteReadier{}} + _, err = r.Reconcile(context.Background(), ctrl.Request{NamespacedName: client.ObjectKeyFromObject(route)}) + require.NoError(t, err) + resources, err := adc.GetResources(proxy.String()) + require.NoError(t, err) + require.Empty(t, resources.Services, "deleted Gateway's route survives reconciliation with an unresolved sibling") +} diff --git a/internal/provider/apisix/gateway_delete_test.go b/internal/provider/apisix/gateway_delete_test.go new file mode 100644 index 00000000..dda6d0bb --- /dev/null +++ b/internal/provider/apisix/gateway_delete_test.go @@ -0,0 +1,102 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you 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 apisix + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/go-logr/logr" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + k8stypes "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" + + adctypes "github.com/apache/apisix-ingress-controller/api/adc" + adcclient "github.com/apache/apisix-ingress-controller/internal/adc/client" + "github.com/apache/apisix-ingress-controller/internal/controller" + "github.com/apache/apisix-ingress-controller/internal/controller/label" + "github.com/apache/apisix-ingress-controller/internal/manager/readiness" + "github.com/apache/apisix-ingress-controller/internal/types" + "github.com/apache/apisix-ingress-controller/internal/utils" +) + +type gatewayDeleteReadier struct{ readiness.ReadinessManager } + +func (gatewayDeleteReadier) Done(client.Object, k8stypes.NamespacedName) {} + +func TestGatewayDeletionRetractsRouteWithUnknownSibling(t *testing.T) { + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _ = json.NewEncoder(w).Encode(adctypes.SyncResult{Status: adctypes.StatusSuccess}) + })) + defer server.Close() + t.Setenv("ADC_SERVER_URL", server.URL) + adc, err := adcclient.New(logr.Discard(), "apisix", time.Second) + require.NoError(t, err) + d := &apisixProvider{client: adc, syncCh: make(chan struct{}, 1), log: logr.Discard()} + + gateway := &gatewayv1.Gateway{ + TypeMeta: metav1.TypeMeta{Kind: "Gateway", APIVersion: gatewayv1.GroupVersion.String()}, + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "deleted"}, + } + route := &gatewayv1.HTTPRoute{ + TypeMeta: metav1.TypeMeta{Kind: "HTTPRoute", APIVersion: gatewayv1.GroupVersion.String()}, + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "route"}, + Spec: gatewayv1.HTTPRouteSpec{CommonRouteSpec: gatewayv1.CommonRouteSpec{ + ParentRefs: []gatewayv1.ParentReference{{Name: "deleted"}, {Name: "unresolved"}}, + }}, + } + sibling := &gatewayv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "unresolved"}, + Spec: gatewayv1.GatewaySpec{GatewayClassName: "missing-class"}, + } + proxy := types.NamespacedNameKind{Kind: "GatewayProxy", Namespace: "default", Name: "proxy"} + configs := map[types.NamespacedNameKind]adctypes.Config{proxy: {Name: proxy.String(), BackendType: "apisix", ServerAddrs: []string{"http://apisix:9180"}}} + + for _, obj := range []client.Object{gateway, route} { + resources := &adctypes.Resources{} + resourceType := adctypes.TypeSSL + if obj == route { + resourceType = adctypes.TypeService + resources.Services = []*adctypes.Service{{Metadata: adctypes.Metadata{ID: "stale-route", Labels: label.GenLabel(route)}}} + } + require.NoError(t, adc.UpdateConfig(context.Background(), adcclient.Task{ + Key: utils.NamespacedNameKind(obj), Configs: configs, ResourceTypes: []string{resourceType}, Resources: resources, Labels: label.GenLabel(obj), + })) + } + + require.NoError(t, d.Delete(context.Background(), gateway)) + scheme := runtime.NewScheme() + require.NoError(t, gatewayv1.Install(scheme)) + cli := fake.NewClientBuilder().WithScheme(scheme).WithObjects(route, sibling).Build() + r := &controller.HTTPRouteReconciler{Client: cli, Scheme: scheme, Log: logr.Discard(), Provider: d, Readier: gatewayDeleteReadier{}} + _, err = r.Reconcile(context.Background(), ctrl.Request{NamespacedName: client.ObjectKeyFromObject(route)}) + require.NoError(t, err) + resources, err := adc.GetResources(proxy.String()) + require.NoError(t, err) + require.Empty(t, resources.Services, "deleted Gateway's route survives reconciliation with an unresolved sibling") +} From a4b8e7d42cf024af89f0e53160b29ba6acf0dc4d Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Mon, 19 Oct 2026 15:49:13 +0800 Subject: [PATCH 5/8] fix: clear GatewayProxy plugins once its last Gateway is gone Gateway deletion only evicted the Gateway's certificates, so its own global rule and the GatewayProxy's plugin metadata stayed in the shared cache and on the data plane after the last Gateway referencing the proxy was deleted. Evict the Gateway's global rule, and clear the plugin metadata once no Gateway or IngressClass references the proxy. --- internal/adc/client/client.go | 28 ++++++++++- internal/provider/apisix/provider_test.go | 57 +++++++++++++++++++++++ internal/provider/common/configmanager.go | 13 ++++++ 3 files changed, 96 insertions(+), 2 deletions(-) diff --git a/internal/adc/client/client.go b/internal/adc/client/client.go index 83d617a1..675578fc 100644 --- a/internal/adc/client/client.go +++ b/internal/adc/client/client.go @@ -178,8 +178,12 @@ func (c *Client) applyStoreChanges(args Task, isDelete bool) (StoreDelta, error) delta.Applied = args.Configs } - for _, cfg := range delta.Deleted { - if err := c.Store.Delete(cfg.Name, args.ResourceTypes, args.Labels); err != nil { + for key, cfg := range delta.Deleted { + resourceTypes := args.ResourceTypes + if args.Key.Kind == types.KindGateway { + resourceTypes = gatewayEvictionTypes(resourceTypes, c.ConfigManager.HasResource(key, isGatewayProxyReferrer)) + } + if err := c.Store.Delete(cfg.Name, resourceTypes, args.Labels); err != nil { c.log.Error(err, "store delete failed", "cfg", cfg, "args", args) return StoreDelta{}, errors.Wrap(err, fmt.Sprintf("store delete failed for config %s", cfg.Name)) } @@ -195,6 +199,26 @@ func (c *Client) applyStoreChanges(args Task, isDelete bool) (StoreDelta, error) return delta, nil } +// gatewayEvictionTypes evicts a Gateway's own global rule, stored under its labels, but +// keeps the GatewayProxy's plugin_metadata, shared by every Gateway and IngressClass +// referencing it, until the last of them is gone. Only the store is affected: the +// immediate sync keeps the caller's resource types, as ADC does not label-filter +// global rules. +func gatewayEvictionTypes(resourceTypes []string, referenced bool) []string { + resourceTypes = slices.DeleteFunc(slices.Clone(resourceTypes), func(t string) bool { + return t == adctypes.TypeGlobalRule || t == adctypes.TypePluginMetadata + }) + resourceTypes = append(resourceTypes, adctypes.TypeGlobalRule) + if !referenced { + resourceTypes = append(resourceTypes, adctypes.TypePluginMetadata) + } + return resourceTypes +} + +func isGatewayProxyReferrer(key types.NamespacedNameKind) bool { + return key.Kind == types.KindGateway || key.Kind == types.KindIngressClass +} + func (c *Client) applySync(ctx context.Context, args Task, delta StoreDelta) error { c.syncMu.RLock() defer c.syncMu.RUnlock() diff --git a/internal/provider/apisix/provider_test.go b/internal/provider/apisix/provider_test.go index 214bdbda..55e6bb3b 100644 --- a/internal/provider/apisix/provider_test.go +++ b/internal/provider/apisix/provider_test.go @@ -228,3 +228,60 @@ func TestDeleteGatewayPreservesSharedProxyResources(t *testing.T) { assert.NotEmpty(t, cli.ConfigManager.Get(utils.NamespacedNameKind(other))) assert.NotEmpty(t, cli.ConfigManager.Get(utils.NamespacedNameKind(route))) } + +func TestDeleteGatewayClearsProxyPluginsWithLastReferrer(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _ = json.NewEncoder(w).Encode(adctypes.SyncResult{Status: adctypes.StatusSuccess}) + })) + defer server.Close() + t.Setenv("ADC_SERVER_URL", server.URL) + proxy := types.NamespacedNameKind{Kind: "GatewayProxy", Namespace: "default", Name: "proxy"} + configs := map[types.NamespacedNameKind]adctypes.Config{proxy: { + Name: proxy.String(), BackendType: "apisix", ServerAddrs: []string{"http://apisix:9180"}, + }} + pluginTypes := []string{adctypes.TypeGlobalRule, adctypes.TypeSSL, adctypes.TypePluginMetadata} + plugins := &adctypes.Resources{ + GlobalRules: adctypes.GlobalRule{"cors": map[string]any{}}, + PluginMetadata: adctypes.PluginMetadata{"http-logger": map[string]any{}}, + } + for name, referrers := range map[string][]types.NamespacedNameKind{ + "last referrer": nil, + "IngressClass remains": {{Kind: "IngressClass", Name: "apisix"}}, + "another Gateway remains": {{Kind: "Gateway", Namespace: "default", Name: "remaining"}}, + } { + t.Run(name, func(t *testing.T) { + cli, err := adcclient.New(logr.Discard(), ProviderTypeAPISIX, time.Second) + require.NoError(t, err) + d := &apisixProvider{client: cli, syncCh: make(chan struct{}, 1), log: logr.Discard()} + gateway := &gatewayv1.Gateway{ + TypeMeta: metav1.TypeMeta{Kind: "Gateway", APIVersion: gatewayv1.GroupVersion.String()}, + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "deleted"}, + } + keys := append(referrers, utils.NamespacedNameKind(gateway)) + for _, key := range keys { + labels := map[string]string{label.LabelKind: key.Kind, label.LabelNamespace: key.Namespace, label.LabelName: key.Name} + require.NoError(t, cli.UpdateConfig(context.Background(), adcclient.Task{ + Key: key, Configs: configs, Labels: labels, ResourceTypes: pluginTypes, Resources: plugins, + })) + } + route := types.NamespacedNameKind{Kind: "HTTPRoute", Namespace: "default", Name: "route"} + require.NoError(t, cli.UpdateConfig(context.Background(), adcclient.Task{ + Key: route, Configs: configs, ResourceTypes: []string{adctypes.TypeService}, Resources: &adctypes.Resources{}, + })) + + require.NoError(t, d.Delete(context.Background(), gateway)) + resources, err := cli.GetResources(proxy.String()) + require.NoError(t, err) + if len(referrers) == 0 { + assert.Empty(t, resources.GlobalRules, "an attached route must not keep the GatewayProxy's plugins alive") + assert.Empty(t, resources.PluginMetadata) + } else { + assert.Equal(t, plugins.GlobalRules, resources.GlobalRules) + assert.Equal(t, plugins.PluginMetadata, resources.PluginMetadata) + } + globalRules, err := cli.ListGlobalRules(proxy.String()) + require.NoError(t, err) + assert.Len(t, globalRules, len(referrers), "the deleted Gateway's own global rule must go") + }) + } +} diff --git a/internal/provider/common/configmanager.go b/internal/provider/common/configmanager.go index 4e1d529b..9542400f 100644 --- a/internal/provider/common/configmanager.go +++ b/internal/provider/common/configmanager.go @@ -18,6 +18,7 @@ package common import ( + "slices" "sync" ) @@ -124,6 +125,18 @@ func (s *ConfigManager[K, T]) Update( return discard } +// HasResource reports whether a resource accepted by match still references config. +func (s *ConfigManager[K, T]) HasResource(config K, match func(K) bool) bool { + s.mu.Lock() + defer s.mu.Unlock() + for resource, configKeys := range s.resourceConfigKeys { + if match(resource) && slices.Contains(configKeys, config) { + return true + } + } + return false +} + func (s *ConfigManager[K, T]) Set(key K, cfg T) { s.mu.Lock() defer s.mu.Unlock() From 6f1e33559b7d47667e2993096c8a3b66a30ad866 Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Mon, 19 Oct 2026 15:49:22 +0800 Subject: [PATCH 6/8] chore: narrow the route parent lock and extend the pod termination wait --- internal/controller/route_parent_resolver.go | 9 ++++++--- test/e2e/scaffold/apisix_deployer.go | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/internal/controller/route_parent_resolver.go b/internal/controller/route_parent_resolver.go index e4ff9eb8..89cab1c8 100644 --- a/internal/controller/route_parent_resolver.go +++ b/internal/controller/route_parent_resolver.go @@ -54,14 +54,15 @@ func (r *routeParentResolver) Forget(key client.ObjectKey) { } func (r *routeParentResolver) Resolve(ctx context.Context, cli client.Client, log logr.Logger, route client.Object, refs []gatewayv1.ParentReference) ([]RouteParentRefContext, error) { - r.mu.Lock() - defer r.mu.Unlock() - + // controller-runtime never reconciles one route concurrently, so the lock only + // guards the map, not the parent lookups between the read and the write. routeKey := client.ObjectKeyFromObject(route) + r.mu.Lock() previous := r.parents[routeKey].parents if r.parents[routeKey].uid != route.GetUID() { previous = nil } + r.mu.Unlock() next := make(map[client.ObjectKey]resolvedParentClass) gateways, err := parseRouteParentRefs(ctx, cli, log, route, refs, func(gw *gatewayv1.Gateway) (*gatewayv1.GatewayClass, error) { key := client.ObjectKeyFromObject(gw) @@ -86,6 +87,8 @@ func (r *routeParentResolver) Resolve(ctx context.Context, cli client.Client, lo if err != nil { return nil, err } + r.mu.Lock() + defer r.mu.Unlock() if len(next) == 0 { delete(r.parents, routeKey) } else { diff --git a/test/e2e/scaffold/apisix_deployer.go b/test/e2e/scaffold/apisix_deployer.go index f841b64d..8edcf1d9 100644 --- a/test/e2e/scaffold/apisix_deployer.go +++ b/test/e2e/scaffold/apisix_deployer.go @@ -286,7 +286,7 @@ func (s *APISIXDeployer) deployDataplane(opts *APISIXDeployOptions) *corev1.Serv // A terminating pod can still be Ready and selected by a new port-forward. err = k8s.WaitUntilNumPodsCreatedE(s.GinkgoT, kubectlOpts, metav1.ListOptions{ LabelSelector: "app.kubernetes.io/name=apisix", - }, 0, 30, time.Second) + }, 0, 60, time.Second) Expect(err).ToNot(HaveOccurred(), "waiting for gateway pods to terminate") } From 01c3195c8b5f981b8b6a3d1fe20d70bb8cd6014d Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Mon, 19 Oct 2026 17:34:57 +0800 Subject: [PATCH 7/8] fix: push evicted Gateway plugins after an API7EE Gateway deletion The API7EE Gateway deletion sync is scoped to SSLs, so the Gateway's evicted global rule and the proxy's plugin metadata stayed on the data plane until the next full sync. Schedule a full sync after the deletion, as the APISIX provider already does. --- internal/provider/api7ee/provider.go | 12 ++++++++++-- internal/provider/api7ee/provider_test.go | 3 ++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/internal/provider/api7ee/provider.go b/internal/provider/api7ee/provider.go index 3ba66db8..bca0f4b6 100644 --- a/internal/provider/api7ee/provider.go +++ b/internal/provider/api7ee/provider.go @@ -227,12 +227,20 @@ func (d *api7eeProvider) Delete(ctx context.Context, obj client.Object) error { labels = label.GenLabel(obj) } - return d.client.Delete(ctx, adcclient.Task{ + if err := d.client.Delete(ctx, adcclient.Task{ Key: nnk, Name: nnk.String(), Labels: labels, ResourceTypes: resourceTypes, - }) + }); err != nil { + return err + } + if _, ok := obj.(*gatewayv1.Gateway); ok { + // The scoped sync above only covers SSLs; push the rest of the proxy snapshot, + // including the Gateway's evicted global rule and plugin metadata. + d.syncNotify() + } + return nil } func (d *api7eeProvider) Start(ctx context.Context) error { diff --git a/internal/provider/api7ee/provider_test.go b/internal/provider/api7ee/provider_test.go index a63ccf77..93d6e05f 100644 --- a/internal/provider/api7ee/provider_test.go +++ b/internal/provider/api7ee/provider_test.go @@ -151,7 +151,7 @@ func TestDeleteGatewayPreservesSharedProxyResources(t *testing.T) { t.Setenv("ADC_SERVER_URL", server.URL) cli, err := adcclient.New(logr.Discard(), "api7ee", time.Second) require.NoError(t, err) - d := &api7eeProvider{client: cli, log: logr.Discard()} + d := &api7eeProvider{client: cli, syncCh: make(chan struct{}, 1), log: logr.Discard()} gateway := &gatewayv1.Gateway{ TypeMeta: metav1.TypeMeta{Kind: "Gateway", APIVersion: gatewayv1.GroupVersion.String()}, ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "deleted"}, @@ -186,6 +186,7 @@ func TestDeleteGatewayPreservesSharedProxyResources(t *testing.T) { require.Len(t, resources.Services, 1) assert.Equal(t, "route", resources.Services[0].ID) assert.Equal(t, []string{adctypes.TypeSSL}, received.Task.Opts.IncludeResourceType) + require.Len(t, d.syncCh, 1, "Gateway deletion must schedule a full sync for the evicted plugins") assert.Equal(t, map[string]string{label.LabelResourceKey: label.GenLabel(gateway)[label.LabelResourceKey]}, received.Task.Opts.LabelSelector) _, err = cli.Sync(context.Background()) require.NoError(t, err) From 3e6283ca2542f7e630071478129065108b6ffc4a Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Tue, 20 Oct 2026 08:50:38 +0800 Subject: [PATCH 8/8] fix: consume API7EE sync notifications when periodic sync is disabled API7EE defaults sync_period to 0, and Start returned before reading syncCh in that case. Notifications from Gateway deletion and GatewayProxy updates were dropped, so an evicted Gateway's global rule and plugin metadata never reached the data plane. Keep the loop running and only skip the ticker. --- internal/provider/api7ee/provider.go | 13 +++-- internal/provider/api7ee/provider_test.go | 60 +++++++++++++++++++++++ 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/internal/provider/api7ee/provider.go b/internal/provider/api7ee/provider.go index bca0f4b6..76438d1c 100644 --- a/internal/provider/api7ee/provider.go +++ b/internal/provider/api7ee/provider.go @@ -262,11 +262,14 @@ func (d *api7eeProvider) Start(ctx context.Context) error { }) } - if d.SyncPeriod < 1 { - return nil + // A zero SyncPeriod only disables the periodic sync; syncCh still carries changes + // that the scoped syncs cannot push, such as an evicted Gateway's plugins. + var tick <-chan time.Time + if d.SyncPeriod > 0 { + ticker := time.NewTicker(d.SyncPeriod) + defer ticker.Stop() + tick = ticker.C } - ticker := time.NewTicker(d.SyncPeriod) - defer ticker.Stop() retrier := common.NewRetrier(common.NewExponentialBackoff(RetryBaseDelay, RetryMaxDelay)) @@ -274,7 +277,7 @@ func (d *api7eeProvider) Start(ctx context.Context) error { select { case <-d.syncCh: case <-retrier.C(): - case <-ticker.C: + case <-tick: case <-ctx.Done(): return nil } diff --git a/internal/provider/api7ee/provider_test.go b/internal/provider/api7ee/provider_test.go index 93d6e05f..67dd9b04 100644 --- a/internal/provider/api7ee/provider_test.go +++ b/internal/provider/api7ee/provider_test.go @@ -40,6 +40,8 @@ import ( apiv2 "github.com/apache/apisix-ingress-controller/api/v2" adcclient "github.com/apache/apisix-ingress-controller/internal/adc/client" "github.com/apache/apisix-ingress-controller/internal/controller/label" + "github.com/apache/apisix-ingress-controller/internal/controller/status" + "github.com/apache/apisix-ingress-controller/internal/manager/readiness" "github.com/apache/apisix-ingress-controller/internal/provider" "github.com/apache/apisix-ingress-controller/internal/types" "github.com/apache/apisix-ingress-controller/internal/utils" @@ -196,3 +198,61 @@ func TestDeleteGatewayPreservesSharedProxyResources(t *testing.T) { assert.NotEmpty(t, cli.ConfigManager.Get(utils.NamespacedNameKind(other))) assert.NotEmpty(t, cli.ConfigManager.Get(utils.NamespacedNameKind(route))) } + +func TestStartConsumesSyncNotificationsWithoutSyncPeriod(t *testing.T) { + requests := make(chan struct{}, 10) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + _ = json.NewEncoder(w).Encode(adctypes.SyncResult{Status: adctypes.StatusSuccess}) + requests <- struct{}{} + })) + defer server.Close() + t.Setenv("ADC_SERVER_URL", server.URL) + cli, err := adcclient.New(logr.Discard(), "api7ee", time.Second) + require.NoError(t, err) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + readier := readiness.NewReadinessManager(nil, logr.Discard()) + require.NoError(t, readier.Start(ctx)) + d := &api7eeProvider{ + client: cli, readier: readier, updater: discardUpdater{}, syncCh: make(chan struct{}, 1), log: logr.Discard(), + } + + route := &gatewayv1.HTTPRoute{ + TypeMeta: metav1.TypeMeta{Kind: "HTTPRoute", APIVersion: gatewayv1.GroupVersion.String()}, + ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "route"}, + } + proxy := types.NamespacedNameKind{Kind: "GatewayProxy", Namespace: "default", Name: "proxy"} + labels := label.GenLabel(route) + require.NoError(t, cli.UpdateConfig(ctx, adcclient.Task{ + Key: utils.NamespacedNameKind(route), Labels: labels, ResourceTypes: []string{adctypes.TypeService}, + Configs: map[types.NamespacedNameKind]adctypes.Config{proxy: { + Name: proxy.String(), BackendType: "apisix", ServerAddrs: []string{"http://apisix:9180"}, + }}, + Resources: &adctypes.Resources{Services: []*adctypes.Service{{Metadata: adctypes.Metadata{ID: "route", Labels: labels}}}}, + })) + + done := make(chan error, 1) + go func() { done <- d.Start(ctx) }() + + select { + case <-requests: + case <-time.After(5 * time.Second): + t.Fatal("startup sync did not run") + } + d.syncNotify() + select { + case <-requests: + case err := <-done: + t.Fatalf("Start returned before consuming the notification: %v", err) + case <-time.After(5 * time.Second): + t.Fatal("sync notification was not consumed") + } + cancel() + require.NoError(t, <-done) +} + +type discardUpdater struct{} + +func (discardUpdater) Update(status.Update) {}