Skip to content

Commit

Permalink
feat: gernerate a catch-all route for rules with no matches and no ho…
Browse files Browse the repository at this point in the history 
…stname in HTTPRoute and GRPCRoute
  • Loading branch information
@randmonkey @rainest
randmonkey authored and rainest committed Aug 18, 2023
1 parent 00b2821 commit f299b4f
Show file tree
Hide file tree
Showing 8 changed files with 207 additions and 35 deletions.
7 changes: 0 additions & 7 deletions internal/dataplane/parser/translate_grpcroute.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,13 +168,6 @@ func validateGRPCRoute(grpcRoute *gatewayv1alpha2.GRPCRoute) error {
if len(grpcRoute.Spec.Rules) == 0 {
return translators.ErrRouteValidationNoRules
}
// REVIEW: remove this to generate a "catch-all" route from rule with no matches and no hostnames in its parent GRPCRoute.
for _, rule := range grpcRoute.Spec.Rules {
if len(rule.Matches) == 0 {
return translators.ErrRouteValidationNoMatchRulesOrHostnamesSpecified
}
}
}

return nil
}
27 changes: 19 additions & 8 deletions internal/dataplane/parser/translate_grpcroute_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,17 @@ func TestIngressRulesFromGRPCRoutesUsingExpressionRoutes(t *testing.T) {
},
},
},
{
Service: kong.Service{
Name: kong.String("grpcroute.default.grpcroute-no-hostnames-no-matches._.0"),
},
Backends: []kongstate.ServiceBackend{
{
Name: "service0",
PortDef: kongstate.PortDef{Mode: kongstate.PortModeByNumber, Number: int32(80)},
},
},
},
},
expectedKongRoutes: map[string][]kongstate.Route{
"grpcroute.default.grpcroute-1._.0": {
Expand All @@ -369,6 +380,14 @@ func TestIngressRulesFromGRPCRoutesUsingExpressionRoutes(t *testing.T) {
},
},
},
"grpcroute.default.grpcroute-no-hostnames-no-matches._.0": {
{
Route: kong.Route{
Name: kong.String("grpcroute.default.grpcroute-no-hostnames-no-matches._.0.0"),
Expression: kong.String(translators.CatchAllHTTPExpression),
},
},
},
},
expectedFailures: []failures.ResourceFailure{
newResourceFailure(translators.ErrRouteValidationNoRules.Error(),
Expand All @@ -379,14 +398,6 @@ func TestIngressRulesFromGRPCRoutesUsingExpressionRoutes(t *testing.T) {
Name: "grpcroute-no-rules",
},
}),
newResourceFailure(translators.ErrRouteValidationNoMatchRulesOrHostnamesSpecified.Error(),
&gatewayv1alpha2.GRPCRoute{
TypeMeta: grpcRouteTypeMeta,
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
Name: "grpcroute-no-hostnames-no-matches",
},
}),
},
},
}
Expand Down
9 changes: 1 addition & 8 deletions internal/dataplane/parser/translate_httproute.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,7 @@ func generateKongRoutesFromHTTPRouteMatches(
// but only backends as long as there are hostnames. In this case, we
// match all traffic based on the hostname and leave all other routing
// options default.
// for rules with no hostnames, we generate a "catch-all" route for it.
r := kongstate.Route{
Ingress: ingressObjectInfo,
Route: kong.Route{
Expand All @@ -348,14 +349,6 @@ func generateKongRoutesFromHTTPRouteMatches(
Tags: tags,
},
}

// 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{}, translators.ErrRouteValidationNoMatchRulesOrHostnamesSpecified
}

// otherwise apply the hostnames to the route
r.Hosts = append(r.Hosts, hostnames...)

return []kongstate.Route{r}, nil
Expand Down
153 changes: 145 additions & 8 deletions internal/dataplane/parser/translate_httproute_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func getIngressRulesFromHTTPRoutesCommonTestCases() []testCaseIngressRulesFromHT
},
},
{
msg: "an HTTPRoute rule with no matches and no hostnames can't be routed",
msg: "an HTTPRoute rule with no matches and no hostnames produces a catch-all rule",
routes: []*gatewayv1beta1.HTTPRoute{{
ObjectMeta: metav1.ObjectMeta{
Name: "basic-httproute",
Expand All @@ -141,14 +141,48 @@ func getIngressRulesFromHTTPRoutesCommonTestCases() []testCaseIngressRulesFromHT
}},
expected: func(routes []*gatewayv1beta1.HTTPRoute) ingressRules {
return ingressRules{
SecretNameToSNIs: newSecretNameToSNIs(),
ServiceNameToParent: map[string]client.Object{},
ServiceNameToServices: make(map[string]kongstate.Service),
SecretNameToSNIs: newSecretNameToSNIs(),
ServiceNameToParent: map[string]client.Object{
"httproute.default.basic-httproute.0": routes[0],
},
ServiceNameToServices: map[string]kongstate.Service{
"httproute.default.basic-httproute.0": {
Service: kong.Service{ // only 1 service should be created
ConnectTimeout: kong.Int(60000),
Host: kong.String("httproute.default.basic-httproute.0"),
Name: kong.String("httproute.default.basic-httproute.0"),
Protocol: kong.String("http"),
ReadTimeout: kong.Int(60000),
Retries: kong.Int(5),
WriteTimeout: kong.Int(60000),
},
Backends: kongstate.ServiceBackends{
builder.NewKongstateServiceBackend("fake-service").WithPortNumber(80).Build(),
},
Namespace: "default",
Routes: []kongstate.Route{{ // only 1 route should be created
Route: kong.Route{
Name: kong.String("httproute.default.basic-httproute.0.0"),
PreserveHost: kong.Bool(true),
Protocols: []*string{
kong.String("http"),
kong.String("https"),
},
Tags: []*string{
kong.String("k8s-name:basic-httproute"),
kong.String("k8s-namespace:default"),
kong.String("k8s-kind:HTTPRoute"),
kong.String("k8s-group:gateway.networking.k8s.io"),
kong.String("k8s-version:v1beta1"),
},
},
Ingress: k8sObjectInfoOfHTTPRoute(routes[0]),
}},
Parent: routes[0],
},
},
}
},
errs: []error{
translators.ErrRouteValidationNoMatchRulesOrHostnamesSpecified,
},
},
{
msg: "a single HTTPRoute with one match and one backendRef results in a single service",
Expand Down Expand Up @@ -2062,6 +2096,109 @@ func TestIngressRulesFromSplitHTTPRouteMatchWithPriority(t *testing.T) {
ExpressionRoutes: true,
},
},
{
name: "precise hostname and no match",
matchWithPriority: translators.SplitHTTPRouteMatchToKongRoutePriority{
Match: translators.SplitHTTPRouteMatch{
Source: &gatewayv1beta1.HTTPRoute{
TypeMeta: httpRouteTypeMeta,
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
Name: "httproute-1",
},
Spec: gatewayv1beta1.HTTPRouteSpec{
Hostnames: []gatewayv1beta1.Hostname{
"a.foo.com",
},
Rules: []gatewayv1beta1.HTTPRouteRule{
{
BackendRefs: []gatewayv1beta1.HTTPBackendRef{
builder.NewHTTPBackendRef("service1").WithPort(80).Build(),
},
},
},
},
},
Hostname: "a.foo.com",
Match: builder.NewHTTPRouteMatch().Build(),
RuleIndex: 0,
MatchIndex: 0,
},
Priority: 1024,
},
expectedKongService: kongstate.Service{
Service: kong.Service{
Name: kong.String("httproute.default.httproute-1.a.foo.com.0"),
},
Backends: []kongstate.ServiceBackend{
{
Name: "service1",
PortDef: kongstate.PortDef{Mode: kongstate.PortModeByNumber, Number: int32(80)},
},
},
},
expectedKongRoute: kongstate.Route{
Route: kong.Route{
Name: kong.String("httproute.default.httproute-1.a.foo.com.0.0"),
Expression: kong.String(`http.host == "a.foo.com"`),
PreserveHost: kong.Bool(true),
StripPath: kong.Bool(false),
Priority: kong.Int(1024),
},
Plugins: []kong.Plugin{},
ExpressionRoutes: true,
},
},
{
name: "no hostname and no match",
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{
{
BackendRefs: []gatewayv1beta1.HTTPBackendRef{
builder.NewHTTPBackendRef("service1").WithPort(80).Build(),
},
},
},
},
},
Hostname: "",
Match: builder.NewHTTPRouteMatch().Build(),
RuleIndex: 0,
MatchIndex: 0,
},
Priority: 1024,
},
expectedKongService: kongstate.Service{
Service: kong.Service{
Name: kong.String("httproute.default.httproute-1._.0"),
},
Backends: []kongstate.ServiceBackend{
{
Name: "service1",
PortDef: kongstate.PortDef{Mode: kongstate.PortModeByNumber, Number: int32(80)},
},
},
},
expectedKongRoute: kongstate.Route{
Route: kong.Route{
Name: kong.String("httproute.default.httproute-1._.0.0"),
Expression: kong.String(translators.CatchAllHTTPExpression),
PreserveHost: kong.Bool(true),
StripPath: kong.Bool(false),
Priority: kong.Int(1024),
},
Plugins: []kong.Plugin{},
ExpressionRoutes: true,
},
},
}

