Added additional checks to prevent duplicate failure domain and zone definitions.

Author: vr4manta

pkg/types/vsphere/validation/machinepool.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 
 	"k8s.io/apimachinery/pkg/util/validation/field"
+	"k8s.io/utils/strings/slices"
 
 	"github.com/openshift/installer/pkg/types"
 	"github.com/openshift/installer/pkg/types/vsphere"
@@ -51,14 +52,24 @@ func ValidateMachinePool(platform *vsphere.Platform, machinePool *types.MachineP
 	}
 
 	if len(vspherePool.Zones) > 0 {
+		var zoneRefs []string
 		if len(platform.FailureDomains) == 0 {
 			return append(allErrs, field.Required(fldPath.Child("zones"), "failureDomains must be defined if zones are defined"))
 		}
-		for _, zone := range vspherePool.Zones {
+		for index, zone := range vspherePool.Zones {
 			err := validate.ClusterName1035(zone)
 			if err != nil {
 				allErrs = append(allErrs, field.Invalid(fldPath.Child("zones"), vspherePool.Zones, err.Error()))
 			}
+
+			// This checks to make sure ref is not duplicated
+			if slices.Contains(zoneRefs, zone) {
+				allErrs = append(allErrs, field.Duplicate(fldPath.Child("zones").Index(index), zone))
+			} else {
+				zoneRefs = append(zoneRefs, zone)
+			}
+
+			// Verify zone references a valid failure domain
 			zoneDefined := false
 			for _, failureDomain := range platform.FailureDomains {
 				if failureDomain.Name == zone {

pkg/types/vsphere/validation/machinepool_test.go

@@ -154,6 +154,22 @@ func TestValidateMachinePool(t *testing.T) {
 			expectedZones:  &[]string{"test-east-1a", "test-east-2a"},
 			expectedErrMsg: "",
 		},
+		{
+			name:     "multi-zone duplicate zones defined for control plane pool",
+			platform: validPlatform(),
+			pool: &types.MachinePool{
+				Name: types.MachinePoolControlPlaneRoleName,
+				Platform: types.MachinePoolPlatform{
+					VSphere: &vsphere.MachinePool{
+						Zones: []string{
+							"test-east-1a",
+							"test-east-1a",
+						},
+					},
+				},
+			},
+			expectedErrMsg: `test-path.zones\[1]: Duplicate value: "test-east-1a"`,
+		},
 		{
 			name:     "multi-zone no zones defined for compute pool",
 			platform: validPlatform(),
@@ -166,6 +182,22 @@ func TestValidateMachinePool(t *testing.T) {
 			expectedZones:  &[]string{"test-east-1a", "test-east-2a"},
 			expectedErrMsg: "",
 		},
+		{
+			name:     "multi-zone duplicate zones defined for compute pool",
+			platform: validPlatform(),
+			pool: &types.MachinePool{
+				Name: types.MachinePoolComputeRoleName,
+				Platform: types.MachinePoolPlatform{
+					VSphere: &vsphere.MachinePool{
+						Zones: []string{
+							"test-east-1a",
+							"test-east-1a",
+						},
+					},
+				},
+			},
+			expectedErrMsg: `test-path.zones\[1]: Duplicate value: "test-east-1a"`,
+		},
 		{
 			name:     "multi-zone undefined zone",
 			platform: validPlatform(),

pkg/types/vsphere/validation/platform.go

@@ -9,6 +9,7 @@ import (
 	"github.com/sirupsen/logrus"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/validation/field"
+	"k8s.io/utils/strings/slices"
 
 	configv1 "github.com/openshift/api/config/v1"
 	"github.com/openshift/installer/pkg/types"
@@ -115,12 +116,20 @@ func validateVCenters(p *vsphere.Platform, fldPath *field.Path) field.ErrorList
 }
 
 func validateFailureDomains(p *vsphere.Platform, fldPath *field.Path, isLegacyUpi bool) field.ErrorList {
+	var fdNames []string
 	allErrs := field.ErrorList{}
 	topologyFld := fldPath.Child("topology")
 	var associatedVCenter *vsphere.VCenter
-	for _, failureDomain := range p.FailureDomains {
+	for index, failureDomain := range p.FailureDomains {
 		if len(failureDomain.Name) == 0 {
 			allErrs = append(allErrs, field.Required(fldPath.Child("name"), "must specify the name"))
+		} else {
+			// Check and make sure the name is not duplicated w/ any other Failure Domains
+			if slices.Contains(fdNames, failureDomain.Name) {
+				allErrs = append(allErrs, field.Duplicate(fldPath.Child("name").Index(index), failureDomain.Name))
+			} else {
+				fdNames = append(fdNames, failureDomain.Name)
+			}
 		}
 		if len(failureDomain.Server) > 0 {
 			for i, vcenter := range p.VCenters {

pkg/types/vsphere/validation/platform_test.go

@@ -319,6 +319,15 @@ func TestValidatePlatform(t *testing.T) {
 			}(),
 			expectedError: `^test-path\.failureDomains\.name: Required value: must specify the name`,
 		},
+		{
+			name: "Multi-zone platform failureDomain duplicate names",
+			platform: func() *vsphere.Platform {
+				p := validPlatform()
+				p.FailureDomains[1].Name = p.FailureDomains[0].Name
+				return p
+			}(),
+			expectedError: `test-path\.failureDomains\.name\[1\]: Duplicate value: "test-east-1a"`,
+		},
 		{
 			name: "Multi-zone platform failureDomain zone missing tag category",
 			platform: func() *vsphere.Platform {