configure webhooks in tests (almost)

Author: erikgb

config/webhook/kustomization.yaml

@@ -4,6 +4,3 @@ resources:
 
 configurations:
 - kustomizeconfig.yaml
-
-patchesStrategicMerge:
-- webhook_patch.yaml

config/webhook/manifests.yaml

@@ -73,18 +73,6 @@ webhooks:
     resources:
     - hierarchyconfigurations
   sideEffects: None
-- admissionReviewVersions:
-  - v1
-  clientConfig:
-    service:
-      name: webhook-service
-      namespace: system
-      path: /validate-objects
-  failurePolicy: Fail
-  name: objects.hnc.x-k8s.io
-  rules:
-  - {}
-  sideEffects: None
 - admissionReviewVersions:
   - v1
   clientConfig:

config/webhook/webhook_patch.yaml

@@ -15,6 +15,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/utils/pointer"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/event"
@@ -24,6 +25,7 @@ import (
 
 	api "sigs.k8s.io/hierarchical-namespaces/api/v1alpha2"
 	"sigs.k8s.io/hierarchical-namespaces/internal/apimeta"
+	"sigs.k8s.io/hierarchical-namespaces/internal/config"
 	"sigs.k8s.io/hierarchical-namespaces/internal/crd"
 	"sigs.k8s.io/hierarchical-namespaces/internal/forest"
 	"sigs.k8s.io/hierarchical-namespaces/internal/objects"
@@ -260,16 +262,49 @@ func (r *Reconciler) syncObjectWebhookConfigs(ctx context.Context) error {
 	}
 	cleanVWC := vwc.DeepCopy()
 
+	webhookFound := false
 	for i, wh := range vwc.Webhooks {
 		if wh.Name == "objects.hnc.x-k8s.io" {
 			vwc.Webhooks[i].Rules = rules
-			if err := r.Patch(ctx, vwc, client.MergeFrom(cleanVWC)); err != nil {
-				return err
-			}
+			webhookFound = true
 			break
 		}
 	}
-	return nil
+	if !webhookFound {
+		failurePolicy := apiadmissionregistrationv1.Fail
+		sideEffects := apiadmissionregistrationv1.SideEffectClassNone
+		vw := apiadmissionregistrationv1.ValidatingWebhook{
+			Name: "objects.hnc.x-k8s.io",
+			ClientConfig: apiadmissionregistrationv1.WebhookClientConfig{
+				Service: &apiadmissionregistrationv1.ServiceReference{
+					Namespace: config.GetHNCNamespace(),
+					Name:      "webhook-service",
+					Path:      pointer.String("/validate-objects"),
+				},
+			},
+			Rules:                   rules,
+			FailurePolicy:           &failurePolicy,
+			SideEffects:             &sideEffects,
+			TimeoutSeconds:          pointer.Int32(2),
+			AdmissionReviewVersions: []string{"v1"},
+			// We only apply this object validator on non-excluded namespaces, which have
+			// the "included-namespace" label set by the HC reconciler, so that when HNC
+			// (webhook service specifically) is down, operations in the excluded
+			// namespaces won't be affected. Validators on HNC CRs are not filtered because
+			// they are supposed to prevent abuse of HNC CRs in excluded namespaces.
+			// Namespace validator is not filtered to prevent abuse of the included-namespace
+			// label on excluded namespaces. Unfortunately, this means that when HNC is
+			// down, we will block updates on all namespaces, even "excluded" ones, but
+			// anyone who can update namespaces like `kube-system` should likely be able to
+			// delete the VWHConfiguration to make the updates.
+			NamespaceSelector: &metav1.LabelSelector{
+				MatchLabels: map[string]string{"hnc.x-k8s.io/included-namespace": "true"},
+			},
+		}
+		vwc.Webhooks = append(vwc.Webhooks, vw)
+	}
+
+	return r.Patch(ctx, vwc, client.MergeFrom(cleanVWC))
 }
 
 // syncObjectReconcilers creates or syncs ObjectReconcilers.

internal/hncconfig/reconciler.go

@@ -71,6 +71,10 @@ func HNCBeforeSuite() {
 	By("configuring test environment")
 	testEnv = &envtest.Environment{
 		CRDDirectoryPaths: []string{filepath.Join("..", "..", "config", "crd", "bases")},
+		// todo(erikgb): Fix tests that breaks when enabling webhooks
+		//WebhookInstallOptions: envtest.WebhookInstallOptions{
+		//	Paths: []string{filepath.Join("..", "..", "config", "webhook")},
+		//},
 	}
 
 	By("starting test environment")
@@ -94,10 +98,14 @@ func HNCBeforeSuite() {
 	// CF: https://github.com/microsoft/azure-databricks-operator/blob/0f722a710fea06b86ecdccd9455336ca712bf775/controllers/suite_test.go
 
 	By("creating manager")
+	webhookInstallOptions := &testEnv.WebhookInstallOptions
 	k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{
 		NewClient:          config.NewClient(false),
 		MetricsBindAddress: "0", // disable metrics serving since 'go test' runs multiple suites in parallel processes
 		Scheme:             scheme.Scheme,
+		Host:               webhookInstallOptions.LocalServingHost,
+		Port:               webhookInstallOptions.LocalServingPort,
+		CertDir:            webhookInstallOptions.LocalServingCertDir,
 	})
 	Expect(err).ToNot(HaveOccurred())
 
@@ -111,6 +119,7 @@ func HNCBeforeSuite() {
 	TestForest = forest.NewForest()
 	err = setup.CreateReconcilers(k8sManager, TestForest, opts)
 	Expect(err).ToNot(HaveOccurred())
+	setup.CreateWebhooks(k8sManager, TestForest, opts)
 
 	By("Creating clients")
 	K8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme})

internal/integtest/setup.go

@@ -32,17 +32,6 @@ const (
 	ServingPath = "/validate-objects"
 )
 
-// Note: the validating webhook FAILS CLOSE. This means that if the webhook goes
-// down, all further changes are forbidden. In addition, the webhook `rules`
-// (groups, resources, versions, verbs) specified in the below kubebuilder marker
-// are overwritten by the `rules` configured in config/webhook/webhook_patch.yaml,
-// because there's no marker for `scope` and we only want this object webhook
-// to work on `namespaced` objects. Please make sure you edit the webhook_patch.yaml
-// file if you want to change the webhook `rules` and better make the rules
-// here the same as what's in the webhook_patch.yaml.
-//
-// +kubebuilder:webhook:admissionReviewVersions=v1,path=/validate-objects,mutating=false,failurePolicy=fail,groups=,resources=,sideEffects=None,verbs=,versions=,name=objects.hnc.x-k8s.io
-
 type Validator struct {
 	Log     logr.Logger
 	Forest  *forest.Forest

internal/objects/validator.go

@@ -35,7 +35,7 @@ func Create(log logr.Logger, mgr ctrl.Manager, f *forest.Forest, opts Options) {
 
 	if !opts.NoWebhooks {
 		log.Info("Registering validating webhook (won't work when running locally; use --no-webhooks)")
-		createWebhooks(mgr, f, opts)
+		CreateWebhooks(mgr, f, opts)
 	}
 
 	log.Info("Registering reconcilers")

internal/setup/reconcilers.go

@@ -54,8 +54,8 @@ func ManageCerts(mgr ctrl.Manager, setupFinished chan struct{}, restartOnSecretR
 	})
 }
 
-// createWebhooks creates all mutators and validators.
-func createWebhooks(mgr ctrl.Manager, f *forest.Forest, opts Options) {
+// CreateWebhooks creates all mutators and validators.
+func CreateWebhooks(mgr ctrl.Manager, f *forest.Forest, opts Options) {
 	// Create webhook for Hierarchy
 	mgr.GetWebhookServer().Register(hierarchyconfig.ServingPath, &webhook.Admission{Handler: &hierarchyconfig.Validator{
 		Log:    ctrl.Log.WithName("hierarchyconfig").WithName("validate"),