From 9739592848ac2f54a100430b0837d91c26c6feca Mon Sep 17 00:00:00 2001
From: Pasquale Congiusti <pasquale.congiusti@gmail.com>
Date: Wed, 24 Apr 2024 10:29:20 +0200
Subject: [PATCH] fix(trait): harmonize service port name

As Knative accepts a limited set of names, we can harmonize the default values to use the same accepted names.

Close #5409
---
 pkg/trait/container.go      | 13 ++++++++-----
 pkg/trait/container_test.go |  4 ++--
 pkg/trait/service_test.go   |  6 +++---
 pkg/trait/trait_test.go     |  2 +-
 4 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/pkg/trait/container.go b/pkg/trait/container.go
index 19ab28ee59..db33091c03 100644
--- a/pkg/trait/container.go
+++ b/pkg/trait/container.go
@@ -43,9 +43,12 @@ import (
 )
 
 const (
-	defaultContainerName     = "integration"
-	defaultContainerPort     = 8080
-	defaultContainerPortName = "http"
+	defaultContainerName = "integration"
+	defaultContainerPort = 8080
+	// Knative does not want name=http, it only supports http1 (HTTP/1) and h2c (HTTP/2)
+	// https://github.com/knative/specs/blob/main/specs/serving/runtime-contract.md#protocols-and-ports
+	// We can harmonize and keep using the same port name also for any other deployment strategy
+	defaultContainerPortName = "h2c"
 	defaultServicePort       = 80
 	containerTraitID         = "container"
 )
@@ -266,13 +269,13 @@ func (t *containerTrait) configureService(e *Environment, container *corev1.Cont
 	if name == "" {
 		name = defaultContainerPortName
 	}
+
 	containerPort := corev1.ContainerPort{
+		Name:          name,
 		ContainerPort: int32(t.Port),
 		Protocol:      corev1.ProtocolTCP,
 	}
 	if !isKnative {
-		// Knative does not want name=http
-		containerPort.Name = name
 		// The service is managed by Knative, so, we only take care of this when it's managed by us
 		service := e.Resources.GetServiceForIntegration(e.Integration)
 		if service != nil {
diff --git a/pkg/trait/container_test.go b/pkg/trait/container_test.go
index 176dd97946..e80f2876d5 100644
--- a/pkg/trait/container_test.go
+++ b/pkg/trait/container_test.go
@@ -588,7 +588,7 @@ func TestDeploymentContainerPorts(t *testing.T) {
 	container := environment.GetIntegrationContainer()
 	assert.Len(t, container.Ports, 1)
 	assert.Equal(t, int32(8081), container.Ports[0].ContainerPort)
-	assert.Equal(t, "http", container.Ports[0].Name)
+	assert.Equal(t, "h2c", container.Ports[0].Name)
 	svc := environment.Resources.GetServiceForIntegration(environment.Integration)
 	assert.Len(t, svc.Spec.Ports, 1)
 	assert.Equal(t, int32(8081), svc.Spec.Ports[0].Port)
@@ -653,5 +653,5 @@ func TestKnativeServiceContainerPorts(t *testing.T) {
 	container := environment.GetIntegrationContainer()
 	assert.Len(t, container.Ports, 1)
 	assert.Equal(t, int32(8081), container.Ports[0].ContainerPort)
-	assert.Equal(t, "", container.Ports[0].Name)
+	assert.Equal(t, "h2c", container.Ports[0].Name)
 }
diff --git a/pkg/trait/service_test.go b/pkg/trait/service_test.go
index 1769110f96..e42cb5fd40 100644
--- a/pkg/trait/service_test.go
+++ b/pkg/trait/service_test.go
@@ -129,13 +129,13 @@ func TestServiceWithDefaults(t *testing.T) {
 
 	assert.Len(t, s.Spec.Ports, 1)
 	assert.Equal(t, int32(80), s.Spec.Ports[0].Port)
-	assert.Equal(t, "http", s.Spec.Ports[0].Name)
-	assert.Equal(t, "http", s.Spec.Ports[0].TargetPort.String())
+	assert.Equal(t, "h2c", s.Spec.Ports[0].Name)
+	assert.Equal(t, "h2c", s.Spec.Ports[0].TargetPort.String())
 
 	assert.Len(t, d.Spec.Template.Spec.Containers, 1)
 	assert.Len(t, d.Spec.Template.Spec.Containers[0].Ports, 1)
 	assert.Equal(t, int32(8080), d.Spec.Template.Spec.Containers[0].Ports[0].ContainerPort)
-	assert.Equal(t, "http", d.Spec.Template.Spec.Containers[0].Ports[0].Name)
+	assert.Equal(t, "h2c", d.Spec.Template.Spec.Containers[0].Ports[0].Name)
 
 	assert.Empty(t, s.Spec.Type) // empty means ClusterIP
 }
diff --git a/pkg/trait/trait_test.go b/pkg/trait/trait_test.go
index db1dc7cf85..a25b97dab9 100644
--- a/pkg/trait/trait_test.go
+++ b/pkg/trait/trait_test.go
@@ -86,7 +86,7 @@ func TestOpenShiftTraitsWithWebAndConfig(t *testing.T) {
 	assert.NotNil(t, env.GetTrait("service"))
 	assert.NotNil(t, env.GetTrait("route"))
 	assert.NotNil(t, res.GetService(func(svc *corev1.Service) bool {
-		return svc.Name == TestDeploymentName && svc.Spec.Ports[0].TargetPort.StrVal == "http"
+		return svc.Name == TestDeploymentName && svc.Spec.Ports[0].TargetPort.StrVal == "h2c"
 	}))
 }