Prevent Invalid id Attribute Schema (Missing Computed)

[bflad]

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:

&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 hashicorp/terraform-provider-aws#11554 (comment). 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.
2. 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

• Ability to Get/Set Only the State Resource Identifier (Without id Attribute Handling) #541
• data source aws_ec2_transit_gateway_route_table doesn't feed data properly in plan phase to resource "aws_ec2_transit_gateway_route" terraform-provider-aws#11554

[glennsarti]

Have to say, I hit this really obtuse error message as well...

    testing_new_config.go:82: no "id" found in attributes
    testing_new.go:63: no "id" found in attributes

That failure output is really bad. And the documentation to fix it is buried at https://developer.hashicorp.com/terraform/plugin/framework/acctests#no-id-found-in-attributes

[bflad]

Hi @glennsarti 👋 Sorry you ran into that frustrating situation.

Since this issue was created awhile ago, provider acceptance testing functionality has been forked into the separate terraform-plugin-testing Go module with its own website documentation. In there is a migration guide, that mainly is about updating import statements to point at the new Go module. Since terraform-plugin-testing v1.5.0, the id attribute is no longer required to exist, and there is also a lot of value add functionality only being added in the new Go module, such as Terraform version checks, plan checks, etc.

At some point the entire terraform-plugin-sdk Go module will be wholly deprecated to recommend terraform-plugin-framework and terraform-plugin-testing, but that product decision has not been scheduled. Maybe in the meantime, we can and should prioritize at least putting deprecation notices for acceptance testing functionality, since that is an "easier" sell for developers as it does not require the amount of rework as framework migrations.