Chore/refactor webhook validation (#510)

- Move webhook validation code to the state package so that the package 
is responsible for enforcing it. As a result, it will not need require 
the users of that package to validate the supplied Gateway API
resources, which will make it easier to use the package and reduce
the number of possible bugs. Fixes
#416
- Fix Webhook Validation is bypassed for existing resources when NKG
starts. Fixes
#433

Author: pleshakov

internal/manager/controllers.go

@@ -26,7 +26,6 @@ type controllerConfig struct {
 	k8sPredicate         predicate.Predicate
 	fieldIndices         index.FieldIndices
 	newReconciler        newReconcilerFunc
-	webhookValidator     reconciler.ValidatorFunc
 }
 
 type controllerOption func(*controllerConfig)
@@ -56,12 +55,6 @@ 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,
@@ -73,7 +66,6 @@ func registerController(
 	objectType client.Object,
 	mgr manager.Manager,
 	eventCh chan<- interface{},
-	recorder reconciler.EventRecorder,
 	options ...controllerOption,
 ) error {
 	cfg := defaultControllerConfig()
@@ -100,8 +92,6 @@ 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

@@ -11,7 +11,6 @@ import (
 	"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"
@@ -21,7 +20,6 @@ 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) {
@@ -86,12 +84,6 @@ func TestRegisterController(t *testing.T) {
 	namespacedNameFilter := filter.CreateFilterForGatewayClass("test")
 	fieldIndexes := index.CreateEndpointSliceFieldIndices()
 
-	webhookValidator := createValidator(func(_ *v1beta1.HTTPRoute) field.ErrorList {
-		return nil
-	})
-
-	eventRecorder := &reconcilerfakes.FakeEventRecorder{}
-
 	eventCh := make(chan<- interface{})
 
 	beSameFunctionPointer := func(expected interface{}) types.GomegaMatcher {
@@ -109,8 +101,6 @@ func TestRegisterController(t *testing.T) {
 				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)
@@ -121,12 +111,10 @@ func TestRegisterController(t *testing.T) {
 				objectType,
 				test.fakes.mgr,
 				eventCh,
-				eventRecorder,
 				withNamespacedNameFilter(namespacedNameFilter),
 				withK8sPredicate(predicate.ServicePortsChangedPredicate{}),
 				withFieldIndices(fieldIndexes),
 				withNewReconciler(newReconciler),
-				withWebhookValidator(webhookValidator),
 			)
 
 			if test.expectedErr == nil {

internal/manager/manager.go

@@ -14,7 +14,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	k8spredicate "sigs.k8s.io/controller-runtime/pkg/predicate"
 	gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1"
-	gwapivalidation "sigs.k8s.io/gateway-api/apis/v1beta1/validation"
 
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/config"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/events"
@@ -75,21 +74,13 @@ func Start(cfg config.Config) error {
 			objectType: &gatewayv1beta1.GatewayClass{},
 			options: []controllerOption{
 				withNamespacedNameFilter(filter.CreateFilterForGatewayClass(cfg.GatewayClassName)),
-				// as of v0.6.2, the Gateway API Webhook doesn't include a validation function
-				// for the GatewayClass resource
 			},
 		},
 		{
 			objectType: &gatewayv1beta1.Gateway{},
-			options: []controllerOption{
-				withWebhookValidator(createValidator(gwapivalidation.ValidateGateway)),
-			},
 		},
 		{
 			objectType: &gatewayv1beta1.HTTPRoute{},
-			options: []controllerOption{
-				withWebhookValidator(createValidator(gwapivalidation.ValidateHTTPRoute)),
-			},
 		},
 		{
 			objectType: &apiv1.Service{},
@@ -111,11 +102,8 @@ 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, recorder, regCfg.options...)
+		err := registerController(ctx, regCfg.objectType, mgr, eventCh, regCfg.options...)
 		if err != nil {
 			return fmt.Errorf("cannot register controller for %T: %w", regCfg.objectType, err)
 		}
@@ -124,6 +112,9 @@ func Start(cfg config.Config) error {
 	secretStore := secrets.NewSecretStore()
 	secretMemoryMgr := secrets.NewSecretDiskMemoryManager(secretsFolder, secretStore)
 
+	recorderName := fmt.Sprintf("nginx-kubernetes-gateway-%s", cfg.GatewayClassName)
+	recorder := mgr.GetEventRecorderFor(recorderName)
+
 	processor := state.NewChangeProcessorImpl(state.ChangeProcessorConfig{
 		GatewayCtlrName:      cfg.GatewayCtlrName,
 		GatewayClassName:     cfg.GatewayClassName,
@@ -134,6 +125,8 @@ func Start(cfg config.Config) error {
 		Validators: validation.Validators{
 			HTTPFieldsValidator: ngxvalidation.HTTPValidator{},
 		},
+		EventRecorder: recorder,
+		Scheme:        scheme,
 	})
 
 	configGenerator := ngxcfg.NewGeneratorImpl()

internal/manager/validators.go

@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"reflect"
 
-	apiv1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -19,9 +18,6 @@ import (
 // If the function returns false, the reconciler will log the returned string.
 type NamespacedNameFilterFunc func(nsname types.NamespacedName) (bool, string)
 
-// ValidatorFunc validates a Kubernetes resource.
-type ValidatorFunc func(object client.Object) error
-
 // Config contains the configuration for the Implementation.
 type Config struct {
 	// Getter gets a resource from the k8s API.
@@ -32,10 +28,6 @@ type Config struct {
 	EventCh chan<- interface{}
 	// NamespacedNameFilter filters resources the controller will process. Can be nil.
 	NamespacedNameFilter NamespacedNameFilterFunc
-	// WebhookValidator validates a resource using the same rules as in the Gateway API Webhook. Can be nil.
-	WebhookValidator ValidatorFunc
-	// EventRecorder records event about resources.
-	EventRecorder EventRecorder
 }
 
 // Implementation is a reconciler for Kubernetes resources.
@@ -66,12 +58,6 @@ func newObject(objectType client.Object) client.Object {
 	return reflect.New(t).Interface().(client.Object)
 }
 
-const (
-	webhookValidationErrorLogMsg = "Rejected the resource because the Gateway API webhook failed to reject it with " +
-		"a validation error; make sure the webhook is installed and running correctly; " +
-		"NKG will delete any existing NGINX configuration that corresponds to the resource"
-)
-
 // Reconcile implements the reconcile.Reconciler Reconcile method.
 func (r *Implementation) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) {
 	logger := log.FromContext(ctx)
@@ -98,22 +84,10 @@ func (r *Implementation) Reconcile(ctx context.Context, req reconcile.Request) (
 		obj = nil
 	}
 
-	var validationError error
-	if obj != nil && r.cfg.WebhookValidator != nil {
-		validationError = r.cfg.WebhookValidator(obj)
-	}
-
-	if validationError != nil {
-		logger.Error(validationError, webhookValidationErrorLogMsg)
-		r.cfg.EventRecorder.Eventf(obj, apiv1.EventTypeWarning, "Rejected",
-			webhookValidationErrorLogMsg+"; validation error: %v", validationError)
-	}
-
 	var e interface{}
 	var op string
 
-	if obj == nil || validationError != nil {
-		// In case of a validation error, we handle the resource as if it was deleted.
+	if obj == nil {
 		e = &events.DeleteEvent{
 			Type:           r.cfg.ObjectType,
 			NamespacedName: req.NamespacedName,