feat: Long-running operation improvements for mongodbatlas_push_based_log_export resource (#3570)

* implement delete_on_create_timeout

* aws provider in test

Author: oarbusi

.changelog/3570.txt

@@ -0,0 +1,3 @@
+```release-note:enhancement
+resource/mongodbatlas_push_based_log_export: Adds `delete_on_create_timeout` attribute to indicate whether to delete the resource if its creation times out
+```

docs/resources/push_based_log_export.md

@@ -59,6 +59,7 @@ output "test" {
 
 ### Optional
 
+- `delete_on_create_timeout` (Boolean) 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`.
 - `prefix_path` (String) S3 directory in which vector writes in order to store the logs. An empty string denotes the root directory.
 - `timeouts` (Attributes) (see [below for nested schema](#nestedatt--timeouts))
 

internal/common/cleanup/handle_timeout.go

@@ -8,6 +8,7 @@ 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"
 )
@@ -99,3 +100,14 @@ 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()
+}

internal/service/pushbasedlogexport/data_source.go

@@ -45,7 +45,7 @@ func (d *pushBasedLogExportDS) Read(ctx context.Context, req datasource.ReadRequ
 		return
 	}
 
-	newTFModel, diags := NewTFPushBasedLogExport(ctx, projectID, logConfig, nil)
+	newTFModel, diags := NewTFPushBasedLogExport(ctx, projectID, logConfig, nil, nil)
 	if diags.HasError() {
 		resp.Diagnostics.Append(diags...)
 		return

internal/service/pushbasedlogexport/model.go

@@ -12,7 +12,7 @@ import (
 	"github.com/mongodb/terraform-provider-mongodbatlas/internal/common/conversion"
 )
 
-func NewTFPushBasedLogExport(ctx context.Context, projectID string, apiResp *admin.PushBasedLogExportProject, timeout *timeouts.Value) (*TFPushBasedLogExportRSModel, diag.Diagnostics) {
+func NewTFPushBasedLogExport(ctx context.Context, projectID string, apiResp *admin.PushBasedLogExportProject, timeout *timeouts.Value, deleteOnCreateTimeout *types.Bool) (*TFPushBasedLogExportRSModel, diag.Diagnostics) {
 	tfModel := &TFPushBasedLogExportRSModel{
 		ProjectID:  types.StringPointerValue(&projectID),
 		BucketName: types.StringPointerValue(apiResp.BucketName),
@@ -25,6 +25,9 @@ func NewTFPushBasedLogExport(ctx context.Context, projectID string, apiResp *adm
 	if timeout != nil {
 		tfModel.Timeouts = *timeout
 	}
+	if deleteOnCreateTimeout != nil {
+		tfModel.DeleteOnCreateTimeout = *deleteOnCreateTimeout
+	}
 	return tfModel, nil
 }
 

internal/service/pushbasedlogexport/model_test.go

@@ -23,11 +23,12 @@ var (
 )
 
 type sdkToTFModelTestCase struct {
-	apiResp         *admin.PushBasedLogExportProject
-	timeout         *timeouts.Value
-	expectedTFModel *pushbasedlogexport.TFPushBasedLogExportRSModel
-	name            string
-	projectID       string
+	apiResp               *admin.PushBasedLogExportProject
+	timeout               *timeouts.Value
+	deleteOnCreateTimeout *types.Bool
+	expectedTFModel       *pushbasedlogexport.TFPushBasedLogExportRSModel
+	name                  string
+	projectID             string
 }
 
 func TestNewTFPushBasedLogExport(t *testing.T) {
@@ -76,7 +77,7 @@ func TestNewTFPushBasedLogExport(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			resultModel, _ := pushbasedlogexport.NewTFPushBasedLogExport(t.Context(), tc.projectID, tc.apiResp, tc.timeout)
+			resultModel, _ := pushbasedlogexport.NewTFPushBasedLogExport(t.Context(), tc.projectID, tc.apiResp, tc.timeout, tc.deleteOnCreateTimeout)
 			if !assert.Equal(t, tc.expectedTFModel, resultModel) {
 				t.Errorf("result model does not match expected output: expected %+v, got %+v", tc.expectedTFModel, resultModel)
 			}

internal/service/pushbasedlogexport/resource.go

@@ -11,6 +11,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/path"
 	"github.com/hashicorp/terraform-plugin-framework/resource"
 
+	"github.com/mongodb/terraform-provider-mongodbatlas/internal/common/cleanup"
 	"github.com/mongodb/terraform-provider-mongodbatlas/internal/common/conversion"
 	"github.com/mongodb/terraform-provider-mongodbatlas/internal/common/retrystrategy"
 	"github.com/mongodb/terraform-provider-mongodbatlas/internal/common/validate"
@@ -74,6 +75,15 @@ 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 {
+		cleanResp, cleanErr := connV2.PushBasedLogExportApi.DeletePushBasedLogConfiguration(ctx, projectID).Execute()
+		if validate.StatusNotFound(cleanResp) {
+			return nil
+		}
+		return cleanErr
+	})
+
 	if err != nil {
 		resp.Diagnostics.AddError("Error when creating push-based log export configuration", err.Error())
 
@@ -84,7 +94,7 @@ func (r *pushBasedLogExportRS) Create(ctx context.Context, req resource.CreateRe
 		return
 	}
 
-	newTFModel, diags := NewTFPushBasedLogExport(ctx, projectID, logExportConfigResp, &tfPlan.Timeouts)
+	newTFModel, diags := NewTFPushBasedLogExport(ctx, projectID, logExportConfigResp, &tfPlan.Timeouts, &tfPlan.DeleteOnCreateTimeout)
 	if diags.HasError() {
 		resp.Diagnostics.Append(diags...)
 		return
@@ -111,7 +121,7 @@ func (r *pushBasedLogExportRS) Read(ctx context.Context, req resource.ReadReques
 		return
 	}
 
-	newTFModel, diags := NewTFPushBasedLogExport(ctx, projectID, logConfig, &tfState.Timeouts)
+	newTFModel, diags := NewTFPushBasedLogExport(ctx, projectID, logConfig, &tfState.Timeouts, &tfState.DeleteOnCreateTimeout)
 	if diags.HasError() {
 		resp.Diagnostics.Append(diags...)
 		return
@@ -148,7 +158,7 @@ func (r *pushBasedLogExportRS) Update(ctx context.Context, req resource.UpdateRe
 		return
 	}
 
-	newTFModel, diags := NewTFPushBasedLogExport(ctx, projectID, logExportConfigResp, &tfPlan.Timeouts)
+	newTFModel, diags := NewTFPushBasedLogExport(ctx, projectID, logExportConfigResp, &tfPlan.Timeouts, &tfPlan.DeleteOnCreateTimeout)
 	if diags.HasError() {
 		resp.Diagnostics.Append(diags...)
 		return

internal/service/pushbasedlogexport/resource_schema.go

@@ -12,6 +12,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 {
@@ -55,16 +56,24 @@ func ResourceSchema(ctx context.Context) schema.Schema {
 				Update: true,
 				Delete: true,
 			}),
+			"delete_on_create_timeout": schema.BoolAttribute{
+				Optional: true,
+				PlanModifiers: []planmodifier.Bool{
+					customplanmodifier.CreateOnlyBoolPlanModifier(),
+				},
+				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`.",
+			},
 		},
 	}
 }
 
 type TFPushBasedLogExportRSModel struct {
-	BucketName types.String   `tfsdk:"bucket_name"`
-	CreateDate types.String   `tfsdk:"create_date"`
-	ProjectID  types.String   `tfsdk:"project_id"`
-	IamRoleID  types.String   `tfsdk:"iam_role_id"`
-	PrefixPath types.String   `tfsdk:"prefix_path"`
-	State      types.String   `tfsdk:"state"`
-	Timeouts   timeouts.Value `tfsdk:"timeouts"`
+	BucketName            types.String   `tfsdk:"bucket_name"`
+	CreateDate            types.String   `tfsdk:"create_date"`
+	ProjectID             types.String   `tfsdk:"project_id"`
+	IamRoleID             types.String   `tfsdk:"iam_role_id"`
+	PrefixPath            types.String   `tfsdk:"prefix_path"`
+	State                 types.String   `tfsdk:"state"`
+	Timeouts              timeouts.Value `tfsdk:"timeouts"`
+	DeleteOnCreateTimeout types.Bool     `tfsdk:"delete_on_create_timeout"`
 }

