fix: surface Gateway API route translation failures in status (backport apache/apisix-ingress-controller#2898)

internal/controller/grpcroute_controller.go

@@ -256,6 +256,17 @@ func (r *GRPCRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 
 	ProcessBackendTrafficPolicy(r.Client, r.Log, tctx)
 
+	// Translate and sync before writing Accepted=True. A translation or
+	// buildConfig failure from Provider.Update used to leave a green status
+	// and only requeue, matching neither Ingress nor ApisixGlobalRule.
+	var syncErr error
+	if isRouteAccepted(gateways) && err == nil {
+		if syncErr = r.Provider.Update(ctx, tctx, gr); syncErr != nil {
+			acceptStatus.status = false
+			acceptStatus.msg = syncErr.Error()
+		}
+	}
+
 	// TODO: diff the old and new status
 	gr.Status.Parents = make([]gatewayv1.RouteParentStatus, 0, len(gateways))
 	for _, gateway := range gateways {
@@ -287,11 +298,7 @@ func (r *GRPCRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 	UpdateStatus(r.Updater, r.Log, tctx)
 
 	if isRouteAccepted(gateways) && err == nil {
-		routeToUpdate := gr
-		if err := r.Provider.Update(ctx, tctx, routeToUpdate); err != nil {
-			return ctrl.Result{}, err
-		}
-		return ctrl.Result{}, nil
+		return ctrl.Result{}, syncErr
 	}
 
 	// The route still resolves to one of our Gateways but no parent accepts it any

internal/controller/grpcroute_controller_test.go

@@ -0,0 +1,133 @@
+// 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"
+)
+
+func newGRPCRouteFixture(t *testing.T) (*GRPCRouteReconciler, *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))
+
+	from := gatewayv1.NamespacesFromAll
+	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.GRPCRoute{
+		ObjectMeta: metav1.ObjectMeta{Namespace: retractRouteNamespace, Name: retractRouteName},
+		Spec: gatewayv1.GRPCRouteSpec{
+			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 &GRPCRouteReconciler{
+		Client:   cli,
+		Scheme:   scheme,
+		Log:      logr.Discard(),
+		Provider: prov,
+		Updater:  updater,
+		Readier:  readier,
+	}, prov, updater
+}
+
+func grpcAcceptedConditionFromUpdater(t *testing.T, updater *recordingUpdater) *metav1.Condition {
+	t.Helper()
+	require.NotEmpty(t, updater.updates)
+	mutated, ok := updater.updates[0].Mutator.Mutate(&gatewayv1.GRPCRoute{}).(*gatewayv1.GRPCRoute)
+	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)
+	return accepted
+}
+
+// A translation / Provider.Update failure must be visible on the route status
+// instead of Accepted=True plus a silent requeue.
+func TestGRPCRouteReconcile_UpdateErrorIsVisibleOnStatus(t *testing.T) {
+	r, prov, updater := newGRPCRouteFixture(t)
+	prov.updateErr = errors.New("translation failed")
+
+	_, err := r.Reconcile(context.Background(), ctrl.Request{
+		NamespacedName: k8stypes.NamespacedName{Namespace: retractRouteNamespace, Name: retractRouteName},
+	})
+
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "translation failed")
+	assert.Equal(t, 1, prov.updated)
+	assert.Empty(t, prov.deleted)
+
+	accepted := grpcAcceptedConditionFromUpdater(t, updater)
+	assert.Equal(t, metav1.ConditionFalse, accepted.Status)
+	assert.Contains(t, accepted.Message, "translation failed")
+}

internal/controller/httproute_controller.go

@@ -273,6 +273,22 @@ func (r *HTTPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		acceptStatus.msg = err.Error()
 	}
 
+	// Translate and sync before writing Accepted=True. A translation or
+	// buildConfig failure from Provider.Update used to leave a green status
+	// and only requeue, matching neither Ingress nor ApisixGlobalRule.
+	var syncErr error
+	if isRouteAccepted(gateways) && err == nil {
+		routeToUpdate := hr
+		if filteredHTTPRoute != nil {
+			r.Log.V(1).Info("filtered httproute", "httproute", utils.NamespacedName(filteredHTTPRoute))
+			routeToUpdate = filteredHTTPRoute
+		}
+		if syncErr = r.Provider.Update(ctx, tctx, routeToUpdate); syncErr != nil {
+			acceptStatus.status = false
+			acceptStatus.msg = syncErr.Error()
+		}
+	}
+
 	// TODO: diff the old and new status
 	hr.Status.Parents = make([]gatewayv1.RouteParentStatus, 0, len(gateways))
 	for _, gateway := range gateways {
@@ -304,15 +320,7 @@ func (r *HTTPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 	UpdateStatus(r.Updater, r.Log, tctx)
 
 	if isRouteAccepted(gateways) && err == nil {
-		routeToUpdate := hr
-		if filteredHTTPRoute != nil {
-			r.Log.V(1).Info("filtered httproute", "httproute", utils.NamespacedName(filteredHTTPRoute))
-			routeToUpdate = filteredHTTPRoute
-		}
-		if err := r.Provider.Update(ctx, tctx, routeToUpdate); err != nil {
-			return ctrl.Result{}, err
-		}
-		return ctrl.Result{}, nil
+		return ctrl.Result{}, syncErr
 	}
 
 	// The route still resolves to one of our Gateways but no parent accepts it any

internal/controller/httproute_controller_retract_test.go

@@ -25,6 +25,7 @@ import (
 	"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"
@@ -36,6 +37,7 @@ import (
 
 	"github.com/apache/apisix-ingress-controller/api/v1alpha1"
 	"github.com/apache/apisix-ingress-controller/internal/controller/config"
+	"github.com/apache/apisix-ingress-controller/internal/controller/indexer"
 	"github.com/apache/apisix-ingress-controller/internal/manager/readiness"
 )
 
@@ -52,7 +54,7 @@ const (
 func newHTTPRouteRetractFixture(
 	t *testing.T,
 	from gatewayv1.FromNamespaces,
-) (*HTTPRouteReconciler, *recordingProvider) {
+) (*HTTPRouteReconciler, *recordingProvider, *recordingUpdater) {
 	t.Helper()
 
 	scheme := runtime.NewScheme()
@@ -95,20 +97,25 @@ func newHTTPRouteRetractFixture(
 	cli := fake.NewClientBuilder().WithScheme(scheme).
 		WithObjects([]client.Object{gatewayClass, gateway, route}...).
 		WithStatusSubresource(route).
+		// The reconcile lists HTTPRoutePolicies by target, which the fake client
+		// only serves once the index exists. No policy is under test here.
+		WithIndex(&v1alpha1.HTTPRoutePolicy{}, indexer.PolicyTargetRefs,
+			func(client.Object) []string { return nil }).
 		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:  &recordingUpdater{},
+		Updater:  updater,
 		Readier:  readier,
-	}, prov
+	}, prov, updater
 }
 
 func reconcileRetractHTTPRoute(t *testing.T, r *HTTPRouteReconciler) (ctrl.Result, error) {
@@ -125,7 +132,7 @@ var retractRouteKey = k8stypes.NamespacedName{Namespace: retractRouteNamespace,
 // 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 := newHTTPRouteRetractFixture(t, gatewayv1.NamespacesFromSame)
+	r, prov, _ := newHTTPRouteRetractFixture(t, gatewayv1.NamespacesFromSame)
 
 	result, err := reconcileRetractHTTPRoute(t, r)
 
@@ -137,22 +144,54 @@ func TestHTTPRouteReconcile_RetractsWhenListenerStopsAllowingRoute(t *testing.T)
 
 // An accepted route must still be published and must not be retracted.
 func TestHTTPRouteReconcile_PublishesAcceptedRoute(t *testing.T) {
-	r, prov := newHTTPRouteRetractFixture(t, gatewayv1.NamespacesFromAll)
+	r, prov, updater := 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)
+
+	accepted := acceptedConditionFromUpdater(t, updater)
+	assert.Equal(t, metav1.ConditionTrue, accepted.Status)
 }
 
 // A provider failure while retracting must surface so the reconcile is retried.
 func TestHTTPRouteReconcile_RetractErrorIsReturned(t *testing.T) {
-	r, prov := newHTTPRouteRetractFixture(t, gatewayv1.NamespacesFromSame)
+	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")
 }
+
+func acceptedConditionFromUpdater(t *testing.T, updater *recordingUpdater) *metav1.Condition {
+	t.Helper()
+	require.NotEmpty(t, updater.updates)
+	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)
+	return accepted
+}
+
+// A translation / Provider.Update failure must be visible on the route status
+// instead of Accepted=True plus a silent requeue.
+func TestHTTPRouteReconcile_UpdateErrorIsVisibleOnStatus(t *testing.T) {
+	r, prov, updater := newHTTPRouteRetractFixture(t, gatewayv1.NamespacesFromAll)
+	prov.updateErr = errors.New("translation failed")
+
+	_, err := reconcileRetractHTTPRoute(t, r)
+
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "translation failed")
+	assert.Equal(t, 1, prov.updated)
+	assert.Empty(t, prov.deleted)
+
+	accepted := acceptedConditionFromUpdater(t, updater)
+	assert.Equal(t, metav1.ConditionFalse, accepted.Status)
+	assert.Contains(t, accepted.Message, "translation failed")
+}

internal/controller/httproute_controller_test.go

@@ -113,18 +113,20 @@ func TestHTTPRouteReconcile_EmptyGateways(t *testing.T) {
 	}
 }
 
-// recordingProvider records the objects passed to Delete and can be told to fail.
+// recordingProvider records the objects passed to Update and Delete, and can be
+// told to fail an update or a delete.
 type recordingProvider struct {
 	updated   int
 	deleted   []k8stypes.NamespacedName
 	deleteErr error
+	updateErr error
 }
 
 func (p *recordingProvider) Register(string, *http.ServeMux) {}
 
 func (p *recordingProvider) Update(context.Context, *provider.TranslateContext, client.Object) error {
 	p.updated++
-	return nil
+	return p.updateErr
 }
 
 func (p *recordingProvider) Delete(_ context.Context, obj client.Object) error {

internal/controller/tcproute_controller.go

@@ -348,6 +348,16 @@ func (r *TCPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 	if r.supportsL4RoutePolicy {
 		ProcessL4RoutePolicy(r.Client, r.Log, tctx, tr.Namespace, tr.Name, KindTCPRoute)
 	}
+	// Translate and sync before writing Accepted=True. A translation or
+	// buildConfig failure from Provider.Update used to leave a green status
+	// and only requeue, matching neither Ingress nor ApisixGlobalRule.
+	var syncErr error
+	if isRouteAccepted(gateways) {
+		if syncErr = r.Provider.Update(ctx, tctx, tr); syncErr != nil {
+			acceptStatus.status = false
+			acceptStatus.msg = syncErr.Error()
+		}
+	}
 	tr.Status.Parents = make([]gatewayv1.RouteParentStatus, 0, len(gateways))
 	for _, gateway := range gateways {
 		parentStatus := gatewayv1.RouteParentStatus{}
@@ -377,11 +387,7 @@ func (r *TCPRouteReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
 	})
 	UpdateStatus(r.Updater, r.Log, tctx)
 	if isRouteAccepted(gateways) {
-		routeToUpdate := tr
-		if err := r.Provider.Update(ctx, tctx, routeToUpdate); err != nil {
-			return ctrl.Result{}, err
-		}
-		return ctrl.Result{}, nil
+		return ctrl.Result{}, syncErr
 	}
 
 	// The route still resolves to one of our Gateways but no parent accepts it any