Skip to content

Commit

Permalink
Add validating webhook for vm-operator VSphereMachine
Browse files Browse the repository at this point in the history
Signed-off-by: Gong Zhang <gongz@vmware.com>
  • Loading branch information
zhanggbj committed Jan 31, 2024
1 parent e0dc2ae commit 3ac090a
Show file tree
Hide file tree
Showing 13 changed files with 276 additions and 9 deletions.
8 changes: 7 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ CRD_ROOT ?= $(MANIFEST_ROOT)/default/crd/bases
SUPERVISOR_CRD_ROOT ?= $(MANIFEST_ROOT)/supervisor/crd
VMOP_CRD_ROOT ?= $(MANIFEST_ROOT)/deployments/integration-tests/crds
WEBHOOK_ROOT ?= $(MANIFEST_ROOT)/webhook
SUPERVISOR_WEBHOOK_ROOT ?= $(MANIFEST_ROOT)/supervisor/webhook
RBAC_ROOT ?= $(MANIFEST_ROOT)/rbac
VERSION ?= $(shell cat clusterctl-settings.json | jq .config.nextVersion -r)
OVERRIDES_DIR := $(HOME)/.cluster-api/overrides/infrastructure-vsphere/$(VERSION)
Expand All @@ -253,14 +254,19 @@ 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),$(VMOP_CRD_ROOT),./config/webhook/manifests.yaml"
$(MAKE) clean-generated-yaml SRC_DIRS="$(CRD_ROOT),$(SUPERVISOR_CRD_ROOT),$(VMOP_CRD_ROOT),./config/webhook/manifests.yaml,./config/supervisor/webhook/manifests.yaml"
$(CONTROLLER_GEN) \
paths=./apis/v1beta1 \
paths=./internal/webhooks \
crd:crdVersions=v1 \
output:crd:dir=$(CRD_ROOT) \
output:webhook:dir=$(WEBHOOK_ROOT) \
webhook
# Generate webhook manifests for supervisor mode separately.
$(CONTROLLER_GEN) \
paths=./internal/webhooks/vmoperator \
output:webhook:dir=$(SUPERVISOR_WEBHOOK_ROOT) \
webhook
$(CONTROLLER_GEN) \
paths=./ \
paths=./controllers/... \
Expand Down
2 changes: 2 additions & 0 deletions config/supervisor/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ resources:
- 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
- ./webhook

patchesStrategicMerge:
- add-configmap-env-vars.yaml

4 changes: 4 additions & 0 deletions config/supervisor/webhook/kustomization.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
resources:
- manifests.yaml

namePrefix: supervisor-
54 changes: 54 additions & 0 deletions config/supervisor/webhook/manifests.yaml
Original file line number Diff line number Diff line change
@@ -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.x-k8s.io
rules:
- apiGroups:
- vmware.infrastructure.cluster.x-k8s.io
apiVersions:
- v1beta1
operations:
- CREATE
- UPDATE
resources:
- vspheremachines
sideEffects: None
5 changes: 5 additions & 0 deletions internal/test/helpers/envtest.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ import (
infrav1 "sigs.k8s.io/cluster-api-provider-vsphere/apis/v1beta1"
"sigs.k8s.io/cluster-api-provider-vsphere/internal/test/helpers/vcsim"
"sigs.k8s.io/cluster-api-provider-vsphere/internal/webhooks"
vmopwebhooks "sigs.k8s.io/cluster-api-provider-vsphere/internal/webhooks/vmoperator"
capvcontext "sigs.k8s.io/cluster-api-provider-vsphere/pkg/context"
"sigs.k8s.io/cluster-api-provider-vsphere/pkg/manager"
)
Expand Down Expand Up @@ -159,6 +160,10 @@ func NewTestEnvironment(ctx context.Context) *TestEnvironment {
Password: simr.Password(),
}
managerOpts.AddToManager = func(ctx context.Context, controllerCtx *capvcontext.ControllerManagerContext, mgr ctrlmgr.Manager) error {
if err := (&vmopwebhooks.VSphereMachineWebhook{}).SetupWebhookWithManager(mgr); err != nil {
return err
}

Check warning on line 165 in internal/test/helpers/envtest.go

View check run for this annotation

Codecov / codecov/patch

internal/test/helpers/envtest.go#L164-L165

Added lines #L164 - L165 were not covered by tests

if err := (&webhooks.VSphereClusterTemplateWebhook{}).SetupWebhookWithManager(mgr); err != nil {
return err
}
Expand Down
3 changes: 2 additions & 1 deletion internal/webhooks/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
105 changes: 105 additions & 0 deletions internal/webhooks/vmoperator/vspheremachine.go
Original file line number Diff line number Diff line change
@@ -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 vmoperator is the package for webhooks of VM Operator resources.
package vmoperator

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.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 VM Operator 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

Check warning on line 54 in internal/webhooks/vmoperator/vspheremachine.go

View check run for this annotation

Codecov / codecov/patch

internal/webhooks/vmoperator/vspheremachine.go#L53-L54

Added lines #L53 - L54 were not covered by tests
}

