From 5d72d84da9dcbc398f371b362723403d065b1185 Mon Sep 17 00:00:00 2001 From: Manuel Alejandro de Brito Fontes Date: Wed, 28 Aug 2019 13:45:20 -0400 Subject: [PATCH] Fix nginx variable service_port (nginx) --- .../ingress/controller/template/template.go | 11 ++ .../controller/template/template_test.go | 176 +++++++++--------- internal/ingress/types_equals.go | 2 +- rootfs/etc/nginx/template/nginx.tmpl | 2 +- 4 files changed, 105 insertions(+), 86 deletions(-) diff --git a/internal/ingress/controller/template/template.go b/internal/ingress/controller/template/template.go index 25277537d2..7d8593de0f 100644 --- a/internal/ingress/controller/template/template.go +++ b/internal/ingress/controller/template/template.go @@ -797,6 +797,7 @@ type ingressInformation struct { Namespace string Rule string Service string + ServicePort string Annotations map[string]string } @@ -810,6 +811,9 @@ func (info *ingressInformation) Equal(other *ingressInformation) bool { if info.Service != other.Service { return false } + if info.ServicePort != other.ServicePort { + return false + } if !reflect.DeepEqual(info.Annotations, other.Annotations) { return false } @@ -848,6 +852,9 @@ func getIngressInformation(i, h, p interface{}) *ingressInformation { if ing.Spec.Backend != nil { info.Service = ing.Spec.Backend.ServiceName + if ing.Spec.Backend.ServicePort.String() != "0" { + info.ServicePort = ing.Spec.Backend.ServicePort.String() + } } for _, rule := range ing.Spec.Rules { @@ -862,6 +869,10 @@ func getIngressInformation(i, h, p interface{}) *ingressInformation { for _, rPath := range rule.HTTP.Paths { if path == rPath.Path { info.Service = rPath.Backend.ServiceName + if rPath.Backend.ServicePort.String() != "0" { + info.ServicePort = rPath.Backend.ServicePort.String() + } + return info } } diff --git a/internal/ingress/controller/template/template_test.go b/internal/ingress/controller/template/template_test.go index 00c9090544..aa173facc2 100644 --- a/internal/ingress/controller/template/template_test.go +++ b/internal/ingress/controller/template/template_test.go @@ -29,7 +29,11 @@ import ( "testing" jsoniter "github.com/json-iterator/go" + + apiv1 "k8s.io/api/core/v1" networking "k8s.io/api/networking/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/ingress-nginx/internal/ingress" "k8s.io/ingress-nginx/internal/ingress/annotations/authreq" @@ -899,104 +903,108 @@ func TestOpentracingPropagateContext(t *testing.T) { } func TestGetIngressInformation(t *testing.T) { - validIngress := &ingress.Ingress{} - invalidIngress := "wrongtype" - host := "host1" - validPath := "/ok" - invalidPath := 10 - - info := getIngressInformation(invalidIngress, host, validPath) - expected := &ingressInformation{} - if !info.Equal(expected) { - t.Errorf("Expected %v, but got %v", expected, info) - } - - info = getIngressInformation(validIngress, host, invalidPath) - if !info.Equal(expected) { - t.Errorf("Expected %v, but got %v", expected, info) - } - // Setup Ingress Resource - validIngress.Namespace = "default" - validIngress.Name = "validIng" - validIngress.Annotations = map[string]string{ - "ingress.annotation": "ok", - } - validIngress.Spec.Backend = &networking.IngressBackend{ - ServiceName: "a-svc", - } - - info = getIngressInformation(validIngress, host, validPath) - expected = &ingressInformation{ - Namespace: "default", - Rule: "validIng", - Annotations: map[string]string{ - "ingress.annotation": "ok", + testcases := map[string]struct { + Ingress interface{} + Host string + Path interface{} + Expected *ingressInformation + }{ + "wrong ingress type": { + "wrongtype", + "host1", + "/ok", + &ingressInformation{}, }, - Service: "a-svc", - } - if !info.Equal(expected) { - t.Errorf("Expected %v, but got %v", expected, info) - } - - validIngress.Spec.Backend = nil - validIngress.Spec.Rules = []networking.IngressRule{ - { - Host: host, - IngressRuleValue: networking.IngressRuleValue{ - HTTP: &networking.HTTPIngressRuleValue{ - Paths: []networking.HTTPIngressPath{ - { - Path: "/ok", - Backend: networking.IngressBackend{ - ServiceName: "b-svc", - }, + "wrong path type": { + &ingress.Ingress{}, + "host1", + 10, + &ingressInformation{}, + }, + "valid ingress definition with name validIng in namespace default": { + &ingress.Ingress{ + networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "validIng", + Namespace: apiv1.NamespaceDefault, + Annotations: map[string]string{ + "ingress.annotation": "ok", + }, + }, + Spec: networking.IngressSpec{ + Backend: &networking.IngressBackend{ + ServiceName: "a-svc", }, }, }, + nil, + }, + "host1", + "", + &ingressInformation{ + Namespace: "default", + Rule: "validIng", + Annotations: map[string]string{ + "ingress.annotation": "ok", + }, + Service: "a-svc", }, }, - {}, - } - - info = getIngressInformation(validIngress, host, validPath) - expected = &ingressInformation{ - Namespace: "default", - Rule: "validIng", - Annotations: map[string]string{ - "ingress.annotation": "ok", - }, - Service: "b-svc", - } - if !info.Equal(expected) { - t.Errorf("Expected %v, but got %v", expected, info) - } - - validIngress.Spec.Rules = append(validIngress.Spec.Rules, networking.IngressRule{ - Host: "host2", - IngressRuleValue: networking.IngressRuleValue{ - HTTP: &networking.HTTPIngressRuleValue{ - Paths: []networking.HTTPIngressPath{ - { - Path: "/ok", - Backend: networking.IngressBackend{ - ServiceName: "c-svc", + "valid ingress definition with name demo in namespace something and path /ok using a service with name b-svc port 80": { + &ingress.Ingress{ + networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "demo", + Namespace: "something", + Annotations: map[string]string{ + "ingress.annotation": "ok", }, }, + Spec: networking.IngressSpec{ + Rules: []networking.IngressRule{ + { + Host: "foo.bar", + IngressRuleValue: networking.IngressRuleValue{ + HTTP: &networking.HTTPIngressRuleValue{ + Paths: []networking.HTTPIngressPath{ + { + Path: "/ok", + Backend: networking.IngressBackend{ + ServiceName: "b-svc", + ServicePort: intstr.FromInt(80), + }, + }, + }, + }, + }, + }, + {}, + }, + }, + }, + nil, + }, + "foo.bar", + "/ok", + &ingressInformation{ + Namespace: "something", + Rule: "demo", + Annotations: map[string]string{ + "ingress.annotation": "ok", }, + Service: "b-svc", + ServicePort: "80", }, }, - }) - - info = getIngressInformation(validIngress, host, validPath) - if !info.Equal(expected) { - t.Errorf("Expected %v, but got %v", expected, info) } - info = getIngressInformation(validIngress, "host2", validPath) - expected.Service = "c-svc" - if !info.Equal(expected) { - t.Errorf("Expected %v, but got %v", expected, info) + for title, testCase := range testcases { + info := getIngressInformation(testCase.Ingress, testCase.Host, testCase.Path) + + if !info.Equal(testCase.Expected) { + t.Fatalf("%s: expected '%v' but returned %v", title, testCase.Expected, info) + } } } diff --git a/internal/ingress/types_equals.go b/internal/ingress/types_equals.go index 7585903091..d13dbe0807 100644 --- a/internal/ingress/types_equals.go +++ b/internal/ingress/types_equals.go @@ -347,7 +347,7 @@ func (l1 *Location) Equal(l2 *Location) bool { } } - if l1.Port.StrVal != l2.Port.StrVal { + if l1.Port.String() != l2.Port.String() { return false } if !(&l1.BasicDigestAuth).Equal(&l2.BasicDigestAuth) { diff --git a/rootfs/etc/nginx/template/nginx.tmpl b/rootfs/etc/nginx/template/nginx.tmpl index dd8631325f..9b449a7e43 100755 --- a/rootfs/etc/nginx/template/nginx.tmpl +++ b/rootfs/etc/nginx/template/nginx.tmpl @@ -955,7 +955,7 @@ stream { set $namespace {{ $ing.Namespace | quote}}; set $ingress_name {{ $ing.Rule | quote }}; set $service_name {{ $ing.Service | quote }}; - set $service_port {{ $location.Port | quote }}; + set $service_port {{ $ing.ServicePort | quote }}; set $location_path {{ $location.Path | escapeLiteralDollar | quote }}; {{ if $all.Cfg.EnableOpentracing }}