Skip to content

Commit dd6e02f

Browse files
sonasingh46mboersma
sonasingh46
authored and
mboersma
committed
allow unsetting acceleratedNetworking deprecated field on azuremachine
Signed-off-by: Ashutosh Kumar <[email protected]>
1 parent cc7c34b commit dd6e02f

File tree

4 files changed

+255
-1
lines changed

4 files changed

+255
-1
lines changed

‎api/v1beta1/azuremachine_webhook.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,10 @@ func (m *AzureMachine) ValidateUpdate(oldRaw runtime.Object) error {
124124
allErrs = append(allErrs, err)
125125
}
126126

127-
if err := webhookutils.ValidateImmutable(
127+
// Spec.AcceleratedNetworking can only be reset to nil and no other changes apart from that
128+
// is accepted if the field is set.
129+
// Ref issue #3518
130+
if err := webhookutils.ValidateZeroTransition(
128131
field.NewPath("Spec", "AcceleratedNetworking"),
129132
old.Spec.AcceleratedNetworking,
130133
m.Spec.AcceleratedNetworking); err != nil {

‎api/v1beta1/azuremachine_webhook_test.go‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,34 @@ func TestAzureMachine_ValidateUpdate(t *testing.T) {
485485
},
486486
wantErr: false,
487487
},
488+
{
489+
name: "validTest: azuremachine.spec.AcceleratedNetworking transition(from true) to nil is acceptable",
490+
oldMachine: &AzureMachine{
491+
Spec: AzureMachineSpec{
492+
AcceleratedNetworking: pointer.Bool(true),
493+
},
494+
},
495+
newMachine: &AzureMachine{
496+
Spec: AzureMachineSpec{
497+
AcceleratedNetworking: nil,
498+
},
499+
},
500+
wantErr: false,
501+
},
502+
{
503+
name: "validTest: azuremachine.spec.AcceleratedNetworking transition(from false) to nil is acceptable",
504+
oldMachine: &AzureMachine{
505+
Spec: AzureMachineSpec{
506+
AcceleratedNetworking: pointer.Bool(false),
507+
},
508+
},
509+
newMachine: &AzureMachine{
510+
Spec: AzureMachineSpec{
511+
AcceleratedNetworking: nil,
512+
},
513+
},
514+
wantErr: false,
515+
},
488516
{
489517
name: "invalidTest: azuremachine.spec.SpotVMOptions is immutable",
490518
oldMachine: &AzureMachine{

‎util/webhook/validator.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,16 @@ func ValidateImmutable(path *field.Path, oldVal, newVal any) *field.Error {
169169
return nil
170170
}
171171

172+
// ValidateZeroTransition validates equality across two values, with only exception to allow
173+
// the value to transition of a zero value.
174+
func ValidateZeroTransition(path *field.Path, oldVal, newVal any) *field.Error {
175+
if reflect.ValueOf(newVal).IsZero() {
176+
// unsetting the field is allowed
177+
return nil
178+
}
179+
return ValidateImmutable(path, oldVal, newVal)
180+
}
181+
172182
// EnsureStringSlicesAreEquivalent returns if two string slices have equal lengths,
173183
// and that they have the exact same items; it does not enforce strict ordering of items.
174184
func EnsureStringSlicesAreEquivalent(a []string, b []string) bool {

‎util/webhook/validator_test.go‎

Lines changed: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"github.com/Azure/go-autorest/autorest/to"
2323
. "github.com/onsi/gomega"
2424
"k8s.io/apimachinery/pkg/util/validation/field"
25+
"k8s.io/utils/pointer"
2526
)
2627

2728
func TestValidateImmutableBoolPtr(t *testing.T) {
@@ -299,3 +300,215 @@ func TestEnsureStringSlicesAreEquivalent(t *testing.T) {
299300
})
300301
}
301302
}
303+
304+
func TestValidateZeroTransitionPtr(t *testing.T) {
305+
testPath := field.NewPath("Spec", "Foo")
306+
307+
tests := []struct {
308+
name string
309+
input1 *bool
310+
input2 *bool
311+
expectedOutput *field.Error
312+
}{
313+
{
314+
name: "nil",
315+
input1: nil,
316+
input2: nil,
317+
},
318+
{
319+
name: "no change",
320+
input1: pointer.Bool(true),
321+
input2: pointer.Bool(true),
322+
},
323+
{
324+
name: "can unset",
325+
input1: pointer.Bool(true),
326+
input2: nil,
327+
},
328+
{
329+
name: "can't set from empty",
330+
input1: nil,
331+
input2: pointer.Bool(true),
332+
expectedOutput: field.Invalid(testPath, nil, setMessage),
333+
},
334+
{
335+
name: "can't change",
336+
input1: pointer.Bool(true),
337+
input2: pointer.Bool(false),
338+
expectedOutput: field.Invalid(testPath, nil, immutableMessage),
339+
},
340+
}
341+
for _, tc := range tests {
342+
t.Run(tc.name, func(t *testing.T) {
343+
g := NewWithT(t)
344+
err := ValidateZeroTransition(testPath, tc.input1, tc.input2)
345+
if tc.expectedOutput != nil {
346+
g.Expect(err).To(HaveOccurred())
347+
g.Expect(err.Detail).To(Equal(tc.expectedOutput.Detail))
348+
g.Expect(err.Type).To(Equal(tc.expectedOutput.Type))
349+
g.Expect(err.Field).To(Equal(tc.expectedOutput.Field))
350+
} else {
351+
g.Expect(err).NotTo(HaveOccurred())
352+
}
353+
})
354+
}
355+
}
356+
357+
func TestValidateZeroTransitionString(t *testing.T) {
358+
testPath := field.NewPath("Spec", "Foo")
359+
360+
tests := []struct {
361+
name string
362+
input1 string
363+
input2 string
364+
expectedOutput *field.Error
365+
}{
366+
{
367+
name: "empty string",
368+
input1: "",
369+
input2: "",
370+
},
371+
{
372+
name: "no change",
373+
input1: "foo",
374+
input2: "foo",
375+
},
376+
{
377+
name: "can unset",
378+
input1: "foo",
379+
input2: "",
380+
},
381+
{
382+
name: "can't set from empty",
383+
input1: "",
384+
input2: "foo",
385+
expectedOutput: field.Invalid(testPath, nil, setMessage),
386+
},
387+
{
388+
name: "can't change",
389+
input1: "foo",
390+
input2: "bar",
391+
expectedOutput: field.Invalid(testPath, nil, immutableMessage),
392+
},
393+
}
394+
for _, tc := range tests {
395+
t.Run(tc.name, func(t *testing.T) {
396+
g := NewWithT(t)
397+
err := ValidateZeroTransition(testPath, tc.input1, tc.input2)
398+
if tc.expectedOutput != nil {
399+
g.Expect(err).To(HaveOccurred())
400+
g.Expect(err.Detail).To(Equal(tc.expectedOutput.Detail))
401+
g.Expect(err.Type).To(Equal(tc.expectedOutput.Type))
402+
g.Expect(err.Field).To(Equal(tc.expectedOutput.Field))
403+
} else {
404+
g.Expect(err).NotTo(HaveOccurred())
405+
}
406+
})
407+
}
408+
}
409+
410+
func TestValidateZeroTransitionStringPtr(t *testing.T) {
411+
testPath := field.NewPath("Spec", "Foo")
412+
413+
tests := []struct {
414+
name string
415+
input1 *string
416+
input2 *string
417+
expectedOutput *field.Error
418+
}{
419+
{
420+
name: "nil",
421+
input1: nil,
422+
input2: nil,
423+
},
424+
{
425+
name: "no change",
426+
input1: pointer.String("foo"),
427+
input2: pointer.String("foo"),
428+
},
429+
{
430+
name: "can unset",
431+
input1: pointer.String("foo"),
432+
input2: nil,
433+
},
434+
{
435+
name: "can't set from empty",
436+
input1: nil,
437+
input2: pointer.String("foo"),
438+
expectedOutput: field.Invalid(testPath, nil, setMessage),
439+
},
440+
{
441+
name: "can't change",
442+
input1: pointer.String("foo"),
443+
input2: pointer.String("bar"),
444+
expectedOutput: field.Invalid(testPath, nil, immutableMessage),
445+
},
446+
}
447+
for _, tc := range tests {
448+
t.Run(tc.name, func(t *testing.T) {
449+
g := NewWithT(t)
450+
err := ValidateZeroTransition(testPath, tc.input1, tc.input2)
451+
if tc.expectedOutput != nil {
452+
g.Expect(err).To(HaveOccurred())
453+
g.Expect(err.Detail).To(Equal(tc.expectedOutput.Detail))
454+
g.Expect(err.Type).To(Equal(tc.expectedOutput.Type))
455+
g.Expect(err.Field).To(Equal(tc.expectedOutput.Field))
456+
} else {
457+
g.Expect(err).NotTo(HaveOccurred())
458+
}
459+
})
460+
}
461+
}
462+
463+
func TestValidateZeroTransitionInt32(t *testing.T) {
464+
testPath := field.NewPath("Spec", "Foo")
465+
466+
tests := []struct {
467+
name string
468+
input1 int32
469+
input2 int32
470+
expectedOutput *field.Error
471+
}{
472+
{
473+
name: "unset",
474+
input1: 0,
475+
input2: 0,
476+
},
477+
{
478+
name: "no change",
479+
input1: 5,
480+
input2: 5,
481+
},
482+
{
483+
name: "can unset",
484+
input1: 5,
485+
input2: 0,
486+
},
487+
{
488+
name: "can't set from empty",
489+
input1: 0,
490+
input2: 5,
491+
expectedOutput: field.Invalid(testPath, nil, setMessage),
492+
},
493+
{
494+
name: "can't change",
495+
input1: 5,
496+
input2: 6,
497+
expectedOutput: field.Invalid(testPath, nil, immutableMessage),
498+
},
499+
}
500+
for _, tc := range tests {
501+
t.Run(tc.name, func(t *testing.T) {
502+
g := NewWithT(t)
503+
err := ValidateZeroTransition(testPath, tc.input1, tc.input2)
504+
if tc.expectedOutput != nil {
505+
g.Expect(err).To(HaveOccurred())
506+
g.Expect(err.Detail).To(Equal(tc.expectedOutput.Detail))
507+
g.Expect(err.Type).To(Equal(tc.expectedOutput.Type))
508+
g.Expect(err.Field).To(Equal(tc.expectedOutput.Field))
509+
} else {
510+
g.Expect(err).NotTo(HaveOccurred())
511+
}
512+
})
513+
}
514+
}

0 commit comments

Comments
 (0)