Skip to content

Commit

Permalink
Adjust webhooks
Browse files Browse the repository at this point in the history 
Signed-off-by: Stefan Büringer buringerst@vmware.com
  • Loading branch information
@sbueringer
sbueringer committed May 23, 2023
1 parent c4846e9 commit ea0dc4d
Show file tree
Hide file tree
Showing 41 changed files with 422 additions and 318 deletions.
15 changes: 8 additions & 7 deletions api/v1beta1/machine_webhook.go
View file
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
View file
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
18 changes: 8 additions & 10 deletions api/v1beta1/machinedeployment_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,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 +165,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
38 changes: 25 additions & 13 deletions api/v1beta1/machinedeployment_webhook_test.go
View file
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
View file
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
View file
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

0 comments on commit ea0dc4d

Please sign in to comment.