Added EventPolicy Webhook (#8091)

* Added EventPolicy Webhook Signed-off-by: Dharmjit Singh <dharmjit.bti@gmail.com> * eventpolicy webhook validation for oidc feature will be called for create or spec updates * fixed pr comments Signed-off-by: Dharmjit Singh <dharmjit.bti@gmail.com> * Include context in channel tests for oidc feature disabled Signed-off-by: Dharmjit Singh <dharmjit.bti@gmail.com> --------- Signed-off-by: Dharmjit Singh <dharmjit.bti@gmail.com>
knative · Jul 19, 2024 · 1e4183f · 1e4183f
1 parent 2cc0a5b
commit 1e4183f
Show file tree

Hide file tree

Showing 4 changed files with 83 additions and 4 deletions.
diff --git a/cmd/webhook/main.go b/cmd/webhook/main.go
@@ -26,6 +26,7 @@ import (
 	kubeclient "knative.dev/pkg/client/injection/kube/client"
 	configmapinformer "knative.dev/pkg/client/injection/kube/informers/core/v1/configmap/filtered"
 
+	eventingv1alpha1 "knative.dev/eventing/pkg/apis/eventing/v1alpha1"
 	eventingv1beta3 "knative.dev/eventing/pkg/apis/eventing/v1beta3"
 	"knative.dev/eventing/pkg/apis/feature"
 	"knative.dev/eventing/pkg/apis/sinks"
@@ -75,6 +76,8 @@ func init() {
 
 var ourTypes = map[schema.GroupVersionKind]resourcesemantics.GenericCRD{
 	// For group eventing.knative.dev.
+	// v1alpha1
+	eventingv1alpha1.SchemeGroupVersion.WithKind("EventPolicy"): &eventingv1alpha1.EventPolicy{},
 	// v1beta1
 	eventingv1beta1.SchemeGroupVersion.WithKind("EventType"): &eventingv1beta1.EventType{},
 	// v1beta2

diff --git a/pkg/apis/eventing/v1alpha1/eventpolicy_validation.go b/pkg/apis/eventing/v1alpha1/eventpolicy_validation.go
@@ -20,10 +20,18 @@ import (
 	"context"
 	"strings"
 
+	"knative.dev/eventing/pkg/apis/feature"
 	"knative.dev/pkg/apis"
 )
 
 func (ep *EventPolicy) Validate(ctx context.Context) *apis.FieldError {
+	// To not allow creation or spec updates of EventPolicy CRs
+	// if the oidc-authentication feature is not enabled
+	if apis.IsInCreate(ctx) || (apis.IsInUpdate(ctx) && apis.IsInSpec(ctx)) {
+		if !feature.FromContext(ctx).IsOIDCAuthentication() {
+			return apis.ErrGeneric("oidc-authentication feature not enabled")
+		}
+	}
 	return ep.Spec.Validate(ctx).ViaField("spec")
 }
 

diff --git a/pkg/apis/eventing/v1alpha1/eventpolicy_validation_test.go b/pkg/apis/eventing/v1alpha1/eventpolicy_validation_test.go
@@ -21,11 +21,57 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"knative.dev/eventing/pkg/apis/feature"
 	"knative.dev/pkg/apis"
 	"knative.dev/pkg/ptr"
 )
 
-func TestEventPolicySpecValidation(t *testing.T) {
+func TestEventPolicySpecValidationWithOIDCAuthenticationFeatureFlagDisabled(t *testing.T) {
+	tests := []struct {
+		name string
+		ep   *EventPolicy
+		want *apis.FieldError
+	}{
+		{
+			name: "valid, from.sub exactly '*'",
+			ep: &EventPolicy{
+				Spec: EventPolicySpec{
+					From: []EventPolicySpecFrom{{
+						Sub: ptr.String("*"),
+					}},
+				},
+			},
+			want: func() *apis.FieldError {
+				return apis.ErrGeneric("oidc-authentication feature not enabled")
+			}(),
+		},
+		{
+			name: "invalid, missing from.ref and from.sub",
+			ep: &EventPolicy{
+				Spec: EventPolicySpec{
+					From: []EventPolicySpecFrom{{}},
+				},
+			},
+			want: func() *apis.FieldError {
+				return apis.ErrGeneric("oidc-authentication feature not enabled")
+			}(),
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			ctx := feature.ToContext(context.TODO(), feature.Flags{
+				feature.OIDCAuthentication: feature.Disabled,
+			})
+			ctx = apis.WithinCreate(ctx)
+			got := test.ep.Validate(ctx)
+			if diff := cmp.Diff(test.want.Error(), got.Error()); diff != "" {
+				t.Errorf("%s: Validate EventPolicySpec (-want, +got) = %v", test.name, diff)
+			}
+		})
+	}
+}
+
+func TestEventPolicySpecValidationWithOIDCAuthenticationFeatureFlagEnabled(t *testing.T) {
 	tests := []struct {
 		name string
 		ep   *EventPolicy
@@ -100,7 +146,7 @@ func TestEventPolicySpecValidation(t *testing.T) {
 			}(),
 		},
 		{
-			name: "invalid, bot from.ref and from.sub set",
+			name: "invalid, both from.ref and from.sub set for the same list element",
 			ep: &EventPolicy{
 				Spec: EventPolicySpec{
 					From: []EventPolicySpecFrom{{
@@ -129,7 +175,7 @@ func TestEventPolicySpecValidation(t *testing.T) {
 			}(),
 		},
 		{
-			name: "invalid, both to.ref and to.selector set",
+			name: "invalid, both to.ref and to.selector set for the same list element",
 			ep: &EventPolicy{
 				Spec: EventPolicySpec{
 					To: []EventPolicySpecTo{
@@ -252,7 +298,10 @@ func TestEventPolicySpecValidation(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			got := test.ep.Validate(context.TODO())
+			ctx := feature.ToContext(context.TODO(), feature.Flags{
+				feature.OIDCAuthentication: feature.Enabled,
+			})
+			got := test.ep.Validate(ctx)
 			if diff := cmp.Diff(test.want.Error(), got.Error()); diff != "" {
 				t.Errorf("%s: Validate EventPolicySpec (-want, +got) = %v", test.name, diff)
 			}

diff --git a/pkg/reconciler/channel/channel_test.go b/pkg/reconciler/channel/channel_test.go
@@ -20,6 +20,7 @@ import (
 	"testing"
 
 	eventingv1alpha1 "knative.dev/eventing/pkg/apis/eventing/v1alpha1"
+	"knative.dev/eventing/pkg/apis/feature"
 
 	v1 "knative.dev/eventing/pkg/apis/messaging/v1"
 
@@ -167,6 +168,9 @@ func TestReconcile(t *testing.T) {
 				WithChannelAddress(&backingChannelAddressable),
 				WithChannelEventPoliciesReadyBecauseOIDCDisabled()),
 		}},
+		Ctx: feature.ToContext(context.Background(), feature.Flags{
+			feature.OIDCAuthentication: feature.Disabled,
+		}),
 	}, {
 		Name: "Already reconciled",
 		Key:  testKey,
@@ -190,6 +194,9 @@ func TestReconcile(t *testing.T) {
 				WithInMemoryChannelDLSUnknown(),
 				WithInMemoryChannelEventPoliciesReady()),
 		},
+		Ctx: feature.ToContext(context.Background(), feature.Flags{
+			feature.OIDCAuthentication: feature.Disabled,
+		}),
 	}, {
 		Name: "Backing channel created",
 		Key:  testKey,
@@ -214,6 +221,9 @@ func TestReconcile(t *testing.T) {
 				WithBackingChannelUnknown("BackingChannelNotConfigured", "BackingChannel has not yet been reconciled."),
 				WithChannelEventPoliciesReadyBecauseOIDCDisabled()),
 		}},
+		Ctx: feature.ToContext(context.Background(), feature.Flags{
+			feature.OIDCAuthentication: feature.Disabled,
+		}),
 	}, {
 		Name: "Backing channel created with delivery",
 		Key:  testKey,
@@ -265,6 +275,9 @@ func TestReconcile(t *testing.T) {
 				WithBackingChannelUnknown("BackingChannelNotConfigured", "BackingChannel has not yet been reconciled."),
 				WithChannelEventPoliciesReadyBecauseOIDCDisabled()),
 		}},
+		Ctx: feature.ToContext(context.Background(), feature.Flags{
+			feature.OIDCAuthentication: feature.Disabled,
+		}),
 	}, {
 		Name: "Generation Bump",
 		Key:  testKey,
@@ -300,6 +313,9 @@ func TestReconcile(t *testing.T) {
 				WithChannelObservedGeneration(42),
 				WithChannelEventPoliciesReadyBecauseOIDCDisabled()),
 		}},
+		Ctx: feature.ToContext(context.Background(), feature.Flags{
+			feature.OIDCAuthentication: feature.Disabled,
+		}),
 	}, {
 		Name: "Updating subscribers statuses",
 		Key:  testKey,
@@ -335,6 +351,9 @@ func TestReconcile(t *testing.T) {
 				WithChannelDLSUnknown(),
 				WithChannelEventPoliciesReadyBecauseOIDCDisabled()),
 		}},
+		Ctx: feature.ToContext(context.Background(), feature.Flags{
+			feature.OIDCAuthentication: feature.Disabled,
+		}),
 	}, {
 		Name: "Should provision applying EventPolicies",
 		Key:  testKey,