Run webhook validation rules inside NKG control plane

deploy/manifests/nginx-gateway.yaml

@@ -17,6 +17,13 @@ rules:
   verbs:
   - list
   - watch
+- apiGroups:
+  - ""
+  resources:
+  - events
+  verbs:
+  - create
+  - patch
 - apiGroups:
   - discovery.k8s.io
   resources:

internal/manager/controllers.go

@@ -26,6 +26,7 @@ type controllerConfig struct {
 	k8sPredicate         predicate.Predicate
 	fieldIndices         index.FieldIndices
 	newReconciler        newReconcilerFunc
+	webhookValidator     reconciler.ValidatorFunc
 }
 
 type controllerOption func(*controllerConfig)
@@ -55,6 +56,12 @@ func withNewReconciler(newReconciler newReconcilerFunc) controllerOption {
 	}
 }
 
+func withWebhookValidator(validator reconciler.ValidatorFunc) controllerOption {
+	return func(cfg *controllerConfig) {
+		cfg.webhookValidator = validator
+	}
+}
+
 func defaultControllerConfig() controllerConfig {
 	return controllerConfig{
 		newReconciler: reconciler.NewImplementation,
@@ -65,7 +72,8 @@ func registerController(
 	ctx context.Context,
 	objectType client.Object,
 	mgr manager.Manager,
-	eventCh chan interface{},
+	eventCh chan<- interface{},
+	recorder reconciler.EventRecorder,
 	options ...controllerOption,
 ) error {
 	cfg := defaultControllerConfig()
@@ -92,6 +100,8 @@ func registerController(
 		ObjectType:           objectType,
 		EventCh:              eventCh,
 		NamespacedNameFilter: cfg.namespacedNameFilter,
+		WebhookValidator:     cfg.webhookValidator,
+		EventRecorder:        recorder,
 	}
 
 	err := builder.Complete(cfg.newReconciler(recCfg))

internal/manager/controllers_test.go

@@ -6,8 +6,12 @@ import (
 	"reflect"
 	"testing"
 
+	. "github.com/onsi/gomega"
+	"github.com/onsi/gomega/gcustom"
+	"github.com/onsi/gomega/types"
 	"k8s.io/apimachinery/pkg/runtime"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	"sigs.k8s.io/gateway-api/apis/v1beta1"
@@ -17,6 +21,7 @@ import (
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/manager/managerfakes"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/manager/predicate"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/reconciler"
+	"github.com/nginxinc/nginx-kubernetes-gateway/internal/reconciler/reconcilerfakes"
 )
 
 func TestRegisterController(t *testing.T) {
@@ -81,114 +86,69 @@ func TestRegisterController(t *testing.T) {
 	namespacedNameFilter := filter.CreateFilterForGatewayClass("test")
 	fieldIndexes := index.CreateEndpointSliceFieldIndices()
 
-	eventCh := make(chan interface{})
+	webhookValidator := createValidator(func(_ *v1beta1.HTTPRoute) field.ErrorList {
+		return nil
+	})
+
+	eventRecorder := &reconcilerfakes.FakeEventRecorder{}
+
+	eventCh := make(chan<- interface{})
+
+	beSameFunctionPointer := func(expected interface{}) types.GomegaMatcher {
+		return gcustom.MakeMatcher(func(f interface{}) (bool, error) {
+			// comparing functions is not allowed in Go, so we're comparing the pointers
+			return reflect.ValueOf(expected).Pointer() == reflect.ValueOf(f).Pointer(), nil
+		})
+	}
 
 	for _, test := range tests {
-		newReconciler := func(c reconciler.Config) *reconciler.Implementation {
-			if c.Getter != test.fakes.mgr.GetClient() {
-				t.Errorf(
-					"regiterController() created a reconciler config with Getter %p but expected %p for case of %q",
-					c.Getter,
-					test.fakes.mgr.GetClient(),
-					test.msg,
-				)
-			}
-			if c.ObjectType != objectType {
-				t.Errorf(
-					"registerController() created a reconciler config with ObjectType %T but expected %T for case of %q",
-					c.ObjectType,
-					objectType,
-					test.msg,
-				)
+		t.Run(test.msg, func(t *testing.T) {
+			g := NewGomegaWithT(t)
+
+			newReconciler := func(c reconciler.Config) *reconciler.Implementation {
+				g.Expect(c.Getter).To(BeIdenticalTo(test.fakes.mgr.GetClient()))
+				g.Expect(c.ObjectType).To(BeIdenticalTo(objectType))
+				g.Expect(c.EventCh).To(BeIdenticalTo(eventCh))
+				g.Expect(c.EventRecorder).To(BeIdenticalTo(eventRecorder))
+				g.Expect(c.WebhookValidator).Should(beSameFunctionPointer(webhookValidator))
+				g.Expect(c.NamespacedNameFilter).Should(beSameFunctionPointer(namespacedNameFilter))
+
+				return reconciler.NewImplementation(c)
 			}
-			if c.EventCh != eventCh {
-				t.Errorf(
-					"registerController() created a reconciler config with EventCh %v but expected %v for case of %q",
-					c.EventCh,
-					eventCh,
-					test.msg,
-				)
-			}
-			// comparing functions is not allowed in Go, so we're comparing the pointers
-			// nolint: lll
-			if reflect.ValueOf(c.NamespacedNameFilter).Pointer() != reflect.ValueOf(namespacedNameFilter).Pointer() {
-				t.Errorf(
-					"registerController() created a reconciler config with NamespacedNameFilter %p but expected %p for case of %q",
-					c.NamespacedNameFilter,
-					namespacedNameFilter,
-					test.msg,
-				)
+
+			err := registerController(
+				context.Background(),
+				objectType,
+				test.fakes.mgr,
+				eventCh,
+				eventRecorder,
+				withNamespacedNameFilter(namespacedNameFilter),
+				withK8sPredicate(predicate.ServicePortsChangedPredicate{}),
+				withFieldIndices(fieldIndexes),
+				withNewReconciler(newReconciler),
+				withWebhookValidator(webhookValidator),
+			)
+
+			if test.expectedErr == nil {
+				g.Expect(err).To(BeNil())
+			} else {
+				g.Expect(err).To(MatchError(test.expectedErr))
 			}
 
-			return reconciler.NewImplementation(c)
-		}
+			indexCallCount := test.fakes.indexer.IndexFieldCallCount()
 
-		err := registerController(
-			context.Background(),
-			objectType,
-			test.fakes.mgr,
-			eventCh,
-			withNamespacedNameFilter(namespacedNameFilter),
-			withK8sPredicate(predicate.ServicePortsChangedPredicate{}),
-			withFieldIndices(fieldIndexes),
-			withNewReconciler(newReconciler),
-		)
-
-		if !errors.Is(err, test.expectedErr) {
-			t.Errorf(
-				"registerController() returned %q but expected %q for case of %q",
-				err,
-				test.expectedErr,
-				test.msg,
-			)
-		}
+			g.Expect(indexCallCount).To(Equal(1))
 
-		indexCallCount := test.fakes.indexer.IndexFieldCallCount()
-		if indexCallCount != 1 {
-			t.Errorf(
-				"registerController() called indexer.IndexField() %d times but expected 1 for case of %q",
-				indexCallCount,
-				test.msg,
-			)
-		} else {
 			_, objType, field, indexFunc := test.fakes.indexer.IndexFieldArgsForCall(0)
 
-			if objType != objectType {
-				t.Errorf(
-					"registerController() called indexer.IndexField() with object type %T but expected %T for case %q",
-					objType,
-					objectType,
-					test.msg,
-				)
-			}
-			if field != index.KubernetesServiceNameIndexField {
-				t.Errorf("registerController() called indexer.IndexField() with field %q but expected %q for case %q",
-					field,
-					index.KubernetesServiceNameIndexField,
-					test.msg,
-				)
-			}
+			g.Expect(objType).To(BeIdenticalTo(objectType))
+			g.Expect(field).To(BeIdenticalTo(index.KubernetesServiceNameIndexField))
 
 			expectedIndexFunc := fieldIndexes[index.KubernetesServiceNameIndexField]
-			// comparing functions is not allowed in Go, so we're comparing the pointers
-			// nolint:lll
-			if reflect.ValueOf(indexFunc).Pointer() != reflect.ValueOf(expectedIndexFunc).Pointer() {
-				t.Errorf("registerController() called indexer.IndexField() with indexFunc %p but expected %p for case %q",
-					indexFunc,
-					expectedIndexFunc,
-					test.msg,
-				)
-			}
-		}
+			g.Expect(indexFunc).To(beSameFunctionPointer(expectedIndexFunc))
 
-		addCallCount := test.fakes.mgr.AddCallCount()
-		if addCallCount != test.expectedMgrAddCallCount {
-			t.Errorf(
-				"registerController() called mgr.Add() %d times but expected %d times for case %q",
-				addCallCount,
-				test.expectedMgrAddCallCount,
-				test.msg,
-			)
-		}
+			addCallCount := test.fakes.mgr.AddCallCount()
+			g.Expect(addCallCount).To(Equal(test.expectedMgrAddCallCount))
+		})
 	}
 }

internal/manager/manager.go

@@ -14,6 +14,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	k8spredicate "sigs.k8s.io/controller-runtime/pkg/predicate"
 	gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"
+	"sigs.k8s.io/gateway-api/apis/v1beta1/validation"
 
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/config"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/events"
@@ -71,13 +72,21 @@ func Start(cfg config.Config) error {
 			objectType: &gatewayv1beta1.GatewayClass{},
 			options: []controllerOption{
 				withNamespacedNameFilter(filter.CreateFilterForGatewayClass(cfg.GatewayClassName)),
+				// as of v0.6.0, the Gateway API Webhook doesn't include a validation function
+				// for the GatewayClass resource
 			},
 		},
 		{
 			objectType: &gatewayv1beta1.Gateway{},
+			options: []controllerOption{
+				withWebhookValidator(createValidator(validation.ValidateGateway)),
+			},
 		},
 		{
 			objectType: &gatewayv1beta1.HTTPRoute{},
+			options: []controllerOption{
+				withWebhookValidator(createValidator(validation.ValidateHTTPRoute)),
+			},
 		},
 		{
 			objectType: &apiv1.Service{},
@@ -99,8 +108,11 @@ func Start(cfg config.Config) error {
 
 	ctx := ctlr.SetupSignalHandler()
 
+	recorderName := fmt.Sprintf("nginx-kubernetes-gateway-%s", cfg.GatewayClassName)
+	recorder := mgr.GetEventRecorderFor(recorderName)
+
 	for _, regCfg := range controllerRegCfgs {
-		err := registerController(ctx, regCfg.objectType, mgr, eventCh, regCfg.options...)
+		err := registerController(ctx, regCfg.objectType, mgr, eventCh, recorder, regCfg.options...)
 		if err != nil {
 			return fmt.Errorf("cannot register controller for %T: %w", regCfg.objectType, err)
 		}

internal/manager/validators.go

@@ -0,0 +1,27 @@
+package manager
+
+import (
+	"errors"
+	"fmt"
+
+	"k8s.io/apimachinery/pkg/util/validation/field"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/nginxinc/nginx-kubernetes-gateway/internal/reconciler"
+)
+
+// createValidator creates a reconciler.ValidatorFunc from a function that validates a resource of type R.
+func createValidator[R client.Object](validate func(R) field.ErrorList) reconciler.ValidatorFunc {
+	return func(obj client.Object) error {
+		if obj == nil {
+			panic(errors.New("obj is nil"))
+		}
+
+		r, ok := obj.(R)
+		if !ok {
+			panic(fmt.Errorf("obj type mismatch: got %T, expected %T", obj, r))
+		}
+
+		return validate(r).ToAggregate()
+	}
+}

internal/manager/validators_test.go

@@ -0,0 +1,77 @@
+package manager
+
+import (
+	"testing"
+
+	. "github.com/onsi/gomega"
+	"k8s.io/apimachinery/pkg/util/validation/field"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/gateway-api/apis/v1beta1"
+)
+
+func TestCreateTypedValidator(t *testing.T) {
+	tests := []struct {
+		name        string
+		obj         client.Object
+		errorList   field.ErrorList
+		expectPanic bool
+		expectErr   bool
+	}{
+		{
+			obj:         &v1beta1.HTTPRoute{},
+			errorList:   field.ErrorList{},
+			expectPanic: false,
+			expectErr:   false,
+			name:        "no errors",
+		},
+		{
+			obj:         &v1beta1.HTTPRoute{},
+			errorList:   []*field.Error{{Detail: "test"}},
+			expectPanic: false,
+			expectErr:   true,
+			name:        "one error",
+		},
+		{
+			obj:         nil,
+			errorList:   field.ErrorList{},
+			expectPanic: true,
+			expectErr:   false,
+			name:        "nil object",
+		},
+		{
+			obj:         &v1beta1.Gateway{},
+			errorList:   field.ErrorList{},
+			expectPanic: true,
+			expectErr:   false,
+			name:        "wrong object type",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			g := NewGomegaWithT(t)
+
+			v := createValidator(createValidateHTTPRouteThatReturns(test.errorList))
+
+			if test.expectPanic {
+				g.Expect(func() { _ = v(test.obj) }).To(Panic())
+				return
+			}
+
+			result := v(test.obj)
+
+			if test.expectErr {
+				g.Expect(result).ToNot(BeNil())
+				return
+			}
+
+			g.Expect(result).To(BeNil())
+		})
+	}
+}
+
+func createValidateHTTPRouteThatReturns(errorList field.ErrorList) func(*v1beta1.HTTPRoute) field.ErrorList {
+	return func(*v1beta1.HTTPRoute) field.ErrorList {
+		return errorList
+	}
+}