Fix issue #661: Ignore Gateways not managed by Lattice Controller (#709)

* Update model_build_lattice_service_test for resolve issue

* Added validation to check if parentRef is managed by vpcLatticeController.

* Fix route_controller_test

* Add validate gateway logic

* Update validate gateway logic and tests

* Update route_controller

* Refactor

* Fix FindTargetGroup for e2e test

* Update XXXRoute update logic

---------

Co-authored-by: Ryan Lymburner <137918933+rlymbur@users.noreply.github.com>

Author: u-kai

pkg/controllers/route_controller.go

@@ -19,6 +19,7 @@ package controllers
 import (
 	"context"
 	"fmt"
+
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 
 	"github.com/pkg/errors"
@@ -50,7 +51,6 @@ import (
 	"github.com/aws/aws-application-networking-k8s/pkg/k8s"
 	"github.com/aws/aws-application-networking-k8s/pkg/model/core"
 	lattice_runtime "github.com/aws/aws-application-networking-k8s/pkg/runtime"
-	"github.com/aws/aws-application-networking-k8s/pkg/utils"
 	k8sutils "github.com/aws/aws-application-networking-k8s/pkg/utils"
 	"github.com/aws/aws-application-networking-k8s/pkg/utils/gwlog"
 )
@@ -216,67 +216,58 @@ func (r *routeReconciler) getRoute(ctx context.Context, req ctrl.Request) (core.
 }
 
 func updateRouteListenerStatus(ctx context.Context, k8sClient client.Client, route core.Route) error {
-	gw := &gwv1.Gateway{}
-
-	gwNamespace := route.Namespace()
-	if route.Spec().ParentRefs()[0].Namespace != nil {
-		gwNamespace = string(*route.Spec().ParentRefs()[0].Namespace)
-	}
-	gwName := types.NamespacedName{
-		Namespace: gwNamespace,
-		// TODO assume one parent for now and point to service network
-		Name: string(route.Spec().ParentRefs()[0].Name),
+	gws, err := findControlledParents(ctx, k8sClient, route)
+	if len(gws) <= 0 {
+		return fmt.Errorf("failed to get gateway for route %s: %w", route.Name(), err)
 	}
-
-	if err := k8sClient.Get(ctx, gwName, gw); err != nil {
-		return fmt.Errorf("update route listener: gw not found, gw: %s, err: %w", gwName, err)
-	}
-
+	// TODO assume one parent for now and point to service network
+	gw := gws[0]
 	return UpdateGWListenerStatus(ctx, k8sClient, gw)
+
 }
 
 func (r *routeReconciler) isRouteRelevant(ctx context.Context, route core.Route) bool {
 	if len(route.Spec().ParentRefs()) == 0 {
 		r.log.Infof(ctx, "Ignore Route which has no ParentRefs gateway %s ", route.Name())
 		return false
 	}
+	// if route has gateway parentRef that is controlled by lattice gateway controller,
+	// then it is relevant
+	gws, _ := findControlledParents(ctx, r.client, route)
+	return len(gws) > 0
+}
 
-	gw := &gwv1.Gateway{}
-
+// findControlledParents returns parent gateways that are controlled by lattice gateway controller
+func findControlledParents(
+	ctx context.Context,
+	client client.Client,
+	route core.Route,
+) ([]*gwv1.Gateway, error) {
+	var result []*gwv1.Gateway
 	gwNamespace := route.Namespace()
-	if route.Spec().ParentRefs()[0].Namespace != nil {
-		gwNamespace = string(*route.Spec().ParentRefs()[0].Namespace)
-	}
-	gwName := types.NamespacedName{
-		Namespace: gwNamespace,
-		Name:      string(route.Spec().ParentRefs()[0].Name),
-	}
-
-	if err := r.client.Get(ctx, gwName, gw); err != nil {
-		r.log.Infof(ctx, "Could not find gateway %s with err %s. Ignoring route %+v whose ParentRef gateway object"+
-			" is not defined.", gwName.String(), err, route.Spec())
-		return false
-	}
-
-	// make sure gateway is an aws-vpc-lattice
-	gwClass := &gwv1.GatewayClass{}
-	gwClassName := types.NamespacedName{
-		Namespace: defaultNamespace,
-		Name:      string(gw.Spec.GatewayClassName),
+	misses := []string{}
+	for _, parentRef := range route.Spec().ParentRefs() {
+		gw := &gwv1.Gateway{}
+		if parentRef.Namespace != nil {
+			gwNamespace = string(*parentRef.Namespace)
+		}
+		gwName := types.NamespacedName{
+			Namespace: gwNamespace,
+			Name:      string(parentRef.Name),
+		}
+		if err := client.Get(ctx, gwName, gw); err != nil {
+			misses = append(misses, gwName.String())
+			continue
+		}
+		if k8s.IsControlledByLatticeGatewayController(ctx, client, gw) {
+			result = append(result, gw)
+		}
 	}
-
-	if err := r.client.Get(ctx, gwClassName, gwClass); err != nil {
-		r.log.Infof(ctx, "Ignore Route not controlled by any GatewayClass %s, %s", route.Name(), route.Namespace())
-		return false
+	var err error
+	if len(misses) > 0 {
+		err = fmt.Errorf("failed to get gateway, name %s", misses)
 	}
-
-	if gwClass.Spec.ControllerName == config.LatticeGatewayControllerName {
-		r.log.Infof(ctx, "Found aws-vpc-lattice for Route for %s, %s", route.Name(), route.Namespace())
-		return true
-	}
-
-	r.log.Infof(ctx, "Ignore non aws-vpc-lattice Route %s, %s", route.Name(), route.Namespace())
-	return false
+	return result, err
 }
 
 func (r *routeReconciler) buildAndDeployModel(
@@ -316,6 +307,21 @@ func (r *routeReconciler) buildAndDeployModel(
 	return stack, err
 }
 
+func (r *routeReconciler) findControlledParentRef(ctx context.Context, route core.Route) (gwv1.ParentReference, error) {
+	gws, err := findControlledParents(ctx, r.client, route)
+	if len(gws) <= 0 {
+		return gwv1.ParentReference{}, fmt.Errorf("failed to get gateway for route %s: %w", route.Name(), err)
+	}
+	// TODO assume one parent for now and point to service network
+	gw := gws[0]
+	for _, parentRef := range route.Spec().ParentRefs() {
+		if string(parentRef.Name) == gw.Name {
+			return parentRef, nil
+		}
+	}
+	return gwv1.ParentReference{}, fmt.Errorf("parentRef not found for route %s", route.Name())
+}
+
 func (r *routeReconciler) reconcileUpsert(ctx context.Context, req ctrl.Request, route core.Route) error {
 	r.log.Infow(ctx, "reconcile, adding or updating", "name", req.Name)
 	r.eventRecorder.Event(route.K8sObject(), corev1.EventTypeNormal,
@@ -337,10 +343,13 @@ func (r *routeReconciler) reconcileUpsert(ctx context.Context, req ctrl.Request,
 
 	if backendRefIPFamiliesErr != nil {
 		httpRouteOld := route.DeepCopy()
+		parentRef, err := r.findControlledParentRef(ctx, route)
+		if err != nil {
+			return err
+		}
 
-		route.Status().UpdateParentRefs(route.Spec().ParentRefs()[0], config.LatticeGatewayControllerName)
-
-		route.Status().UpdateRouteCondition(metav1.Condition{
+		route.Status().UpdateParentRefs(parentRef, config.LatticeGatewayControllerName)
+		route.Status().UpdateRouteCondition(parentRef, metav1.Condition{
 			Type:               string(gwv1.RouteConditionAccepted),
 			Status:             metav1.ConditionFalse,
 			ObservedGeneration: route.K8sObject().GetGeneration(),
@@ -358,8 +367,13 @@ func (r *routeReconciler) reconcileUpsert(ctx context.Context, req ctrl.Request,
 	if _, err := r.buildAndDeployModel(ctx, route); err != nil {
 		if services.IsConflictError(err) {
 			// Stop reconciliation of this route if the route cannot be owned / has conflict
-			route.Status().UpdateParentRefs(route.Spec().ParentRefs()[0], config.LatticeGatewayControllerName)
-			route.Status().UpdateRouteCondition(metav1.Condition{
+			parentRef, parentRefErr := r.findControlledParentRef(ctx, route)
+			if parentRefErr != nil {
+				// if parentRef not found, we cannot update route status
+				return parentRefErr
+			}
+			route.Status().UpdateParentRefs(parentRef, config.LatticeGatewayControllerName)
+			route.Status().UpdateRouteCondition(parentRef, metav1.Condition{
 				Type:               string(gwv1.RouteConditionAccepted),
 				Status:             metav1.ConditionFalse,
 				ObservedGeneration: route.K8sObject().GetGeneration(),
@@ -499,24 +513,6 @@ func (r *routeReconciler) hasNotAcceptedCondition(route core.Route) bool {
 	return false
 }
 
-// find Gateway by Route and parentRef, returns nil if not found
-func (r *routeReconciler) findRouteParentGw(ctx context.Context, route core.Route, parentRef gwv1.ParentReference) (*gwv1.Gateway, error) {
-	ns := route.Namespace()
-	if parentRef.Namespace != nil && *parentRef.Namespace != "" {
-		ns = string(*parentRef.Namespace)
-	}
-	gwName := types.NamespacedName{
-		Namespace: ns,
-		Name:      string(parentRef.Name),
-	}
-	gw := &gwv1.Gateway{}
-	err := r.client.Get(ctx, gwName, gw)
-	if err != nil {
-		return nil, client.IgnoreNotFound(err)
-	}
-	return gw, nil
-}
-
 // Validation rules for route parentRefs
 //
 // Will ignore status update when:
@@ -532,15 +528,13 @@ func (r *routeReconciler) validateRouteParentRefs(ctx context.Context, route cor
 	}
 
 	parentStatuses := []gwv1.RouteParentStatus{}
+	gws, err := findControlledParents(ctx, r.client, route)
+	if len(gws) <= 0 {
+		return nil, fmt.Errorf("failed to get gateway for route %s: %w", route.Name(), err)
+	}
+	// TODO assume one parent for now and point to service network
+	gw := gws[0]
 	for _, parentRef := range route.Spec().ParentRefs() {
-		gw, err := r.findRouteParentGw(ctx, route, parentRef)
-		if err != nil {
-			return nil, err
-		}
-		if gw == nil {
-			continue // ignore status update if gw not found
-		}
-
 		noMatchingParent := true
 		for _, listener := range gw.Spec.Listeners {
 			if parentRef.Port != nil && *parentRef.Port != listener.Port {
@@ -554,7 +548,7 @@ func (r *routeReconciler) validateRouteParentRefs(ctx context.Context, route cor
 
 		parentStatus := gwv1.RouteParentStatus{
 			ParentRef:      parentRef,
-			ControllerName: "application-networking.k8s.aws/gateway-api-controller",
+			ControllerName: config.LatticeGatewayControllerName,
 			Conditions:     []metav1.Condition{},
 		}
 
@@ -573,7 +567,7 @@ func (r *routeReconciler) validateRouteParentRefs(ctx context.Context, route cor
 }
 
 // set of valid Kinds for Route Backend References
-var validBackendKinds = utils.NewSet("Service", "ServiceImport")
+var validBackendKinds = k8sutils.NewSet("Service", "ServiceImport")
 
 // validate route's backed references, will return non-accepted
 // condition if at least one backendRef not in a valid state

pkg/controllers/route_controller_test.go

@@ -2,6 +2,8 @@ package controllers
 
 import (
 	"context"
+	"testing"
+
 	mock_client "github.com/aws/aws-application-networking-k8s/mocks/controller-runtime/client"
 	anv1alpha1 "github.com/aws/aws-application-networking-k8s/pkg/apis/applicationnetworking/v1alpha1"
 	aws2 "github.com/aws/aws-application-networking-k8s/pkg/aws"
@@ -27,7 +29,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"sigs.k8s.io/external-dns/endpoint"
 	gwv1 "sigs.k8s.io/gateway-api/apis/v1"
-	"testing"
 )
 
 func TestRouteReconciler_ReconcileCreates(t *testing.T) {
@@ -52,8 +53,7 @@ func TestRouteReconciler_ReconcileCreates(t *testing.T) {
 
 	gwClass := &gwv1.GatewayClass{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      "amazon-vpc-lattice",
-			Namespace: defaultNamespace,
+			Name: "amazon-vpc-lattice",
 		},
 		Spec: gwv1.GatewayClassSpec{
 			ControllerName: config.LatticeGatewayControllerName,
@@ -79,6 +79,25 @@ func TestRouteReconciler_ReconcileCreates(t *testing.T) {
 			},
 		},
 	}
+
+	notVpcLattice := &gwv1.Gateway{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "not-vpc-lattice",
+			Namespace: "ns1",
+		},
+		Spec: gwv1.GatewaySpec{
+			GatewayClassName: "not-amazon-vpc-lattice",
+			Listeners: []gwv1.Listener{
+				{
+					Name:     "http",
+					Protocol: "HTTP",
+					Port:     80,
+				},
+			},
+		},
+	}
+
+	k8sClient.Create(ctx, notVpcLattice.DeepCopy())
 	k8sClient.Create(ctx, gw.DeepCopy())
 
 	svc := &corev1.Service{
@@ -131,6 +150,10 @@ func TestRouteReconciler_ReconcileCreates(t *testing.T) {
 		Spec: gwv1.HTTPRouteSpec{
 			CommonRouteSpec: gwv1.CommonRouteSpec{
 				ParentRefs: []gwv1.ParentReference{
+					// if route has multiple parents, we'll only use the managed vpc lattice gateway
+					{
+						Name: "not-vpc-lattice",
+					},
 					{
 						Name: "my-gateway",
 					},

pkg/gateway/model_build_lattice_service.go

@@ -124,7 +124,21 @@ func (t *latticeServiceModelBuildTask) buildLatticeService(ctx context.Context)
 	}
 
 	for _, parentRef := range t.route.Spec().ParentRefs() {
-		spec.ServiceNetworkNames = append(spec.ServiceNetworkNames, string(parentRef.Name))
+		gw := &gwv1.Gateway{}
+		parentNamespace := t.route.Namespace()
+		if parentRef.Namespace != nil {
+			parentNamespace = string(*parentRef.Namespace)
+		}
+		err := t.client.Get(ctx, client.ObjectKey{Name: string(parentRef.Name), Namespace: parentNamespace}, gw)
+		if err != nil {
+			t.log.Infof(ctx, "Ignoring route %s because failed to get gateway %s: %v", t.route.Name(), gw.Spec.GatewayClassName, err)
+			continue
+		}
+		if k8s.IsControlledByLatticeGatewayController(ctx, t.client, gw) {
+			spec.ServiceNetworkNames = append(spec.ServiceNetworkNames, string(parentRef.Name))
+		} else {
+			t.log.Infof(ctx, "Ignoring route %s because gateway %s is not managed by lattice gateway controller", t.route.Name(), gw.Name)
+		}
 	}
 	if config.ServiceNetworkOverrideMode {
 		spec.ServiceNetworkNames = []string{config.DefaultServiceNetwork}
@@ -160,7 +174,9 @@ func (t *latticeServiceModelBuildTask) buildLatticeService(ctx context.Context)
 
 // returns empty string if not found
 func (t *latticeServiceModelBuildTask) getACMCertArn(ctx context.Context) (string, error) {
-	gw, err := t.getGateway(ctx)
+	// when a service is associate to multiple service network(s), all listener config MUST be same
+	// so here we are only using the 1st gateway
+	gw, err := t.findGateway(ctx)
 	if err != nil {
 		if apierrors.IsNotFound(err) && !t.route.DeletionTimestamp().IsZero() {
 			return "", nil // ok if we're deleting the route
@@ -169,10 +185,8 @@ func (t *latticeServiceModelBuildTask) getACMCertArn(ctx context.Context) (strin
 	}
 
 	for _, parentRef := range t.route.Spec().ParentRefs() {
-		if parentRef.Name != t.route.Spec().ParentRefs()[0].Name {
-			// when a service is associate to multiple service network(s), all listener config MUST be same
-			// so here we are only using the 1st gateway
-			t.log.Debugf(ctx, "Ignore ParentRef of different gateway %s-%s", parentRef.Name, *parentRef.Namespace)
+		if string(parentRef.Name) != gw.Name {
+			t.log.Debugf(ctx, "Ignore ParentRef of different gateway %s", parentRef.Name)
 			continue
 		}