From b69b33d1a120f4292c0a0bc9928d57de475b797c Mon Sep 17 00:00:00 2001 From: Doyoon Kim Date: Thu, 16 Nov 2023 16:11:33 -0800 Subject: [PATCH 1/2] Fix e2e tests and examples to be compatible w/ latest CRDs --- examples/my-hotel-gateway-tls.yaml | 7 ++-- examples/parking-route-path.yaml | 1 + examples/second-account-gw1-full-setup.yaml | 1 + examples/tls-route-with-own-cert.yaml | 2 +- pkg/aws/services/tagging.go | 1 + test/pkg/test/framework.go | 41 ++++++++++----------- test/pkg/test/gateway.go | 9 +++++ test/pkg/test/header_match_httproute.go | 1 + test/pkg/test/method_match_httproute.go | 2 + test/pkg/test/path_match_httproute.go | 7 ++++ test/pkg/test/weighted_routing_httproute.go | 7 ++++ test/suites/integration/byoc_test.go | 11 +++++- 12 files changed, 62 insertions(+), 28 deletions(-) diff --git a/examples/my-hotel-gateway-tls.yaml b/examples/my-hotel-gateway-tls.yaml index d46bcd2d..2e830f48 100644 --- a/examples/my-hotel-gateway-tls.yaml +++ b/examples/my-hotel-gateway-tls.yaml @@ -11,10 +11,9 @@ spec: - name: https protocol: HTTPS port: 443 - - name: tls-with-customer-cert - protocol: HTTPS - port: 443 tls: mode: Terminate + certificateRefs: + - name: unused options: - application-networking.k8s.aws/certificate-arn: arn:aws:acm:us-west-2::certificate/4555204d-07e1-43f0-a533-d02750f41545 + application-networking.k8s.aws/certificate-arn: "" # arn:aws:acm:us-west-2::certificate/ diff --git a/examples/parking-route-path.yaml b/examples/parking-route-path.yaml index 2e34e1ba..6fec3783 100644 --- a/examples/parking-route-path.yaml +++ b/examples/parking-route-path.yaml @@ -10,6 +10,7 @@ spec: - backendRefs: - name: parking-ver1 kind: Service + port: 80 matches: - path: type: PathPrefix diff --git a/examples/second-account-gw1-full-setup.yaml b/examples/second-account-gw1-full-setup.yaml index 66411f1c..e5b5c3dd 100644 --- a/examples/second-account-gw1-full-setup.yaml +++ b/examples/second-account-gw1-full-setup.yaml @@ -62,6 +62,7 @@ spec: - backendRefs: - name: second-account-gw1-svc kind: Service + port: 80 matches: - path: type: PathPrefix diff --git a/examples/tls-route-with-own-cert.yaml b/examples/tls-route-with-own-cert.yaml index 90154236..abb3db7e 100644 --- a/examples/tls-route-with-own-cert.yaml +++ b/examples/tls-route-with-own-cert.yaml @@ -7,7 +7,7 @@ spec: - tls-parking.my-test.com parentRefs: - name: my-hotel - sectionName: tls-with-customer-cert + sectionName: https rules: - backendRefs: - name: parking-ver3 diff --git a/pkg/aws/services/tagging.go b/pkg/aws/services/tagging.go index cb52498b..a3266a46 100644 --- a/pkg/aws/services/tagging.go +++ b/pkg/aws/services/tagging.go @@ -17,6 +17,7 @@ const ( resourceTypePrefix = "vpc-lattice:" ResourceTypeTargetGroup ResourceType = resourceTypePrefix + "targetgroup" + ResourceTypeService ResourceType = resourceTypePrefix + "service" // https://docs.aws.amazon.com/resourcegroupstagging/latest/APIReference/API_GetResources.html#API_GetResources_RequestSyntax maxArnsPerGetResourcesApi = 100 diff --git a/test/pkg/test/framework.go b/test/pkg/test/framework.go index 586ae030..ba4bf739 100644 --- a/test/pkg/test/framework.go +++ b/test/pkg/test/framework.go @@ -107,6 +107,7 @@ type Framework struct { controllerRuntimeConfig *rest.Config Log gwlog.Logger LatticeClient services.Lattice + TaggingClient services.Tagging Ec2Client *ec2.EC2 GrpcurlRunner *corev1.Pod DefaultTags services.Tags @@ -123,10 +124,12 @@ func NewFramework(ctx context.Context, log gwlog.Logger, testNamespace string) * Region: config.Region, ClusterName: config.ClusterName, } + sess := session.Must(session.NewSession()) framework := &Framework{ Client: lo.Must(client.New(controllerRuntimeConfig, client.Options{Scheme: testScheme})), - LatticeClient: services.NewDefaultLattice(session.Must(session.NewSession()), config.Region), // region is currently hardcoded - Ec2Client: ec2.New(session.Must(session.NewSession(&aws.Config{Region: aws.String(config.Region)}))), + LatticeClient: services.NewDefaultLattice(sess, config.Region), + TaggingClient: services.NewDefaultTagging(sess, config.Region), + Ec2Client: ec2.New(sess, &aws.Config{Region: aws.String(config.Region)}), GrpcurlRunner: &corev1.Pod{}, ctx: ctx, Log: log, @@ -150,25 +153,17 @@ func (env *Framework) ExpectToBeClean(ctx context.Context) { }) Eventually(func(g Gomega) { - retrievedServices, _ := env.LatticeClient.ListServicesAsList(ctx, &vpclattice.ListServicesInput{}) - for _, service := range retrievedServices { - env.Log.Infof("Found service, checking if created by current EKS Cluster: %v", service) - managed, err := env.Cloud.IsArnManaged(ctx, *service.Arn) - if err == nil { // ignore error as they can be a shared resource. - g.Expect(managed).To(BeFalse()) - } - } + arns, err := env.TaggingClient.FindResourcesByTags(ctx, services.ResourceTypeService, env.DefaultTags) + env.Log.Infow("Expecting no services created by the controller", "found", arns) + g.Expect(err).To(BeNil()) + g.Expect(arns).To(BeEmpty()) + }).Should(Succeed()) - retrievedTargetGroups, _ := env.LatticeClient.ListTargetGroupsAsList(ctx, &vpclattice.ListTargetGroupsInput{ - VpcIdentifier: &config.VpcID, - }) - for _, tg := range retrievedTargetGroups { - env.Log.Infof("Found TargetGroup: %s, checking if created by current EKS Cluster", *tg.Id) - managed, err := env.Cloud.IsArnManaged(ctx, *tg.Arn) - if err == nil { // ignore error as they can be a shared resource. - g.Expect(managed).To(BeFalse()) - } - } + Eventually(func(g Gomega) { + arns, err := env.TaggingClient.FindResourcesByTags(ctx, services.ResourceTypeTargetGroup, env.DefaultTags) + env.Log.Infow("Expecting no target groups created by the controller", "found", arns) + g.Expect(err).To(BeNil()) + g.Expect(arns).To(BeEmpty()) }).Should(Succeed()) } @@ -185,7 +180,11 @@ func objectsInfo(objs []client.Object) string { func (env *Framework) ExpectCreated(ctx context.Context, objects ...client.Object) { env.Log.Infof("Creating objects: %s", objectsInfo(objects)) parallel.ForEach(objects, func(obj client.Object, _ int) { - Expect(env.Create(ctx, obj)).WithOffset(1).To(Succeed()) + err := env.Create(ctx, obj) + if err != nil { + env.Log.Infof("%s", err.Error()) + } + Expect(err).To(BeNil()) }) } diff --git a/test/pkg/test/gateway.go b/test/pkg/test/gateway.go index 05712a9e..14f996d2 100644 --- a/test/pkg/test/gateway.go +++ b/test/pkg/test/gateway.go @@ -1,6 +1,7 @@ package test import ( + "github.com/samber/lo" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" gwv1 "sigs.k8s.io/gateway-api/apis/v1" ) @@ -27,6 +28,14 @@ func (env *Framework) NewGateway(name string, namespace string) *gwv1.Gateway { Name: "https", Protocol: gwv1.HTTPSProtocolType, Port: 443, + TLS: &gwv1.GatewayTLSConfig{ + Mode: lo.ToPtr(gwv1.TLSModeTerminate), + CertificateRefs: []gwv1.SecretObjectReference{ + { + Name: "dummy", + }, + }, + }, }, }, }, diff --git a/test/pkg/test/header_match_httproute.go b/test/pkg/test/header_match_httproute.go index 29e30bf0..10126097 100644 --- a/test/pkg/test/header_match_httproute.go +++ b/test/pkg/test/header_match_httproute.go @@ -16,6 +16,7 @@ func (env *Framework) NewHeaderMatchHttpRoute(parentRefsGateway *gwv1.Gateway, s BackendObjectReference: gwv1.BackendObjectReference{ Name: gwv1.ObjectName(service.Name), Kind: lo.ToPtr(gwv1.Kind("Service")), + Port: (*gwv1.PortNumber)(&service.Spec.Ports[0].Port), }, }, }}, diff --git a/test/pkg/test/method_match_httproute.go b/test/pkg/test/method_match_httproute.go index a929febd..93b8bd4f 100644 --- a/test/pkg/test/method_match_httproute.go +++ b/test/pkg/test/method_match_httproute.go @@ -16,6 +16,7 @@ func (env *Framework) NewMethodMatchHttpRoute(parentRefsGateway *gwv1.Gateway, g BackendObjectReference: gwv1.BackendObjectReference{ Name: gwv1.ObjectName(getService.Name), Kind: lo.ToPtr(gwv1.Kind("Service")), + Port: (*gwv1.PortNumber)(&postService.Spec.Ports[0].Port), }, }, }}, @@ -32,6 +33,7 @@ func (env *Framework) NewMethodMatchHttpRoute(parentRefsGateway *gwv1.Gateway, g BackendObjectReference: gwv1.BackendObjectReference{ Name: gwv1.ObjectName(postService.Name), Kind: lo.ToPtr(gwv1.Kind("Service")), + Port: (*gwv1.PortNumber)(&postService.Spec.Ports[0].Port), }, }, }}, diff --git a/test/pkg/test/path_match_httproute.go b/test/pkg/test/path_match_httproute.go index c6fc3cf3..702b07c9 100644 --- a/test/pkg/test/path_match_httproute.go +++ b/test/pkg/test/path_match_httproute.go @@ -4,6 +4,7 @@ import ( "strconv" "github.com/samber/lo" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" gwv1 "sigs.k8s.io/gateway-api/apis/v1" @@ -20,6 +21,11 @@ func (env *Framework) NewPathMatchHttpRoute(parentRefsGateway *gwv1.Gateway, bac httpns = &namespace } for i, object := range backendRefObjects { + var port *gwv1.PortNumber + if svc, ok := object.(*corev1.Service); ok { + pv := gwv1.PortNumber(svc.Spec.Ports[0].Port) + port = &pv + } rule := gwv1.HTTPRouteRule{ BackendRefs: []gwv1.HTTPBackendRef{{ BackendRef: gwv1.BackendRef{ @@ -27,6 +33,7 @@ func (env *Framework) NewPathMatchHttpRoute(parentRefsGateway *gwv1.Gateway, bac Name: gwv1.ObjectName(object.GetName()), Namespace: (*gwv1.Namespace)(httpns), Kind: lo.ToPtr(gwv1.Kind(object.GetObjectKind().GroupVersionKind().Kind)), + Port: port, }, }, }}, diff --git a/test/pkg/test/weighted_routing_httproute.go b/test/pkg/test/weighted_routing_httproute.go index a05d1ca3..6b6ff8c5 100644 --- a/test/pkg/test/weighted_routing_httproute.go +++ b/test/pkg/test/weighted_routing_httproute.go @@ -2,6 +2,7 @@ package test import ( "github.com/samber/lo" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" gwv1 "sigs.k8s.io/gateway-api/apis/v1" @@ -17,11 +18,17 @@ func (env *Framework) NewWeightedRoutingHttpRoute(parentRefsGateway *gwv1.Gatewa var backendRefs []gwv1.HTTPBackendRef for _, objectAndWeight := range backendRefObjectAndWeights { + var port *gwv1.PortNumber + if svc, ok := objectAndWeight.Object.(*corev1.Service); ok { + pv := gwv1.PortNumber(svc.Spec.Ports[0].Port) + port = &pv + } backendRefs = append(backendRefs, gwv1.HTTPBackendRef{ BackendRef: gwv1.BackendRef{ BackendObjectReference: gwv1.BackendObjectReference{ Name: gwv1.ObjectName(objectAndWeight.Object.GetName()), Kind: lo.ToPtr(gwv1.Kind(objectAndWeight.Object.GetObjectKind().GroupVersionKind().Kind)), + Port: port, }, Weight: lo.ToPtr(objectAndWeight.Weight), }, diff --git a/test/suites/integration/byoc_test.go b/test/suites/integration/byoc_test.go index a3b54450..0a4d0fcd 100644 --- a/test/suites/integration/byoc_test.go +++ b/test/suites/integration/byoc_test.go @@ -21,6 +21,7 @@ import ( "github.com/aws/aws-sdk-go/service/route53" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/samber/lo" "golang.org/x/exp/slices" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -61,7 +62,7 @@ var _ = Describe("Bring your own certificate (BYOC)", Ordered, func() { log.Infof("created certificate: %s", certArn) // add new certificate to gateway spec - addGatewayBYOCListener(certArn) + addGatewayBYOCListener(cname, certArn) log.Infof("added listener with cert to gateway") // create and deploy service for traffic test @@ -234,7 +235,7 @@ func deleteCert(client *acm.ACM, arn string) error { return err } -func addGatewayBYOCListener(certArn string) { +func addGatewayBYOCListener(cname, certArn string) { gw := &gwv1.Gateway{} testFramework.Get(context.TODO(), types.NamespacedName{ Namespace: testGateway.Namespace, @@ -244,12 +245,18 @@ func addGatewayBYOCListener(certArn string) { byocListener := gwv1.Listener{ Name: "byoc", Port: 443, + Hostname: lo.ToPtr(gwv1.Hostname(cname)), Protocol: gwv1.HTTPSProtocolType, TLS: &gwv1.GatewayTLSConfig{ Mode: &tlsMode, Options: map[gwv1.AnnotationKey]gwv1.AnnotationValue{ "application-networking.k8s.aws/certificate-arn": gwv1.AnnotationValue(certArn), }, + CertificateRefs: []gwv1.SecretObjectReference{ + { + Name: "dummy", + }, + }, }, } gw.Spec.Listeners = append(gw.Spec.Listeners, byocListener) From d59ec6481793f0bbe7491d8f9e158f972702162f Mon Sep 17 00:00:00 2001 From: Doyoon Kim Date: Thu, 16 Nov 2023 17:21:43 -0800 Subject: [PATCH 2/2] Fix empty rule matching behavior --- pkg/gateway/model_build_rule.go | 7 +++++++ pkg/gateway/model_build_rule_test.go | 13 +++++++++++++ test/pkg/test/framework.go | 6 +----- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/pkg/gateway/model_build_rule.go b/pkg/gateway/model_build_rule.go index 82de6448..e09766cd 100644 --- a/pkg/gateway/model_build_rule.go +++ b/pkg/gateway/model_build_rule.go @@ -61,6 +61,13 @@ func (t *latticeServiceModelBuildTask) buildRules(ctx context.Context, stackList if err := t.updateRuleSpecWithHeaderMatches(match, &ruleSpec); err != nil { return err } + } else { + // Match every traffic on no matches + ruleSpec.PathMatchValue = "/" + ruleSpec.PathMatchPrefix = true + if _, ok := rule.(*core.GRPCRouteRule); ok { + ruleSpec.Method = string(gwv1.HTTPMethodPost) + } } ruleTgList, err := t.getTargetGroupsForRuleAction(ctx, rule) diff --git a/pkg/gateway/model_build_rule_test.go b/pkg/gateway/model_build_rule_test.go index 5f25a188..041c8e74 100644 --- a/pkg/gateway/model_build_rule_test.go +++ b/pkg/gateway/model_build_rule_test.go @@ -153,6 +153,8 @@ func Test_RuleModelBuild(t *testing.T) { expectedSpec: []model.RuleSpec{ // note priority is only calculated at synthesis b/c it requires access to existing rules { StackListenerId: "listener-id", + PathMatchPrefix: true, + PathMatchValue: "/", Action: model.RuleAction{ TargetGroups: []*model.RuleTargetGroup{ { @@ -195,6 +197,8 @@ func Test_RuleModelBuild(t *testing.T) { expectedSpec: []model.RuleSpec{ { StackListenerId: "listener-id", + PathMatchPrefix: true, + PathMatchValue: "/", Action: model.RuleAction{ TargetGroups: []*model.RuleTargetGroup{ { @@ -243,6 +247,8 @@ func Test_RuleModelBuild(t *testing.T) { expectedSpec: []model.RuleSpec{ { StackListenerId: "listener-id", + PathMatchPrefix: true, + PathMatchValue: "/", Action: model.RuleAction{ TargetGroups: []*model.RuleTargetGroup{ { @@ -564,6 +570,9 @@ func Test_RuleModelBuild(t *testing.T) { expectedSpec: []model.RuleSpec{ { StackListenerId: "listener-id", + Method: string(httpPost), + PathMatchPrefix: true, + PathMatchValue: "/", Action: model.RuleAction{ TargetGroups: []*model.RuleTargetGroup{ { @@ -1407,6 +1416,8 @@ func Test_RuleModelBuild(t *testing.T) { expectedSpec: []model.RuleSpec{ { StackListenerId: "listener-id", + PathMatchPrefix: true, + PathMatchValue: "/", Action: model.RuleAction{ TargetGroups: []*model.RuleTargetGroup{ { @@ -1453,6 +1464,8 @@ func Test_RuleModelBuild(t *testing.T) { expectedSpec: []model.RuleSpec{ { StackListenerId: "listener-id", + PathMatchPrefix: true, + PathMatchValue: "/", Action: model.RuleAction{ TargetGroups: []*model.RuleTargetGroup{ { diff --git a/test/pkg/test/framework.go b/test/pkg/test/framework.go index ba4bf739..04f1ba5a 100644 --- a/test/pkg/test/framework.go +++ b/test/pkg/test/framework.go @@ -180,11 +180,7 @@ func objectsInfo(objs []client.Object) string { func (env *Framework) ExpectCreated(ctx context.Context, objects ...client.Object) { env.Log.Infof("Creating objects: %s", objectsInfo(objects)) parallel.ForEach(objects, func(obj client.Object, _ int) { - err := env.Create(ctx, obj) - if err != nil { - env.Log.Infof("%s", err.Error()) - } - Expect(err).To(BeNil()) + Expect(env.Create(ctx, obj)).WithOffset(1).To(Succeed()) }) }