Merge pull request #2296 from laozc/runtime-deps

🌱 Move webhooks out of API packages

Makefile

@@ -246,6 +246,7 @@ generate-manifests: $(CONTROLLER_GEN) ## Generate manifests e.g. CRD, RBAC etc.
 		paths=./apis/v1alpha3 \
 		paths=./apis/v1alpha4 \
 		paths=./apis/v1beta1 \
+		paths=./internal/webhooks \
 		crd:crdVersions=v1 \
 		output:crd:dir=$(CRD_ROOT) \
 		output:webhook:dir=$(WEBHOOK_ROOT) \
@@ -455,15 +456,15 @@ setup-envtest: $(SETUP_ENVTEST) ## Set up envtest (download kubebuilder assets)
 
 .PHONY: test
 test: $(SETUP_ENVTEST) $(GOVC) ## Run unit tests
-	KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" GOVC_BIN_PATH=$(GOVC) go test -v ./apis/... ./controllers/... ./pkg/... $(TEST_ARGS)
+	KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" GOVC_BIN_PATH=$(GOVC) go test -v ./apis/... ./controllers/... ./pkg/... ./internal/... $(TEST_ARGS)
 
 .PHONY: test-verbose
 test-verbose: ## Run unit tests with verbose flag
 	$(MAKE) test TEST_ARGS="$(TEST_ARGS) -v"
 
 .PHONY: test-junit
 test-junit: $(SETUP_ENVTEST) $(GOTESTSUM) $(GOVC) ## Run unit tests
