combine Kong services for HTTPRoutes when using expression routes

Kong · Dec 10, 2024 · caaeac9 · caaeac9
1 parent 5ac94d2
commit caaeac9
Show file tree

Hide file tree

Showing 7 changed files with 233 additions and 634 deletions.
diff --git a/internal/dataplane/testdata/golden/httproute-example/expression-routes-on_golden.yaml b/internal/dataplane/testdata/golden/httproute-example/expression-routes-on_golden.yaml
@@ -1,9 +1,9 @@
 _format_version: "3.0"
 services:
 - connect_timeout: 60000
-  host: httproute.default.httproute-testing._.3
-  id: 23683d37-44ee-559b-add2-a0b650ad36c0
-  name: httproute.default.httproute-testing._.3
+  host: httproute.default.httproute-testing.3
+  id: efb4a7d7-791f-504e-b005-0bd38bf1435e
+  name: httproute.default.httproute-testing.3
   port: 8080
   protocol: http
   read_timeout: 60000
@@ -29,9 +29,9 @@ services:
   - k8s-version:v1
   write_timeout: 60000
 - connect_timeout: 60000
-  host: httproute.default.httproute-testing._.2
-  id: ff054481-b58a-588f-a8c4-e6cedb6cb489
-  name: httproute.default.httproute-testing._.2
+  host: httproute.default.httproute-testing.1
+  id: 672dc964-a7f8-5a80-a518-96fc39500cc8
+  name: httproute.default.httproute-testing.1
   port: 80
   protocol: http
   read_timeout: 60000
@@ -51,21 +51,6 @@ services:
     - k8s-group:gateway.networking.k8s.io
     - k8s-version:v1
     - k8s-named-route-rule:content
-  tags:
-  - k8s-name:httpbin
-  - k8s-namespace:default
-  - k8s-kind:Service
-  - k8s-version:v1
-  write_timeout: 60000
-- connect_timeout: 60000
-  host: httproute.default.httproute-testing._.1
-  id: 13be00e6-9c94-561f-9598-8896cd755468
-  name: httproute.default.httproute-testing._.1
-  port: 80
-  protocol: http
-  read_timeout: 60000
-  retries: 5
-  routes:
   - expression: (http.path == "/echo") || (http.path ^= "/echo/")
     https_redirect_status_code: 426
     id: 88d36cfe-fbb0-5d7a-93c1-df18d1db3a12
@@ -87,9 +72,9 @@ services:
   - k8s-version:v1
   write_timeout: 60000
 - connect_timeout: 60000
-  host: httproute.default.httproute-testing._.0
-  id: 2fad71d1-7599-5c6e-9d4f-4afd44f99587
-  name: httproute.default.httproute-testing._.0
+  host: httproute.default.httproute-testing.0
+  id: 4e3cb785-a8d0-5866-aa05-117f7c64f24d
+  name: httproute.default.httproute-testing.0
   port: 8080
   protocol: http
   read_timeout: 60000
@@ -118,28 +103,21 @@ services:
   write_timeout: 60000
 upstreams:
 - algorithm: round-robin
-  name: httproute.default.httproute-testing._.3
+  name: httproute.default.httproute-testing.3
   tags:
   - k8s-name:nginx
   - k8s-namespace:default
   - k8s-kind:Service
   - k8s-version:v1
 - algorithm: round-robin
-  name: httproute.default.httproute-testing._.2
-  tags:
-  - k8s-name:httpbin
-  - k8s-namespace:default
-  - k8s-kind:Service
-  - k8s-version:v1
-- algorithm: round-robin
-  name: httproute.default.httproute-testing._.1
+  name: httproute.default.httproute-testing.1
   tags:
   - k8s-name:httpbin
   - k8s-namespace:default
   - k8s-kind:Service
   - k8s-version:v1
 - algorithm: round-robin
-  name: httproute.default.httproute-testing._.0
+  name: httproute.default.httproute-testing.0
   tags:
   - k8s-name:httproute-testing
   - k8s-namespace:default

diff --git a/...aplane/testdata/golden/httproute-url-rewrite-path-prefix/expression-routes-on_golden.yaml b/...aplane/testdata/golden/httproute-url-rewrite-path-prefix/expression-routes-on_golden.yaml
@@ -1,9 +1,9 @@
 _format_version: "3.0"
 services:
 - connect_timeout: 60000
