Skip to content

Commit

Permalink
Fix incorrect service name being set
Browse files Browse the repository at this point in the history 
Fix: kubernetes#10210

This handles the case where multiple rules have identical paths, but
differing types.
  • Loading branch information
@adrianmoisey
adrianmoisey committed Apr 22, 2024
1 parent 48fbdfe commit ca7e1e6
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 3 deletions.
12 changes: 11 additions & 1 deletion internal/ingress/controller/template/template.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
text_template "text/template"

networkingv1 "k8s.io/api/networking/v1"
v1 "k8s.io/api/networking/v1"
"k8s.io/apimachinery/pkg/util/sets"
"k8s.io/klog/v2"

Expand Down Expand Up @@ -1060,7 +1061,7 @@ func (info *ingressInformation) Equal(other *ingressInformation) bool {
return true
}

func getIngressInformation(i, h, p interface{}) *ingressInformation {
func getIngressInformation(i, h, p, t interface{}) *ingressInformation {
ing, ok := i.(*ingress.Ingress)
if !ok {
klog.Errorf("expected an '*ingress.Ingress' type but %T was returned", i)
Expand All @@ -1079,6 +1080,11 @@ func getIngressInformation(i, h, p interface{}) *ingressInformation {
return &ingressInformation{}
}

ingressPathType, ok := t.(*v1.PathType)
if !ok {
klog.Errorf("expected a '*v1.PathType' type but %T was returned", t)
}

if ing == nil {
return &ingressInformation{}
}
Expand Down Expand Up @@ -1127,6 +1133,10 @@ func getIngressInformation(i, h, p interface{}) *ingressInformation {
continue
}

if *ingressPathType != *rPath.PathType {
continue
}

if rPath.Backend.Service == nil {
continue
}
Expand Down
69 changes: 68 additions & 1 deletion internal/ingress/controller/template/template_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ func init() {

var (
pathPrefix networking.PathType = networking.PathTypePrefix
pathExact networking.PathType = networking.PathTypeExact

// TODO: add tests for SSLPassthrough
tmplFuncTestcases = map[string]struct {
Expand Down Expand Up @@ -1157,18 +1158,21 @@ func TestGetIngressInformation(t *testing.T) {
Ingress interface{}
Host string
Path interface{}
PathType interface{}
Expected *ingressInformation
}{
"wrong ingress type": {
"wrongtype",
"host1",
"/ok",
"",
&ingressInformation{},
},
"wrong path type": {
&ingress.Ingress{},
"host1",
10,
"",
&ingressInformation{},
},
"valid ingress definition with name validIng in namespace default using a service with name a-svc port number 8080": {
Expand All @@ -1195,6 +1199,7 @@ func TestGetIngressInformation(t *testing.T) {
},
"host1",
"",
"",
&ingressInformation{
Namespace: "default",
Rule: "validIng",
Expand Down Expand Up @@ -1230,6 +1235,7 @@ func TestGetIngressInformation(t *testing.T) {
},
"host1",
"",
"",
&ingressInformation{
Namespace: "default",
Rule: "validIng",
Expand Down Expand Up @@ -1262,6 +1268,7 @@ func TestGetIngressInformation(t *testing.T) {
},
"host1",
"",
"",
&ingressInformation{
Namespace: "default",
Rule: "validIng",
Expand Down Expand Up @@ -1312,6 +1319,7 @@ func TestGetIngressInformation(t *testing.T) {
},
"foo.bar",
"/ok",
&pathPrefix,
&ingressInformation{
Namespace: "something",
Rule: "demo",
Expand Down Expand Up @@ -1362,6 +1370,7 @@ func TestGetIngressInformation(t *testing.T) {
},
"foo.bar",
"/ok",
&pathPrefix,
&ingressInformation{
Namespace: "something",
Rule: "demo",
Expand Down Expand Up @@ -1407,6 +1416,7 @@ func TestGetIngressInformation(t *testing.T) {
},
"foo.bar",
"/ok",
&pathPrefix,
&ingressInformation{
Namespace: "something",
Rule: "demo",
Expand Down Expand Up @@ -1462,6 +1472,7 @@ func TestGetIngressInformation(t *testing.T) {
},
"foo.bar",
"/oksvc",
&pathPrefix,
&ingressInformation{
Namespace: "something",
Rule: "demo",
Expand All @@ -1472,10 +1483,66 @@ func TestGetIngressInformation(t *testing.T) {
ServicePort: "b-svc-80",
},
},
"valid ingress definition with name demo in namespace something and two path / with Prefix and Exact": {
&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: "/",
PathType: &pathPrefix,
Backend: networking.IngressBackend{
Service: &networking.IngressServiceBackend{
Name: "a-svc",
},
},
},
{
Path: "/",
PathType: &pathExact,
Backend: networking.IngressBackend{
Service: &networking.IngressServiceBackend{
Name: "b-svc",
},
},
},
},
},
},
},
},
},
},
},
"foo.bar",
"/",
&pathExact,
&ingressInformation{
Path: "/",
Namespace: "something",
Rule: "demo",
Annotations: map[string]string{
"ingress.annotation": "ok",
},
Service: "b-svc",
},
},
}

for title, testCase := range testcases {
info := getIngressInformation(testCase.Ingress, testCase.Host, testCase.Path)
info := getIngressInformation(testCase.Ingress, testCase.Host, testCase.Path, testCase.PathType)

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 rootfs/etc/nginx/template/nginx.tmpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1247,7 +1247,7 @@ stream {
{{ end }}

location {{ $path }} {
{{ $ing := (getIngressInformation $location.Ingress $server.Hostname $location.IngressPath) }}
{{ $ing := (getIngressInformation $location.Ingress $server.Hostname $location.IngressPath $location.PathType) }}
set $namespace {{ $ing.Namespace | quote}};
set $ingress_name {{ $ing.Rule | quote }};
set $service_name {{ $ing.Service | quote }};
Expand Down

0 comments on commit ca7e1e6

Please sign in to comment.