Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
randmonkey committed May 10, 2023
1 parent e23b288 commit 1f6f91c
Show file tree
Hide file tree
Showing 9 changed files with 76 additions and 77 deletions.
11 changes: 0 additions & 11 deletions internal/dataplane/parser/translate_errors.go

This file was deleted.

3 changes: 2 additions & 1 deletion internal/dataplane/parser/translate_grpcroute.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"

"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/kongstate"
"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/parser/translators"
"github.com/kong/kubernetes-ingress-controller/v2/internal/util"
)

Expand Down Expand Up @@ -53,7 +54,7 @@ func (p *Parser) ingressRulesFromGRPCRoute(result *ingressRules, grpcroute *gate
spec := grpcroute.Spec

if len(spec.Rules) == 0 {
return errRouteValidationNoRules
return translators.ErrRouteValidationNoRules
}

// each rule may represent a different set of backend services that will be accepting
Expand Down
26 changes: 10 additions & 16 deletions internal/dataplane/parser/translate_httproute.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func validateHTTPRoute(httproute *gatewayv1beta1.HTTPRoute) error {
// are invalid somehow make it past validation (e.g. the webhook is not enabled) we can
// at least try to provide a helpful message about the situation in the manager logs.
if len(spec.Rules) == 0 {
return errRouteValidationNoRules
return translators.ErrRouteValidationNoRules
}

return nil
Expand Down Expand Up @@ -137,22 +137,18 @@ func (p *Parser) ingressRulesFromHTTPRouteLegacyFallback(httproute *gatewayv1bet
// HTTPRoute specification into a []*string slice, which is the type required by translating to matchers
// in expression based routes.
func getHTTPRouteHostnamesAsSliceOfStrings(httproute *gatewayv1beta1.HTTPRoute) []string {
hostnames := make([]string, 0, len(httproute.Spec.Hostnames))
for _, hostname := range httproute.Spec.Hostnames {
hostnames = append(hostnames, string(hostname))
}
return hostnames
return lo.Map(httproute.Spec.Hostnames, func(h gatewayv1beta1.Hostname, _ int) string {
return string(h)
})
}

// getHTTPRouteHostnamesAsSliceOfStringPointers translates the hostnames defined
// in an HTTPRoute specification into a []*string slice, which is the type required
// by kong.Route{}.
func getHTTPRouteHostnamesAsSliceOfStringPointers(httproute *gatewayv1beta1.HTTPRoute) []*string {
hostnames := make([]*string, 0, len(httproute.Spec.Hostnames))
for _, hostname := range httproute.Spec.Hostnames {
hostnames = append(hostnames, kong.String(string(hostname)))
}
return hostnames
return lo.Map(httproute.Spec.Hostnames, func(h gatewayv1beta1.Hostname, _ int) *string {
return kong.String(string(h))
})
}

// generateKongRoutesFromHTTPRouteRule converts an HTTPRoute rule to one or more
Expand Down Expand Up @@ -242,9 +238,7 @@ func generateKongRouteFromTranslation(
// get the hostnames from the HTTPRoute
hostnames := getHTTPRouteHostnamesAsSliceOfStrings(httproute)
return translators.GenerateKongExpressionRoutesFromHTTPRouteMatches(
translation.Name,
translation.Matches,
translation.Filters,
translation,
objectInfo,
hostnames,
tags,
Expand Down Expand Up @@ -294,7 +288,7 @@ func generateKongRoutesFromHTTPRouteMatches(
// however in this case there must actually be some present hostnames
// configured for the HTTPRoute or else it's not valid.
if len(hostnames) == 0 {
return []kongstate.Route{}, errRouteValidationNoMatchRulesOrHostnamesSpecified
return []kongstate.Route{}, translators.ErrRouteValidationNoMatchRulesOrHostnamesSpecified
}

// otherwise apply the hostnames to the route
Expand All @@ -305,7 +299,7 @@ func generateKongRoutesFromHTTPRouteMatches(

// TODO: implement query param matches (https:/Kong/kubernetes-ingress-controller/issues/2778)
if len(matches[0].QueryParams) > 0 {
return []kongstate.Route{}, errRouteValidationQueryParamMatchesUnsupported
return []kongstate.Route{}, translators.ErrRouteValidationQueryParamMatchesUnsupported
}

r := generateKongstateHTTPRoute(routeName, ingressObjectInfo, hostnames)
Expand Down
7 changes: 4 additions & 3 deletions internal/dataplane/parser/translate_httproute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"

"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/kongstate"
"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/parser/translators"
"github.com/kong/kubernetes-ingress-controller/v2/internal/store"
"github.com/kong/kubernetes-ingress-controller/v2/internal/util"
"github.com/kong/kubernetes-ingress-controller/v2/internal/util/builder"
Expand Down Expand Up @@ -143,7 +144,7 @@ func getIngressRulesFromHTTPRoutesCommonTestCases() []testCaseIngressRulesFromHT
}
},
errs: []error{
errRouteValidationNoMatchRulesOrHostnamesSpecified,
translators.ErrRouteValidationNoMatchRulesOrHostnamesSpecified,
},
},
{
Expand Down Expand Up @@ -233,7 +234,7 @@ func getIngressRulesFromHTTPRoutesCommonTestCases() []testCaseIngressRulesFromHT
}
},
errs: []error{
errRouteValidationNoRules,
translators.ErrRouteValidationNoRules,
},
},
{
Expand Down Expand Up @@ -263,7 +264,7 @@ func getIngressRulesFromHTTPRoutesCommonTestCases() []testCaseIngressRulesFromHT
}
},
errs: []error{
errRouteValidationQueryParamMatchesUnsupported,
translators.ErrRouteValidationQueryParamMatchesUnsupported,
},
},
{
Expand Down
4 changes: 3 additions & 1 deletion internal/dataplane/parser/translate_tcproute.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"fmt"

gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"

"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/parser/translators"
)