-  host: httproute.default.httproute-testing._.0
-  id: 2fad71d1-7599-5c6e-9d4f-4afd44f99587
-  name: httproute.default.httproute-testing._.0
+  host: httproute.default.httproute-testing.0
+  id: 4e3cb785-a8d0-5866-aa05-117f7c64f24d
+  name: httproute.default.httproute-testing.0
   port: 8080
   protocol: http
   read_timeout: 60000
@@ -42,7 +42,7 @@ services:
   write_timeout: 60000
 upstreams:
 - algorithm: round-robin
-  name: httproute.default.httproute-testing._.0
+  name: httproute.default.httproute-testing.0
   tags:
   - k8s-name:httproute-testing
   - k8s-namespace:default

diff --git a/internal/dataplane/translator/subtranslator/httproute.go b/internal/dataplane/translator/subtranslator/httproute.go
@@ -61,16 +61,51 @@ func TranslateHTTPRoutesToKongstateServices(
 	storer store.Storer,
 	routes []*gatewayapi.HTTPRoute,
 	combinedServicesFromDifferentHTTPRoutes bool,
+	expressionRoutes bool,
 ) HTTPRoutesTranslationResult {
 	serviceNameToRules := groupRulesFromHTTPRoutesByKongServiceName(routes, combinedServicesFromDifferentHTTPRoutes)
 
+	// first, split HTTPRoutes by hostnames and matches.
+	ruleToSplitMatchesWithPriorities := make(map[string][]SplitHTTPRouteMatchToKongRoutePriority)
+	if expressionRoutes {
+		splitHTTPRouteMatches := []SplitHTTPRouteMatch{}
+		for _, route := range routes {
+			splitHTTPRouteMatches = append(splitHTTPRouteMatches, SplitHTTPRoute(route)...)
+		}
+		// assign priorities to split HTTPRoutes.
+		splitHTTPRouteMatchesWithPriorities := AssignRoutePriorityToSplitHTTPRouteMatches(logger, splitHTTPRouteMatches)
+		for _, matchWithPriority := range splitHTTPRouteMatchesWithPriorities {
+			sourceRoute := matchWithPriority.Match.Source
+			ruleKey := fmt.Sprintf("%s/%s.%d", sourceRoute.Namespace, sourceRoute.Name, matchWithPriority.Match.RuleIndex)
+			ruleToSplitMatchesWithPriorities[ruleKey] = append(ruleToSplitMatchesWithPriorities[ruleKey], matchWithPriority)
+		}
+	}
+
 	kongstateServiceCache := map[string]kongstate.Service{}
 	routeTranslationErrors := map[k8stypes.NamespacedName][]error{}
 	for serviceName, rulesMeta := range serviceNameToRules {
 		if len(rulesMeta) == 0 {
 			continue
 		}
-		service, err := translateHTTPRouteRulesMetaToKongstateService(logger, storer, serviceName, rulesMeta)
+
+		// Sort the rules according to the namespace, name and rule number to guarantee a fixed order.
+		sort.SliceStable(rulesMeta, func(i, j int) bool {
+			return rulesMeta[i].getRuleKey() < rulesMeta[j].getRuleKey()
+		})
+
+		matchesWithPriorities := []SplitHTTPRouteMatchToKongRoutePriority{}
+		if expressionRoutes {
+			for _, ruleMeta := range rulesMeta {
+				ruleKey := ruleMeta.getRuleKey()
+				matchesWithPriorities = append(matchesWithPriorities, ruleToSplitMatchesWithPriorities[ruleKey]...)
+			}
+		}
+		service, err := translateHTTPRouteRulesMetaToKongstateService(
+			logger, storer, serviceName, rulesMeta,
+			expressionRoutes,
+			matchesWithPriorities,
+		)
+		// Set translation errors for involved HTTPRoutes on failure of translation.
 		if err != nil {
 			httpRoutes := extractUniqueHTTPRoutes(rulesMeta)
 			for _, route := range httpRoutes {
@@ -82,6 +117,7 @@ func TranslateHTTPRoutesToKongstateServices(
 			}
 			continue
 		}
+
 		kongstateServiceCache[serviceName] = service
 	}
 
@@ -165,6 +201,8 @@ func translateHTTPRouteRulesMetaToKongstateService(
 	storer store.Storer,
 	serviceName string,
 	rulesMeta []httpRouteRuleMeta,
+	expressionRoutes bool,
+	matchesWithPriorities []SplitHTTPRouteMatchToKongRoutePriority,
 ) (kongstate.Service, error) {
 	// Fill in the common fields of the kongstate.Service.
 	service := kongstate.Service{
@@ -235,13 +273,21 @@ func translateHTTPRouteRulesMetaToKongstateService(
 		applyTimeoutToServiceFromHTTPRouteRule(&service, ruleMeta.Rule)
 	}
 
-	routes, err := translateHTTPRouteRulesMetaToKongstateRoutes(rulesMeta)
-	if err != nil {
-		return kongstate.Service{}, err
+	if expressionRoutes {
+		routes, err := translateHTTPRouteRulesMetaToKongstateRoutesWithExpression(matchesWithPriorities)
+		if err != nil {
+			return kongstate.Service{}, err
+		}
+		service.Routes = make([]kongstate.Route, 0, len(routes))
+		service.Routes = append(service.Routes, routes...)
+	} else {
+		routes, err := translateHTTPRouteRulesMetaToKongstateRoutes(rulesMeta)
+		if err != nil {
+			return kongstate.Service{}, err
+		}
+		service.Routes = make([]kongstate.Route, 0, len(routes))
+		service.Routes = append(service.Routes, routes...)
 	}
-	// preallocate slice for the routes.
-	service.Routes = make([]kongstate.Route, 0, len(routes))
-	service.Routes = append(service.Routes, routes...)
 
 	return service, nil
 }
@@ -355,6 +401,24 @@ func translateHTTPRouteRulesMetaToKongstateRoutes(
 	return routes, nil
 }
 
+func translateHTTPRouteRulesMetaToKongstateRoutesWithExpression(
+	matchesWithPriorities []SplitHTTPRouteMatchToKongRoutePriority,
+) ([]kongstate.Route, error) {
+	routes := []kongstate.Route{}
+	for _, matchWithPriority := range matchesWithPriorities {
+		// Since each match is assigned a deterministic priority, we have to generate one route for each split match
+		// because every match have a different priority.
+		// TODO: update the algorithm to assign priorities to matches to make it possible to consilidate some matches.
+		// For example, we can assign the same priority to multiple matches from the same rule if they tie on the priority from the fixed fields.
+		route, err := KongExpressionRouteFromHTTPRouteMatchWithPriority(matchWithPriority)
+		if err != nil {
+			return []kongstate.Route{}, err
+		}
+		routes = append(routes, *route)
+	}
+	return routes, nil
+}
+
 // extractUniqueHTTPRoutes extracts unique HTTPRoutes in a grouped list of HTTPRouteRuleMeta.
 func extractUniqueHTTPRoutes(rulesMeta []httpRouteRuleMeta) []*gatewayapi.HTTPRoute {
 	routes := lo.Map(rulesMeta, func(m httpRouteRuleMeta, _ int) *gatewayapi.HTTPRoute {
@@ -428,6 +492,10 @@ func (m httpRouteRuleMeta) getHTTPBackendRefsKey() string {
 	return getSortedItemsString(m.Rule.BackendRefs)
 }
 
+func (m httpRouteRuleMeta) getRuleKey() string {
+	return fmt.Sprintf("%s/%s.%d", m.parentRoute.Namespace, m.parentRoute.Name, m.RuleNumber)
+}
+
 // getFiltersKey computes a key from a list of filters.
 // The order of the filters is not important.
 func (m httpRouteRuleMeta) getFiltersKey() string {

diff --git a/internal/dataplane/translator/subtranslator/httproute_atc.go b/internal/dataplane/translator/subtranslator/httproute_atc.go
@@ -175,9 +175,6 @@ func pathMatcherFromHTTPPathMatch(pathMatch *gatewayapi.HTTPPathMatch) atc.Match
 			atc.NewPredicateHTTPPath(atc.OpPrefixMatch, path+"/"),
 		)
 	case gatewayapi.PathMatchRegularExpression:
-		// TODO: for compatibility with kong traditional routes, here we append the ^ prefix to match the path from beginning.
-		// Could we allow the regex to match any part of the path?
-		// https://github.com/Kong/kubernetes-ingress-controller/issues/3983
 		return atc.NewPredicateHTTPPath(atc.OpRegexMatch, appendRegexBeginIfNotExist(path))
 	}
 

diff --git a/internal/dataplane/translator/subtranslator/httproute_test.go b/internal/dataplane/translator/subtranslator/httproute_test.go
@@ -1694,7 +1694,7 @@ func TestTranslateHTTPRoutesToKongstateServices(t *testing.T) {
 				oldHTTPRoutes = append(oldHTTPRoutes, r.DeepCopy())
 			}
 
-			translationResult := TranslateHTTPRoutesToKongstateServices(logger, fakestore, tc.httpRoutes, true)
+			translationResult := TranslateHTTPRoutesToKongstateServices(logger, fakestore, tc.httpRoutes, true, false)
 			require.Len(t, translationResult.HTTPRouteNameToTranslationErrors, 0, "Should not get translation errors in translating")
 
 			kongstateServices := translationResult.ServiceNameToKongstateService
@@ -1720,3 +1720,117 @@ func TestTranslateHTTPRoutesToKongstateServices(t *testing.T) {
 		})
 	}
 }
+
+func TestTranslateHTTPRouteRulesMetaToKongstateRoutes(t *testing.T) {
+
+	httpRouteTypeMeta := metav1.TypeMeta{
+		APIVersion: "gateway.networking.k8s.io/v1",
+		Kind:       "HTTPRoute",
+	}
+	backendRefList := []gatewayapi.HTTPBackendRef{
+		{
+			BackendRef: gatewayapi.BackendRef{
+				BackendObjectReference: gatewayapi.BackendObjectReference{
+					Kind:      lo.ToPtr(gatewayapi.Kind("Service")),
+					Name:      gatewayapi.ObjectName("service-1"),
+					Namespace: lo.ToPtr(gatewayapi.Namespace("default")),
+					Port:      lo.ToPtr(gatewayapi.PortNumber(80)),
+				},
+			},
+		},
+	}
+	httpRouteWithoutHost := &gatewayapi.HTTPRoute{
+		TypeMeta: httpRouteTypeMeta,
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: "default",
+			Name:      "httproute-1",
+		},
+	}
+
+	testCases := []struct {
+		name           string
+		rulesMeta      []httpRouteRuleMeta
+		expectedRoutes []kongstate.Route
+		expectError    bool
+	}{
+		{
+			name: "multiple rules with the same(empty) filter from the same HTTPRoute",
+			rulesMeta: []httpRouteRuleMeta{
+				{
+					Rule: gatewayapi.HTTPRouteRule{
+						BackendRefs: backendRefList,
+						Matches: []gatewayapi.HTTPRouteMatch{
+							{
+								Path: &gatewayapi.HTTPPathMatch{
+									Type:  lo.ToPtr(gatewayapi.PathMatchExact),
+									Value: lo.ToPtr("/foo"),
+								},
+							},
+							{
+								Path: &gatewayapi.HTTPPathMatch{
+									Type:  lo.ToPtr(gatewayapi.PathMatchExact),
+									Value: lo.ToPtr("/bar"),
+								},
+							},
+						},
+					},
+					RuleNumber:  0,
+					parentRoute: httpRouteWithoutHost,
+				},
+				{
+					Rule: gatewayapi.HTTPRouteRule{
+						BackendRefs: backendRefList,
+						Matches: []gatewayapi.HTTPRouteMatch{
+							{
+								Path: &gatewayapi.HTTPPathMatch{
+									Type:  lo.ToPtr(gatewayapi.PathMatchExact),
+									Value: lo.ToPtr("/baz"),
+								},
+							},
+						},
+					},
+					RuleNumber:  1,
+					parentRoute: httpRouteWithoutHost,
+				},
+			},
+			expectedRoutes: []kongstate.Route{
+				{
+					Route: kong.Route{
+						Name:         kong.String("httproute.default.httproute-1.0.0"),
+						Paths:        kong.StringSlice("~/foo$", "~/bar$", "~/baz$"),
+						PreserveHost: kong.Bool(true),
+						StripPath:    kong.Bool(false),
+						Protocols: []*string{
+							kong.String("http"),
+							kong.String("https"),
+						},
+						Tags: []*string{
+							kong.String("k8s-name:httproute-1"),
+							kong.String("k8s-namespace:default"),
+							kong.String("k8s-kind:HTTPRoute"),
+							kong.String("k8s-group:gateway.networking.k8s.io"),
+							kong.String("k8s-version:v1"),
+						},
+					},
+					Ingress: util.FromK8sObject(httpRouteWithoutHost),
+				},
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			routes, err := translateHTTPRouteRulesMetaToKongstateRoutes(tc.rulesMeta)
+			if tc.expectError {
+				require.Error(t, err)
+				return
+			}
+
+			require.NoError(t, err)
+			require.Len(t, routes, len(tc.expectedRoutes))
+			for i, expectedRoute := range tc.expectedRoutes {
+				require.Equal(t, expectedRoute, routes[i])
+			}
+		})
+	}
+}