Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions api/v1beta1/machine_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"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"

"sigs.k8s.io/cluster-api/util/version"
)
Expand Down Expand Up @@ -71,22 +72,22 @@ func (m *Machine) Default() {
}

// ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
func (m *Machine) ValidateCreate() error {
return m.validate(nil)
func (m *Machine) ValidateCreate() (admission.Warnings, error) {
return nil, m.validate(nil)
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
func (m *Machine) ValidateUpdate(old runtime.Object) error {
func (m *Machine) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
oldM, ok := old.(*Machine)
if !ok {
return apierrors.NewBadRequest(fmt.Sprintf("expected a Machine but got a %T", old))
return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a Machine but got a %T", old))
}
return m.validate(oldM)
return nil, m.validate(oldM)
}

// ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
func (m *Machine) ValidateDelete() error {
return nil
func (m *Machine) ValidateDelete() (admission.Warnings, error) {
return nil, nil
}

func (m *Machine) validate(old *Machine) error {
Expand Down
41 changes: 27 additions & 14 deletions api/v1beta1/machine_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,15 @@ func TestMachineBootstrapValidation(t *testing.T) {
Spec: MachineSpec{Bootstrap: tt.bootstrap},
}
if tt.expectErr {
g.Expect(m.ValidateCreate()).NotTo(Succeed())
g.Expect(m.ValidateUpdate(m)).NotTo(Succeed())
_, err := m.ValidateCreate()
g.Expect(err).To(HaveOccurred())
_, err = m.ValidateUpdate(m)
g.Expect(err).To(HaveOccurred())
} else {
g.Expect(m.ValidateCreate()).To(Succeed())
g.Expect(m.ValidateUpdate(m)).To(Succeed())
_, err := m.ValidateCreate()
g.Expect(err).ToNot(HaveOccurred())
_, err = m.ValidateUpdate(m)
g.Expect(err).ToNot(HaveOccurred())
}
})
}
Expand Down Expand Up @@ -137,11 +141,15 @@ func TestMachineNamespaceValidation(t *testing.T) {
}

if tt.expectErr {
g.Expect(m.ValidateCreate()).NotTo(Succeed())
g.Expect(m.ValidateUpdate(m)).NotTo(Succeed())
_, err := m.ValidateCreate()
g.Expect(err).To(HaveOccurred())
_, err = m.ValidateUpdate(m)
g.Expect(err).To(HaveOccurred())
} else {
g.Expect(m.ValidateCreate()).To(Succeed())
g.Expect(m.ValidateUpdate(m)).To(Succeed())
_, err := m.ValidateCreate()
g.Expect(err).ToNot(HaveOccurred())
_, err = m.ValidateUpdate(m)
g.Expect(err).ToNot(HaveOccurred())
}
})
}
Expand Down Expand Up @@ -185,10 +193,11 @@ func TestMachineClusterNameImmutable(t *testing.T) {
},
}

_, err := newMachine.ValidateUpdate(oldMachine)
if tt.expectErr {
g.Expect(newMachine.ValidateUpdate(oldMachine)).NotTo(Succeed())
g.Expect(err).To(HaveOccurred())
} else {
g.Expect(newMachine.ValidateUpdate(oldMachine)).To(Succeed())
g.Expect(err).NotTo(HaveOccurred())
}
})
}
Expand Down Expand Up @@ -239,11 +248,15 @@ func TestMachineVersionValidation(t *testing.T) {
}

