Skip to content

Commit

Permalink
refactor: split HTTPRoute into SplitHTTPRouteMatch struct (#4434)
Browse files Browse the repository at this point in the history 
Refactor splitting of HTTPRoute: put each match of HTTPRoute
into SplitHTTPRouteMatch structure, instead of copying metadata
and split match into another HTTPRoute.
  • Loading branch information
@randmonkey
randmonkey committed Jul 31, 2023
1 parent 0dfd102 commit e686d34
Show file tree
Hide file tree
Showing 6 changed files with 525 additions and 751 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ Adding a new version? You'll need three changes:
them to expression based Kong routes. The assigning method follows the
[specification on priorities of matches in `HTTPRoute`][httproute-specification].
[#4296](https://github.com/Kong/kubernetes-ingress-controller/pull/4296)
[#4434](https://github.com/Kong/kubernetes-ingress-controller/pull/4434)
- Assign priorities to routes translated from GRPCRoutes when the parser translates
them to expression based Kong routes. The priority order follows the
[specification on match priorities in GRPCRoute][grpcroute-specification].
Expand Down
42 changes: 21 additions & 21 deletions internal/dataplane/parser/translate_httproute.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,25 +83,25 @@ func validateHTTPRoute(httproute *gatewayv1beta1.HTTPRoute) error {
// and finally translate the split HTTPRoutes into Kong services and routes with assigned priorities.
func (p *Parser) ingressRulesFromHTTPRoutesUsingExpressionRoutes(httpRoutes []*gatewayv1beta1.HTTPRoute, result *ingressRules) {
// first, split HTTPRoutes by hostnames and matches.
splitHTTPRoutes := []*gatewayv1beta1.HTTPRoute{}
splitHTTPRouteMatches := []translators.SplitHTTPRouteMatch{}
for _, httproute := range httpRoutes {
if err := validateHTTPRoute(httproute); err != nil {
p.registerTranslationFailure(fmt.Sprintf("HTTPRoute can't be routed: %s", err), httproute)
continue
}
splitHTTPRoutes = append(splitHTTPRoutes, translators.SplitHTTPRoute(httproute)...)
splitHTTPRouteMatches = append(splitHTTPRouteMatches, translators.SplitHTTPRoute(httproute)...)
}
// assign priorities to split HTTPRoutes.
splitHTTPRoutesWithPriorities := translators.AssignRoutePriorityToSplitHTTPRoutes(logrusr.New(p.logger), splitHTTPRoutes)
splitHTTPRoutesWithPriorities := translators.AssignRoutePriorityToSplitHTTPRouteMatches(logrusr.New(p.logger), splitHTTPRouteMatches)
httpRouteNameToTranslationFailure := map[k8stypes.NamespacedName][]error{}

// translate split HTTPRoutes to ingress rules, including services, routes, upstreams.
// translate split HTTPRoute matches to ingress rules, including services, routes, upstreams.
for _, httpRouteWithPriority := range splitHTTPRoutesWithPriorities {
err := p.ingressRulesFromSplitHTTPRouteWithPriority(result, httpRouteWithPriority)
err := p.ingressRulesFromSplitHTTPRouteMatchWithPriority(result, httpRouteWithPriority)
if err != nil {
nsName := k8stypes.NamespacedName{
Namespace: httpRouteWithPriority.HTTPRoute.Namespace,
Name: httpRouteWithPriority.HTTPRoute.Name,
Namespace: httpRouteWithPriority.Match.Source.Namespace,
Name: httpRouteWithPriority.Match.Source.Name,
}
httpRouteNameToTranslationFailure[nsName] = append(httpRouteNameToTranslationFailure[nsName], err)
}
Expand Down Expand Up @@ -527,23 +527,23 @@ func httpBackendRefsToBackendRefs(httpBackendRef []gatewayv1beta1.HTTPBackendRef
return backendRefs
}

func (p *Parser) ingressRulesFromSplitHTTPRouteWithPriority(
// ingressRulesFromSplitHTTPRouteMatchWithPriority translates a single match split from HTTPRoute
// to ingress rule, including Kong service and Kong route.
func (p *Parser) ingressRulesFromSplitHTTPRouteMatchWithPriority(
rules *ingressRules,
httpRouteWithPriority translators.SplitHTTPRouteToKongRoutePriority,
httpRouteMatchWithPriority translators.SplitHTTPRouteMatchToKongRoutePriority,
) error {
httpRoute := httpRouteWithPriority.HTTPRoute
if len(httpRoute.Spec.Rules) == 0 {
return translators.ErrRouteValidationNoRules
}

httpRouteRule := httpRoute.Spec.Rules[0]
if len(httpRoute.Spec.Hostnames) == 0 && len(httpRouteRule.Matches) == 0 {
return translators.ErrRouteValidationNoMatchRulesOrHostnamesSpecified
match := httpRouteMatchWithPriority.Match
httpRoute := httpRouteMatchWithPriority.Match.Source
if match.RuleIndex >= len(httpRoute.Spec.Rules) {
p.logger.Errorf("split match has rule index %d out of bound of rules in source HTTPRoute %d",
match.RuleIndex, len(httpRoute.Spec.Rules))
return nil
}

backendRefs := httpBackendRefsToBackendRefs(httpRouteRule.BackendRefs)

serviceName := translators.KongServiceNameFromHTTPRouteWithPriority(httpRouteWithPriority)
rule := httpRoute.Spec.Rules[match.RuleIndex]
backendRefs := httpBackendRefsToBackendRefs(rule.BackendRefs)
serviceName := translators.KongServiceNameFromSplitHTTPRouteMatch(httpRouteMatchWithPriority.Match)

kongService, err := generateKongServiceFromBackendRefWithName(
p.logger,
Expand All @@ -560,7 +560,7 @@ func (p *Parser) ingressRulesFromSplitHTTPRouteWithPriority(

kongService.Routes = append(
kongService.Routes,
translators.KongExpressionRouteFromHTTPRouteWithPriority(httpRouteWithPriority),
translators.KongExpressionRouteFromHTTPRouteMatchWithPriority(httpRouteMatchWithPriority),
)
// cache the service to avoid duplicates in further loop iterations
rules.ServiceNameToServices[serviceName] = kongService
Expand Down
165 changes: 84 additions & 81 deletions internal/dataplane/parser/translate_httproute_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1825,7 +1825,7 @@ func TestIngressRulesFromHTTPRoutesUsingExpressionRoutes(t *testing.T) {
"should have expected number of services")
for _, expectedKongService := range tc.expectedKongServices {
kongService, ok := result.ServiceNameToServices[*expectedKongService.Name]
require.Truef(t, ok, "should find service %s", expectedKongService.Name)
require.Truef(t, ok, "should find service %s", *expectedKongService.Name)
require.Equal(t, expectedKongService.Backends, kongService.Backends)
// check routes
expectedKongRoutes := tc.expectedKongRoutes[*kongService.Name]
Expand Down Expand Up @@ -1854,7 +1854,7 @@ func TestIngressRulesFromHTTPRoutesUsingExpressionRoutes(t *testing.T) {
}
}

func TestIngressRulesWithPriority(t *testing.T) {
func TestIngressRulesFromSplitHTTPRouteMatchWithPriority(t *testing.T) {
fakestore, err := store.NewFakeStore(store.FakeObjects{})
require.NoError(t, err)
parser := mustNewParser(t, fakestore)
Expand All @@ -1863,37 +1863,38 @@ func TestIngressRulesWithPriority(t *testing.T) {
httpRouteTypeMeta := metav1.TypeMeta{Kind: "HTTPRoute", APIVersion: gatewayv1beta1.SchemeGroupVersion.String()}

testCases := []struct {
name string
httpRouteWithPriority translators.SplitHTTPRouteToKongRoutePriority
expectedKongService kongstate.Service
expectedKongRoute kongstate.Route
expectedError error
name string
matchWithPriority translators.SplitHTTPRouteMatchToKongRoutePriority
expectedKongService kongstate.Service
expectedKongRoute kongstate.Route
expectedError error
}{
{
name: "no hostname",
httpRouteWithPriority: translators.SplitHTTPRouteToKongRoutePriority{
HTTPRoute: &gatewayv1beta1.HTTPRoute{
TypeMeta: httpRouteTypeMeta,
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
Name: "httproute-1",
Annotations: map[string]string{
translators.InternalRuleIndexAnnotationKey: "0",
translators.InternalMatchIndexAnnotationKey: "0",
matchWithPriority: translators.SplitHTTPRouteMatchToKongRoutePriority{
Match: translators.SplitHTTPRouteMatch{
Source: &gatewayv1beta1.HTTPRoute{
TypeMeta: httpRouteTypeMeta,
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
Name: "httproute-1",
},
},
Spec: gatewayv1beta1.HTTPRouteSpec{
Rules: []gatewayv1beta1.HTTPRouteRule{
{
Matches: []gatewayv1beta1.HTTPRouteMatch{
builder.NewHTTPRouteMatch().WithPathExact("/v1/foo").Build(),
},
BackendRefs: []gatewayv1beta1.HTTPBackendRef{
builder.NewHTTPBackendRef("service1").WithPort(80).Build(),
Spec: gatewayv1beta1.HTTPRouteSpec{
Rules: []gatewayv1beta1.HTTPRouteRule{
{
Matches: []gatewayv1beta1.HTTPRouteMatch{
builder.NewHTTPRouteMatch().WithPathExact("/v1/foo").Build(),
},
BackendRefs: []gatewayv1beta1.HTTPBackendRef{
builder.NewHTTPBackendRef("service1").WithPort(80).Build(),
},
},
},
},
},
Match: builder.NewHTTPRouteMatch().WithPathExact("/v1/foo").Build(),
RuleIndex: 0,
MatchIndex: 0,
},
Priority: 1024,
},
Expand Down Expand Up @@ -1922,38 +1923,41 @@ func TestIngressRulesWithPriority(t *testing.T) {
},
{
name: "precise hostname and filter",
httpRouteWithPriority: translators.SplitHTTPRouteToKongRoutePriority{
HTTPRoute: &gatewayv1beta1.HTTPRoute{
TypeMeta: httpRouteTypeMeta,
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
Name: "httproute-1",
Annotations: map[string]string{
translators.InternalRuleIndexAnnotationKey: "0",
translators.InternalMatchIndexAnnotationKey: "1",
},
},
Spec: gatewayv1beta1.HTTPRouteSpec{
Hostnames: []gatewayv1beta1.Hostname{
"foo.com",
matchWithPriority: translators.SplitHTTPRouteMatchToKongRoutePriority{
Match: translators.SplitHTTPRouteMatch{
Source: &gatewayv1beta1.HTTPRoute{
TypeMeta: httpRouteTypeMeta,
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
Name: "httproute-1",
},
Rules: []gatewayv1beta1.HTTPRouteRule{
{
Matches: []gatewayv1beta1.HTTPRouteMatch{
builder.NewHTTPRouteMatch().WithPathExact("/v1/foo").Build(),
},
BackendRefs: []gatewayv1beta1.HTTPBackendRef{
builder.NewHTTPBackendRef("service1").WithPort(80).Build(),
},
Filters: []gatewayv1beta1.HTTPRouteFilter{
builder.NewHTTPRouteRequestRedirectFilter().
WithRequestRedirectStatusCode(301).
WithRequestRedirectHost("bar.com").
Build(),
Spec: gatewayv1beta1.HTTPRouteSpec{
Hostnames: []gatewayv1beta1.Hostname{
"foo.com",
},
Rules: []gatewayv1beta1.HTTPRouteRule{
{
Matches: []gatewayv1beta1.HTTPRouteMatch{
builder.NewHTTPRouteMatch().WithPathExact("/foo").Build(),
builder.NewHTTPRouteMatch().WithPathExact("/v1/foo").Build(),
},
BackendRefs: []gatewayv1beta1.HTTPBackendRef{
builder.NewHTTPBackendRef("service1").WithPort(80).Build(),
},
Filters: []gatewayv1beta1.HTTPRouteFilter{
builder.NewHTTPRouteRequestRedirectFilter().
WithRequestRedirectStatusCode(301).
WithRequestRedirectHost("bar.com").
Build(),
},
},
},
},
},
Hostname: "foo.com",
Match: builder.NewHTTPRouteMatch().WithPathExact("/v1/foo").Build(),
RuleIndex: 0,
MatchIndex: 1,
},
Priority: 1024,
},
Expand Down Expand Up @@ -1997,33 +2001,35 @@ func TestIngressRulesWithPriority(t *testing.T) {
},
{
name: "wildcard hostname with multiple backends",
httpRouteWithPriority: translators.SplitHTTPRouteToKongRoutePriority{
HTTPRoute: &gatewayv1beta1.HTTPRoute{
TypeMeta: httpRouteTypeMeta,
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
Name: "httproute-1",
Annotations: map[string]string{
translators.InternalRuleIndexAnnotationKey: "0",
translators.InternalMatchIndexAnnotationKey: "0",
},
},
Spec: gatewayv1beta1.HTTPRouteSpec{
Hostnames: []gatewayv1beta1.Hostname{
"*.foo.com",
matchWithPriority: translators.SplitHTTPRouteMatchToKongRoutePriority{
Match: translators.SplitHTTPRouteMatch{
Source: &gatewayv1beta1.HTTPRoute{
TypeMeta: httpRouteTypeMeta,
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
Name: "httproute-1",
},
Rules: []gatewayv1beta1.HTTPRouteRule{
{
Matches: []gatewayv1beta1.HTTPRouteMatch{
builder.NewHTTPRouteMatch().WithPathExact("/v1/foo").Build(),
},
BackendRefs: []gatewayv1beta1.HTTPBackendRef{
builder.NewHTTPBackendRef("service1").WithPort(80).WithWeight(10).Build(),
builder.NewHTTPBackendRef("service2").WithPort(80).WithWeight(20).Build(),
Spec: gatewayv1beta1.HTTPRouteSpec{
Hostnames: []gatewayv1beta1.Hostname{
"*.foo.com",
},
Rules: []gatewayv1beta1.HTTPRouteRule{
{
Matches: []gatewayv1beta1.HTTPRouteMatch{
builder.NewHTTPRouteMatch().WithPathExact("/v1/foo").Build(),
},
BackendRefs: []gatewayv1beta1.HTTPBackendRef{
builder.NewHTTPBackendRef("service1").WithPort(80).WithWeight(10).Build(),
builder.NewHTTPBackendRef("service2").WithPort(80).WithWeight(20).Build(),
},
},
},
},
},
Hostname: "*.foo.com",
Match: builder.NewHTTPRouteMatch().WithPathExact("/v1/foo").Build(),
RuleIndex: 0,
MatchIndex: 0,
},
Priority: 1024,
},
Expand Down Expand Up @@ -2061,11 +2067,12 @@ func TestIngressRulesWithPriority(t *testing.T) {
for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
tc.expectedKongRoute.Tags = util.GenerateTagsForObject(tc.httpRouteWithPriority.HTTPRoute)
tc.expectedKongRoute.Ingress = util.FromK8sObject(tc.httpRouteWithPriority.HTTPRoute)
match := tc.matchWithPriority.Match
tc.expectedKongRoute.Tags = util.GenerateTagsForObject(match.Source)
tc.expectedKongRoute.Ingress = util.FromK8sObject(match.Source)

res := newIngressRules()
err := parser.ingressRulesFromSplitHTTPRouteWithPriority(&res, tc.httpRouteWithPriority)
err := parser.ingressRulesFromSplitHTTPRouteMatchWithPriority(&res, tc.matchWithPriority)
if tc.expectedError != nil {
require.ErrorAs(t, err, tc.expectedError)
return
Expand All @@ -2080,10 +2087,6 @@ func TestIngressRulesWithPriority(t *testing.T) {
}
}

func HTTPMethodPointer(method gatewayv1beta1.HTTPMethod) *gatewayv1beta1.HTTPMethod {
return &method
}

func k8sObjectInfoOfHTTPRoute(route *gatewayv1beta1.HTTPRoute) util.K8sObjectInfo {
// parsers always provide the annotations map, even if route didn't have any
anotations := route.Annotations
Expand Down
12 changes: 2 additions & 10 deletions internal/dataplane/parser/translators/grpcroute_atc_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -829,12 +829,8 @@ func TestAssignRoutePriorityToSplitGRPCRouteMatches(t *testing.T) {
{
Source: &gatewayv1alpha2.GRPCRoute{
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
Name: "grpcroute-2",
Annotations: map[string]string{
InternalRuleIndexAnnotationKey: "0",
InternalMatchIndexAnnotationKey: "0",
},
Namespace: "default",
Name: "grpcroute-2",
CreationTimestamp: metav1.NewTime(now.Add(-5 * time.Second)),
},
Spec: gatewayv1alpha2.GRPCRouteSpec{
Expand Down Expand Up @@ -1090,10 +1086,6 @@ func TestKongExpressionRouteFromSplitGRPCRouteWithPriority(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
Name: "precise-hostname-regex-method",
Annotations: map[string]string{
InternalRuleIndexAnnotationKey: "0",
InternalMatchIndexAnnotationKey: "0",
},
},
Spec: gatewayv1alpha2.GRPCRouteSpec{
Hostnames: []gatewayv1alpha2.Hostname{
Expand Down
Loading

0 comments on commit e686d34

Please sign in to comment.