Skip to content

Commit 1892587

Browse files
sbueringersbueringer
committed
Add validation to ensure ClusterName fields are equal in MD/MS/MP
Signed-off-by: Stefan Büringer buringerst@vmware.com
1 parent ea1a774 commit 1892587

File tree

6 files changed

+195
-4
lines changed

6 files changed

+195
-4
lines changed

‎exp/internal/webhooks/machinepool.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,17 @@ func (webhook *MachinePool) validate(oldObj, newObj *clusterv1.MachinePool) erro
174174
)
175175
}
176176

177+
if newObj.Spec.ClusterName != newObj.Spec.Template.Spec.ClusterName {
178+
allErrs = append(
179+
allErrs,
180+
field.Invalid(
181+
specPath.Child("clusterName"),
182+
newObj.Spec.ClusterName,
183+
"spec.clusterName and spec.template.spec.clusterName must be set to the same value",
184+
),
185+
)
186+
}
187+
177188
if newObj.Spec.Template.Spec.Version != "" {
178189
if !version.KubeSemver.MatchString(newObj.Spec.Template.Spec.Version) {
179190
allErrs = append(allErrs, field.Invalid(specPath.Child("template", "spec", "version"), newObj.Spec.Template.Spec.Version, "must be a valid semantic version"))

‎exp/internal/webhooks/machinepool_test.go

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -309,7 +309,8 @@ func TestMachinePoolClusterNameImmutable(t *testing.T) {
309309
ClusterName: tt.newClusterName,
310310
Template: clusterv1.MachineTemplateSpec{
311311
Spec: clusterv1.MachineSpec{
312-
Bootstrap: clusterv1.Bootstrap{ConfigRef: &clusterv1.ContractVersionedObjectReference{}},
312+
ClusterName: tt.newClusterName,
313+
Bootstrap: clusterv1.Bootstrap{ConfigRef: &clusterv1.ContractVersionedObjectReference{}},
313314
},
314315
},
315316
},
@@ -320,7 +321,8 @@ func TestMachinePoolClusterNameImmutable(t *testing.T) {
320321
ClusterName: tt.oldClusterName,
321322
Template: clusterv1.MachineTemplateSpec{
322323
Spec: clusterv1.MachineSpec{
323-
Bootstrap: clusterv1.Bootstrap{ConfigRef: &clusterv1.ContractVersionedObjectReference{}},
324+
ClusterName: tt.oldClusterName,
325+
Bootstrap: clusterv1.Bootstrap{ConfigRef: &clusterv1.ContractVersionedObjectReference{}},
324326
},
325327
},
326328
},
@@ -338,6 +340,56 @@ func TestMachinePoolClusterNameImmutable(t *testing.T) {
338340
}
339341
}
340342

343+
func TestMachinePoolClusterNamesEqual(t *testing.T) {
344+
tests := []struct {
345+
name string
346+
specClusterName string
347+
specTemplateSpecClusterName string
348+
expectErr bool
349+
}{
350+
{
351+
name: "clusterName fields are set to the same value",
352+
specClusterName: "foo",
353+
specTemplateSpecClusterName: "foo",
354+
expectErr: false,
355+
},
356+
{
357+
name: "clusterName fields are set to different values",
358+
specClusterName: "foo",
359+
specTemplateSpecClusterName: "bar",
360+
expectErr: true,
361+
},
362+
}
363+
364+
for _, tt := range tests {
365+
t.Run(tt.name, func(t *testing.T) {
366+
g := NewWithT(t)
367+
368+
ms := &clusterv1.MachinePool{
369+
Spec: clusterv1.MachinePoolSpec{
370+
ClusterName: tt.specClusterName,
371+
Template: clusterv1.MachineTemplateSpec{
372+
Spec: clusterv1.MachineSpec{
373+
ClusterName: tt.specTemplateSpecClusterName,
374+
Bootstrap: clusterv1.Bootstrap{
375+
DataSecretName: ptr.To("data-secret"),
376+
},
377+
},
378+
},
379+
},
380+
}
381+
382+
warnings, err := (&MachinePool{}).ValidateCreate(ctx, ms)
383+
if tt.expectErr {
384+
g.Expect(err).To(HaveOccurred())
385+
} else {
386+
g.Expect(err).ToNot(HaveOccurred())
387+
}
388+
g.Expect(warnings).To(BeEmpty())
389+
})
390+
}
391+
}
392+
341393
func TestMachinePoolVersionValidation(t *testing.T) {
342394
tests := []struct {
343395
name string

‎internal/webhooks/machinedeployment.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,17 @@ func (webhook *MachineDeployment) validate(oldMD, newMD *clusterv1.MachineDeploy
241241
)
242242
}
243243

244+
if newMD.Spec.ClusterName != newMD.Spec.Template.Spec.ClusterName {
245+
allErrs = append(
246+
allErrs,
247+
field.Invalid(
248+
specPath.Child("clusterName"),
249+
newMD.Spec.ClusterName,
250+
"spec.clusterName and spec.template.spec.clusterName must be set to the same value",
251+
),
252+
)
253+
}
254+
244255
if newMD.Spec.Strategy != nil && newMD.Spec.Strategy.RollingUpdate != nil {
245256
total := 1
246257
if newMD.Spec.Replicas != nil {

‎internal/webhooks/machinedeployment_test.go

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ func TestMachineDeploymentDefault(t *testing.T) {
4343
ClusterName: "test-cluster",
4444
Template: clusterv1.MachineTemplateSpec{
4545
Spec: clusterv1.MachineSpec{
46-
Version: "1.19.10",
46+
ClusterName: "test-cluster",
47+
Version: "1.19.10",
4748
Bootstrap: clusterv1.Bootstrap{
4849
DataSecretName: ptr.To("data-secret"),
4950
},
@@ -154,7 +155,8 @@ func TestMachineDeploymentReferenceDefault(t *testing.T) {
154155
ClusterName: "test-cluster",
155156
Template: clusterv1.MachineTemplateSpec{
156157
Spec: clusterv1.MachineSpec{
157-
Version: "1.19.10",
158+
ClusterName: "test-cluster",
159+
Version: "1.19.10",
158160
Bootstrap: clusterv1.Bootstrap{
159161
ConfigRef: &clusterv1.ContractVersionedObjectReference{},
160162
},
@@ -690,6 +692,7 @@ func TestMachineDeploymentClusterNameImmutable(t *testing.T) {
690692
ClusterName: tt.newClusterName,
691693
Template: clusterv1.MachineTemplateSpec{
692694
Spec: clusterv1.MachineSpec{
695+
ClusterName: tt.newClusterName,
693696
Bootstrap: clusterv1.Bootstrap{
694697
DataSecretName: ptr.To("data-secret"),
695698
},
@@ -703,6 +706,7 @@ func TestMachineDeploymentClusterNameImmutable(t *testing.T) {
703706
ClusterName: tt.oldClusterName,
704707
Template: clusterv1.MachineTemplateSpec{
705708
Spec: clusterv1.MachineSpec{
709+
ClusterName: tt.oldClusterName,
706710
Bootstrap: clusterv1.Bootstrap{
707711
DataSecretName: ptr.To("data-secret"),
708712
},
@@ -728,6 +732,56 @@ func TestMachineDeploymentClusterNameImmutable(t *testing.T) {
728732
}
729733
}
730734

735+
func TestMachineDeploymentClusterNamesEqual(t *testing.T) {
736+
tests := []struct {
737+
name string
738+
specClusterName string
739+
specTemplateSpecClusterName string
740+
expectErr bool
741+
}{
742+
{
743+
name: "clusterName fields are set to the same value",
744+
specClusterName: "foo",
745+
specTemplateSpecClusterName: "foo",
746+
expectErr: false,
747+
},
748+
{
749+
name: "clusterName fields are set to different values",
750+
specClusterName: "foo",
751+
specTemplateSpecClusterName: "bar",
752+
expectErr: true,
753+
},
754+
}
755+
756+
for _, tt := range tests {
757+
t.Run(tt.name, func(t *testing.T) {
758+
g := NewWithT(t)
759+
760+
ms := &clusterv1.MachineDeployment{
761+
Spec: clusterv1.MachineDeploymentSpec{
762+
ClusterName: tt.specClusterName,
763+
Template: clusterv1.MachineTemplateSpec{
764+
Spec: clusterv1.MachineSpec{
765+
ClusterName: tt.specTemplateSpecClusterName,
766+
Bootstrap: clusterv1.Bootstrap{
767+
DataSecretName: ptr.To("data-secret"),
768+
},
769+
},
770+
},
771+
},
772+
}
773+
774+
warnings, err := (&MachineDeployment{}).ValidateCreate(ctx, ms)
775+
if tt.expectErr {
776+
g.Expect(err).To(HaveOccurred())
777+
} else {
778+
g.Expect(err).ToNot(HaveOccurred())
779+
}
780+
g.Expect(warnings).To(BeEmpty())
781+
})
782+
}
783+
}
784+
731785
func TestMachineDeploymentTemplateMetadataValidation(t *testing.T) {
732786
tests := []struct {
733787
name string

‎internal/webhooks/machineset.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,17 @@ func (webhook *MachineSet) validate(oldMS, newMS *clusterv1.MachineSet) error {
209209
)
210210
}
211211

212+
if newMS.Spec.ClusterName != newMS.Spec.Template.Spec.ClusterName {
213+
allErrs = append(
214+
allErrs,
215+
field.Invalid(
216+
specPath.Child("clusterName"),
217+
newMS.Spec.ClusterName,
218+
"spec.clusterName and spec.template.spec.clusterName must be set to the same value",
219+
),
220+
)
221+
}
222+
212223
if newMS.Spec.Template.Spec.Version != "" {
213224
if !version.KubeSemver.MatchString(newMS.Spec.Template.Spec.Version) {
214225
allErrs = append(

‎internal/webhooks/machineset_test.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,7 @@ func TestMachineSetClusterNameImmutable(t *testing.T) {
398398
ClusterName: tt.newClusterName,
399399
Template: clusterv1.MachineTemplateSpec{
400400
Spec: clusterv1.MachineSpec{
401+
ClusterName: tt.newClusterName,
401402
Bootstrap: clusterv1.Bootstrap{
402403
DataSecretName: ptr.To("data-secret"),
403404
},
@@ -411,6 +412,7 @@ func TestMachineSetClusterNameImmutable(t *testing.T) {
411412
ClusterName: tt.oldClusterName,
412413
Template: clusterv1.MachineTemplateSpec{
413414
Spec: clusterv1.MachineSpec{
415+
ClusterName: tt.oldClusterName,
414416
Bootstrap: clusterv1.Bootstrap{
415417
DataSecretName: ptr.To("data-secret"),
416418
},
@@ -430,6 +432,56 @@ func TestMachineSetClusterNameImmutable(t *testing.T) {
430432
}
431433
}
432434

435+
func TestMachineSetClusterNamesEqual(t *testing.T) {
436+
tests := []struct {
437+
name string
438+
specClusterName string
439+
specTemplateSpecClusterName string
440+
expectErr bool
441+
}{
442+
{
443+
name: "clusterName fields are set to the same value",
444+
specClusterName: "foo",
445+
specTemplateSpecClusterName: "foo",
446+
expectErr: false,
447+
},
448+
{
449+
name: "clusterName fields are set to different values",
450+
specClusterName: "foo",
451+
specTemplateSpecClusterName: "bar",
452+
expectErr: true,
453+
},
454+
}
455+
456+
for _, tt := range tests {
457+
t.Run(tt.name, func(t *testing.T) {
458+
g := NewWithT(t)
459+
460+
ms := &clusterv1.MachineSet{
461+
Spec: clusterv1.MachineSetSpec{
462+
ClusterName: tt.specClusterName,
463+
Template: clusterv1.MachineTemplateSpec{
464+
Spec: clusterv1.MachineSpec{
465+
ClusterName: tt.specTemplateSpecClusterName,
466+
Bootstrap: clusterv1.Bootstrap{
467+
DataSecretName: ptr.To("data-secret"),
468+
},
469+
},
470+
},
471+
},
472+
}
473+
474+
warnings, err := (&MachineSet{}).ValidateCreate(ctx, ms)
475+
if tt.expectErr {
476+
g.Expect(err).To(HaveOccurred())
477+
} else {
478+
g.Expect(err).ToNot(HaveOccurred())
479+
}
480+
g.Expect(warnings).To(BeEmpty())
481+
})
482+
}
483+
}
484+
433485
func TestMachineSetVersionValidation(t *testing.T) {
434486
tests := []struct {
435487
name string

0 commit comments

Comments
 (0)