Convert variable types before applying defaults

[liamcervante]

Fixes #31978

Currently Terraform attempts to apply defaults in variables before calling go-cty to convert the variables into their actual concrete types. This can lead to a situation where HCL cannot apply the defaults because of a type incompatibility and then panics and crashes Terraform (#31978). It's kind of impossible to recover from or predict this situation without modifying the HCL public functions (eg. make the panic return an error, or add a new function like CheckCanApplyDefaults). We can't do this check with go-cty because as far as go-cty is concerned these are perfectly valid conversions. Note, that HCL and go-cty disagree here about what is or what isn't a valid type conversion.

This PR switches the order of these operations. We now go through go-cty to convert to the concrete type first, then when we call the apply defaults function the types are guaranteed to match and HCL doesn't need to worry about any kind of conversions.

This change requires an upstream fix in go-cty, so go-cty is updated as part of this change.

Target Release

1.3.4

Draft CHANGELOG entry

BUG FIXES

• Input and Module Variables: Convert variable types before attempting to apply default values

[alisdair]

I'm not sure this change is valid, but I'm also not sure it's invalid.

We originally applied defaults before converting type in part to allow us to distinguish between missing object attributes and attributes with null value. This distinction was later removed in order to harmonize the treatment of null with other contexts, so it may no longer be necessary.

However, you should be able to assign a map value to an object variable, so the existing test that you changed surprises me. From glancing at the code I'm not immediately sure why that would have broken, but I'll take another look.

[liamcervante]

why that would have broken

It breaks because when it arrives in the convert package it's a map of strings. The convert package tries to turn it into the object but because one of the attributes of the object is a complex object that is not convertable from a string the whole convert attempt fails. This is different to the behaviour of inserting defaults which is quite happy to convert a map of strings into the complex object type as it is going through putting the correct defaults in.

I did have a look at going into go-cty and fixing the convert function to allow this. Here's a test case I wrote in go-cty that matches what the test case here is essentially trying to do: zclconf/go-cty#139.

[liamcervante]

I've reverted the change I had to make to the other test, which means the tests are now going to report as failing. But once the following PRs are merged into go-cty and released this test will start passing again:

• Strip optional attributes from null objects during conversion zclconf/go-cty#140
• Allow maps to convert into objects when missing optional attributes are not compatible zclconf/go-cty#139
• Remove optional attributes from null values in all collections zclconf/go-cty#141

[liamcervante]

This is ready for review!

[github-actions]

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.