Implement AddResource(): Explicitly Set Resource into State Regardless of Returned Errors

[zliang-akamai]

Use-cases

Currently, the behavior of the framework is:

Any response errors will cause Terraform to mark the resource as tainted for recreation on the next Terraform plan.

While SDKv2 put the resource into state whenever d.SetId("some_id") is called, even an error is returned.

Framework has the RemoveResource() function for removing the resource from the state, which seems to be equivalent to SDKv2's d.SetId(""). But there isn't something in the framework that's equivalent to d.SetId("some_id") to explicitly set the resource into state.

Sometimes, after a resource is created on the cloud, the provider would have to do some other operations for the resource. If those operations failed and an error included in the response of framework resource's Create function, this resource will become dangling resource because it won't be managed by Terraform.

Attempted Solutions

As a workaround, resp.AddWarning() can be used instead of resp.AddError(). But this is also problematic because warnings are too weak to notify users about the failing operations. Also, in a helper function that does an operations for both Create and Update, it is a little bit annoying to determine either an error or a warning should be added.

Proposal

It would be worth to consider that adding a function like AddResource() to allow setting the resource explicitly.

[bflad]

Hi @zliang-akamai 👋 Thank you for raising this.

The framework equivalent of terraform-plugin-sdk's (helper/schema.ResourceData).SetId() method is calling (tfsdk.State).SetAttribute(). The tfsdk.State type is found in the resource.CreateResponse.State, so the following example snippet should accomplish the same goal of only setting the id attribute:

func (r ExampleResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) {
	var data ExampleResourceModel

	resp.Diagnostics.Append(req.Plan.Get(ctx, &data)...)

	if resp.Diagnostics.HasError() {
		return
	}

	resp.State.SetAttribute(ctx, path.Root("id"), types.StringValue("testing"))
}

Depending on the schema, it may also be possible to use the data model setup and set only the id (or whatever) attribute, such as:

func (r ExampleResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) {
	var data ExampleResourceModel

	resp.Diagnostics.Append(req.Plan.Get(ctx, &data)...)

	if resp.Diagnostics.HasError() {
		return
	}

	data.ID = types.StringValue("testing")

	resp.Diagnostics.Append(resp.State.Set(ctx, data)...)
}

To replicate your use case, I added a resp.Diagnostics.AddError("create error summary", "create error detail") as the last line of the Create method for both of these examples and setup a terraform-plugin-testing based acceptance test which captures that Terraform should be saving the resource instance as tainted and proposing recreation of the resource:

package provider

import (
	"regexp"
	"testing"

	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
	"github.com/hashicorp/terraform-plugin-testing/plancheck"
)

func TestExampleResource_basic(t *testing.T) {
	resource.UnitTest(t, resource.TestCase{
		ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
		Steps: []resource.TestStep{
			{
				Config: `resource "framework_example" "test" {}`,
				Check: resource.ComposeAggregateTestCheckFunc(
					resource.TestCheckResourceAttr("framework_example.test", "id", "testing"),
				),
				ExpectError: regexp.MustCompile(`create error summary`),
			},
			{
				Config: `resource "framework_example" "test" {}`,
				ConfigPlanChecks: resource.ConfigPlanChecks{
					PreApply: []plancheck.PlanCheck{
						plancheck.ExpectResourceAction("framework_example.test", plancheck.ResourceActionReplace),
					},
				},
				ExpectError: regexp.MustCompile(`create error summary`),
			},
		},
	})
}

If you are seeing different behavior, one thing you can double check is whether your logic is returning early when checking for error diagnostics before the resource.CreateResponse.State is being set at all. Otherwise if you are still running into trouble, I think we will need to see the relevant code to triage further.

Can you please let us know? Thanks.

[zliang-akamai]

Thank you @bflad! It tuned out to be my mistake that I didn't set 2 of the IDs required for Read so my state was cleaned up by the refresh. Thank you so much for the detailed explanation of it!

[github-actions]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.