internal/service/pushbasedlogexport/resource_test.go

@@ -44,7 +44,7 @@ func basicTestCase(tb testing.TB) *resource.TestCase {
 		CheckDestroy:             checkDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: configBasic(projectID, s3BucketName1, s3BucketName2, s3BucketPolicyName, awsIAMRoleName, awsIAMRolePolicyName, nonEmptyPrefixPath, true),
+				Config: configBasic(projectID, s3BucketName1, s3BucketName2, s3BucketPolicyName, awsIAMRoleName, awsIAMRolePolicyName, nonEmptyPrefixPath, true, "", nil),
 				Check:  resource.ComposeAggregateTestCheckFunc(commonChecks(s3BucketName1, nonEmptyPrefixPath)...),
 			},
 			{
@@ -86,7 +86,7 @@ func noPrefixPathTestCase(tb testing.TB) *resource.TestCase {
 		CheckDestroy:             checkDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: configBasic(projectID, s3BucketName1, s3BucketName2, s3BucketPolicyName, awsIAMRoleName, awsIAMRolePolicyName, defaultPrefixPath, false),
+				Config: configBasic(projectID, s3BucketName1, s3BucketName2, s3BucketPolicyName, awsIAMRoleName, awsIAMRolePolicyName, defaultPrefixPath, false, "", nil),
 				Check:  resource.ComposeAggregateTestCheckFunc(commonChecks(s3BucketName1, defaultPrefixPath)...),
 			},
 		},
