Implement low level admission handler interface for ByoHost webhook

apis/infrastructure/v1beta1/byohost_webhook.go

@@ -4,48 +4,44 @@
 package v1beta1
 
 import (
-	"errors"
-
-	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/runtime/schema"
-	ctrl "sigs.k8s.io/controller-runtime"
-	logf "sigs.k8s.io/controller-runtime/pkg/log"
-	"sigs.k8s.io/controller-runtime/pkg/webhook"
-)
-
-// log is for logging in this package
-var byohostlog = logf.Log.WithName("byohost-resource")
+	"context"
+	"net/http"
 
-// SetupWebhookWithManager sets up the webhook for the byohost resource
-func (byoHost *ByoHost) SetupWebhookWithManager(mgr ctrl.Manager) error {
-	return ctrl.NewWebhookManagedBy(mgr).
-		For(byoHost).
-		Complete()
-}
+	v1 "k8s.io/api/admission/v1"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+)
 
 //+kubebuilder:webhook:path=/validate-infrastructure-cluster-x-k8s-io-v1beta1-byohost,mutating=false,failurePolicy=fail,sideEffects=None,groups=infrastructure.cluster.x-k8s.io,resources=byohosts,verbs=create;update;delete,versions=v1beta1,name=vbyohost.kb.io,admissionReviewVersions={v1,v1beta1}
 
-var _ webhook.Validator = &ByoHost{}
-
-// ValidateCreate implements webhook.Validator so a webhook will be registered for the type
-func (byoHost *ByoHost) ValidateCreate() error {
-	return nil
+// +k8s:deepcopy-gen=false
+// ByoHostValidator validates ByoHosts
+type ByoHostValidator struct {
+	//	Client  client.Client
+	decoder *admission.Decoder
 }
 
-// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
-func (byoHost *ByoHost) ValidateUpdate(old runtime.Object) error {
-	return nil
-}
+// nolint: gocritic
+// Handle handles all the requests for ByoHost resource
+func (v *ByoHostValidator) Handle(ctx context.Context, req admission.Request) admission.Response {
+	if req.Operation == v1.Delete {
+		byoHost := &ByoHost{}
+		err := v.decoder.DecodeRaw(req.OldObject, byoHost)
+		if err != nil {
+			return admission.Errored(http.StatusBadRequest, err)
+		}
+
+		if byoHost.Status.MachineRef != nil {
+			return admission.Denied("cannot delete ByoHost when MachineRef is assigned")
+		}
+	}
 
-// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
-func (byoHost *ByoHost) ValidateDelete() error {
-	byohostlog.Info("validate delete", "name", byoHost.Name)
-	groupResource := schema.GroupResource{Group: "infrastructure.cluster.x-k8s.io", Resource: "byohost"}
+	// TODO: verify if req.UserInfo.Username has rbac permission to update the byohost
 
-	if byoHost.Status.MachineRef != nil {
-		return apierrors.NewForbidden(groupResource, byoHost.Name, errors.New("cannot delete ByoHost when MachineRef is assigned"))
-	}
+	return admission.Allowed("")
+}
 
+// InjectDecoder injects the decoder.
+func (v *ByoHostValidator) InjectDecoder(d *admission.Decoder) error {
+	v.decoder = d
 	return nil
 }

apis/infrastructure/v1beta1/byohost_webhook_test.go

@@ -43,10 +43,12 @@ var _ = Describe("ByohostWebhook", func() {
 				Spec: byohv1beta1.ByoHostSpec{},
 			}
 			Expect(k8sClientUncached.Create(ctx, byoHost)).Should(Succeed())
+
 		})
 
 		It("should not reject the request", func() {
-			err := byoHost.ValidateDelete()
+
+			err := k8sClientUncached.Delete(ctx, byoHost)
 			Expect(err).To(BeNil())
 		})
 
@@ -78,12 +80,13 @@ var _ = Describe("ByohostWebhook", func() {
 					APIVersion: byoHost.APIVersion,
 				}
 				Expect(ph.Patch(ctx, byoHost, patch.WithStatusObservedGeneration{})).Should(Succeed())
+
 			})
 
 			It("should reject the request", func() {
-				err := byoHost.ValidateDelete()
+				err := k8sClientUncached.Delete(ctx, byoHost)
 				Expect(err).To(HaveOccurred())
-				Expect(err).To(MatchError("byohost.infrastructure.cluster.x-k8s.io \"" + byoHost.Name + "\" is forbidden: cannot delete ByoHost when MachineRef is assigned"))
+				Expect(err).To(MatchError("admission webhook \"vbyohost.kb.io\" denied the request: cannot delete ByoHost when MachineRef is assigned"))
 			})
 		})
 	})

