Skip to content
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

Consider Explicit Error Diagnostics When MoveResourceState/ReadResource/UpgradeResourceState Response State Includes Unknown Value #902

Open
bflad opened this issue Jan 8, 2024 · 0 comments
Labels
bug Something isn't working

Comments

@bflad
Copy link
Member

bflad commented Jan 8, 2024

Module version

v1.4.1

Relevant provider source code

ReadResource case:

package provider

import (
	"context"

	"github.com/hashicorp/terraform-plugin-framework/resource"
	"github.com/hashicorp/terraform-plugin-framework/resource/schema"
	"github.com/hashicorp/terraform-plugin-framework/types"
)

var (
	_ resource.Resource = &Testing262Resource{}
)

func NewTesting262Resource() resource.Resource {
	return &Testing262Resource{}
}

type Testing262Resource struct{}

func (r *Testing262Resource) Metadata(_ context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) {
	resp.TypeName = req.ProviderTypeName + "_testing262"
}

func (r *Testing262Resource) Schema(_ context.Context, _ resource.SchemaRequest, resp *resource.SchemaResponse) {
	resp.Schema = schema.Schema{
		Attributes: map[string]schema.Attribute{
			"required_string": schema.StringAttribute{
				Required: true,
			},
		},
	}
}

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

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

	if resp.Diagnostics.HasError() {
		return
	}

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

func (r Testing262Resource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) {
	var data Testing262ResourceModel

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

	if resp.Diagnostics.HasError() {
		return
	}

	// intentionally invalid implementation for example code
	data.RequiredString = types.StringUnknown()

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

func (r Testing262Resource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) {
	var data Testing262ResourceModel

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

	if resp.Diagnostics.HasError() {
		return
	}

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

func (r Testing262Resource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) {
	var data Testing262ResourceModel

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

type Testing262ResourceModel struct {
	RequiredString types.String `tfsdk:"required_string"`
}

UpgradeResourceState case:

package provider

import (
	"context"

	"github.com/hashicorp/terraform-plugin-framework/resource"
	"github.com/hashicorp/terraform-plugin-framework/resource/schema"
	"github.com/hashicorp/terraform-plugin-framework/types"
)

var (
	_ resource.Resource                 = &Testing262Resource{}
	_ resource.ResourceWithUpgradeState = &Testing262Resource{}
)

func NewTesting262Resource() resource.Resource {
	return &Testing262Resource{}
}

type Testing262Resource struct{}

func (r *Testing262Resource) Metadata(_ context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) {
	resp.TypeName = req.ProviderTypeName + "_testing262"
}

func (r *Testing262Resource) Schema(_ context.Context, _ resource.SchemaRequest, resp *resource.SchemaResponse) {
	resp.Schema = schema.Schema{
		Attributes: map[string]schema.Attribute{
			"required_string": schema.StringAttribute{
				Required: true,
			},
		},
		Version: 1, // remove for prior version apply
	}
}

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

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

	if resp.Diagnostics.HasError() {
		return
	}

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

func (r Testing262Resource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) {
	var data Testing262ResourceModel

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

	if resp.Diagnostics.HasError() {
		return
	}

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

func (r Testing262Resource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) {
	var data Testing262ResourceModel

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

	if resp.Diagnostics.HasError() {
		return
	}

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

func (r Testing262Resource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) {
	var data Testing262ResourceModel

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

func (r Testing262Resource) UpgradeState(ctx context.Context) map[int64]resource.StateUpgrader {
	return map[int64]resource.StateUpgrader{
		0: {
			// Same version 0 and version 1 schema for example simplicity
			PriorSchema: &schema.Schema{
				Attributes: map[string]schema.Attribute{
					"required_string": schema.StringAttribute{
						Required: true,
					},
				},
			},
			StateUpgrader: func(ctx context.Context, req resource.UpgradeStateRequest, resp *resource.UpgradeStateResponse) {
				var data Testing262ResourceModel

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

				if resp.Diagnostics.HasError() {
					return
				}

				// intentionally invalid implementation for example code
				data.RequiredString = types.StringUnknown()

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

type Testing262ResourceModel struct {
	RequiredString types.String `tfsdk:"required_string"`
}

Terraform Configuration Files

resource "framework_testing262" "test" {
  required_string = "test"
}

Expected Behavior

The framework should let provider developers know when their managed resource Read or UpgradeState method implementations are incorrect. This is preferable to happen in the framework since Terraform core does not raise its own error currently, which means that even if/when it does, it will only occur in upgraded Terraform environments.

This can be implemented by walking the response states and raising error diagnostics for all unknown values encountered. This should prevent Terraform from saving the errant state while also notifying practitioners and developers with explicit messaging about the problem.

Actual Behavior

Weird behaviors next terraform plan as noted in hashicorp/terraform#34502 (confusing error after errant Read) and hashicorp/terraform#34503 (confusing plan after errant UpgradeState).

Steps to Reproduce

  1. terraform apply
  2. terraform plan

References

@bflad bflad added the bug Something isn't working label Jan 8, 2024
bflad added a commit that referenced this issue Jan 9, 2024
…e errors

Reference: #902
Reference: hashicorp/terraform#34502
Reference: hashicorp/terraform#34503

Unknown values are never valid in resource state and while Terraform nor the framework actually raise these errors right now, one or both will in the near future. This documents the errant implementation detail for developers in the meantime since it otherwise causes confusing Terraform behaviors for practitioners.
bflad added a commit that referenced this issue Jan 9, 2024
…e errors (#903)

Reference: #902
Reference: hashicorp/terraform#34502
Reference: hashicorp/terraform#34503

Unknown values are never valid in resource state and while Terraform nor the framework actually raise these errors right now, one or both will in the near future. This documents the errant implementation detail for developers in the meantime since it otherwise causes confusing Terraform behaviors for practitioners.
@bflad bflad changed the title Consider Explicit Error Diagnostics When ReadResource/UpgradeResourceState Response State Includes Unknown Value Consider Explicit Error Diagnostics When MoveResourceState/ReadResource/UpgradeResourceState Response State Includes Unknown Value Feb 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant