hashicorp / hashicorp/terraform-plugin-sdk
Prevent Invalid id Attribute Schema (Missing Computed)
- 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
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