hashicorp / hashicorp/terraform-plugin-sdk

Prevent Invalid id Attribute Schema (Missing Computed)

Open
#654 2 comments 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.3.0
```

### Use-cases

The following schema for a root `id` attribute is currently "valid" but does not work in all cases, especially in more recent versions of Terraform CLI:

```go
&schema.Resource{
// ...
Schema: map[string]*schema.Schema{
// ...
"id": {
Type: schema.TypeString,
Optional: true,
},
},
}
```

For practitioners attempting to reference the `id` attribute, there is confusing behavior when the attribute is not configured. The quickest explanation about this problem is https://github.com/hashicorp/terraform-provider-aws/issues/11554#issuecomment-740972275. The current Terraform Plugin SDK implicitly always sets the `id` attribute as part of calling `(helper/schema.ResourceData).SetId()`, which is required for all properly implemented managed resources and data sources. Given that the SDK allows this schema, it likely should be updated to either error check it or handle the situation automatically.

### Attempted Solutions

Writing new schema validation outside the existing Terraform Plugin SDK validation seems like a lot of mostly duplicate effort.

Writing JSON schema output tooling for this case could be a lot of effort, not only to bootstrap the tool or correct `jq` logic, but also checking for this problem on code submission would require Terraform CLI to call the `providers schema -json` command against a locally built provider so its just a bit harder to setup in CI (at least for other providers that haven't needed this yet 😉 ).

Writing Go static analysis tooling for this case is difficult because it requires semantic knowledge to know that it is indeed only checking a root `id` attribute for a resource and code styling for setting up resources can vary across providers.

### Proposal

Some options that come to mind:

1. Add `(helper/schema.Resource).InternalValidate()` logic (near the other schema checking) so provider developers receive an error during unit testing about this issue.
1. Automatically add `Computed: true` to root `id` attribute schemas that are missing it. Introduces magic (and probably not preferred magic), but prevents the issue.

### References

- https://github.com/hashicorp/terraform-plugin-sdk/issues/541
- https://github.com/hashicorp/terraform-provider-aws/issues/11554

Contributor guide

Open the contributing guide

Research direction

Start with helper/schema.Resource.InternalValidate and the existing schema-checking logic mentioned in the issue. Reproduce the root id schema case from the example, then determine whether validation or automatic Computed handling fits the SDK's behavior. Done means the invalid schema is consistently rejected or handled, with coverage for the reported case.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, terraform
Domain
tooling
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.