From 7d82903ce96c6685d7aee98141c89a1b4e0cd0e9 Mon Sep 17 00:00:00 2001 From: Laszlo Janosi Date: Fri, 7 Aug 2020 17:09:14 +0000 Subject: [PATCH] Fix panic in ingress class validation If an ingress had no class annotation, nor IngressClassName at all, and an IngressClass resource was created for the ingress-nginx there was a panic when the controller tried to check the IngressClassName of the Ingress. --- internal/ingress/annotations/class/main.go | 5 +- .../ingress/annotations/class/main_test.go | 58 ++++++++++++++----- 2 files changed, 46 insertions(+), 17 deletions(-) diff --git a/internal/ingress/annotations/class/main.go b/internal/ingress/annotations/class/main.go index ef8df711e..6b6bfa434 100644 --- a/internal/ingress/annotations/class/main.go +++ b/internal/ingress/annotations/class/main.go @@ -63,5 +63,8 @@ func IsValid(ing *networking.Ingress) bool { } // 4. with IngressClass - return k8s.IngressClass.Name == *ing.Spec.IngressClassName + if ing.Spec.IngressClassName != nil { + return k8s.IngressClass.Name == *ing.Spec.IngressClassName + } + return false } diff --git a/internal/ingress/annotations/class/main_test.go b/internal/ingress/annotations/class/main_test.go index f38eeb790..9aa891822 100644 --- a/internal/ingress/annotations/class/main_test.go +++ b/internal/ingress/annotations/class/main_test.go @@ -22,45 +22,71 @@ import ( api "k8s.io/api/core/v1" networking "k8s.io/api/networking/v1beta1" meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/ingress-nginx/internal/k8s" ) func TestIsValidClass(t *testing.T) { dc := DefaultClass ic := IngressClass + k8sic := k8s.IngressClass + v1Ready := k8s.IsIngressV1Ready // restore original values after the tests defer func() { DefaultClass = dc IngressClass = ic + k8s.IngressClass = k8sic + k8s.IsIngressV1Ready = v1Ready }() tests := []struct { ingress string controller string defClass string + annotation bool + k8sClass *networking.IngressClass + v1Ready bool isValid bool }{ - {"", "", "nginx", true}, - {"", "nginx", "nginx", true}, - {"nginx", "nginx", "nginx", true}, - {"custom", "custom", "nginx", true}, - {"", "killer", "nginx", false}, - {"custom", "nginx", "nginx", false}, + {"", "", "nginx", true, nil, false, true}, + {"", "nginx", "nginx", true, nil, false, true}, + {"nginx", "nginx", "nginx", true, nil, false, true}, + {"custom", "custom", "nginx", true, nil, false, true}, + {"", "killer", "nginx", true, nil, false, false}, + {"custom", "nginx", "nginx", true, nil, false, false}, + {"", "custom", "nginx", false, + &networking.IngressClass{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "custom", + }, + }, + false, false}, + {"", "custom", "nginx", false, + &networking.IngressClass{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "custom", + }, + }, + true, false}, } - ing := &networking.Ingress{ - ObjectMeta: meta_v1.ObjectMeta{ - Name: "foo", - Namespace: api.NamespaceDefault, - }, - } - - data := map[string]string{} - ing.SetAnnotations(data) for _, test := range tests { - ing.Annotations[IngressKey] = test.ingress + ing := &networking.Ingress{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "foo", + Namespace: api.NamespaceDefault, + }, + } + + data := map[string]string{} + ing.SetAnnotations(data) + if test.annotation { + ing.Annotations[IngressKey] = test.ingress + } IngressClass = test.controller DefaultClass = test.defClass + k8s.IngressClass = test.k8sClass + k8s.IsIngressV1Ready = test.v1Ready b := IsValid(ing) if b != test.isValid {