framework: Add corner tests for set defaulting bug in plugin framework (

#270) * add tests for 783 * add nested attribute tests * comments * comments * version checks
hashicorp · Sep 18, 2024 · 25b10e6 · 25b10e6
1 parent 8773819
commit 25b10e6
Show file tree

Hide file tree

Showing 8 changed files with 634 additions and 0 deletions.
diff --git a/internal/framework5provider/provider.go b/internal/framework5provider/provider.go
@@ -77,6 +77,7 @@ func (p *testProvider) Resources(_ context.Context) []func() resource.Resource {
 		NewFloat64PrecisionResource,
 		NewTFSDKReflectionResource,
 		NewMoveStateResource,
+		NewSetNestedBlockWithDefaultsResource,
 	}
 }
 

diff --git a/internal/framework5provider/set_nested_block_with_defaults.go b/internal/framework5provider/set_nested_block_with_defaults.go
@@ -0,0 +1,99 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+package framework
+
+import (
+	"context"
+
+	"github.com/hashicorp/terraform-plugin-framework/resource"
+	"github.com/hashicorp/terraform-plugin-framework/resource/schema"
+	"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringdefault"
+	"github.com/hashicorp/terraform-plugin-framework/types"
+)
+
+var _ resource.Resource = SetNestedBlockWithDefaultsResource{}
+
+func NewSetNestedBlockWithDefaultsResource() resource.Resource {
+	return &SetNestedBlockWithDefaultsResource{}
+}
+
+// SetNestedBlockWithDefaultsResource is used for a test asserting a bug that has yet to be fixed in plugin framework
+// with defaults being used in an attribute inside of a set.
+//
+// This bug can be observed with various different outcomes: producing duplicate set element errors, incorrect diffs during plan,
+// consistent diffs with values switching back and forth, etc. Example bug reports:
+//   - https://github.com/hashicorp/terraform-plugin-framework/issues/783
+//   - https://github.com/hashicorp/terraform-plugin-framework/issues/867
+//   - https://github.com/hashicorp/terraform-plugin-framework/issues/1036
+type SetNestedBlockWithDefaultsResource struct{}
+
+func (r SetNestedBlockWithDefaultsResource) Metadata(_ context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) {
+	resp.TypeName = req.ProviderTypeName + "_set_nested_block_with_defaults"
+}
+
+func (r SetNestedBlockWithDefaultsResource) Schema(_ context.Context, _ resource.SchemaRequest, resp *resource.SchemaResponse) {
+	resp.Schema = schema.Schema{
+		Blocks: map[string]schema.Block{
+			"set": schema.SetNestedBlock{
+				NestedObject: schema.NestedBlockObject{
+					Attributes: map[string]schema.Attribute{
+						"value": schema.StringAttribute{
+							Optional: true,
+							Computed: true,
+							Default:  stringdefault.StaticString("zero"),
+						},
+						"default_value": schema.StringAttribute{
+							Optional: true,
+							Computed: true,
+							Default:  stringdefault.StaticString("this is a default"),
+						},
+					},
+				},
+			},
+		},
+	}
+}
+
+func (r SetNestedBlockWithDefaultsResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) {
+	var data SetNestedBlockWithDefaultsResourceModel
+
+	resp.Diagnostics.Append(req.Plan.Get(ctx, &data)...)
+
+	if resp.Diagnostics.HasError() {
+		return
+	}
+
+	resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
+}
+
+func (r SetNestedBlockWithDefaultsResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) {
+	var data SetNestedBlockWithDefaultsResourceModel
+
+	resp.Diagnostics.Append(req.State.Get(ctx, &data)...)
+
+	if resp.Diagnostics.HasError() {
+		return
+	}
+
+	resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
+}
+
+func (r SetNestedBlockWithDefaultsResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) {
+	var data SetNestedBlockWithDefaultsResourceModel
+
+	resp.Diagnostics.Append(req.Plan.Get(ctx, &data)...)
+
+	if resp.Diagnostics.HasError() {
+		return
+	}
+
+	resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
+}
+
+func (r SetNestedBlockWithDefaultsResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) {
+}
+
+type SetNestedBlockWithDefaultsResourceModel struct {
+	Set types.Set `tfsdk:"set"`
+}
diff --git a/internal/framework5provider/set_nested_block_with_defaults_test.go b/internal/framework5provider/set_nested_block_with_defaults_test.go
@@ -0,0 +1,111 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+package framework
+
+import (
+	"regexp"
+	"testing"
+
+	"github.com/hashicorp/terraform-plugin-framework/providerserver"
+	"github.com/hashicorp/terraform-plugin-go/tfprotov5"
+	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
+	"github.com/hashicorp/terraform-plugin-testing/knownvalue"
+	"github.com/hashicorp/terraform-plugin-testing/plancheck"
+	"github.com/hashicorp/terraform-plugin-testing/tfjsonpath"
+	"github.com/hashicorp/terraform-plugin-testing/tfversion"
+)
+
+// This test asserts a bug that has yet to be fixed in plugin framework with defaults being used in an attribute inside of a set.
+//
+// This bug can be observed with various different outcomes: producing duplicate set element errors, incorrect diffs during plan,
+// consistent diffs with values switching back and forth, etc. Example bug reports:
+//   - https://github.com/hashicorp/terraform-plugin-framework/issues/783
+//   - https://github.com/hashicorp/terraform-plugin-framework/issues/867
+//   - https://github.com/hashicorp/terraform-plugin-framework/issues/1036
+//
+// They all originate from the same root cause, which is when using `Default` on multiple attributes inside of a set, when one default
+// value is applied, the other default values may also be applied due to the set element being modified during traversal. The reason this
+// results in differing behavior is because Terraform core can't apply data consistency rules to sets that contain objects, so instead of
+// a single consistent error message, we get a bunch of different errors/odd behavior depending on the exact result of the defaulting logic.
+//
+// This specific test will successfully apply with the correct data, then following refresh/plan/apply commands will all raise
+// a "duplicate set element" error. Since the framework logic defaults the set elements while traversing it and set elements are identified by their
+// value, follow-up path lookups for other set elements can't find the correct data, resulting in default values being applied incorrectly
+// for the "value" attributes.
+//
+//	Error: Duplicate Set Element
+//
+//	with framework_set_nested_block_with_defaults.test,
+//	on terraform_plugin_test.tf line 11, in resource "framework_set_nested_block_with_defaults" "test":
+//	11: resource "framework_set_nested_block_with_defaults" "test" {
+//
+//	This attribute contains duplicate values of:
+//	tftypes.Object["default_value":tftypes.String,
+//	"value":tftypes.String]<"default_value":tftypes.String<"this is a default">,
+//	"value":tftypes.String<"zero">>
+//
+// Once this bug is fixed, the ExpectError regex in this test should be removed and the plan check should be switched to a state check.
+func TestSetNestedBlockWithDefaults(t *testing.T) {
+	resource.UnitTest(t, resource.TestCase{
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			// The "Duplicate Set Element" error was introduced in Terraform 1.4
+			tfversion.SkipBelow(tfversion.Version1_4_0),
+		},
+		ProtoV5ProviderFactories: map[string]func() (tfprotov5.ProviderServer, error){
+			"framework": providerserver.NewProtocol5WithError(New()),
+		},
+		Steps: []resource.TestStep{
+			{
+				Config: `resource "framework_set_nested_block_with_defaults" "test" {
+					set {
+						value = "one"
+					}
+					set {
+						value = "two"
+					}
+					set {
+						value = "three"
+					}
+				}`,
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{
+						plancheck.ExpectKnownValue(
+							"framework_set_nested_block_with_defaults.test",
+							tfjsonpath.New("set"),
+							knownvalue.SetExact(
+								[]knownvalue.Check{
+									knownvalue.ObjectExact(
+										map[string]knownvalue.Check{
+											// During plan and after the first apply, this value will be "one"
+											// After the first refresh, the bug will cause this value to be defaulted to "zero"
+											"value":         knownvalue.StringExact("one"),
+											"default_value": knownvalue.StringExact("this is a default"),
+										},
+									),
+									knownvalue.ObjectExact(
+										map[string]knownvalue.Check{
+											// During plan and after the first apply, this value will be "two"
+											// After the first refresh, the bug will cause this value to be defaulted to "zero"
+											"value":         knownvalue.StringExact("two"),
+											"default_value": knownvalue.StringExact("this is a default"),
+										},
+									),
+									knownvalue.ObjectExact(
+										map[string]knownvalue.Check{
+											// During plan and after the first apply, this value will be "three"
+											// After the first refresh, the bug will cause this value to be defaulted to "zero"
+											"value":         knownvalue.StringExact("three"),
+											"default_value": knownvalue.StringExact("this is a default"),
+										},
+									),
+								},
+							),
+						),
+					},
+				},
+				ExpectError: regexp.MustCompile(`Error: Duplicate Set Element`),
+			},
+		},
+	})
+}
diff --git a/internal/framework6provider/provider.go b/internal/framework6provider/provider.go
@@ -76,6 +76,8 @@ func (p *testProvider) Resources(_ context.Context) []func() resource.Resource {
 		NewFloat64PrecisionResource,
 		NewTFSDKReflectionResource,
 		NewMoveStateResource,
+		NewSetNestedBlockWithDefaultsResource,
+		NewSetNestedAttributeWithDefaultsResource,
 	}
 }
 

