fix: restore defaulting to protocol "grpcs" for gRPC Service (#5283)

CHANGELOG.md

@@ -114,6 +114,7 @@ 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)
 
 ### Changed
 

examples/gateway-grpcroute-via-http.yaml

@@ -8,6 +8,8 @@ metadata:
   name: grpcbin-via-http
   labels:
     app: grpcbin-via-http
+  annotations:
+    konghq.com/protocol: grpc
 spec:
   ports:
   - name: grpc
@@ -64,7 +66,7 @@ spec:
   parentRefs:
   - name: kong
   hostnames:
-  - "example.com"
+  - example-grpc-via-http.com
   rules:
   - backendRefs:
     - name: grpcbin-via-http

examples/gateway-grpcroute-via-https.yaml

@@ -8,6 +8,8 @@ metadata:
   name: grpcbin-via-https
   labels:
     app: grpcbin-via-https
+  annotations:
+    konghq.com/protocol: grpcs
 spec:
   ports:
   - name: grpc

internal/dataplane/translator/translate_grpcroute.go

@@ -57,9 +57,10 @@ func (t *Translator) ingressRulesFromGRPCRoute(result *ingressRules, grpcroute *
 	// 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 {
-		// create a service and attach the routes to it
+		// Create a service and attach the routes to it. Protocol for Service can be set via K8s object annotation
+		// "konghq.com/protocol", by default use "grpcs" to not break existing behavior when annotation is not specified.
 		service, err := generateKongServiceFromBackendRefWithRuleNumber(
-			t.logger, t.storer, result, grpcroute, ruleNumber, t.getProtocolForKongService(grpcroute), grpcBackendRefsToBackendRefs(rule.BackendRefs)...,
+			t.logger, t.storer, result, grpcroute, ruleNumber, "grpcs", grpcBackendRefsToBackendRefs(rule.BackendRefs)...,
 		)
 		if err != nil {
 			return err
@@ -123,13 +124,16 @@ func (t *Translator) ingressRulesFromGRPCRouteWithPriority(
 	grpcRouteRule := grpcRoute.Spec.Rules[match.RuleIndex]
 
 	serviceName := subtranslator.KongServiceNameFromSplitGRPCRouteMatch(match)
+
+	// Create a service and attach the routes to it. Protocol for Service can be set via K8s object annotation
+	// "konghq.com/protocol", by default use "grpcs" to not break existing behavior when annotation is not specified.
 	kongService, _ := generateKongServiceFromBackendRefWithName(
 		t.logger,
 		t.storer,
 		rules,
 		serviceName,
 		grpcRoute,
-		t.getProtocolForKongService(grpcRoute),
+		"grpcs",
 		grpcBackendRefsToBackendRefs(grpcRouteRule.BackendRefs)...,
 	)
 	kongService.Routes = append(
@@ -141,14 +145,6 @@ func (t *Translator) ingressRulesFromGRPCRouteWithPriority(
 	rules.ServiceNameToParent[serviceName] = grpcRoute
 }
 
-func (t *Translator) getProtocolForKongService(grpcRoute *gatewayapi.GRPCRoute) string {
-	// When Gateway listens on HTTP use "grpc" protocol for the service. Otherwise for HTTPS use "grpcs".
-	if len(t.getGatewayListeningPorts(grpcRoute.Namespace, gatewayapi.HTTPProtocolType, grpcRoute.Spec.ParentRefs)) > 0 {
-		return "grpc"
-	}
-	return "grpcs"
-}
-
 func grpcBackendRefsToBackendRefs(grpcBackendRef []gatewayapi.GRPCBackendRef) []gatewayapi.BackendRef {
 	backendRefs := make([]gatewayapi.BackendRef, 0, len(grpcBackendRef))
 

test/integration/isolated/examples_grpc_test.go

@@ -20,6 +20,31 @@ import (
 )
 
 func TestGRPCRouteExample(t *testing.T) {
+	testGRPC := func(ctx context.Context, t *testing.T, manifestName string, gatewayPort int, hostname string, enableTLS bool) {
+		cluster := GetClusterFromCtx(ctx)
+		proxyURL := GetProxyURLFromCtx(ctx)
+		manifestPath := manifestPath(manifestName)
+		t.Logf("applying yaml manifest %s", manifestPath)
+		b, err := os.ReadFile(manifestPath)
+		assert.NoError(t, err)
+		manifest := string(b)
+		assert.NoError(t, clusters.ApplyManifestByYAML(ctx, cluster, manifest))
+
+		t.Log("verifying that GRPCRoute becomes routable")
+		assert.Eventually(t, func() bool {
+			if err := grpcEchoResponds(
+				ctx, fmt.Sprintf("%s:%d", proxyURL.Hostname(), gatewayPort), hostname, "kong", enableTLS,
+			); err != nil {
+				t.Log(err)
+				return false
+			}
+			return true
+		}, consts.IngressWait, consts.WaitTick)
+
+		t.Logf("deleting yaml manifest %s", manifestPath)
+		assert.NoError(t, clusters.DeleteManifestByYAML(ctx, cluster, manifest))
+	}
+
 	f := features.
 		New("example").
 		WithLabel(testlabels.Example, testlabels.ExampleTrue).
@@ -32,36 +57,14 @@ func TestGRPCRouteExample(t *testing.T) {
 			}),
 		)).
 		Assess("deploying to cluster works and deployed GRPC via HTTP responds", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context {
-			testGRPC(ctx, t, manifestPath("gateway-grpcroute-via-http.yaml"), ktfkong.DefaultProxyHTTPPort, false)
+			testGRPC(ctx, t, "gateway-grpcroute-via-http.yaml", ktfkong.DefaultProxyHTTPPort, "example-grpc-via-http.com", false)
 			return ctx
 		}).
 		Assess("deploying to cluster works and deployed GRPC via HTTPS responds", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context {
-			testGRPC(ctx, t, manifestPath("gateway-grpcroute-via-https.yaml"), ktfkong.DefaultProxyTLSServicePort, true)
+			testGRPC(ctx, t, "gateway-grpcroute-via-https.yaml", ktfkong.DefaultProxyTLSServicePort, "example.com", true)
 			return ctx
 		}).
 		Teardown(featureTeardown())
 
 	tenv.Test(t, f.Feature())
 }
-
-func testGRPC(ctx context.Context, t *testing.T, manifestPath string, gatewayPort int, enableTLS bool) {
-	cleaner := GetFromCtxForT[*clusters.Cleaner](ctx, t)
-	cluster := GetClusterFromCtx(ctx)
-	proxyURL := GetProxyURLFromCtx(ctx)
-	t.Logf("applying yaml manifest %s", manifestPath)
-	b, err := os.ReadFile(manifestPath)
-	assert.NoError(t, err)
-	assert.NoError(t, clusters.ApplyManifestByYAML(ctx, cluster, string(b)))
-	cleaner.AddManifest(string(b))
-
-	t.Log("verifying that GRPCRoute becomes routable")
-	assert.Eventually(t, func() bool {
-		if err := grpcEchoResponds(
-			ctx, fmt.Sprintf("%s:%d", proxyURL.Hostname(), gatewayPort), "example.com", "kong", enableTLS,
-		); err != nil {
-			t.Log(err)
-			return false
-		}
-		return true
-	}, consts.IngressWait, consts.WaitTick)
-}

test/integration/isolated/grpc_test.go

@@ -44,7 +44,8 @@ func TestGRPCRouteEssentials(t *testing.T) {
 		WithLabel(testlabels.NetworkingFamily, testlabels.NetworkingFamilyGatewayAPI).
 		WithLabel(testlabels.Kind, testlabels.KindGRPCRoute).
 		WithSetup("deploy kong addon into cluster", featureSetup()).
-		Assess("deploying Gateway and example GRPC service exposed via GRPCRoute", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context {
+		Assess("deploying Gateway and example GRPC service (without konghq.com/protocol annotation) exposed via GRPCRoute over HTTPS", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context {
+			// On purpose omit protocol annotation to test defaulting to "grpcs" that is preserved to not break users' configs.
 			cleaner := GetFromCtxForT[*clusters.Cleaner](ctx, t)
 			cluster := GetClusterFromCtx(ctx)
 			namespace := GetNamespaceForT(ctx, t)
@@ -83,17 +84,22 @@ func TestGRPCRouteEssentials(t *testing.T) {
 
 			t.Log("deploying a new gateway")
 			gateway, err := helpers.DeployGateway(ctx, gatewayClient, namespace, gatewayClassName, func(gw *gatewayapi.Gateway) {
-				gw.Spec.Listeners = builder.NewListener("https").
-					HTTPS().
-					WithPort(ktfkong.DefaultProxyTLSServicePort).
-					WithHostname(testHostname).
-					WithTLSConfig(&gatewayapi.GatewayTLSConfig{
-						CertificateRefs: []gatewayapi.SecretObjectReference{
-							{
-								Name: gatewayapi.ObjectName(secret.Name),
+				// Besides default HTTP listener, add a HTTPS listener.
+				gw.Spec.Listeners = append(
+					gw.Spec.Listeners,
+					builder.NewListener("https").
+						HTTPS().
+						WithPort(ktfkong.DefaultProxyTLSServicePort).
+						WithHostname(testHostname).
+						WithTLSConfig(&gatewayapi.GatewayTLSConfig{
+							CertificateRefs: []gatewayapi.SecretObjectReference{
+								{
+									Name: gatewayapi.ObjectName(secret.Name),
+								},
 							},
-						},
-					}).IntoSlice()
+						}).
+						Build(),
+				)
 			})
 			assert.NoError(t, err)
 			cleaner.Add(gateway)

test/integration/isolated/ingress_test.go

@@ -159,7 +159,7 @@ func TestIngressGRPC(t *testing.T) {
 
 			return ctx
 		}).
-		Assess("checking whether Ingress status is updated and gRPC traffic over HTTPS is properly routed", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context {
+		Assess("checking whether Ingress status is updated and gRPC traffic is properly routed", func(ctx context.Context, t *testing.T, c *envconf.Config) context.Context {
 			t.Log("waiting for updated ingress status to include IP")
 			assert.Eventually(t, func() bool {
 				cluster := GetClusterFromCtx(ctx)