Skip to content

refactor: Uses delete_on_create_timeout with default=true support across TPF resources #3809

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 27, 2025
Merged

refactor: Uses delete_on_create_timeout with default=true support across TPF resources #3809

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c7497f3
refactor: Uses delete_on_create_timeout with default support across T…
EspenAlbert Oct 23, 2025
5c6f18c
test: Add delete_on_create_timeout to ImportStateVerifyIgnore for mul…
EspenAlbert Oct 23, 2025
63f827c
test: Update timeout test to validate delete_on_create_timeout update…
EspenAlbert Oct 23, 2025
468d6a0
test: Enhance search deployment tests to include stable state checks …
EspenAlbert Oct 23, 2025
c89d6c8
Merge branch 'master' into CLOUDP-343190_tpf_delete_on_create_with_de…
EspenAlbert Oct 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions internal/common/cleanup/handle_timeout.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (

"github.com/hashicorp/terraform-plugin-framework-timeouts/resource/timeouts"
"github.com/hashicorp/terraform-plugin-framework/diag"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/retry"
"github.com/mongodb/terraform-provider-mongodbatlas/internal/common/constant"
)
Expand Down Expand Up @@ -100,14 +99,3 @@ func ResolveTimeout(ctx context.Context, t *timeouts.Value, operationName string
}
return timeoutDuration
}

// ResolveDeleteOnCreateTimeout returns true if delete_on_create_timeout should be enabled.
// Default behavior is true when not explicitly set to false.
func ResolveDeleteOnCreateTimeout(deleteOnCreateTimeout types.Bool) bool {
// If null or unknown, default to true
if deleteOnCreateTimeout.IsNull() || deleteOnCreateTimeout.IsUnknown() {
return true
}
// Otherwise use the explicit value
return deleteOnCreateTimeout.ValueBool()
}
2 changes: 1 addition & 1 deletion internal/service/advancedcluster/resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ func (r *rs) Create(ctx context.Context, req resource.CreateRequest, resp *resou
isFlex := IsFlex(latestReq.ReplicationSpecs)
projectID, clusterName := waitParams.ProjectID, waitParams.ClusterName
clusterDetailStr := fmt.Sprintf("Cluster name %s (project_id=%s).", clusterName, projectID)
if cleanup.ResolveDeleteOnCreateTimeout(plan.DeleteOnCreateTimeout) {
if plan.DeleteOnCreateTimeout.ValueBool() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can plan.DeleteOnCreateTimeout be null?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it has a default value of true

var deferCall func()
ctx, deferCall = cleanup.OnTimeout(
ctx, waitParams.Timeout, diags.AddWarning, clusterDetailStr, DeleteClusterNoWait(r.Client, projectID, clusterName, isFlex),
Expand Down
6 changes: 5 additions & 1 deletion internal/service/advancedcluster/schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,11 @@ func resourceSchema(ctx context.Context) schema.Schema {
MarkdownDescription: "Date and time when MongoDB Cloud created this cluster. This parameter expresses its value in ISO 8601 format in UTC.",
},
"delete_on_create_timeout": schema.BoolAttribute{
Optional: true,
Computed: true,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see we're adding computed but we're also adding the plan modifier. I would like certainty that we're not adding one more field to the output plan that says known after apply. We're getting a few GH issues lately talking about the plan verbosity

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually this PR improves that behavior by showing the planned true value (see doc)

Optional: true,
PlanModifiers: []planmodifier.Bool{
customplanmodifier.CreateOnlyBoolWithDefault(true),
},
MarkdownDescription: "Indicates whether to delete the resource being created if a timeout is reached when waiting for completion. When set to `true` and timeout occurs, it triggers the deletion and returns immediately without waiting for deletion to complete. When set to `false`, the timeout will not trigger resource deletion. If you suspect a transient error when the value is `true`, wait before retrying to allow resource deletion to finish. Default is `true`.",
},
"encryption_at_rest_provider": schema.StringAttribute{
Expand Down
2 changes: 1 addition & 1 deletion internal/service/encryptionatrestprivateendpoint/resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func (r *encryptionAtRestPrivateEndpointRS) Create(ctx context.Context, req reso
}

finalResp, err := waitStateTransition(ctx, projectID, cloudProvider, createResp.GetId(), connV2.EncryptionAtRestUsingCustomerKeyManagementApi, createTimeout)
err = cleanup.HandleCreateTimeout(cleanup.ResolveDeleteOnCreateTimeout(earPrivateEndpointPlan.DeleteOnCreateTimeout), err, func(ctxCleanup context.Context) error {
err = cleanup.HandleCreateTimeout(earPrivateEndpointPlan.DeleteOnCreateTimeout.ValueBool(), err, func(ctxCleanup context.Context) error {
cleanResp, cleanErr := connV2.EncryptionAtRestUsingCustomerKeyManagementApi.RequestPrivateEndpointDeletion(ctxCleanup, projectID, cloudProvider, createResp.GetId()).Execute()
if validate.StatusNotFound(cleanResp) {
return nil
Expand Down
3 changes: 2 additions & 1 deletion internal/service/encryptionatrestprivateendpoint/resource_schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,10 @@ func ResourceSchema(ctx context.Context) schema.Schema {
Delete: true,
}),
"delete_on_create_timeout": schema.BoolAttribute{
Computed: true,
Optional: true,
PlanModifiers: []planmodifier.Bool{
customplanmodifier.CreateOnly(),
customplanmodifier.CreateOnlyBoolWithDefault(true),
},
MarkdownDescription: "Indicates whether to delete the resource being created if a timeout is reached when waiting for completion. When set to `true` and timeout occurs, it triggers the deletion and returns immediately without waiting for deletion to complete. When set to `false`, the timeout will not trigger resource deletion. If you suspect a transient error when the value is `true`, wait before retrying to allow resource deletion to finish. Default is `true`.",
},
Expand Down
27 changes: 13 additions & 14 deletions internal/service/encryptionatrestprivateendpoint/resource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,17 @@ const (
earResourceName = "mongodbatlas_encryption_at_rest.test"
)

func importStep(config string) resource.TestStep {
return resource.TestStep{
Config: config,
ResourceName: resourceName,
ImportStateIdFunc: importStateIDFunc(resourceName),
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"delete_on_create_timeout"},
}
}

func TestAccEncryptionAtRestPrivateEndpoint_Azure_basic(t *testing.T) {
resource.Test(t, *basicTestCaseAzure(t))
}
Expand Down Expand Up @@ -106,13 +117,7 @@ func basicTestCaseAzure(tb testing.TB) *resource.TestCase {
Config: configAzureBasic(projectID, azureKeyVault, region, false),
Check: checkBasic(projectID, *azureKeyVault.AzureEnvironment, region, false),
},
{
Config: configAzureBasic(projectID, azureKeyVault, region, false),
ResourceName: resourceName,
ImportStateIdFunc: importStateIDFunc(resourceName),
ImportState: true,
ImportStateVerify: true,
},
importStep(configAzureBasic(projectID, azureKeyVault, region, false)),
},
}
}
Expand Down Expand Up @@ -209,13 +214,7 @@ func basicTestCaseAWS(tb testing.TB) *resource.TestCase {
Config: configAWSBasic(projectID, awsIAMRoleName, awsIAMRolePolicyName, &awsKmsPrivateNetworking),
Check: checkBasic(projectID, "AWS", region, true),
},
{
Config: configAWSBasic(projectID, awsIAMRoleName, awsIAMRolePolicyName, &awsKms),
ResourceName: resourceName,
ImportStateIdFunc: importStateIDFunc(resourceName),
ImportState: true,
ImportStateVerify: true,
},
importStep(configAWSBasic(projectID, awsIAMRoleName, awsIAMRolePolicyName, &awsKms)),
},
}
}
Expand Down
2 changes: 1 addition & 1 deletion internal/service/pushbasedlogexport/resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func (r *pushBasedLogExportRS) Create(ctx context.Context, req resource.CreateRe
logExportConfigResp, err := WaitStateTransition(ctx, projectID, connV2.PushBasedLogExportApi,
retryTimeConfig(timeout, minTimeoutCreateUpdate))

err = cleanup.HandleCreateTimeout(cleanup.ResolveDeleteOnCreateTimeout(tfPlan.DeleteOnCreateTimeout), err, func(ctx context.Context) error {
err = cleanup.HandleCreateTimeout(tfPlan.DeleteOnCreateTimeout.ValueBool(), err, func(ctx context.Context) error {
cleanResp, cleanErr := connV2.PushBasedLogExportApi.DeleteLogExport(ctx, projectID).Execute()
if validate.StatusNotFound(cleanResp) {
return nil
Expand Down
3 changes: 2 additions & 1 deletion internal/service/pushbasedlogexport/resource_schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,10 @@ func ResourceSchema(ctx context.Context) schema.Schema {
Delete: true,
}),
"delete_on_create_timeout": schema.BoolAttribute{
Computed: true,
Optional: true,
PlanModifiers: []planmodifier.Bool{
customplanmodifier.CreateOnly(),
customplanmodifier.CreateOnlyBoolWithDefault(true),
},
MarkdownDescription: "Indicates whether to delete the resource being created if a timeout is reached when waiting for completion. When set to `true` and timeout occurs, it triggers the deletion and returns immediately without waiting for deletion to complete. When set to `false`, the timeout will not trigger resource deletion. If you suspect a transient error when the value is `true`, wait before retrying to allow resource deletion to finish. Default is `true`.",
},
Expand Down
1 change: 1 addition & 0 deletions internal/service/pushbasedlogexport/resource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ func basicTestCase(tb testing.TB) *resource.TestCase {
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIdentifierAttribute: "project_id",
ImportStateVerifyIgnore: []string{"delete_on_create_timeout"},
},
},
}
Expand Down
2 changes: 1 addition & 1 deletion internal/service/searchdeployment/resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func (r *rs) Create(ctx context.Context, req resource.CreateRequest, resp *resou

deploymentResp, err := WaitSearchNodeStateTransition(ctx, projectID, clusterName, connV2.AtlasSearchApi,
RetryTimeConfig(createTimeout, minTimeoutCreateUpdate))
err = cleanup.HandleCreateTimeout(cleanup.ResolveDeleteOnCreateTimeout(plan.DeleteOnCreateTimeout), err, func(ctxCleanup context.Context) error {
err = cleanup.HandleCreateTimeout(plan.DeleteOnCreateTimeout.ValueBool(), err, func(ctxCleanup context.Context) error {
_, err := connV2.AtlasSearchApi.DeleteClusterSearchDeployment(ctxCleanup, projectID, clusterName).Execute()
return err
})
Expand Down
7 changes: 6 additions & 1 deletion internal/service/searchdeployment/resource_schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
"github.com/hashicorp/terraform-plugin-framework/schema/validator"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/mongodb/terraform-provider-mongodbatlas/internal/common/customplanmodifier"
)

func ResourceSchema(ctx context.Context) schema.Schema {
Expand Down Expand Up @@ -68,7 +69,11 @@ func ResourceSchema(ctx context.Context) schema.Schema {
Optional: true,
},
"delete_on_create_timeout": schema.BoolAttribute{
Optional: true,
Computed: true,
Optional: true,
PlanModifiers: []planmodifier.Bool{
customplanmodifier.CreateOnlyBoolWithDefault(true),
},
MarkdownDescription: "Indicates whether to delete the resource being created if a timeout is reached when waiting for completion. When set to `true` and timeout occurs, it triggers the deletion and returns immediately without waiting for deletion to complete. When set to `false`, the timeout will not trigger resource deletion. If you suspect a transient error when the value is `true`, wait before retrying to allow resource deletion to finish. Default is `true`.",
},
"encryption_at_rest_provider": schema.StringAttribute{
Expand Down
25 changes: 13 additions & 12 deletions internal/service/searchdeployment/resource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ const (
dataSourceID = "data.mongodbatlas_search_deployment.test"
)

func importStep(tfConfig string) resource.TestStep {
func importStep() resource.TestStep {
return resource.TestStep{
Config: tfConfig,
ResourceName: resourceID,
ImportStateIdFunc: importStateIDFunc(resourceID),
ImportState: true,
ImportStateVerify: true,
ResourceName: resourceID,
ImportStateIdFunc: importStateIDFunc(resourceID),
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"delete_on_create_timeout"},
}
}
func TestAccSearchDeployment_basic(t *testing.T) {
Expand All @@ -48,9 +48,10 @@ func TestAccSearchDeployment_basic(t *testing.T) {
Config: updateStepNoWait,
Check: updateStep.Check,
},
// Changes: skip_wait_on_update true -> null
Copy link
Collaborator Author

@EspenAlbert EspenAlbert Oct 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This summarizes the change in behavior well.
The old way allowed updating to null while the new way keeps the state value if removed (UseStateForUnknown behavior).

This is why we need the ImportStateVerifyIgnore: []string{"delete_on_create_timeout"} in import as it will not be present in the state after import.
Because once the value has been used in create it remains unchanged.

Note: A fresh import and setting delete_on_create_timeout in the config will raise an update error saying it cannot be updated.

// skip_wait_on_update = false again, and wait for stable state_name
updateStep,
importStep(updateStep.Config),

importStep(),
},
})
}
Expand Down Expand Up @@ -109,17 +110,17 @@ func TestAccSearchDeployment_timeoutTest(t *testing.T) {
),
},
{
Config: configWithTimeout(timeoutsStrLongFalse),
Check: resource.TestCheckResourceAttr(resourceID, "delete_on_create_timeout", "false"),
Config: configWithTimeout(timeoutsStrLongFalse),
ExpectError: regexp.MustCompile("delete_on_create_timeout cannot be updated or set after import.*"),
},
{
Config: configWithTimeout(""),
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckNoResourceAttr(resourceID, "delete_on_create_timeout"),
resource.TestCheckResourceAttrSet(resourceID, "delete_on_create_timeout"), // Will keep value from state
resource.TestCheckNoResourceAttr(resourceID, "timeouts.create"),
),
},
importStep(configWithTimeout("")),
importStep(),
},
})
}
Expand Down
2 changes: 1 addition & 1 deletion internal/service/streamprocessor/resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func (r *streamProcessorRS) Create(ctx context.Context, req resource.CreateReque
}

streamProcessorResp, err := WaitStateTransitionWithTimeout(ctx, streamProcessorParams, connV2.StreamsApi, []string{InitiatingState, CreatingState}, []string{CreatedState}, createTimeout)
err = cleanup.HandleCreateTimeout(cleanup.ResolveDeleteOnCreateTimeout(plan.DeleteOnCreateTimeout), err, func(ctxCleanup context.Context) error {
err = cleanup.HandleCreateTimeout(plan.DeleteOnCreateTimeout.ValueBool(), err, func(ctxCleanup context.Context) error {
_, err := connV2.StreamsApi.DeleteStreamProcessor(ctxCleanup, projectID, workspaceOrInstanceName, processorName).Execute()
if err != nil {
return err
Expand Down
3 changes: 2 additions & 1 deletion internal/service/streamprocessor/resource_schema.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,10 @@ func ResourceSchema(ctx context.Context) schema.Schema {
Create: true,
}),
"delete_on_create_timeout": schema.BoolAttribute{
Computed: true,
Optional: true,
PlanModifiers: []planmodifier.Bool{
customplanmodifier.CreateOnly(),
customplanmodifier.CreateOnlyBoolWithDefault(true),
},
MarkdownDescription: "Indicates whether to delete the resource being created if a timeout is reached when waiting for completion. When set to `true` and timeout occurs, it triggers the deletion and returns immediately without waiting for deletion to complete. When set to `false`, the timeout will not trigger resource deletion. If you suspect a transient error when the value is `true`, wait before retrying to allow resource deletion to finish. Default is `true`.",
},
Expand Down
26 changes: 12 additions & 14 deletions internal/service/streamprocessor/resource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,16 @@ var (
testLogDestConfig = connectionConfig{connectionType: connTypeTestLog, pipelineStepIsSource: false}
)

func importStep() resource.TestStep {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be a var as well. Same as in resource_database_user_test.go

Copy link
Member

@lantoli lantoli Oct 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i normally prefer functions to global vars, which is safer in case the var is modified later and might affect other tests. (some exceptions apply, e.g. for an array of strings, if we're sure they're not modified)

return resource.TestStep{
ResourceName: resourceName,
ImportStateIdFunc: importStateIDFunc(resourceName),
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"delete_on_create_timeout", "stats"},
}
}

func TestAccStreamProcessor_basic(t *testing.T) {
resource.ParallelTest(t, *basicTestCase(t))
}
Expand Down Expand Up @@ -66,13 +76,7 @@ func basicTestCase(t *testing.T) *resource.TestCase {
Check: composeStreamProcessorChecks(projectID, workspaceName, processorName, streamprocessor.StartedState, true, false),
ConfigStateChecks: pluralConfigStateChecks(processorName, streamprocessor.StartedState, workspaceName, true, false),
},
{
ResourceName: resourceName,
ImportStateIdFunc: importStateIDFunc(resourceName),
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"stats"},
},
importStep(),
}}
}

Expand Down Expand Up @@ -150,13 +154,7 @@ func TestAccStreamProcessor_withOptions(t *testing.T) {
Check: composeStreamProcessorChecks(projectID, workspaceName, processorName, streamprocessor.CreatedState, false, true),
ConfigStateChecks: pluralConfigStateChecks(processorName, streamprocessor.CreatedState, workspaceName, false, true),
},
{
ResourceName: resourceName,
ImportStateIdFunc: importStateIDFunc(resourceName),
ImportState: true,
ImportStateVerify: true,
ImportStateVerifyIgnore: []string{"stats"},
},
importStep(),
}})
}

Expand Down