@@ -116,6 +116,39 @@ func createFailure(tb testing.TB) *resource.TestCase {
 	}
 }
 
+func TestAccPushBasedLogExport_createTimeoutWithDeleteOnCreateTimeout(t *testing.T) {
+	resource.Test(t, *createTimeoutWithDeleteOnCreateTimeout(t))
+}
+
+func createTimeoutWithDeleteOnCreateTimeout(tb testing.TB) *resource.TestCase {
+	tb.Helper()
+
+	var (
+		projectID             = acc.ProjectIDExecution(tb)
+		s3BucketNamePrefix    = acc.RandomS3BucketName()
+		s3BucketName1         = fmt.Sprintf("%s-1", s3BucketNamePrefix)
+		s3BucketName2         = fmt.Sprintf("%s-2", s3BucketNamePrefix)
+		s3BucketPolicyName    = fmt.Sprintf("%s-s3-policy", s3BucketNamePrefix)
+		awsIAMRoleName        = acc.RandomIAMRole()
+		awsIAMRolePolicyName  = fmt.Sprintf("%s-policy", awsIAMRoleName)
+		createTimeout         = "1s"
+		deleteOnCreateTimeout = true
+	)
+
+	return &resource.TestCase{
+		PreCheck:                 func() { acc.PreCheckBasic(tb) },
+		ExternalProviders:        acc.ExternalProvidersOnlyAWS(),
+		ProtoV6ProviderFactories: acc.TestAccProviderV6Factories,
+		CheckDestroy:             checkDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config:      configBasic(projectID, s3BucketName1, s3BucketName2, s3BucketPolicyName, awsIAMRoleName, awsIAMRolePolicyName, nonEmptyPrefixPath, true, acc.TimeoutConfig(&createTimeout, nil, nil, true), &deleteOnCreateTimeout),
+				ExpectError: regexp.MustCompile("will run cleanup because delete_on_create_timeout is true"),
+			},
+		},
+	}
+}
+
 func pushBasedLogExportInvalidConfig(projectID string) string {
 	return fmt.Sprintf(`resource "mongodbatlas_push_based_log_export" "test" {
 		project_id  = %[1]q
@@ -140,7 +173,7 @@ func addAttrChecks(checks []resource.TestCheckFunc, mapChecks map[string]string)
 	return acc.AddAttrChecks(datasourceName, checks, mapChecks)
 }
 
-func configBasic(projectID, s3BucketName1, s3BucketName2, s3BucketPolicyName, awsIAMRoleName, awsIAMRolePolicyName, prefixPath string, usePrefixPath bool) string {
+func configBasic(projectID, s3BucketName1, s3BucketName2, s3BucketPolicyName, awsIAMRoleName, awsIAMRolePolicyName, prefixPath string, usePrefixPath bool, timeoutConfig string, deleteOnCreateTimeout *bool) string {
 	test := fmt.Sprintf(`
 	 	locals {
 				project_id = %[1]q
@@ -155,7 +188,7 @@ func configBasic(projectID, s3BucketName1, s3BucketName2, s3BucketPolicyName, aw
 
 			   %[8]s		
 	`, projectID, s3BucketName1, s3BucketName2, s3BucketPolicyName, awsIAMRoleName, awsIAMRolePolicyName,
-		awsIAMroleAuthAndS3Config(s3BucketName1, s3BucketName2), pushBasedLogExportConfig(false, usePrefixPath, prefixPath))
+		awsIAMroleAuthAndS3Config(s3BucketName1, s3BucketName2), pushBasedLogExportConfig(false, usePrefixPath, prefixPath, timeoutConfig, deleteOnCreateTimeout))
 	return test
 }
 
@@ -174,13 +207,17 @@ func configBasicUpdated(projectID, s3BucketName1, s3BucketName2, s3BucketPolicyN
 
 			   %[8]s
 	`, projectID, s3BucketName1, s3BucketName2, s3BucketPolicyName, awsIAMRoleName, awsIAMRolePolicyName,
-		awsIAMroleAuthAndS3Config(s3BucketName1, s3BucketName2), pushBasedLogExportConfig(true, usePrefixPath, prefixPath)) // updating the S3 bucket to use for push-based log config
+		awsIAMroleAuthAndS3Config(s3BucketName1, s3BucketName2), pushBasedLogExportConfig(true, usePrefixPath, prefixPath, "", nil)) // updating the S3 bucket to use for push-based log config
 	return test
 }
 
 // pushBasedLogExportConfig returns config for mongodbatlas_push_based_log_export resource and data source.
 // This method uses the project and S3 bucket created in awsIAMroleAuthAndS3Config()
-func pushBasedLogExportConfig(useBucket2, usePrefixPath bool, prefixPath string) string {
+func pushBasedLogExportConfig(useBucket2, usePrefixPath bool, prefixPath, timeoutConfig string, deleteOnCreateTimeout *bool) string {
+	deleteOnCreateTimeoutAttr := ""
+	if deleteOnCreateTimeout != nil {
+		deleteOnCreateTimeoutAttr = fmt.Sprintf("delete_on_create_timeout = %[1]t", *deleteOnCreateTimeout)
+	}
 	bucketNameAttr := "bucket_name = aws_s3_bucket.log_bucket_1.bucket"
 	if useBucket2 {
 		bucketNameAttr = "bucket_name = aws_s3_bucket.log_bucket_2.bucket"
@@ -191,10 +228,12 @@ func pushBasedLogExportConfig(useBucket2, usePrefixPath bool, prefixPath string)
 			%[1]s
 			iam_role_id = mongodbatlas_cloud_provider_access_authorization.auth_role.role_id
 			prefix_path = %[2]q
+			%[4]s
+			%[5]s
 		}
 		
 		%[3]s
-		`, bucketNameAttr, prefixPath, pushBasedLogExportDataSourceConfig())
+		`, bucketNameAttr, prefixPath, pushBasedLogExportDataSourceConfig(), deleteOnCreateTimeoutAttr, timeoutConfig)
 	}
 
 	return fmt.Sprintf(`resource "mongodbatlas_push_based_log_export" "test" {