Skip to content

Commit 0da7ab8

Browse files
rlymburrlymbur
authored
Merge branch 'main' into correct-go-toolchain-version
2 parents cbb6a63 + 00115ee commit 0da7ab8

14 files changed

+550
-339
lines changed

‎pkg/controllers/route_controller.go

+76-82
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package controllers
1919
import (
2020
"context"
2121
"fmt"
22+
2223
"sigs.k8s.io/controller-runtime/pkg/controller"
2324

2425
"github.com/pkg/errors"
@@ -50,7 +51,6 @@ import (
5051
"github.com/aws/aws-application-networking-k8s/pkg/k8s"
5152
"github.com/aws/aws-application-networking-k8s/pkg/model/core"
5253
lattice_runtime "github.com/aws/aws-application-networking-k8s/pkg/runtime"
53-
"github.com/aws/aws-application-networking-k8s/pkg/utils"
5454
k8sutils "github.com/aws/aws-application-networking-k8s/pkg/utils"
5555
"github.com/aws/aws-application-networking-k8s/pkg/utils/gwlog"
5656
)
@@ -216,67 +216,58 @@ func (r *routeReconciler) getRoute(ctx context.Context, req ctrl.Request) (core.
216216
}
217217

218218
func updateRouteListenerStatus(ctx context.Context, k8sClient client.Client, route core.Route) error {
219-
gw := &gwv1.Gateway{}
220-
221-
gwNamespace := route.Namespace()
222-
if route.Spec().ParentRefs()[0].Namespace != nil {
223-
gwNamespace = string(*route.Spec().ParentRefs()[0].Namespace)
224-
}
225-
gwName := types.NamespacedName{
226-
Namespace: gwNamespace,
227-
// TODO assume one parent for now and point to service network
228-
Name: string(route.Spec().ParentRefs()[0].Name),
219+
gws, err := findControlledParents(ctx, k8sClient, route)
220+
if len(gws) <= 0 {
221+
return fmt.Errorf("failed to get gateway for route %s: %w", route.Name(), err)
229222
}
230-
231-
if err := k8sClient.Get(ctx, gwName, gw); err != nil {
232-
return fmt.Errorf("update route listener: gw not found, gw: %s, err: %w", gwName, err)
233-
}
234-
223+
// TODO assume one parent for now and point to service network
224+
gw := gws[0]
235225
return UpdateGWListenerStatus(ctx, k8sClient, gw)
226+
236227
}
237228

238229
func (r *routeReconciler) isRouteRelevant(ctx context.Context, route core.Route) bool {
239230
if len(route.Spec().ParentRefs()) == 0 {
240231
r.log.Infof(ctx, "Ignore Route which has no ParentRefs gateway %s ", route.Name())
241232
return false
242233
}
234+
// if route has gateway parentRef that is controlled by lattice gateway controller,
235+
// then it is relevant
236+
gws, _ := findControlledParents(ctx, r.client, route)
237+
return len(gws) > 0
238+
}
243239

244-
gw := &gwv1.Gateway{}
245-
240+
// findControlledParents returns parent gateways that are controlled by lattice gateway controller
241+
func findControlledParents(
242+
ctx context.Context,
243+
client client.Client,
244+
route core.Route,
245+
) ([]*gwv1.Gateway, error) {
246+
var result []*gwv1.Gateway
246247
gwNamespace := route.Namespace()
247-
if route.Spec().ParentRefs()[0].Namespace != nil {
248-
gwNamespace = string(*route.Spec().ParentRefs()[0].Namespace)
249-
}
250-
gwName := types.NamespacedName{
251-
Namespace: gwNamespace,
252-
Name: string(route.Spec().ParentRefs()[0].Name),
253-
}
254-
255-
if err := r.client.Get(ctx, gwName, gw); err != nil {
256-
r.log.Infof(ctx, "Could not find gateway %s with err %s. Ignoring route %+v whose ParentRef gateway object"+
257-
" is not defined.", gwName.String(), err, route.Spec())
258-
return false
259-
}
260-
261-
// make sure gateway is an aws-vpc-lattice
262-
gwClass := &gwv1.GatewayClass{}
263-
gwClassName := types.NamespacedName{
264-
Namespace: defaultNamespace,
265-
Name: string(gw.Spec.GatewayClassName),
248+
misses := []string{}
249+
for _, parentRef := range route.Spec().ParentRefs() {
250+
gw := &gwv1.Gateway{}
251+
if parentRef.Namespace != nil {
252+
gwNamespace = string(*parentRef.Namespace)
253+
}
254+
gwName := types.NamespacedName{
255+
Namespace: gwNamespace,
256+
Name: string(parentRef.Name),
257+
}
258+
if err := client.Get(ctx, gwName, gw); err != nil {
259+
misses = append(misses, gwName.String())
260+
continue
261+
}
262+
if k8s.IsControlledByLatticeGatewayController(ctx, client, gw) {
263+
result = append(result, gw)
264+
}
266265
}
267-
268-
if err := r.client.Get(ctx, gwClassName, gwClass); err != nil {
269-
r.log.Infof(ctx, "Ignore Route not controlled by any GatewayClass %s, %s", route.Name(), route.Namespace())
270-
return false
266+
var err error
267+
if len(misses) > 0 {
268+
err = fmt.Errorf("failed to get gateway, name %s", misses)
271269
}
272-
273-
if gwClass.Spec.ControllerName == config.LatticeGatewayControllerName {
274-
r.log.Infof(ctx, "Found aws-vpc-lattice for Route for %s, %s", route.Name(), route.Namespace())
275-
return true
276-
}
277-
278-
r.log.Infof(ctx, "Ignore non aws-vpc-lattice Route %s, %s", route.Name(), route.Namespace())
279-
return false
270+
return result, err
280271
}
281272

282273
func (r *routeReconciler) buildAndDeployModel(
@@ -316,6 +307,21 @@ func (r *routeReconciler) buildAndDeployModel(
316307
return stack, err
317308
}
318309

310+
func (r *routeReconciler) findControlledParentRef(ctx context.Context, route core.Route) (gwv1.ParentReference, error) {
311+
gws, err := findControlledParents(ctx, r.client, route)
312+
if len(gws) <= 0 {
313+
return gwv1.ParentReference{}, fmt.Errorf("failed to get gateway for route %s: %w", route.Name(), err)
314+
}
315+
// TODO assume one parent for now and point to service network
316+
gw := gws[0]
317+
for _, parentRef := range route.Spec().ParentRefs() {
318+
if string(parentRef.Name) == gw.Name {
319+
return parentRef, nil
320+
}
321+
}
322+
return gwv1.ParentReference{}, fmt.Errorf("parentRef not found for route %s", route.Name())
323+
}
324+
319325
func (r *routeReconciler) reconcileUpsert(ctx context.Context, req ctrl.Request, route core.Route) error {
320326
r.log.Infow(ctx, "reconcile, adding or updating", "name", req.Name)
321327
r.eventRecorder.Event(route.K8sObject(), corev1.EventTypeNormal,
@@ -337,10 +343,13 @@ func (r *routeReconciler) reconcileUpsert(ctx context.Context, req ctrl.Request,
337343

338344
if backendRefIPFamiliesErr != nil {
339345
httpRouteOld := route.DeepCopy()
346+
parentRef, err := r.findControlledParentRef(ctx, route)
347+
if err != nil {
348+
return err
349+
}
340350

341-
route.Status().UpdateParentRefs(route.Spec().ParentRefs()[0], config.LatticeGatewayControllerName)
342-
343-
route.Status().UpdateRouteCondition(metav1.Condition{
351+
route.Status().UpdateParentRefs(parentRef, config.LatticeGatewayControllerName)
352+
route.Status().UpdateRouteCondition(parentRef, metav1.Condition{
344353
Type: string(gwv1.RouteConditionAccepted),
345354
Status: metav1.ConditionFalse,
346355
ObservedGeneration: route.K8sObject().GetGeneration(),
@@ -358,8 +367,13 @@ func (r *routeReconciler) reconcileUpsert(ctx context.Context, req ctrl.Request,
358367
if _, err := r.buildAndDeployModel(ctx, route); err != nil {
359368
if services.IsConflictError(err) {
360369
// Stop reconciliation of this route if the route cannot be owned / has conflict
361-
route.Status().UpdateParentRefs(route.Spec().ParentRefs()[0], config.LatticeGatewayControllerName)
362-
route.Status().UpdateRouteCondition(metav1.Condition{
370+
parentRef, parentRefErr := r.findControlledParentRef(ctx, route)
371+
if parentRefErr != nil {
372+
// if parentRef not found, we cannot update route status
373+
return parentRefErr
374+
}
375+
route.Status().UpdateParentRefs(parentRef, config.LatticeGatewayControllerName)
376+
route.Status().UpdateRouteCondition(parentRef, metav1.Condition{
363377
Type: string(gwv1.RouteConditionAccepted),
364378
Status: metav1.ConditionFalse,
365379
ObservedGeneration: route.K8sObject().GetGeneration(),
@@ -499,24 +513,6 @@ func (r *routeReconciler) hasNotAcceptedCondition(route core.Route) bool {
499513
return false
500514
}
501515

502-
// find Gateway by Route and parentRef, returns nil if not found
503-
func (r *routeReconciler) findRouteParentGw(ctx context.Context, route core.Route, parentRef gwv1.ParentReference) (*gwv1.Gateway, error) {
504-
ns := route.Namespace()
505-
if parentRef.Namespace != nil && *parentRef.Namespace != "" {
506-
ns = string(*parentRef.Namespace)
507-
}
508-
gwName := types.NamespacedName{
509-
Namespace: ns,
510-
Name: string(parentRef.Name),
511-
}
512-
gw := &gwv1.Gateway{}
513-
err := r.client.Get(ctx, gwName, gw)
514-
if err != nil {
515-
return nil, client.IgnoreNotFound(err)
516-
}
517-
return gw, nil
518-
}
519-
520516
// Validation rules for route parentRefs
521517
//
522518
// Will ignore status update when:
@@ -532,15 +528,13 @@ func (r *routeReconciler) validateRouteParentRefs(ctx context.Context, route cor
532528
}
533529

534530
parentStatuses := []gwv1.RouteParentStatus{}
531+
gws, err := findControlledParents(ctx, r.client, route)
532+
if len(gws) <= 0 {
533+
return nil, fmt.Errorf("failed to get gateway for route %s: %w", route.Name(), err)
534+
}
535+
// TODO assume one parent for now and point to service network
536+
gw := gws[0]
535537
for _, parentRef := range route.Spec().ParentRefs() {
536-
gw, err := r.findRouteParentGw(ctx, route, parentRef)
537-
if err != nil {
538-
return nil, err
539-
}
540-
if gw == nil {
541-
continue // ignore status update if gw not found
542-
}
543-
544538
noMatchingParent := true
545539
for _, listener := range gw.Spec.Listeners {
546540
if parentRef.Port != nil && *parentRef.Port != listener.Port {
@@ -554,7 +548,7 @@ func (r *routeReconciler) validateRouteParentRefs(ctx context.Context, route cor
554548

555549
parentStatus := gwv1.RouteParentStatus{
556550
ParentRef: parentRef,
557-
ControllerName: "application-networking.k8s.aws/gateway-api-controller",
551+
ControllerName: config.LatticeGatewayControllerName,
558552
Conditions: []metav1.Condition{},
559553
}
560554

@@ -573,7 +567,7 @@ func (r *routeReconciler) validateRouteParentRefs(ctx context.Context, route cor
573567
}
574568

575569
// set of valid Kinds for Route Backend References
576-
var validBackendKinds = utils.NewSet("Service", "ServiceImport")
570+
var validBackendKinds = k8sutils.NewSet("Service", "ServiceImport")
577571

578572
// validate route's backed references, will return non-accepted
579573
// condition if at least one backendRef not in a valid state

‎pkg/controllers/route_controller_test.go

+26-3
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package controllers
22

33
import (
44
"context"
5+
"testing"
6+
57
mock_client "github.com/aws/aws-application-networking-k8s/mocks/controller-runtime/client"
68
anv1alpha1 "github.com/aws/aws-application-networking-k8s/pkg/apis/applicationnetworking/v1alpha1"
79
aws2 "github.com/aws/aws-application-networking-k8s/pkg/aws"
@@ -27,7 +29,6 @@ import (
2729
"sigs.k8s.io/controller-runtime/pkg/reconcile"
2830
"sigs.k8s.io/external-dns/endpoint"
2931
gwv1 "sigs.k8s.io/gateway-api/apis/v1"
30-
"testing"
3132
)
3233

3334
func TestRouteReconciler_ReconcileCreates(t *testing.T) {
@@ -52,8 +53,7 @@ func TestRouteReconciler_ReconcileCreates(t *testing.T) {
5253

5354
gwClass := &gwv1.GatewayClass{
5455
ObjectMeta: metav1.ObjectMeta{
55-
Name: "amazon-vpc-lattice",
56-
Namespace: defaultNamespace,
56+
Name: "amazon-vpc-lattice",
5757
},
5858
Spec: gwv1.GatewayClassSpec{
5959
ControllerName: config.LatticeGatewayControllerName,
@@ -79,6 +79,25 @@ func TestRouteReconciler_ReconcileCreates(t *testing.T) {
7979
},
8080
},
8181
}
82+
83+
notVpcLattice := &gwv1.Gateway{
84+
ObjectMeta: metav1.ObjectMeta{
85+
Name: "not-vpc-lattice",
86+
Namespace: "ns1",
87+
},
88+
Spec: gwv1.GatewaySpec{
89+
GatewayClassName: "not-amazon-vpc-lattice",
90+
Listeners: []gwv1.Listener{
91+
{
92+
Name: "http",
93+
Protocol: "HTTP",
94+
Port: 80,
95+
},
96+
},
97+
},
98+
}
99+
100+
k8sClient.Create(ctx, notVpcLattice.DeepCopy())
82101
k8sClient.Create(ctx, gw.DeepCopy())
83102

84103
svc := &corev1.Service{
@@ -131,6 +150,10 @@ func TestRouteReconciler_ReconcileCreates(t *testing.T) {
131150
Spec: gwv1.HTTPRouteSpec{
132151
CommonRouteSpec: gwv1.CommonRouteSpec{
133152
ParentRefs: []gwv1.ParentReference{
153+
// if route has multiple parents, we'll only use the managed vpc lattice gateway
154+
{
155+
Name: "not-vpc-lattice",
156+
},
134157
{
135158
Name: "my-gateway",
136159
},

‎pkg/gateway/model_build_lattice_service.go

+20-6
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,21 @@ func (t *latticeServiceModelBuildTask) buildLatticeService(ctx context.Context)
124124
}
125125

126126
for _, parentRef := range t.route.Spec().ParentRefs() {
127-
spec.ServiceNetworkNames = append(spec.ServiceNetworkNames, string(parentRef.Name))
127+
gw := &gwv1.Gateway{}
128+
parentNamespace := t.route.Namespace()
129+
if parentRef.Namespace != nil {
130+
parentNamespace = string(*parentRef.Namespace)
131+
}
132+
err := t.client.Get(ctx, client.ObjectKey{Name: string(parentRef.Name), Namespace: parentNamespace}, gw)
133+
if err != nil {
134+
t.log.Infof(ctx, "Ignoring route %s because failed to get gateway %s: %v", t.route.Name(), gw.Spec.GatewayClassName, err)
135+
continue
136+
}
137+
if k8s.IsControlledByLatticeGatewayController(ctx, t.client, gw) {
138+
spec.ServiceNetworkNames = append(spec.ServiceNetworkNames, string(parentRef.Name))
139+
} else {
140+
t.log.Infof(ctx, "Ignoring route %s because gateway %s is not managed by lattice gateway controller", t.route.Name(), gw.Name)
141+
}
128142
}
129143
if config.ServiceNetworkOverrideMode {
130144
spec.ServiceNetworkNames = []string{config.DefaultServiceNetwork}
@@ -160,7 +174,9 @@ func (t *latticeServiceModelBuildTask) buildLatticeService(ctx context.Context)
160174

161175
// returns empty string if not found
162176
func (t *latticeServiceModelBuildTask) getACMCertArn(ctx context.Context) (string, error) {
163-
gw, err := t.getGateway(ctx)
177+
// when a service is associate to multiple service network(s), all listener config MUST be same
178+
// so here we are only using the 1st gateway
179+
gw, err := t.findGateway(ctx)
164180
if err != nil {
165181
if apierrors.IsNotFound(err) && !t.route.DeletionTimestamp().IsZero() {
166182
return "", nil // ok if we're deleting the route
@@ -169,10 +185,8 @@ func (t *latticeServiceModelBuildTask) getACMCertArn(ctx context.Context) (strin
169185
}
170186

171187
for _, parentRef := range t.route.Spec().ParentRefs() {
172-
if parentRef.Name != t.route.Spec().ParentRefs()[0].Name {
173-
// when a service is associate to multiple service network(s), all listener config MUST be same
174-
// so here we are only using the 1st gateway
175-
t.log.Debugf(ctx, "Ignore ParentRef of different gateway %s-%s", parentRef.Name, *parentRef.Namespace)
188+
if string(parentRef.Name) != gw.Name {
189+
t.log.Debugf(ctx, "Ignore ParentRef of different gateway %s", parentRef.Name)
176190
continue
177191
}
178192

0 commit comments

Comments
 (0)