diff --git a/internal/framework6provider/set_nested_attribute_with_defaults.go b/internal/framework6provider/set_nested_attribute_with_defaults.go
@@ -0,0 +1,100 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+package framework
+
+import (
+	"context"
+
+	"github.com/hashicorp/terraform-plugin-framework/resource"
+	"github.com/hashicorp/terraform-plugin-framework/resource/schema"
+	"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringdefault"
+	"github.com/hashicorp/terraform-plugin-framework/types"
+)
+
+var _ resource.Resource = SetNestedAttributeWithDefaultsResource{}
+
+func NewSetNestedAttributeWithDefaultsResource() resource.Resource {
+	return &SetNestedAttributeWithDefaultsResource{}
+}
+
+// SetNestedAttributeWithDefaultsResource is used for a test asserting a bug that has yet to be fixed in plugin framework
+// with defaults being used in an attribute inside of a set.
+//
+// This bug can be observed with various different outcomes: producing duplicate set element errors, incorrect diffs during plan,
+// consistent diffs with values switching back and forth, etc. Example bug reports:
+//   - https://github.com/hashicorp/terraform-plugin-framework/issues/783
+//   - https://github.com/hashicorp/terraform-plugin-framework/issues/867
+//   - https://github.com/hashicorp/terraform-plugin-framework/issues/1036
+type SetNestedAttributeWithDefaultsResource struct{}
+
+func (r SetNestedAttributeWithDefaultsResource) Metadata(_ context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) {
+	resp.TypeName = req.ProviderTypeName + "_set_nested_attribute_with_defaults"
+}
+
+func (r SetNestedAttributeWithDefaultsResource) Schema(_ context.Context, _ resource.SchemaRequest, resp *resource.SchemaResponse) {
+	resp.Schema = schema.Schema{
+		Attributes: map[string]schema.Attribute{
+			"set": schema.SetNestedAttribute{
+				Required: true,
+				NestedObject: schema.NestedAttributeObject{
+					Attributes: map[string]schema.Attribute{
+						"value": schema.StringAttribute{
+							Optional: true,
+							Computed: true,
+							Default:  stringdefault.StaticString("zero"),
+						},
+						"default_value": schema.StringAttribute{
+							Optional: true,
+							Computed: true,
+							Default:  stringdefault.StaticString("this is a default"),
+						},
+					},
+				},
+			},
+		},
+	}
+}
+
+func (r SetNestedAttributeWithDefaultsResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) {
+	var data SetNestedAttributeWithDefaultsResourceModel
+
+	resp.Diagnostics.Append(req.Plan.Get(ctx, &data)...)
+
+	if resp.Diagnostics.HasError() {
+		return
+	}
+
+	resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
+}
+
+func (r SetNestedAttributeWithDefaultsResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) {
+	var data SetNestedAttributeWithDefaultsResourceModel
+
+	resp.Diagnostics.Append(req.State.Get(ctx, &data)...)
+
+	if resp.Diagnostics.HasError() {
+		return
+	}
+
+	resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
+}
+
+func (r SetNestedAttributeWithDefaultsResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) {
+	var data SetNestedAttributeWithDefaultsResourceModel
+
+	resp.Diagnostics.Append(req.Plan.Get(ctx, &data)...)
+
+	if resp.Diagnostics.HasError() {
+		return
+	}
+
+	resp.Diagnostics.Append(resp.State.Set(ctx, &data)...)
+}
+
+func (r SetNestedAttributeWithDefaultsResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) {
+}
+
+type SetNestedAttributeWithDefaultsResourceModel struct {
+	Set types.Set `tfsdk:"set"`
+}