Skip to content

Commit

Permalink
Fix nginx variable service_port (nginx) (kubernetes#4500)
Browse files Browse the repository at this point in the history
  • Loading branch information
@aledbf
aledbf authored Aug 31, 2019
1 parent 72cb7f5 commit c7d2444
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 86 deletions.
11 changes: 11 additions & 0 deletions internal/ingress/controller/template/template.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -797,6 +797,7 @@ type ingressInformation struct {
Namespace string
Rule string
Service string
ServicePort string
Annotations map[string]string
}

Expand All @@ -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
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}
}
Expand Down
176 changes: 92 additions & 84 deletions internal/ingress/controller/template/template_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion internal/ingress/types_equals.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion rootfs/etc/nginx/template/nginx.tmpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down

0 comments on commit c7d2444

Please sign in to comment.