fix: use port name when port number is not provided in combined routes

internal/dataplane/parser/translate_ingress.go

@@ -16,6 +16,19 @@ import (
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/util"
 )
 
+func serviceBackendPortToStr(port netv1.ServiceBackendPort) string {
+	if port.Name != "" {
+		return fmt.Sprintf("pname-%s", port.Name)
+	}
+	return fmt.Sprintf("pnum-%d", port.Number)
+}
+
+var priorityForPath = map[netv1.PathType]int{
+	netv1.PathTypeExact:                  300,
+	netv1.PathTypePrefix:                 200,
+	netv1.PathTypeImplementationSpecific: 100,
+}
+
 func (p *Parser) ingressRulesFromIngressV1beta1() ingressRules {
 	result := newIngressRules()
 
@@ -102,7 +115,7 @@ func (p *Parser) ingressRulesFromIngressV1beta1() ingressRules {
 						Namespace: ingress.Namespace,
 						Backends: []kongstate.ServiceBackend{{
 							Name:    rule.Backend.ServiceName,
-							PortDef: PortDefFromIntStr(rule.Backend.ServicePort),
+							PortDef: translators.PortDefFromIntStr(rule.Backend.ServicePort),
 						}},
 						Parent: ingress,
 					}
@@ -147,7 +160,7 @@ func (p *Parser) ingressRulesFromIngressV1beta1() ingressRules {
 				Namespace: ingress.Namespace,
 				Backends: []kongstate.ServiceBackend{{
 					Name:    defaultBackend.ServiceName,
-					PortDef: PortDefFromIntStr(defaultBackend.ServicePort),
+					PortDef: translators.PortDefFromIntStr(defaultBackend.ServicePort),
 				}},
 				Parent: &ingress,
 			}
@@ -261,7 +274,7 @@ func (p *Parser) ingressRulesFromIngressV1() ingressRules {
 						r.Hosts = kong.StringSlice(rule.Host)
 					}
 
-					port := PortDefFromServiceBackendPort(&rulePath.Backend.Service.Port)
+					port := translators.PortDefFromServiceBackendPort(&rulePath.Backend.Service.Port)
 					serviceName := fmt.Sprintf("%s.%s.%s", ingress.Namespace, rulePath.Backend.Service.Name,
 						serviceBackendPortToStr(rulePath.Backend.Service.Port))
 					service, ok := result.ServiceNameToServices[serviceName]
@@ -307,7 +320,7 @@ func (p *Parser) ingressRulesFromIngressV1() ingressRules {
 	if len(allDefaultBackends) > 0 {
 		ingress := allDefaultBackends[0]
 		defaultBackend := allDefaultBackends[0].Spec.DefaultBackend
-		port := PortDefFromServiceBackendPort(&defaultBackend.Service.Port)
+		port := translators.PortDefFromServiceBackendPort(&defaultBackend.Service.Port)
 		serviceName := fmt.Sprintf("%s.%s.%s", allDefaultBackends[0].Namespace, defaultBackend.Service.Name,
 			port.CanonicalString())
 		service, ok := result.ServiceNameToServices[serviceName]
@@ -327,7 +340,7 @@ func (p *Parser) ingressRulesFromIngressV1() ingressRules {
 				Namespace: ingress.Namespace,
 				Backends: []kongstate.ServiceBackend{{
 					Name:    defaultBackend.Service.Name,
-					PortDef: PortDefFromServiceBackendPort(&defaultBackend.Service.Port),
+					PortDef: translators.PortDefFromServiceBackendPort(&defaultBackend.Service.Port),
 				}},
 				Parent: &ingress,
 			}

internal/dataplane/parser/translate_knative.go

@@ -113,7 +113,7 @@ func (p *Parser) ingressRulesFromKnativeIngress() ingressRules {
 						Namespace: ingress.Namespace,
 						Backends: []kongstate.ServiceBackend{{
 							Name:    knativeBackend.ServiceName,
-							PortDef: PortDefFromIntStr(knativeBackend.ServicePort),
+							PortDef: translators.PortDefFromIntStr(knativeBackend.ServicePort),
 						}},
 						Parent: ingress,
 					}

internal/dataplane/parser/translators/ingress.go

@@ -97,17 +97,17 @@ func (i *ingressTranslationIndex) add(ingress *netv1.Ingress) {
 			}
 
 			serviceName := httpIngressPath.Backend.Service.Name
-			servicePort := httpIngressPath.Backend.Service.Port.Number
+			port := PortDefFromServiceBackendPort(&httpIngressPath.Backend.Service.Port)
 
-			cacheKey := fmt.Sprintf("%s.%s.%s.%s.%d", ingress.Namespace, ingress.Name, ingressRule.Host, serviceName, servicePort)
+			cacheKey := fmt.Sprintf("%s.%s.%s.%s.%s", ingress.Namespace, ingress.Name, ingressRule.Host, serviceName, port.CanonicalString())
 			meta, ok := i.cache[cacheKey]
 			if !ok {
 				meta = &ingressTranslationMeta{
 					ingressNamespace: ingress.Namespace,
 					ingressName:      ingress.Name,
 					ingressHost:      ingressRule.Host,
 					serviceName:      serviceName,
-					servicePort:      servicePort,
+					servicePort:      port,
 					addRegexPrefix:   i.addRegexPrefix,
 				}
 			}
@@ -122,15 +122,10 @@ func (i *ingressTranslationIndex) add(ingress *netv1.Ingress) {
 func (i *ingressTranslationIndex) translate() []*kongstate.Service {
 	kongStateServiceCache := make(map[string]*kongstate.Service)
 	for _, meta := range i.cache {
-		portDef := kongstate.PortDef{
-			Number: meta.servicePort,
-			Mode:   kongstate.PortModeByNumber,
-		}
-
-		kongServiceName := fmt.Sprintf("%s.%s.%s.%s", meta.ingressNamespace, meta.ingressName, meta.serviceName, portDef.CanonicalString())
+		kongServiceName := fmt.Sprintf("%s.%s.%s.%s", meta.ingressNamespace, meta.ingressName, meta.serviceName, meta.servicePort.CanonicalString())
 		kongStateService, ok := kongStateServiceCache[kongServiceName]
 		if !ok {
-			kongStateService = meta.translateIntoKongStateService(kongServiceName, portDef)
+			kongStateService = meta.translateIntoKongStateService(kongServiceName, meta.servicePort)
 		}
 
 		route := meta.translateIntoKongRoutes()
@@ -157,7 +152,7 @@ type ingressTranslationMeta struct {
 	ingressName        string
 	ingressHost        string
 	serviceName        string
-	servicePort        int32
+	servicePort        kongstate.PortDef
 	paths              []netv1.HTTPIngressPath
 	addRegexPrefix     bool
 }
@@ -167,7 +162,7 @@ func (m *ingressTranslationMeta) translateIntoKongStateService(kongServiceName s
 		Namespace: m.ingressNamespace,
 		Service: kong.Service{
 			Name:           kong.String(kongServiceName),
-			Host:           kong.String(fmt.Sprintf("%s.%s.%d.svc", m.serviceName, m.ingressNamespace, portDef.Number)),
+			Host:           kong.String(fmt.Sprintf("%s.%s.%s.svc", m.serviceName, m.ingressNamespace, portDef.CanonicalString())),
 			Port:           kong.Int(defaultHTTPPort),
 			Protocol:       kong.String("http"),
 			Path:           kong.String("/"),
@@ -200,7 +195,7 @@ func (m *ingressTranslationMeta) translateIntoKongRoutes() *kongstate.Route {
 		// '_' is not allowed in host, so we use '_' to replace '*' since '*' is not allowed in Kong.
 		ingressHost = strings.ReplaceAll(ingressHost, "*", "_")
 	}
-	routeName := fmt.Sprintf("%s.%s.%s.%s.%d", m.ingressNamespace, m.ingressName, m.serviceName, ingressHost, m.servicePort)
+	routeName := fmt.Sprintf("%s.%s.%s.%s.%s", m.ingressNamespace, m.ingressName, m.serviceName, ingressHost, m.servicePort.CanonicalString())
 	route := &kongstate.Route{
 		Ingress: util.K8sObjectInfo{
 			Namespace:   m.ingressNamespace,

internal/dataplane/parser/translators/ingress_test.go

@@ -1078,11 +1078,80 @@ func TestTranslateIngress(t *testing.T) {
 				Parent: expectedParentIngress(),
 			}},
 		},
+		{
+			name: "use port name when service port number is not provided",
+			ingress: &netv1.Ingress{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "test-ingress",
+					Namespace: corev1.NamespaceDefault,
+				},
+				Spec: netv1.IngressSpec{
+					Rules: []netv1.IngressRule{{
+						Host: "konghq.com",
+						IngressRuleValue: netv1.IngressRuleValue{
+							HTTP: &netv1.HTTPIngressRuleValue{
+								Paths: []netv1.HTTPIngressPath{{
+									Path: "/api/",
+									Backend: netv1.IngressBackend{
+										Service: &netv1.IngressServiceBackend{
+											Name: "test-service",
+											Port: netv1.ServiceBackendPort{
+												Name: "http",
+											},
+										},
+									},
+								}},
+							},
+						},
+					}},
+				},
+			},
+			expected: []*kongstate.Service{{
+				Namespace: corev1.NamespaceDefault,
+				Service: kong.Service{
+					Name:           kong.String("default.test-ingress.test-service.http"),
+					Host:           kong.String("test-service.default.http.svc"),
+					ConnectTimeout: kong.Int(int(defaultServiceTimeout.Milliseconds())),
+					Path:           kong.String("/"),
+					Port:           kong.Int(80),
+					Protocol:       kong.String("http"),
+					Retries:        kong.Int(defaultRetries),
+					ReadTimeout:    kong.Int(int(defaultServiceTimeout.Milliseconds())),
+					WriteTimeout:   kong.Int(int(defaultServiceTimeout.Milliseconds())),
+				},
+				Routes: []kongstate.Route{{
+					Ingress: util.K8sObjectInfo{
+						Name:      "test-ingress",
+						Namespace: corev1.NamespaceDefault,
+					},
+					Route: kong.Route{
+						Name:              kong.String("default.test-ingress.test-service.konghq.com.http"),
+						Hosts:             kong.StringSlice("konghq.com"),
+						Paths:             kong.StringSlice("/api/"), // default ImplementationSpecific
+						PreserveHost:      kong.Bool(true),
+						Protocols:         kong.StringSlice("http", "https"),
+						RegexPriority:     kong.Int(0),
+						StripPath:         kong.Bool(false),
+						ResponseBuffering: kong.Bool(true),
+						RequestBuffering:  kong.Bool(true),
+					},
+				}},
+				Backends: []kongstate.ServiceBackend{{
+					Name:      "test-service",
+					Namespace: corev1.NamespaceDefault,
+					PortDef: kongstate.PortDef{
+						Mode: kongstate.PortModeByName,
+						Name: "http",
+					},
+				}},
+				Parent: expectedParentIngress(),
+			}},
+		},
 	}
 
 	for _, tt := range tts {
 		t.Run(tt.name, func(t *testing.T) {
-			assert.Equal(t, TranslateIngress(tt.ingress, false), tt.expected)
+			assert.Equal(t, tt.expected, TranslateIngress(tt.ingress, false))
 		})
 	}
 }

internal/dataplane/parser/translate.go → internal/dataplane/parser/translators/portdef.go

@@ -1,33 +1,18 @@
-package parser
+package translators
 
 import (
-	"fmt"
-
 	netv1 "k8s.io/api/networking/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
 
 	"github.com/kong/kubernetes-ingress-controller/v2/internal/dataplane/kongstate"
 )
 
-func serviceBackendPortToStr(port netv1.ServiceBackendPort) string {
-	if port.Name != "" {
-		return fmt.Sprintf("pname-%s", port.Name)
-	}
-	return fmt.Sprintf("pnum-%d", port.Number)
-}
-
-var priorityForPath = map[netv1.PathType]int{
-	netv1.PathTypeExact:                  300,
-	netv1.PathTypePrefix:                 200,
-	netv1.PathTypeImplementationSpecific: 100,
-}
-
 func PortDefFromServiceBackendPort(sbp *netv1.ServiceBackendPort) kongstate.PortDef {
 	switch {
-	case sbp.Name != "":
-		return kongstate.PortDef{Mode: kongstate.PortModeByName, Name: sbp.Name}
 	case sbp.Number != 0:
 		return kongstate.PortDef{Mode: kongstate.PortModeByNumber, Number: sbp.Number}
+	case sbp.Name != "":
+		return kongstate.PortDef{Mode: kongstate.PortModeByName, Name: sbp.Name}
 	default:
 		return kongstate.PortDef{Mode: kongstate.PortModeImplicit}
 	}

test/integration/ingress_test.go

@@ -16,6 +16,7 @@ import (
 	"github.com/kong/go-kong/kong"
 	"github.com/kong/kubernetes-testing-framework/pkg/clusters"
 	"github.com/kong/kubernetes-testing-framework/pkg/utils/kubernetes/generators"
+	"github.com/samber/lo"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	corev1 "k8s.io/api/core/v1"
@@ -1251,3 +1252,93 @@ func TestIngressMatchByHost(t *testing.T) {
 	defer resp.Body.Close()
 	require.Equal(t, resp.StatusCode, http.StatusNotFound)
 }
+
+func TestIngressWorksWithServiceBackendsSpecifyingOnlyPortNames(t *testing.T) {
+	t.Parallel()
+	ns, cleaner := setup(t)
+	defer func() {
+		if t.Failed() {
+			output, err := cleaner.DumpDiagnostics(ctx, t.Name())
+			t.Logf("%s failed, dumped diagnostics to %s", t.Name(), output)
+			assert.NoError(t, err)
+		}
+		assert.NoError(t, cleaner.Cleanup(ctx))
+	}()
+
+	client := env.Cluster().Client()
+
+	t.Log("deploying a minimal HTTP container deployment to test Ingress routes")
+	container := generators.NewContainer("httpbin", test.HTTPBinImage, 80)
+	deployment := generators.NewDeploymentForContainer(container)
+	deployment.Spec.Template.Spec.Containers[0].Ports[0].Name = "http"
+	deployment, err := client.AppsV1().Deployments(ns.Name).Create(ctx, deployment, metav1.CreateOptions{})
+	require.NoError(t, err)
+	cleaner.Add(deployment)
+
+	t.Logf("exposing deployment %s via service", deployment.Name)
+	service := generators.NewServiceForDeployment(deployment, corev1.ServiceTypeLoadBalancer)
+	service, err = client.CoreV1().Services(ns.Name).Create(ctx, service, metav1.CreateOptions{})
+	require.NoError(t, err)
+	cleaner.Add(service)
+
+	t.Logf("creating an ingress for service %s with ingress.class %s", service.Name, ingressClass)
+
+	// TODO: create a similar helper to generators.NewIngressForServiceWithClusterVersion()
+	// which will accept a param to parametrize the port name or number.
+	ingress := &netv1.Ingress{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: service.Name,
+			Annotations: map[string]string{
+				annotations.IngressClassKey: ingressClass,
+				"konghq.com/strip-path":     "true",
+			},
+		},
+		Spec: netv1.IngressSpec{
+			Rules: []netv1.IngressRule{
+				{
+					IngressRuleValue: netv1.IngressRuleValue{
+						HTTP: &netv1.HTTPIngressRuleValue{
+							Paths: []netv1.HTTPIngressPath{
+								{
+									Path:     "/",
+									PathType: lo.ToPtr(netv1.PathTypePrefix),
+									Backend: netv1.IngressBackend{
+										Service: &netv1.IngressServiceBackend{
+											Name: service.Name,
+											Port: netv1.ServiceBackendPort{
+												Name: service.Spec.Ports[0].Name,
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+	_, err = client.NetworkingV1().Ingresses(ns.Name).Create(ctx, ingress, metav1.CreateOptions{})
+	require.NoError(t, err)
+	cleaner.Add(ingress)
+
+	t.Log("waiting for routes from Ingress to be operational")
+	require.Eventually(t, func() bool {
+		resp, err := httpc.Get(proxyURL.String())
+		if err != nil {
+			t.Logf("WARNING: error while waiting for %s: %v", proxyURL, err)
+			return false
+		}
+		defer resp.Body.Close()
+		if resp.StatusCode == http.StatusOK {
+			// now that the ingress backend is routable, make sure the contents we're getting back are what we expect
+			// Expected: "<title>httpbin.org</title>"
+			b := new(bytes.Buffer)
+			n, err := b.ReadFrom(resp.Body)
+			require.NoError(t, err)
+			require.True(t, n > 0)
+			return strings.Contains(b.String(), "<title>httpbin.org</title>")
+		}
+		return false
+	}, ingressWait, waitTick)
+}