for _, tc := range testCases {
Expand All @@ -2079,7 +2216,7 @@ func TestIngressRulesFromSplitHTTPRouteMatchWithPriority(t *testing.T) {
}
require.NoError(t, err)
kongService, ok := res.ServiceNameToServices[*tc.expectedKongService.Name]
require.True(t, ok)
require.Truef(t, ok, "should find service %s", *tc.expectedKongService.Name)
require.Equal(t, tc.expectedKongService.Backends, kongService.Backends)
require.Len(t, kongService.Routes, 1)
require.Equal(t, tc.expectedKongRoute, kongService.Routes[0])
Expand Down
10 changes: 10 additions & 0 deletions internal/dataplane/parser/translators/atc_utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,16 @@ const (
ResourceKindBitsGRPCRoute = 1
)

const (
// CatchAllHTTPExpression is the expression to match all HTTP/HTTPS requests.
// For rules with no matches and no hostnames in its parent HTTPRoute or GRPCRoute,
// we need to generate a "catch-all" route for the rule:
// https://github.com/Kong/kubernetes-ingress-controller/issues/4526
// but Kong does not allow empty expression in expression router,
// so we define this is we need a route to match all HTTP/HTTPS requests.
CatchAllHTTPExpression = `(net.protocol == "http") || (net.protocol == "https")`
)

// -----------------------------------------------------------------------------
// Translator - common functions in translating expression(ATC) routes from multiple kinds of k8s objects.
// -----------------------------------------------------------------------------
Expand Down
4 changes: 4 additions & 0 deletions internal/dataplane/parser/translators/grpcroute_atc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,10 @@ func KongExpressionRouteFromSplitGRPCRouteMatchWithPriority(
grpcRoute.Annotations,
)
atc.ApplyExpression(&r.Route, matcher, matchWithPriority.Priority)
if r.Expression == nil || len(*r.Expression) == 0 {
r.Expression = kong.String(CatchAllHTTPExpression)
r.Priority = &matchWithPriority.Priority
}

return r
}
Expand Down
19 changes: 17 additions & 2 deletions internal/dataplane/parser/translators/httproute_atc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ func GenerateKongExpressionRoutesFromHTTPRouteMatches(

if len(translation.Matches) == 0 {
if len(hostnames) == 0 {
return []kongstate.Route{}, ErrRouteValidationNoMatchRulesOrHostnamesSpecified
r.Expression = kong.String(CatchAllHTTPExpression)
return []kongstate.Route{r}, nil
}

hostMatcher := hostMatcherFromHosts(hostnames)
atc.ApplyExpression(&r.Route, hostMatcher, 1)
return []kongstate.Route{r}, nil
Expand Down Expand Up @@ -238,6 +238,15 @@ func SplitHTTPRoute(httproute *gatewayv1beta1.HTTPRoute) []SplitHTTPRouteMatch {
splitHTTPRouteByMatch := func(hostname string) []SplitHTTPRouteMatch {
ret := []SplitHTTPRouteMatch{}
for ruleIndex, rule := range httproute.Spec.Rules {
if len(rule.Matches) == 0 {
ret = append(ret, SplitHTTPRouteMatch{
Source: httproute,
Hostname: hostname,
Match: gatewayv1beta1.HTTPRouteMatch{},
RuleIndex: ruleIndex,
MatchIndex: 0,
})
}
for matchIndex, match := range rule.Matches {
ret = append(ret, SplitHTTPRouteMatch{
Source: httproute,
Expand Down Expand Up @@ -537,6 +546,12 @@ func KongExpressionRouteFromHTTPRouteMatchWithPriority(

atc.ApplyExpression(&r.Route, atc.And(matchers...), httpRouteMatchWithPriority.Priority)

// generate a "catch-all" route if the generated expression is empty.
if r.Expression == nil || len(*r.Expression) == 0 {
r.Expression = kong.String(CatchAllHTTPExpression)
r.Priority = kong.Int(httpRouteMatchWithPriority.Priority)
}

// translate filters in the rule.
if match.RuleIndex < len(httproute.Spec.Rules) {
rule := httproute.Spec.Rules[match.RuleIndex]
Expand Down
13 changes: 11 additions & 2 deletions internal/dataplane/parser/translators/httproute_atc_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,17 @@ func TestGenerateKongExpressionRoutesFromHTTPRouteMatches(t *testing.T) {
name: "no hostnames and no matches",
routeName: "empty_route.default.0.0",
ingressObjectInfo: util.K8sObjectInfo{},
expectedRoutes: []kongstate.Route{},
expectedError: ErrRouteValidationNoMatchRulesOrHostnamesSpecified,
expectedRoutes: []kongstate.Route{
{
Route: kong.Route{
Name: kong.String("empty_route.default.0.0"),
PreserveHost: kong.Bool(true),
StripPath: kong.Bool(false),
Expression: kong.String(CatchAllHTTPExpression),
},
ExpressionRoutes: true,
},
},
},
{
name: "no matches but have hostnames",
Expand Down

0 comments on commit f299b4f

Please sign in to comment.