diff --git a/Makefile b/Makefile index 00ebb22eb7..e7d8b56819 100644 --- a/Makefile +++ b/Makefile @@ -256,9 +256,10 @@ LDFLAGS ?= $(shell hack/version.sh) # Allow overriding manifest generation destination directory MANIFEST_ROOT ?= ./config CRD_ROOT ?= $(MANIFEST_ROOT)/default/crd/bases -SUPERVISOR_CRD_ROOT ?= $(MANIFEST_ROOT)/supervisor/crd +SUPERVISOR_CRD_ROOT ?= $(MANIFEST_ROOT)/supervisor/crd/bases VCSIM_CRD_ROOT ?= $(VCSIM_DIR)/config/crd/bases -WEBHOOK_ROOT ?= $(MANIFEST_ROOT)/webhook +WEBHOOK_ROOT ?= $(MANIFEST_ROOT)/govmomi/webhook +SUPERVISOR_WEBHOOK_ROOT ?= $(MANIFEST_ROOT)/supervisor/webhook RBAC_ROOT ?= $(MANIFEST_ROOT)/rbac VCSIM_RBAC_ROOT ?= $(VCSIM_DIR)/config/rbac NETOP_RBAC_ROOT ?= $(NETOP_DIR)/config/rbac @@ -282,7 +283,7 @@ generate: ## Run all generate targets .PHONY: generate-manifests generate-manifests: $(CONTROLLER_GEN) ## Generate manifests e.g. CRD, RBAC etc. - $(MAKE) clean-generated-yaml SRC_DIRS="$(CRD_ROOT),$(SUPERVISOR_CRD_ROOT),./config/webhook/manifests.yaml" + $(MAKE) clean-generated-yaml SRC_DIRS="$(CRD_ROOT),$(SUPERVISOR_CRD_ROOT),./config/govmomi/webhook/manifests.yaml,./config/supervisor/webhook/manifests.yaml" $(CONTROLLER_GEN) \ paths=./apis/v1alpha3 \ paths=./apis/v1alpha4 \ @@ -292,6 +293,11 @@ generate-manifests: $(CONTROLLER_GEN) ## Generate manifests e.g. CRD, RBAC etc. output:crd:dir=$(CRD_ROOT) \ output:webhook:dir=$(WEBHOOK_ROOT) \ webhook + # Generate webhook manifests for supervisor mode separately. + $(CONTROLLER_GEN) \ + paths=./internal/webhooks/vmware\ + output:webhook:dir=$(SUPERVISOR_WEBHOOK_ROOT) \ + webhook $(CONTROLLER_GEN) \ paths=./ \ paths=./controllers/... \ diff --git a/config/base/kustomization.yaml b/config/base/kustomization.yaml index 2a29e33019..708aed071f 100644 --- a/config/base/kustomization.yaml +++ b/config/base/kustomization.yaml @@ -12,7 +12,6 @@ resources: bases: - ../rbac - ../manager - - ../webhook - ../certmanager patchesStrategicMerge: @@ -20,7 +19,6 @@ patchesStrategicMerge: - manager_pull_policy.yaml - manager_credentials_patch.yaml - manager_webhook_patch.yaml - - webhookcainjection_patch.yaml - manager_role_aggregation_patch.yaml # Protect the /metrics endpoint by putting it behind auth. # Only one of manager_auth_proxy_patch.yaml and @@ -40,30 +38,4 @@ patchesStrategicMerge: # Uncomment 'CAINJECTION' in crd/kustomization.yaml to enable the CA injection in the admission webhooks. # 'CERTMANAGER' needs to be enabled to use ca injection #- webhookcainjection_patch.yaml -vars: - - name: CERTIFICATE_NAMESPACE # namespace of the certificate CR - objref: - kind: Certificate - group: cert-manager.io - version: v1 - name: serving-cert # this name should match the one in certificate.yaml - fieldref: - fieldpath: metadata.namespace - - name: CERTIFICATE_NAME - objref: - kind: Certificate - group: cert-manager.io - version: v1 - name: serving-cert # this name should match the one in certificate.yaml - - name: SERVICE_NAMESPACE # namespace of the service - objref: - kind: Service - version: v1 - name: webhook-service - fieldref: - fieldpath: metadata.namespace - - name: SERVICE_NAME - objref: - kind: Service - version: v1 - name: webhook-service + diff --git a/config/default/kustomization.yaml b/config/default/kustomization.yaml index 8fab3f1054..64feccea51 100644 --- a/config/default/kustomization.yaml +++ b/config/default/kustomization.yaml @@ -12,3 +12,32 @@ commonLabels: resources: - ../base - ./crd + - ../govmomi/webhook + +vars: + - name: CERTIFICATE_NAMESPACE # namespace of the certificate CR + objref: + kind: Certificate + group: cert-manager.io + version: v1 + name: serving-cert # this name should match the one in certificate.yaml + fieldref: + fieldpath: metadata.namespace + - name: CERTIFICATE_NAME + objref: + kind: Certificate + group: cert-manager.io + version: v1 + name: serving-cert # this name should match the one in certificate.yaml + - name: SERVICE_NAMESPACE # namespace of the service + objref: + kind: Service + version: v1 + name: webhook-service + fieldref: + fieldpath: metadata.namespace + - name: SERVICE_NAME + objref: + kind: Service + version: v1 + name: webhook-service diff --git a/config/webhook/kustomization.yaml b/config/govmomi/webhook/kustomization.yaml similarity index 59% rename from config/webhook/kustomization.yaml rename to config/govmomi/webhook/kustomization.yaml index f8b6568c45..50a72342ba 100644 --- a/config/webhook/kustomization.yaml +++ b/config/govmomi/webhook/kustomization.yaml @@ -4,3 +4,6 @@ resources: configurations: - kustomizeconfig.yaml + +patchesStrategicMerge: + - webhookcainjection_patch.yaml diff --git a/config/webhook/kustomizeconfig.yaml b/config/govmomi/webhook/kustomizeconfig.yaml similarity index 99% rename from config/webhook/kustomizeconfig.yaml rename to config/govmomi/webhook/kustomizeconfig.yaml index ffedce2237..6398bf91b7 100644 --- a/config/webhook/kustomizeconfig.yaml +++ b/config/govmomi/webhook/kustomizeconfig.yaml @@ -9,7 +9,6 @@ nameReference: group: admissionregistration.k8s.io path: webhooks/clientConfig/service/name - namespace: - kind: ValidatingWebhookConfiguration group: admissionregistration.k8s.io diff --git a/config/webhook/manifests.yaml b/config/govmomi/webhook/manifests.yaml similarity index 93% rename from config/webhook/manifests.yaml rename to config/govmomi/webhook/manifests.yaml index 7c867e393e..ec0cfbacb4 100644 --- a/config/webhook/manifests.yaml +++ b/config/govmomi/webhook/manifests.yaml @@ -76,7 +76,7 @@ webhooks: path: /mutate-infrastructure-cluster-x-k8s-io-v1beta1-vspherevm failurePolicy: Fail matchPolicy: Equivalent - name: default.vspherevm.infrastructure.x-k8s.io + name: default.vspherevm.infrastructure.cluster.x-k8s.io rules: - apiGroups: - infrastructure.cluster.x-k8s.io @@ -103,7 +103,7 @@ webhooks: path: /validate-infrastructure-cluster-x-k8s-io-v1beta1-vsphereclustertemplate failurePolicy: Fail matchPolicy: Equivalent - name: validation.vsphereclustertemplate.infrastructure.x-k8s.io + name: validation.vsphereclustertemplate.infrastructure.cluster.x-k8s.io rules: - apiGroups: - infrastructure.cluster.x-k8s.io @@ -145,7 +145,7 @@ webhooks: path: /validate-infrastructure-cluster-x-k8s-io-v1beta1-vspheremachine failurePolicy: Fail matchPolicy: Equivalent - name: validation.vspheremachine.infrastructure.x-k8s.io + name: validation.vspheremachine.infrastructure.cluster.x-k8s.io rules: - apiGroups: - infrastructure.cluster.x-k8s.io @@ -166,7 +166,7 @@ webhooks: path: /validate-infrastructure-cluster-x-k8s-io-v1beta1-vspheremachinetemplate failurePolicy: Fail matchPolicy: Equivalent - name: validation.vspheremachinetemplate.infrastructure.x-k8s.io + name: validation.vspheremachinetemplate.infrastructure.cluster.x-k8s.io rules: - apiGroups: - infrastructure.cluster.x-k8s.io @@ -187,7 +187,7 @@ webhooks: path: /validate-infrastructure-cluster-x-k8s-io-v1beta1-vspherevm failurePolicy: Fail matchPolicy: Equivalent - name: validation.vspherevm.infrastructure.x-k8s.io + name: validation.vspherevm.infrastructure.cluster.x-k8s.io rules: - apiGroups: - infrastructure.cluster.x-k8s.io diff --git a/config/webhook/service.yaml b/config/govmomi/webhook/service.yaml similarity index 100% rename from config/webhook/service.yaml rename to config/govmomi/webhook/service.yaml diff --git a/config/base/webhookcainjection_patch.yaml b/config/govmomi/webhook/webhookcainjection_patch.yaml similarity index 100% rename from config/base/webhookcainjection_patch.yaml rename to config/govmomi/webhook/webhookcainjection_patch.yaml diff --git a/config/supervisor/crd/vmware.infrastructure.cluster.x-k8s.io_providerserviceaccounts.yaml b/config/supervisor/crd/bases/vmware.infrastructure.cluster.x-k8s.io_providerserviceaccounts.yaml similarity index 100% rename from config/supervisor/crd/vmware.infrastructure.cluster.x-k8s.io_providerserviceaccounts.yaml rename to config/supervisor/crd/bases/vmware.infrastructure.cluster.x-k8s.io_providerserviceaccounts.yaml diff --git a/config/supervisor/crd/vmware.infrastructure.cluster.x-k8s.io_vsphereclusters.yaml b/config/supervisor/crd/bases/vmware.infrastructure.cluster.x-k8s.io_vsphereclusters.yaml similarity index 100% rename from config/supervisor/crd/vmware.infrastructure.cluster.x-k8s.io_vsphereclusters.yaml rename to config/supervisor/crd/bases/vmware.infrastructure.cluster.x-k8s.io_vsphereclusters.yaml diff --git a/config/supervisor/crd/vmware.infrastructure.cluster.x-k8s.io_vsphereclustertemplates.yaml b/config/supervisor/crd/bases/vmware.infrastructure.cluster.x-k8s.io_vsphereclustertemplates.yaml similarity index 100% rename from config/supervisor/crd/vmware.infrastructure.cluster.x-k8s.io_vsphereclustertemplates.yaml rename to config/supervisor/crd/bases/vmware.infrastructure.cluster.x-k8s.io_vsphereclustertemplates.yaml diff --git a/config/supervisor/crd/vmware.infrastructure.cluster.x-k8s.io_vspheremachines.yaml b/config/supervisor/crd/bases/vmware.infrastructure.cluster.x-k8s.io_vspheremachines.yaml similarity index 100% rename from config/supervisor/crd/vmware.infrastructure.cluster.x-k8s.io_vspheremachines.yaml rename to config/supervisor/crd/bases/vmware.infrastructure.cluster.x-k8s.io_vspheremachines.yaml diff --git a/config/supervisor/crd/vmware.infrastructure.cluster.x-k8s.io_vspheremachinetemplates.yaml b/config/supervisor/crd/bases/vmware.infrastructure.cluster.x-k8s.io_vspheremachinetemplates.yaml similarity index 100% rename from config/supervisor/crd/vmware.infrastructure.cluster.x-k8s.io_vspheremachinetemplates.yaml rename to config/supervisor/crd/bases/vmware.infrastructure.cluster.x-k8s.io_vspheremachinetemplates.yaml diff --git a/config/supervisor/kustomization.yaml b/config/supervisor/kustomization.yaml index 7b8eb5511e..b90f3292f8 100644 --- a/config/supervisor/kustomization.yaml +++ b/config/supervisor/kustomization.yaml @@ -10,11 +10,40 @@ commonLabels: cluster.x-k8s.io/v1beta1: v1beta1 resources: - ../base - - crd/vmware.infrastructure.cluster.x-k8s.io_vspheremachines.yaml - - crd/vmware.infrastructure.cluster.x-k8s.io_vsphereclusters.yaml - - crd/vmware.infrastructure.cluster.x-k8s.io_vspheremachinetemplates.yaml - - crd/vmware.infrastructure.cluster.x-k8s.io_vsphereclustertemplates.yaml - - crd/vmware.infrastructure.cluster.x-k8s.io_providerserviceaccounts.yaml + - crd/bases/vmware.infrastructure.cluster.x-k8s.io_vspheremachines.yaml + - crd/bases/vmware.infrastructure.cluster.x-k8s.io_vsphereclusters.yaml + - crd/bases/vmware.infrastructure.cluster.x-k8s.io_vspheremachinetemplates.yaml + - crd/bases/vmware.infrastructure.cluster.x-k8s.io_vsphereclustertemplates.yaml + - crd/bases/vmware.infrastructure.cluster.x-k8s.io_providerserviceaccounts.yaml + - ./webhook patchesStrategicMerge: - add-configmap-env-vars.yaml + +vars: + - name: CERTIFICATE_NAMESPACE # namespace of the certificate CR + objref: + kind: Certificate + group: cert-manager.io + version: v1 + name: serving-cert # this name should match the one in certificate.yaml + fieldref: + fieldpath: metadata.namespace + - name: CERTIFICATE_NAME + objref: + kind: Certificate + group: cert-manager.io + version: v1 + name: serving-cert # this name should match the one in certificate.yaml + - name: SERVICE_NAMESPACE # namespace of the service + objref: + kind: Service + version: v1 + name: webhook-service + fieldref: + fieldpath: metadata.namespace + - name: SERVICE_NAME + objref: + kind: Service + version: v1 + name: webhook-service diff --git a/config/supervisor/webhook/kustomization.yaml b/config/supervisor/webhook/kustomization.yaml new file mode 100644 index 0000000000..50a72342ba --- /dev/null +++ b/config/supervisor/webhook/kustomization.yaml @@ -0,0 +1,9 @@ +resources: +- service.yaml +- manifests.yaml + +configurations: + - kustomizeconfig.yaml + +patchesStrategicMerge: + - webhookcainjection_patch.yaml diff --git a/config/supervisor/webhook/kustomizeconfig.yaml b/config/supervisor/webhook/kustomizeconfig.yaml new file mode 100644 index 0000000000..6398bf91b7 --- /dev/null +++ b/config/supervisor/webhook/kustomizeconfig.yaml @@ -0,0 +1,20 @@ +nameReference: +- kind: Service + version: v1 + fieldSpecs: + - kind: ValidatingWebhookConfiguration + group: admissionregistration.k8s.io + path: webhooks/clientConfig/service/name + - kind: MutatingWebhookConfiguration + group: admissionregistration.k8s.io + path: webhooks/clientConfig/service/name + +namespace: +- kind: ValidatingWebhookConfiguration + group: admissionregistration.k8s.io + path: webhooks/clientConfig/service/namespace + create: true +- kind: MutatingWebhookConfiguration + group: admissionregistration.k8s.io + path: webhooks/clientConfig/service/namespace + create: true diff --git a/config/supervisor/webhook/manifests.yaml b/config/supervisor/webhook/manifests.yaml new file mode 100644 index 0000000000..c14c8c4426 --- /dev/null +++ b/config/supervisor/webhook/manifests.yaml @@ -0,0 +1,54 @@ +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: MutatingWebhookConfiguration +metadata: + name: mutating-webhook-configuration +webhooks: +- admissionReviewVersions: + - v1beta1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /mutate-vmware-infrastructure-cluster-x-k8s-io-v1beta1-vspheremachine + failurePolicy: Fail + matchPolicy: Equivalent + name: default.vspheremachine.vmware.infrastructure.cluster.x-k8s.io + rules: + - apiGroups: + - vmware.infrastructure.cluster.x-k8s.io + apiVersions: + - v1beta1 + operations: + - CREATE + - UPDATE + resources: + - vspheremachines + sideEffects: None +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingWebhookConfiguration +metadata: + name: validating-webhook-configuration +webhooks: +- admissionReviewVersions: + - v1beta1 + clientConfig: + service: + name: webhook-service + namespace: system + path: /validate-vmware-infrastructure-cluster-x-k8s-io-v1beta1-vspheremachine + failurePolicy: Fail + matchPolicy: Equivalent + name: validation.vspheremachine.vmware.infrastructure.cluster.x-k8s.io + rules: + - apiGroups: + - vmware.infrastructure.cluster.x-k8s.io + apiVersions: + - v1beta1 + operations: + - CREATE + - UPDATE + resources: + - vspheremachines + sideEffects: None diff --git a/config/supervisor/webhook/service.yaml b/config/supervisor/webhook/service.yaml new file mode 100644 index 0000000000..711977f54f --- /dev/null +++ b/config/supervisor/webhook/service.yaml @@ -0,0 +1,9 @@ +apiVersion: v1 +kind: Service +metadata: + name: webhook-service + namespace: system +spec: + ports: + - port: 443 + targetPort: webhook-server diff --git a/config/supervisor/webhook/webhookcainjection_patch.yaml b/config/supervisor/webhook/webhookcainjection_patch.yaml new file mode 100644 index 0000000000..ab69fe9496 --- /dev/null +++ b/config/supervisor/webhook/webhookcainjection_patch.yaml @@ -0,0 +1,15 @@ +# This patch add annotation to admission webhook config and +# the variables $(NAMESPACE) and $(CERTIFICATENAME) will be substituted by kustomize. +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingWebhookConfiguration +metadata: + name: validating-webhook-configuration + annotations: + cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME) +--- +apiVersion: admissionregistration.k8s.io/v1 +kind: MutatingWebhookConfiguration +metadata: + name: mutating-webhook-configuration + annotations: + cert-manager.io/inject-ca-from: $(CERTIFICATE_NAMESPACE)/$(CERTIFICATE_NAME) diff --git a/controllers/vmware/test/controllers_suite_test.go b/controllers/vmware/test/controllers_suite_test.go index f343c4d1f4..e796db9d1a 100644 --- a/controllers/vmware/test/controllers_suite_test.go +++ b/controllers/vmware/test/controllers_suite_test.go @@ -40,6 +40,7 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1" + "sigs.k8s.io/cluster-api-provider-vsphere/internal/test/helpers" ) var ( @@ -77,13 +78,16 @@ func getTestEnv() (*envtest.Environment, *rest.Config) { localTestEnv := &envtest.Environment{ CRDDirectoryPaths: []string{ - filepath.Join(root, "config", "supervisor", "crd"), + filepath.Join(root, "config", "supervisor", "crd", "bases"), filepath.Join(root, "config", "deployments", "integration-tests", "crds"), filepath.Join(clusterAPIDir, "config", "crd", "bases"), }, ControlPlaneStopTimeout: 60 * time.Second, } + configPath := filepath.Clean(filepath.Join(root, "config", "supervisor", "webhook", "manifests.yaml")) + helpers.InitializeWebhookInEnvironment(localTestEnv, configPath) + localCfg, err := localTestEnv.Start() Expect(err).ToNot(HaveOccurred()) Expect(localCfg).ToNot(BeNil()) diff --git a/controllers/vmware/test/controllers_test.go b/controllers/vmware/test/controllers_test.go index 51a1b55a06..a5e3c89bda 100644 --- a/controllers/vmware/test/controllers_test.go +++ b/controllers/vmware/test/controllers_test.go @@ -19,7 +19,9 @@ package test import ( "context" "fmt" + "net" "reflect" + "strconv" "time" . "github.com/onsi/ginkgo/v2" @@ -30,16 +32,19 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" + "k8s.io/klog/v2" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/controller-runtime/pkg/cache" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" ctrlmgr "sigs.k8s.io/controller-runtime/pkg/manager" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" + "sigs.k8s.io/controller-runtime/pkg/webhook" infrav1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1" vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1" "sigs.k8s.io/cluster-api-provider-vsphere/controllers" + vmwarewebhooks "sigs.k8s.io/cluster-api-provider-vsphere/internal/webhooks/vmware" capvcontext "sigs.k8s.io/cluster-api-provider-vsphere/pkg/context" "sigs.k8s.io/cluster-api-provider-vsphere/pkg/manager" ) @@ -223,6 +228,13 @@ func getManager(cfg *rest.Config, networkProvider string) manager.Manager { Metrics: metricsserver.Options{ BindAddress: "0", }, + WebhookServer: webhook.NewServer( + webhook.Options{ + Port: testEnv.WebhookInstallOptions.LocalServingPort, + CertDir: testEnv.WebhookInstallOptions.LocalServingCertDir, + Host: "0.0.0.0", + }, + ), }, KubeConfig: cfg, NetworkProvider: networkProvider, @@ -235,6 +247,9 @@ func getManager(cfg *rest.Config, networkProvider string) manager.Manager { return err } + if err := (&vmwarewebhooks.VSphereMachineWebhook{}).SetupWebhookWithManager(mgr); err != nil { + return err + } return controllers.AddMachineControllerToManager(ctx, controllerCtx, mgr, true, controllerOpts) } @@ -256,11 +271,33 @@ func initManagerAndBuildClient(networkProvider string) (client.Client, context.C if managerRuntimeError != nil { _, _ = fmt.Fprintln(GinkgoWriter, "Manager failed at runtime") } + // wait for webhook port to be open prior to running tests + waitForWebhooks() }() return k8sClient, managerCancel } +func waitForWebhooks() { + port := testEnv.WebhookInstallOptions.LocalServingPort + + klog.V(2).Infof("Waiting for webhook port %d to be open prior to running tests", port) + timeout := 1 * time.Second + for { + time.Sleep(1 * time.Second) + conn, err := net.DialTimeout("tcp", net.JoinHostPort("127.0.0.1", strconv.Itoa(port)), timeout) + if err != nil { + klog.V(2).Infof("Webhook port is not ready, will retry in %v: %s", timeout, err) + continue + } + if err := conn.Close(); err != nil { + klog.V(2).Info("Connection to webhook port could not be closed. Continuing with tests...") + } + klog.V(2).Info("Webhook port is now open. Continuing with tests...") + return + } +} + func prepareClient(isLoadBalanced bool) (cli client.Client, cancelation context.CancelFunc) { networkProvider := "" if isLoadBalanced { diff --git a/internal/test/helpers/envtest.go b/internal/test/helpers/envtest.go index 66dc52f747..584e80974f 100644 --- a/internal/test/helpers/envtest.go +++ b/internal/test/helpers/envtest.go @@ -84,7 +84,7 @@ func init() { crdPaths := []string{ filepath.Join(root, "config", "default", "crd", "bases"), - filepath.Join(root, "config", "supervisor", "crd"), + filepath.Join(root, "config", "supervisor", "crd", "bases"), } // append CAPI CRDs path @@ -113,8 +113,16 @@ type ( // NewTestEnvironment creates a new environment spinning up a local api-server. func NewTestEnvironment(ctx context.Context) *TestEnvironment { + // Get the root of the current file to use in CRD paths. + _, filename, _, ok := goruntime.Caller(0) + if !ok { + klog.Fatalf("Failed to get information for current file from runtime") + } + root := path.Join(path.Dir(filename), "..", "..", "..") + configPath := filepath.Join(root, "config", "govmomi", "webhook", "manifests.yaml") + // initialize webhook here to be able to test the envtest install via webhookOptions - initializeWebhookInEnvironment() + InitializeWebhookInEnvironment(env, configPath) if _, err := env.Start(); err != nil { err = kerrors.NewAggregate([]error{err, env.Stop()}) diff --git a/internal/test/helpers/webhook.go b/internal/test/helpers/webhook.go index b4c0b22b3d..a06ed2bca7 100644 --- a/internal/test/helpers/webhook.go +++ b/internal/test/helpers/webhook.go @@ -19,9 +19,6 @@ package helpers import ( "net" "os" - "path" - "path/filepath" - goruntime "runtime" "strconv" "time" @@ -76,14 +73,12 @@ func appendWebhookConfiguration(configyamlFile []byte, tag string) ([]*v1.Mutati return mutatingWebhooks, validatingWebhooks, err } -func initializeWebhookInEnvironment() { - // Get the root of the current file to use in CRD paths. - _, filename, _, ok := goruntime.Caller(0) - if !ok { - klog.Fatalf("Failed to get information for current file from runtime") +// InitializeWebhookInEnvironment initializes WebhookInstallOptions for the provided environment. +func InitializeWebhookInEnvironment(e *envtest.Environment, configPath string) { + if configPath == "" { + klog.Fatalf("webhook configuration path is empty") } - root := path.Join(path.Dir(filename), "..", "..", "..") - configyamlFile, err := os.ReadFile(filepath.Clean(filepath.Join(root, "config", "webhook", "manifests.yaml"))) + configyamlFile, err := os.ReadFile(configPath) //nolint:gosec if err != nil { klog.Fatalf("Failed to read core webhook configuration file: %v ", err) } @@ -96,7 +91,7 @@ func initializeWebhookInEnvironment() { klog.Fatalf("Failed to append core controller webhook config: %v", err) } - env.WebhookInstallOptions = envtest.WebhookInstallOptions{ + e.WebhookInstallOptions = envtest.WebhookInstallOptions{ MaxTime: 20 * time.Second, PollInterval: time.Second, ValidatingWebhooks: validatingWebhooks, diff --git a/internal/webhooks/util.go b/internal/webhooks/util.go index cc485089d6..d64685eeed 100644 --- a/internal/webhooks/util.go +++ b/internal/webhooks/util.go @@ -22,7 +22,8 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" ) -func aggregateObjErrors(gk schema.GroupKind, name string, allErrs field.ErrorList) error { +// AggregateObjErrors aggregates a list of field errors into a single Invalid API error. +func AggregateObjErrors(gk schema.GroupKind, name string, allErrs field.ErrorList) error { if len(allErrs) == 0 { return nil } diff --git a/internal/webhooks/vmware/vspheremachine.go b/internal/webhooks/vmware/vspheremachine.go new file mode 100644 index 0000000000..f2c688c7cf --- /dev/null +++ b/internal/webhooks/vmware/vspheremachine.go @@ -0,0 +1,105 @@ +/* +Copyright 2024 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 vmware is the package for webhooks of vmware resources. +package vmware + +import ( + "context" + "fmt" + + 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" + + vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1" + "sigs.k8s.io/cluster-api-provider-vsphere/internal/webhooks" +) + +// +kubebuilder:webhook:verbs=create;update,path=/validate-vmware-infrastructure-cluster-x-k8s-io-v1beta1-vspheremachine,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=vmware.infrastructure.cluster.x-k8s.io,resources=vspheremachines,versions=v1beta1,name=validation.vspheremachine.vmware.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1 +// +kubebuilder:webhook:verbs=create;update,path=/mutate-vmware-infrastructure-cluster-x-k8s-io-v1beta1-vspheremachine,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=vmware.infrastructure.cluster.x-k8s.io,resources=vspheremachines,versions=v1beta1,name=default.vspheremachine.vmware.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1 + +// VSphereMachineWebhook implements a validation and defaulting webhook for VSphereMachine. +type VSphereMachineWebhook struct{} + +var _ webhook.CustomValidator = &VSphereMachineWebhook{} +var _ webhook.CustomDefaulter = &VSphereMachineWebhook{} + +func (webhook *VSphereMachineWebhook) SetupWebhookWithManager(mgr ctrl.Manager) error { + return ctrl.NewWebhookManagedBy(mgr). + For(&vmwarev1.VSphereMachine{}). + WithValidator(webhook). + WithDefaulter(webhook). + Complete() +} + +// Default implements webhook.Defaulter so a webhook will be registered for the type. +func (webhook *VSphereMachineWebhook) Default(_ context.Context, _ runtime.Object) error { + return nil +} + +// ValidateCreate implements webhook.Validator so a webhook will be registered for the type. +func (webhook *VSphereMachineWebhook) 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 (webhook *VSphereMachineWebhook) ValidateUpdate(_ context.Context, oldRaw runtime.Object, newRaw runtime.Object) (admission.Warnings, error) { + var allErrs field.ErrorList + + newTyped, ok := newRaw.(*vmwarev1.VSphereMachine) + if !ok { + return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a VSphereMachine but got a %T", newRaw)) + } + + oldTyped, ok := oldRaw.(*vmwarev1.VSphereMachine) + if !ok { + return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a VSphereMachine but got a %T", oldRaw)) + } + + newSpec, oldSpec := newTyped.Spec, oldTyped.Spec + + // In VM operator, following fields are immutable, so CAPV should not allow to update them. + // - ImageName + // - ClassName + // - StorageClass + // - MinHardwareVersion + if newSpec.ImageName != oldSpec.ImageName { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "imageName"), "cannot be modified")) + } + + if newSpec.ClassName != oldSpec.ClassName { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "className"), "cannot be modified")) + } + + if newSpec.StorageClass != oldSpec.StorageClass { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "storageClass"), "cannot be modified")) + } + + if newSpec.MinHardwareVersion != oldSpec.MinHardwareVersion { + allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "minHardwareVersion"), "cannot be modified")) + } + + return nil, webhooks.AggregateObjErrors(newTyped.GroupVersionKind().GroupKind(), newTyped.Name, allErrs) +} + +// ValidateDelete implements webhook.Validator so a webhook will be registered for the type. +func (webhook *VSphereMachineWebhook) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) { + return nil, nil +} diff --git a/internal/webhooks/vmware/vspheremachine_test.go b/internal/webhooks/vmware/vspheremachine_test.go new file mode 100644 index 0000000000..acde154723 --- /dev/null +++ b/internal/webhooks/vmware/vspheremachine_test.go @@ -0,0 +1,94 @@ +/* +Copyright 2024 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 vmware + +import ( + "context" + "testing" + + . "github.com/onsi/gomega" + + vmwarev1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/vmware/v1beta1" +) + +func TestVSphereMachine_ValidateUpdate(t *testing.T) { + g := NewWithT(t) + + fakeProviderID := "fake-000000" + tests := []struct { + name string + oldVSphereMachine *vmwarev1.VSphereMachine + vsphereMachine *vmwarev1.VSphereMachine + wantErr bool + }{ + { + name: "updating ProviderID can be done", + oldVSphereMachine: createVSphereMachine(nil, "tkgs-old-imagename", "best-effort-xsmall", "wcpglobalstorageprofile", "vmx-15"), + vsphereMachine: createVSphereMachine(&fakeProviderID, "tkgs-old-imagename", "best-effort-xsmall", "wcpglobalstorageprofile", "vmx-15"), + wantErr: false, + }, + { + name: "updating ImageName cannot be done", + oldVSphereMachine: createVSphereMachine(nil, "tkgs-old-imagename", "best-effort-xsmall", "wcpglobalstorageprofile", "vmx-15"), + vsphereMachine: createVSphereMachine(nil, "tkgs-new-imagename", "best-effort-xsmall", "wcpglobalstorageprofile", "vmx-15"), + wantErr: true, + }, + { + name: "updating ClassName cannot be done", + oldVSphereMachine: createVSphereMachine(nil, "tkgs-imagename", "old-best-effort-xsmall", "wcpglobalstorageprofile", "vmx-15"), + vsphereMachine: createVSphereMachine(nil, "tkgs-imagename", "new-best-effort-xsmall", "wcpglobalstorageprofile", "vmx-15"), + wantErr: true, + }, + { + name: "updating StorageClass cannot be done", + oldVSphereMachine: createVSphereMachine(nil, "tkgs-imagename", "best-effort-xsmall", "old-wcpglobalstorageprofile", "vmx-15"), + vsphereMachine: createVSphereMachine(nil, "tkgs-imagename", "best-effort-xsmall", "new-wcpglobalstorageprofile", "vmx-15"), + wantErr: true, + }, + { + name: "updating MinHardwareVersion cannot be done", + oldVSphereMachine: createVSphereMachine(nil, "tkgs-imagename", "best-effort-xsmall", "wcpglobalstorageprofile", "vmx-15"), + vsphereMachine: createVSphereMachine(nil, "tkgs-imagename", "best-effort-xsmall", "wcpglobalstorageprofile", "vmx-16"), + wantErr: true, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(_ *testing.T) { + webhook := &VSphereMachineWebhook{} + _, err := webhook.ValidateUpdate(context.Background(), tc.oldVSphereMachine, tc.vsphereMachine) + if tc.wantErr { + g.Expect(err).To(HaveOccurred()) + } else { + g.Expect(err).NotTo(HaveOccurred()) + } + }) + } +} + +func createVSphereMachine(providerID *string, imageName, className, storageClass, minHardwareVersion string) *vmwarev1.VSphereMachine { + vSphereMachine := &vmwarev1.VSphereMachine{ + Spec: vmwarev1.VSphereMachineSpec{ + ProviderID: providerID, + ImageName: imageName, + ClassName: className, + StorageClass: storageClass, + MinHardwareVersion: minHardwareVersion, + }, + } + + return vSphereMachine +} diff --git a/internal/webhooks/vsphereclustertemplate.go b/internal/webhooks/vsphereclustertemplate.go index d9e8372180..e7e6c1472c 100644 --- a/internal/webhooks/vsphereclustertemplate.go +++ b/internal/webhooks/vsphereclustertemplate.go @@ -31,7 +31,7 @@ import ( infrav1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1" ) -// +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 +// +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.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1 // VSphereClusterTemplateWebhook implements a validation and defaulting webhook for VSphereClusterTemplate. type VSphereClusterTemplateWebhook struct{} diff --git a/internal/webhooks/vspherefailuredomain.go b/internal/webhooks/vspherefailuredomain.go index 3406506c91..53c0a23886 100644 --- a/internal/webhooks/vspherefailuredomain.go +++ b/internal/webhooks/vspherefailuredomain.go @@ -77,7 +77,7 @@ func (webhook *VSphereFailureDomainWebhook) ValidateCreate(_ context.Context, ra allErrs = append(allErrs, field.Forbidden(field.NewPath("spec", "Topology", "ComputeCluster"), fmt.Sprintf("cannot be nil if zone's Failure Domain type is %s", obj.Spec.Zone.Type))) } - return nil, aggregateObjErrors(obj.GroupVersionKind().GroupKind(), obj.Name, allErrs) + return nil, AggregateObjErrors(obj.GroupVersionKind().GroupKind(), obj.Name, allErrs) } // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type. diff --git a/internal/webhooks/vspheremachine.go b/internal/webhooks/vspheremachine.go index 745e5200e6..420df4733d 100644 --- a/internal/webhooks/vspheremachine.go +++ b/internal/webhooks/vspheremachine.go @@ -33,7 +33,7 @@ import ( infrav1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1" ) -// +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1beta1-vspheremachine,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=vspheremachines,versions=v1beta1,name=validation.vspheremachine.infrastructure.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1 +// +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1beta1-vspheremachine,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=vspheremachines,versions=v1beta1,name=validation.vspheremachine.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1 // +kubebuilder:webhook:verbs=create;update,path=/mutate-infrastructure-cluster-x-k8s-io-v1beta1-vspheremachine,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=vspheremachines,versions=v1beta1,name=default.vspheremachine.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1 // VSphereMachineWebhook implements a validation and defaulting webhook for VSphereMachine. @@ -93,7 +93,7 @@ func (webhook *VSphereMachineWebhook) ValidateCreate(_ context.Context, raw runt } } - return nil, aggregateObjErrors(obj.GroupVersionKind().GroupKind(), obj.Name, allErrs) + return nil, AggregateObjErrors(obj.GroupVersionKind().GroupKind(), obj.Name, allErrs) } // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type. @@ -153,7 +153,7 @@ func (webhook *VSphereMachineWebhook) ValidateUpdate(_ context.Context, oldRaw r allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "cannot be modified")) } - return nil, aggregateObjErrors(newTyped.GroupVersionKind().GroupKind(), newTyped.Name, allErrs) + return nil, AggregateObjErrors(newTyped.GroupVersionKind().GroupKind(), newTyped.Name, allErrs) } // ValidateDelete implements webhook.Validator so a webhook will be registered for the type. diff --git a/internal/webhooks/vspheremachinetemplate.go b/internal/webhooks/vspheremachinetemplate.go index 55024869eb..a88af09892 100644 --- a/internal/webhooks/vspheremachinetemplate.go +++ b/internal/webhooks/vspheremachinetemplate.go @@ -35,7 +35,7 @@ import ( const machineTemplateImmutableMsg = "VSphereMachineTemplate spec.template.spec field is immutable. Please create a new resource instead." -// +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1beta1-vspheremachinetemplate,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=vspheremachinetemplates,versions=v1beta1,name=validation.vspheremachinetemplate.infrastructure.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1 +// +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1beta1-vspheremachinetemplate,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=vspheremachinetemplates,versions=v1beta1,name=validation.vspheremachinetemplate.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1 // VSphereMachineTemplateWebhook implements a validation and defaulting webhook for VSphereMachineTemplate. type VSphereMachineTemplateWebhook struct{} @@ -84,7 +84,7 @@ func (webhook *VSphereMachineTemplateWebhook) ValidateCreate(_ context.Context, allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "template", "spec", "guestSoftPowerOffTimeout"), spec.GuestSoftPowerOffTimeout, "should be greater than 0")) } } - return nil, aggregateObjErrors(obj.GroupVersionKind().GroupKind(), obj.Name, allErrs) + return nil, AggregateObjErrors(obj.GroupVersionKind().GroupKind(), obj.Name, allErrs) } // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type. @@ -108,7 +108,7 @@ func (webhook *VSphereMachineTemplateWebhook) ValidateUpdate(ctx context.Context !reflect.DeepEqual(newTyped.Spec.Template.Spec, oldTyped.Spec.Template.Spec) { allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "template", "spec"), newTyped, machineTemplateImmutableMsg)) } - return nil, aggregateObjErrors(newTyped.GroupVersionKind().GroupKind(), newTyped.Name, allErrs) + return nil, AggregateObjErrors(newTyped.GroupVersionKind().GroupKind(), newTyped.Name, allErrs) } // ValidateDelete implements webhook.Validator so a webhook will be registered for the type. diff --git a/internal/webhooks/vspherevm.go b/internal/webhooks/vspherevm.go index 47585516ea..576df56f4c 100644 --- a/internal/webhooks/vspherevm.go +++ b/internal/webhooks/vspherevm.go @@ -33,8 +33,8 @@ import ( infrav1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1" ) -// +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1beta1-vspherevm,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=vspherevms,versions=v1beta1,name=validation.vspherevm.infrastructure.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1 -// +kubebuilder:webhook:verbs=create;update,path=/mutate-infrastructure-cluster-x-k8s-io-v1beta1-vspherevm,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=vspherevms,versions=v1beta1,name=default.vspherevm.infrastructure.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1 +// +kubebuilder:webhook:verbs=create;update,path=/validate-infrastructure-cluster-x-k8s-io-v1beta1-vspherevm,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=vspherevms,versions=v1beta1,name=validation.vspherevm.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1 +// +kubebuilder:webhook:verbs=create;update,path=/mutate-infrastructure-cluster-x-k8s-io-v1beta1-vspherevm,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=infrastructure.cluster.x-k8s.io,resources=vspherevms,versions=v1beta1,name=default.vspherevm.infrastructure.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1beta1 // VSphereVMWebhook implements a validation and defaulting webhook for VSphereVM. type VSphereVMWebhook struct{} @@ -95,7 +95,7 @@ func (webhook *VSphereVMWebhook) ValidateCreate(_ context.Context, raw runtime.O allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "guestSoftPowerOffTimeout"), spec.GuestSoftPowerOffTimeout, "should be greater than 0")) } } - return nil, aggregateObjErrors(objValue.GroupVersionKind().GroupKind(), objValue.Name, allErrs) + return nil, AggregateObjErrors(objValue.GroupVersionKind().GroupKind(), objValue.Name, allErrs) } // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type. @@ -156,7 +156,7 @@ func (webhook *VSphereVMWebhook) ValidateUpdate(_ context.Context, oldRaw runtim allErrs = append(allErrs, field.Forbidden(field.NewPath("spec"), "cannot be modified")) } - return nil, aggregateObjErrors(newTyped.GroupVersionKind().GroupKind(), newTyped.Name, allErrs) + return nil, AggregateObjErrors(newTyped.GroupVersionKind().GroupKind(), newTyped.Name, allErrs) } // ValidateDelete implements webhook.Validator so a webhook will be registered for the type. diff --git a/main.go b/main.go index f10fb5f101..650e73b25c 100644 --- a/main.go +++ b/main.go @@ -53,6 +53,7 @@ import ( "sigs.k8s.io/cluster-api-provider-vsphere/controllers" "sigs.k8s.io/cluster-api-provider-vsphere/feature" "sigs.k8s.io/cluster-api-provider-vsphere/internal/webhooks" + vmwarewebhooks "sigs.k8s.io/cluster-api-provider-vsphere/internal/webhooks/vmware" capvcontext "sigs.k8s.io/cluster-api-provider-vsphere/pkg/context" "sigs.k8s.io/cluster-api-provider-vsphere/pkg/manager" "sigs.k8s.io/cluster-api-provider-vsphere/pkg/session" @@ -378,6 +379,9 @@ func setupVAPIControllers(ctx context.Context, controllerCtx *capvcontext.Contro } func setupSupervisorControllers(ctx context.Context, controllerCtx *capvcontext.ControllerManagerContext, mgr ctrlmgr.Manager, tracker *remote.ClusterCacheTracker) error { + if err := (&vmwarewebhooks.VSphereMachineWebhook{}).SetupWebhookWithManager(mgr); err != nil { + return err + } if err := controllers.AddClusterControllerToManager(ctx, controllerCtx, mgr, true, concurrency(vSphereClusterConcurrency)); err != nil { return err }