Skip to content

feat: Long-running operation improvements for mongodbatlas_encryption_at_rest_private_endpoint resource #3561

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.

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 22 commits into from
Aug 12, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions .changelog/3561.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
```release-note:enhancement
resource/mongodbatlas_encryption_at_rest_private_endpoint: Adds `timeouts` attribute for create and delete operations
```

```release-note:enhancement
resource/mongodbatlas_encryption_at_rest_private_endpoint: Adds `delete_on_create_timeout` attribute to indicate whether to delete the resource if its creation times out
```
1 change: 1 addition & 0 deletions docs/guides/2.0.0-upgrade-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ The Terraform MongoDB Atlas Provider version 2.0.0 has the following new feature
- `mongodbatlas_advanced_cluster`
- `mongodbatlas_cloud_backup_snapshot`
- `mongodbatlas_cluster_outage_simulation`
- `mongodbatlas_encryption_at_rest_private_endpoint`
- `mongodbatlas_flex_cluster`
- `mongodbatlas_network_peering`
- `mongodbatlas_online_archive`
Expand Down
13 changes: 13 additions & 0 deletions docs/resources/encryption_at_rest_private_endpoint.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,26 @@ resource "mongodbatlas_encryption_at_rest_private_endpoint" "endpoint" {
- `project_id` (String) Unique 24-hexadecimal digit string that identifies your project.
- `region_name` (String) Cloud provider region in which the Encryption At Rest private endpoint is located.

### 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`.
- `timeouts` (Attributes) (see [below for nested schema](#nestedatt--timeouts))

### Read-Only

- `error_message` (String) Error message for failures associated with the Encryption At Rest private endpoint.
- `id` (String) Unique 24-hexadecimal digit string that identifies the Private Endpoint Service.
- `private_endpoint_connection_name` (String) Connection name of the Azure Private Endpoint.
- `status` (String) State of the Encryption At Rest private endpoint.

<a id="nestedatt--timeouts"></a>
### Nested Schema for `timeouts`
Copy link
Collaborator

Choose a reason for hiding this comment

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

@oarbusi I think we should leave a short why the update is not there, thoughts?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The update does not have timeout because the update operation is not supported in this resource. Considering that there's a note in the resource doc stating that update is not supported, I think it's not necessary to mention why there's no timeout for the update


Optional:

- `create` (String) A string that can be [parsed as a duration](https://pkg.go.dev/time#ParseDuration) consisting of numbers and unit suffixes, such as "30s" or "2h45m". Valid time units are "s" (seconds), "m" (minutes), "h" (hours).
- `delete` (String) A string that can be [parsed as a duration](https://pkg.go.dev/time#ParseDuration) consisting of numbers and unit suffixes, such as "30s" or "2h45m". Valid time units are "s" (seconds), "m" (minutes), "h" (hours). Setting a timeout for a Delete operation is only applicable if changes are saved into state before the destroy operation occurs.

## Import
Encryption At Rest Private Endpoint resource can be imported using the project ID, cloud provider, and private endpoint ID. The format must be `{project_id}-{cloud_provider}-{private_endpoint_id}` e.g.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func (d *encryptionAtRestPrivateEndpointDS) Schema(ctx context.Context, req data
}

func (d *encryptionAtRestPrivateEndpointDS) Read(ctx context.Context, req datasource.ReadRequest, resp *datasource.ReadResponse) {
var earPrivateEndpointConfig TFEarPrivateEndpointModel
var earPrivateEndpointConfig TFEarPrivateEndpointModelDS
resp.Diagnostics.Append(req.Config.Get(ctx, &earPrivateEndpointConfig)...)
if resp.Diagnostics.HasError() {
return
Expand All @@ -48,5 +48,5 @@ func (d *encryptionAtRestPrivateEndpointDS) Read(ctx context.Context, req dataso
return
}

resp.Diagnostics.Append(resp.State.Set(ctx, NewTFEarPrivateEndpoint(*endpointModel, projectID))...)
resp.Diagnostics.Append(resp.State.Set(ctx, NewTFEarPrivateEndpointDS(*endpointModel, projectID))...)
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,19 @@ func DSAttributes(withArguments bool) map[string]schema.Attribute {
}
}

// TFEarPrivateEndpointModelDS represents the model for data sources (without timeout fields)
type TFEarPrivateEndpointModelDS struct {
CloudProvider types.String `tfsdk:"cloud_provider"`
ErrorMessage types.String `tfsdk:"error_message"`
ProjectID types.String `tfsdk:"project_id"`
ID types.String `tfsdk:"id"`
PrivateEndpointConnectionName types.String `tfsdk:"private_endpoint_connection_name"`
RegionName types.String `tfsdk:"region_name"`
Status types.String `tfsdk:"status"`
}

type TFEncryptionAtRestPrivateEndpointsDSModel struct {
CloudProvider types.String `tfsdk:"cloud_provider"`
ProjectID types.String `tfsdk:"project_id"`
Results []TFEarPrivateEndpointModel `tfsdk:"results"`
CloudProvider types.String `tfsdk:"cloud_provider"`
ProjectID types.String `tfsdk:"project_id"`
Results []TFEarPrivateEndpointModelDS `tfsdk:"results"`
}
17 changes: 15 additions & 2 deletions internal/service/encryptionatrestprivateendpoint/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ func NewEarPrivateEndpointReq(tfPlan *TFEarPrivateEndpointModel) *admin.EARPriva
}

func NewTFEarPrivateEndpoints(projectID, cloudProvider string, sdkResults []admin.EARPrivateEndpoint) *TFEncryptionAtRestPrivateEndpointsDSModel {
results := make([]TFEarPrivateEndpointModel, len(sdkResults))
results := make([]TFEarPrivateEndpointModelDS, len(sdkResults))
for i := range sdkResults {
result := NewTFEarPrivateEndpoint(sdkResults[i], projectID)
result := NewTFEarPrivateEndpointDS(sdkResults[i], projectID)
results[i] = result
}
return &TFEncryptionAtRestPrivateEndpointsDSModel{
Expand All @@ -42,3 +42,16 @@ func NewTFEarPrivateEndpoints(projectID, cloudProvider string, sdkResults []admi
Results: results,
}
}

// NewTFEarPrivateEndpointDS creates a new data source model without timeout fields
func NewTFEarPrivateEndpointDS(apiResp admin.EARPrivateEndpoint, projectID string) TFEarPrivateEndpointModelDS {
return TFEarPrivateEndpointModelDS{
ProjectID: types.StringValue(projectID),
CloudProvider: conversion.StringNullIfEmpty(apiResp.GetCloudProvider()),
ErrorMessage: conversion.StringNullIfEmpty(apiResp.GetErrorMessage()),
ID: conversion.StringNullIfEmpty(apiResp.GetId()),
RegionName: conversion.StringNullIfEmpty(apiResp.GetRegionName()),
Status: conversion.StringNullIfEmpty(apiResp.GetStatus()),
PrivateEndpointConnectionName: conversion.StringNullIfEmpty(apiResp.GetPrivateEndpointConnectionName()),
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ func TestEncryptionAtRestPrivateEndpointPluralDSSDKToTFModel(t *testing.T) {
expectedTFModel: &encryptionatrestprivateendpoint.TFEncryptionAtRestPrivateEndpointsDSModel{
CloudProvider: types.StringValue(testCloudProvider),
ProjectID: types.StringValue(testProjectID),
Results: []encryptionatrestprivateendpoint.TFEarPrivateEndpointModel{
Results: []encryptionatrestprivateendpoint.TFEarPrivateEndpointModelDS{
{
CloudProvider: types.StringValue(testCloudProvider),
ErrorMessage: types.StringNull(),
Expand Down
30 changes: 27 additions & 3 deletions internal/service/encryptionatrestprivateendpoint/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -63,13 +64,28 @@ func (r *encryptionAtRestPrivateEndpointRS) Create(ctx context.Context, req reso
return
}

finalResp, err := waitStateTransition(ctx, projectID, cloudProvider, createResp.GetId(), connV2.EncryptionAtRestUsingCustomerKeyManagementApi)
createTimeout := cleanup.ResolveTimeout(ctx, &earPrivateEndpointPlan.Timeouts, cleanup.OperationCreate, &resp.Diagnostics)
if resp.Diagnostics.HasError() {
return
}

finalResp, err := waitStateTransition(ctx, projectID, cloudProvider, createResp.GetId(), connV2.EncryptionAtRestUsingCustomerKeyManagementApi, createTimeout)
err = cleanup.HandleCreateTimeout(cleanup.ResolveDeleteOnCreateTimeout(earPrivateEndpointPlan.DeleteOnCreateTimeout), err, func(ctxCleanup context.Context) error {
cleanResp, cleanErr := connV2.EncryptionAtRestUsingCustomerKeyManagementApi.RequestEncryptionAtRestPrivateEndpointDeletion(ctxCleanup, projectID, cloudProvider, createResp.GetId()).Execute()
if validate.StatusNotFound(cleanResp) {
return nil
}
Comment on lines +75 to +77
Copy link
Member

Choose a reason for hiding this comment

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

q: I see in some resources (e.g. networkpeering) the sanity check of seeing if delete return 404 is not being made. For consistency, would it make sense to move this check into cleanup.HandleCreateTimeout, and have cleanup only call the DELETE operation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

IMO HandleCreateTimeout should only handle the delete on create logic, and avoid doing the 404 case, to keep the method focused and clear

Copy link
Member

Choose a reason for hiding this comment

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

In that case would consider that privatelinkendpointservice, privatelinkendpoint, onlinearchive, networkpeering, and cloudbackupsnapshot do not have this check in their cleanup function. Not certain if we want to keep this as a general practice.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think this is unrelated to long-running operations, but we could create a CLOUDP to investigate and unify behavior on 404 across the resources you mentioned?

Copy link
Member

@lantoli lantoli Aug 12, 2025

Choose a reason for hiding this comment

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

i suppose we could keep the code simple here, e.g.:

err = cleanup.HandleCreateTimeout(cleanup.ResolveDeleteOnCreateTimeout(earPrivateEndpointPlan.DeleteOnCreateTimeout), err, func(ctxCleanup context.Context) error {
		_, cleanErr := connV2.EncryptionAtRestUsingCustomerKeyManagementApi.RequestEncryptionAtRestPrivateEndpointDeletion(ctxCleanup, projectID, cloudProvider, createResp.GetId()).Execute()
      return cleanErr
	})

and in HandleCreateTimeout don't raise error if cleanup funcs return 404 error, if we expect this will happen with most of the resources

return cleanErr
})

if err != nil {
resp.Diagnostics.AddError("error when waiting for status transition in creation", err.Error())
return
}

privateEndpointModel := NewTFEarPrivateEndpoint(*finalResp, projectID)
privateEndpointModel.Timeouts = earPrivateEndpointPlan.Timeouts
privateEndpointModel.DeleteOnCreateTimeout = earPrivateEndpointPlan.DeleteOnCreateTimeout
resp.Diagnostics.Append(resp.State.Set(ctx, privateEndpointModel)...)

diags := CheckErrorMessageAndStatus(finalResp)
Expand Down Expand Up @@ -98,7 +114,10 @@ func (r *encryptionAtRestPrivateEndpointRS) Read(ctx context.Context, req resour
return
}

resp.Diagnostics.Append(resp.State.Set(ctx, NewTFEarPrivateEndpoint(*endpointModel, projectID))...)
privateEndpointModel := NewTFEarPrivateEndpoint(*endpointModel, projectID)
privateEndpointModel.Timeouts = earPrivateEndpointState.Timeouts
privateEndpointModel.DeleteOnCreateTimeout = earPrivateEndpointState.DeleteOnCreateTimeout
resp.Diagnostics.Append(resp.State.Set(ctx, privateEndpointModel)...)

diags := CheckErrorMessageAndStatus(endpointModel)
resp.Diagnostics.Append(diags...)
Expand All @@ -124,7 +143,12 @@ func (r *encryptionAtRestPrivateEndpointRS) Delete(ctx context.Context, req reso
return
}

model, err := WaitDeleteStateTransition(ctx, projectID, cloudProvider, endpointID, connV2.EncryptionAtRestUsingCustomerKeyManagementApi)
deleteTimeout := cleanup.ResolveTimeout(ctx, &earPrivateEndpointState.Timeouts, cleanup.OperationDelete, &resp.Diagnostics)
if resp.Diagnostics.HasError() {
return
}

model, err := WaitDeleteStateTransition(ctx, projectID, cloudProvider, endpointID, connV2.EncryptionAtRestUsingCustomerKeyManagementApi, deleteTimeout)
if err != nil {
resp.Diagnostics.AddError("error when waiting for status transition in delete", err.Error())
return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@ package encryptionatrestprivateendpoint
import (
"context"

"github.com/hashicorp/terraform-plugin-framework-timeouts/resource/timeouts"
"github.com/hashicorp/terraform-plugin-framework/resource/schema"
"github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier"
"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 @@ -38,16 +41,29 @@ func ResourceSchema(ctx context.Context) schema.Schema {
Computed: true,
MarkdownDescription: "State of the Encryption At Rest private endpoint.",
},
"timeouts": timeouts.Attributes(ctx, timeouts.Opts{
Create: 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 TFEarPrivateEndpointModel struct {
CloudProvider types.String `tfsdk:"cloud_provider"`
ErrorMessage types.String `tfsdk:"error_message"`
ProjectID types.String `tfsdk:"project_id"`
ID types.String `tfsdk:"id"`
PrivateEndpointConnectionName types.String `tfsdk:"private_endpoint_connection_name"`
RegionName types.String `tfsdk:"region_name"`
Status types.String `tfsdk:"status"`
CloudProvider types.String `tfsdk:"cloud_provider"`
ErrorMessage types.String `tfsdk:"error_message"`
ProjectID types.String `tfsdk:"project_id"`
ID types.String `tfsdk:"id"`
PrivateEndpointConnectionName types.String `tfsdk:"private_endpoint_connection_name"`
RegionName types.String `tfsdk:"region_name"`
Status types.String `tfsdk:"status"`
Timeouts timeouts.Value `tfsdk:"timeouts"`
DeleteOnCreateTimeout types.Bool `tfsdk:"delete_on_create_timeout"`
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"os"
"regexp"
"testing"
"time"

Expand Down Expand Up @@ -32,6 +33,34 @@ func TestAccEncryptionAtRestPrivateEndpoint_Azure_basic(t *testing.T) {
resource.Test(t, *basicTestCaseAzure(t))
}

func TestAccEncryptionAtRestPrivateEndpoint_createTimeoutWithDeleteOnCreate(t *testing.T) {
// This test is skipped because it creates a race condition with other tests:
// 1. This test creates an encryption at rest private endpoint with a 1s timeout, causing it to fail and trigger cleanup
// 2. The private endpoint deletion doesn't complete immediately
// 3. Other tests share the same project and attempt to disable encryption at rest during cleanup
// 4. MongoDB Atlas returns "CANNOT_DISABLE_ENCRYPTION_AT_REST_REQUIRE_PRIVATE_NETWORKING_WHILE_PRIVATE_ENDPOINTS_EXIST"
// because the private endpoint from this test is still being deleted
// This race condition occurs even when tests don't run in parallel due to the async nature of private endpoint deletion.
Copy link
Member

Choose a reason for hiding this comment

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

would it work if using a different project?

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, project needs some set up in order to make this work.

acc.SkipTestForCI(t)
var (
createTimeout = "1s"
deleteOnCreateTimeout = true
region = conversion.AWSRegionToMongoDBRegion(os.Getenv("AWS_REGION"))
// Create encryption at rest configuration outside of test configuration to avoid cleanup issues
projectID = acc.EncryptionAtRestExecution(t)
)
resource.Test(t, resource.TestCase{
PreCheck: func() { acc.PreCheckEncryptionAtRestEnvAWS(t) },
ProtoV6ProviderFactories: acc.TestAccProviderV6Factories,
Steps: []resource.TestStep{
{
Config: configEARPrivateEndpointWithTimeout(projectID, region, acc.TimeoutConfig(&createTimeout, nil, nil), &deleteOnCreateTimeout),
ExpectError: regexp.MustCompile("will run cleanup because delete_on_create_timeout is true"),
},
},
})
}

func basicTestCaseAzure(tb testing.TB) *resource.TestCase {
tb.Helper()
var (
Expand Down Expand Up @@ -316,19 +345,57 @@ func checkBasic(projectID, cloudProvider, region string, expectApproved bool) re
}

func configAWSBasic(projectID string, awsKms *admin.AWSKMSConfiguration, region string) string {
return configAWSBasicWithTimeout(projectID, awsKms, region, "", nil)
}

func configAWSBasicWithTimeout(projectID string, awsKms *admin.AWSKMSConfiguration, region, timeoutConfig string, deleteOnCreateTimeout *bool) string {
encryptionAtRestConfig := acc.ConfigAwsKms(projectID, awsKms, false, true, false)

deleteOnCreateTimeoutConfig := ""
if deleteOnCreateTimeout != nil {
deleteOnCreateTimeoutConfig = fmt.Sprintf(`
delete_on_create_timeout = %[1]t
`, *deleteOnCreateTimeout)
}

config := fmt.Sprintf(`
%[1]s
resource "mongodbatlas_encryption_at_rest_private_endpoint" "test" {
project_id = mongodbatlas_encryption_at_rest.test.project_id
cloud_provider = "AWS"
region_name = %[2]q
%[3]s
%[4]s
}
%[3]s
%[5]s
`, encryptionAtRestConfig, region, configDS())
`, encryptionAtRestConfig, region, deleteOnCreateTimeoutConfig, timeoutConfig, configDS())

return config
}

func configEARPrivateEndpointWithTimeout(projectID, region, timeoutConfig string, deleteOnCreateTimeout *bool) string {
deleteOnCreateTimeoutConfig := ""
if deleteOnCreateTimeout != nil {
deleteOnCreateTimeoutConfig = fmt.Sprintf(`
delete_on_create_timeout = %[1]t
`, *deleteOnCreateTimeout)
}

config := fmt.Sprintf(`
resource "mongodbatlas_encryption_at_rest_private_endpoint" "test" {
project_id = %[1]q
cloud_provider = "AWS"
region_name = %[2]q
%[3]s
%[4]s
}
%[5]s
`, projectID, region, deleteOnCreateTimeoutConfig, timeoutConfig, configDS())

return config
}
Expand Down
Loading
Loading