Fix orphaned NSX allocation on IP realization failure

When allocation_ip is unset and realization fails, Create tried to clean
up via resourceNsxtPolicyIPAddressAllocationDelete before d.SetId() had
ever been called, so the delete silently no-op'd on an empty ID and the
allocation was leaked on NSX Manager with no Terraform state pointing to
it. Set the ID temporarily so the real delete path (including cache
invalidation) runs, then clear it again so Create still reports failure.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Author: ksamoray

nsxt/resource_nsxt_policy_ip_address_allocation.go

@@ -144,9 +144,7 @@ func resourceNsxtPolicyIPAddressAllocationCreate(d *schema.ResourceData, m inter
 		stateConf := nsxtPolicyWaitForRealizationStateConf(connector, d, m, *obj.Path, timeout)
 		entity, err := stateConf.WaitForState()
 		if err != nil {
-			// Clean up NSX allocation
-			resourceNsxtPolicyIPAddressAllocationDelete(d, m)
-			return err
+			return cleanupFailedIPAddressAllocation(d, m, id, err)
 		}
 		realizedResource := entity.(model.GenericPolicyRealizedResource)
 		for _, attr := range realizedResource.ExtendedAttributes {
@@ -158,9 +156,7 @@ func resourceNsxtPolicyIPAddressAllocationCreate(d *schema.ResourceData, m inter
 				return resourceNsxtPolicyIPAddressAllocationRead(d, m)
 			}
 		}
-		// Clean up NSX allocation
-		resourceNsxtPolicyIPAddressAllocationDelete(d, m)
-		return fmt.Errorf("Failed to get realized IP for path %s", d.Get("path"))
+		return cleanupFailedIPAddressAllocation(d, m, id, fmt.Errorf("Failed to get realized IP for path %s", d.Get("path")))
 	}
 
 	d.SetId(id)
@@ -169,6 +165,20 @@ func resourceNsxtPolicyIPAddressAllocationCreate(d *schema.ResourceData, m inter
 	return resourceNsxtPolicyIPAddressAllocationRead(d, m)
 }
 