// -----------------------------------------------------------------------------
Expand Down Expand Up @@ -51,7 +53,7 @@ func (p *Parser) ingressRulesFromTCPRoute(result *ingressRules, tcproute *gatewa
// are invalid somehow make it past validation (e.g. the webhook is not enabled) we can
// at least try to provide a helpful message about the situation in the manager logs.
if len(spec.Rules) == 0 {
return errRouteValidationNoRules
return translators.ErrRouteValidationNoRules
}

// each rule may represent a different set of backend services that will be accepting
Expand Down
3 changes: 2 additions & 1 deletion internal/dataplane/parser/translate_tlsroute.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"

"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/parser/translators"
"github.com/kong/kubernetes-ingress-controller/v2/internal/store"
)

Expand Down Expand Up @@ -55,7 +56,7 @@ func (p *Parser) ingressRulesFromTLSRoute(result *ingressRules, tlsroute *gatewa
return fmt.Errorf("no hostnames provided")
}
if len(spec.Rules) == 0 {
return errRouteValidationNoRules
return translators.ErrRouteValidationNoRules
}

tlsPassthrough, err := p.isTLSRoutePassthrough(tlsroute)
Expand Down
4 changes: 3 additions & 1 deletion internal/dataplane/parser/translate_udproute.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"fmt"

gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"

"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/parser/translators"
)

