Skip to content

Handle named (non-numeric) ports correctly #7311

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/plugin/commands/ingresses/ingresses.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down
75 changes: 75 additions & 0 deletions cmd/plugin/commands/ingresses/ingresses_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/*
Copyright 2021 The Kubernetes Authors.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package ingresses

import (
"testing"

networking "k8s.io/api/networking/v1"
"k8s.io/apimachinery/pkg/util/intstr"
)

func TestGetIngressInformation(t *testing.T) {

testcases := map[string]struct {
ServiceBackend *networking.IngressServiceBackend
wantName string
wantPort intstr.IntOrString
}{
"empty ingressServiceBackend": {
ServiceBackend: &networking.IngressServiceBackend{},
wantName: "",
wantPort: intstr.IntOrString{},
},
"ingressServiceBackend with port 8080": {
ServiceBackend: &networking.IngressServiceBackend{
Name: "test",
Port: networking.ServiceBackendPort{
Number: 8080,
},
},
wantName: "test",
wantPort: intstr.IntOrString{
Type: intstr.Int,
IntVal: 8080,
},
},
"ingressServiceBackend with port name a-svc": {
ServiceBackend: &networking.IngressServiceBackend{
Name: "test",
Port: networking.ServiceBackendPort{
Name: "a-svc",
},
},
wantName: "test",
wantPort: intstr.IntOrString{
Type: intstr.String,
StrVal: "a-svc",
},
},
}

for title, testCase := range testcases {
gotName, gotPort := serviceToNameAndPort(testCase.ServiceBackend)
if gotName != testCase.wantName {
t.Fatalf("%s: expected '%v' but returned %v", title, testCase.wantName, gotName)
}
if gotPort != testCase.wantPort {
t.Fatalf("%s: expected '%v' but returned %v", title, testCase.wantPort, gotPort)
}
}
}
4 changes: 4 additions & 0 deletions internal/ingress/controller/template/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -988,6 +988,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
}
}

Expand Down Expand Up @@ -1022,6 +1024,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
Expand Down
120 changes: 120 additions & 0 deletions internal/ingress/controller/template/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -940,6 +940,76 @@ func TestGetIngressInformation(t *testing.T) {
10,
&ingressInformation{},
},
"valid ingress definition with name validIng in namespace default using a service with name a-svc port number 8080": {
&ingress.Ingress{
Ingress: networking.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "validIng",
Namespace: apiv1.NamespaceDefault,
Annotations: map[string]string{
"ingress.annotation": "ok",
},
},
Spec: networking.IngressSpec{
DefaultBackend: &networking.IngressBackend{
Service: &networking.IngressServiceBackend{
Name: "a-svc",
Port: networking.ServiceBackendPort{
Number: 8080,
},
},
},
},
},
},
"host1",
"",
&ingressInformation{
Namespace: "default",
Rule: "validIng",
Path: "/",
Annotations: map[string]string{
"ingress.annotation": "ok",
},
Service: "a-svc",
ServicePort: "8080",
},
},
"valid ingress definition with name validIng in namespace default using a service with name a-svc port name b-svc": {
&ingress.Ingress{
Ingress: networking.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "validIng",
Namespace: apiv1.NamespaceDefault,
Annotations: map[string]string{
"ingress.annotation": "ok",
},
},
Spec: networking.IngressSpec{
DefaultBackend: &networking.IngressBackend{
Service: &networking.IngressServiceBackend{
Name: "a-svc",
Port: networking.ServiceBackendPort{
Name: "b-svc",
},
},
},
},
},
},
"host1",
"",
&ingressInformation{
Namespace: "default",
Rule: "validIng",
Path: "/",
Annotations: map[string]string{
"ingress.annotation": "ok",
},
Service: "a-svc",
ServicePort: "b-svc",
},
},
"valid ingress definition with name validIng in namespace default": {
&ingress.Ingress{
Ingress: networking.Ingress{
Expand Down Expand Up @@ -1021,6 +1091,56 @@ func TestGetIngressInformation(t *testing.T) {
ServicePort: "80",
},
},
"valid ingress definition with name demo in namespace something and path /ok using a service with name b-svc port name b-svc-80": {
&ingress.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",
PathType: &pathPrefix,
Backend: networking.IngressBackend{
Service: &networking.IngressServiceBackend{
Name: "b-svc",
Port: networking.ServiceBackendPort{
Name: "b-svc-80",
},
},
},
},
},
},
},
},
{},
},
},
},
},
"foo.bar",
"/ok",
&ingressInformation{
Namespace: "something",
Rule: "demo",
Annotations: map[string]string{
"ingress.annotation": "ok",
},
Service: "b-svc",
ServicePort: "b-svc-80",
},
},
}

for title, testCase := range testcases {
Expand Down