// 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

Check warning on line 59 in internal/webhooks/vmoperator/vspheremachine.go

View check run for this annotation

Codecov / codecov/patch

internal/webhooks/vmoperator/vspheremachine.go#L58-L59

Added lines #L58 - L59 were not covered by tests
}

// 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 VM Operator VSphereMachine but got a %T", newRaw))
}

Check warning on line 69 in internal/webhooks/vmoperator/vspheremachine.go

View check run for this annotation

Codecov / codecov/patch

internal/webhooks/vmoperator/vspheremachine.go#L68-L69

Added lines #L68 - L69 were not covered by tests

oldTyped, ok := oldRaw.(*vmwarev1.VSphereMachine)
if !ok {
return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a VM Operator VSphereMachine but got a %T", oldRaw))
}

Check warning on line 74 in internal/webhooks/vmoperator/vspheremachine.go

View check run for this annotation

Codecov / codecov/patch

internal/webhooks/vmoperator/vspheremachine.go#L73-L74

Added lines #L73 - L74 were not covered by tests

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.Invalid(field.NewPath("spec", "imageName"), newTyped.Spec, "updating imageName is not allowed"))
}

if newSpec.ClassName != oldSpec.ClassName {
allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "className"), newTyped.Spec, "updating className is not allowed"))
}

if newSpec.StorageClass != oldSpec.StorageClass {
allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "storageClass"), newTyped.Spec, "updating storageClass is not allowed"))
}

if newSpec.MinHardwareVersion != oldSpec.MinHardwareVersion {
allErrs = append(allErrs, field.Invalid(field.NewPath("spec", "minHardwareVersion"), newTyped.Spec, "updating minHardwareVersion is not allowed"))
}

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

Check warning on line 104 in internal/webhooks/vmoperator/vspheremachine.go

View check run for this annotation

Codecov / codecov/patch

internal/webhooks/vmoperator/vspheremachine.go#L103-L104

Added lines #L103 - L104 were not covered by tests
}
86 changes: 86 additions & 0 deletions internal/webhooks/vmoperator/vspheremachine_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/*
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 vmoperator

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)

tests := []struct {
name string
oldVSphereMachine *vmwarev1.VSphereMachine
vsphereMachine *vmwarev1.VSphereMachine
wantErr bool
}{
{
name: "updating ImageName cannot be done",
oldVSphereMachine: createVSphereMachine("tkgs-old-imagename", "best-effort-xsmall", "wcpglobalstorageprofile", "vmx-15"),
vsphereMachine: createVSphereMachine("tkgs-new-imagename", "best-effort-xsmall", "wcpglobalstorageprofile", "vmx-15"),
wantErr: true,
},
{
name: "updating ClassName cannot be done",
oldVSphereMachine: createVSphereMachine("tkgs-imagename", "old-best-effort-xsmall", "wcpglobalstorageprofile", "vmx-15"),
vsphereMachine: createVSphereMachine("tkgs-imagename", "new-best-effort-xsmall", "wcpglobalstorageprofile", "vmx-15"),
wantErr: true,
},
{
name: "updating StorageClass cannot be done",
oldVSphereMachine: createVSphereMachine("tkgs-imagename", "best-effort-xsmall", "old-wcpglobalstorageprofile", "vmx-15"),
vsphereMachine: createVSphereMachine("tkgs-imagename", "best-effort-xsmall", "new-wcpglobalstorageprofile", "vmx-15"),
wantErr: true,
},
{
name: "updating MinHardwareVersion cannot be done",
oldVSphereMachine: createVSphereMachine("tkgs-imagename", "best-effort-xsmall", "wcpglobalstorageprofile", "vmx-15"),
vsphereMachine: createVSphereMachine("tkgs-imagename", "best-effort-xsmall", "wcpglobalstorageprofile", "vmx-16"),
wantErr: true,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *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(imageName, className, storageClass, minHardwareVersion string) *vmwarev1.VSphereMachine {
vSphereMachine := &vmwarev1.VSphereMachine{
Spec: vmwarev1.VSphereMachineSpec{
ImageName: imageName,
ClassName: className,
StorageClass: storageClass,
MinHardwareVersion: minHardwareVersion,
},
}

return vSphereMachine
}
2 changes: 1 addition & 1 deletion internal/webhooks/vspherefailuredomain.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions internal/webhooks/vspheremachine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions internal/webhooks/vspheremachinetemplate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions internal/webhooks/vspherevm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
Loading

0 comments on commit 3ac090a

Please sign in to comment.