if tt.expectErr {
g.Expect(m.ValidateCreate()).NotTo(Succeed())
g.Expect(m.ValidateUpdate(m)).NotTo(Succeed())
_, err := m.ValidateCreate()
g.Expect(err).To(HaveOccurred())
_, err = m.ValidateUpdate(m)
g.Expect(err).To(HaveOccurred())
} else {
g.Expect(m.ValidateCreate()).To(Succeed())
g.Expect(m.ValidateUpdate(m)).To(Succeed())
_, err := m.ValidateCreate()
g.Expect(err).ToNot(HaveOccurred())
_, err = m.ValidateUpdate(m)
g.Expect(err).ToNot(HaveOccurred())
}
})
}
Expand Down
22 changes: 9 additions & 13 deletions api/v1beta1/machinedeployment_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import (
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/apimachinery/pkg/util/validation"
"k8s.io/apimachinery/pkg/util/validation/field"
"k8s.io/klog/v2"
"k8s.io/utils/pointer"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/webhook"
Expand All @@ -57,10 +56,8 @@ var _ webhook.Validator = &MachineDeployment{}

// MachineDeploymentDefaulter creates a new CustomDefaulter for MachineDeployments.
func MachineDeploymentDefaulter(scheme *runtime.Scheme) webhook.CustomDefaulter {
// Note: The error return parameter is always nil and will be dropped with the next CR release.
decoder, _ := admission.NewDecoder(scheme)
return &machineDeploymentDefaulter{
decoder: decoder,
decoder: admission.NewDecoder(scheme),
}
}

Expand Down Expand Up @@ -167,22 +164,22 @@ func (webhook *machineDeploymentDefaulter) Default(ctx context.Context, obj runt
}

// ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
func (m *MachineDeployment) ValidateCreate() error {
return m.validate(nil)
func (m *MachineDeployment) ValidateCreate() (admission.Warnings, error) {
return nil, m.validate(nil)
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
func (m *MachineDeployment) ValidateUpdate(old runtime.Object) error {
func (m *MachineDeployment) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
oldMD, ok := old.(*MachineDeployment)
if !ok {
return apierrors.NewBadRequest(fmt.Sprintf("expected a MachineDeployment but got a %T", old))
return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a MachineDeployment but got a %T", old))
}
return m.validate(oldMD)
return nil, m.validate(oldMD)
}

// ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
func (m *MachineDeployment) ValidateDelete() error {
return nil
func (m *MachineDeployment) ValidateDelete() (admission.Warnings, error) {
return nil, nil
}

func (m *MachineDeployment) validate(old *MachineDeployment) error {
Expand Down Expand Up @@ -302,8 +299,7 @@ func calculateMachineDeploymentReplicas(ctx context.Context, oldMD *MachineDeplo
return *newMD.Spec.Replicas, nil
}

// TODO(sbueringer): drop this with the next CR version that adds the MD key automatically.
log := ctrl.LoggerFrom(ctx).WithValues("MachineDeployment", klog.KObj(newMD))
log := ctrl.LoggerFrom(ctx)

// If both autoscaler annotations are set, use them to calculate the default value.
minSizeString, hasMinSizeAnnotation := newMD.Annotations[AutoscalerMinSizeAnnotation]
Expand Down
38 changes: 25 additions & 13 deletions api/v1beta1/machinedeployment_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -393,11 +393,15 @@ func TestMachineDeploymentValidation(t *testing.T) {
},
}
if tt.expectErr {
g.Expect(md.ValidateCreate()).NotTo(Succeed())
g.Expect(md.ValidateUpdate(md)).NotTo(Succeed())
_, err := md.ValidateCreate()
g.Expect(err).To(HaveOccurred())
_, err = md.ValidateUpdate(md)
g.Expect(err).To(HaveOccurred())
} else {
g.Expect(md.ValidateCreate()).To(Succeed())
g.Expect(md.ValidateUpdate(md)).To(Succeed())
_, err := md.ValidateCreate()
g.Expect(err).NotTo(HaveOccurred())
_, err = md.ValidateUpdate(md)
g.Expect(err).NotTo(HaveOccurred())
}
})
}
Expand Down Expand Up @@ -452,11 +456,15 @@ func TestMachineDeploymentVersionValidation(t *testing.T) {
}

if tt.expectErr {
g.Expect(md.ValidateCreate()).NotTo(Succeed())
g.Expect(md.ValidateUpdate(md)).NotTo(Succeed())
_, err := md.ValidateCreate()
g.Expect(err).To(HaveOccurred())
_, err = md.ValidateUpdate(md)
g.Expect(err).To(HaveOccurred())
} else {
g.Expect(md.ValidateCreate()).To(Succeed())
g.Expect(md.ValidateUpdate(md)).To(Succeed())
_, err := md.ValidateCreate()
g.Expect(err).ToNot(HaveOccurred())
_, err = md.ValidateUpdate(md)
g.Expect(err).ToNot(HaveOccurred())
}
})
}
Expand Down Expand Up @@ -499,10 +507,11 @@ func TestMachineDeploymentClusterNameImmutable(t *testing.T) {
},
}

