Handle named (non-numeric) ports correctly

[liggitt]

NGINX Ingress controller version: v1.0.0-alpha.1

Kubernetes version (use kubectl version): v1.19+

Environment: all

• Cloud provider or hardware configuration: all
• OS (e.g. from /etc/os-release): all
• Kernel (e.g. uname -a): all
• Install tools: all
• Others: all

What happened:

In pre-1.0 versions, numeric and string serviceport values were used:

https://github.com/kubernetes/ingress-nginx/blob/nginx-0.30.0/internal/ingress/controller/template/template.go#L859-L861

https://github.com/kubernetes/ingress-nginx/blob/nginx-0.30.0/internal/ingress/controller/template/template.go#L876-L878

In 1.0.0-alpha, only numeric ports are used. Something like this is required to continue to handle non-numeric ports:

diff --git a/cmd/plugin/commands/ingresses/ingresses.go b/cmd/plugin/commands/ingresses/ingresses.go
index 422705320..dff967103 100644
--- a/cmd/plugin/commands/ingresses/ingresses.go
+++ b/cmd/plugin/commands/ingresses/ingresses.go
@@ -227,7 +227,7 @@ func serviceToNameAndPort(svc *networking.IngressServiceBackend) (string, intstr
 			return svcName, intstr.FromInt(int(svc.Port.Number))
 		}
 		if svc.Port.Name != "" {
-			return svcName, intstr.FromString(svc.Port.String())
+			return svcName, intstr.FromString(svc.Port.Name)
 		}
 	}
 	return "", intstr.IntOrString{}
diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go
index 698f232a4..c313526a1 100644
--- a/internal/ingress/controller/template/template.go
+++ b/internal/ingress/controller/template/template.go
@@ -904,6 +904,8 @@ func getIngressInformation(i, h, p interface{}) *ingressInformation {
 		info.Service = ing.Spec.DefaultBackend.Service.Name
 		if ing.Spec.DefaultBackend.Service.Port.Number > 0 {
 			info.ServicePort = strconv.Itoa(int(ing.Spec.DefaultBackend.Service.Port.Number))
+		} else {
+			info.ServicePort = ing.Spec.DefaultBackend.Service.Port.Name
 		}
 	}
 
@@ -938,6 +940,8 @@ func getIngressInformation(i, h, p interface{}) *ingressInformation {
 			info.Service = rPath.Backend.Service.Name
 			if rPath.Backend.Service.Port.Number > 0 {
 				info.ServicePort = strconv.Itoa(int(rPath.Backend.Service.Port.Number))
+			} else {
+				info.ServicePort = rPath.Backend.Service.Port.Name
 			}
 
 			return info

That also indicates test coverage of non-numeric port paths is missing

[rikatz]

/triage accepted
/priority critical-urgent
/assign @rikatz

[cpanato]

/assign @cpanato
/unassign @rikatz

[rikatz]

@liggitt correction made by @cpanato works.

I also did tested with older ingress version and it was working with services with named ports but probably those files you found are something that we've missed and may have some problem in the future.

Tests made:

• Create a deployment with echoserver
• Expose echoserver service with a namedport (echosvc1) and the current port (8080)
• Map an ingress object, and use port.name instead of port.number in ingress specification

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: minimal-ingress
  annotations:
    nginx.ingress.kubernetes.io/rewrite-target: /
spec:
  rules:
  - http:
      paths:
      - path: /testpath
        pathType: Prefix
        backend:
          service:
            name: echoserver1
            port:
              name: echosvc1

Thanks!