Skip to content

Commit 9eeff91

Browse files
sonasingh46k8s-infra-cherrypick-robot
sonasingh46
authored and
k8s-infra-cherrypick-robot
committed
allow unsetting acceleratedNetworking deprecated field on azuremachine
Signed-off-by: Ashutosh Kumar <[email protected]>
1 parent b723c2a commit 9eeff91

File tree

4 files changed

+254
-1
lines changed

4 files changed

+254
-1
lines changed

‎api/v1beta1/azuremachine_webhook.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,10 @@ func (mw *azureMachineWebhook) ValidateUpdate(ctx context.Context, oldObj, newOb
154154
allErrs = append(allErrs, err)
155155
}
156156

157-
if err := webhookutils.ValidateImmutable(
157+
// Spec.AcceleratedNetworking can only be reset to nil and no other changes apart from that
158+
// is accepted if the field is set.
159+
// Ref issue #3518
160+
if err := webhookutils.ValidateZeroTransition(
158161
field.NewPath("Spec", "AcceleratedNetworking"),
159162
old.Spec.AcceleratedNetworking,
160163
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
@@ -536,6 +536,34 @@ func TestAzureMachine_ValidateUpdate(t *testing.T) {
536536
},
537537
wantErr: false,
538538
},
539+
{
540+
name: "validTest: azuremachine.spec.AcceleratedNetworking transition(from true) to nil is acceptable",
541+
oldMachine: &AzureMachine{
542+
Spec: AzureMachineSpec{
543+
AcceleratedNetworking: pointer.Bool(true),
544+
},
545+
},
546+
newMachine: &AzureMachine{
547+
Spec: AzureMachineSpec{
548+
AcceleratedNetworking: nil,
549+
},
550+
},
551+
wantErr: false,
552+
},
553+
{
554+
name: "validTest: azuremachine.spec.AcceleratedNetworking transition(from false) to nil is acceptable",
555+
oldMachine: &AzureMachine{
556+
Spec: AzureMachineSpec{
557+
AcceleratedNetworking: pointer.Bool(false),
558+
},
559+
},
560+
newMachine: &AzureMachine{
561+
Spec: AzureMachineSpec{
562+
AcceleratedNetworking: nil,
563+
},
564+
},
565+
wantErr: false,
566+
},
539567
{
540568
name: "invalidTest: azuremachine.spec.SpotVMOptions is immutable",
541569
oldMachine: &AzureMachine{

‎util/webhook/validator.go‎

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

55+
// ValidateZeroTransition validates equality across two values, with only exception to allow
56+
// the value to transition of a zero value.
57+
func ValidateZeroTransition(path *field.Path, oldVal, newVal any) *field.Error {
58+
if reflect.ValueOf(newVal).IsZero() {
59+
// unsetting the field is allowed
60+
return nil
61+
}
62+
return ValidateImmutable(path, oldVal, newVal)
63+
}
64+
5565
// EnsureStringSlicesAreEquivalent returns if two string slices have equal lengths,
5666
// and that they have the exact same items; it does not enforce strict ordering of items.
5767
func EnsureStringSlicesAreEquivalent(a []string, b []string) bool {

‎util/webhook/validator_test.go‎

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

0 commit comments

Comments
 (0)