Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nicer error when list/map assigned to string argument #3009

Merged
merged 1 commit into from
Oct 23, 2015
Merged

Nicer error when list/map assigned to string argument #3009

merged 1 commit into from
Oct 23, 2015

Conversation

apparentlymart
Copy link
Contributor

For example:

resource "aws_route_table_association" "foo" {
    route_table_id = "baz"
    subnet_id = ["a", "b"] // this is supposed to be just a string
}

Previous this would return the following sort of error:

expected type 'string', got unconvertible type '[]interface {}'

This is the raw error returned by the underlying mapstructure library. This is not a helpful error message for anyone who doesn't know Go's type system, and it exposes Terraform's internals to the UI.

Instead, catch these cases before we try to use mapstructure and return a more straightforward message.

This also fixes a crash caused when a computed list is assigned to a string attribute. Previously this was not caught at all due to the short-circuit for computed attributes, but now we check for lists and maps before we check for computed, and so we can fail in a nicer way in this case rather than crashing during the later Diff operation. This is safe because a computed list is materially different than a computed primitive: it's a []interface{} with some of the values being interpolated strings, instead of just a single interpolated string.

This issue was exposed by #2984.

@jszwedko
Copy link
Contributor

jszwedko commented Sep 4, 2015

👏

@josephholsten
Copy link
Contributor

💃 @phinze, @catsby, @radeksimko how does this feel to your 👀?

@catsby
Copy link
Contributor

catsby commented Oct 21, 2015

This fixes #3577

Previous this would return the following sort of error:
expected type 'string', got unconvertible type '[]interface {}'

This is the raw error returned by the underlying mapstructure library.
This is not a helpful error message for anyone who doesn't know Go's
type system, and it exposes Terraform's internals to the UI.

Instead we'll catch these cases before we try to use mapstructure and
return a more straightforward message.

By checking the type before the IsComputed exception this also avoids
a crash caused when the assigned value is a computed list. Otherwise
the list of interpolations is allowed through here and then crashes later
during Diff when the value is not a primitive as expected.
@apparentlymart apparentlymart merged commit a671825 into hashicorp:master Oct 23, 2015
@apparentlymart
Copy link
Contributor Author

Given how long this has been sitting here, how simple this is, that there are now three separate bug reports for this, and that @catsby apparently tested it, I went ahead and merged this.

@apparentlymart apparentlymart deleted the list-string-crash branch October 23, 2015 04:45
@catsby
Copy link
Contributor

catsby commented Oct 26, 2015

👍

@ghost
Copy link

ghost commented Apr 30, 2020

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.

@ghost ghost locked and limited conversation to collaborators Apr 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants