From a349f981c031f40390e438d36563f35adacd6a06 Mon Sep 17 00:00:00 2001 From: Chin2691 Date: Mon, 18 May 2026 12:59:39 +0530 Subject: [PATCH] fix: OCPBUGS-86012: reset associatedVCenter in failure domain validation loop The associatedVCenter variable was declared outside the for loop that iterates over failure domains, causing it to retain a stale pointer from a previous iteration. When a failure domain referenced a non-existent vCenter server after a valid one, the nil check was skipped and validation passed silently. The installer then crashed during machine asset generation with a confusing error. Move the variable declaration inside the loop so it resets on each iteration. Co-authored-by: Cursor --- pkg/types/vsphere/validation/platform.go | 3 +- pkg/types/vsphere/validation/platform_test.go | 70 +++++++++++++++++++ 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/pkg/types/vsphere/validation/platform.go b/pkg/types/vsphere/validation/platform.go index c1709bd23c3..681d5ff1a6f 100644 --- a/pkg/types/vsphere/validation/platform.go +++ b/pkg/types/vsphere/validation/platform.go @@ -160,11 +160,10 @@ func validateFailureDomains(p *vsphere.Platform, platformFldPath *field.Path, fl tagUrnPattern := regexp.MustCompile(`^(urn):(vmomi):(InventoryServiceTag):([0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}):([^:]+)$`) allErrs := field.ErrorList{} topologyFld := fldPath.Child("topology") - var associatedVCenter *vsphere.VCenter - zoneNames := make(map[string]string) for index, failureDomain := range p.FailureDomains { + var associatedVCenter *vsphere.VCenter if regionName, ok := zoneNames[failureDomain.Zone]; !ok { zoneNames[failureDomain.Zone] = failureDomain.Region } else if regionName == failureDomain.Region { diff --git a/pkg/types/vsphere/validation/platform_test.go b/pkg/types/vsphere/validation/platform_test.go index 2e3cfbe1e1d..c9f503d83c9 100644 --- a/pkg/types/vsphere/validation/platform_test.go +++ b/pkg/types/vsphere/validation/platform_test.go @@ -1013,3 +1013,73 @@ func installConfig() *installConfigBuilder { func (icb *installConfigBuilder) build() *types.InstallConfig { return &icb.InstallConfig } + +func TestValidateFailureDomainStaleVCenter(t *testing.T) { + cases := []struct { + name string + platform *vsphere.Platform + expectError string + }{ + { + name: "second failure domain with invalid server should report error", + platform: func() *vsphere.Platform { + p := validPlatform() + p.FailureDomains = append(p.FailureDomains, vsphere.FailureDomain{ + Name: "test-east-3a", + Region: "test-east", + Zone: "test-east-3a", + Server: "non-existent-vcenter", + Topology: vsphere.Topology{ + Datacenter: "test-datacenter", + ComputeCluster: "/test-datacenter/host/test-cluster", + Datastore: "/test-datacenter/datastore/test-datastore", + Networks: []string{"test-portgroup"}, + ResourcePool: "/test-datacenter/host/test-cluster/Resources/test-resourcepool", + Folder: "/test-datacenter/vm/test-folder", + }, + }) + return p + }(), + expectError: "server does not exist in vcenters", + }, + { + name: "second failure domain with invalid server should not validate datacenter against wrong vcenter", + platform: func() *vsphere.Platform { + p := validPlatform() + p.FailureDomains = append(p.FailureDomains, vsphere.FailureDomain{ + Name: "test-east-3a", + Region: "test-east", + Zone: "test-east-3a", + Server: "non-existent-vcenter", + Topology: vsphere.Topology{ + Datacenter: "wrong-datacenter", + ComputeCluster: "/wrong-datacenter/host/test-cluster", + Datastore: "/wrong-datacenter/datastore/test-datastore", + Networks: []string{"test-portgroup"}, + Folder: "/wrong-datacenter/vm/test-folder", + }, + }) + return p + }(), + expectError: "server does not exist in vcenters", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + fldPath := field.NewPath("platform", "vsphere", "failureDomains") + platformPath := field.NewPath("platform", "vsphere") + errs := validateFailureDomains(tc.platform, platformPath, fldPath, false) + + found := false + for _, e := range errs { + if strings.Contains(e.Error(), tc.expectError) { + found = true + } + } + if !found { + t.Errorf("expected error containing %q but got: %v", tc.expectError, errs) + } + }) + } +}