From 7fc2b068dbe34b984caca2ebd84130d34541ce11 Mon Sep 17 00:00:00 2001 From: Ryan Lymburner Date: Tue, 13 May 2025 10:51:52 -0700 Subject: [PATCH 1/4] Added unit tests --- pkg/gateway/model_build_rule.go | 59 ++++++-- pkg/gateway/model_build_rule_test.go | 219 ++++++++++++++++++++++++++- pkg/utils/priority_queue.go | 51 +++++++ 3 files changed, 314 insertions(+), 15 deletions(-) create mode 100644 pkg/utils/priority_queue.go diff --git a/pkg/gateway/model_build_rule.go b/pkg/gateway/model_build_rule.go index bfd1a81e..aad0e89c 100644 --- a/pkg/gateway/model_build_rule.go +++ b/pkg/gateway/model_build_rule.go @@ -1,15 +1,18 @@ package gateway import ( + "container/heap" "context" "errors" "fmt" + "strconv" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/types" anv1alpha1 "github.com/aws/aws-application-networking-k8s/pkg/apis/applicationnetworking/v1alpha1" "github.com/aws/aws-application-networking-k8s/pkg/model/core" + "github.com/aws/aws-application-networking-k8s/pkg/utils" "github.com/aws/aws-sdk-go/aws" @@ -32,10 +35,33 @@ func (t *latticeServiceModelBuildTask) buildRules(ctx context.Context, stackList // note we only build rules for non-deleted routes t.log.Debugf(ctx, "Processing %d rules", len(t.route.Spec().Rules())) + // Skip if route is being deleted + if !t.route.DeletionTimestamp().IsZero() { + t.log.Debugf(ctx, "Skipping rule creation since the route is deleted") + return nil + } + + // Create a priority queue to organize rules + pq := make(utils.PriorityQueue, 0, len(t.route.Spec().Rules())) + + // First pass: build all rules and add them to priority queue for i, rule := range t.route.Spec().Rules() { + // Default priority is index + 1 + priority := int64(i + 1) + + // Check for priority annotation in format: application-networking.k8s.aws/rule-{index}-priority + if priorityStr, ok := t.route.K8sObject().GetAnnotations()[fmt.Sprintf("application-networking.k8s.aws/rule-%d-priority", i)]; ok { + if p, err := strconv.ParseInt(priorityStr, 10, 64); err == nil { + priority = p + t.log.Debugf(ctx, "Using priority %d from annotation for rule %d", priority, i) + } else { + t.log.Warnf(ctx, "Invalid priority value in annotation for rule %d: %s", i, priorityStr) + } + } + ruleSpec := model.RuleSpec{ StackListenerId: stackListenerId, - Priority: int64(i + 1), + Priority: priority, } if len(rule.Matches()) > 1 { @@ -81,17 +107,28 @@ func (t *latticeServiceModelBuildTask) buildRules(ctx context.Context, stackList TargetGroups: ruleTgList, } - // don't bother adding rules on delete, these will be removed automatically with the owning route/lattice service - // target groups will still be present and removed as needed - if t.route.DeletionTimestamp().IsZero() { - stackRule, err := model.NewRule(t.stack, ruleSpec) - if err != nil { - return err - } - t.log.Debugf(ctx, "Added rule %d to the stack (ID %s)", stackRule.Spec.Priority, stackRule.ID()) - } else { - t.log.Debugf(ctx, "Skipping adding rule %d to the stack since the route is deleted", ruleSpec.Priority) + // Create the rule but don't add to stack yet + stackRule, err := model.NewRule(t.stack, ruleSpec) + if err != nil { + return err } + + // Add to priority queue + pq.Push(&utils.Item{ + Value: stackRule, + Priority: int32(priority), + }) + } + + // Initialize heap + heap.Init(&pq) + + // Add rules to stack in priority order + for pq.Len() > 0 { + item := pq.Pop().(*utils.Item) + stackRule := item.Value.(*model.Rule) + t.stack.AddResource(stackRule) + t.log.Debugf(ctx, "Added rule %d to the stack (ID %s)", stackRule.Spec.Priority, stackRule.ID()) } return nil diff --git a/pkg/gateway/model_build_rule_test.go b/pkg/gateway/model_build_rule_test.go index 5c147f89..3f2d1fad 100644 --- a/pkg/gateway/model_build_rule_test.go +++ b/pkg/gateway/model_build_rule_test.go @@ -153,6 +153,7 @@ func Test_RuleModelBuild(t *testing.T) { StackListenerId: "listener-id", PathMatchPrefix: true, PathMatchValue: "/", + Priority: 1, Action: model.RuleAction{ TargetGroups: []*model.RuleTargetGroup{ { @@ -197,6 +198,7 @@ func Test_RuleModelBuild(t *testing.T) { StackListenerId: "listener-id", PathMatchPrefix: true, PathMatchValue: "/", + Priority: 1, Action: model.RuleAction{ TargetGroups: []*model.RuleTargetGroup{ { @@ -247,6 +249,7 @@ func Test_RuleModelBuild(t *testing.T) { StackListenerId: "listener-id", PathMatchPrefix: true, PathMatchValue: "/", + Priority: 1, Action: model.RuleAction{ TargetGroups: []*model.RuleTargetGroup{ { @@ -321,6 +324,7 @@ func Test_RuleModelBuild(t *testing.T) { StackListenerId: "listener-id", PathMatchExact: true, PathMatchValue: path1, + Priority: 1, Action: model.RuleAction{ TargetGroups: []*model.RuleTargetGroup{ { @@ -334,6 +338,7 @@ func Test_RuleModelBuild(t *testing.T) { StackListenerId: "listener-id", PathMatchPrefix: true, PathMatchValue: path2, + Priority: 2, Action: model.RuleAction{ TargetGroups: []*model.RuleTargetGroup{ { @@ -397,6 +402,7 @@ func Test_RuleModelBuild(t *testing.T) { { StackListenerId: "listener-id", Method: string(httpGet), + Priority: 1, Action: model.RuleAction{ TargetGroups: []*model.RuleTargetGroup{ { @@ -409,6 +415,7 @@ func Test_RuleModelBuild(t *testing.T) { { StackListenerId: "listener-id", Method: string(httpPost), + Priority: 2, Action: model.RuleAction{ TargetGroups: []*model.RuleTargetGroup{ { @@ -494,6 +501,7 @@ func Test_RuleModelBuild(t *testing.T) { StackListenerId: "listener-id", PathMatchExact: true, PathMatchValue: path1, + Priority: 1, Action: model.RuleAction{ TargetGroups: []*model.RuleTargetGroup{ { @@ -507,6 +515,7 @@ func Test_RuleModelBuild(t *testing.T) { StackListenerId: "listener-id", PathMatchExact: true, PathMatchValue: path2, + Priority: 2, Action: model.RuleAction{ TargetGroups: []*model.RuleTargetGroup{ { @@ -523,6 +532,7 @@ func Test_RuleModelBuild(t *testing.T) { StackListenerId: "listener-id", PathMatchExact: true, PathMatchValue: path3, + Priority: 3, Action: model.RuleAction{ TargetGroups: []*model.RuleTargetGroup{ { @@ -571,6 +581,7 @@ func Test_RuleModelBuild(t *testing.T) { Method: string(httpPost), PathMatchPrefix: true, PathMatchValue: "/", + Priority: 1, Action: model.RuleAction{ TargetGroups: []*model.RuleTargetGroup{ { @@ -659,6 +670,7 @@ func Test_RuleModelBuild(t *testing.T) { StackListenerId: "listener-id", PathMatchExact: true, PathMatchValue: "/service/method1", + Priority: 1, Method: string(httpPost), Action: model.RuleAction{ TargetGroups: []*model.RuleTargetGroup{ @@ -673,6 +685,7 @@ func Test_RuleModelBuild(t *testing.T) { StackListenerId: "listener-id", PathMatchExact: true, PathMatchValue: "/service/method2", + Priority: 2, Method: string(httpPost), Action: model.RuleAction{ TargetGroups: []*model.RuleTargetGroup{ @@ -690,6 +703,7 @@ func Test_RuleModelBuild(t *testing.T) { StackListenerId: "listener-id", PathMatchExact: true, PathMatchValue: "/service/method3", + Priority: 3, Method: string(httpPost), Action: model.RuleAction{ TargetGroups: []*model.RuleTargetGroup{ @@ -747,6 +761,7 @@ func Test_RuleModelBuild(t *testing.T) { expectedSpec: []model.RuleSpec{ { StackListenerId: "listener-id", + Priority: 1, MatchedHeaders: []vpclattice.HeaderMatch{ { Name: &hdr1, @@ -813,6 +828,7 @@ func Test_RuleModelBuild(t *testing.T) { expectedSpec: []model.RuleSpec{ { StackListenerId: "listener-id", + Priority: 1, MatchedHeaders: []vpclattice.HeaderMatch{ { Name: &hdr1, @@ -892,6 +908,7 @@ func Test_RuleModelBuild(t *testing.T) { StackListenerId: "listener-id", PathMatchExact: true, PathMatchValue: path1, + Priority: 1, MatchedHeaders: []vpclattice.HeaderMatch{ { Name: &hdr1, @@ -971,6 +988,7 @@ func Test_RuleModelBuild(t *testing.T) { StackListenerId: "listener-id", PathMatchPrefix: true, PathMatchValue: path1, + Priority: 1, MatchedHeaders: []vpclattice.HeaderMatch{ { Name: &hdr1, @@ -1153,6 +1171,7 @@ func Test_RuleModelBuild(t *testing.T) { StackListenerId: "listener-id", PathMatchExact: true, PathMatchValue: "/service/method", + Priority: 1, Method: "POST", Action: model.RuleAction{ TargetGroups: []*model.RuleTargetGroup{ @@ -1206,6 +1225,7 @@ func Test_RuleModelBuild(t *testing.T) { StackListenerId: "listener-id", PathMatchPrefix: true, PathMatchValue: "/service/", + Priority: 1, Method: "POST", Action: model.RuleAction{ TargetGroups: []*model.RuleTargetGroup{ @@ -1259,6 +1279,7 @@ func Test_RuleModelBuild(t *testing.T) { StackListenerId: "listener-id", PathMatchPrefix: true, PathMatchValue: "/", + Priority: 1, Method: "POST", Action: model.RuleAction{ TargetGroups: []*model.RuleTargetGroup{ @@ -1339,6 +1360,7 @@ func Test_RuleModelBuild(t *testing.T) { StackListenerId: "listener-id", PathMatchPrefix: true, PathMatchValue: "/service/", + Priority: 1, Method: "POST", Action: model.RuleAction{ TargetGroups: []*model.RuleTargetGroup{ @@ -1416,6 +1438,7 @@ func Test_RuleModelBuild(t *testing.T) { StackListenerId: "listener-id", PathMatchPrefix: true, PathMatchValue: "/", + Priority: 1, Action: model.RuleAction{ TargetGroups: []*model.RuleTargetGroup{ { @@ -1464,6 +1487,7 @@ func Test_RuleModelBuild(t *testing.T) { StackListenerId: "listener-id", PathMatchPrefix: true, PathMatchValue: "/", + Priority: 1, Action: model.RuleAction{ TargetGroups: []*model.RuleTargetGroup{ { @@ -1479,6 +1503,196 @@ func Test_RuleModelBuild(t *testing.T) { }, }, }, + { + name: "rule priority, default assignment", + wantErrIsNil: true, + route: core.NewHTTPRoute(gwv1.HTTPRoute{ + ObjectMeta: apimachineryv1.ObjectMeta{ + Name: "service1", + Namespace: "default", + }, + Spec: gwv1.HTTPRouteSpec{ + CommonRouteSpec: gwv1.CommonRouteSpec{ + ParentRefs: []gwv1.ParentReference{ + { + Name: "gw1", + SectionName: &httpSectionName, + }, + }, + }, + Rules: []gwv1.HTTPRouteRule{ + { + BackendRefs: []gwv1.HTTPBackendRef{ + { + BackendRef: backendRef1, + }, + }, + }, + { + BackendRefs: []gwv1.HTTPBackendRef{ + { + BackendRef: backendRef2, + }, + }, + }, + }, + }, + }), + expectedSpec: []model.RuleSpec{ + { + StackListenerId: "listener-id", + PathMatchPrefix: true, + PathMatchValue: "/", + Priority: 1, + Action: model.RuleAction{ + TargetGroups: []*model.RuleTargetGroup{ + { + StackTargetGroupId: "tg-0", + Weight: int64(weight1), + }, + }, + }, + }, + { + StackListenerId: "listener-id", + PathMatchPrefix: true, + PathMatchValue: "/", + Priority: 2, + Action: model.RuleAction{ + TargetGroups: []*model.RuleTargetGroup{ + { + SvcImportTG: &model.SvcImportTargetGroup{ + K8SServiceName: string(backendRef2.Name), + K8SServiceNamespace: "default", + }, + Weight: int64(weight2), + }, + }, + }, + }, + }, + }, + { + name: "rule priority, annotation override", + wantErrIsNil: true, + route: core.NewHTTPRoute(gwv1.HTTPRoute{ + ObjectMeta: apimachineryv1.ObjectMeta{ + Name: "service1", + Namespace: "default", + Annotations: map[string]string{ + "application-networking.k8s.aws/rule-0-priority": "100", + "application-networking.k8s.aws/rule-1-priority": "50", + }, + }, + Spec: gwv1.HTTPRouteSpec{ + CommonRouteSpec: gwv1.CommonRouteSpec{ + ParentRefs: []gwv1.ParentReference{ + { + Name: "gw1", + SectionName: &httpSectionName, + }, + }, + }, + Rules: []gwv1.HTTPRouteRule{ + { + BackendRefs: []gwv1.HTTPBackendRef{ + { + BackendRef: backendRef1, + }, + }, + }, + { + BackendRefs: []gwv1.HTTPBackendRef{ + { + BackendRef: backendRef2, + }, + }, + }, + }, + }, + }), + expectedSpec: []model.RuleSpec{ + { + StackListenerId: "listener-id", + PathMatchPrefix: true, + PathMatchValue: "/", + Priority: 100, + Action: model.RuleAction{ + TargetGroups: []*model.RuleTargetGroup{ + { + StackTargetGroupId: "tg-0", + Weight: int64(weight1), + }, + }, + }, + }, + { + StackListenerId: "listener-id", + PathMatchPrefix: true, + PathMatchValue: "/", + Priority: 50, + Action: model.RuleAction{ + TargetGroups: []*model.RuleTargetGroup{ + { + SvcImportTG: &model.SvcImportTargetGroup{ + K8SServiceName: string(backendRef2.Name), + K8SServiceNamespace: "default", + }, + Weight: int64(weight2), + }, + }, + }, + }, + }, + }, + { + name: "rule priority, invalid annotation", + wantErrIsNil: true, + route: core.NewHTTPRoute(gwv1.HTTPRoute{ + ObjectMeta: apimachineryv1.ObjectMeta{ + Name: "service1", + Namespace: "default", + Annotations: map[string]string{ + "application-networking.k8s.aws/rule-0-priority": "invalid", + }, + }, + Spec: gwv1.HTTPRouteSpec{ + CommonRouteSpec: gwv1.CommonRouteSpec{ + ParentRefs: []gwv1.ParentReference{ + { + Name: "gw1", + SectionName: &httpSectionName, + }, + }, + }, + Rules: []gwv1.HTTPRouteRule{ + { + BackendRefs: []gwv1.HTTPBackendRef{ + { + BackendRef: backendRef1, + }, + }, + }, + }, + }, + }), + expectedSpec: []model.RuleSpec{ + { + StackListenerId: "listener-id", + PathMatchPrefix: true, + PathMatchValue: "/", + Priority: 1, + Action: model.RuleAction{ + TargetGroups: []*model.RuleTargetGroup{ + { + StackTargetGroupId: "tg-0", + Weight: int64(weight1), + }, + }, + }, + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -1537,10 +1751,7 @@ func validateEqual(t *testing.T, expectedRules []model.RuleSpec, actualRules []* assert.Equal(t, expectedSpec.PathMatchPrefix, actualRule.Spec.PathMatchPrefix) assert.Equal(t, expectedSpec.PathMatchExact, actualRule.Spec.PathMatchExact) assert.Equal(t, expectedSpec.Method, actualRule.Spec.Method) - - // priority is not determined by model building, but in synthesis, so we don't - // validate priority here - + assert.Equal(t, expectedSpec.Priority, actualRule.Spec.Priority) assert.True(t, reflect.DeepEqual(expectedSpec.MatchedHeaders, actualRule.Spec.MatchedHeaders)) assert.Equal(t, len(expectedSpec.Action.TargetGroups), len(actualRule.Spec.Action.TargetGroups)) diff --git a/pkg/utils/priority_queue.go b/pkg/utils/priority_queue.go new file mode 100644 index 00000000..684fe5f7 --- /dev/null +++ b/pkg/utils/priority_queue.go @@ -0,0 +1,51 @@ +package utils + +// An Item is something we manage in a priority queue. +type Item struct { + Value any + Priority int32 // The priority of the item in the queue. + // The Index is needed by update and is maintained by the heap.Interface methods. + Index int // The Index of the item in the heap. +} + +// A PriorityQueue implements heap.Interface and holds Items. +type PriorityQueue []*Item + +func (pq PriorityQueue) Len() int { return len(pq) } + +func (pq PriorityQueue) Less(i, j int) bool { + // We want Pop to give us the highest, not lowest, priority so we use greater than here. + return pq[i].Priority > pq[j].Priority +} + +func (pq PriorityQueue) Swap(i, j int) { + pq[i], pq[j] = pq[j], pq[i] + pq[i].Index = i + pq[j].Index = j +} + +func (pq *PriorityQueue) Push(x any) { + n := len(*pq) + item := x.(*Item) + item.Index = n + *pq = append(*pq, item) +} + +// Peek returns the highest priority item without removing it from the queue. +// Returns nil if the queue is empty. +func (pq *PriorityQueue) Peek() *Item { + if len(*pq) == 0 { + return nil + } + return (*pq)[0] +} + +func (pq *PriorityQueue) Pop() any { + old := *pq + n := len(old) + item := old[n-1] + old[n-1] = nil // don't stop the GC from reclaiming the item eventually + item.Index = -1 // for safety + *pq = old[0 : n-1] + return item +} From 981e2cd7599ecfa064e17c96457f5b5cb8d36e02 Mon Sep 17 00:00:00 2001 From: Ryan Lymburner Date: Tue, 13 May 2025 13:15:17 -0700 Subject: [PATCH 2/4] Added integration tests --- .../httproute_rule_priority_test.go | 244 ++++++++++++++++++ 1 file changed, 244 insertions(+) create mode 100644 test/suites/integration/httproute_rule_priority_test.go diff --git a/test/suites/integration/httproute_rule_priority_test.go b/test/suites/integration/httproute_rule_priority_test.go new file mode 100644 index 00000000..3d1de602 --- /dev/null +++ b/test/suites/integration/httproute_rule_priority_test.go @@ -0,0 +1,244 @@ +package integration + +import ( + "github.com/aws/aws-application-networking-k8s/pkg/model/core" + "github.com/aws/aws-application-networking-k8s/test/pkg/test" + "github.com/aws/aws-sdk-go/service/vpclattice" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/samber/lo" + appsv1 "k8s.io/api/apps/v1" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "log" + "os" + gwv1 "sigs.k8s.io/gateway-api/apis/v1" +) + +var _ = Describe("HTTPRoute rule priorities", func() { + var ( + deployment1 *appsv1.Deployment + deployment2 *appsv1.Deployment + deployment3 *appsv1.Deployment + service1 *v1.Service + service2 *v1.Service + service3 *v1.Service + rulePriorityHttpRoute *gwv1.HTTPRoute + ) + + It("HTTPRoute should support manual rule priorities through annotations", func() { + // Create three different apps to test priority routing + deployment1, service1 = testFramework.NewHttpApp(test.HTTPAppOptions{Name: "priority-test-v1", Namespace: k8snamespace}) + deployment2, service2 = testFramework.NewHttpApp(test.HTTPAppOptions{Name: "priority-test-v2", Namespace: k8snamespace}) + deployment3, service3 = testFramework.NewHttpApp(test.HTTPAppOptions{Name: "priority-test-v3", Namespace: k8snamespace}) + + // Create HTTPRoute with rules having different priorities + rulePriorityHttpRoute = &gwv1.HTTPRoute{ + ObjectMeta: metav1.ObjectMeta{ + Name: "priority-test-route", + Namespace: k8snamespace, + }, + Spec: gwv1.HTTPRouteSpec{ + CommonRouteSpec: gwv1.CommonRouteSpec{ + ParentRefs: []gwv1.ParentReference{ + { + Name: gwv1.ObjectName(testGateway.Name), + }, + }, + }, + Rules: []gwv1.HTTPRouteRule{ + { + // High priority rule (1) - should be evaluated first + Matches: []gwv1.HTTPRouteMatch{ + { + Path: &gwv1.HTTPPathMatch{ + Type: lo.ToPtr(gwv1.PathMatchPathPrefix), + Value: lo.ToPtr("/api"), + }, + }, + }, + BackendRefs: []gwv1.HTTPBackendRef{ + { + BackendRef: gwv1.BackendRef{ + BackendObjectReference: gwv1.BackendObjectReference{ + Name: gwv1.ObjectName(service1.Name), + Port: lo.ToPtr(gwv1.PortNumber(80)), + }, + }, + }, + }, + Filters: []gwv1.HTTPRouteFilter{}, + }, + { + // Low priority rule (100) - should be evaluated last + Matches: []gwv1.HTTPRouteMatch{ + { + Path: &gwv1.HTTPPathMatch{ + Type: lo.ToPtr(gwv1.PathMatchPathPrefix), + Value: lo.ToPtr("/api/v2"), + }, + }, + }, + BackendRefs: []gwv1.HTTPBackendRef{ + { + BackendRef: gwv1.BackendRef{ + BackendObjectReference: gwv1.BackendObjectReference{ + Name: gwv1.ObjectName(service2.Name), + Port: lo.ToPtr(gwv1.PortNumber(80)), + }, + }, + }, + }, + Filters: []gwv1.HTTPRouteFilter{}, + }, + { + // Medium priority rule (50) - should be evaluated second + Matches: []gwv1.HTTPRouteMatch{ + { + Path: &gwv1.HTTPPathMatch{ + Type: lo.ToPtr(gwv1.PathMatchPathPrefix), + Value: lo.ToPtr("/api/special"), + }, + }, + }, + BackendRefs: []gwv1.HTTPBackendRef{ + { + BackendRef: gwv1.BackendRef{ + BackendObjectReference: gwv1.BackendObjectReference{ + Name: gwv1.ObjectName(service3.Name), + Port: lo.ToPtr(gwv1.PortNumber(80)), + }, + }, + }, + }, + Filters: []gwv1.HTTPRouteFilter{}, + }, + }, + }, + } + + // Add priority annotations to the HTTPRoute + if rulePriorityHttpRoute.Annotations == nil { + rulePriorityHttpRoute.Annotations = make(map[string]string) + } + rulePriorityHttpRoute.Annotations["application-networking.k8s.aws/rule-0-priority"] = "1" // High priority + rulePriorityHttpRoute.Annotations["application-networking.k8s.aws/rule-1-priority"] = "100" // Low priority + rulePriorityHttpRoute.Annotations["application-networking.k8s.aws/rule-2-priority"] = "50" // Medium priority + + // Create Kubernetes API Objects + testFramework.ExpectCreated(ctx, + rulePriorityHttpRoute, + service1, + deployment1, + service2, + deployment2, + service3, + deployment3, + ) + + route, _ := core.NewRoute(rulePriorityHttpRoute) + vpcLatticeService := testFramework.GetVpcLatticeService(ctx, route) + + // Verify target groups + targetGroupV1 := testFramework.GetTargetGroup(ctx, service1) + Expect(*targetGroupV1.VpcIdentifier).To(Equal(os.Getenv("CLUSTER_VPC_ID"))) + Expect(*targetGroupV1.Protocol).To(Equal("HTTP")) + targetsV1 := testFramework.GetTargets(ctx, targetGroupV1, deployment1) + Expect(*targetGroupV1.Port).To(BeEquivalentTo(80)) + for _, target := range targetsV1 { + Expect(*target.Port).To(BeEquivalentTo(service1.Spec.Ports[0].TargetPort.IntVal)) + Expect(*target.Status).To(Or( + Equal(vpclattice.TargetStatusInitial), + Equal(vpclattice.TargetStatusHealthy), + )) + } + + targetGroupV2 := testFramework.GetTargetGroup(ctx, service2) + Expect(*targetGroupV2.VpcIdentifier).To(Equal(os.Getenv("CLUSTER_VPC_ID"))) + Expect(*targetGroupV2.Protocol).To(Equal("HTTP")) + targetsV2 := testFramework.GetTargets(ctx, targetGroupV2, deployment2) + Expect(*targetGroupV2.Port).To(BeEquivalentTo(80)) + for _, target := range targetsV2 { + Expect(*target.Port).To(BeEquivalentTo(service2.Spec.Ports[0].TargetPort.IntVal)) + Expect(*target.Status).To(Or( + Equal(vpclattice.TargetStatusInitial), + Equal(vpclattice.TargetStatusHealthy), + )) + } + + targetGroupV3 := testFramework.GetTargetGroup(ctx, service3) + Expect(*targetGroupV3.VpcIdentifier).To(Equal(os.Getenv("CLUSTER_VPC_ID"))) + Expect(*targetGroupV3.Protocol).To(Equal("HTTP")) + targetsV3 := testFramework.GetTargets(ctx, targetGroupV3, deployment3) + Expect(*targetGroupV3.Port).To(BeEquivalentTo(80)) + for _, target := range targetsV3 { + Expect(*target.Port).To(BeEquivalentTo(service3.Spec.Ports[0].TargetPort.IntVal)) + Expect(*target.Status).To(Or( + Equal(vpclattice.TargetStatusInitial), + Equal(vpclattice.TargetStatusHealthy), + )) + } + + log.Println("Verifying VPC lattice service listeners and rules") + Eventually(func(g Gomega) { + listListenerResp, err := testFramework.LatticeClient.ListListenersWithContext(ctx, &vpclattice.ListListenersInput{ + ServiceIdentifier: vpcLatticeService.Id, + }) + g.Expect(err).To(BeNil()) + g.Expect(len(listListenerResp.Items)).To(BeEquivalentTo(1)) + listener := listListenerResp.Items[0] + g.Expect(*listener.Port).To(BeEquivalentTo(testGateway.Spec.Listeners[0].Port)) + listenerId := listener.Id + + listRulesResp, err := testFramework.LatticeClient.ListRulesWithContext(ctx, &vpclattice.ListRulesInput{ + ListenerIdentifier: listenerId, + ServiceIdentifier: vpcLatticeService.Id, + }) + nonDefaultRules := lo.Filter(listRulesResp.Items, func(rule *vpclattice.RuleSummary, _ int) bool { + return rule.IsDefault == nil || *rule.IsDefault == false + }) + ruleIds := lo.Map(nonDefaultRules, func(rule *vpclattice.RuleSummary, _ int) *string { + return rule.Id + }) + + g.Expect(len(ruleIds)).To(Equal(3)) + + // Verify rules are created with correct priorities + rules := make([]*vpclattice.GetRuleOutput, len(ruleIds)) + for i, ruleId := range ruleIds { + rule, err := testFramework.LatticeClient.GetRuleWithContext(ctx, &vpclattice.GetRuleInput{ + ServiceIdentifier: vpcLatticeService.Id, + ListenerIdentifier: listenerId, + RuleIdentifier: ruleId, + }) + g.Expect(err).To(BeNil()) + rules[i] = rule + } + + // Verify rule priorities are set correctly + // Rule priorities in VPC Lattice should match our annotations + for _, rule := range rules { + switch *rule.Match.HttpMatch.PathMatch.Match.Prefix { + case "/api/v2": + g.Expect(*rule.Priority).To(BeEquivalentTo(100)) + case "/api/special": + g.Expect(*rule.Priority).To(BeEquivalentTo(50)) + case "/api": + g.Expect(*rule.Priority).To(BeEquivalentTo(1)) + } + } + }).WithOffset(1).Should(Succeed()) + }) + + AfterEach(func() { + testFramework.ExpectDeletedThenNotFound(ctx, + rulePriorityHttpRoute, + deployment1, + deployment2, + deployment3, + service1, + service2, + service3, + ) + }) +}) From 5fa464dabc28648ede70bc7f5daf5ddbf38ae4c1 Mon Sep 17 00:00:00 2001 From: Ryan Lymburner Date: Tue, 13 May 2025 13:21:58 -0700 Subject: [PATCH 3/4] Updated advanced configuration documentation --- docs/guides/advanced-configurations.md | 32 ++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/docs/guides/advanced-configurations.md b/docs/guides/advanced-configurations.md index acc2b8c2..10fadf1e 100644 --- a/docs/guides/advanced-configurations.md +++ b/docs/guides/advanced-configurations.md @@ -16,6 +16,38 @@ However, the controller utilizes [IMDS](https://docs.aws.amazon.com/AWSEC2/lates - **If your cluster cannot access to IMDS.** ensure to specify the[configuration variables](environment.md) when installing the controller. +### Rule Priority Configuration + +You can manually assign priorities to rules using the custom annotation `application-networking.k8s.aws/rule-{index}-priority`. This annotation allows you to explicitly set the priority for specific rules in your route configurations. + +For example, to set priorities for multiple rules in an HTTPRoute: + +```yaml +apiVersion: gateway.networking.k8s.io/v1beta1 +kind: HTTPRoute +metadata: + name: example-route + annotations: + application-networking.k8s.aws/rule-0-priority: "200" # First rule gets higher priority + application-networking.k8s.aws/rule-1-priority: "100" # Second rule gets lower priority +spec: + rules: + - matches: # This is rule[0] + - path: + type: PathPrefix + value: /api/v2 + - matches: # This is rule[1] + - path: + type: PathPrefix + value: /api +``` + +The `{index}` in the annotation corresponds to the zero-based index of the rule in the rules array. In this example: +- `rule-0-priority: "200"` applies to the first rule matching `/api/v2` +- `rule-1-priority: "100"` applies to the second rule matching `/api` + +Higher priority values indicate higher precedence, so requests to `/api/v2` will be matched by the first rule (priority 200) before the second rule (priority 100) is considered. + ### IPv6 support IPv6 address type is automatically used for your services and pods if From bbd820566ec96cc60501003da493dbdd0d21e771 Mon Sep 17 00:00:00 2001 From: Ryan Lymburner Date: Tue, 13 May 2025 14:49:26 -0700 Subject: [PATCH 4/4] Correct priority queue --- pkg/gateway/model_build_rule.go | 78 +++++++++++++++++++-------------- pkg/utils/priority_queue.go | 12 ++--- 2 files changed, 52 insertions(+), 38 deletions(-) diff --git a/pkg/gateway/model_build_rule.go b/pkg/gateway/model_build_rule.go index aad0e89c..1241d2a2 100644 --- a/pkg/gateway/model_build_rule.go +++ b/pkg/gateway/model_build_rule.go @@ -1,7 +1,6 @@ package gateway import ( - "container/heap" "context" "errors" "fmt" @@ -35,14 +34,9 @@ func (t *latticeServiceModelBuildTask) buildRules(ctx context.Context, stackList // note we only build rules for non-deleted routes t.log.Debugf(ctx, "Processing %d rules", len(t.route.Spec().Rules())) - // Skip if route is being deleted - if !t.route.DeletionTimestamp().IsZero() { - t.log.Debugf(ctx, "Skipping rule creation since the route is deleted") - return nil - } - - // Create a priority queue to organize rules - pq := make(utils.PriorityQueue, 0, len(t.route.Spec().Rules())) + // Track rules with and without priority + rulesWithoutPriority := make([]core.RouteRule, 0) + priorityQueue := make(utils.PriorityQueue, 0) // First pass: build all rules and add them to priority queue for i, rule := range t.route.Spec().Rules() { @@ -57,11 +51,42 @@ func (t *latticeServiceModelBuildTask) buildRules(ctx context.Context, stackList } else { t.log.Warnf(ctx, "Invalid priority value in annotation for rule %d: %s", i, priorityStr) } + + priorityQueue.Push(&utils.Item{ + Value: rule, + Priority: int32(priority), + }) + + } else { + rulesWithoutPriority = append(rulesWithoutPriority, rule) } + } + + // Assign rules without a manually assigned priority a priority in sequential order following the greatest + // manually assigned priority + for _, ruleSpec := range rulesWithoutPriority { + // No manually assigned priorities + topItem, err := priorityQueue.Peek() + if err == nil { + t.log.Debugf(ctx, "Setting default rule priority set to: %d", topItem.Priority+1) + priorityQueue.Push(&utils.Item{ + Value: ruleSpec, + Priority: topItem.Priority + 1, + }) + } else { + t.log.Debugf(ctx, "Setting default rule priority set to: %d", 1) + priorityQueue.Push(&utils.Item{ + Value: ruleSpec, + Priority: 1, + }) + } + } + for _, item := range priorityQueue { + rule := item.Value.(core.RouteRule) ruleSpec := model.RuleSpec{ StackListenerId: stackListenerId, - Priority: priority, + Priority: int64(item.Priority), } if len(rule.Matches()) > 1 { @@ -88,14 +113,12 @@ func (t *latticeServiceModelBuildTask) buildRules(ctx context.Context, stackList 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) @@ -107,28 +130,17 @@ func (t *latticeServiceModelBuildTask) buildRules(ctx context.Context, stackList TargetGroups: ruleTgList, } - // Create the rule but don't add to stack yet - stackRule, err := model.NewRule(t.stack, ruleSpec) - if err != nil { - return err + // don't bother adding rules on delete, these will be removed automatically with the owning route/lattice service + // target groups will still be present and removed as needed + if t.route.DeletionTimestamp().IsZero() { + stackRule, err := model.NewRule(t.stack, ruleSpec) + if err != nil { + return err + } + t.log.Debugf(ctx, "Added rule %d to the stack (ID %s)", stackRule.Spec.Priority, stackRule.ID()) + } else { + t.log.Debugf(ctx, "Skipping adding rule %d to the stack since the route is deleted", ruleSpec.Priority) } - - // Add to priority queue - pq.Push(&utils.Item{ - Value: stackRule, - Priority: int32(priority), - }) - } - - // Initialize heap - heap.Init(&pq) - - // Add rules to stack in priority order - for pq.Len() > 0 { - item := pq.Pop().(*utils.Item) - stackRule := item.Value.(*model.Rule) - t.stack.AddResource(stackRule) - t.log.Debugf(ctx, "Added rule %d to the stack (ID %s)", stackRule.Spec.Priority, stackRule.ID()) } return nil diff --git a/pkg/utils/priority_queue.go b/pkg/utils/priority_queue.go index 684fe5f7..47681117 100644 --- a/pkg/utils/priority_queue.go +++ b/pkg/utils/priority_queue.go @@ -1,5 +1,7 @@ package utils +import "fmt" + // An Item is something we manage in a priority queue. type Item struct { Value any @@ -15,7 +17,7 @@ func (pq PriorityQueue) Len() int { return len(pq) } func (pq PriorityQueue) Less(i, j int) bool { // We want Pop to give us the highest, not lowest, priority so we use greater than here. - return pq[i].Priority > pq[j].Priority + return pq[i].Priority < pq[j].Priority } func (pq PriorityQueue) Swap(i, j int) { @@ -32,12 +34,12 @@ func (pq *PriorityQueue) Push(x any) { } // Peek returns the highest priority item without removing it from the queue. -// Returns nil if the queue is empty. -func (pq *PriorityQueue) Peek() *Item { +// Returns nil and an error if the queue is empty. +func (pq *PriorityQueue) Peek() (*Item, error) { if len(*pq) == 0 { - return nil + return nil, fmt.Errorf("priority queue is empty") } - return (*pq)[0] + return (*pq)[len(*pq)-1], nil } func (pq *PriorityQueue) Pop() any {