hashicorp / hashicorp/terraform-plugin-sdk

Prevent Resource Runtime Error "doesn't support update" Via Schema Validation

Open
#689 1 comment 1 reaction 0 assignees View on GitHub
enhancement
Dominant language
Go
Stars
485
Forks
244
Avg merge
19h 57m
Merged PRs (30d)
4

Description

### SDK version

```
v2.4.0
```

### Use-cases

Currently, there is `schema.InternalValidate()` logic to error during unit testing that a resource's schema is invalid:

```
--- FAIL: TestProvider (0.00s)
main_test.go:18: err: 1 error occurred:
* resource example_thing: No Update defined, must set ForceNew on: []string{"block_attr"}
```

However, it is possible to create a schema that passes this check, but still fails in a similar manner during runtime, which is an error such as:

```
--- FAIL: TestAccResourceThing (2.16s)
main_test.go:23: Step 2/2 error: Error running apply:
Error: doesn't support update
```

This can occur if top level (root) attributes all have `ForceNew`, but block (nested) attributes do not. While this may seem trivial in this small context, in providers with large numbers of external contributors of varying levels or resources with large numbers of attributes, the acceptance testing or knowledge of said problem can be difficult to prevent the issue.

Here's a full "one file" provider (okay it is two files because of testing 😛 ) to reproduce the behavior.

```go
package main

import (
"context"

"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-plugin-sdk/v2/plugin"
)

type apiClient struct{}

func provider() *schema.Provider {
return &schema.Provider{
ConfigureContextFunc: func(context.Context, *schema.ResourceData) (interface{}, diag.Diagnostics) {
return &apiClient{}, nil
},
ResourcesMap: map[string]*schema.Resource{
"example_thing": resourceThing(),
},
}
}

func resourceThing() *schema.Resource {
return &schema.Resource{
CreateContext: resourceThingCreate,
ReadContext: schema.NoopContext,
DeleteContext: schema.NoopContext,

Schema: map[string]*schema.Schema{
"block_attr": {
Type: schema.TypeList,
Required: true,
// ForceNew: true, // Remove this line's comment to fix TestProvider
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"nested_attr": {
Type: schema.TypeString,
Required: true,
// ForceNew: true, // Remove this line's comment to fix TestAccResourceThing
},
},
},
},
},
}
}

func resourceThingCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
d.SetId("example")
return nil
}

func main() {
opts := &plugin.ServeOpts{
ProviderFunc: func() *schema.Provider {
return provider()
},
}

plugin.Serve(opts)
}
```

```go
package main

import (
"testing"

"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
)

var providerFactories = map[string]func() (*schema.Provider, error){
"example": func() (*schema.Provider, error) {
return provider(), nil
},
}

func TestProvider(t *testing.T) {
if err := provider().InternalValidate(); err != nil {
t.Fatalf("err: %s", err)
}
}

func TestAccResourceThing(t *testing.T) {
resource.UnitTest(t, resource.TestCase{
ProviderFactories: providerFactories,
Steps: []resource.TestStep{
{
Config: `
resource "example_thing" "test" {
block_attr {
nested_attr = "one"
}
}
`,
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr("example_thing.test", "block_attr.#", "1"),
resource.TestCheckResourceAttr("example_thing.test", "block_attr.0.nested_attr", "one"),
),
},
{
Config: `
resource "example_thing" "test" {
block_attr {
nested_attr = "two"
}
}
`,
Check: resource.ComposeAggregateTestCheckFunc(
resource.TestCheckResourceAttr("example_thing.test", "block_attr.#", "1"),
resource.TestCheckResourceAttr("example_thing.test", "block_attr.0.nested_attr", "two"),
),
},
},
})
}
```

### Attempted Solutions

Manually creating a provider test logic to check for this.

### Proposal

Enhance the existing `InternalValidate()` checking to also verify if there is no `Update`/`UpdateContext` function declared that all block attributes (recursive) also declare `ForceNew: true`. Can be hidden behind a testing environment variable if "breaking".

### References

- https://github.com/hashicorp/terraform-provider-aws/issues/2520
- https://github.com/hashicorp/terraform-provider-aws/issues/7351
- https://github.com/hashicorp/terraform-provider-aws/issues/8017

Contributor guide

Open the contributing guide

Research direction

Start with schema.InternalValidate and the provider() reproduction in the issue, then inspect the TestProvider and TestAccResourceThing examples to understand the validation and runtime paths. Add coverage for recursively checking nested block attributes when no Update or UpdateContext function exists, and confirm the example fails validation rather than reaching the runtime error.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, terraform
Domain
backend-api-design, tooling
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.