From 23c4a58b4a73a2ef77bf64aa3c991397fa814024 Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Sun, 16 Feb 2025 11:21:57 +0900 Subject: [PATCH 1/2] Fix missing changes in PR#709 --- pkg/controllers/eventhandlers/gateway.go | 49 ++++++---------------- pkg/controllers/gateway_controller.go | 40 ++++-------------- pkg/controllers/route_controller.go | 41 ++---------------- pkg/k8s/utils.go | 31 ++++++++++++++ pkg/webhook/pod_mutator_test.go | 6 +-- pkg/webhook/pod_readiness_gate_injector.go | 46 +++++--------------- 6 files changed, 69 insertions(+), 144 deletions(-) diff --git a/pkg/controllers/eventhandlers/gateway.go b/pkg/controllers/eventhandlers/gateway.go index 9208c406..bb17ea8c 100644 --- a/pkg/controllers/eventhandlers/gateway.go +++ b/pkg/controllers/eventhandlers/gateway.go @@ -4,6 +4,7 @@ import ( "context" "time" + "github.com/aws/aws-application-networking-k8s/pkg/k8s" "github.com/aws/aws-application-networking-k8s/pkg/model/core" "github.com/aws/aws-application-networking-k8s/pkg/utils/gwlog" @@ -17,8 +18,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" gwv1 "sigs.k8s.io/gateway-api/apis/v1" - - "github.com/aws/aws-application-networking-k8s/pkg/config" ) type enqueueRequestsForGatewayEvent struct { @@ -74,41 +73,11 @@ func (h *enqueueRequestsForGatewayEvent) enqueueImpactedRoutes(ctx context.Conte } for _, route := range routes { - if len(route.Spec().ParentRefs()) <= 0 { - h.log.Debugf(ctx, "Ignoring Route with no parentRef %s-%s", route.Name(), route.Namespace()) - continue - } - - // find the parent gw object - var 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), - } - - gw := &gwv1.Gateway{} - if err := h.client.Get(ctx, gwName, gw); err != nil { - h.log.Debugf(ctx, "Ignoring Route with unknown parentRef %s-%s", route.Name(), route.Namespace()) - continue - } - - // find the parent gateway class name - gwClass := &gwv1.GatewayClass{} - gwClassName := types.NamespacedName{ - Namespace: "default", - Name: string(gw.Spec.GatewayClassName), - } - - if err := h.client.Get(ctx, gwClassName, gwClass); err != nil { - h.log.Debugf(ctx, "Ignoring Route with unknown Gateway %s-%s", route.Name(), route.Namespace()) - continue - } - - if gwClass.Spec.ControllerName == config.LatticeGatewayControllerName { + parents, err := k8s.FindControlledParents(ctx, h.client, route) + // If there is one or more parents, even if an error occurs, + // it is not an error related to the parent controlled by the Lattice Controller, so enqueue the route + if len(parents) > 0 { + // parents are controlled by lattice gateway controller, so enqueue the route h.log.Debugf(ctx, "Adding Route %s-%s to queue due to Gateway event", route.Name(), route.Namespace()) queue.Add(reconcile.Request{ NamespacedName: types.NamespacedName{ @@ -116,6 +85,12 @@ func (h *enqueueRequestsForGatewayEvent) enqueueImpactedRoutes(ctx context.Conte Name: route.Name(), }, }) + continue + } + if err != nil { + h.log.Debugf(ctx, "Ignoring Route with unknown parentRef %s-%s", route.Name(), route.Namespace()) + continue } + h.log.Debugf(ctx, "Ignoring Route %s-%s with no controlled parent", route.Name(), route.Namespace()) } } diff --git a/pkg/controllers/gateway_controller.go b/pkg/controllers/gateway_controller.go index 74ab4a76..ebcf9e8e 100644 --- a/pkg/controllers/gateway_controller.go +++ b/pkg/controllers/gateway_controller.go @@ -36,7 +36,6 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -146,19 +145,8 @@ func (r *gatewayReconciler) reconcile(ctx context.Context, req ctrl.Request) err return client.IgnoreNotFound(err) } - gwClass := &gwv1.GatewayClass{} - gwClassName := types.NamespacedName{ - Namespace: defaultNamespace, - Name: string(gw.Spec.GatewayClassName), - } - - if err := r.client.Get(ctx, gwClassName, gwClass); err != nil { - r.log.Infow(ctx, "GatewayClass is not found", "name", req.Name, "gwclass", gwClassName) - return client.IgnoreNotFound(err) - } - - if gwClass.Spec.ControllerName != config.LatticeGatewayControllerName { - r.log.Infow(ctx, "GatewayClass is not recognized", "name", req.Name, "gwClassControllerName", gwClass.Spec.ControllerName) + if !k8s.IsControlledByLatticeGatewayController(ctx, r.client, gw) { + r.log.Infow(ctx, "Gateway is not controlled by Lattice Gateway Controller", "name", req.Name) return nil } @@ -176,27 +164,15 @@ func (r *gatewayReconciler) reconcileDelete(ctx context.Context, gw *gwv1.Gatewa } for _, route := range routes { - if len(route.Spec().ParentRefs()) <= 0 { - continue - } - 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), - } - - httpGw := &gwv1.Gateway{} - if err := r.client.Get(ctx, gwName, httpGw); err != nil { - continue - } - - if httpGw.Name == gw.Name && httpGw.Namespace == gw.Namespace { + parents, err := k8s.FindControlledParents(ctx, r.client, route) + if len(parents) > 0 { + gw := parents[0] return fmt.Errorf("cannot delete gateway %s/%s - found referencing route %s/%s", gw.Namespace, gw.Name, route.Namespace(), route.Name()) } + if err != nil { + continue + } } err = r.finalizerManager.RemoveFinalizers(ctx, gw, gatewayFinalizer) diff --git a/pkg/controllers/route_controller.go b/pkg/controllers/route_controller.go index 199f48f0..82319760 100644 --- a/pkg/controllers/route_controller.go +++ b/pkg/controllers/route_controller.go @@ -216,7 +216,7 @@ func (r *routeReconciler) getRoute(ctx context.Context, req ctrl.Request) (core. } func updateRouteListenerStatus(ctx context.Context, k8sClient client.Client, route core.Route) error { - gws, err := findControlledParents(ctx, k8sClient, route) + gws, err := k8s.FindControlledParents(ctx, k8sClient, route) if len(gws) <= 0 { return fmt.Errorf("failed to get gateway for route %s: %w", route.Name(), err) } @@ -233,43 +233,10 @@ func (r *routeReconciler) isRouteRelevant(ctx context.Context, route core.Route) } // if route has gateway parentRef that is controlled by lattice gateway controller, // then it is relevant - gws, _ := findControlledParents(ctx, r.client, route) + gws, _ := k8s.FindControlledParents(ctx, r.client, route) return len(gws) > 0 } -// 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() - 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) - } - } - var err error - if len(misses) > 0 { - err = fmt.Errorf("failed to get gateway, name %s", misses) - } - return result, err -} - func (r *routeReconciler) buildAndDeployModel( ctx context.Context, route core.Route, @@ -308,7 +275,7 @@ func (r *routeReconciler) buildAndDeployModel( } func (r *routeReconciler) findControlledParentRef(ctx context.Context, route core.Route) (gwv1.ParentReference, error) { - gws, err := findControlledParents(ctx, r.client, route) + gws, err := k8s.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) } @@ -528,7 +495,7 @@ func (r *routeReconciler) validateRouteParentRefs(ctx context.Context, route cor } parentStatuses := []gwv1.RouteParentStatus{} - gws, err := findControlledParents(ctx, r.client, route) + gws, err := k8s.FindControlledParents(ctx, r.client, route) if len(gws) <= 0 { return nil, fmt.Errorf("failed to get gateway for route %s: %w", route.Name(), err) } diff --git a/pkg/k8s/utils.go b/pkg/k8s/utils.go index 7df4ec0f..42f1dfff 100644 --- a/pkg/k8s/utils.go +++ b/pkg/k8s/utils.go @@ -2,8 +2,10 @@ package k8s import ( "context" + "fmt" "github.com/aws/aws-application-networking-k8s/pkg/config" + "github.com/aws/aws-application-networking-k8s/pkg/model/core" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" @@ -67,6 +69,35 @@ func IsControlledByLatticeGatewayController(ctx context.Context, c client.Client return gwClass.Spec.ControllerName == config.LatticeGatewayControllerName } +// 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() + 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 IsControlledByLatticeGatewayController(ctx, client, gw) { + result = append(result, gw) + } + } + var err error + if len(misses) > 0 { + err = fmt.Errorf("failed to get gateways, %s", misses) + } + return result, err +} + func ObjExists(ctx context.Context, c client.Client, key types.NamespacedName, obj client.Object) (bool, error) { err := c.Get(ctx, key, obj) if err != nil { diff --git a/pkg/webhook/pod_mutator_test.go b/pkg/webhook/pod_mutator_test.go index 1590ed7b..c384353c 100644 --- a/pkg/webhook/pod_mutator_test.go +++ b/pkg/webhook/pod_mutator_test.go @@ -2,6 +2,8 @@ package webhook import ( "context" + "testing" + anv1alpha1 "github.com/aws/aws-application-networking-k8s/pkg/apis/applicationnetworking/v1alpha1" "github.com/aws/aws-application-networking-k8s/pkg/utils/gwlog" "github.com/stretchr/testify/assert" @@ -11,7 +13,6 @@ import ( clientgoscheme "k8s.io/client-go/kubernetes/scheme" testclient "sigs.k8s.io/controller-runtime/pkg/client/fake" gwv1 "sigs.k8s.io/gateway-api/apis/v1" - "testing" ) func Test_ReadinessGateInjection(t *testing.T) { @@ -1079,8 +1080,7 @@ func Test_ReadinessGateInjection(t *testing.T) { gwClass := &gwv1.GatewayClass{ ObjectMeta: metav1.ObjectMeta{ - Name: "amazon-vpc-lattice", - Namespace: "default", + Name: "amazon-vpc-lattice", }, Spec: gwv1.GatewayClassSpec{ ControllerName: "application-networking.k8s.aws/gateway-api-controller", diff --git a/pkg/webhook/pod_readiness_gate_injector.go b/pkg/webhook/pod_readiness_gate_injector.go index 39556885..6a0b5f7d 100644 --- a/pkg/webhook/pod_readiness_gate_injector.go +++ b/pkg/webhook/pod_readiness_gate_injector.go @@ -2,15 +2,14 @@ package webhook import ( "context" + anv1alpha1 "github.com/aws/aws-application-networking-k8s/pkg/apis/applicationnetworking/v1alpha1" - "github.com/aws/aws-application-networking-k8s/pkg/config" k8sutils "github.com/aws/aws-application-networking-k8s/pkg/k8s" "github.com/aws/aws-application-networking-k8s/pkg/model/core" "github.com/aws/aws-application-networking-k8s/pkg/utils/gwlog" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/labels" - "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" gwv1 "sigs.k8s.io/gateway-api/apis/v1" ) @@ -178,41 +177,18 @@ func (m *PodReadinessGateInjector) routeHasLatticeGateway(ctx context.Context, r m.log.Debugf(ctx, "Route %s/%s has no parentRefs", route.Namespace(), route.Name()) return false } - - 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, - Name: string(route.Spec().ParentRefs()[0].Name), - } - - if err := m.k8sClient.Get(ctx, gwName, gw); err != nil { - m.log.Debugf(ctx, "Unable to retrieve gateway %s/%s for route %s/%s, %s", - gwName.Namespace, gwName.Name, route.Namespace(), route.Name(), err) - return false - } - - // make sure gateway is an aws-vpc-lattice - gwClass := &gwv1.GatewayClass{} - gwClassName := types.NamespacedName{ - Namespace: "default", - Name: string(gw.Spec.GatewayClassName), + parents, err := k8sutils.FindControlledParents(ctx, m.k8sClient, route) + // If there is at least one parent element and an error exists, + // it is not an error related to the parent controlled by the Lattice Controller, so return true + if len(parents) > 0 { + gw := parents[0] + m.log.Debugf(ctx, "Gateway %s/%s is a lattice gateway", gw.Namespace, gw.Name) + return true } - - if err := m.k8sClient.Get(ctx, gwClassName, gwClass); err != nil { - m.log.Debugf(ctx, "Unable to retrieve gateway class %s/%s for gateway %s/%s, %s", - gwClassName.Namespace, gwClass.Name, gwName.Namespace, gwName.Name, err) + if err != nil { + m.log.Debugf(ctx, "Unable to retrieve controlled parents for route %s/%s, %s", route.Namespace(), route.Name(), err) return false } - - if gwClass.Spec.ControllerName == config.LatticeGatewayControllerName { - m.log.Debugf(ctx, "Gateway %s/%s is a lattice gateway", gwName.Namespace, gwName.Name) - return true - } - - m.log.Debugf(ctx, "Gateway %s/%s is not a lattice gateway", gwName.Namespace, gwName.Name) + m.log.Debugf(ctx, "Route %s/%s has no controlled lattice gateway", route.Namespace(), route.Name()) return false } From 6900ddb74d7f2551f8bdf5688dedc980e1fe18f9 Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Wed, 19 Feb 2025 19:38:46 +0900 Subject: [PATCH 2/2] Fix controller name --- pkg/controllers/gateway_controller.go | 2 +- pkg/webhook/pod_readiness_gate_injector.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/controllers/gateway_controller.go b/pkg/controllers/gateway_controller.go index ebcf9e8e..ae81c4f9 100644 --- a/pkg/controllers/gateway_controller.go +++ b/pkg/controllers/gateway_controller.go @@ -146,7 +146,7 @@ func (r *gatewayReconciler) reconcile(ctx context.Context, req ctrl.Request) err } if !k8s.IsControlledByLatticeGatewayController(ctx, r.client, gw) { - r.log.Infow(ctx, "Gateway is not controlled by Lattice Gateway Controller", "name", req.Name) + r.log.Infow(ctx, "Gateway is not controlled by AWS Gateway API Controller", "name", req.Name) return nil } diff --git a/pkg/webhook/pod_readiness_gate_injector.go b/pkg/webhook/pod_readiness_gate_injector.go index 6a0b5f7d..2caec5c0 100644 --- a/pkg/webhook/pod_readiness_gate_injector.go +++ b/pkg/webhook/pod_readiness_gate_injector.go @@ -179,16 +179,16 @@ func (m *PodReadinessGateInjector) routeHasLatticeGateway(ctx context.Context, r } parents, err := k8sutils.FindControlledParents(ctx, m.k8sClient, route) // If there is at least one parent element and an error exists, - // it is not an error related to the parent controlled by the Lattice Controller, so return true + // it is not an error related to the parent controlled by the AWS Gateway API Controller, so return true if len(parents) > 0 { gw := parents[0] - m.log.Debugf(ctx, "Gateway %s/%s is a lattice gateway", gw.Namespace, gw.Name) + m.log.Debugf(ctx, "Gateway %s/%s is a AWS Gateway API Controller", gw.Namespace, gw.Name) return true } if err != nil { m.log.Debugf(ctx, "Unable to retrieve controlled parents for route %s/%s, %s", route.Namespace(), route.Name(), err) return false } - m.log.Debugf(ctx, "Route %s/%s has no controlled lattice gateway", route.Namespace(), route.Name()) + m.log.Debugf(ctx, "Route %s/%s has no controlled AWS Gateway API Controller", route.Namespace(), route.Name()) return false }