Fix IngressClass logic for newer releases (#7341)

* Fix IngressClass logic for newer releases

Signed-off-by: Ricardo Pchevuzinske Katz <ricardo.katz@gmail.com>

* Change e2e tests for the new IngressClass presence

* Fix chart and admission tests

Signed-off-by: Ricardo Pchevuzinske Katz <ricardo.katz@gmail.com>

* Fix helm chart test

Signed-off-by: Ricardo Pchevuzinske Katz <ricardo.katz@gmail.com>

* Fix reviews

* Remove ingressclass code from admission
This commit is contained in:
Ricardo Katz 2021-07-28 18:58:46 -03:00 committed by GitHub
parent 0d57e87819
commit cef147a24d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
68 changed files with 1450 additions and 637 deletions

View file

@ -37,13 +37,47 @@ import (
"sigs.k8s.io/controller-runtime/pkg/envtest"
"k8s.io/ingress-nginx/internal/ingress"
"k8s.io/ingress-nginx/internal/ingress/annotations/class"
"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
"k8s.io/ingress-nginx/internal/ingress/controller/ingressclass"
"k8s.io/ingress-nginx/test/e2e/framework"
)
var pathPrefix networking.PathType = networking.PathTypePrefix
var DefaultClassConfig = &ingressclass.IngressClassConfiguration{
Controller: ingressclass.DefaultControllerName,
AnnotationValue: ingressclass.DefaultAnnotationValue,
WatchWithoutClass: false,
}
var (
commonIngressSpec = networking.IngressSpec{
Rules: []networking.IngressRule{
{
Host: "dummy",
IngressRuleValue: networking.IngressRuleValue{
HTTP: &networking.HTTPIngressRuleValue{
Paths: []networking.HTTPIngressPath{
{
Path: "/",
PathType: &pathPrefix,
Backend: networking.IngressBackend{
Service: &networking.IngressServiceBackend{
Name: "http-svc",
Port: networking.ServiceBackendPort{
Number: 80,
},
},
},
},
},
},
},
},
},
}
)
func TestStore(t *testing.T) {
//TODO: move env definition to docker image?
os.Setenv("KUBEBUILDER_ASSETS", "/usr/local/bin")
@ -86,7 +120,8 @@ func TestStore(t *testing.T) {
10*time.Minute,
clientSet,
updateCh,
false)
false,
DefaultClassConfig)
storer.Run(stopCh)
@ -116,9 +151,10 @@ func TestStore(t *testing.T) {
}
})
t.Run("should return one event for add, update and delete of ingress", func(t *testing.T) {
t.Run("should return no event for add, update and delete of ingress as the existing ingressclass is not the expected", func(t *testing.T) {
ns := createNamespace(clientSet, t)
defer deleteNamespace(ns, clientSet, t)
createConfigMap(clientSet, ns, t)
stopCh := make(chan struct{})
@ -163,40 +199,20 @@ func TestStore(t *testing.T) {
10*time.Minute,
clientSet,
updateCh,
false)
false,
DefaultClassConfig)
storer.Run(stopCh)
ic := createIngressClass(clientSet, t, "not-k8s.io/not-ingress-nginx")
defer deleteIngressClass(ic, clientSet, t)
validSpec := commonIngressSpec
validSpec.IngressClassName = &ic
ing := ensureIngress(&networking.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "dummy",
Name: "dummy-no-class",
Namespace: ns,
},
Spec: networking.IngressSpec{
Rules: []networking.IngressRule{
{
Host: "dummy",
IngressRuleValue: networking.IngressRuleValue{
HTTP: &networking.HTTPIngressRuleValue{
Paths: []networking.HTTPIngressPath{
{
Path: "/",
PathType: &pathPrefix,
Backend: networking.IngressBackend{
Service: &networking.IngressServiceBackend{
Name: "http-svc",
Port: networking.ServiceBackendPort{
Number: 80,
},
},
},
},
},
},
},
},
},
},
Spec: validSpec,
}, clientSet, t)
err := framework.WaitForIngressInNamespace(clientSet, ns, ing.Name)
@ -205,40 +221,113 @@ func TestStore(t *testing.T) {
}
time.Sleep(1 * time.Second)
// create an invalid ingress (different class)
ni := ing.DeepCopy()
ni.Spec.Rules[0].Host = "update-dummy"
_ = ensureIngress(ni, clientSet, t)
if err != nil {
t.Errorf("error creating ingress: %v", err)
}
// Secret takes a bit to update
time.Sleep(3 * time.Second)
err = clientSet.NetworkingV1().Ingresses(ni.Namespace).Delete(context.TODO(), ni.Name, metav1.DeleteOptions{})
if err != nil {
t.Errorf("error deleting ingress: %v", err)
}
err = framework.WaitForNoIngressInNamespace(clientSet, ni.Namespace, ni.Name)
if err != nil {
t.Errorf("error waiting for secret: %v", err)
}
time.Sleep(1 * time.Second)
if atomic.LoadUint64(&add) != 0 {
t.Errorf("expected 0 event of type Create but %v occurred", add)
}
if atomic.LoadUint64(&upd) != 0 {
t.Errorf("expected 0 event of type Update but %v occurred", upd)
}
if atomic.LoadUint64(&del) != 0 {
t.Errorf("expected 0 event of type Delete but %v occurred", del)
}
})
t.Run("should return one event for add, update and delete of ingress", func(t *testing.T) {
ns := createNamespace(clientSet, t)
defer deleteNamespace(ns, clientSet, t)
ic := createIngressClass(clientSet, t, ingressclass.DefaultControllerName)
defer deleteIngressClass(ic, clientSet, t)
createConfigMap(clientSet, ns, t)
stopCh := make(chan struct{})
updateCh := channels.NewRingChannel(1024)
var add uint64
var upd uint64
var del uint64
go func(ch *channels.RingChannel) {
for {
evt, ok := <-ch.Out()
if !ok {
return
}
e := evt.(Event)
if e.Obj == nil {
continue
}
if _, ok := e.Obj.(*networking.Ingress); !ok {
continue
}
switch e.Type {
case CreateEvent:
atomic.AddUint64(&add, 1)
case UpdateEvent:
atomic.AddUint64(&upd, 1)
case DeleteEvent:
atomic.AddUint64(&del, 1)
}
}
}(updateCh)
storer := New(
ns,
fmt.Sprintf("%v/config", ns),
fmt.Sprintf("%v/tcp", ns),
fmt.Sprintf("%v/udp", ns),
"",
10*time.Minute,
clientSet,
updateCh,
false,
DefaultClassConfig)
storer.Run(stopCh)
validSpec := commonIngressSpec
validSpec.IngressClassName = &ic
ing := ensureIngress(&networking.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "dummy-class",
Namespace: ns,
},
Spec: validSpec,
}, clientSet, t)
err := framework.WaitForIngressInNamespace(clientSet, ns, ing.Name)
if err != nil {
t.Errorf("error waiting for secret: %v", err)
}
time.Sleep(1 * time.Second)
// create an invalid ingress (no ingress class and no watchWithoutClass config)
invalidIngress := ensureIngress(&networking.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "custom-class",
Name: "no-class",
Namespace: ns,
Annotations: map[string]string{
class.IngressKey: "something",
},
},
Spec: networking.IngressSpec{
Rules: []networking.IngressRule{
{
Host: "dummy",
IngressRuleValue: networking.IngressRuleValue{
HTTP: &networking.HTTPIngressRuleValue{
Paths: []networking.HTTPIngressPath{
{
Path: "/",
PathType: &pathPrefix,
Backend: networking.IngressBackend{
Service: &networking.IngressServiceBackend{
Name: "http-svc",
Port: networking.ServiceBackendPort{
Number: 80,
},
},
},
},
},
},
},
},
},
},
Spec: commonIngressSpec,
}, clientSet, t)
defer deleteIngress(invalidIngress, clientSet, t)
@ -253,7 +342,7 @@ func TestStore(t *testing.T) {
err = clientSet.NetworkingV1().Ingresses(ni.Namespace).Delete(context.TODO(), ni.Name, metav1.DeleteOptions{})
if err != nil {
t.Errorf("error creating ingress: %v", err)
t.Errorf("error deleting ingress: %v", err)
}
err = framework.WaitForNoIngressInNamespace(clientSet, ni.Namespace, ni.Name)
@ -273,7 +362,7 @@ func TestStore(t *testing.T) {
}
})
t.Run("should not receive updates for ingress with invalid class", func(t *testing.T) {
t.Run("should return two events for add and delete and one for update of ingress and watch-without-class", func(t *testing.T) {
ns := createNamespace(clientSet, t)
defer deleteNamespace(ns, clientSet, t)
createConfigMap(clientSet, ns, t)
@ -311,6 +400,224 @@ func TestStore(t *testing.T) {
}
}(updateCh)
ingressClassconfig := &ingressclass.IngressClassConfiguration{
Controller: ingressclass.DefaultControllerName,
AnnotationValue: ingressclass.DefaultAnnotationValue,
WatchWithoutClass: true,
}
storer := New(
ns,
fmt.Sprintf("%v/config", ns),
fmt.Sprintf("%v/tcp", ns),
fmt.Sprintf("%v/udp", ns),
"",
10*time.Minute,
clientSet,
updateCh,
false,
ingressClassconfig)
storer.Run(stopCh)
validIngress1 := ensureIngress(&networking.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "ing1",
Namespace: ns,
},
Spec: commonIngressSpec,
}, clientSet, t)
err := framework.WaitForIngressInNamespace(clientSet, ns, validIngress1.Name)
if err != nil {
t.Errorf("error waiting for ingress: %v", err)
}
otherIngress := commonIngressSpec
otherIngress.Rules[0].Host = "other-ingress"
validIngress2 := ensureIngress(&networking.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "ing2",
Namespace: ns,
},
Spec: otherIngress,
}, clientSet, t)
err = framework.WaitForIngressInNamespace(clientSet, ns, validIngress2.Name)
if err != nil {
t.Errorf("error waiting for ingress: %v", err)
}
time.Sleep(1 * time.Second)
validIngressUpdated := validIngress1.DeepCopy()
validIngressUpdated.Spec.Rules[0].Host = "update-dummy"
_ = ensureIngress(validIngressUpdated, clientSet, t)
if err != nil {
t.Errorf("error updating ingress: %v", err)
}
// Secret takes a bit to update
time.Sleep(3 * time.Second)
err = clientSet.NetworkingV1().Ingresses(validIngressUpdated.Namespace).Delete(context.TODO(), validIngressUpdated.Name, metav1.DeleteOptions{})
if err != nil {
t.Errorf("error deleting ingress: %v", err)
}
err = clientSet.NetworkingV1().Ingresses(validIngress2.Namespace).Delete(context.TODO(), validIngress2.Name, metav1.DeleteOptions{})
if err != nil {
t.Errorf("error deleting ingress: %v", err)
}
err = framework.WaitForNoIngressInNamespace(clientSet, validIngressUpdated.Namespace, validIngressUpdated.Name)
if err != nil {
t.Errorf("error waiting for ingress deletion: %v", err)
}
err = framework.WaitForNoIngressInNamespace(clientSet, validIngress2.Namespace, validIngress2.Name)
if err != nil {
t.Errorf("error waiting for ingress deletion: %v", err)
}
time.Sleep(1 * time.Second)
if atomic.LoadUint64(&add) != 2 {
t.Errorf("expected 0 event of type Create but %v occurred", add)
}
if atomic.LoadUint64(&upd) != 1 {
t.Errorf("expected 0 event of type Update but %v occurred", upd)
}
if atomic.LoadUint64(&del) != 2 {
t.Errorf("expected 0 event of type Delete but %v occurred", del)
}
})
t.Run("should not receive updates for ingress with invalid class annotation", func(t *testing.T) {
ns := createNamespace(clientSet, t)
defer deleteNamespace(ns, clientSet, t)
createConfigMap(clientSet, ns, t)
stopCh := make(chan struct{})
updateCh := channels.NewRingChannel(1024)
var add uint64
var upd uint64
var del uint64
// TODO: This repeats a lot, transform in a local function
go func(ch *channels.RingChannel) {
for {
evt, ok := <-ch.Out()
if !ok {
return
}
e := evt.(Event)
if e.Obj == nil {
continue
}
if _, ok := e.Obj.(*networking.Ingress); !ok {
continue
}
switch e.Type {
case CreateEvent:
atomic.AddUint64(&add, 1)
case UpdateEvent:
atomic.AddUint64(&upd, 1)
case DeleteEvent:
atomic.AddUint64(&del, 1)
}
}
}(updateCh)
storer := New(
ns,
fmt.Sprintf("%v/config", ns),
fmt.Sprintf("%v/tcp", ns),
fmt.Sprintf("%v/udp", ns),
"",
10*time.Minute,
clientSet,
updateCh,
false,
DefaultClassConfig)
storer.Run(stopCh)
// create an invalid ingress (different class)
invalidIngress := ensureIngress(&networking.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "custom-class",
Namespace: ns,
Annotations: map[string]string{
ingressclass.IngressKey: "something",
},
},
Spec: commonIngressSpec,
}, clientSet, t)
err := framework.WaitForIngressInNamespace(clientSet, ns, invalidIngress.Name)
if err != nil {
t.Errorf("error waiting for ingress: %v", err)
}
time.Sleep(1 * time.Second)
invalidIngressUpdated := invalidIngress.DeepCopy()
invalidIngressUpdated.Spec.Rules[0].Host = "update-dummy"
_ = ensureIngress(invalidIngressUpdated, clientSet, t)
if err != nil {
t.Errorf("error creating ingress: %v", err)
}
// Secret takes a bit to update
time.Sleep(3 * time.Second)
if atomic.LoadUint64(&add) != 0 {
t.Errorf("expected 0 event of type Create but %v occurred", add)
}
if atomic.LoadUint64(&upd) != 0 {
t.Errorf("expected 0 event of type Update but %v occurred", upd)
}
if atomic.LoadUint64(&del) != 0 {
t.Errorf("expected 0 event of type Delete but %v occurred", del)
}
})
t.Run("should not receive updates for ingress with invalid class specification", func(t *testing.T) {
ns := createNamespace(clientSet, t)
defer deleteNamespace(ns, clientSet, t)
ic := createIngressClass(clientSet, t, ingressclass.DefaultControllerName)
defer deleteIngressClass(ic, clientSet, t)
createConfigMap(clientSet, ns, t)
stopCh := make(chan struct{})
updateCh := channels.NewRingChannel(1024)
var add uint64
var upd uint64
var del uint64
go func(ch *channels.RingChannel) {
for {
evt, ok := <-ch.Out()
if !ok {
return
}
e := evt.(Event)
if e.Obj == nil {
continue
}
if _, ok := e.Obj.(*networking.Ingress); !ok {
continue
}
switch e.Type {
case CreateEvent:
atomic.AddUint64(&add, 1)
case UpdateEvent:
atomic.AddUint64(&upd, 1)
case DeleteEvent:
atomic.AddUint64(&del, 1)
}
}
}(updateCh)
storer := New(
ns,
fmt.Sprintf("%v/config", ns),
@ -320,44 +627,20 @@ func TestStore(t *testing.T) {
10*time.Minute,
clientSet,
updateCh,
false)
false,
DefaultClassConfig)
storer.Run(stopCh)
invalidSpec := commonIngressSpec
invalidClassName := "blo123"
invalidSpec.IngressClassName = &invalidClassName
// create an invalid ingress (different class)
invalidIngress := ensureIngress(&networking.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "custom-class",
Namespace: ns,
Annotations: map[string]string{
class.IngressKey: "something",
},
},
Spec: networking.IngressSpec{
Rules: []networking.IngressRule{
{
Host: "dummy",
IngressRuleValue: networking.IngressRuleValue{
HTTP: &networking.HTTPIngressRuleValue{
Paths: []networking.HTTPIngressPath{
{
Path: "/",
PathType: &pathPrefix,
Backend: networking.IngressBackend{
Service: &networking.IngressServiceBackend{
Name: "http-svc",
Port: networking.ServiceBackendPort{
Number: 80,
},
},
},
},
},
},
},
},
},
},
Spec: invalidSpec,
}, clientSet, t)
err := framework.WaitForIngressInNamespace(clientSet, ns, invalidIngress.Name)
if err != nil {
@ -428,7 +711,8 @@ func TestStore(t *testing.T) {
10*time.Minute,
clientSet,
updateCh,
false)
false,
DefaultClassConfig)
storer.Run(stopCh)
@ -474,6 +758,8 @@ func TestStore(t *testing.T) {
t.Run("should receive events from secret referenced from ingress", func(t *testing.T) {
ns := createNamespace(clientSet, t)
defer deleteNamespace(ns, clientSet, t)
ic := createIngressClass(clientSet, t, ingressclass.DefaultControllerName)
defer deleteIngressClass(ic, clientSet, t)
createConfigMap(clientSet, ns, t)
stopCh := make(chan struct{})
@ -494,6 +780,11 @@ func TestStore(t *testing.T) {
if e.Obj == nil {
continue
}
// We should skip IngressClass events
if _, ok := e.Obj.(*networking.IngressClass); ok {
continue
}
switch e.Type {
case CreateEvent:
atomic.AddUint64(&add, 1)
@ -514,7 +805,8 @@ func TestStore(t *testing.T) {
10*time.Minute,
clientSet,
updateCh,
false)
false,
DefaultClassConfig)
storer.Run(stopCh)
@ -527,6 +819,7 @@ func TestStore(t *testing.T) {
Namespace: ns,
},
Spec: networking.IngressSpec{
IngressClassName: &ic,
TLS: []networking.IngressTLS{
{
SecretName: secretName,
@ -586,6 +879,8 @@ func TestStore(t *testing.T) {
t.Run("should create an ingress with a secret which does not exist", func(t *testing.T) {
ns := createNamespace(clientSet, t)
defer deleteNamespace(ns, clientSet, t)
ic := createIngressClass(clientSet, t, ingressclass.DefaultControllerName)
defer deleteIngressClass(ic, clientSet, t)
createConfigMap(clientSet, ns, t)
stopCh := make(chan struct{})
@ -606,6 +901,12 @@ func TestStore(t *testing.T) {
if e.Obj == nil {
continue
}
// We should skip IngressClass objects here
if _, ok := e.Obj.(*networking.IngressClass); ok {
continue
}
switch e.Type {
case CreateEvent:
atomic.AddUint64(&add, 1)
@ -626,7 +927,8 @@ func TestStore(t *testing.T) {
10*time.Minute,
clientSet,
updateCh,
false)
false,
DefaultClassConfig)
storer.Run(stopCh)
@ -639,6 +941,7 @@ func TestStore(t *testing.T) {
Namespace: ns,
},
Spec: networking.IngressSpec{
IngressClassName: &ic,
TLS: []networking.IngressTLS{
{
Hosts: secretHosts,
@ -733,6 +1036,33 @@ func deleteNamespace(ns string, clientSet kubernetes.Interface, t *testing.T) {
}
}
func createIngressClass(clientSet kubernetes.Interface, t *testing.T, controller string) string {
t.Helper()
ingressclass := &networking.IngressClass{
ObjectMeta: metav1.ObjectMeta{
Name: fmt.Sprintf("ingress-nginx-%v", time.Now().Unix()),
//Namespace: "xpto" // TODO: We don't support namespaced ingress-class yet
},
Spec: networking.IngressClassSpec{
Controller: controller,
},
}
ic, err := clientSet.NetworkingV1().IngressClasses().Create(context.TODO(), ingressclass, metav1.CreateOptions{})
if err != nil {
t.Errorf("error creating ingress class: %v", err)
}
return ic.Name
}
func deleteIngressClass(ic string, clientSet kubernetes.Interface, t *testing.T) {
t.Helper()
err := clientSet.NetworkingV1().IngressClasses().Delete(context.TODO(), ic, metav1.DeleteOptions{})
if err != nil {
t.Errorf("error deleting the ingress class: %v", err)
}
}
func createConfigMap(clientSet kubernetes.Interface, ns string, t *testing.T) string {
t.Helper()
@ -790,6 +1120,7 @@ func newStore(t *testing.T) *k8sStore {
return &k8sStore{
listers: &Lister{
// add more listers if needed
IngressClass: IngressClassLister{cache.NewStore(cache.MetaNamespaceKeyFunc)},
Ingress: IngressLister{cache.NewStore(cache.MetaNamespaceKeyFunc)},
IngressWithAnnotation: IngressWithAnnotationsLister{cache.NewStore(cache.DeletionHandlingMetaNamespaceKeyFunc)},
},
@ -867,18 +1198,18 @@ func TestUpdateSecretIngressMap(t *testing.T) {
func TestListIngresses(t *testing.T) {
s := newStore(t)
invalidIngressClass := "something"
validIngressClass := ingressclass.DefaultControllerName
ingressToIgnore := &ingress.Ingress{
Ingress: networking.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "test-2",
Namespace: "testns",
Annotations: map[string]string{
class.IngressKey: "something",
},
Name: "test-2",
Namespace: "testns",
CreationTimestamp: metav1.NewTime(time.Now()),
},
Spec: networking.IngressSpec{
IngressClassName: &invalidIngressClass,
DefaultBackend: &networking.IngressBackend{
Service: &networking.IngressServiceBackend{
Name: "demo",
@ -900,6 +1231,7 @@ func TestListIngresses(t *testing.T) {
CreationTimestamp: metav1.NewTime(time.Now()),
},
Spec: networking.IngressSpec{
IngressClassName: &validIngressClass,
Rules: []networking.IngressRule{
{
Host: "foo.bar",
@ -926,13 +1258,13 @@ func TestListIngresses(t *testing.T) {
}
s.listers.IngressWithAnnotation.Add(ingressWithoutPath)
ingressWithNginxClass := &ingress.Ingress{
ingressWithNginxClassAnnotation := &ingress.Ingress{
Ingress: networking.Ingress{
ObjectMeta: metav1.ObjectMeta{
Name: "test-4",
Namespace: "testns",
Annotations: map[string]string{
class.IngressKey: "nginx",
ingressclass.IngressKey: ingressclass.DefaultAnnotationValue,
},
CreationTimestamp: metav1.NewTime(time.Now()),
},
@ -963,7 +1295,7 @@ func TestListIngresses(t *testing.T) {
},
},
}
s.listers.IngressWithAnnotation.Add(ingressWithNginxClass)
s.listers.IngressWithAnnotation.Add(ingressWithNginxClassAnnotation)
ingresses := s.ListIngresses()