+// cleanupFailedIPAddressAllocation deletes an IPAddressAllocation that was already
+// created on NSX (via Patch) but whose realization failed, so it isn't leaked.
+// The ID is set temporarily so resourceNsxtPolicyIPAddressAllocationDelete can be
+// reused (including its cache invalidation), then cleared again since Create is
+// failing and no resource should end up in Terraform state.
+func cleanupFailedIPAddressAllocation(d *schema.ResourceData, m interface{}, id string, originalErr error) error {
+	d.SetId(id)
+	if deleteErr := resourceNsxtPolicyIPAddressAllocationDelete(d, m); deleteErr != nil {
+		log.Printf("[WARNING] Failed to clean up IPAddressAllocation with ID %s after create failure: %v", id, deleteErr)
+	}
+	d.SetId("")
+	return originalErr
+}
+
 func resourceNsxtPolicyIPAddressAllocationRead(d *schema.ResourceData, m interface{}) error {
 	connector := getPolicyConnector(m)
 	client := cliIpAllocationsClient(getSessionContext(d, m), connector)

nsxt/utgomock_resource_nsxt_policy_ip_address_allocation_test.go

@@ -7,6 +7,7 @@
 package nsxt
 
 import (
+	"errors"
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
@@ -18,8 +19,10 @@ import (
 	"go.uber.org/mock/gomock"
 
 	ippoolsapi "github.com/vmware/terraform-provider-nsxt/api/infra/ip_pools"
+	realizedstateapi "github.com/vmware/terraform-provider-nsxt/api/infra/realized_state"
 	utl "github.com/vmware/terraform-provider-nsxt/api/utl"
 	ippoolmocks "github.com/vmware/terraform-provider-nsxt/mocks/infra/ip_pools"
+	realizedstatemocks "github.com/vmware/terraform-provider-nsxt/mocks/infra/realized_state"
 )
 
 var (
@@ -55,6 +58,13 @@ func minimalNsxtIpAllocData() map[string]interface{} {
 	}
 }
 
+func minimalNsxtIpAllocDataNoIP() map[string]interface{} {
+	data := minimalNsxtIpAllocData()
+	delete(data, "allocation_ip")
+	data["timeout"] = 5
+	return data
+}
+
 func setupNsxtIpAllocMock(t *testing.T, ctrl *gomock.Controller) (*ippoolmocks.MockIpAllocationsClient, func()) {
 	mockSDK := ippoolmocks.NewMockIpAllocationsClient(ctrl)
 	mockWrapper := &ippoolsapi.IpAddressAllocationClientContext{
@@ -69,6 +79,20 @@ func setupNsxtIpAllocMock(t *testing.T, ctrl *gomock.Controller) (*ippoolmocks.M
 	return mockSDK, func() { cliIpAllocationsClient = original }
 }
 
+func setupNsxtIpAllocRealizedMock(t *testing.T, ctrl *gomock.Controller) (*realizedstatemocks.MockRealizedEntitiesClient, func()) {
+	mockSDK := realizedstatemocks.NewMockRealizedEntitiesClient(ctrl)
+	mockWrapper := &realizedstateapi.RealizedEntityClientContext{
+		Client:     mockSDK,
+		ClientType: utl.Local,
+	}
+
+	original := cliRealizedEntitiesClient
+	cliRealizedEntitiesClient = func(_ utl.SessionContext, _ vapiProtocolClient.Connector) *realizedstateapi.RealizedEntityClientContext {
+		return mockWrapper
+	}
+	return mockSDK, func() { cliRealizedEntitiesClient = original }
+}
+
 func TestMockResourceNsxtPolicyIPAddressAllocationCreate(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
@@ -103,6 +127,64 @@ func TestMockResourceNsxtPolicyIPAddressAllocationCreate(t *testing.T) {
 	})
 }
 
+func TestMockResourceNsxtPolicyIPAddressAllocationCreateRealizationCleanup(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+	mockSDK, restoreAlloc := setupNsxtIpAllocMock(t, ctrl)
+	defer restoreAlloc()
+	mockRealizedSDK, restoreRealized := setupNsxtIpAllocRealizedMock(t, ctrl)
+	defer restoreRealized()
+
+	t.Run("Create cleans up and clears ID when realization fails", func(t *testing.T) {
+		notFoundErr := vapiErrors.NotFound{}
+		gomock.InOrder(
+			mockSDK.EXPECT().Get(nsxtIpAllocPoolID, nsxtIpAllocID).Return(nsxModel.IpAddressAllocation{}, notFoundErr),
+			mockSDK.EXPECT().Patch(nsxtIpAllocPoolID, nsxtIpAllocID, gomock.Any()).Return(nil),
+			mockSDK.EXPECT().Get(nsxtIpAllocPoolID, nsxtIpAllocID).Return(nsxtIpAllocAPIResponse(), nil),
+		)
+		mockRealizedSDK.EXPECT().List(nsxtIpAllocPath, nil).Return(nsxModel.GenericPolicyRealizedResourceListResult{}, errors.New("realization query failed"))
+		// Cleanup must delete the already-created NSX object, keyed by the
+		// resource ID directly (not read back from d.Id(), which isn't set yet).
+		mockSDK.EXPECT().Delete(nsxtIpAllocPoolID, nsxtIpAllocID).Return(nil)
+
+		res := resourceNsxtPolicyIPAddressAllocation()
+		d := schema.TestResourceDataRaw(t, res.Schema, minimalNsxtIpAllocDataNoIP())
+
+		err := resourceNsxtPolicyIPAddressAllocationCreate(d, newGoMockProviderClient())
+		require.Error(t, err)
+		assert.Equal(t, "", d.Id())
+	})
+
+	t.Run("Create cleans up and clears ID when realized IP attribute is missing", func(t *testing.T) {
+		notFoundErr := vapiErrors.NotFound{}
+		realizedState := "REALIZED"
+		otherAttrKey := "other_attr"
+		gomock.InOrder(
+			mockSDK.EXPECT().Get(nsxtIpAllocPoolID, nsxtIpAllocID).Return(nsxModel.IpAddressAllocation{}, notFoundErr),
+			mockSDK.EXPECT().Patch(nsxtIpAllocPoolID, nsxtIpAllocID, gomock.Any()).Return(nil),
+			mockSDK.EXPECT().Get(nsxtIpAllocPoolID, nsxtIpAllocID).Return(nsxtIpAllocAPIResponse(), nil),
+		)
+		mockRealizedSDK.EXPECT().List(nsxtIpAllocPath, nil).Return(nsxModel.GenericPolicyRealizedResourceListResult{
+			Results: []nsxModel.GenericPolicyRealizedResource{
+				{
+					State: &realizedState,
+					ExtendedAttributes: []nsxModel.AttributeVal{
+						{Key: &otherAttrKey, Values: []string{"x"}},
+					},
+				},
+			},
+		}, nil)
+		mockSDK.EXPECT().Delete(nsxtIpAllocPoolID, nsxtIpAllocID).Return(nil)
+
+		res := resourceNsxtPolicyIPAddressAllocation()
+		d := schema.TestResourceDataRaw(t, res.Schema, minimalNsxtIpAllocDataNoIP())
+
+		err := resourceNsxtPolicyIPAddressAllocationCreate(d, newGoMockProviderClient())
+		require.Error(t, err)
+		assert.Equal(t, "", d.Id())
+	})
+}
+
 func TestMockResourceNsxtPolicyIPAddressAllocationRead(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()