This is an automated email from the ASF dual-hosted git repository.
AlinsRan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/apisix-ingress-controller.git
The following commit(s) were added to refs/heads/master by this push:
new c69d50cf fix: retract route configuration when no parent accepts the
route any more (#2858)
c69d50cf is described below
commit c69d50cf6e69ec7bf138f446277110ba4e8fbea7
Author: AlinsRan <[email protected]>
AuthorDate: Fri Sep 11 14:18:09 2026 +0800
fix: retract route configuration when no parent accepts the route any more
(#2858)
---
internal/controller/grpcroute_controller.go | 15 ++
internal/controller/httproute_controller.go | 15 ++
.../httproute_controller_retract_test.go | 177 +++++++++++++++++++++
internal/controller/tcproute_controller.go | 15 ++
internal/controller/tlsroute_controller.go | 15 ++
internal/controller/udproute_controller.go | 15 ++
internal/controller/utils.go | 15 +-
test/e2e/gatewayapi/httproute.go | 122 ++++++++++++++
8 files changed, 387 insertions(+), 2 deletions(-)
diff --git a/internal/controller/grpcroute_controller.go
b/internal/controller/grpcroute_controller.go
index df50f135..b2e0997c 100644
--- a/internal/controller/grpcroute_controller.go
+++ b/internal/controller/grpcroute_controller.go
@@ -291,6 +291,21 @@ func (r *GRPCRouteReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (
if err := r.Provider.Update(ctx, tctx, routeToUpdate); err !=
nil {
return ctrl.Result{}, err
}
+ return ctrl.Result{}, nil
+ }
+
+ // The route still resolves to one of our Gateways but no parent
accepts it any
+ // more, so retract what an earlier reconcile published. The store is
what every
+ // sync pushes, so leaving the entry keeps the data plane serving the
route.
+ // Provider.Delete derives the resource labels from the object Kind,
which is not
+ // set on every object read through the client.
+ gr.TypeMeta = metav1.TypeMeta{
+ Kind: KindGRPCRoute,
+ APIVersion: gatewayv1.GroupVersion.String(),
+ }
+ if err := r.Provider.Delete(ctx, gr); err != nil {
+ r.Log.Error(err, "failed to delete grpcroute", "grpcroute",
utils.NamespacedName(gr))
+ return ctrl.Result{}, err
}
return ctrl.Result{}, nil
}
diff --git a/internal/controller/httproute_controller.go
b/internal/controller/httproute_controller.go
index 285899b8..3c76ab92 100644
--- a/internal/controller/httproute_controller.go
+++ b/internal/controller/httproute_controller.go
@@ -293,6 +293,21 @@ func (r *HTTPRouteReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (
if err := r.Provider.Update(ctx, tctx, routeToUpdate); err !=
nil {
return ctrl.Result{}, err
}
+ return ctrl.Result{}, nil
+ }
+
+ // The route still resolves to one of our Gateways but no parent
accepts it any
+ // more, so retract what an earlier reconcile published. The store is
what every
+ // sync pushes, so leaving the entry keeps the data plane serving the
route.
+ // Provider.Delete derives the resource labels from the object Kind,
which is not
+ // set on every object read through the client.
+ hr.TypeMeta = metav1.TypeMeta{
+ Kind: KindHTTPRoute,
+ APIVersion: gatewayv1.GroupVersion.String(),
+ }
+ if err := r.Provider.Delete(ctx, hr); err != nil {
+ r.Log.Error(err, "failed to delete httproute", "httproute",
utils.NamespacedName(hr))
+ return ctrl.Result{}, err
}
return ctrl.Result{}, nil
}
diff --git a/internal/controller/httproute_controller_retract_test.go
b/internal/controller/httproute_controller_retract_test.go
new file mode 100644
index 00000000..857c2fa5
--- /dev/null
+++ b/internal/controller/httproute_controller_retract_test.go
@@ -0,0 +1,177 @@
+// 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/assert"
+ "github.com/stretchr/testify/require"
+ "k8s.io/apimachinery/pkg/api/meta"
+ metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+ "k8s.io/apimachinery/pkg/runtime"
+ k8stypes "k8s.io/apimachinery/pkg/types"
+ clientgoscheme "k8s.io/client-go/kubernetes/scheme"
+ 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"
+
+ "github.com/apache/apisix-ingress-controller/api/v1alpha1"
+ "github.com/apache/apisix-ingress-controller/internal/controller/config"
+ "github.com/apache/apisix-ingress-controller/internal/manager/readiness"
+)
+
+const (
+ retractGatewayNamespace = "infra"
+ retractRouteNamespace = "tenant"
+ retractRouteName = "route"
+)
+
+// newHTTPRouteRetractFixture builds a Gateway of our class in
retractGatewayNamespace
+// and an HTTPRoute in retractRouteNamespace that names it as its parent. from
+// controls the listener's allowedRoutes, which is what revoking
cross-namespace
+// access changes.
+func newHTTPRouteRetractFixture(
+ t *testing.T,
+ from gatewayv1.FromNamespaces,
+) (*HTTPRouteReconciler, *recordingProvider, *recordingUpdater) {
+ t.Helper()
+
+ scheme := runtime.NewScheme()
+ require.NoError(t, clientgoscheme.AddToScheme(scheme))
+ require.NoError(t, gatewayv1.Install(scheme))
+ require.NoError(t, v1alpha1.AddToScheme(scheme))
+
+ gatewayClass := &gatewayv1.GatewayClass{
+ ObjectMeta: metav1.ObjectMeta{Name: "apisix"},
+ Spec: gatewayv1.GatewayClassSpec{
+ ControllerName:
gatewayv1.GatewayController(config.ControllerConfig.ControllerName),
+ },
+ }
+ gateway := &gatewayv1.Gateway{
+ ObjectMeta: metav1.ObjectMeta{Namespace:
retractGatewayNamespace, Name: "gw"},
+ Spec: gatewayv1.GatewaySpec{
+ GatewayClassName: "apisix",
+ Listeners: []gatewayv1.Listener{{
+ Name: "http",
+ Protocol: gatewayv1.HTTPProtocolType,
+ Port: 80,
+ AllowedRoutes: &gatewayv1.AllowedRoutes{
+ Namespaces:
&gatewayv1.RouteNamespaces{From: &from},
+ },
+ }},
+ },
+ }
+ route := &gatewayv1.HTTPRoute{
+ ObjectMeta: metav1.ObjectMeta{Namespace: retractRouteNamespace,
Name: retractRouteName},
+ Spec: gatewayv1.HTTPRouteSpec{
+ // A route with hostnames of its own is what makes
filterHostnames fail
+ // once no listener matches, which is how a route-wide
reason used to
+ // overwrite the parent's own.
+ Hostnames: []gatewayv1.Hostname{"tenant.example"},
+ CommonRouteSpec: gatewayv1.CommonRouteSpec{
+ ParentRefs: []gatewayv1.ParentReference{{
+ Name:
gatewayv1.ObjectName(gateway.Name),
+ Namespace:
(*gatewayv1.Namespace)(&gateway.Namespace),
+ }},
+ },
+ },
+ }
+
+ cli := fake.NewClientBuilder().WithScheme(scheme).
+ WithObjects([]client.Object{gatewayClass, gateway, route}...).
+ WithStatusSubresource(route).
+ Build()
+
+ readier := readiness.NewReadinessManager(cli, logr.Discard())
+ require.NoError(t, readier.Start(context.Background()))
+
+ prov := &recordingProvider{}
+ updater := &recordingUpdater{}
+ return &HTTPRouteReconciler{
+ Client: cli,
+ Scheme: scheme,
+ Log: logr.Discard(),
+ Provider: prov,
+ Updater: updater,
+ Readier: readier,
+ }, prov, updater
+}
+
+func reconcileRetractHTTPRoute(t *testing.T, r *HTTPRouteReconciler)
(ctrl.Result, error) {
+ t.Helper()
+ return r.Reconcile(context.Background(), ctrl.Request{
+ NamespacedName: k8stypes.NamespacedName{Namespace:
retractRouteNamespace, Name: retractRouteName},
+ })
+}
+
+var retractRouteKey = k8stypes.NamespacedName{Namespace:
retractRouteNamespace, Name: retractRouteName}
+
+// Narrowing a listener's allowedRoutes leaves the HTTPRoute in place but
stops it
+// being accepted. The configuration an earlier reconcile published must be
+// retracted, otherwise the data plane keeps serving a route the Gateway no
longer
+// admits and only deleting the HTTPRoute clears it.
+func TestHTTPRouteReconcile_RetractsWhenListenerStopsAllowingRoute(t
*testing.T) {
+ r, prov, updater := newHTTPRouteRetractFixture(t,
gatewayv1.NamespacesFromSame)
+
+ result, err := reconcileRetractHTTPRoute(t, r)
+
+ require.NoError(t, err)
+ assert.Equal(t, ctrl.Result{}, result)
+ assert.Equal(t, []k8stypes.NamespacedName{retractRouteKey},
prov.deleted)
+ assert.Zero(t, prov.updated, "a route that is not accepted must not be
published")
+
+ // The reason must say why this parent rejected the route. The
route-wide
+ // status derived from filterHostnames would otherwise report
+ // NoMatchingListenerHostname, which is a symptom of nothing having
matched
+ // rather than the cause.
+ require.Len(t, updater.updates, 1)
+ mutated, ok :=
updater.updates[0].Mutator.Mutate(&gatewayv1.HTTPRoute{}).(*gatewayv1.HTTPRoute)
+ require.True(t, ok)
+ require.Len(t, mutated.Status.Parents, 1)
+ accepted :=
meta.FindStatusCondition(mutated.Status.Parents[0].Conditions,
string(gatewayv1.RouteConditionAccepted))
+ require.NotNil(t, accepted)
+ assert.Equal(t, metav1.ConditionFalse, accepted.Status)
+ assert.Equal(t, string(gatewayv1.RouteReasonNotAllowedByListeners),
accepted.Reason)
+}
+
+// An accepted route must still be published and must not be retracted.
+func TestHTTPRouteReconcile_PublishesAcceptedRoute(t *testing.T) {
+ r, prov, _ := newHTTPRouteRetractFixture(t, gatewayv1.NamespacesFromAll)
+
+ _, err := reconcileRetractHTTPRoute(t, r)
+
+ require.NoError(t, err)
+ assert.Empty(t, prov.deleted, "an accepted route must not be retracted")
+ assert.Equal(t, 1, prov.updated)
+}
+
+// A provider failure while retracting must surface so the reconcile is
retried.
+func TestHTTPRouteReconcile_RetractErrorIsReturned(t *testing.T) {
+ r, prov, _ := newHTTPRouteRetractFixture(t,
gatewayv1.NamespacesFromSame)
+ prov.deleteErr = errors.New("provider unavailable")
+
+ _, err := reconcileRetractHTTPRoute(t, r)
+
+ require.Error(t, err)
+ assert.Contains(t, err.Error(), "provider unavailable")
+}
diff --git a/internal/controller/tcproute_controller.go
b/internal/controller/tcproute_controller.go
index 4df650a3..b8d6f97e 100644
--- a/internal/controller/tcproute_controller.go
+++ b/internal/controller/tcproute_controller.go
@@ -381,6 +381,21 @@ func (r *TCPRouteReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (c
if err := r.Provider.Update(ctx, tctx, routeToUpdate); err !=
nil {
return ctrl.Result{}, err
}
+ return ctrl.Result{}, nil
+ }
+
+ // The route still resolves to one of our Gateways but no parent
accepts it any
+ // more, so retract what an earlier reconcile published. The store is
what every
+ // sync pushes, so leaving the entry keeps the data plane serving the
route.
+ // Provider.Delete derives the resource labels from the object Kind,
which is not
+ // set on every object read through the client.
+ tr.TypeMeta = metav1.TypeMeta{
+ Kind: KindTCPRoute,
+ APIVersion: gatewayv1.GroupVersion.String(),
+ }
+ if err := r.Provider.Delete(ctx, tr); err != nil {
+ r.Log.Error(err, "failed to delete tcproute", "tcproute",
utils.NamespacedName(tr))
+ return ctrl.Result{}, err
}
return ctrl.Result{}, nil
}
diff --git a/internal/controller/tlsroute_controller.go
b/internal/controller/tlsroute_controller.go
index 3b23f000..5bbd5a59 100644
--- a/internal/controller/tlsroute_controller.go
+++ b/internal/controller/tlsroute_controller.go
@@ -373,6 +373,21 @@ func (r *TLSRouteReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (c
if err := r.Provider.Update(ctx, tctx, routeToUpdate); err !=
nil {
return ctrl.Result{}, err
}
+ return ctrl.Result{}, nil
+ }
+
+ // The route still resolves to one of our Gateways but no parent
accepts it any
+ // more, so retract what an earlier reconcile published. The store is
what every
+ // sync pushes, so leaving the entry keeps the data plane serving the
route.
+ // Provider.Delete derives the resource labels from the object Kind,
which is not
+ // set on every object read through the client.
+ tr.TypeMeta = metav1.TypeMeta{
+ Kind: types.KindTLSRoute,
+ APIVersion: gatewayv1.GroupVersion.String(),
+ }
+ if err := r.Provider.Delete(ctx, tr); err != nil {
+ r.Log.Error(err, "failed to delete tlsroute", "tlsroute",
utils.NamespacedName(tr))
+ return ctrl.Result{}, err
}
return ctrl.Result{}, nil
}
diff --git a/internal/controller/udproute_controller.go
b/internal/controller/udproute_controller.go
index 31a3ab2a..3ca88907 100644
--- a/internal/controller/udproute_controller.go
+++ b/internal/controller/udproute_controller.go
@@ -381,6 +381,21 @@ func (r *UDPRouteReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (c
if err := r.Provider.Update(ctx, tctx, routeToUpdate); err !=
nil {
return ctrl.Result{}, err
}
+ return ctrl.Result{}, nil
+ }
+
+ // The route still resolves to one of our Gateways but no parent
accepts it any
+ // more, so retract what an earlier reconcile published. The store is
what every
+ // sync pushes, so leaving the entry keeps the data plane serving the
route.
+ // Provider.Delete derives the resource labels from the object Kind,
which is not
+ // set on every object read through the client.
+ tr.TypeMeta = metav1.TypeMeta{
+ Kind: KindUDPRoute,
+ APIVersion: gatewayv1.GroupVersion.String(),
+ }
+ if err := r.Provider.Delete(ctx, tr); err != nil {
+ r.Log.Error(err, "failed to delete udproute", "udproute",
utils.NamespacedName(tr))
+ return ctrl.Result{}, err
}
return ctrl.Result{}, nil
}
diff --git a/internal/controller/utils.go b/internal/controller/utils.go
index deceecdd..14bda2bb 100644
--- a/internal/controller/utils.go
+++ b/internal/controller/utils.go
@@ -268,9 +268,20 @@ func SetRouteConditionAccepted(routeParentStatus
*gatewayv1.RouteParentStatus, g
condition.Reason =
string(gatewayv1.RouteReasonNoMatchingListenerHostname)
}
- if !IsConditionPresentAndEqual(routeParentStatus.Conditions, condition)
&& !slices.ContainsFunc(routeParentStatus.Conditions, func(item
metav1.Condition) bool {
- return item.Type == condition.Type && item.Status ==
metav1.ConditionFalse && condition.Status == metav1.ConditionTrue
+ // ParseRouteParentRefs already recorded why this particular parent
rejected the
+ // route: NotAllowedByListeners, NoMatchingParent,
NoMatchingListenerHostname.
+ // status and message here are route-wide, derived from whatever failed
first
+ // across every parent, so they must not overwrite that. Leaving them
to do so
+ // reports a route rejected by allowedRoutes as
NoMatchingListenerHostname,
+ // because filterHostnames finds no listener to intersect against once
nothing
+ // matched, and its generic error lands here.
+ if slices.ContainsFunc(routeParentStatus.Conditions, func(item
metav1.Condition) bool {
+ return item.Type == condition.Type && item.Status ==
metav1.ConditionFalse
}) {
+ return
+ }
+
+ if !IsConditionPresentAndEqual(routeParentStatus.Conditions, condition)
{
routeParentStatus.Conditions =
MergeCondition(routeParentStatus.Conditions, condition)
}
}
diff --git a/test/e2e/gatewayapi/httproute.go b/test/e2e/gatewayapi/httproute.go
index 48cc9e47..9635d335 100644
--- a/test/e2e/gatewayapi/httproute.go
+++ b/test/e2e/gatewayapi/httproute.go
@@ -220,6 +220,128 @@ spec:
})
})
+ Context("HTTPRoute revoked by its listener", func() {
+ // The listener starts out admitting HTTPRoute and is then
narrowed to
+ // GRPCRoute only. The route object is untouched throughout,
which is the
+ // point: revoking a route's access must not require editing
the route.
+ var gatewayAllowingKinds = `
+apiVersion: gateway.networking.k8s.io/v1
+kind: Gateway
+metadata:
+ name: %s
+spec:
+ gatewayClassName: %s
+ listeners:
+ - name: http1
+ protocol: HTTP
+ port: 80
+ allowedRoutes:
+ kinds:
+ - group: gateway.networking.k8s.io
+ kind: %s
+ infrastructure:
+ parametersRef:
+ group: apisix.apache.org
+ kind: GatewayProxy
+ name: apisix-proxy-config
+`
+
+ var route = `
+apiVersion: gateway.networking.k8s.io/v1
+kind: HTTPRoute
+metadata:
+ name: httpbin
+spec:
+ parentRefs:
+ - name: %s
+ hostnames:
+ - httpbin.example
+ rules:
+ - matches:
+ - path:
+ type: Exact
+ value: /get
+ backendRefs:
+ - name: httpbin-service-e2e-test
+ port: 80
+`
+
+ // allowKind rewrites the listener to admit only the given
route kind.
+ var allowKind = func(kind string) {
+ Expect(s.CreateResourceFromString(
+ fmt.Sprintf(gatewayAllowingKinds,
s.Namespace(), s.Namespace(), kind),
+ )).NotTo(HaveOccurred(), "applying Gateway allowing
"+kind)
+ }
+
+ BeforeEach(func() {
+ By("create GatewayProxy")
+
Expect(s.CreateResourceFromString(s.GetGatewayProxySpec())).NotTo(HaveOccurred(),
"creating GatewayProxy")
+
+ By("create GatewayClass")
+
Expect(s.CreateResourceFromString(s.GetGatewayClassYaml())).NotTo(HaveOccurred(),
"creating GatewayClass")
+ s.RetryAssertion(func() string {
+ gcyaml, _ := s.GetResourceYaml("GatewayClass",
s.Namespace())
+ return gcyaml
+ }).Should(ContainSubstring("message: the gatewayclass
has been accepted by the apisix-ingress-controller"),
+ "check GatewayClass condition")
+
+ By("create Gateway admitting HTTPRoute")
+ allowKind("HTTPRoute")
+ s.RetryAssertion(func() string {
+ gwyaml, _ := s.GetResourceYaml("Gateway",
s.Namespace())
+ return gwyaml
+ }).Should(ContainSubstring("message: the gateway has
been accepted by the apisix-ingress-controller"),
+ "check Gateway condition status")
+ })
+
+ It("stops serving the route and resumes when the listener
admits it again", func() {
+ By("create HTTPRoute")
+ s.ResourceApplied("HTTPRoute", "httpbin",
fmt.Sprintf(route, s.Namespace()), 1)
+
+ s.RequestAssert(&scaffold.RequestAssert{
+ Method: "GET",
+ Path: "/get",
+ Host: "httpbin.example",
+ Check:
scaffold.WithExpectedStatus(http.StatusOK),
+ Timeout: time.Second * 30,
+ Interval: time.Second * 2,
+ })
+
+ By("narrow the listener to GRPCRoute, leaving the
HTTPRoute untouched")
+ allowKind("GRPCRoute")
+
+ By("the route reports that no listener accepts it")
+ s.RetryAssertion(func() string {
+ routeYaml, _ := s.GetResourceYaml("HTTPRoute",
"httpbin")
+ return routeYaml
+ }).Should(ContainSubstring("reason:
NotAllowedByListeners"), "check HTTPRoute condition")
+
+ By("and the data plane stops serving it")
+ // Without the retraction the previously published
route keeps
+ // forwarding, so the status and the data plane
disagree until the
+ // HTTPRoute itself is deleted.
+ s.RequestAssert(&scaffold.RequestAssert{
+ Method: "GET",
+ Path: "/get",
+ Host: "httpbin.example",
+ Check:
scaffold.WithExpectedStatus(http.StatusNotFound),
+ Timeout: time.Second * 30,
+ Interval: time.Second * 2,
+ })
+
+ By("restore the listener and the route is served again")
+ allowKind("HTTPRoute")
+ s.RequestAssert(&scaffold.RequestAssert{
+ Method: "GET",
+ Path: "/get",
+ Host: "httpbin.example",
+ Check:
scaffold.WithExpectedStatus(http.StatusOK),
+ Timeout: time.Second * 30,
+ Interval: time.Second * 2,
+ })
+ })
+ })
+
Context("HTTPRoute with Multiple Gateway", Serial, func() {
var additionalGatewayGroupID string
var additionalSvc *corev1.Service
