Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Aug 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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