Fix EC2 instance panic

[mergenci]

Fixes crossplane-contrib/provider-upjet-aws#1579.

Cause

The panic originates from:
https://github.com/hashicorp/terraform-provider-aws/blob/v5.78.0/internal/service/ec2/ec2_instance.go#L2385

if v, ok := d.GetOk("root_block_device"); ok && len(v.([]interface{})) > 0 && blockDeviceIsRoot(instanceBd, instance) {
	bd[names.AttrTags] = tags.ResolveDuplicates(ctx, defaultTagsConfig, ignoreTagsConfig, d, "root_block_device[0].tags", nil).Map()
}

The code checks for existence of root_block_device, using d.GetOk("root_block_device"), before accessing its tags root_block_device[0].tags. The problem stems from the fact that Terraform has multiple sources of resource data which are combined as needed. These sources are: state, config, diff, set. When a piece of data exists in multiple sources, the one that comes later in the list is used.

Theoretically, d.GetOk() and tags.ResolveDuplicates() may read from different sources, which may cause different kinds of errors. In our case though, both read from state. Unfortunately, even the sources have “subsources”, from which a data can be read. For state, these “subsources” are Attributes, RawConfig, RawState, RawPlan

Whereas d.GetOk() uses the multilevel reader that reads from Attributes, tags.ResolveDuplicates() reads from RawConfig.

To summarize, d.GetOk("root_block_device") returns d.state.Attributes["root_block_device"], which exists. Then, tags.ResolveDuplicates() accesses d.state.RawConfig.v["root_block_device"], which is nil.

Fix

The panic occurs at cty.LengthInt(), which is called from GetAnyAttr(). Note that, LengthInt() is called only for list or tuple types. Nil tuples don't panic in LengthInt(). So, to prevent the panic, we only check whether the value is a non-nil and known list before calling LengthInt().

[github-actions]

Community Note

Voting for Prioritization

• Please vote on this pull request by adding a 👍 reaction to the original post to help the community and maintainers prioritize this pull request.
• Please see our prioritization guide for information on how we prioritize.
• Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.

For Submitters

• Review the contribution guide relating to the type of change you are making to ensure all of the necessary steps have been taken.
• For new resources and data sources, use skaff to generate scaffolding with comments detailing common expectations.
• Whether or not the branch has been rebased will not impact prioritization, but doing so is always a welcome surprise.

[erhancagirici]

LGTM

[mergenci]

I would like to summarize our discussion with Erhan regarding how Crossplane provider behavior differs from that of Terraform workflow's.

1. Whereas Terraform CLI processes HCL representation, we process JSON. An example behavior difference that is especially relevant to this PR stems from HCL's processing of zero length list blocks into cty.ListValEmpty(). In contrast, we leave unspecified (effectively zero length) lists as nil.
2. We omit some logic performed by Terraform CLI. For instance, Terraform CLI omits displaying computed outputs as diff, if user has specified an equivalent field.
3. We omit some logic performed by Terraform provider server. Even Terraform Plugin SDK and Terraform Plugin Framework servers operate differently. For instance, Terraform Plugin Framework server reifies null values and nullifies empty values. These operations are not performed by Terraform Plugin SDK server.

The next time we encounter a panic because of nil values, we plan to dig a little deeper into these differences to address the root causes.