fix: allow GRPCRoute without hostnames and matches

Kong · Dec 7, 2023 · eda2916 · eda2916
1 parent b876880
commit eda2916
Show file tree

Hide file tree

Showing 5 changed files with 113 additions and 72 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -103,6 +103,7 @@ Adding a new version? You'll need three changes:
   [#5302](https://github.com/Kong/kubernetes-ingress-controller/pull/5302)
 - Added support for GRPC over HTTP (without TLS) in Gateway API.
   [#5128](https://github.com/Kong/kubernetes-ingress-controller/pull/5128)
+  [#5283](https://github.com/Kong/kubernetes-ingress-controller/pull/5283)
 - Added `-init-cache-sync-duration` CLI flag. This flag configures how long the controller waits for Kubernetes resources to populate at startup before generating the initial Kong configuration. It also fixes a bug that removed the default 5 second wait period.
   [#5238](https://github.com/Kong/kubernetes-ingress-controller/pull/5238)
 - Added `--emit-kubernetes-events` CLI flag to disable the creation of events
@@ -121,9 +122,11 @@ Adding a new version? You'll need three changes:
   errors with a GRPCRoute.
   [#5267](https://github.com/Kong/kubernetes-ingress-controller/pull/5267)
   [#5275](https://github.com/Kong/kubernetes-ingress-controller/pull/5275)
-  [#5283](https://github.com/Kong/kubernetes-ingress-controller/pull/5283)
 - Restore the diagnostics server functionality, which was accidentally disabled.
   [#5270](https://github.com/Kong/kubernetes-ingress-controller/pull/5270)
+- Allow configuring a GRPCRoute without hostnames and matches that catch all
+  requests.
+  [#5303](https://github.com/Kong/kubernetes-ingress-controller/pull/5303)
 
 ### Changed
 

diff --git a/internal/dataplane/translator/subtranslator/grpcroute.go b/internal/dataplane/translator/subtranslator/grpcroute.go
@@ -40,48 +40,54 @@ func GenerateKongRoutesFromGRPCRouteRule(
 	if ruleNumber >= len(grpcroute.Spec.Rules) {
 		return nil
 	}
-	rule := grpcroute.Spec.Rules[ruleNumber]
 
-	routes := make([]kongstate.Route, 0, len(rule.Matches))
-	// gather the k8s object information and hostnames from the grpcroute
+	routeName := func(namespace string, name string, ruleNumber int, matchNumber int) *string {
+		return kong.String(fmt.Sprintf(
+			"grpcroute.%s.%s.%d.%d",
+			namespace,
+			name,
+			ruleNumber,
+			matchNumber,
+		))
+	}
+
+	// Gather the K8s object information and hostnames from the GRPCRoute.
 	ingressObjectInfo := util.FromK8sObject(grpcroute)
 	tags := util.GenerateTagsForObject(grpcroute)
-
-	// generate a route to match hostnames only if there is no match in the rule.
+	grpcProtocols := kong.StringSlice("grpc", "grpcs")
+	rule := grpcroute.Spec.Rules[ruleNumber]
+	// Kong Route expects to have for gRPC, at least one of Hosts, Headers or Paths fields set.
+	// For no matches it can be a catch-all or route based on hostnames.
 	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"),
+				Name:      routeName(grpcroute.Namespace, grpcroute.Name, ruleNumber, 0),
+				Protocols: grpcProtocols,
 				Tags:      tags,
 			},
 		}
-		r.Hosts = getGRPCRouteHostnamesAsSliceOfStringPointers(grpcroute)
+		if configuredHostnames := getGRPCRouteHostnamesAsSliceOfStringPointers(grpcroute); len(configuredHostnames) > 0 {
+			// Match based on hostnames.
+			r.Hosts = configuredHostnames
+		} else {
+			// No hostnames configured, so this is a catch-all.
+			// https://docs.konghq.com/gateway/latest/production/configuring-a-grpc-service/#single-grpc-service-and-route
+			r.Paths = kong.StringSlice("/")
+		}
 		return []kongstate.Route{r}
 	}
 
+	// Rule matches are configured, hostname may be specified too.
+	routes := make([]kongstate.Route, 0, len(rule.Matches))
 	for matchNumber, match := range rule.Matches {
-		routeName := fmt.Sprintf(
-			"grpcroute.%s.%s.%d.%d",
-			grpcroute.Namespace,
-			grpcroute.Name,
-			ruleNumber,
-			matchNumber,
-		)
-
 		r := kongstate.Route{
 			Ingress: ingressObjectInfo,
 			Route: kong.Route{
-				Name:      kong.String(routeName),
-				Protocols: kong.StringSlice("grpc", "grpcs"),
+				Name:      routeName(grpcroute.Namespace, grpcroute.Name, ruleNumber, matchNumber),
+				Protocols: grpcProtocols,
 				Tags:      tags,
+				Hosts:     getGRPCRouteHostnamesAsSliceOfStringPointers(grpcroute),
 			},
 		}
 
@@ -110,10 +116,6 @@ func GenerateKongRoutesFromGRPCRouteRule(
 			r.Paths = append(r.Paths, path)
 		}
 
-		if len(grpcroute.Spec.Hostnames) > 0 {
-			r.Hosts = getGRPCRouteHostnamesAsSliceOfStringPointers(grpcroute)
-		}
-
 		r.Headers = map[string][]string{}
 		for _, hmatch := range match.Headers {
 			name := string(hmatch.Name)
@@ -122,7 +124,6 @@ func GenerateKongRoutesFromGRPCRouteRule(
 
 		routes = append(routes, r)
 	}
-
 	return routes
 }
 
@@ -134,6 +135,9 @@ func GenerateKongRoutesFromGRPCRouteRule(
 // in an GRPCRoute specification into a []*string slice, which is the type required
 // by kong.Route{}.
 func getGRPCRouteHostnamesAsSliceOfStringPointers(grpcroute *gatewayapi.GRPCRoute) []*string {
+	if len(grpcroute.Spec.Hostnames) == 0 {
+		return nil
+	}
 	return lo.Map(grpcroute.Spec.Hostnames, func(h gatewayapi.Hostname, _ int) *string {
 		return lo.ToPtr(string(h))
 	})

diff --git a/internal/dataplane/translator/subtranslator/grpcroute_test.go b/internal/dataplane/translator/subtranslator/grpcroute_test.go
@@ -249,6 +249,34 @@ func TestGenerateKongRoutesFromGRPCRouteRule(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:        "single no hostnames, no matches",
+			objectName:  "catch-all",
+			annotations: map[string]string{},
+			rule:        gatewayapi.GRPCRouteRule{},
+			expectedRoutes: []kongstate.Route{
+				{
+					Ingress: util.K8sObjectInfo{
+						Name:             "catch-all",
+						Namespace:        "default",
+						Annotations:      map[string]string{},
+						GroupVersionKind: grpcRouteGVK,
+					},
+					Route: kong.Route{
+						Name:      kong.String("grpcroute.default.catch-all.0.0"),
+						Protocols: kong.StringSlice("grpc", "grpcs"),
+						Tags: kong.StringSlice(
+							"k8s-name:catch-all",
+							"k8s-namespace:default",
+							"k8s-kind:GRPCRoute",
+							"k8s-group:gateway.networking.k8s.io",
+							"k8s-version:v1alpha2",
+						),
+						Paths: kong.StringSlice("/"),
+					},
+				},
+			},
+		},
 	}
 
 	for _, tc := range testCases {
@@ -260,3 +288,45 @@ func TestGenerateKongRoutesFromGRPCRouteRule(t *testing.T) {
 		})
 	}
 }
+
+func TestGetGRPCRouteHostnamesAsSliceOfStringPointers(t *testing.T) {
+	for _, tC := range []struct {
+		name      string
+		grpcroute *gatewayapi.GRPCRoute
+		expected  []*string
+	}{
+		{
+			name: "single hostname",
+			grpcroute: &gatewayapi.GRPCRoute{
+				Spec: gatewayapi.GRPCRouteSpec{
+					Hostnames: []gatewayapi.Hostname{"example.com"},
+				},
+			},
+			expected: []*string{
+				lo.ToPtr("example.com"),
+			},
+		},
+		{
+			name: "multiple hostnames",
+			grpcroute: &gatewayapi.GRPCRoute{
+				Spec: gatewayapi.GRPCRouteSpec{
+					Hostnames: []gatewayapi.Hostname{"example.com", "api.example.com"},
+				},
+			},
+			expected: []*string{
+				lo.ToPtr("example.com"),
+				lo.ToPtr("api.example.com"),
+			},
+		},
+		{
+			name:      "nil hostnames",
+			grpcroute: &gatewayapi.GRPCRoute{},
+			expected:  nil,
+		},
+	} {
+		t.Run(tC.name, func(t *testing.T) {
+			result := getGRPCRouteHostnamesAsSliceOfStringPointers(tC.grpcroute)
+			require.Equal(t, tC.expected, result)
+		})
+	}
+}
diff --git a/internal/dataplane/translator/translate_grpcroute.go b/internal/dataplane/translator/translate_grpcroute.go
@@ -28,14 +28,14 @@ func (t *Translator) ingressRulesFromGRPCRoutes() ingressRules {
 	}
 
 	var errs []error
-	for _, grpcroute := range grpcRouteList {
-		if err := t.ingressRulesFromGRPCRoute(&result, grpcroute); err != nil {
-			err = fmt.Errorf("GRPCRoute %s/%s can't be routed: %w", grpcroute.Namespace, grpcroute.Name, err)
+	for _, grpcRoute := range grpcRouteList {
+		if err := t.ingressRulesFromGRPCRoute(&result, grpcRoute); err != nil {
+			err = fmt.Errorf("GRPCRoute %s/%s can't be routed: %w", grpcRoute.Namespace, grpcRoute.Name, err)
 			errs = append(errs, err)
 		} else {
 			// at this point the object has been configured and can be
 			// reported as successfully translated.
-			t.registerSuccessfullyTranslatedObject(grpcroute)
+			t.registerSuccessfullyTranslatedObject(grpcRoute)
 		}
 	}
 
@@ -47,10 +47,6 @@ func (t *Translator) ingressRulesFromGRPCRoutes() ingressRules {
 }
 
 func (t *Translator) ingressRulesFromGRPCRoute(result *ingressRules, grpcroute *gatewayapi.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 metadata about the object
 	spec := grpcroute.Spec
 
@@ -86,11 +82,6 @@ func (t *Translator) ingressRulesFromGRPCRoutesUsingExpressionRoutes(grpcRoutes
 	// after they are translated, register the success event in the translator.
 	translatedGRPCRoutes := []*gatewayapi.GRPCRoute{}
 	for _, grpcRoute := range grpcRoutes {
-		// validate the GRPCRoute before it gets split by hostnames and matches.
-		if err := validateGRPCRoute(grpcRoute); err != nil {
-			t.registerTranslationFailure(err.Error(), grpcRoute)
-			continue
-		}
 		splitGRPCRouteMatches = append(splitGRPCRouteMatches, subtranslator.SplitGRPCRoute(grpcRoute)...)
 		translatedGRPCRoutes = append(translatedGRPCRoutes, grpcRoute)
 	}
@@ -153,12 +144,3 @@ func grpcBackendRefsToBackendRefs(grpcBackendRef []gatewayapi.GRPCBackendRef) []
 	}
 	return backendRefs
 }
-
-func validateGRPCRoute(grpcRoute *gatewayapi.GRPCRoute) error {
-	if len(grpcRoute.Spec.Hostnames) == 0 {
-		if len(grpcRoute.Spec.Rules) == 0 {
-			return subtranslator.ErrRouteValidationNoRules
-		}
-	}
-	return nil
-}
diff --git a/internal/dataplane/translator/translate_grpcroute_test.go b/internal/dataplane/translator/translate_grpcroute_test.go
@@ -2,7 +2,6 @@ package translator
 
 import (
 	"strconv"
-	"strings"
 	"testing"
 
 	"github.com/go-logr/zapr"
@@ -34,7 +33,6 @@ func TestIngressRulesFromGRPCRoutesUsingExpressionRoutes(t *testing.T) {
 		expectedKongServices []kongstate.Service
 		// service name -> routes
 		expectedKongRoutes map[string][]kongstate.Route
-		expectedFailures   []failures.ResourceFailure
 	}{
 		{
 			name: "single GRPCRoute with multiple hostnames and multiple rules",
@@ -384,16 +382,6 @@ func TestIngressRulesFromGRPCRoutesUsingExpressionRoutes(t *testing.T) {
 					},
 				},
 			},
-			expectedFailures: []failures.ResourceFailure{
-				newResourceFailure(t, subtranslator.ErrRouteValidationNoRules.Error(),
-					&gatewayapi.GRPCRoute{
-						TypeMeta: grpcRouteTypeMeta,
-						ObjectMeta: metav1.ObjectMeta{
-							Namespace: "default",
-							Name:      "grpcroute-no-rules",
-						},
-					}),
-			},
 		},
 	}
 
@@ -427,15 +415,9 @@ func TestIngressRulesFromGRPCRoutesUsingExpressionRoutes(t *testing.T) {
 					require.Equal(t, *expectedRoute.Expression, *r.Expression)
 				}
 			}
-			// check translation failures
+			// Check translation failures.
 			translationFailures := failureCollector.PopResourceFailures()
-			require.Equal(t, len(tc.expectedFailures), len(translationFailures))
-			for _, expectedTranslationFailure := range tc.expectedFailures {
-				expectedFailureMessage := expectedTranslationFailure.Message()
-				require.True(t, lo.ContainsBy(translationFailures, func(failure failures.ResourceFailure) bool {
-					return strings.Contains(failure.Message(), expectedFailureMessage)
-				}))
-			}
+			require.Empty(t, translationFailures)
 		})
 
 	}