hashicorp / hashicorp/terraform-plugin-framework

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

Open
#902 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Go
Stars
384
Forks
107
Avg merge
3m
Merged PRs (30d)
1

Description

### Module version

```
v1.4.1
```

### Relevant provider source code

`ReadResource` case:

```go
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:

```go
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

```hcl
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 https://github.com/hashicorp/terraform/issues/34502 (confusing error after errant `Read`) and https://github.com/hashicorp/terraform/issues/34503 (confusing plan after errant `UpgradeState`).

### Steps to Reproduce

1. `terraform apply`
2. `terraform plan`

### References

- https://github.com/hashicorp/terraform-plugin-testing/issues/262
- https://github.com/hashicorp/terraform/issues/34502
- https://github.com/hashicorp/terraform/issues/34503

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.