Skip to content

Commit

Permalink
Refactor MachineDeployment webhook to match others
Browse files Browse the repository at this point in the history
  • Loading branch information
JoelSpeed committed Aug 14, 2023
1 parent 12eaa07 commit 0e63639
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2753,7 +2753,9 @@ func newFakeMachineDeploymentTopologyState(name string, infrastructureMachineTem

scheme := runtime.NewScheme()
_ = clusterv1.AddToScheme(scheme)
if err := (webhooks.MachineDeploymentWebhook(scheme)).Default(admission.NewContextWithRequest(ctx, admission.Request{}), mdState.Object); err != nil {
if err := (&webhooks.MachineDeployment{
Decoder: admission.NewDecoder(scheme),
}).Default(admission.NewContextWithRequest(ctx, admission.Request{}), mdState.Object); err != nil {
panic(err)
}
return mdState
Expand Down
41 changes: 16 additions & 25 deletions internal/webhooks/machinedeployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,13 @@ import (
"sigs.k8s.io/cluster-api/util/version"
)

// Webhook is the interface for all webhooks.
type Webhook interface {
webhook.CustomDefaulter
webhook.CustomValidator
SetupWebhookWithManager(ctrl.Manager) error
}
func (webhook *MachineDeployment) SetupWebhookWithManager(mgr ctrl.Manager) error {
if webhook.Decoder == nil {
webhook.Decoder = admission.NewDecoder(mgr.GetScheme())
}

func (webhook *machineDeployment) SetupWebhookWithManager(mgr ctrl.Manager) error {
// This registers MachineDeployment as a validating webhook and
// machineDeploymentDefaulter as a defaulting webhook.
// MachineDeploymentDefaulter as a defaulting webhook.
return ctrl.NewWebhookManagedBy(mgr).
For(&clusterv1.MachineDeployment{}).
WithDefaulter(webhook).
Expand All @@ -61,22 +58,16 @@ func (webhook *machineDeployment) SetupWebhookWithManager(mgr ctrl.Manager) erro
// +kubebuilder:webhook:verbs=create;update,path=/validate-cluster-x-k8s-io-v1beta1-machinedeployment,mutating=false,failurePolicy=fail,matchPolicy=Equivalent,groups=cluster.x-k8s.io,resources=machinedeployments,versions=v1beta1,name=validation.machinedeployment.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1
// +kubebuilder:webhook:verbs=create;update,path=/mutate-cluster-x-k8s-io-v1beta1-machinedeployment,mutating=true,failurePolicy=fail,matchPolicy=Equivalent,groups=cluster.x-k8s.io,resources=machinedeployments,versions=v1beta1,name=default.machinedeployment.cluster.x-k8s.io,sideEffects=None,admissionReviewVersions=v1;v1beta1

// MachineDeploymentWebhook creates a new Webhook for MachineDeployments.
func MachineDeploymentWebhook(scheme *runtime.Scheme) Webhook {
return &machineDeployment{
decoder: admission.NewDecoder(scheme),
}
}

type machineDeployment struct {
decoder *admission.Decoder
// MachineDeployment implements a validation and defaulting webhook for MachineDeployment.
type MachineDeployment struct {
Decoder *admission.Decoder
}

var _ webhook.CustomDefaulter = &machineDeployment{}
var _ webhook.CustomValidator = &machineDeployment{}
var _ webhook.CustomDefaulter = &MachineDeployment{}
var _ webhook.CustomValidator = &MachineDeployment{}

// Default implements webhook.CustomDefaulter.
func (webhook *machineDeployment) Default(ctx context.Context, obj runtime.Object) error {
func (webhook MachineDeployment) Default(ctx context.Context, obj runtime.Object) error {
m, ok := obj.(*clusterv1.MachineDeployment)
if !ok {
return apierrors.NewBadRequest(fmt.Sprintf("expected a MachineDeployment but got a %T", obj))
Expand All @@ -94,7 +85,7 @@ func (webhook *machineDeployment) Default(ctx context.Context, obj runtime.Objec
var oldMD *clusterv1.MachineDeployment
if req.Operation == v1.Update {
oldMD = &clusterv1.MachineDeployment{}
if err := webhook.decoder.DecodeRaw(req.OldObject, oldMD); err != nil {
if err := webhook.Decoder.DecodeRaw(req.OldObject, oldMD); err != nil {
return errors.Wrapf(err, "failed to decode oldObject to MachineDeployment")
}
}
Expand Down Expand Up @@ -173,7 +164,7 @@ func (webhook *machineDeployment) Default(ctx context.Context, obj runtime.Objec
}

// ValidateCreate implements webhook.CustomValidator so a webhook will be registered for the type.
func (webhook *machineDeployment) ValidateCreate(_ context.Context, obj runtime.Object) (admission.Warnings, error) {
func (webhook MachineDeployment) ValidateCreate(_ context.Context, obj runtime.Object) (admission.Warnings, error) {
m, ok := obj.(*clusterv1.MachineDeployment)
if !ok {
return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a MachineDeployment but got a %T", obj))
Expand All @@ -183,7 +174,7 @@ func (webhook *machineDeployment) ValidateCreate(_ context.Context, obj runtime.
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
func (webhook *machineDeployment) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
func (webhook MachineDeployment) ValidateUpdate(_ context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
oldMD, ok := oldObj.(*clusterv1.MachineDeployment)
if !ok {
return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a MachineDeployment but got a %T", oldObj))
Expand All @@ -198,11 +189,11 @@ func (webhook *machineDeployment) ValidateUpdate(_ context.Context, oldObj, newO
}

// ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
func (webhook *machineDeployment) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) {
func (webhook MachineDeployment) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) {
return nil, nil
}

func (webhook *machineDeployment) validate(oldMD, newMD *clusterv1.MachineDeployment) error {
func (webhook *MachineDeployment) validate(oldMD, newMD *clusterv1.MachineDeployment) error {
var allErrs field.ErrorList
// The MachineDeployment name is used as a label value. This check ensures names which are not be valid label values are rejected.
if errs := validation.IsValidLabelValue(newMD.Name); len(errs) != 0 {
Expand Down
20 changes: 15 additions & 5 deletions internal/webhooks/machinedeployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@ func TestMachineDeploymentDefault(t *testing.T) {

scheme := runtime.NewScheme()
g.Expect(clusterv1.AddToScheme(scheme)).To(Succeed())
webhook := MachineDeploymentWebhook(scheme)
webhook := MachineDeployment{
Decoder: admission.NewDecoder(scheme),
}

reqCtx := admission.NewContextWithRequest(ctx, admission.Request{
AdmissionRequest: admissionv1.AdmissionRequest{
Expand Down Expand Up @@ -400,7 +402,9 @@ func TestMachineDeploymentValidation(t *testing.T) {

scheme := runtime.NewScheme()
g.Expect(clusterv1.AddToScheme(scheme)).To(Succeed())
webhook := MachineDeploymentWebhook(scheme)
webhook := MachineDeployment{
Decoder: admission.NewDecoder(scheme),
}

if tt.expectErr {
warnings, err := webhook.ValidateCreate(ctx, md)
Expand Down Expand Up @@ -470,7 +474,9 @@ func TestMachineDeploymentVersionValidation(t *testing.T) {

scheme := runtime.NewScheme()
g.Expect(clusterv1.AddToScheme(scheme)).To(Succeed())
webhook := MachineDeploymentWebhook(scheme)
webhook := MachineDeployment{
Decoder: admission.NewDecoder(scheme),
}

if tt.expectErr {
warnings, err := webhook.ValidateCreate(ctx, md)
Expand Down Expand Up @@ -530,7 +536,9 @@ func TestMachineDeploymentClusterNameImmutable(t *testing.T) {

scheme := runtime.NewScheme()
g.Expect(clusterv1.AddToScheme(scheme)).To(Succeed())
webhook := MachineDeploymentWebhook(scheme)
webhook := MachineDeployment{
Decoder: admission.NewDecoder(scheme),
}

warnings, err := webhook.ValidateUpdate(ctx, oldMD, newMD)
if tt.expectErr {
Expand Down Expand Up @@ -580,7 +588,9 @@ func TestMachineDeploymentTemplateMetadataValidation(t *testing.T) {

scheme := runtime.NewScheme()
g.Expect(clusterv1.AddToScheme(scheme)).To(Succeed())
webhook := MachineDeploymentWebhook(scheme)
webhook := MachineDeployment{
Decoder: admission.NewDecoder(scheme),
}

if tt.expectErr {
warnings, err := webhook.ValidateCreate(ctx, md)
Expand Down
9 changes: 7 additions & 2 deletions webhooks/alias.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package webhooks
import (
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"

"sigs.k8s.io/cluster-api/internal/webhooks"
)
Expand Down Expand Up @@ -56,11 +57,15 @@ func (webhook *Machine) SetupWebhookWithManager(mgr ctrl.Manager) error {
}

// MachineDeployment implements a validating and defaulting webhook for MachineDeployment.
type MachineDeployment struct{}
type MachineDeployment struct {
Decoder *admission.Decoder
}

// SetupWebhookWithManager sets up MachineDeployment webhooks.
func (webhook *MachineDeployment) SetupWebhookWithManager(mgr ctrl.Manager) error {
return webhooks.MachineDeploymentWebhook(mgr.GetScheme()).SetupWebhookWithManager(mgr)
return (&webhooks.MachineDeployment{
Decoder: webhook.Decoder,
}).SetupWebhookWithManager(mgr)
}

// MachineSet implements a validating and defaulting webhook for MachineSet.
Expand Down

0 comments on commit 0e63639

Please sign in to comment.