_, err := newMD.ValidateUpdate(oldMD)
if tt.expectErr {
g.Expect(newMD.ValidateUpdate(oldMD)).NotTo(Succeed())
g.Expect(err).To(HaveOccurred())
} else {
g.Expect(newMD.ValidateUpdate(oldMD)).To(Succeed())
g.Expect(err).NotTo(HaveOccurred())
}
})
}
Expand Down Expand Up @@ -530,18 +539,21 @@ func defaultValidateTestCustomDefaulter(object admission.Validator, customDefaul
t.Run("validate-on-create", func(t *testing.T) {
g := NewWithT(t)
g.Expect(customDefaulter.Default(ctx, createCopy)).To(Succeed())
g.Expect(createCopy.ValidateCreate()).To(Succeed())
_, err := createCopy.ValidateCreate()
g.Expect(err).NotTo(HaveOccurred())
})
t.Run("validate-on-update", func(t *testing.T) {
g := NewWithT(t)
g.Expect(customDefaulter.Default(ctx, defaultingUpdateCopy)).To(Succeed())
g.Expect(customDefaulter.Default(ctx, updateCopy)).To(Succeed())
g.Expect(defaultingUpdateCopy.ValidateUpdate(updateCopy)).To(Succeed())
_, err := defaultingUpdateCopy.ValidateUpdate(updateCopy)
g.Expect(err).NotTo(HaveOccurred())
})
t.Run("validate-on-delete", func(t *testing.T) {
g := NewWithT(t)
g.Expect(customDefaulter.Default(ctx, deleteCopy)).To(Succeed())
g.Expect(deleteCopy.ValidateDelete()).To(Succeed())
_, err := deleteCopy.ValidateDelete()
g.Expect(err).NotTo(HaveOccurred())
})
}
}
15 changes: 8 additions & 7 deletions api/v1beta1/machinehealthcheck_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"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"
)

var (
Expand Down Expand Up @@ -84,22 +85,22 @@ func (m *MachineHealthCheck) Default() {
}

// ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
func (m *MachineHealthCheck) ValidateCreate() error {
return m.validate(nil)
func (m *MachineHealthCheck) ValidateCreate() (admission.Warnings, error) {
return nil, m.validate(nil)
}

// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type.
func (m *MachineHealthCheck) ValidateUpdate(old runtime.Object) error {
func (m *MachineHealthCheck) ValidateUpdate(old runtime.Object) (admission.Warnings, error) {
mhc, ok := old.(*MachineHealthCheck)
if !ok {
return apierrors.NewBadRequest(fmt.Sprintf("expected a MachineHealthCheck but got a %T", old))
return nil, apierrors.NewBadRequest(fmt.Sprintf("expected a MachineHealthCheck but got a %T", old))
}
return m.validate(mhc)
return nil, m.validate(mhc)
}

// ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
func (m *MachineHealthCheck) ValidateDelete() error {
return nil
func (m *MachineHealthCheck) ValidateDelete() (admission.Warnings, error) {
return nil, nil
}

func (m *MachineHealthCheck) validate(old *MachineHealthCheck) error {
Expand Down
53 changes: 35 additions & 18 deletions api/v1beta1/machinehealthcheck_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,15 @@ func TestMachineHealthCheckLabelSelectorAsSelectorValidation(t *testing.T) {
},
}
if tt.expectErr {
g.Expect(mhc.ValidateCreate()).NotTo(Succeed())
g.Expect(mhc.ValidateUpdate(mhc)).NotTo(Succeed())
_, err := mhc.ValidateCreate()
g.Expect(err).To(HaveOccurred())
_, err = mhc.ValidateUpdate(mhc)
g.Expect(err).To(HaveOccurred())
} else {
g.Expect(mhc.ValidateCreate()).To(Succeed())
g.Expect(mhc.ValidateUpdate(mhc)).To(Succeed())
_, err := mhc.ValidateCreate()
g.Expect(err).NotTo(HaveOccurred())
_, err = mhc.ValidateUpdate(mhc)
g.Expect(err).NotTo(HaveOccurred())
}
})
}
Expand Down Expand Up @@ -160,10 +164,11 @@ func TestMachineHealthCheckClusterNameImmutable(t *testing.T) {
},
}

