From 6909929a71989b0f98dc22e7bffd0f44907baf2c Mon Sep 17 00:00:00 2001 From: Espen Albert Date: Fri, 5 Sep 2025 16:21:41 +0100 Subject: [PATCH 1/4] refactor: Use new create-only plan modifier in all resources --- .../common/customplanmodifier/create_only.go | 50 ++++--------------- internal/service/flexcluster/resource.go | 3 +- .../service/flexcluster/resource_schema.go | 5 +- internal/service/flexcluster/resource_test.go | 9 ++-- .../project/resource_project_schema.go | 2 +- 5 files changed, 20 insertions(+), 49 deletions(-) diff --git a/internal/common/customplanmodifier/create_only.go b/internal/common/customplanmodifier/create_only.go index c7de0750ef..6bd259f87f 100644 --- a/internal/common/customplanmodifier/create_only.go +++ b/internal/common/customplanmodifier/create_only.go @@ -22,36 +22,6 @@ func CreateOnlyBoolPlanModifier() planmodifier.Bool { return &createOnlyAttributePlanModifier{} } -// Plan modifier that implements create-only behavior for multiple attribute types -type createOnlyAttributePlanModifier struct{} - -func (d *createOnlyAttributePlanModifier) Description(ctx context.Context) string { - return d.MarkdownDescription(ctx) -} - -func (d *createOnlyAttributePlanModifier) MarkdownDescription(ctx context.Context) string { - return "Ensures that update operations fail when attempting to modify a create-only attribute." -} - -func (d *createOnlyAttributePlanModifier) PlanModifyString(ctx context.Context, req planmodifier.StringRequest, resp *planmodifier.StringResponse) { - validateCreateOnly(req.PlanValue, req.StateValue, req.Path, &resp.Diagnostics) -} - -func (d *createOnlyAttributePlanModifier) PlanModifyBool(ctx context.Context, req planmodifier.BoolRequest, resp *planmodifier.BoolResponse) { - validateCreateOnly(req.PlanValue, req.StateValue, req.Path, &resp.Diagnostics) -} - -// validateCreateOnly checks if an attribute value has changed and adds an error if it has -func validateCreateOnly(planValue, stateValue attr.Value, attrPath path.Path, diagnostics *diag.Diagnostics, -) { - if !stateValue.IsNull() && !stateValue.Equal(planValue) { - diagnostics.AddError( - fmt.Sprintf("%s cannot be updated", attrPath), - fmt.Sprintf("%s cannot be updated", attrPath), - ) - } -} - type CreateOnlyModifier interface { planmodifier.String planmodifier.Bool @@ -66,23 +36,23 @@ func CreateOnlyAttributePlanModifier() CreateOnlyModifier { return &createOnlyAttributePlanModifier{} } -// CreateOnlyAttributePlanModifierWithBoolDefault sets a default value on create operation that will show in the plan. +// CreateOnlyBoolWithDefaultPlanModifier sets a default value on create operation that will show in the plan. // This avoids any custom logic in the resource "Create" handler. // On update the default has no impact and the UseStateForUnknown behavior is observed instead. // Always use Optional+Computed when using a default value. -func CreateOnlyAttributePlanModifierWithBoolDefault(b bool) CreateOnlyModifier { - return &createOnlyAttributePlanModifierWithBoolDefault{defaultBool: &b} +func CreateOnlyBoolWithDefaultPlanModifier(b bool) CreateOnlyModifier { + return &createOnlyAttributePlanModifier{defaultBool: &b} } -type createOnlyAttributePlanModifierWithBoolDefault struct { +type createOnlyAttributePlanModifier struct { defaultBool *bool } -func (d *createOnlyAttributePlanModifierWithBoolDefault) Description(ctx context.Context) string { +func (d *createOnlyAttributePlanModifier) Description(ctx context.Context) string { return d.MarkdownDescription(ctx) } -func (d *createOnlyAttributePlanModifierWithBoolDefault) MarkdownDescription(ctx context.Context) string { +func (d *createOnlyAttributePlanModifier) MarkdownDescription(ctx context.Context) string { return "Ensures the update operation fails when updating an attribute. If the read after import don't equal the configuration value it will also raise an error." } @@ -90,11 +60,11 @@ func isCreate(t *tfsdk.State) bool { return t.Raw.IsNull() } -func (d *createOnlyAttributePlanModifierWithBoolDefault) UseDefault() bool { +func (d *createOnlyAttributePlanModifier) UseDefault() bool { return d.defaultBool != nil } -func (d *createOnlyAttributePlanModifierWithBoolDefault) PlanModifyBool(ctx context.Context, req planmodifier.BoolRequest, resp *planmodifier.BoolResponse) { +func (d *createOnlyAttributePlanModifier) PlanModifyBool(ctx context.Context, req planmodifier.BoolRequest, resp *planmodifier.BoolResponse) { if isCreate(&req.State) { if !IsKnown(req.PlanValue) && d.UseDefault() { resp.PlanValue = types.BoolPointerValue(d.defaultBool) @@ -109,7 +79,7 @@ func (d *createOnlyAttributePlanModifierWithBoolDefault) PlanModifyBool(ctx cont } } -func (d *createOnlyAttributePlanModifierWithBoolDefault) PlanModifyString(ctx context.Context, req planmodifier.StringRequest, resp *planmodifier.StringResponse) { +func (d *createOnlyAttributePlanModifier) PlanModifyString(ctx context.Context, req planmodifier.StringRequest, resp *planmodifier.StringResponse) { if isCreate(&req.State) { return } @@ -128,7 +98,7 @@ func isUpdated(state, plan attr.Value) bool { return !state.Equal(plan) } -func (d *createOnlyAttributePlanModifierWithBoolDefault) addDiags(diags *diag.Diagnostics, attrPath path.Path, stateValue attr.Value) { +func (d *createOnlyAttributePlanModifier) addDiags(diags *diag.Diagnostics, attrPath path.Path, stateValue attr.Value) { message := fmt.Sprintf("%s cannot be updated or set after import, remove it from the configuration or use the state value (see below).", attrPath) detail := fmt.Sprintf("The current state value is %s", stateValue) diags.AddError(message, detail) diff --git a/internal/service/flexcluster/resource.go b/internal/service/flexcluster/resource.go index f81318e60b..c82e36bbfb 100644 --- a/internal/service/flexcluster/resource.go +++ b/internal/service/flexcluster/resource.go @@ -80,8 +80,7 @@ func (r *rs) Create(ctx context.Context, req resource.CreateRequest, resp *resou flexClusterResp, err := CreateFlexCluster(ctx, projectID, clusterName, flexClusterReq, connV2.FlexClustersApi, &createTimeout) // Handle timeout with cleanup logic - deleteOnCreateTimeout := cleanup.ResolveDeleteOnCreateTimeout(tfModel.DeleteOnCreateTimeout) - err = cleanup.HandleCreateTimeout(deleteOnCreateTimeout, err, func(ctxCleanup context.Context) error { + err = cleanup.HandleCreateTimeout(tfModel.DeleteOnCreateTimeout.ValueBool(), err, func(ctxCleanup context.Context) error { cleanResp, cleanErr := r.Client.AtlasV2.FlexClustersApi.DeleteFlexCluster(ctxCleanup, projectID, clusterName).Execute() if validate.StatusNotFound(cleanResp) { return nil diff --git a/internal/service/flexcluster/resource_schema.go b/internal/service/flexcluster/resource_schema.go index a382145ca2..5c094e5a79 100644 --- a/internal/service/flexcluster/resource_schema.go +++ b/internal/service/flexcluster/resource_schema.go @@ -59,7 +59,7 @@ func ResourceSchema(ctx context.Context) schema.Schema { "region_name": schema.StringAttribute{ Required: true, PlanModifiers: []planmodifier.String{ - customplanmodifier.CreateOnlyStringPlanModifier(), + customplanmodifier.CreateOnlyAttributePlanModifier(), }, MarkdownDescription: "Human-readable label that identifies the geographic location of your MongoDB flex cluster. The region you choose can affect network latency for clients accessing your databases. For a complete list of region names, see [AWS](https://docs.atlas.mongodb.com/reference/amazon-aws/#std-label-amazon-aws), [GCP](https://docs.atlas.mongodb.com/reference/google-gcp/), and [Azure](https://docs.atlas.mongodb.com/reference/microsoft-azure/).", }, @@ -148,8 +148,9 @@ func ResourceSchema(ctx context.Context) schema.Schema { }, "delete_on_create_timeout": schema.BoolAttribute{ Optional: true, + Computed: true, PlanModifiers: []planmodifier.Bool{ - customplanmodifier.CreateOnlyBoolPlanModifier(), + customplanmodifier.CreateOnlyBoolWithDefaultPlanModifier(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`.", }, diff --git a/internal/service/flexcluster/resource_test.go b/internal/service/flexcluster/resource_test.go index 2cef4ca1cc..b787ada201 100644 --- a/internal/service/flexcluster/resource_test.go +++ b/internal/service/flexcluster/resource_test.go @@ -104,10 +104,11 @@ func basicTestCase(t *testing.T) *resource.TestCase { Check: checksFlexCluster(projectID, clusterName, false, true), }, { - ResourceName: resourceName, - ImportStateIdFunc: acc.ImportStateIDFuncProjectIDClusterName(resourceName, "project_id", "name"), - ImportState: true, - ImportStateVerify: true, + ResourceName: resourceName, + ImportStateIdFunc: acc.ImportStateIDFuncProjectIDClusterName(resourceName, "project_id", "name"), + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"delete_on_create_timeout"}, }, }, } diff --git a/internal/service/project/resource_project_schema.go b/internal/service/project/resource_project_schema.go index c62f576731..9c13e29d6b 100644 --- a/internal/service/project/resource_project_schema.go +++ b/internal/service/project/resource_project_schema.go @@ -61,7 +61,7 @@ func ResourceSchema(ctx context.Context) schema.Schema { // Provider produced invalid plan: planned an invalid value for a non-computed attribute. Optional: true, Computed: true, - PlanModifiers: []planmodifier.Bool{customplanmodifier.CreateOnlyAttributePlanModifierWithBoolDefault(true)}, + PlanModifiers: []planmodifier.Bool{customplanmodifier.CreateOnlyBoolWithDefaultPlanModifier(true)}, }, "is_collect_database_specifics_statistics_enabled": schema.BoolAttribute{ Computed: true, From 6e7ffbbd81bf478e51a779c75d98d416ba4cb5ea Mon Sep 17 00:00:00 2001 From: Espen Albert Date: Mon, 8 Sep 2025 08:35:34 +0100 Subject: [PATCH 2/4] refactor: Replace CreateOnlyAttributePlanModifier with CreateOnlyStringPlanModifier and add CreateOnlyBoolPlanModifier --- .../{create_only.go => create_only_bool.go} | 44 +++++++---------- .../customplanmodifier/create_only_string.go | 49 +++++++++++++++++++ .../service/flexcluster/resource_schema.go | 2 +- 3 files changed, 67 insertions(+), 28 deletions(-) rename internal/common/customplanmodifier/{create_only.go => create_only_bool.go} (62%) create mode 100644 internal/common/customplanmodifier/create_only_string.go diff --git a/internal/common/customplanmodifier/create_only.go b/internal/common/customplanmodifier/create_only_bool.go similarity index 62% rename from internal/common/customplanmodifier/create_only.go rename to internal/common/customplanmodifier/create_only_bool.go index 6bd259f87f..63b9700d10 100644 --- a/internal/common/customplanmodifier/create_only.go +++ b/internal/common/customplanmodifier/create_only_bool.go @@ -12,59 +12,45 @@ import ( "github.com/hashicorp/terraform-plugin-framework/types" ) -// CreateOnlyStringPlanModifier creates a plan modifier that prevents updates to string attributes. -func CreateOnlyStringPlanModifier() planmodifier.String { - return &createOnlyAttributePlanModifier{} -} - // CreateOnlyBoolPlanModifier creates a plan modifier that prevents updates to boolean attributes. -func CreateOnlyBoolPlanModifier() planmodifier.Bool { - return &createOnlyAttributePlanModifier{} -} - -type CreateOnlyModifier interface { - planmodifier.String - planmodifier.Bool -} - -// CreateOnlyAttributePlanModifier returns a plan modifier that ensures that update operations fails when the attribute is changed. // This is useful for attributes only supported in create and not in update. // It shows a helpful error message helping the user to update their config to match the state. -// Never use a schema.Default for create only attributes, instead use WithXXXDefault, the default will lead to plan changes that are not expected after import. +// Never use a schema.Default for create only attributes, instead use `WithDefault`, the default will lead to plan changes that are not expected after import. // Implement CopyFromPlan if the attribute is not in the API Response. -func CreateOnlyAttributePlanModifier() CreateOnlyModifier { - return &createOnlyAttributePlanModifier{} +func CreateOnlyBoolPlanModifier() planmodifier.Bool { + return &createOnlyBoolPlanModifier{} } // CreateOnlyBoolWithDefaultPlanModifier sets a default value on create operation that will show in the plan. // This avoids any custom logic in the resource "Create" handler. // On update the default has no impact and the UseStateForUnknown behavior is observed instead. // Always use Optional+Computed when using a default value. -func CreateOnlyBoolWithDefaultPlanModifier(b bool) CreateOnlyModifier { - return &createOnlyAttributePlanModifier{defaultBool: &b} +func CreateOnlyBoolWithDefaultPlanModifier(b bool) planmodifier.Bool { + return &createOnlyBoolPlanModifier{defaultBool: &b} } -type createOnlyAttributePlanModifier struct { +type createOnlyBoolPlanModifier struct { defaultBool *bool } -func (d *createOnlyAttributePlanModifier) Description(ctx context.Context) string { +func (d *createOnlyBoolPlanModifier) Description(ctx context.Context) string { return d.MarkdownDescription(ctx) } -func (d *createOnlyAttributePlanModifier) MarkdownDescription(ctx context.Context) string { +func (d *createOnlyBoolPlanModifier) MarkdownDescription(ctx context.Context) string { return "Ensures the update operation fails when updating an attribute. If the read after import don't equal the configuration value it will also raise an error." } +// isCreate uses the full state to check if this is a create operation func isCreate(t *tfsdk.State) bool { return t.Raw.IsNull() } -func (d *createOnlyAttributePlanModifier) UseDefault() bool { +func (d *createOnlyBoolPlanModifier) UseDefault() bool { return d.defaultBool != nil } -func (d *createOnlyAttributePlanModifier) PlanModifyBool(ctx context.Context, req planmodifier.BoolRequest, resp *planmodifier.BoolResponse) { +func (d *createOnlyBoolPlanModifier) PlanModifyBool(ctx context.Context, req planmodifier.BoolRequest, resp *planmodifier.BoolResponse) { if isCreate(&req.State) { if !IsKnown(req.PlanValue) && d.UseDefault() { resp.PlanValue = types.BoolPointerValue(d.defaultBool) @@ -79,7 +65,7 @@ func (d *createOnlyAttributePlanModifier) PlanModifyBool(ctx context.Context, re } } -func (d *createOnlyAttributePlanModifier) PlanModifyString(ctx context.Context, req planmodifier.StringRequest, resp *planmodifier.StringResponse) { +func (d *createOnlyBoolPlanModifier) PlanModifyString(ctx context.Context, req planmodifier.StringRequest, resp *planmodifier.StringResponse) { if isCreate(&req.State) { return } @@ -91,6 +77,10 @@ func (d *createOnlyAttributePlanModifier) PlanModifyString(ctx context.Context, } } +// isUpdated checks if the attribute was updated. +// Special case when the attribute is removed/set to null in the plan: +// Computed Attribute: returns false (unknown in the plan) +// Optional Attribute: returns true if the state has a value func isUpdated(state, plan attr.Value) bool { if !IsKnown(plan) { return false @@ -98,7 +88,7 @@ func isUpdated(state, plan attr.Value) bool { return !state.Equal(plan) } -func (d *createOnlyAttributePlanModifier) addDiags(diags *diag.Diagnostics, attrPath path.Path, stateValue attr.Value) { +func (d *createOnlyBoolPlanModifier) addDiags(diags *diag.Diagnostics, attrPath path.Path, stateValue attr.Value) { message := fmt.Sprintf("%s cannot be updated or set after import, remove it from the configuration or use the state value (see below).", attrPath) detail := fmt.Sprintf("The current state value is %s", stateValue) diags.AddError(message, detail) diff --git a/internal/common/customplanmodifier/create_only_string.go b/internal/common/customplanmodifier/create_only_string.go new file mode 100644 index 0000000000..3a56ef5ad9 --- /dev/null +++ b/internal/common/customplanmodifier/create_only_string.go @@ -0,0 +1,49 @@ +package customplanmodifier + +import ( + "context" + "fmt" + + "github.com/hashicorp/terraform-plugin-framework/attr" + "github.com/hashicorp/terraform-plugin-framework/diag" + "github.com/hashicorp/terraform-plugin-framework/path" + planmodifier "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" +) + +// CreateOnlyStringPlanModifier creates a plan modifier that prevents updates to string attributes. +// This is useful for attributes only supported in create and not in update. +// It shows a helpful error message helping the user to update their config to match the state. +// Never use a schema.Default for create only attributes, instead use `WithDefault`, the default will lead to plan changes that are not expected after import. +// No default value implemented for string until we have a use case. +// Implement CopyFromPlan if the attribute is not in the API Response. +func CreateOnlyStringPlanModifier() planmodifier.String { + return &createOnlyStringPlanModifier{} +} + +type createOnlyStringPlanModifier struct{} + +func (d *createOnlyStringPlanModifier) Description(ctx context.Context) string { + return d.MarkdownDescription(ctx) +} + +func (d *createOnlyStringPlanModifier) MarkdownDescription(ctx context.Context) string { + return "Ensures the update operation fails when updating an attribute. If the read after import don't equal the configuration value it will also raise an error." +} + +func (d *createOnlyStringPlanModifier) PlanModifyString(ctx context.Context, req planmodifier.StringRequest, resp *planmodifier.StringResponse) { + if isCreate(&req.State) { + return + } + if isUpdated(req.StateValue, req.PlanValue) { + d.addDiags(&resp.Diagnostics, req.Path, req.StateValue) + } + if !IsKnown(req.PlanValue) { + resp.PlanValue = req.StateValue + } +} + +func (d *createOnlyStringPlanModifier) addDiags(diags *diag.Diagnostics, attrPath path.Path, stateValue attr.Value) { + message := fmt.Sprintf("%s cannot be updated or set after import, remove it from the configuration or use the state value (see below).", attrPath) + detail := fmt.Sprintf("The current state value is %s", stateValue) + diags.AddError(message, detail) +} diff --git a/internal/service/flexcluster/resource_schema.go b/internal/service/flexcluster/resource_schema.go index 5c094e5a79..60eb4a29ff 100644 --- a/internal/service/flexcluster/resource_schema.go +++ b/internal/service/flexcluster/resource_schema.go @@ -59,7 +59,7 @@ func ResourceSchema(ctx context.Context) schema.Schema { "region_name": schema.StringAttribute{ Required: true, PlanModifiers: []planmodifier.String{ - customplanmodifier.CreateOnlyAttributePlanModifier(), + customplanmodifier.CreateOnlyStringPlanModifier(), }, MarkdownDescription: "Human-readable label that identifies the geographic location of your MongoDB flex cluster. The region you choose can affect network latency for clients accessing your databases. For a complete list of region names, see [AWS](https://docs.atlas.mongodb.com/reference/amazon-aws/#std-label-amazon-aws), [GCP](https://docs.atlas.mongodb.com/reference/google-gcp/), and [Azure](https://docs.atlas.mongodb.com/reference/microsoft-azure/).", }, From a0ad4a7ebafe84e11726015f10a606b5932fb41c Mon Sep 17 00:00:00 2001 From: Espen Albert Date: Mon, 8 Sep 2025 08:36:34 +0100 Subject: [PATCH 3/4] refactor: Shorten names of public plan modifier --- internal/common/customplanmodifier/create_only_bool.go | 8 ++++---- .../common/customplanmodifier/create_only_string.go | 4 ++-- .../encryptionatrestprivateendpoint/resource_schema.go | 2 +- internal/service/flexcluster/resource_schema.go | 10 +++++----- internal/service/project/resource_project_schema.go | 4 ++-- internal/service/pushbasedlogexport/resource_schema.go | 2 +- internal/service/streamprocessor/resource_schema.go | 2 +- 7 files changed, 16 insertions(+), 16 deletions(-) diff --git a/internal/common/customplanmodifier/create_only_bool.go b/internal/common/customplanmodifier/create_only_bool.go index 63b9700d10..54ab12c296 100644 --- a/internal/common/customplanmodifier/create_only_bool.go +++ b/internal/common/customplanmodifier/create_only_bool.go @@ -12,20 +12,20 @@ import ( "github.com/hashicorp/terraform-plugin-framework/types" ) -// CreateOnlyBoolPlanModifier creates a plan modifier that prevents updates to boolean attributes. +// CreateOnlyBool creates a plan modifier that prevents updates to boolean attributes. // This is useful for attributes only supported in create and not in update. // It shows a helpful error message helping the user to update their config to match the state. // Never use a schema.Default for create only attributes, instead use `WithDefault`, the default will lead to plan changes that are not expected after import. // Implement CopyFromPlan if the attribute is not in the API Response. -func CreateOnlyBoolPlanModifier() planmodifier.Bool { +func CreateOnlyBool() planmodifier.Bool { return &createOnlyBoolPlanModifier{} } -// CreateOnlyBoolWithDefaultPlanModifier sets a default value on create operation that will show in the plan. +// CreateOnlyBoolWithDefault sets a default value on create operation that will show in the plan. // This avoids any custom logic in the resource "Create" handler. // On update the default has no impact and the UseStateForUnknown behavior is observed instead. // Always use Optional+Computed when using a default value. -func CreateOnlyBoolWithDefaultPlanModifier(b bool) planmodifier.Bool { +func CreateOnlyBoolWithDefault(b bool) planmodifier.Bool { return &createOnlyBoolPlanModifier{defaultBool: &b} } diff --git a/internal/common/customplanmodifier/create_only_string.go b/internal/common/customplanmodifier/create_only_string.go index 3a56ef5ad9..9bc0bb568f 100644 --- a/internal/common/customplanmodifier/create_only_string.go +++ b/internal/common/customplanmodifier/create_only_string.go @@ -10,13 +10,13 @@ import ( planmodifier "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" ) -// CreateOnlyStringPlanModifier creates a plan modifier that prevents updates to string attributes. +// CreateOnlyString creates a plan modifier that prevents updates to string attributes. // This is useful for attributes only supported in create and not in update. // It shows a helpful error message helping the user to update their config to match the state. // Never use a schema.Default for create only attributes, instead use `WithDefault`, the default will lead to plan changes that are not expected after import. // No default value implemented for string until we have a use case. // Implement CopyFromPlan if the attribute is not in the API Response. -func CreateOnlyStringPlanModifier() planmodifier.String { +func CreateOnlyString() planmodifier.String { return &createOnlyStringPlanModifier{} } diff --git a/internal/service/encryptionatrestprivateendpoint/resource_schema.go b/internal/service/encryptionatrestprivateendpoint/resource_schema.go index e04b0ee1fa..e9b25e8d70 100644 --- a/internal/service/encryptionatrestprivateendpoint/resource_schema.go +++ b/internal/service/encryptionatrestprivateendpoint/resource_schema.go @@ -48,7 +48,7 @@ func ResourceSchema(ctx context.Context) schema.Schema { "delete_on_create_timeout": schema.BoolAttribute{ Optional: true, PlanModifiers: []planmodifier.Bool{ - customplanmodifier.CreateOnlyBoolPlanModifier(), + customplanmodifier.CreateOnlyBool(), }, 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`.", }, diff --git a/internal/service/flexcluster/resource_schema.go b/internal/service/flexcluster/resource_schema.go index 60eb4a29ff..c8d9c0d4cb 100644 --- a/internal/service/flexcluster/resource_schema.go +++ b/internal/service/flexcluster/resource_schema.go @@ -22,14 +22,14 @@ func ResourceSchema(ctx context.Context) schema.Schema { "project_id": schema.StringAttribute{ Required: true, PlanModifiers: []planmodifier.String{ - customplanmodifier.CreateOnlyStringPlanModifier(), + customplanmodifier.CreateOnlyString(), }, MarkdownDescription: "Unique 24-hexadecimal character string that identifies the project.", }, "name": schema.StringAttribute{ Required: true, PlanModifiers: []planmodifier.String{ - customplanmodifier.CreateOnlyStringPlanModifier(), + customplanmodifier.CreateOnlyString(), }, MarkdownDescription: "Human-readable label that identifies the instance.", }, @@ -38,7 +38,7 @@ func ResourceSchema(ctx context.Context) schema.Schema { "backing_provider_name": schema.StringAttribute{ Required: true, PlanModifiers: []planmodifier.String{ - customplanmodifier.CreateOnlyStringPlanModifier(), + customplanmodifier.CreateOnlyString(), }, MarkdownDescription: "Cloud service provider on which MongoDB Cloud provisioned the flex cluster.", }, @@ -59,7 +59,7 @@ func ResourceSchema(ctx context.Context) schema.Schema { "region_name": schema.StringAttribute{ Required: true, PlanModifiers: []planmodifier.String{ - customplanmodifier.CreateOnlyStringPlanModifier(), + customplanmodifier.CreateOnlyString(), }, MarkdownDescription: "Human-readable label that identifies the geographic location of your MongoDB flex cluster. The region you choose can affect network latency for clients accessing your databases. For a complete list of region names, see [AWS](https://docs.atlas.mongodb.com/reference/amazon-aws/#std-label-amazon-aws), [GCP](https://docs.atlas.mongodb.com/reference/google-gcp/), and [Azure](https://docs.atlas.mongodb.com/reference/microsoft-azure/).", }, @@ -150,7 +150,7 @@ func ResourceSchema(ctx context.Context) schema.Schema { Optional: true, Computed: true, PlanModifiers: []planmodifier.Bool{ - customplanmodifier.CreateOnlyBoolWithDefaultPlanModifier(true), + 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`.", }, diff --git a/internal/service/project/resource_project_schema.go b/internal/service/project/resource_project_schema.go index 9c13e29d6b..4f643dc729 100644 --- a/internal/service/project/resource_project_schema.go +++ b/internal/service/project/resource_project_schema.go @@ -53,7 +53,7 @@ func ResourceSchema(ctx context.Context) schema.Schema { "project_owner_id": schema.StringAttribute{ Optional: true, PlanModifiers: []planmodifier.String{ - customplanmodifier.CreateOnlyStringPlanModifier(), + customplanmodifier.CreateOnlyString(), }, }, "with_default_alerts_settings": schema.BoolAttribute{ @@ -61,7 +61,7 @@ func ResourceSchema(ctx context.Context) schema.Schema { // Provider produced invalid plan: planned an invalid value for a non-computed attribute. Optional: true, Computed: true, - PlanModifiers: []planmodifier.Bool{customplanmodifier.CreateOnlyBoolWithDefaultPlanModifier(true)}, + PlanModifiers: []planmodifier.Bool{customplanmodifier.CreateOnlyBoolWithDefault(true)}, }, "is_collect_database_specifics_statistics_enabled": schema.BoolAttribute{ Computed: true, diff --git a/internal/service/pushbasedlogexport/resource_schema.go b/internal/service/pushbasedlogexport/resource_schema.go index f809a80b7d..e8bc665bda 100644 --- a/internal/service/pushbasedlogexport/resource_schema.go +++ b/internal/service/pushbasedlogexport/resource_schema.go @@ -59,7 +59,7 @@ func ResourceSchema(ctx context.Context) schema.Schema { "delete_on_create_timeout": schema.BoolAttribute{ Optional: true, PlanModifiers: []planmodifier.Bool{ - customplanmodifier.CreateOnlyBoolPlanModifier(), + customplanmodifier.CreateOnlyBool(), }, 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`.", }, diff --git a/internal/service/streamprocessor/resource_schema.go b/internal/service/streamprocessor/resource_schema.go index a180be509a..de88830542 100644 --- a/internal/service/streamprocessor/resource_schema.go +++ b/internal/service/streamprocessor/resource_schema.go @@ -81,7 +81,7 @@ func ResourceSchema(ctx context.Context) schema.Schema { "delete_on_create_timeout": schema.BoolAttribute{ Optional: true, PlanModifiers: []planmodifier.Bool{ - customplanmodifier.CreateOnlyBoolPlanModifier(), + customplanmodifier.CreateOnlyBool(), }, 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`.", }, From 674b795498991b622d77e479f2f8c6417e04f843 Mon Sep 17 00:00:00 2001 From: Espen Albert Date: Mon, 8 Sep 2025 13:47:57 +0100 Subject: [PATCH 4/4] refactor: Address PR comments --- .../customplanmodifier/create_only_bool.go | 17 +++-------------- .../customplanmodifier/create_only_string.go | 2 +- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/internal/common/customplanmodifier/create_only_bool.go b/internal/common/customplanmodifier/create_only_bool.go index 54ab12c296..ad740aa6f5 100644 --- a/internal/common/customplanmodifier/create_only_bool.go +++ b/internal/common/customplanmodifier/create_only_bool.go @@ -16,7 +16,7 @@ import ( // This is useful for attributes only supported in create and not in update. // It shows a helpful error message helping the user to update their config to match the state. // Never use a schema.Default for create only attributes, instead use `WithDefault`, the default will lead to plan changes that are not expected after import. -// Implement CopyFromPlan if the attribute is not in the API Response. +// If the attribute is not in the API Response implement CopyFromPlan behavior when converting API Model to TF Model. func CreateOnlyBool() planmodifier.Bool { return &createOnlyBoolPlanModifier{} } @@ -25,6 +25,7 @@ func CreateOnlyBool() planmodifier.Bool { // This avoids any custom logic in the resource "Create" handler. // On update the default has no impact and the UseStateForUnknown behavior is observed instead. // Always use Optional+Computed when using a default value. +// If the attribute is not in the API Response implement CopyFromPlan behavior when converting API Model to TF Model. func CreateOnlyBoolWithDefault(b bool) planmodifier.Bool { return &createOnlyBoolPlanModifier{defaultBool: &b} } @@ -38,7 +39,7 @@ func (d *createOnlyBoolPlanModifier) Description(ctx context.Context) string { } func (d *createOnlyBoolPlanModifier) MarkdownDescription(ctx context.Context) string { - return "Ensures the update operation fails when updating an attribute. If the read after import don't equal the configuration value it will also raise an error." + return "Ensures the update operation fails when updating an attribute. If the read after import doesn't equal the configuration value it will also raise an error." } // isCreate uses the full state to check if this is a create operation @@ -65,18 +66,6 @@ func (d *createOnlyBoolPlanModifier) PlanModifyBool(ctx context.Context, req pla } } -func (d *createOnlyBoolPlanModifier) PlanModifyString(ctx context.Context, req planmodifier.StringRequest, resp *planmodifier.StringResponse) { - if isCreate(&req.State) { - return - } - if isUpdated(req.StateValue, req.PlanValue) { - d.addDiags(&resp.Diagnostics, req.Path, req.StateValue) - } - if !IsKnown(req.PlanValue) { - resp.PlanValue = req.StateValue - } -} - // isUpdated checks if the attribute was updated. // Special case when the attribute is removed/set to null in the plan: // Computed Attribute: returns false (unknown in the plan) diff --git a/internal/common/customplanmodifier/create_only_string.go b/internal/common/customplanmodifier/create_only_string.go index 9bc0bb568f..99c58f4f76 100644 --- a/internal/common/customplanmodifier/create_only_string.go +++ b/internal/common/customplanmodifier/create_only_string.go @@ -15,7 +15,7 @@ import ( // It shows a helpful error message helping the user to update their config to match the state. // Never use a schema.Default for create only attributes, instead use `WithDefault`, the default will lead to plan changes that are not expected after import. // No default value implemented for string until we have a use case. -// Implement CopyFromPlan if the attribute is not in the API Response. +// If the attribute is not in the API Response implement CopyFromPlan behavior when converting API Model to TF Model. func CreateOnlyString() planmodifier.String { return &createOnlyStringPlanModifier{} }