Skip to content

Commit 1c2faee

Browse files
M00nF1shM00nF1sh
committed
tolerate ingressClass
1 parent 4cb7510 commit 1c2faee

File tree

2 files changed

+139
-19
lines changed

2 files changed

+139
-19
lines changed

‎webhooks/networking/ingress_validator.go

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package networking
22

33
import (
44
"context"
5+
awssdk "github.com/aws/aws-sdk-go/aws"
56
"github.com/go-logr/logr"
67
"github.com/pkg/errors"
78
networking "k8s.io/api/networking/v1beta1"
@@ -54,7 +55,7 @@ func (v *ingressValidator) ValidateCreate(ctx context.Context, obj runtime.Objec
5455
if err := v.checkGroupNameAnnotationUsage(ing, nil); err != nil {
5556
return err
5657
}
57-
if err := v.checkIngressClassUsage(ctx, ing); err != nil {
58+
if err := v.checkIngressClassUsage(ctx, ing, nil); err != nil {
5859
return err
5960
}
6061
return nil
@@ -69,7 +70,7 @@ func (v *ingressValidator) ValidateUpdate(ctx context.Context, obj runtime.Objec
6970
if err := v.checkGroupNameAnnotationUsage(ing, oldIng); err != nil {
7071
return err
7172
}
72-
if err := v.checkIngressClassUsage(ctx, ing); err != nil {
73+
if err := v.checkIngressClassUsage(ctx, ing, oldIng); err != nil {
7374
return err
7475
}
7576
return nil
@@ -125,20 +126,38 @@ func (v *ingressValidator) checkGroupNameAnnotationUsage(ing *networking.Ingress
125126
usedInOldIng = true
126127
}
127128
}
128-
if !usedInOldIng && usedInNewIng {
129-
return errors.Errorf("new usage of `%s/%s` annotation is forbidden", annotations.AnnotationPrefixIngress, annotations.IngressSuffixGroupName)
130-
}
131-
if usedInOldIng && usedInNewIng && (oldGroupName != newGroupName) {
132-
return errors.Errorf("new value of `%s/%s` annotation is forbidden", annotations.AnnotationPrefixIngress, annotations.IngressSuffixGroupName)
129+
130+
if usedInNewIng {
131+
if !usedInOldIng || (newGroupName != oldGroupName) {
132+
return errors.Errorf("new usage of `%s/%s` annotation is forbidden", annotations.AnnotationPrefixIngress, annotations.IngressSuffixGroupName)
133+
}
133134
}
134135
return nil
135136
}
136137

137-
func (v *ingressValidator) checkIngressClassUsage(ctx context.Context, ing *networking.Ingress) error {
138+
// checkIngressClassUsage checks the usage of "ingressClassName" field.
139+
// if ingressClassName is mutated, it must refer to a existing & valid IngressClass.
140+
func (v *ingressValidator) checkIngressClassUsage(ctx context.Context, ing *networking.Ingress, oldIng *networking.Ingress) error {
141+
usedInNewIng := false
142+
usedInOldIng := false
143+
newIngressClassName := ""
144+
oldIngressClassName := ""
145+
138146
if ing.Spec.IngressClassName != nil {
139-
_, err := v.classLoader.Load(ctx, ing)
140-
if err != nil {
141-
return err
147+
usedInNewIng = true
148+
newIngressClassName = awssdk.StringValue(ing.Spec.IngressClassName)
149+
}
150+
if oldIng != nil && oldIng.Spec.IngressClassName != nil {
151+
usedInOldIng = true
152+
oldIngressClassName = awssdk.StringValue(oldIng.Spec.IngressClassName)
153+
}
154+
155+
if usedInNewIng {
156+
if !usedInOldIng || (newIngressClassName != oldIngressClassName) {
157+
_, err := v.classLoader.Load(ctx, ing)
158+
if err != nil {
159+
return err
160+
}
142161
}
143162
}
144163
return nil

‎webhooks/networking/ingress_validator_test.go

Lines changed: 109 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -448,7 +448,7 @@ func Test_ingressValidator_checkGroupNameAnnotationUsage(t *testing.T) {
448448
},
449449
},
450450
},
451-
wantErr: errors.New("new value of `alb.ingress.kubernetes.io/group.name` annotation is forbidden"),
451+
wantErr: errors.New("new usage of `alb.ingress.kubernetes.io/group.name` annotation is forbidden"),
452452
},
453453
}
454454
for _, tt := range tests {
@@ -479,7 +479,8 @@ func Test_ingressValidator_checkIngressClassUsage(t *testing.T) {
479479
}
480480

481481
type args struct {
482-
ing *networking.Ingress
482+
ing *networking.Ingress
483+
oldIng *networking.Ingress
483484
}
484485
tests := []struct {
485486
name string
@@ -488,7 +489,7 @@ func Test_ingressValidator_checkIngressClassUsage(t *testing.T) {
488489
wantErr error
489490
}{
490491
{
491-
name: "IngressClassName isn't specified",
492+
name: "ingress creation without IngressClassName",
492493
env: env{},
493494
args: args{
494495
ing: &networking.Ingress{
@@ -504,7 +505,7 @@ func Test_ingressValidator_checkIngressClassUsage(t *testing.T) {
504505
wantErr: nil,
505506
},
506507
{
507-
name: "IngressClass didn't exists",
508+
name: "ingress creation with IngressClassName that refers to non-existent IngressClass",
508509
env: env{},
509510
args: args{
510511
ing: &networking.Ingress{
@@ -520,7 +521,7 @@ func Test_ingressValidator_checkIngressClassUsage(t *testing.T) {
520521
wantErr: errors.New("invalid ingress class: ingressclasses.networking.k8s.io \"awesome-class\" not found"),
521522
},
522523
{
523-
name: "IngressClass exists but IngressClassParams unspecified",
524+
name: "ingress creation with IngressClassName that refers to IngressClass without params",
524525
env: env{
525526
ingClassList: []*networking.IngressClass{
526527
{
@@ -544,7 +545,7 @@ func Test_ingressValidator_checkIngressClassUsage(t *testing.T) {
544545
wantErr: nil,
545546
},
546547
{
547-
name: "IngressClass exists and IngressClassParams exists, and namespaceSelector mismatches",
548+
name: "ingress creation with IngressClassName that refers to IngressClass with IngressClassParams with mismatch namespaceSelector",
548549
env: env{
549550
nsList: []*corev1.Namespace{
550551
{
@@ -600,7 +601,7 @@ func Test_ingressValidator_checkIngressClassUsage(t *testing.T) {
600601
wantErr: errors.New("invalid ingress class: namespaceSelector of IngressClassParams awesome-class-params mismatch"),
601602
},
602603
{
603-
name: "IngressClass exists and IngressClassParams exists, and namespaceSelector matches",
604+
name: "ingress creation with IngressClassName that refers to IngressClass with IngressClassParams with matches namespaceSelector",
604605
env: env{
605606
nsList: []*corev1.Namespace{
606607
{
@@ -651,6 +652,106 @@ func Test_ingressValidator_checkIngressClassUsage(t *testing.T) {
651652
},
652653
wantErr: nil,
653654
},
655+
{
656+
name: "ingress updates with removed IngressClassName",
657+
env: env{},
658+
args: args{
659+
ing: &networking.Ingress{
660+
ObjectMeta: metav1.ObjectMeta{
661+
Namespace: "awesome-ns",
662+
Name: "awesome-ing",
663+
},
664+
Spec: networking.IngressSpec{
665+
IngressClassName: nil,
666+
},
667+
},
668+
oldIng: &networking.Ingress{
669+
ObjectMeta: metav1.ObjectMeta{
670+
Namespace: "awesome-ns",
671+
Name: "awesome-ing",
672+
},
673+
Spec: networking.IngressSpec{
674+
IngressClassName: awssdk.String("awesome-class"),
675+
},
676+
},
677+
},
678+
wantErr: nil,
679+
},
680+
{
681+
name: "ingress updates with changed IngressClassName that refers to non-existent IngressClass",
682+
env: env{},
683+
args: args{
684+
ing: &networking.Ingress{
685+
ObjectMeta: metav1.ObjectMeta{
686+
Namespace: "awesome-ns",
687+
Name: "awesome-ing",
688+
},
689+
Spec: networking.IngressSpec{
690+
IngressClassName: awssdk.String("awesome-class"),
691+
},
692+
},
693+
oldIng: &networking.Ingress{
694+
ObjectMeta: metav1.ObjectMeta{
695+
Namespace: "awesome-ns",
696+
Name: "awesome-ing",
697+
},
698+
Spec: networking.IngressSpec{
699+
IngressClassName: awssdk.String("old-awesome-class"),
700+
},
701+
},
702+
},
703+
wantErr: errors.New("invalid ingress class: ingressclasses.networking.k8s.io \"awesome-class\" not found"),
704+
},
705+
{
706+
name: "ingress updates with added IngressClassName that refers to non-existent IngressClass",
707+
env: env{},
708+
args: args{
709+
ing: &networking.Ingress{
710+
ObjectMeta: metav1.ObjectMeta{
711+
Namespace: "awesome-ns",
712+
Name: "awesome-ing",
713+
},
714+
Spec: networking.IngressSpec{
715+
IngressClassName: awssdk.String("awesome-class"),
716+
},
717+
},
718+
oldIng: &networking.Ingress{
719+
ObjectMeta: metav1.ObjectMeta{
720+
Namespace: "awesome-ns",
721+
Name: "awesome-ing",
722+
},
723+
Spec: networking.IngressSpec{
724+
IngressClassName: nil,
725+
},
726+
},
727+
},
728+
wantErr: errors.New("invalid ingress class: ingressclasses.networking.k8s.io \"awesome-class\" not found"),
729+
},
730+
{
731+
name: "ingress updates with unchanged IngressClassName that refers to non-existent IngressClass",
732+
env: env{},
733+
args: args{
734+
ing: &networking.Ingress{
735+
ObjectMeta: metav1.ObjectMeta{
736+
Namespace: "awesome-ns",
737+
Name: "awesome-ing",
738+
},
739+
Spec: networking.IngressSpec{
740+
IngressClassName: awssdk.String("awesome-class"),
741+
},
742+
},
743+
oldIng: &networking.Ingress{
744+
ObjectMeta: metav1.ObjectMeta{
745+
Namespace: "awesome-ns",
746+
Name: "awesome-ing",
747+
},
748+
Spec: networking.IngressSpec{
749+
IngressClassName: awssdk.String("awesome-class"),
750+
},
751+
},
752+
},
753+
wantErr: nil,
754+
},
654755
}
655756
for _, tt := range tests {
656757
t.Run(tt.name, func(t *testing.T) {
@@ -675,7 +776,7 @@ func Test_ingressValidator_checkIngressClassUsage(t *testing.T) {
675776
v := &ingressValidator{
676777
classLoader: ingress.NewDefaultClassLoader(k8sClient),
677778
}
678-
err := v.checkIngressClassUsage(ctx, tt.args.ing)
779+
err := v.checkIngressClassUsage(ctx, tt.args.ing, tt.args.oldIng)
679780
if tt.wantErr != nil {
680781
assert.EqualError(t, err, tt.wantErr.Error())
681782
} else {

0 commit comments

Comments
 (0)