-	set +o errexit; (KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" GOVC_BIN_PATH=$(GOVC) go test -json ./apis/... ./controllers/... ./pkg/... $(TEST_ARGS); echo $$? > $(ARTIFACTS)/junit.exitcode) | tee $(ARTIFACTS)/junit.stdout
+	set +o errexit; (KUBEBUILDER_ASSETS="$(KUBEBUILDER_ASSETS)" GOVC_BIN_PATH=$(GOVC) go test -json ./apis/... ./controllers/... ./pkg/... ./internal/... $(TEST_ARGS); echo $$? > $(ARTIFACTS)/junit.exitcode) | tee $(ARTIFACTS)/junit.stdout
 	$(GOTESTSUM) --junitfile $(ARTIFACTS)/junit.xml --raw-command cat $(ARTIFACTS)/junit.stdout
 	exit $$(cat $(ARTIFACTS)/junit.exitcode)
 

internal/webhooks/doc.go

@@ -0,0 +1,18 @@
+/*
+Copyright 2023 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// Package webhooks contains webhooks for the infrastructure v1beta1 API group.
+package webhooks

apis/v1beta1/webhooks.go → internal/webhooks/util.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package v1beta1
+package webhooks
 
 import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"

apis/v1beta1/vsphereclustertemplate_webhook.go → internal/webhooks/vsphereclustertemplate.go

@@ -14,43 +14,59 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package v1beta1
+package webhooks
 
 import (
+	"context"
+	"fmt"
 	"reflect"
 
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
+
+	infrav1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1"
 )
 
-func (r *VSphereClusterTemplate) SetupWebhookWithManager(mgr ctrl.Manager) error {
+// +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1beta1-vsphereclustertemplate,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=vsphereclustertemplates,versions=v1beta1,name=validation.vsphereclustertemplate.infrastructure.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1
+
+// VSphereClusterTemplateWebhook implements a validation and defaulting webhook for VSphereClusterTemplate.
+type VSphereClusterTemplateWebhook struct{}
+
+var _ webhook.CustomValidator = &VSphereClusterTemplateWebhook{}
+
+func (webhook *VSphereClusterTemplateWebhook) SetupWebhookWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewWebhookManagedBy(mgr).
-		For(r).
+		For(&infrav1.VSphereClusterTemplate{}).
+		WithValidator(webhook).
 		Complete()
 }
 
-// +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1beta1-vsphereclustertemplate,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=vsphereclustertemplates,versions=v1beta1,name=validation.vsphereclustertemplate.infrastructure.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1
-
-var _ webhook.Validator = &VSphereClusterTemplate{}
-
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
-func (r *VSphereClusterTemplate) ValidateCreate() (admission.Warnings, error) {
+func (webhook *VSphereClusterTemplateWebhook) ValidateCreate(_ context.Context, _ runtime.Object) (admission.Warnings, error) {
 	return nil, nil
 }
 
 // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
-func (r *VSphereClusterTemplate) ValidateUpdate(oldRaw runtime.Object) (admission.Warnings, error) {
-	old := oldRaw.(*VSphereClusterTemplate)
-	if !reflect.DeepEqual(r.Spec.Template.Spec, old.Spec.Template.Spec) {
+func (webhook *VSphereClusterTemplateWebhook) ValidateUpdate(_ context.Context, oldRaw runtime.Object, newRaw runtime.Object) (admission.Warnings, error) {
+	oldTyped, ok := oldRaw.(*infrav1.VSphereClusterTemplate)
+	if !ok {
+		return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a VSphereClusterTemplate but got a %T", oldRaw))
+	}
+	newTyped, ok := newRaw.(*infrav1.VSphereClusterTemplate)
+	if !ok {
+		return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a VSphereClusterTemplate but got a %T", newRaw))
+	}
+	if !reflect.DeepEqual(newTyped.Spec.Template.Spec, oldTyped.Spec.Template.Spec) {
 		return nil, field.Forbidden(field.NewPath("spec", "template", "spec"), "VSphereClusterTemplate spec is immutable")
 	}
 	return nil, nil
 }
 
 // ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
-func (r *VSphereClusterTemplate) ValidateDelete() (admission.Warnings, error) {
+func (webhook *VSphereClusterTemplateWebhook) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) {
 	return nil, nil
 }

apis/v1beta1/vspheredeploymentzone_webhook.go → internal/webhooks/vspheredeploymentzone.go

@@ -14,27 +14,44 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package v1beta1
+package webhooks
 
 import (
+	"context"
+	"fmt"
+
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/utils/pointer"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
+
+	infrav1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1"
 )
 
-func (z *VSphereDeploymentZone) SetupWebhookWithManager(mgr ctrl.Manager) error {
+// +kubebuilder:webhook:verbs=create;update,path=/mutate-infrastructure-cluster-x-k8s-io-v1beta1-vspheredeploymentzone,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=vspheredeploymentzones,versions=v1beta1,name=default.vspheredeploymentzone.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1
+
+// VSphereDeploymentZoneWebhook implements a validation and defaulting webhook for VSphereDeploymentZone.
+type VSphereDeploymentZoneWebhook struct{}
+
+var _ webhook.CustomDefaulter = &VSphereDeploymentZoneWebhook{}
+
+func (webhook *VSphereDeploymentZoneWebhook) SetupWebhookWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewWebhookManagedBy(mgr).
-		For(z).
+		For(&infrav1.VSphereDeploymentZone{}).
+		WithDefaulter(webhook).
 		Complete()
 }
 
-// +kubebuilder:webhook:verbs=create;update,path=/mutate-infrastructure-cluster-x-k8s-io-v1beta1-vspheredeploymentzone,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=vspheredeploymentzones,versions=v1beta1,name=default.vspheredeploymentzone.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1
-
-var _ webhook.Defaulter = &VSphereDeploymentZone{}
-
 // Default implements webhook.Defaulter so a webhook will be registered for the type.
-func (z *VSphereDeploymentZone) Default() {
-	if z.Spec.ControlPlane == nil {
-		z.Spec.ControlPlane = pointer.Bool(true)
+func (webhook *VSphereDeploymentZoneWebhook) Default(_ context.Context, obj runtime.Object) error {
+	typedObj, ok := obj.(*infrav1.VSphereDeploymentZone)
+	if !ok {
+		return apierrors.NewBadRequest(fmt.Sprintf("expected a VSphereDeploymentZone but got a %T", obj))
 	}
+	if typedObj.Spec.ControlPlane == nil {
+		typedObj.Spec.ControlPlane = pointer.Bool(true)
+	}
+
+	return nil
 }

apis/v1beta1/vspheredeploymentzone_webhook_test.go → internal/webhooks/vspheredeploymentzone_test.go

@@ -14,13 +14,16 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package v1beta1
+package webhooks
 
 import (
+	"context"
 	"testing"
 
 	. "github.com/onsi/gomega"
 	"k8s.io/utils/pointer"
+
+	infrav1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1"
 )
 
 func TestVSphereDeploymentZone_Default(t *testing.T) {
@@ -47,12 +50,13 @@ func TestVSphereDeploymentZone_Default(t *testing.T) {
 		// Need to reinit the test variable
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
-			vdz := VSphereDeploymentZone{
-				Spec: VSphereDeploymentZoneSpec{
+			vdz := infrav1.VSphereDeploymentZone{
+				Spec: infrav1.VSphereDeploymentZoneSpec{
 					ControlPlane: tt.boolPtr,
 				},
 			}
-			vdz.Default()
+			webhook := VSphereDeploymentZoneWebhook{}
+			g.Expect(webhook.Default(context.Background(), &vdz)).NotTo(HaveOccurred())
 			g.Expect(vdz.Spec.ControlPlane).NotTo(BeNil())
 			g.Expect(*vdz.Spec.ControlPlane).To(Equal(tt.expectedVal))
 		})