apis/infrastructure/v1beta1/webhook_suite_test.go

@@ -28,17 +28,19 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/envtest/printer"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
+	"sigs.k8s.io/controller-runtime/pkg/webhook"
 )
 
 // These tests use Ginkgo (BDD-style Go testing framework). Refer to
 // http://onsi.github.io/ginkgo/ to learn more about Ginkgo.
 
 var (
-	cfg       *rest.Config
-	k8sClient client.Client
-	testEnv   *envtest.Environment
-	ctx       context.Context
-	cancel    context.CancelFunc
+	cfg               *rest.Config
+	k8sClient         client.Client
+	testUserK8sClient client.Client
+	testEnv           *envtest.Environment
+	ctx               context.Context
+	cancel            context.CancelFunc
 )
 
 func TestAPIs(t *testing.T) {
@@ -96,13 +98,21 @@ var _ = BeforeSuite(func() {
 		MetricsBindAddress: "0",
 	})
 	Expect(err).NotTo(HaveOccurred())
+	user, err := testEnv.ControlPlane.AddUser(envtest.User{
+		Name:   "test-user",
+		Groups: []string{"byoh:hosts"},
+	}, nil)
+	Expect(err).NotTo(HaveOccurred())
 
-	err = (&byohv1beta1.ByoHost{}).SetupWebhookWithManager(mgr)
+	testUserK8sClient, err = client.New(user.Config(), client.Options{Scheme: scheme.Scheme})
 	Expect(err).NotTo(HaveOccurred())
+	Expect(k8sClient).NotTo(BeNil())
 
 	err = (&byohv1beta1.ByoCluster{}).SetupWebhookWithManager(mgr)
 	Expect(err).NotTo(HaveOccurred())
 
+	mgr.GetWebhookServer().Register("/validate-infrastructure-cluster-x-k8s-io-v1beta1-byohost", &webhook.Admission{Handler: &byohv1beta1.ByoHostValidator{}})
+
 	//+kubebuilder:scaffold:webhook
 
 	go func() {

config/crd/bases/infrastructure.cluster.x-k8s.io_byohosts.yaml

@@ -77,7 +77,7 @@ spec:
                 type: object
               installationSecret:
                 description: InstallationSecret is an optional reference to InstallationSecret
-                  generated by InstallerController for k8s installation
+                  generated by InstallerController for K8s installation
                 properties:
                   apiVersion:
                     description: API version of the referent.

main.go

@@ -20,6 +20,7 @@ import (
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/healthz"
+	"sigs.k8s.io/controller-runtime/pkg/webhook"
 
 	byohcontrollers "github.com/vmware-tanzu/cluster-api-provider-bringyourownhost/controllers/infrastructure"
 
@@ -118,14 +119,13 @@ func main() {
 		os.Exit(1)
 	}
 
-	if err = (&infrastructurev1beta1.ByoHost{}).SetupWebhookWithManager(mgr); err != nil {
-		setupLog.Error(err, "unable to create webhook", "webhook", "ByoHost")
-		os.Exit(1)
-	}
 	if err = (&infrastructurev1beta1.ByoCluster{}).SetupWebhookWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create webhook", "webhook", "ByoCluster")
 		os.Exit(1)
 	}
+
+	mgr.GetWebhookServer().Register("/validate-infrastructure-cluster-x-k8s-io-v1beta1-byohost", &webhook.Admission{Handler: &infrastructurev1beta1.ByoHostValidator{}})
+
 	//+kubebuilder:scaffold:builder
 
 	if err := mgr.AddHealthzCheck("healthz", healthz.Ping); err != nil {