_, err := newMHC.ValidateUpdate(oldMHC)
if tt.expectErr {
g.Expect(newMHC.ValidateUpdate(oldMHC)).NotTo(Succeed())
g.Expect(err).To(HaveOccurred())
} else {
g.Expect(newMHC.ValidateUpdate(oldMHC)).To(Succeed())
g.Expect(err).NotTo(HaveOccurred())
}
})
}
Expand Down Expand Up @@ -211,11 +216,15 @@ func TestMachineHealthCheckUnhealthyConditions(t *testing.T) {
},
}
if tt.expectErr {
g.Expect(mhc.ValidateCreate()).NotTo(Succeed())
g.Expect(mhc.ValidateUpdate(mhc)).NotTo(Succeed())
_, err := mhc.ValidateCreate()
g.Expect(err).To(HaveOccurred())
_, err = mhc.ValidateUpdate(mhc)
g.Expect(err).To(HaveOccurred())
} else {
g.Expect(mhc.ValidateCreate()).To(Succeed())
g.Expect(mhc.ValidateUpdate(mhc)).To(Succeed())
_, err := mhc.ValidateCreate()
g.Expect(err).NotTo(HaveOccurred())
_, err = mhc.ValidateUpdate(mhc)
g.Expect(err).NotTo(HaveOccurred())
}
})
}
Expand Down Expand Up @@ -286,11 +295,15 @@ func TestMachineHealthCheckNodeStartupTimeout(t *testing.T) {
}

if tt.expectErr {
g.Expect(mhc.ValidateCreate()).NotTo(Succeed())
g.Expect(mhc.ValidateUpdate(mhc)).NotTo(Succeed())
_, err := mhc.ValidateCreate()
g.Expect(err).To(HaveOccurred())
_, err = mhc.ValidateUpdate(mhc)
g.Expect(err).To(HaveOccurred())
} else {
g.Expect(mhc.ValidateCreate()).To(Succeed())
g.Expect(mhc.ValidateUpdate(mhc)).To(Succeed())
_, err := mhc.ValidateCreate()
g.Expect(err).NotTo(HaveOccurred())
_, err = mhc.ValidateUpdate(mhc)
g.Expect(err).NotTo(HaveOccurred())
}
}
}
Expand Down Expand Up @@ -345,11 +358,15 @@ func TestMachineHealthCheckMaxUnhealthy(t *testing.T) {
}

if tt.expectErr {
g.Expect(mhc.ValidateCreate()).NotTo(Succeed())
g.Expect(mhc.ValidateUpdate(mhc)).NotTo(Succeed())
_, err := mhc.ValidateCreate()
g.Expect(err).To(HaveOccurred())
_, err = mhc.ValidateUpdate(mhc)
g.Expect(err).To(HaveOccurred())
} else {
g.Expect(mhc.ValidateCreate()).To(Succeed())
g.Expect(mhc.ValidateUpdate(mhc)).To(Succeed())
_, err := mhc.ValidateCreate()
g.Expect(err).NotTo(HaveOccurred())
_, err = mhc.ValidateUpdate(mhc)
g.Expect(err).NotTo(HaveOccurred())
}
}
}
Expand Down
Loading