fix: restore defaulting to protocol "grpcs" for gRPC Service

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/ingress_test.go

@@ -87,7 +87,7 @@ func TestIngressGRPC(t *testing.T) {
 
 			exposeWithService := func(p kongProtocolAnnotation) *corev1.Service {
 				grpcBinPort := gRPCBinPort
-				if p == gRPCS {
+				if p == gRPCS || p == "" {
 					grpcBinPort = gRPCSBinPort
 				}
 				kongProtocol := string(p)
@@ -105,7 +105,7 @@ func TestIngressGRPC(t *testing.T) {
 
 			// Deploy two services, one for gRPC and one for gRPCS. Two protocols in one service annotation (konghq.com/protocol) are not supported.
 			serviceGRPC := exposeWithService(gRPC)
-			serviceGRPCS := exposeWithService(gRPCS)
+			serviceGRPCS := exposeWithService("") // Service without annotation should default to gRPCS, to not break existing behavior.
 
 			ingressClass := GetIngressClassFromCtx(ctx)
 			t.Logf("creating an ingress for services: %s, %s with ingress.class %s", serviceGRPC.Name, serviceGRPCS.Name, ingressClass)
@@ -152,14 +152,14 @@ func TestIngressGRPC(t *testing.T) {
 					},
 				},
 			).Build()
-			ingress.Annotations[annotations.AnnotationPrefix+annotations.ProtocolsKey] = fmt.Sprintf("%s,%s", gRPC, gRPCS)
+			ingress.Annotations[annotations.AnnotationPrefix+annotations.ProtocolsKey] = string(gRPC) // fmt.Sprintf("%s,%s", gRPC, gRPCS)
 			assert.NoError(t, clusters.DeployIngress(ctx, cluster, namespace, ingress))
 			cleaner.Add(ingress)
 			ctx = SetInCtxForT(ctx, t, ingress)
 
 			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)