helper/schema: Fail schema validation if Elem is not of a valid type

[jimmy-btn]

In several places in the helper/schema package, Terraform assumes that the Elem field is either a *Resource or a *Schema, and panics if this is not the case.

The best example of this is in the CoreConfigSchema() function in helper/schema/core_schema.go

This doesn't normally appear to be a problem, but I've managed to tickle it while working on some other changes.

This PR introduces an explicit test for the type of the Elem field as part of the InternalValidate function, which is used to validate the provider schema in unit tests. Schema's which fail this test are definitely invalid.

Unfortunately this will cause this test to begin (correctly) failing for some providers. I have PRs ready to go for AWS and Google, and will keep an eye on others.

[apparentlymart]

Thanks for working on this, @jimmy-btn!

I agree that this is a good thing to catch in InternalValidate, and your change here looks good to me.

I see you've already posted the AWS provider PR for the lambda function environment variables. Thanks for that! It seems like we can safely wait until that and the Google provider issue are addressed before we merge here... would you agree? If so, I think that's preferable just to avoid the need to carefully coordinate updating the vendoring for these when we know there are issues.

For other providers I think we can play it by ear... helper/schema is vendored into the providers anyway, so if there are issues we will catch them in a test failure for a future PR to update the vendoring, at which point I'm sure we can address them quickly before we merge.

[jimmy-btn]

Sounds great to me - I'll land the PRs for the Google and AWS providers then come back and update this thread.

[jimmy-btn]

@apparentlymart - I've hit a roadblock in upgrading the AWS provider, and would need to merge #17097 first.

It looks like the field reader/writers do expect Elem to be a ValueType for Maps, but I'm unsure if this is by design or not. In either case, there's an inconsistency between that code and CoreConfigSchema that needs to be resolved, and an inconsistency between Lists and Maps that I think should be resolved.

Would you mind weighing in on the above PR? I would very much value your thoughts!

[teamterraform]

Hello, and sorry for the long silence!
The Terraform SDK has been extracted to its own repository (https://github.com/hashicorp/terraform-plugin-sdk). In support of this we are freezing the related codepaths in Terraform and closing PRs. Thank you!

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.