Skip to content

Commit

Permalink
fix: generate routes with hostname match only when GRPCRoute rule doe…
Browse files Browse the repository at this point in the history
…s not have matches (#4512)

* fix: generate routes with hostname match only when GRPCRoute rule does not have matches

* add CHANGELOG and move validation of GRPCRoutes to a separate function
  • Loading branch information
randmonkey authored Aug 17, 2023
1 parent 6f8ae7b commit 13c8d70
Show file tree
Hide file tree
Showing 7 changed files with 187 additions and 9 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ Adding a new version? You'll need three changes:
- Display Service ports on generated Kong services, instead of a static default
value. This change is cosmetic only.
[#4503](https://github.com/Kong/kubernetes-ingress-controller/pull/4503)
- Create routes that match any service and method for `GRPCRoute` rules with no
matches.
[#4512](https://github.com/Kong/kubernetes-ingress-controller/issues/4512)

## [2.11.0]

Expand Down
32 changes: 23 additions & 9 deletions internal/dataplane/parser/translate_grpcroute.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,13 @@ func (p *Parser) ingressRulesFromGRPCRoutes() ingressRules {
}

func (p *Parser) ingressRulesFromGRPCRoute(result *ingressRules, grpcroute *gatewayv1alpha2.GRPCRoute) error {
// validate the grpcRoute before it gets translated
if err := validateGRPCRoute(grpcroute); err != nil {
return err
}
// first we grab the spec and gather some metdata about the object
spec := grpcroute.Spec

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

// each rule may represent a different set of backend services that will be accepting
// traffic, so we make separate routes and Kong services for every present rule.
for ruleNumber, rule := range spec.Rules {
Expand Down Expand Up @@ -97,11 +97,9 @@ func (p *Parser) ingressRulesFromGRPCRoutesUsingExpressionRoutes(grpcRoutes []*g
// after they are translated, register the success event in the parser.
translatedGRPCRoutes := []*gatewayv1alpha2.GRPCRoute{}
for _, grpcRoute := range grpcRoutes {
if len(grpcRoute.Spec.Rules) == 0 {
p.registerTranslationFailure(
translators.ErrRouteValidationNoRules.Error(),
grpcRoute,
)
// validate the GRPCRoute before it gets split by hostnames and matches.
if err := validateGRPCRoute(grpcRoute); err != nil {
p.registerTranslationFailure(err.Error(), grpcRoute)
continue
}
splitGRPCRouteMatches = append(splitGRPCRouteMatches, translators.SplitGRPCRoute(grpcRoute)...)
Expand Down Expand Up @@ -164,3 +162,19 @@ func grpcBackendRefsToBackendRefs(grpcBackendRef []gatewayv1alpha2.GRPCBackendRe
}
return backendRefs
}

func validateGRPCRoute(grpcRoute *gatewayv1alpha2.GRPCRoute) error {
if len(grpcRoute.Spec.Hostnames) == 0 {
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
}
26 changes: 26 additions & 0 deletions internal/dataplane/parser/translate_grpcroute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,24 @@ func TestIngressRulesFromGRPCRoutesUsingExpressionRoutes(t *testing.T) {
},
Spec: gatewayv1alpha2.GRPCRouteSpec{},
},
{
TypeMeta: grpcRouteTypeMeta,
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
Name: "grpcroute-no-hostnames-no-matches",
},
Spec: gatewayv1alpha2.GRPCRouteSpec{
Rules: []gatewayv1alpha2.GRPCRouteRule{
{
BackendRefs: []gatewayv1alpha2.GRPCBackendRef{
{
BackendRef: builder.NewBackendRef("service0").WithPort(80).Build(),
},
},
},
},
},
},
},
expectedKongServices: []kongstate.Service{
{
Expand Down Expand Up @@ -361,6 +379,14 @@ 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
19 changes: 19 additions & 0 deletions internal/dataplane/parser/translators/grpcroute.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,25 @@ func GenerateKongRoutesFromGRPCRouteRule(
// gather the k8s object information and hostnames from the grpcroute
ingressObjectInfo := util.FromK8sObject(grpcroute)

// generate a route to match hostnames only if there is no match in the rule.
if len(rule.Matches) == 0 {
routeName := fmt.Sprintf(
"grpcroute.%s.%s.%d.0",
grpcroute.Namespace,
grpcroute.Name,
ruleNumber,
)
r := kongstate.Route{
Ingress: ingressObjectInfo,
Route: kong.Route{
Name: kong.String(routeName),
Protocols: kong.StringSlice("grpc", "grpcs"),
},
}
r.Hosts = getGRPCRouteHostnamesAsSliceOfStringPointers(grpcroute)
return []kongstate.Route{r}
}

for matchNumber, match := range rule.Matches {
routeName := fmt.Sprintf(
"grpcroute.%s.%s.%d.%d",
Expand Down
32 changes: 32 additions & 0 deletions internal/dataplane/parser/translators/grpcroute_atc.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,28 @@ func GenerateKongExpressionRoutesFromGRPCRouteRule(grpcroute *gatewayv1alpha2.GR
// gather the k8s object information and hostnames from the grpcroute
ingressObjectInfo := util.FromK8sObject(grpcroute)

// generate a route to match hostnames only if there is no match in the rule.
if len(rule.Matches) == 0 {
routeName := fmt.Sprintf(
"grpcroute.%s.%s.%d.0",
grpcroute.Namespace,
grpcroute.Name,
ruleNumber,
)
r := kongstate.Route{
Ingress: ingressObjectInfo,
Route: kong.Route{
Name: kong.String(routeName),
},
ExpressionRoutes: true,
}
hostnames := getGRPCRouteHostnamesAsSliceOfStrings(grpcroute)
// assign an empty match to generate matchers by only hostnames and annotations.
matcher := generateMathcherFromGRPCMatch(gatewayv1alpha2.GRPCRouteMatch{}, hostnames, ingressObjectInfo.Annotations)
atc.ApplyExpression(&r.Route, matcher, 1)
return []kongstate.Route{r}
}

for matchNumber, match := range rule.Matches {
routeName := fmt.Sprintf(
"grpcroute.%s.%s.%d.%d",
Expand Down Expand Up @@ -168,6 +190,16 @@ func SplitGRPCRoute(grpcroute *gatewayv1alpha2.GRPCRoute) []SplitGRPCRouteMatch
splitMatches := []SplitGRPCRouteMatch{}
splitGRPCRouteByMatch := func(hostname string) {
for ruleIndex, rule := range grpcroute.Spec.Rules {
// split out a match with only the hostname (non-empty only) when there are no matches in rule.
if len(rule.Matches) == 0 {
splitMatches = append(splitMatches, SplitGRPCRouteMatch{
Source: grpcroute,
Hostname: hostname,
Match: gatewayv1alpha2.GRPCRouteMatch{}, // empty grpcRoute match with ALL nil fields
RuleIndex: ruleIndex,
MatchIndex: 0,
})
}
for matchIndex, match := range rule.Matches {
splitMatches = append(splitMatches, SplitGRPCRouteMatch{
Source: grpcroute,
Expand Down
61 changes: 61 additions & 0 deletions internal/dataplane/parser/translators/grpcroute_atc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,29 @@ func TestGenerateKongExpressionRoutesFromGRPCRouteRule(t *testing.T) {
},
},
},
{
name: "single hostname with no match",
objectName: "hostname-only",
hostnames: []string{"foo.com"},
rule: gatewayv1alpha2.GRPCRouteRule{
Matches: []gatewayv1alpha2.GRPCRouteMatch{},
},
expectedRoutes: []kongstate.Route{
{
Ingress: util.K8sObjectInfo{
Name: "hostname-only",
Namespace: "default",
GroupVersionKind: grpcRouteGVK,
},
ExpressionRoutes: true,
Route: kong.Route{
Name: kong.String("grpcroute.default.hostname-only.0.0"),
Expression: kong.String(`http.host == "foo.com"`),
Priority: kong.Int(1),
},
},
},
},
}

for _, tc := range testCases {
Expand Down Expand Up @@ -368,6 +391,24 @@ func TestSplitGRPCRoute(t *testing.T) {
},
},
},
{
TypeMeta: grpcRouteTypeMeta,
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
Name: "grpcroute-multiple-hostnames-no-match",
},
Spec: gatewayv1alpha2.GRPCRouteSpec{
Hostnames: []gatewayv1alpha2.Hostname{
gatewayv1alpha2.Hostname("pets.com"),
gatewayv1alpha2.Hostname("pets.net"),
},
Rules: []gatewayv1alpha2.GRPCRouteRule{
{
BackendRefs: namesToBackendRefs([]string{"listpets"}),
},
},
},
},
}

testCases := []struct {
Expand Down Expand Up @@ -442,6 +483,26 @@ func TestSplitGRPCRoute(t *testing.T) {
},
},
},
{
name: "multiple hostnames and no match",
grpcRoute: testGRPCRoutes[3],
expectedSplitMatches: []SplitGRPCRouteMatch{
{
Source: testGRPCRoutes[3],
Hostname: string(testGRPCRoutes[3].Spec.Hostnames[0]),
Match: gatewayv1alpha2.GRPCRouteMatch{},
RuleIndex: 0,
MatchIndex: 0,
},
{
Source: testGRPCRoutes[3],
Hostname: string(testGRPCRoutes[3].Spec.Hostnames[1]),
Match: gatewayv1alpha2.GRPCRouteMatch{},
RuleIndex: 0,
MatchIndex: 0,
},
},
},
}

for i, tc := range testCases {
Expand Down
23 changes: 23 additions & 0 deletions internal/dataplane/parser/translators/grpcroute_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,29 @@ func TestGenerateKongRoutesFromGRPCRouteRule(t *testing.T) {
},
},
},
{
name: "single hostname, no matches",
objectName: "hostname-only",
annotations: map[string]string{},
hostnames: []string{"foo.com"},
rule: gatewayv1alpha2.GRPCRouteRule{},
prependRegexPrefix: true,
expectedRoutes: []kongstate.Route{
{
Ingress: util.K8sObjectInfo{
Name: "hostname-only",
Namespace: "default",
Annotations: map[string]string{},
GroupVersionKind: grpcRouteGVK,
},
Route: kong.Route{
Name: kong.String("grpcroute.default.hostname-only.0.0"),
Protocols: kong.StringSlice("grpc", "grpcs"),
Hosts: kong.StringSlice("foo.com"),
},
},
},
},
}

for _, tc := range testCases {
Expand Down

0 comments on commit 13c8d70

Please sign in to comment.