From eb0724e1d7c85585b03a7ebf9e921d2a2c78ab5d Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Sun, 9 Feb 2025 08:59:55 +0900 Subject: [PATCH 1/9] Update model_build_lattice_service_test for resolve issue --- pkg/controllers/route_controller.go | 30 ++- .../model_build_lattice_service_test.go | 232 +++++++++++------- 2 files changed, 166 insertions(+), 96 deletions(-) diff --git a/pkg/controllers/route_controller.go b/pkg/controllers/route_controller.go index 11f1025f..c4fb797f 100644 --- a/pkg/controllers/route_controller.go +++ b/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" ) @@ -85,6 +85,7 @@ func RegisterAllRouteControllers( mgr ctrl.Manager, ) error { mgrClient := mgr.GetClient() + gwEventHandler := eventhandlers.NewEnqueueRequestGatewayEvent(log, mgrClient) svcEventHandler := eventhandlers.NewServiceEventHandler(log, mgrClient) @@ -92,9 +93,24 @@ func RegisterAllRouteControllers( routeType core.RouteType gatewayApiType client.Object }{ - {core.HttpRouteType, &gwv1.HTTPRoute{}}, - {core.GrpcRouteType, &gwv1.GRPCRoute{}}, - {core.TlsRouteType, &gwv1alpha2.TLSRoute{}}, + {core.HttpRouteType, &gwv1.HTTPRoute{ + TypeMeta: metav1.TypeMeta{ + APIVersion: gwv1.GroupVersion.String(), + Kind: "HTTPRoute", + }, + }}, + {core.GrpcRouteType, &gwv1.GRPCRoute{ + TypeMeta: metav1.TypeMeta{ + APIVersion: gwv1.GroupVersion.String(), + Kind: "GRPCRoute", + }, + }}, + {core.TlsRouteType, &gwv1alpha2.TLSRoute{ + TypeMeta: metav1.TypeMeta{ + APIVersion: gwv1.GroupVersion.String(), + Kind: "TLSRoute", + }, + }}, } for _, routeInfo := range routeInfos { @@ -169,7 +185,6 @@ func (r *routeReconciler) reconcile(ctx context.Context, req ctrl.Request) error if err != nil { return client.IgnoreNotFound(err) } - if err = r.client.Get(ctx, req.NamespacedName, route.K8sObject()); err != nil { return client.IgnoreNotFound(err) } @@ -261,8 +276,7 @@ func (r *routeReconciler) isRouteRelevant(ctx context.Context, route core.Route) // make sure gateway is an aws-vpc-lattice gwClass := &gwv1.GatewayClass{} gwClassName := types.NamespacedName{ - Namespace: defaultNamespace, - Name: string(gw.Spec.GatewayClassName), + Name: string(gw.Spec.GatewayClassName), } if err := r.client.Get(ctx, gwClassName, gwClass); err != nil { @@ -573,7 +587,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 diff --git a/pkg/gateway/model_build_lattice_service_test.go b/pkg/gateway/model_build_lattice_service_test.go index c8b31f36..1fa8484b 100644 --- a/pkg/gateway/model_build_lattice_service_test.go +++ b/pkg/gateway/model_build_lattice_service_test.go @@ -2,6 +2,9 @@ package gateway import ( "context" + "testing" + + "github.com/aws/aws-application-networking-k8s/pkg/config" "github.com/aws/aws-application-networking-k8s/pkg/k8s" "github.com/aws/aws-application-networking-k8s/pkg/model/core" model "github.com/aws/aws-application-networking-k8s/pkg/model/lattice" @@ -13,7 +16,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_LatticeServiceModelBuild(t *testing.T) { @@ -25,6 +27,24 @@ func Test_LatticeServiceModelBuild(t *testing.T) { var weight2 = int32(90) var namespace = gwv1.Namespace("default") + vpcLatticeGatewayClass := gwv1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gwClass", + }, + Spec: gwv1.GatewayClassSpec{ + ControllerName: config.LatticeGatewayControllerName, + }, + } + vpcLatticeGateway := gwv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway1", + Namespace: "default", + }, + Spec: gwv1.GatewaySpec{ + GatewayClassName: gwv1.ObjectName(vpcLatticeGatewayClass.Name), + }, + } + namespacePtr := func(ns string) *gwv1.Namespace { p := gwv1.Namespace(ns) return &p @@ -52,7 +72,8 @@ func Test_LatticeServiceModelBuild(t *testing.T) { tests := []struct { name string - gw gwv1.Gateway + gwClass gwv1.GatewayClass + gws []gwv1.Gateway route core.Route wantErrIsNil bool wantIsDeleted bool @@ -62,12 +83,8 @@ func Test_LatticeServiceModelBuild(t *testing.T) { name: "Add LatticeService with hostname", wantIsDeleted: false, wantErrIsNil: true, - gw: gwv1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Name: "gateway1", - Namespace: "default", - }, - }, + gwClass: vpcLatticeGatewayClass, + gws: []gwv1.Gateway{vpcLatticeGateway}, route: core.NewHTTPRoute(gwv1.HTTPRoute{ ObjectMeta: metav1.ObjectMeta{ Name: "service1", @@ -77,8 +94,8 @@ func Test_LatticeServiceModelBuild(t *testing.T) { CommonRouteSpec: gwv1.CommonRouteSpec{ ParentRefs: []gwv1.ParentReference{ { - Name: "gateway1", - Namespace: namespacePtr("default"), + Name: gwv1.ObjectName(vpcLatticeGateway.Name), + Namespace: namespacePtr(vpcLatticeGateway.Namespace), }, }, }, @@ -95,18 +112,16 @@ func Test_LatticeServiceModelBuild(t *testing.T) { RouteType: core.HttpRouteType, }, CustomerDomainName: "test1.test.com", - ServiceNetworkNames: []string{"gateway1"}, + ServiceNetworkNames: []string{vpcLatticeGateway.Name}, }, }, { name: "Add LatticeService", wantIsDeleted: false, wantErrIsNil: true, - gw: gwv1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Name: "gateway1", - Namespace: "default", - }, + gwClass: vpcLatticeGatewayClass, + gws: []gwv1.Gateway{ + vpcLatticeGateway, }, route: core.NewHTTPRoute(gwv1.HTTPRoute{ ObjectMeta: metav1.ObjectMeta{ @@ -117,8 +132,8 @@ func Test_LatticeServiceModelBuild(t *testing.T) { CommonRouteSpec: gwv1.CommonRouteSpec{ ParentRefs: []gwv1.ParentReference{ { - Name: "gateway1", - Namespace: namespacePtr("default"), + Name: gwv1.ObjectName(vpcLatticeGateway.Name), + Namespace: namespacePtr(vpcLatticeGateway.Namespace), }, }, }, @@ -130,18 +145,16 @@ func Test_LatticeServiceModelBuild(t *testing.T) { RouteNamespace: "default", RouteType: core.HttpRouteType, }, - ServiceNetworkNames: []string{"gateway1"}, + ServiceNetworkNames: []string{vpcLatticeGateway.Name}, }, }, { name: "Add LatticeService with GRPCRoute", wantIsDeleted: false, wantErrIsNil: true, - gw: gwv1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Name: "gateway1", - Namespace: "test", - }, + gwClass: vpcLatticeGatewayClass, + gws: []gwv1.Gateway{ + vpcLatticeGateway, }, route: core.NewGRPCRoute(gwv1.GRPCRoute{ ObjectMeta: metav1.ObjectMeta{ @@ -152,7 +165,8 @@ func Test_LatticeServiceModelBuild(t *testing.T) { CommonRouteSpec: gwv1.CommonRouteSpec{ ParentRefs: []gwv1.ParentReference{ { - Name: "gateway1", + Name: gwv1.ObjectName(vpcLatticeGateway.Name), + Namespace: namespacePtr(vpcLatticeGateway.Namespace), }, }, }, @@ -164,24 +178,28 @@ func Test_LatticeServiceModelBuild(t *testing.T) { RouteNamespace: "test", RouteType: core.GrpcRouteType, }, - ServiceNetworkNames: []string{"gateway1"}, + ServiceNetworkNames: []string{vpcLatticeGateway.Name}, }, }, { + // TODO: name: "Delete LatticeService", wantIsDeleted: true, wantErrIsNil: true, - gw: gwv1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Name: "gateway2", - Namespace: "ns1", - }, - Spec: gwv1.GatewaySpec{ - Listeners: []gwv1.Listener{ - { - Name: httpSectionName, - Port: 80, - Protocol: "HTTP", + gwClass: vpcLatticeGatewayClass, + gws: []gwv1.Gateway{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway2", + Namespace: "ns1", + }, + Spec: gwv1.GatewaySpec{ + Listeners: []gwv1.Listener{ + { + Name: httpSectionName, + Port: 80, + Protocol: "HTTP", + }, }, }, }, @@ -229,22 +247,22 @@ func Test_LatticeServiceModelBuild(t *testing.T) { name: "Service with customer Cert ARN", wantIsDeleted: false, wantErrIsNil: true, - gw: gwv1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Name: "gateway1", - Namespace: "default", - }, - Spec: gwv1.GatewaySpec{ - Listeners: []gwv1.Listener{ - { - Name: "tls", - Port: 443, - Protocol: "HTTPS", - TLS: &gwv1.GatewayTLSConfig{ - Mode: &tlsModeTerminate, - CertificateRefs: nil, - Options: map[gwv1.AnnotationKey]gwv1.AnnotationValue{ - "application-networking.k8s.aws/certificate-arn": "cert-arn", + gwClass: vpcLatticeGatewayClass, + gws: []gwv1.Gateway{ + { + ObjectMeta: vpcLatticeGateway.ObjectMeta, + Spec: gwv1.GatewaySpec{ + Listeners: []gwv1.Listener{ + { + Name: "tls", + Port: 443, + Protocol: "HTTPS", + TLS: &gwv1.GatewayTLSConfig{ + Mode: &tlsModeTerminate, + CertificateRefs: nil, + Options: map[gwv1.AnnotationKey]gwv1.AnnotationValue{ + "application-networking.k8s.aws/certificate-arn": "cert-arn", + }, }, }, }, @@ -260,8 +278,8 @@ func Test_LatticeServiceModelBuild(t *testing.T) { CommonRouteSpec: gwv1.CommonRouteSpec{ ParentRefs: []gwv1.ParentReference{ { - Name: "gateway1", - Namespace: namespacePtr("default"), + Name: gwv1.ObjectName(vpcLatticeGateway.Name), + Namespace: namespacePtr(vpcLatticeGateway.Namespace), SectionName: &tlsSectionName, }, }, @@ -275,16 +293,14 @@ func Test_LatticeServiceModelBuild(t *testing.T) { RouteType: core.HttpRouteType, }, CustomerCertARN: "cert-arn", - ServiceNetworkNames: []string{"gateway1"}, + ServiceNetworkNames: []string{vpcLatticeGateway.Name}, }, }, { - name: "GW does not exist", - gw: gwv1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Name: "gateway1", - Namespace: "default", - }, + name: "GW does not exist", + gwClass: vpcLatticeGatewayClass, + gws: []gwv1.Gateway{ + vpcLatticeGateway, }, route: core.NewHTTPRoute(gwv1.HTTPRoute{ ObjectMeta: metav1.ObjectMeta{ @@ -308,20 +324,20 @@ func Test_LatticeServiceModelBuild(t *testing.T) { name: "Service with TLS section but no cert arn", wantIsDeleted: false, wantErrIsNil: true, - gw: gwv1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Name: "gateway1", - Namespace: "default", - }, - Spec: gwv1.GatewaySpec{ - Listeners: []gwv1.Listener{ - { - Name: "tls", - Port: 443, - Protocol: "HTTPS", - TLS: &gwv1.GatewayTLSConfig{ - Mode: &tlsModeTerminate, - CertificateRefs: nil, + gwClass: vpcLatticeGatewayClass, + gws: []gwv1.Gateway{ + { + ObjectMeta: vpcLatticeGateway.ObjectMeta, + Spec: gwv1.GatewaySpec{ + Listeners: []gwv1.Listener{ + { + Name: "tls", + Port: 443, + Protocol: "HTTPS", + TLS: &gwv1.GatewayTLSConfig{ + Mode: &tlsModeTerminate, + CertificateRefs: nil, + }, }, }, }, @@ -336,8 +352,8 @@ func Test_LatticeServiceModelBuild(t *testing.T) { CommonRouteSpec: gwv1.CommonRouteSpec{ ParentRefs: []gwv1.ParentReference{ { - Name: "gateway1", - Namespace: namespacePtr("default"), + Name: gwv1.ObjectName(vpcLatticeGateway.Name), + Namespace: namespacePtr(vpcLatticeGateway.Namespace), SectionName: &tlsSectionName, }, }, @@ -350,18 +366,16 @@ func Test_LatticeServiceModelBuild(t *testing.T) { RouteNamespace: "default", RouteType: core.HttpRouteType, }, - ServiceNetworkNames: []string{"gateway1"}, + ServiceNetworkNames: []string{vpcLatticeGateway.Name}, }, }, { name: "Multiple service networks", wantIsDeleted: false, wantErrIsNil: true, - gw: gwv1.Gateway{ - ObjectMeta: metav1.ObjectMeta{ - Name: "gateway1", - Namespace: "default", - }, + gwClass: vpcLatticeGatewayClass, + gws: []gwv1.Gateway{ + vpcLatticeGateway, }, route: core.NewHTTPRoute(gwv1.HTTPRoute{ ObjectMeta: metav1.ObjectMeta{ @@ -372,8 +386,8 @@ func Test_LatticeServiceModelBuild(t *testing.T) { CommonRouteSpec: gwv1.CommonRouteSpec{ ParentRefs: []gwv1.ParentReference{ { - Name: "gateway1", - Namespace: namespacePtr("default"), + Name: gwv1.ObjectName(vpcLatticeGateway.Name), + Namespace: namespacePtr(vpcLatticeGateway.Namespace), }, { Name: "gateway2", @@ -389,9 +403,48 @@ func Test_LatticeServiceModelBuild(t *testing.T) { RouteNamespace: "default", RouteType: core.HttpRouteType, }, - ServiceNetworkNames: []string{"gateway1", "gateway2"}, + ServiceNetworkNames: []string{vpcLatticeGateway.Name, "gateway2"}, }, }, + //{ + // name: "Multiple service networks with one different controller", + // wantIsDeleted: false, + // wantErrIsNil: true, + // gw: gwv1.Gateway{ + // ObjectMeta: metav1.ObjectMeta{ + // Name: "gateway1", + // Namespace: "default", + // }, + // }, + // route: core.NewHTTPRoute(gwv1.HTTPRoute{ + // ObjectMeta: metav1.ObjectMeta{ + // Name: "service1", + // Namespace: "default", + // }, + // Spec: gwv1.HTTPRouteSpec{ + // CommonRouteSpec: gwv1.CommonRouteSpec{ + // ParentRefs: []gwv1.ParentReference{ + // { + // Name: "gateway1", + // Namespace: namespacePtr("default"), + // }, + // { + // Name: "not-lattice", + // Namespace: namespacePtr("ns2"), + // }, + // }, + // }, + // }, + // }), + // expected: model.ServiceSpec{ + // ServiceTagFields: model.ServiceTagFields{ + // RouteName: "service1", + // RouteNamespace: "default", + // RouteType: core.HttpRouteType, + // }, + // ServiceNetworkNames: []string{"gateway1"}, + // }, + //}, } for _, tt := range tests { @@ -405,7 +458,10 @@ func Test_LatticeServiceModelBuild(t *testing.T) { gwv1.Install(k8sSchema) k8sClient := testclient.NewClientBuilder().WithScheme(k8sSchema).Build() - assert.NoError(t, k8sClient.Create(ctx, tt.gw.DeepCopy())) + assert.NoError(t, k8sClient.Create(ctx, &tt.gwClass)) + for _, gw := range tt.gws { + assert.NoError(t, k8sClient.Create(ctx, gw.DeepCopy())) + } stack := core.NewDefaultStack(core.StackID(k8s.NamespacedName(tt.route.K8sObject()))) task := &latticeServiceModelBuildTask{ From c4a6caeaf4978a37f13979e457b01399342ba969 Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Sun, 9 Feb 2025 09:33:12 +0900 Subject: [PATCH 2/9] Added validation to check if parentRef is managed by vpcLatticeController. --- pkg/gateway/model_build_lattice_service.go | 23 ++++ .../model_build_lattice_service_test.go | 121 +++++++++--------- 2 files changed, 87 insertions(+), 57 deletions(-) diff --git a/pkg/gateway/model_build_lattice_service.go b/pkg/gateway/model_build_lattice_service.go index dd54994b..1d9618fb 100644 --- a/pkg/gateway/model_build_lattice_service.go +++ b/pkg/gateway/model_build_lattice_service.go @@ -124,6 +124,29 @@ func (t *latticeServiceModelBuildTask) buildLatticeService(ctx context.Context) } for _, parentRef := range t.route.Spec().ParentRefs() { + 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 { + //TODO: error message + t.log.Infof(ctx, "Ignore %s route because failed to get gateway %s: %v", t.route.Name(), parentRef.Name, err) + continue + } + gwClass := &gwv1.GatewayClass{} + // GatewayClass is cluster-scoped resource, so we don't need to specify namespace + err = t.client.Get(ctx, client.ObjectKey{Name: string(gw.Spec.GatewayClassName)}, gwClass) + if err != nil { + //TODO: error message + t.log.Infof(ctx, "Ignore %s route because failed to get gateway class %s: %v", t.route.Name(), gw.Spec.GatewayClassName, err) + continue + } + if gwClass.Spec.ControllerName != config.LatticeGatewayControllerName { + t.log.Infof(ctx, "Ignore %s route because gateway class %s is not for lattice gateway", t.route.Name(), gw.Spec.GatewayClassName) + continue + } spec.ServiceNetworkNames = append(spec.ServiceNetworkNames, string(parentRef.Name)) } if config.ServiceNetworkOverrideMode { diff --git a/pkg/gateway/model_build_lattice_service_test.go b/pkg/gateway/model_build_lattice_service_test.go index 1fa8484b..2aa23381 100644 --- a/pkg/gateway/model_build_lattice_service_test.go +++ b/pkg/gateway/model_build_lattice_service_test.go @@ -182,27 +182,12 @@ func Test_LatticeServiceModelBuild(t *testing.T) { }, }, { - // TODO: name: "Delete LatticeService", wantIsDeleted: true, wantErrIsNil: true, gwClass: vpcLatticeGatewayClass, gws: []gwv1.Gateway{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "gateway2", - Namespace: "ns1", - }, - Spec: gwv1.GatewaySpec{ - Listeners: []gwv1.Listener{ - { - Name: httpSectionName, - Port: 80, - Protocol: "HTTP", - }, - }, - }, - }, + vpcLatticeGateway, }, route: core.NewHTTPRoute(gwv1.HTTPRoute{ ObjectMeta: metav1.ObjectMeta{ @@ -215,7 +200,8 @@ func Test_LatticeServiceModelBuild(t *testing.T) { CommonRouteSpec: gwv1.CommonRouteSpec{ ParentRefs: []gwv1.ParentReference{ { - Name: "gateway2", + Name: gwv1.ObjectName(vpcLatticeGateway.Name), + Namespace: namespacePtr(vpcLatticeGateway.Namespace), SectionName: &httpSectionName, }, }, @@ -240,7 +226,7 @@ func Test_LatticeServiceModelBuild(t *testing.T) { RouteNamespace: "ns1", RouteType: core.HttpRouteType, }, - ServiceNetworkNames: []string{"gateway2"}, + ServiceNetworkNames: []string{vpcLatticeGateway.Name}, }, }, { @@ -252,6 +238,7 @@ func Test_LatticeServiceModelBuild(t *testing.T) { { ObjectMeta: vpcLatticeGateway.ObjectMeta, Spec: gwv1.GatewaySpec{ + GatewayClassName: gwv1.ObjectName(vpcLatticeGatewayClass.Name), Listeners: []gwv1.Listener{ { Name: "tls", @@ -329,6 +316,7 @@ func Test_LatticeServiceModelBuild(t *testing.T) { { ObjectMeta: vpcLatticeGateway.ObjectMeta, Spec: gwv1.GatewaySpec{ + GatewayClassName: gwv1.ObjectName(vpcLatticeGatewayClass.Name), Listeners: []gwv1.Listener{ { Name: "tls", @@ -376,6 +364,15 @@ func Test_LatticeServiceModelBuild(t *testing.T) { gwClass: vpcLatticeGatewayClass, gws: []gwv1.Gateway{ vpcLatticeGateway, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway2", + Namespace: "ns2", + }, + Spec: gwv1.GatewaySpec{ + GatewayClassName: gwv1.ObjectName(vpcLatticeGatewayClass.Name), + }, + }, }, route: core.NewHTTPRoute(gwv1.HTTPRoute{ ObjectMeta: metav1.ObjectMeta{ @@ -406,45 +403,55 @@ func Test_LatticeServiceModelBuild(t *testing.T) { ServiceNetworkNames: []string{vpcLatticeGateway.Name, "gateway2"}, }, }, - //{ - // name: "Multiple service networks with one different controller", - // wantIsDeleted: false, - // wantErrIsNil: true, - // gw: gwv1.Gateway{ - // ObjectMeta: metav1.ObjectMeta{ - // Name: "gateway1", - // Namespace: "default", - // }, - // }, - // route: core.NewHTTPRoute(gwv1.HTTPRoute{ - // ObjectMeta: metav1.ObjectMeta{ - // Name: "service1", - // Namespace: "default", - // }, - // Spec: gwv1.HTTPRouteSpec{ - // CommonRouteSpec: gwv1.CommonRouteSpec{ - // ParentRefs: []gwv1.ParentReference{ - // { - // Name: "gateway1", - // Namespace: namespacePtr("default"), - // }, - // { - // Name: "not-lattice", - // Namespace: namespacePtr("ns2"), - // }, - // }, - // }, - // }, - // }), - // expected: model.ServiceSpec{ - // ServiceTagFields: model.ServiceTagFields{ - // RouteName: "service1", - // RouteNamespace: "default", - // RouteType: core.HttpRouteType, - // }, - // ServiceNetworkNames: []string{"gateway1"}, - // }, - //}, + { + name: "Multiple service networks with one different controller", + wantIsDeleted: false, + wantErrIsNil: true, + gwClass: vpcLatticeGatewayClass, + gws: []gwv1.Gateway{ + vpcLatticeGateway, + // managed by different controller gateway + { + ObjectMeta: metav1.ObjectMeta{ + Name: "not-lattice", + Namespace: "ns2", + }, + Spec: gwv1.GatewaySpec{ + GatewayClassName: gwv1.ObjectName("not-lattice-gwClass"), + }, + }, + }, + route: core.NewHTTPRoute(gwv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "service1", + Namespace: "default", + }, + Spec: gwv1.HTTPRouteSpec{ + CommonRouteSpec: gwv1.CommonRouteSpec{ + // has two parent refs and one is not managed by lattice + ParentRefs: []gwv1.ParentReference{ + { + Name: gwv1.ObjectName(vpcLatticeGateway.Name), + Namespace: namespacePtr(vpcLatticeGateway.Namespace), + }, + { + Name: "not-lattice", + Namespace: namespacePtr("ns2"), + }, + }, + }, + }, + }), + expected: model.ServiceSpec{ + ServiceTagFields: model.ServiceTagFields{ + RouteName: "service1", + RouteNamespace: "default", + RouteType: core.HttpRouteType, + }, + // only the lattice gateway is added + ServiceNetworkNames: []string{vpcLatticeGateway.Name}, + }, + }, } for _, tt := range tests { From e002b4e088864397728ec1b9844d3ce09f782256 Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Sun, 9 Feb 2025 09:52:18 +0900 Subject: [PATCH 3/9] Fix route_controller_test --- .github/workflows/deploy-dev.yml | 41 ++++++++++++++++++++++ pkg/controllers/route_controller.go | 23 +++--------- pkg/controllers/route_controller_test.go | 6 ++-- pkg/gateway/model_build_lattice_service.go | 8 ++--- 4 files changed, 51 insertions(+), 27 deletions(-) create mode 100644 .github/workflows/deploy-dev.yml diff --git a/.github/workflows/deploy-dev.yml b/.github/workflows/deploy-dev.yml new file mode 100644 index 00000000..1703a606 --- /dev/null +++ b/.github/workflows/deploy-dev.yml @@ -0,0 +1,41 @@ +name: Build and Deploy Apps + +on: + workflow_dispatch: + push: +jobs: + build: + name: Build and Push to ECR with Docker Buildx Cache + runs-on: ubuntu-latest + permissions: + contents: read + id-token: write + + strategy: + matrix: + service: [aws-gateway-controller] + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Set up Docker Buildx + uses: docker/setup-buildx-action@v3 + + - name: Configure AWS credentials from OIDC + uses: aws-actions/configure-aws-credentials@v4 + with: + aws-region: ap-northeast-1 + role-to-assume: ${{ vars.AWS_ROLE_ARN }} + role-session-name: GitHubActions + + - name: Login to Amazon ECR + uses: aws-actions/amazon-ecr-login@v2 + + - name: Build and push Docker image with Buildx + run: | + docker buildx build \ + -t "${{ vars.AWS_ACCOUNT_ID }}.dkr.ecr.ap-northeast-1.amazonaws.com/${{ matrix.service }}:${{github.sha}}" \ + -t "${{ vars.AWS_ACCOUNT_ID }}.dkr.ecr.ap-northeast-1.amazonaws.com/${{ matrix.service }}:latest" \ + --push \ + . \ No newline at end of file diff --git a/pkg/controllers/route_controller.go b/pkg/controllers/route_controller.go index c4fb797f..b23ceb04 100644 --- a/pkg/controllers/route_controller.go +++ b/pkg/controllers/route_controller.go @@ -85,7 +85,6 @@ func RegisterAllRouteControllers( mgr ctrl.Manager, ) error { mgrClient := mgr.GetClient() - gwEventHandler := eventhandlers.NewEnqueueRequestGatewayEvent(log, mgrClient) svcEventHandler := eventhandlers.NewServiceEventHandler(log, mgrClient) @@ -93,24 +92,9 @@ func RegisterAllRouteControllers( routeType core.RouteType gatewayApiType client.Object }{ - {core.HttpRouteType, &gwv1.HTTPRoute{ - TypeMeta: metav1.TypeMeta{ - APIVersion: gwv1.GroupVersion.String(), - Kind: "HTTPRoute", - }, - }}, - {core.GrpcRouteType, &gwv1.GRPCRoute{ - TypeMeta: metav1.TypeMeta{ - APIVersion: gwv1.GroupVersion.String(), - Kind: "GRPCRoute", - }, - }}, - {core.TlsRouteType, &gwv1alpha2.TLSRoute{ - TypeMeta: metav1.TypeMeta{ - APIVersion: gwv1.GroupVersion.String(), - Kind: "TLSRoute", - }, - }}, + {core.HttpRouteType, &gwv1.HTTPRoute{}}, + {core.GrpcRouteType, &gwv1.GRPCRoute{}}, + {core.TlsRouteType, &gwv1alpha2.TLSRoute{}}, } for _, routeInfo := range routeInfos { @@ -185,6 +169,7 @@ func (r *routeReconciler) reconcile(ctx context.Context, req ctrl.Request) error if err != nil { return client.IgnoreNotFound(err) } + if err = r.client.Get(ctx, req.NamespacedName, route.K8sObject()); err != nil { return client.IgnoreNotFound(err) } diff --git a/pkg/controllers/route_controller_test.go b/pkg/controllers/route_controller_test.go index 3a9ff567..d00ed3b1 100644 --- a/pkg/controllers/route_controller_test.go +++ b/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, diff --git a/pkg/gateway/model_build_lattice_service.go b/pkg/gateway/model_build_lattice_service.go index 1d9618fb..fa27b5dd 100644 --- a/pkg/gateway/model_build_lattice_service.go +++ b/pkg/gateway/model_build_lattice_service.go @@ -131,20 +131,18 @@ func (t *latticeServiceModelBuildTask) buildLatticeService(ctx context.Context) } err := t.client.Get(ctx, client.ObjectKey{Name: string(parentRef.Name), Namespace: parentNamespace}, gw) if err != nil { - //TODO: error message - t.log.Infof(ctx, "Ignore %s route because failed to get gateway %s: %v", t.route.Name(), parentRef.Name, err) + t.log.Infof(ctx, "Ignoring route %s because failed to get gateway %s: %v", t.route.Name(), gw.Spec.GatewayClassName, err) continue } gwClass := &gwv1.GatewayClass{} // GatewayClass is cluster-scoped resource, so we don't need to specify namespace err = t.client.Get(ctx, client.ObjectKey{Name: string(gw.Spec.GatewayClassName)}, gwClass) if err != nil { - //TODO: error message - t.log.Infof(ctx, "Ignore %s route because failed to get gateway class %s: %v", t.route.Name(), gw.Spec.GatewayClassName, err) + t.log.Infof(ctx, "Ignoring route %s because failed to get gateway class %s: %v", t.route.Name(), gw.Spec.GatewayClassName, err) continue } if gwClass.Spec.ControllerName != config.LatticeGatewayControllerName { - t.log.Infof(ctx, "Ignore %s route because gateway class %s is not for lattice gateway", t.route.Name(), gw.Spec.GatewayClassName) + t.log.Infof(ctx, "Ignoring route %s because gateway class %s is not for a VPCLattice", t.route.Name(), gw.Spec.GatewayClassName) continue } spec.ServiceNetworkNames = append(spec.ServiceNetworkNames, string(parentRef.Name)) From 21ba0afbd788c2592cfbbf3eeb8163d47aab244d Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Sun, 9 Feb 2025 19:15:36 +0900 Subject: [PATCH 4/9] Add validate gateway logic --- .github/workflows/deploy-dev.yml | 41 -------------------------------- pkg/k8s/utils.go | 14 +++++++++++ 2 files changed, 14 insertions(+), 41 deletions(-) delete mode 100644 .github/workflows/deploy-dev.yml diff --git a/.github/workflows/deploy-dev.yml b/.github/workflows/deploy-dev.yml deleted file mode 100644 index 1703a606..00000000 --- a/.github/workflows/deploy-dev.yml +++ /dev/null @@ -1,41 +0,0 @@ -name: Build and Deploy Apps - -on: - workflow_dispatch: - push: -jobs: - build: - name: Build and Push to ECR with Docker Buildx Cache - runs-on: ubuntu-latest - permissions: - contents: read - id-token: write - - strategy: - matrix: - service: [aws-gateway-controller] - - steps: - - name: Checkout code - uses: actions/checkout@v4 - - - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v3 - - - name: Configure AWS credentials from OIDC - uses: aws-actions/configure-aws-credentials@v4 - with: - aws-region: ap-northeast-1 - role-to-assume: ${{ vars.AWS_ROLE_ARN }} - role-session-name: GitHubActions - - - name: Login to Amazon ECR - uses: aws-actions/amazon-ecr-login@v2 - - - name: Build and push Docker image with Buildx - run: | - docker buildx build \ - -t "${{ vars.AWS_ACCOUNT_ID }}.dkr.ecr.ap-northeast-1.amazonaws.com/${{ matrix.service }}:${{github.sha}}" \ - -t "${{ vars.AWS_ACCOUNT_ID }}.dkr.ecr.ap-northeast-1.amazonaws.com/${{ matrix.service }}:latest" \ - --push \ - . \ No newline at end of file diff --git a/pkg/k8s/utils.go b/pkg/k8s/utils.go index 8e53a927..df987ed0 100644 --- a/pkg/k8s/utils.go +++ b/pkg/k8s/utils.go @@ -2,11 +2,14 @@ package k8s import ( "context" + + "github.com/aws/aws-application-networking-k8s/pkg/config" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/discovery" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" gwv1 "sigs.k8s.io/gateway-api/apis/v1" ) @@ -53,6 +56,17 @@ func IsGVKSupported(mgr ctrl.Manager, groupVersion string, kind string) (bool, e return false, nil } +// validate if the gateway is managed by the lattice gateway controller +func IsManagedGateway(ctx context.Context, c client.Client, gw *gwv1.Gateway) bool { + gwClass := &gwv1.GatewayClass{} + // GatewayClass is cluster-scoped resource, so we don't need to specify namespace + err := c.Get(ctx, client.ObjectKey{Name: string(gw.Spec.GatewayClassName)}, gwClass) + if err != nil { + return false + } + return gwClass.Spec.ControllerName == config.LatticeGatewayControllerName +} + 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 { From 1fe9ea4f5ea3f65c7c2232bd3c806160a20f3072 Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Sun, 9 Feb 2025 19:16:27 +0900 Subject: [PATCH 5/9] Update validate gateway logic and tests --- pkg/gateway/model_build_lattice_service.go | 23 +- .../model_build_lattice_service_test.go | 2 +- pkg/gateway/model_build_listener.go | 62 +++-- pkg/gateway/model_build_listener_test.go | 260 +++++++++++------- pkg/gateway/model_build_targets_test.go | 6 + .../integration/access_log_policy_test.go | 2 +- 6 files changed, 208 insertions(+), 147 deletions(-) diff --git a/pkg/gateway/model_build_lattice_service.go b/pkg/gateway/model_build_lattice_service.go index fa27b5dd..3d3214f5 100644 --- a/pkg/gateway/model_build_lattice_service.go +++ b/pkg/gateway/model_build_lattice_service.go @@ -134,18 +134,11 @@ func (t *latticeServiceModelBuildTask) buildLatticeService(ctx context.Context) t.log.Infof(ctx, "Ignoring route %s because failed to get gateway %s: %v", t.route.Name(), gw.Spec.GatewayClassName, err) continue } - gwClass := &gwv1.GatewayClass{} - // GatewayClass is cluster-scoped resource, so we don't need to specify namespace - err = t.client.Get(ctx, client.ObjectKey{Name: string(gw.Spec.GatewayClassName)}, gwClass) - if err != nil { - t.log.Infof(ctx, "Ignoring route %s because failed to get gateway class %s: %v", t.route.Name(), gw.Spec.GatewayClassName, err) - continue - } - if gwClass.Spec.ControllerName != config.LatticeGatewayControllerName { - t.log.Infof(ctx, "Ignoring route %s because gateway class %s is not for a VPCLattice", t.route.Name(), gw.Spec.GatewayClassName) - continue + if k8s.IsManagedGateway(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) } - spec.ServiceNetworkNames = append(spec.ServiceNetworkNames, string(parentRef.Name)) } if config.ServiceNetworkOverrideMode { spec.ServiceNetworkNames = []string{config.DefaultServiceNetwork} @@ -181,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.getFirstGateway(ctx) if err != nil { if apierrors.IsNotFound(err) && !t.route.DeletionTimestamp().IsZero() { return "", nil // ok if we're deleting the route @@ -190,9 +185,7 @@ 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 + if string(parentRef.Name) != gw.Name { t.log.Debugf(ctx, "Ignore ParentRef of different gateway %s-%s", parentRef.Name, *parentRef.Namespace) continue } diff --git a/pkg/gateway/model_build_lattice_service_test.go b/pkg/gateway/model_build_lattice_service_test.go index 2aa23381..a49ae2d6 100644 --- a/pkg/gateway/model_build_lattice_service_test.go +++ b/pkg/gateway/model_build_lattice_service_test.go @@ -465,7 +465,7 @@ func Test_LatticeServiceModelBuild(t *testing.T) { gwv1.Install(k8sSchema) k8sClient := testclient.NewClientBuilder().WithScheme(k8sSchema).Build() - assert.NoError(t, k8sClient.Create(ctx, &tt.gwClass)) + assert.NoError(t, k8sClient.Create(ctx, tt.gwClass.DeepCopy())) for _, gw := range tt.gws { assert.NoError(t, k8sClient.Create(ctx, gw.DeepCopy())) } diff --git a/pkg/gateway/model_build_listener.go b/pkg/gateway/model_build_listener.go index 3a962df9..c7d9d3f1 100644 --- a/pkg/gateway/model_build_listener.go +++ b/pkg/gateway/model_build_listener.go @@ -7,9 +7,10 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/vpclattice" - "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" gwv1 "sigs.k8s.io/gateway-api/apis/v1" + "github.com/aws/aws-application-networking-k8s/pkg/k8s" model "github.com/aws/aws-application-networking-k8s/pkg/model/lattice" ) @@ -20,16 +21,13 @@ const ( func (t *latticeServiceModelBuildTask) extractListenerInfo( ctx context.Context, parentRef gwv1.ParentReference, + gw *gwv1.Gateway, ) (int64, string, error) { if parentRef.SectionName != nil { t.log.Debugf(ctx, "Listener parentRef SectionName is %s", *parentRef.SectionName) } t.log.Debugf(ctx, "Building Listener for Route %s-%s", t.route.Name(), t.route.Namespace()) - gw, err := t.getGateway(ctx) - if err != nil { - return 0, "", err - } // If no SectionName is specified, use the first listener port if parentRef.SectionName == nil { if len(gw.Spec.Listeners) == 0 { @@ -58,42 +56,54 @@ func isTLSPassthroughGatewayListener(listener *gwv1.Listener) bool { return listener.Protocol == gwv1.TLSProtocolType && listener.TLS != nil && listener.TLS.Mode != nil && *listener.TLS.Mode == gwv1.TLSModePassthrough } -func (t *latticeServiceModelBuildTask) getGateway(ctx context.Context) (*gwv1.Gateway, error) { - var gwNamespace = t.route.Namespace() - if t.route.Spec().ParentRefs()[0].Namespace != nil { - gwNamespace = string(*t.route.Spec().ParentRefs()[0].Namespace) - } - +func (t *latticeServiceModelBuildTask) getFirstGateway(ctx context.Context) (*gwv1.Gateway, error) { gw := &gwv1.Gateway{} - gwName := types.NamespacedName{ - Namespace: gwNamespace, - Name: string(t.route.Spec().ParentRefs()[0].Name), - } + gwNamespace := t.route.Namespace() + fails := []string{} + for _, parentRef := range t.route.Spec().ParentRefs() { + if parentRef.Namespace != nil { + gwNamespace = string(*parentRef.Namespace) + } + gwName := client.ObjectKey{ + Namespace: gwNamespace, + Name: string(parentRef.Name), + } + if err := t.client.Get(ctx, gwName, gw); err != nil { + t.log.Infof(ctx, "Ignoring route %s because failed to get gateway %s: %v", t.route.Name(), parentRef.Name, err) + continue + } + if k8s.IsManagedGateway(ctx, t.client, gw) { + return gw, nil + } + fails = append(fails, gwName.String()) - if err := t.client.Get(ctx, gwName, gw); err != nil { - return nil, fmt.Errorf("failed to get gateway, name %s, err %w", gwName, err) } - return gw, nil + return nil, fmt.Errorf("failed to get gateway, name %s", fails) } func (t *latticeServiceModelBuildTask) buildListeners(ctx context.Context, stackSvcId string) error { - if len(t.route.Spec().ParentRefs()) == 0 { - t.log.Debugf(ctx, "No ParentRefs on route %s-%s, nothing to do", t.route.Name(), t.route.Namespace()) - } if !t.route.DeletionTimestamp().IsZero() { t.log.Debugf(ctx, "Route %s-%s is deleted, skipping listener build", t.route.Name(), t.route.Namespace()) return nil } + if len(t.route.Spec().ParentRefs()) == 0 { + t.log.Debugf(ctx, "No ParentRefs on route %s-%s, nothing to do", t.route.Name(), t.route.Namespace()) + return nil + } + + // 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.getFirstGateway(ctx) + if err != nil { + return err + } 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 { continue } - port, protocol, err := t.extractListenerInfo(ctx, parentRef) + port, protocol, err := t.extractListenerInfo(ctx, parentRef, gw) if err != nil { return err } diff --git a/pkg/gateway/model_build_listener_test.go b/pkg/gateway/model_build_listener_test.go index 762f4ce6..03a994b8 100644 --- a/pkg/gateway/model_build_listener_test.go +++ b/pkg/gateway/model_build_listener_test.go @@ -2,24 +2,24 @@ package gateway import ( "context" - "errors" "testing" + anv1alpha1 "github.com/aws/aws-application-networking-k8s/pkg/apis/applicationnetworking/v1alpha1" + "github.com/aws/aws-application-networking-k8s/pkg/config" + "github.com/aws/aws-application-networking-k8s/pkg/k8s" + "github.com/aws/aws-application-networking-k8s/pkg/model/core" + model "github.com/aws/aws-application-networking-k8s/pkg/model/lattice" + "github.com/aws/aws-application-networking-k8s/pkg/utils/gwlog" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/vpclattice" "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + testclient "sigs.k8s.io/controller-runtime/pkg/client/fake" gwv1 "sigs.k8s.io/gateway-api/apis/v1" gwv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" - - 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" - "github.com/aws/aws-application-networking-k8s/pkg/k8s" - "github.com/aws/aws-application-networking-k8s/pkg/model/core" - model "github.com/aws/aws-application-networking-k8s/pkg/model/lattice" - "github.com/aws/aws-application-networking-k8s/pkg/utils/gwlog" ) type K8sGatewayListenerType int @@ -30,12 +30,6 @@ const ( TLS_PASSTHROUGH ) -// PortNumberPtr translates an int to a *PortNumber -func PortNumberPtr(p int) *gwv1.PortNumber { - result := gwv1.PortNumber(p) - return &result -} - func Test_ListenerModelBuild(t *testing.T) { var sectionName gwv1.SectionName = "my-gw-listener" var missingSectionName gwv1.SectionName = "miss" @@ -47,26 +41,53 @@ func Test_ListenerModelBuild(t *testing.T) { Kind: &serviceKind, }, } + vpcLatticeGatewayClass := gwv1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gwClass", + }, + Spec: gwv1.GatewayClassSpec{ + ControllerName: config.LatticeGatewayControllerName, + }, + } + vpcLatticeGateway := gwv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "gateway1", + Namespace: "default", + }, + Spec: gwv1.GatewaySpec{ + GatewayClassName: gwv1.ObjectName(vpcLatticeGatewayClass.Name), + }, + } + vpcLatticeGatewayWithListeners := func(listeners ...gwv1.Listener) gwv1.Gateway { + gw := vpcLatticeGateway.DeepCopy() + gw.Spec.Listeners = listeners + return *gw + } + + tlsModePassthrough := gwv1.TLSModePassthrough + tlsModeTerminate := gwv1.TLSModeTerminate + serviceImportName := gwv1.ObjectName("k8s-service3") tests := []struct { name string - gwListenerPort gwv1.PortNumber + gw gwv1.Gateway route core.Route wantErrIsNil bool k8sGetGatewayCall bool brTgBuilderBuildCall bool k8sGetServiceImportCall bool - k8sGatewayReturnOK bool - k8sGatewayListenerType K8sGatewayListenerType expectedSpec []model.ListenerSpec }{ { - name: "Build HTTP listener", - gwListenerPort: *PortNumberPtr(80), - wantErrIsNil: true, - k8sGetGatewayCall: true, - k8sGatewayReturnOK: true, - k8sGatewayListenerType: HTTP, + name: "Build HTTP listener", + wantErrIsNil: true, + k8sGetGatewayCall: true, + gw: vpcLatticeGatewayWithListeners( + gwv1.Listener{ + Port: 80, + Protocol: "HTTP", + Name: sectionName, + }), route: core.NewHTTPRoute(gwv1.HTTPRoute{ ObjectMeta: metav1.ObjectMeta{ Name: "service1", @@ -76,7 +97,7 @@ func Test_ListenerModelBuild(t *testing.T) { CommonRouteSpec: gwv1.CommonRouteSpec{ ParentRefs: []gwv1.ParentReference{ { - Name: "gw1", + Name: gwv1.ObjectName(vpcLatticeGateway.Name), SectionName: §ionName, }, }, @@ -106,12 +127,18 @@ func Test_ListenerModelBuild(t *testing.T) { }, }, { - name: "Build HTTPS listener", - gwListenerPort: *PortNumberPtr(443), - wantErrIsNil: true, - k8sGetGatewayCall: true, - k8sGatewayReturnOK: true, - k8sGatewayListenerType: HTTPS, + name: "Build HTTPS listener", + wantErrIsNil: true, + k8sGetGatewayCall: true, + gw: vpcLatticeGatewayWithListeners( + gwv1.Listener{ + Port: 443, + Protocol: "HTTPS", + Name: sectionName, + TLS: &gwv1.GatewayTLSConfig{ + Mode: &tlsModeTerminate, + }, + }), route: core.NewHTTPRoute(gwv1.HTTPRoute{ ObjectMeta: metav1.ObjectMeta{ Name: "service1", @@ -121,7 +148,7 @@ func Test_ListenerModelBuild(t *testing.T) { CommonRouteSpec: gwv1.CommonRouteSpec{ ParentRefs: []gwv1.ParentReference{ { - Name: "gw1", + Name: gwv1.ObjectName(vpcLatticeGateway.Name), SectionName: §ionName, }, }, @@ -152,13 +179,19 @@ func Test_ListenerModelBuild(t *testing.T) { }, { name: "Build TLS_PASSTHROUGH listener", - gwListenerPort: *PortNumberPtr(443), wantErrIsNil: true, k8sGetGatewayCall: true, k8sGetServiceImportCall: true, - k8sGatewayReturnOK: true, brTgBuilderBuildCall: true, - k8sGatewayListenerType: TLS_PASSTHROUGH, + gw: vpcLatticeGatewayWithListeners( + gwv1.Listener{ + Port: 443, + Protocol: "TLS", + Name: sectionName, + TLS: &gwv1.GatewayTLSConfig{ + Mode: &tlsModePassthrough, + }, + }), route: core.NewTLSRoute(gwv1alpha2.TLSRoute{ ObjectMeta: metav1.ObjectMeta{ Name: "service1", @@ -168,7 +201,7 @@ func Test_ListenerModelBuild(t *testing.T) { CommonRouteSpec: gwv1.CommonRouteSpec{ ParentRefs: []gwv1.ParentReference{ { - Name: "gw1", + Name: gwv1.ObjectName(vpcLatticeGateway.Name), SectionName: §ionName, }, }, @@ -192,7 +225,7 @@ func Test_ListenerModelBuild(t *testing.T) { }, { BackendObjectReference: gwv1.BackendObjectReference{ - Name: "k8s-service3", + Name: serviceImportName, Kind: &serviceImportKind, }, Weight: aws.Int32(90), @@ -223,7 +256,7 @@ func Test_ListenerModelBuild(t *testing.T) { { SvcImportTG: &model.SvcImportTargetGroup{ K8SServiceNamespace: "default", - K8SServiceName: "k8s-service3", + K8SServiceName: string(serviceImportName), VpcId: "vpc-123", K8SClusterName: "eks-cluster", }, @@ -237,12 +270,18 @@ func Test_ListenerModelBuild(t *testing.T) { }, { name: "TLSRoute has more than one rule, build TLS_PASSTHROUGH listener failed", - gwListenerPort: *PortNumberPtr(443), wantErrIsNil: false, k8sGetGatewayCall: true, k8sGetServiceImportCall: false, - k8sGatewayReturnOK: true, - k8sGatewayListenerType: TLS_PASSTHROUGH, + gw: vpcLatticeGatewayWithListeners( + gwv1.Listener{ + Port: 443, + Protocol: "TLS", + Name: sectionName, + TLS: &gwv1.GatewayTLSConfig{ + Mode: &tlsModePassthrough, + }, + }), route: core.NewTLSRoute(gwv1alpha2.TLSRoute{ ObjectMeta: metav1.ObjectMeta{ Name: "service1", @@ -252,7 +291,7 @@ func Test_ListenerModelBuild(t *testing.T) { CommonRouteSpec: gwv1.CommonRouteSpec{ ParentRefs: []gwv1.ParentReference{ { - Name: "gw1", + Name: gwv1.ObjectName(vpcLatticeGateway.Name), SectionName: §ionName, }, }, @@ -312,7 +351,6 @@ func Test_ListenerModelBuild(t *testing.T) { }, { name: "no parentref", - gwListenerPort: *PortNumberPtr(80), wantErrIsNil: true, k8sGetGatewayCall: false, route: core.NewHTTPRoute(gwv1.HTTPRoute{ @@ -338,11 +376,9 @@ func Test_ListenerModelBuild(t *testing.T) { expectedSpec: []model.ListenerSpec{}, // empty list }, { - name: "No k8sgateway object", - gwListenerPort: *PortNumberPtr(80), - wantErrIsNil: false, - k8sGetGatewayCall: true, - k8sGatewayReturnOK: false, + name: "No k8sgateway object", + wantErrIsNil: false, + k8sGetGatewayCall: false, route: core.NewHTTPRoute(gwv1.HTTPRoute{ ObjectMeta: metav1.ObjectMeta{ Name: "service1", @@ -352,7 +388,7 @@ func Test_ListenerModelBuild(t *testing.T) { CommonRouteSpec: gwv1.CommonRouteSpec{ ParentRefs: []gwv1.ParentReference{ { - Name: "gw1", + Name: gwv1.ObjectName(vpcLatticeGateway.Name), SectionName: §ionName, }, }, @@ -370,11 +406,18 @@ func Test_ListenerModelBuild(t *testing.T) { }), }, { - name: "no section name", - gwListenerPort: *PortNumberPtr(80), - wantErrIsNil: false, - k8sGetGatewayCall: true, - k8sGatewayReturnOK: true, + name: "No gateway managed by vpc lattice", + wantErrIsNil: false, + k8sGetGatewayCall: true, + gw: gwv1.Gateway{ + ObjectMeta: metav1.ObjectMeta{ + Name: "non-lattice", + Namespace: "default", + }, + Spec: gwv1.GatewaySpec{ + GatewayClassName: gwv1.ObjectName("gwClass"), + }, + }, route: core.NewHTTPRoute(gwv1.HTTPRoute{ ObjectMeta: metav1.ObjectMeta{ Name: "service1", @@ -384,7 +427,43 @@ func Test_ListenerModelBuild(t *testing.T) { CommonRouteSpec: gwv1.CommonRouteSpec{ ParentRefs: []gwv1.ParentReference{ { - Name: "gw1", + Name: "non-lattice", + SectionName: §ionName, + }, + }, + }, + Rules: []gwv1.HTTPRouteRule{ + { + BackendRefs: []gwv1.HTTPBackendRef{ + { + BackendRef: backendRef, + }, + }, + }, + }, + }, + }), + }, + { + name: "no section name", + wantErrIsNil: false, + k8sGetGatewayCall: true, + gw: vpcLatticeGatewayWithListeners( + gwv1.Listener{ + Port: 80, + Protocol: "HTTP", + Name: sectionName, + }), + route: core.NewHTTPRoute(gwv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "service1", + Namespace: "default", + }, + Spec: gwv1.HTTPRouteSpec{ + CommonRouteSpec: gwv1.CommonRouteSpec{ + ParentRefs: []gwv1.ParentReference{ + { + Name: gwv1.ObjectName(vpcLatticeGateway.Name), SectionName: &missingSectionName, }, }, @@ -409,60 +488,33 @@ func Test_ListenerModelBuild(t *testing.T) { defer c.Finish() ctx := context.TODO() - mockK8sClient := mock_client.NewMockClient(c) - mockBrTgBuilder := NewMockBackendRefTargetGroupModelBuilder(c) - stack := core.NewDefaultStack(core.StackID(k8s.NamespacedName(tt.route.K8sObject()))) + k8sSchema := runtime.NewScheme() + clientgoscheme.AddToScheme(k8sSchema) + gwv1.Install(k8sSchema) + anv1alpha1.Install(k8sSchema) + k8sClient := testclient.NewClientBuilder().WithScheme(k8sSchema).Build() + assert.NoError(t, k8sClient.Create(ctx, vpcLatticeGatewayClass.DeepCopy())) if tt.k8sGetGatewayCall { - mockK8sClient.EXPECT().Get(ctx, gomock.Any(), gomock.AssignableToTypeOf(&gwv1.Gateway{})).DoAndReturn( - func(ctx context.Context, gwName types.NamespacedName, gw *gwv1.Gateway, arg3 ...interface{}) error { - if !tt.k8sGatewayReturnOK { - return errors.New("unknown k8s object") - } - var gwListener gwv1.Listener - switch tt.k8sGatewayListenerType { - case HTTP: - gwListener = gwv1.Listener{ - Port: tt.gwListenerPort, - Protocol: "HTTP", - Name: sectionName, - } - case HTTPS: - mode := gwv1.TLSModeTerminate - gwListener = gwv1.Listener{ - Port: tt.gwListenerPort, - Protocol: "HTTPS", - Name: sectionName, - TLS: &gwv1.GatewayTLSConfig{ - Mode: &mode, - }, - } - case TLS_PASSTHROUGH: - mode := gwv1.TLSModePassthrough - gwListener = gwv1.Listener{ - Port: tt.gwListenerPort, - Protocol: "TLS", - Name: sectionName, - TLS: &gwv1.GatewayTLSConfig{ - Mode: &mode, - }, - } - } - gw.Spec.Listeners = append(gw.Spec.Listeners, gwListener) - return nil - }, - ) + assert.NoError(t, k8sClient.Create(ctx, tt.gw.DeepCopy())) } + + mockBrTgBuilder := NewMockBackendRefTargetGroupModelBuilder(c) + stack := core.NewDefaultStack(core.StackID(k8s.NamespacedName(tt.route.K8sObject()))) + if tt.k8sGetServiceImportCall { - mockK8sClient.EXPECT().Get(ctx, gomock.Any(), gomock.AssignableToTypeOf(&anv1alpha1.ServiceImport{})).DoAndReturn( - func(ctx context.Context, svcName types.NamespacedName, svcImport *anv1alpha1.ServiceImport, arg3 ...interface{}) error { - svcImport.Annotations = make(map[string]string) - svcImport.Annotations["application-networking.k8s.aws/aws-vpc"] = "vpc-123" - svcImport.Annotations["application-networking.k8s.aws/aws-eks-cluster-name"] = "eks-cluster" - return nil + k8sClient.Create(ctx, &anv1alpha1.ServiceImport{ + ObjectMeta: metav1.ObjectMeta{ + Name: string(serviceImportName), + Namespace: "default", + Annotations: map[string]string{ + "application-networking.k8s.aws/aws-vpc": "vpc-123", + "application-networking.k8s.aws/aws-eks-cluster-name": "eks-cluster", + }, }, - ) + }) } + if tt.brTgBuilderBuildCall { gomock.InOrder( mockBrTgBuilder.EXPECT().Build(ctx, tt.route, gomock.Any(), gomock.Any()). @@ -478,7 +530,7 @@ func Test_ListenerModelBuild(t *testing.T) { task := &latticeServiceModelBuildTask{ log: gwlog.FallbackLogger, route: tt.route, - client: mockK8sClient, + client: k8sClient, stack: stack, brTgBuilder: mockBrTgBuilder, } diff --git a/pkg/gateway/model_build_targets_test.go b/pkg/gateway/model_build_targets_test.go index 8d021c44..56016555 100644 --- a/pkg/gateway/model_build_targets_test.go +++ b/pkg/gateway/model_build_targets_test.go @@ -24,6 +24,12 @@ import ( discoveryv1 "k8s.io/api/discovery/v1" ) +// PortNumberPtr translates an int to a *PortNumber +func PortNumberPtr(p int) *gwv1.PortNumber { + result := gwv1.PortNumber(p) + return &result +} + func Test_Targets(t *testing.T) { namespacePtr := func(ns string) *gwv1.Namespace { p := gwv1.Namespace(ns) diff --git a/test/suites/integration/access_log_policy_test.go b/test/suites/integration/access_log_policy_test.go index 39385504..3791abd1 100644 --- a/test/suites/integration/access_log_policy_test.go +++ b/test/suites/integration/access_log_policy_test.go @@ -86,7 +86,7 @@ var _ = Describe("Access Log Policy", Ordered, func() { ) BeforeAll(func() { - SetDefaultEventuallyTimeout(5 * time.Minute) + SetDefaultEventuallyTimeout(10 * time.Minute) SetDefaultEventuallyPollingInterval(10 * time.Second) awsResourceName = awsResourceNamePrefix + utils.RandomAlphaString(10) From d89f80ea37ac281fb9d11418f12eeac2b2e65c6c Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Mon, 10 Feb 2025 07:14:11 +0900 Subject: [PATCH 6/9] Update route_controller --- pkg/controllers/route_controller.go | 106 +++++++++++---------- pkg/controllers/route_controller_test.go | 23 +++++ pkg/gateway/model_build_lattice_service.go | 2 +- 3 files changed, 79 insertions(+), 52 deletions(-) diff --git a/pkg/controllers/route_controller.go b/pkg/controllers/route_controller.go index b23ceb04..cd4ec2ef 100644 --- a/pkg/controllers/route_controller.go +++ b/pkg/controllers/route_controller.go @@ -216,22 +216,10 @@ 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), - } - - if err := k8sClient.Get(ctx, gwName, gw); err != nil { - return fmt.Errorf("update route listener: gw not found, gw: %s, err: %w", gwName, err) + gw, err := findFirstParentGw(ctx, k8sClient, route) + if err != nil { + return fmt.Errorf("failed to get gateway for route %s: %w", route.Name(), err) } - return UpdateGWListenerStatus(ctx, k8sClient, gw) } @@ -240,42 +228,37 @@ func (r *routeReconciler) isRouteRelevant(ctx context.Context, route core.Route) r.log.Infof(ctx, "Ignore Route which has no ParentRefs gateway %s ", route.Name()) return false } + // if route has gateway parentRef that is managed by lattice gateway controller, + // then it is relevant + gw, err := findFirstParentGw(ctx, r.client, route) + return gw != nil && err == nil +} +func findFirstParentGw( + ctx context.Context, + client client.Client, + route core.Route, +) (*gwv1.Gateway, 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, - 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{ - Name: string(gw.Spec.GatewayClassName), - } - - 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 - } - - 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 + fails := []string{} + for _, parentRef := range route.Spec().ParentRefs() { + 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 { + continue + } + if k8s.IsManagedGateway(ctx, client, gw) { + return gw, nil + } + fails = append(fails, gwName.String()) } - - r.log.Infof(ctx, "Ignore non aws-vpc-lattice Route %s, %s", route.Name(), route.Namespace()) - return false + return nil, fmt.Errorf("failed to get gateway, name %s", fails) } func (r *routeReconciler) buildAndDeployModel( @@ -315,6 +298,19 @@ func (r *routeReconciler) buildAndDeployModel( return stack, err } +func (r *routeReconciler) findFirstParentRef(ctx context.Context, route core.Route) (gwv1.ParentReference, error) { + gw, err := findFirstParentGw(ctx, r.client, route) + if err != nil { + return gwv1.ParentReference{}, err + } + 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, @@ -336,9 +332,12 @@ func (r *routeReconciler) reconcileUpsert(ctx context.Context, req ctrl.Request, if backendRefIPFamiliesErr != nil { httpRouteOld := route.DeepCopy() + parentRef, err := r.findFirstParentRef(ctx, route) + if err != nil { + return err + } - route.Status().UpdateParentRefs(route.Spec().ParentRefs()[0], config.LatticeGatewayControllerName) - + route.Status().UpdateParentRefs(parentRef, config.LatticeGatewayControllerName) route.Status().UpdateRouteCondition(metav1.Condition{ Type: string(gwv1.RouteConditionAccepted), Status: metav1.ConditionFalse, @@ -357,7 +356,12 @@ 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) + parentRef, parentRefErr := r.findFirstParentRef(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(metav1.Condition{ Type: string(gwv1.RouteConditionAccepted), Status: metav1.ConditionFalse, @@ -553,7 +557,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{}, } diff --git a/pkg/controllers/route_controller_test.go b/pkg/controllers/route_controller_test.go index d00ed3b1..cf87eccf 100644 --- a/pkg/controllers/route_controller_test.go +++ b/pkg/controllers/route_controller_test.go @@ -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", }, diff --git a/pkg/gateway/model_build_lattice_service.go b/pkg/gateway/model_build_lattice_service.go index 3d3214f5..5d414a0b 100644 --- a/pkg/gateway/model_build_lattice_service.go +++ b/pkg/gateway/model_build_lattice_service.go @@ -186,7 +186,7 @@ func (t *latticeServiceModelBuildTask) getACMCertArn(ctx context.Context) (strin for _, parentRef := range t.route.Spec().ParentRefs() { if string(parentRef.Name) != gw.Name { - t.log.Debugf(ctx, "Ignore ParentRef of different gateway %s-%s", parentRef.Name, *parentRef.Namespace) + t.log.Debugf(ctx, "Ignore ParentRef of different gateway %s", parentRef.Name) continue } From 362a219a3a27c582b4a408373f239223d664797b Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Mon, 10 Feb 2025 08:10:37 +0900 Subject: [PATCH 7/9] Refactor --- pkg/controllers/route_controller.go | 81 +++++++++---------- pkg/gateway/model_build_lattice_service.go | 4 +- pkg/gateway/model_build_listener.go | 6 +- pkg/k8s/utils.go | 2 +- .../integration/access_log_policy_test.go | 7 +- 5 files changed, 46 insertions(+), 54 deletions(-) diff --git a/pkg/controllers/route_controller.go b/pkg/controllers/route_controller.go index cd4ec2ef..f576ed3b 100644 --- a/pkg/controllers/route_controller.go +++ b/pkg/controllers/route_controller.go @@ -216,11 +216,14 @@ func (r *routeReconciler) getRoute(ctx context.Context, req ctrl.Request) (core. } func updateRouteListenerStatus(ctx context.Context, k8sClient client.Client, route core.Route) error { - gw, err := findFirstParentGw(ctx, k8sClient, route) - if err != nil { + gws, err := findControlledParents(ctx, k8sClient, route) + if len(gws) <= 0 { return 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] return UpdateGWListenerStatus(ctx, k8sClient, gw) + } func (r *routeReconciler) isRouteRelevant(ctx context.Context, route core.Route) bool { @@ -228,21 +231,23 @@ func (r *routeReconciler) isRouteRelevant(ctx context.Context, route core.Route) r.log.Infof(ctx, "Ignore Route which has no ParentRefs gateway %s ", route.Name()) return false } - // if route has gateway parentRef that is managed by lattice gateway controller, + // if route has gateway parentRef that is controlled by lattice gateway controller, // then it is relevant - gw, err := findFirstParentGw(ctx, r.client, route) - return gw != nil && err == nil + gws, _ := findControlledParents(ctx, r.client, route) + return len(gws) > 0 } -func findFirstParentGw( +// 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) { - gw := &gwv1.Gateway{} +) ([]*gwv1.Gateway, error) { + var result []*gwv1.Gateway gwNamespace := route.Namespace() - fails := []string{} + misses := []string{} for _, parentRef := range route.Spec().ParentRefs() { + gw := &gwv1.Gateway{} if parentRef.Namespace != nil { gwNamespace = string(*parentRef.Namespace) } @@ -251,14 +256,18 @@ func findFirstParentGw( Name: string(parentRef.Name), } if err := client.Get(ctx, gwName, gw); err != nil { + misses = append(misses, gwName.String()) continue } - if k8s.IsManagedGateway(ctx, client, gw) { - return gw, nil + if k8s.IsControlledByLatticeGatewayController(ctx, client, gw) { + result = append(result, gw) } - fails = append(fails, gwName.String()) } - return nil, fmt.Errorf("failed to get gateway, name %s", fails) + var err error + if len(misses) > 0 { + err = fmt.Errorf("failed to get gateway, name %s", misses) + } + return result, err } func (r *routeReconciler) buildAndDeployModel( @@ -298,11 +307,13 @@ func (r *routeReconciler) buildAndDeployModel( return stack, err } -func (r *routeReconciler) findFirstParentRef(ctx context.Context, route core.Route) (gwv1.ParentReference, error) { - gw, err := findFirstParentGw(ctx, r.client, route) - if err != nil { - return gwv1.ParentReference{}, 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 @@ -332,7 +343,7 @@ func (r *routeReconciler) reconcileUpsert(ctx context.Context, req ctrl.Request, if backendRefIPFamiliesErr != nil { httpRouteOld := route.DeepCopy() - parentRef, err := r.findFirstParentRef(ctx, route) + parentRef, err := r.findControlledParentRef(ctx, route) if err != nil { return err } @@ -356,7 +367,7 @@ 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 - parentRef, parentRefErr := r.findFirstParentRef(ctx, route) + parentRef, parentRefErr := r.findControlledParentRef(ctx, route) if parentRefErr != nil { // if parentRef not found, we cannot update route status return parentRefErr @@ -502,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: @@ -535,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 { diff --git a/pkg/gateway/model_build_lattice_service.go b/pkg/gateway/model_build_lattice_service.go index 5d414a0b..14a7d38c 100644 --- a/pkg/gateway/model_build_lattice_service.go +++ b/pkg/gateway/model_build_lattice_service.go @@ -134,7 +134,7 @@ func (t *latticeServiceModelBuildTask) buildLatticeService(ctx context.Context) t.log.Infof(ctx, "Ignoring route %s because failed to get gateway %s: %v", t.route.Name(), gw.Spec.GatewayClassName, err) continue } - if k8s.IsManagedGateway(ctx, t.client, gw) { + 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) @@ -176,7 +176,7 @@ func (t *latticeServiceModelBuildTask) buildLatticeService(ctx context.Context) func (t *latticeServiceModelBuildTask) getACMCertArn(ctx context.Context) (string, error) { // 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.getFirstGateway(ctx) + 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 diff --git a/pkg/gateway/model_build_listener.go b/pkg/gateway/model_build_listener.go index c7d9d3f1..16c9cf4a 100644 --- a/pkg/gateway/model_build_listener.go +++ b/pkg/gateway/model_build_listener.go @@ -56,7 +56,7 @@ func isTLSPassthroughGatewayListener(listener *gwv1.Listener) bool { return listener.Protocol == gwv1.TLSProtocolType && listener.TLS != nil && listener.TLS.Mode != nil && *listener.TLS.Mode == gwv1.TLSModePassthrough } -func (t *latticeServiceModelBuildTask) getFirstGateway(ctx context.Context) (*gwv1.Gateway, error) { +func (t *latticeServiceModelBuildTask) findGateway(ctx context.Context) (*gwv1.Gateway, error) { gw := &gwv1.Gateway{} gwNamespace := t.route.Namespace() fails := []string{} @@ -72,7 +72,7 @@ func (t *latticeServiceModelBuildTask) getFirstGateway(ctx context.Context) (*gw t.log.Infof(ctx, "Ignoring route %s because failed to get gateway %s: %v", t.route.Name(), parentRef.Name, err) continue } - if k8s.IsManagedGateway(ctx, t.client, gw) { + if k8s.IsControlledByLatticeGatewayController(ctx, t.client, gw) { return gw, nil } fails = append(fails, gwName.String()) @@ -93,7 +93,7 @@ func (t *latticeServiceModelBuildTask) buildListeners(ctx context.Context, stack // 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.getFirstGateway(ctx) + gw, err := t.findGateway(ctx) if err != nil { return err } diff --git a/pkg/k8s/utils.go b/pkg/k8s/utils.go index df987ed0..7df4ec0f 100644 --- a/pkg/k8s/utils.go +++ b/pkg/k8s/utils.go @@ -57,7 +57,7 @@ func IsGVKSupported(mgr ctrl.Manager, groupVersion string, kind string) (bool, e } // validate if the gateway is managed by the lattice gateway controller -func IsManagedGateway(ctx context.Context, c client.Client, gw *gwv1.Gateway) bool { +func IsControlledByLatticeGatewayController(ctx context.Context, c client.Client, gw *gwv1.Gateway) bool { gwClass := &gwv1.GatewayClass{} // GatewayClass is cluster-scoped resource, so we don't need to specify namespace err := c.Get(ctx, client.ObjectKey{Name: string(gw.Spec.GatewayClassName)}, gwClass) diff --git a/test/suites/integration/access_log_policy_test.go b/test/suites/integration/access_log_policy_test.go index 3791abd1..24a98c98 100644 --- a/test/suites/integration/access_log_policy_test.go +++ b/test/suites/integration/access_log_policy_test.go @@ -2,12 +2,13 @@ package integration import ( "fmt" + "strings" + "time" + anaws "github.com/aws/aws-application-networking-k8s/pkg/aws" "github.com/aws/aws-application-networking-k8s/pkg/utils" arn2 "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/service/resourcegroupstaggingapi" - "strings" - "time" "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/session" @@ -86,7 +87,7 @@ var _ = Describe("Access Log Policy", Ordered, func() { ) BeforeAll(func() { - SetDefaultEventuallyTimeout(10 * time.Minute) + SetDefaultEventuallyTimeout(5 * time.Minute) SetDefaultEventuallyPollingInterval(10 * time.Second) awsResourceName = awsResourceNamePrefix + utils.RandomAlphaString(10) From 5e1bcbb3c6177e739eb3f92b0ec47988f8c05e8a Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Tue, 11 Feb 2025 09:23:24 +0900 Subject: [PATCH 8/9] Fix FindTargetGroup for e2e test --- test/pkg/test/framework.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/test/pkg/test/framework.go b/test/pkg/test/framework.go index 5086ab06..fafe4bbb 100644 --- a/test/pkg/test/framework.go +++ b/test/pkg/test/framework.go @@ -389,6 +389,7 @@ func (env *Framework) FindTargetGroupFromSpec(ctx context.Context, tgSpec model. if err != nil { return nil, err } + var candidate *vpclattice.TargetGroupSummary for _, targetGroup := range targetGroups { if aws.StringValue(targetGroup.Protocol) != tgSpec.Protocol { @@ -416,10 +417,13 @@ func (env *Framework) FindTargetGroupFromSpec(ctx context.Context, tgSpec model. continue } - // close enough :D - return targetGroup, nil + // Select the most recently created target group. + // This handles cases where previous test cleanups were incomplete. + if candidate == nil || candidate.CreatedAt.Before(*targetGroup.CreatedAt) { + candidate = targetGroup + } } - return nil, nil + return candidate, nil } // TODO: Create a new function that only verifying deployment len(podList.Items)==*deployment.Spec.Replicas, and don't do lattice.ListTargets() api call From b97fdcf262675bdbc4a9b0fab3dc0d08daf83286 Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Tue, 11 Feb 2025 10:25:26 +0900 Subject: [PATCH 9/9] Update XXXRoute update logic --- pkg/controllers/route_controller.go | 4 ++-- pkg/model/core/grpcroute.go | 24 +++++++++++++++++------- pkg/model/core/httproute.go | 24 +++++++++++++++++------- pkg/model/core/route.go | 2 +- pkg/model/core/tlsroute.go | 24 +++++++++++++++++------- 5 files changed, 54 insertions(+), 24 deletions(-) diff --git a/pkg/controllers/route_controller.go b/pkg/controllers/route_controller.go index f576ed3b..199f48f0 100644 --- a/pkg/controllers/route_controller.go +++ b/pkg/controllers/route_controller.go @@ -349,7 +349,7 @@ func (r *routeReconciler) reconcileUpsert(ctx context.Context, req ctrl.Request, } route.Status().UpdateParentRefs(parentRef, config.LatticeGatewayControllerName) - route.Status().UpdateRouteCondition(metav1.Condition{ + route.Status().UpdateRouteCondition(parentRef, metav1.Condition{ Type: string(gwv1.RouteConditionAccepted), Status: metav1.ConditionFalse, ObservedGeneration: route.K8sObject().GetGeneration(), @@ -373,7 +373,7 @@ func (r *routeReconciler) reconcileUpsert(ctx context.Context, req ctrl.Request, return parentRefErr } route.Status().UpdateParentRefs(parentRef, config.LatticeGatewayControllerName) - route.Status().UpdateRouteCondition(metav1.Condition{ + route.Status().UpdateRouteCondition(parentRef, metav1.Condition{ Type: string(gwv1.RouteConditionAccepted), Status: metav1.ConditionFalse, ObservedGeneration: route.K8sObject().GetGeneration(), diff --git a/pkg/model/core/grpcroute.go b/pkg/model/core/grpcroute.go index 6f563d08..4933a389 100644 --- a/pkg/model/core/grpcroute.go +++ b/pkg/model/core/grpcroute.go @@ -146,16 +146,26 @@ func (s *GRPCRouteStatus) SetParents(parents []gwv1.RouteParentStatus) { } func (s *GRPCRouteStatus) UpdateParentRefs(parent gwv1.ParentReference, controllerName gwv1.GatewayController) { - if len(s.Parents()) == 0 { - s.SetParents(make([]gwv1.RouteParentStatus, 1)) + for i, p := range s.Parents() { + if p.ParentRef.Name == parent.Name { + s.Parents()[i].ParentRef = parent + s.Parents()[i].ControllerName = controllerName + return + } } - - s.Parents()[0].ParentRef = parent - s.Parents()[0].ControllerName = controllerName + s.SetParents(append(s.Parents(), gwv1.RouteParentStatus{ + ParentRef: parent, + ControllerName: controllerName, + })) } -func (s *GRPCRouteStatus) UpdateRouteCondition(condition metav1.Condition) { - s.Parents()[0].Conditions = utils.GetNewConditions(s.Parents()[0].Conditions, condition) +func (s *GRPCRouteStatus) UpdateRouteCondition(parent gwv1.ParentReference, condition metav1.Condition) { + for i, p := range s.Parents() { + if p.ParentRef.Name == parent.Name { + s.Parents()[i].Conditions = utils.GetNewConditions(p.Conditions, condition) + return + } + } } type GRPCRouteRule struct { diff --git a/pkg/model/core/httproute.go b/pkg/model/core/httproute.go index 775a06e7..f3161f80 100644 --- a/pkg/model/core/httproute.go +++ b/pkg/model/core/httproute.go @@ -146,16 +146,26 @@ func (s *HTTPRouteStatus) SetParents(parents []gwv1.RouteParentStatus) { } func (s *HTTPRouteStatus) UpdateParentRefs(parent gwv1.ParentReference, controllerName gwv1.GatewayController) { - if len(s.Parents()) == 0 { - s.SetParents(make([]gwv1.RouteParentStatus, 1)) + for i, p := range s.Parents() { + if p.ParentRef.Name == parent.Name { + s.Parents()[i].ParentRef = parent + s.Parents()[i].ControllerName = controllerName + return + } } - - s.Parents()[0].ParentRef = parent - s.Parents()[0].ControllerName = controllerName + s.SetParents(append(s.Parents(), gwv1.RouteParentStatus{ + ParentRef: parent, + ControllerName: controllerName, + })) } -func (s *HTTPRouteStatus) UpdateRouteCondition(condition metav1.Condition) { - s.Parents()[0].Conditions = utils.GetNewConditions(s.Parents()[0].Conditions, condition) +func (s *HTTPRouteStatus) UpdateRouteCondition(parent gwv1.ParentReference, condition metav1.Condition) { + for i, p := range s.Parents() { + if p.ParentRef.Name == parent.Name { + s.Parents()[i].Conditions = utils.GetNewConditions(p.Conditions, condition) + return + } + } } type HTTPRouteRule struct { diff --git a/pkg/model/core/route.go b/pkg/model/core/route.go index 462422d5..12c3ff46 100644 --- a/pkg/model/core/route.go +++ b/pkg/model/core/route.go @@ -69,7 +69,7 @@ type RouteStatus interface { Parents() []gwv1.RouteParentStatus SetParents(parents []gwv1.RouteParentStatus) UpdateParentRefs(parent gwv1.ParentReference, controllerName gwv1.GatewayController) - UpdateRouteCondition(condition metav1.Condition) + UpdateRouteCondition(parent gwv1.ParentReference, condition metav1.Condition) } type RouteRule interface { diff --git a/pkg/model/core/tlsroute.go b/pkg/model/core/tlsroute.go index 0c984ed2..45628637 100644 --- a/pkg/model/core/tlsroute.go +++ b/pkg/model/core/tlsroute.go @@ -148,16 +148,26 @@ func (s *TLSRouteStatus) SetParents(parents []gwv1.RouteParentStatus) { } func (s *TLSRouteStatus) UpdateParentRefs(parent gwv1.ParentReference, controllerName gwv1.GatewayController) { - if len(s.Parents()) == 0 { - s.SetParents(make([]gwv1.RouteParentStatus, 1)) + for i, p := range s.Parents() { + if p.ParentRef.Name == parent.Name { + s.Parents()[i].ParentRef = parent + s.Parents()[i].ControllerName = controllerName + return + } } - - s.Parents()[0].ParentRef = parent - s.Parents()[0].ControllerName = controllerName + s.SetParents(append(s.Parents(), gwv1.RouteParentStatus{ + ParentRef: parent, + ControllerName: controllerName, + })) } -func (s *TLSRouteStatus) UpdateRouteCondition(condition metav1.Condition) { - s.Parents()[0].Conditions = utils.GetNewConditions(s.Parents()[0].Conditions, condition) +func (s *TLSRouteStatus) UpdateRouteCondition(parent gwv1.ParentReference, condition metav1.Condition) { + for i, p := range s.Parents() { + if p.ParentRef.Name == parent.Name { + s.Parents()[i].Conditions = utils.GetNewConditions(p.Conditions, condition) + return + } + } } type TLSRouteRule struct {