validation ingress condition annotation in validation webhook (kubern…

…etes-sigs#2735)
adammw · Nov 9, 2023 · f944783 · f944783
1 parent d85feb1
commit f944783
Show file tree

Hide file tree

Showing 4 changed files with 152 additions and 2 deletions.
diff --git a/pkg/ingress/config_types.go b/pkg/ingress/config_types.go
@@ -322,7 +322,7 @@ type RuleCondition struct {
 	SourceIPConfig *SourceIPConditionConfig `json:"sourceIPConfig"`
 }
 
-func (c *RuleCondition) validate() error {
+func (c *RuleCondition) Validate() error {
 	switch c.Field {
 	case RuleConditionFieldHostHeader:
 		if c.HostHeaderConfig == nil {

diff --git a/pkg/ingress/enhanced_backend_builder.go b/pkg/ingress/enhanced_backend_builder.go
@@ -165,7 +165,7 @@ func (b *defaultEnhancedBackendBuilder) buildConditions(_ context.Context, ingAn
 		return nil, err
 	}
 	for _, condition := range conditions {
-		if err := condition.validate(); err != nil {
+		if err := condition.Validate(); err != nil {
 			return nil, err
 		}
 	}

diff --git a/webhooks/networking/ingress_validator.go b/webhooks/networking/ingress_validator.go
@@ -2,6 +2,7 @@ package networking
 
 import (
 	"context"
+	"fmt"
 
 	awssdk "github.com/aws/aws-sdk-go/aws"
 	"github.com/go-logr/logr"
@@ -59,6 +60,9 @@ func (v *ingressValidator) ValidateCreate(ctx context.Context, obj runtime.Objec
 	if err := v.checkIngressClassUsage(ctx, ing, nil); err != nil {
 		return err
 	}
+	if err := v.checkIngressAnnotationConditions(ing); err != nil {
+		return err
+	}
 	return nil
 }
 
@@ -74,6 +78,9 @@ func (v *ingressValidator) ValidateUpdate(ctx context.Context, obj runtime.Objec
 	if err := v.checkIngressClassUsage(ctx, ing, oldIng); err != nil {
 		return err
 	}
+	if err := v.checkIngressAnnotationConditions(ing); err != nil {
+		return err
+	}
 	return nil
 }
 
@@ -164,6 +171,33 @@ func (v *ingressValidator) checkIngressClassUsage(ctx context.Context, ing *netw
 	return nil
 }
 
+// checkGroupNameAnnotationUsage checks the validity of "conditions.${conditions-name}" annotation.
+func (v *ingressValidator) checkIngressAnnotationConditions(ing *networking.Ingress) error {
+	for _, rule := range ing.Spec.Rules {
+		for _, path := range rule.HTTP.Paths {
+			var conditions []ingress.RuleCondition
+			annotationKey := fmt.Sprintf("conditions.%v", path.Backend.Service.Name)
+			_, err := v.annotationParser.ParseJSONAnnotation(annotationKey, &conditions, ing.Annotations)
+			if err != nil {
+				return err
+			}
+
+			for _, condition := range conditions {
+				if err := condition.Validate(); err != nil {
+					return fmt.Errorf("ignoring Ingress %s/%s since invalid alb.ingress.kubernetes.io/conditions.%s annotation: %w",
+						ing.Namespace,
+						ing.Name,
+						path.Backend.Service.Name,
+						err,
+					)
+				}
+			}
+		}
+	}
+
+	return nil
+}
+
 // +kubebuilder:webhook:path=/validate-networking-v1-ingress,mutating=false,failurePolicy=fail,groups=networking.k8s.io,resources=ingresses,verbs=create;update,versions=v1,name=vingress.elbv2.k8s.aws,sideEffects=None,matchPolicy=Equivalent,webhookVersions=v1,admissionReviewVersions=v1beta1
 
 func (v *ingressValidator) SetupWithManager(mgr ctrl.Manager) {

diff --git a/webhooks/networking/ingress_validator_test.go b/webhooks/networking/ingress_validator_test.go
@@ -787,3 +787,119 @@ func Test_ingressValidator_checkIngressClassUsage(t *testing.T) {
 		})
 	}
 }
+
+func Test_ingressValidator_checkIngressAnnotationConditions(t *testing.T) {
+	type fields struct {
+		disableIngressGroupAnnotation bool
+	}
+	type args struct {
+		ing *networking.Ingress
+	}
+	tests := []struct {
+		name    string
+		fields  fields
+		args    args
+		wantErr error
+	}{
+		{
+			name: "ingress has valid condition",
+			fields: fields{
+				disableIngressGroupAnnotation: false,
+			},
+			args: args{
+				ing: &networking.Ingress{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: "ns-1",
+						Name:      "ing-1",
+						Annotations: map[string]string{
+							"alb.ingress.kubernetes.io/condition.svc-1": `[{"field":"query-string","queryStringConfig":{"values":[{"key":"paramA","value":"paramAValue"}]}}]`,
+						},
+					},
+					Spec: networking.IngressSpec{
+						Rules: []networking.IngressRule{
+							{
+								IngressRuleValue: networking.IngressRuleValue{
+									HTTP: &networking.HTTPIngressRuleValue{
+										Paths: []networking.HTTPIngressPath{
+											{
+												Path: "/ing-1-path",
+												Backend: networking.IngressBackend{
+													Service: &networking.IngressServiceBackend{
+														Name: "svc-1",
+														Port: networking.ServiceBackendPort{
+															Name: "https",
+														},
+													},
+												},
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			wantErr: nil,
+		},
+		{
+			name: "ingress has invalid condition",
+			fields: fields{
+				disableIngressGroupAnnotation: false,
+			},
+			args: args{
+				ing: &networking.Ingress{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: "ns-1",
+						Name:      "ing-1",
+						Annotations: map[string]string{
+							"alb.ingress.kubernetes.io/conditions.svc-1": `[{"field":"query-string","queryStringConfig":{"values":[{"key":"paramA","value":""}]}}]`,
+						},
+					},
+					Spec: networking.IngressSpec{
+						Rules: []networking.IngressRule{
+							{
+								IngressRuleValue: networking.IngressRuleValue{
+									HTTP: &networking.HTTPIngressRuleValue{
+										Paths: []networking.HTTPIngressPath{
+											{
+												Path: "/ing-1-path",
+												Backend: networking.IngressBackend{
+													Service: &networking.IngressServiceBackend{
+														Name: "svc-1",
+														Port: networking.ServiceBackendPort{
+															Name: "https",
+														},
+													},
+												},
+											},
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			wantErr: errors.New("ignoring Ingress ns-1/ing-1 since invalid alb.ingress.kubernetes.io/conditions.svc-1 annotation: invalid queryStringConfig: value cannot be empty"),
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			annotationParser := annotations.NewSuffixAnnotationParser("alb.ingress.kubernetes.io")
+			classAnnotationMatcher := ingress.NewDefaultClassAnnotationMatcher("alb")
+			v := &ingressValidator{
+				annotationParser:              annotationParser,
+				classAnnotationMatcher:        classAnnotationMatcher,
+				disableIngressGroupAnnotation: tt.fields.disableIngressGroupAnnotation,
+				logger:                        logr.Discard(),
+			}
+			err := v.checkIngressAnnotationConditions(tt.args.ing)
+			if tt.wantErr != nil {
+				assert.EqualError(t, err, tt.wantErr.Error())
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}