// -----------------------------------------------------------------------------
Expand Down Expand Up @@ -51,7 +53,7 @@ func (p *Parser) ingressRulesFromUDPRoute(result *ingressRules, udproute *gatewa
// are invalid somehow make it past validation (e.g. the webhook is not enabled) we can
// at least try to provide a helpful message about the situation in the manager logs.
if len(spec.Rules) == 0 {
return errRouteValidationNoRules
return translators.ErrRouteValidationNoRules
}

// each rule may represent a different set of backend services that will be accepting
Expand Down
87 changes: 47 additions & 40 deletions internal/dataplane/parser/translators/httproute_atc.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,7 @@ import (
// GenerateKongExpressionRoutesFromHTTPRouteMatches generates Kong routes from HTTPRouteRule
// pointing to a specific backend.
func GenerateKongExpressionRoutesFromHTTPRouteMatches(
routeName string,
matches []gatewayv1beta1.HTTPRouteMatch,
filters []gatewayv1beta1.HTTPRouteFilter,
translation KongRouteTranslation,
ingressObjectInfo util.K8sObjectInfo,
hostnames []string,
tags []*string,
Expand All @@ -28,14 +26,14 @@ func GenerateKongExpressionRoutesFromHTTPRouteMatches(
r := kongstate.Route{
Ingress: ingressObjectInfo,
Route: kong.Route{
Name: kong.String(routeName),
Name: kong.String(translation.Name),
PreserveHost: kong.Bool(true),
Tags: tags,
},
ExpressionRoutes: true,
}

if len(matches) == 0 {
if len(translation.Matches) == 0 {
if len(hostnames) == 0 {
return []kongstate.Route{}, ErrRouteValidationNoMatchRulesOrHostnamesSpecified
}
Expand All @@ -45,48 +43,17 @@ func GenerateKongExpressionRoutesFromHTTPRouteMatches(
return []kongstate.Route{r}, nil
}

_, hasRedirectFilter := lo.Find(filters, func(filter gatewayv1beta1.HTTPRouteFilter) bool {
_, hasRedirectFilter := lo.Find(translation.Filters, func(filter gatewayv1beta1.HTTPRouteFilter) bool {
return filter.Type == gatewayv1beta1.HTTPRouteFilterRequestRedirect
})
// if the rule has request redirect filter(s), we need to generate a route for each match to
// attach plugins for the filter.
if hasRedirectFilter {
routes := make([]kongstate.Route, 0, len(matches))
for _, match := range matches {
matchRoute := kongstate.Route{
Ingress: ingressObjectInfo,
Route: kong.Route{
Name: kong.String(routeName),
PreserveHost: kong.Bool(true),
Tags: tags,
},
ExpressionRoutes: true,
}
// generate matcher for this HTTPRoute Match.
matcher := atc.And(generateMatcherFromHTTPRouteMatch(match))

// add matcher from parent httproute (hostnames, protocols, SNIs) to be ANDed with the matcher from match.
matchersFromParent := matchersFromParentHTTPRoute(hostnames, ingressObjectInfo.Annotations)
for _, m := range matchersFromParent {
matcher.And(m)
}
atc.ApplyExpression(&matchRoute.Route, matcher, 1)

// we need to extract the path to configure redirect path of the plugins for request redirect filter.
path := ""
if match.Path != nil && match.Path.Value != nil {
path = *match.Path.Value
}
plugins := GeneratePluginsFromHTTPRouteFilters(filters, path, tags)
matchRoute.Plugins = plugins

routes = append(routes, matchRoute)
}
return routes, nil
return generateKongExpressionRoutesWithRequestRedirectFilter(translation, ingressObjectInfo, hostnames, tags)
}

// if we do not need to generate a kong route for each match, we OR matchers from all matches together.
routeMatcher := atc.And(atc.Or(generateMatchersFromHTTPRouteMatches(matches)...))
routeMatcher := atc.And(atc.Or(generateMatchersFromHTTPRouteMatches(translation.Matches)...))
// add matcher from parent httproute (hostnames, protocols, SNIs) to be ANDed with the matcher from match.
matchersFromParent := matchersFromParentHTTPRoute(hostnames, ingressObjectInfo.Annotations)
for _, matcher := range matchersFromParent {
Expand All @@ -95,11 +62,51 @@ func GenerateKongExpressionRoutesFromHTTPRouteMatches(

atc.ApplyExpression(&r.Route, routeMatcher, 1)
// generate plugins.
plugins := GeneratePluginsFromHTTPRouteFilters(filters, "", tags)
plugins := GeneratePluginsFromHTTPRouteFilters(translation.Filters, "", tags)
r.Plugins = plugins
return []kongstate.Route{r}, nil
}

func generateKongExpressionRoutesWithRequestRedirectFilter(
translation KongRouteTranslation,
ingressObjectInfo util.K8sObjectInfo,
hostnames []string,
tags []*string,
) ([]kongstate.Route, error) {
routes := make([]kongstate.Route, 0, len(translation.Matches))
for _, match := range translation.Matches {
matchRoute := kongstate.Route{
Ingress: ingressObjectInfo,
Route: kong.Route{
Name: kong.String(translation.Name),
PreserveHost: kong.Bool(true),
Tags: tags,
},
ExpressionRoutes: true,
}
// generate matcher for this HTTPRoute Match.
matcher := atc.And(generateMatcherFromHTTPRouteMatch(match))

// add matcher from parent httproute (hostnames, protocols, SNIs) to be ANDed with the matcher from match.
matchersFromParent := matchersFromParentHTTPRoute(hostnames, ingressObjectInfo.Annotations)
for _, m := range matchersFromParent {
matcher.And(m)
}
atc.ApplyExpression(&matchRoute.Route, matcher, 1)

// we need to extract the path to configure redirect path of the plugins for request redirect filter.
path := ""
if match.Path != nil && match.Path.Value != nil {
path = *match.Path.Value
}
plugins := GeneratePluginsFromHTTPRouteFilters(translation.Filters, path, tags)
matchRoute.Plugins = plugins

routes = append(routes, matchRoute)
}
return routes, nil
}

func generateMatchersFromHTTPRouteMatches(matches []gatewayv1beta1.HTTPRouteMatch) []atc.Matcher {
ret := make([]atc.Matcher, 0, len(matches))
for _, match := range matches {
Expand Down
8 changes: 5 additions & 3 deletions internal/dataplane/parser/translators/httproute_atc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,9 +234,11 @@ func TestGenerateKongExpressionRoutesFromHTTPRouteMatches(t *testing.T) {
tc := tc
t.Run(tc.name, func(t *testing.T) {
routes, err := GenerateKongExpressionRoutesFromHTTPRouteMatches(
tc.routeName,
tc.matches,
tc.filters,
KongRouteTranslation{
Name: tc.routeName,
Matches: tc.matches,
Filters: tc.filters,
},
tc.ingressObjectInfo,
tc.hostnames,
kong.StringSlice(tc.tags...),
Expand Down

0 comments on commit 1f6f91c

Please sign in to comment.