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

flatmap: mark computed list as a computed value in Expand #12210

Merged
merged 1 commit into from
Feb 23, 2017

Conversation

mitchellh
Copy link
Contributor

Fixes #12183

The fix is in flatmap for this but the entire issue is a bit more
complex. Given a schema with a computed set, if you reference it like
this:

lookup(attr[0], "field")

And "attr" contains a computed set within it, it would panic even though
"field" is available. There were a couple avenues I could've taken to
fix this:

1.) Any complex value containing any unknown value at any point is
entirely unknown.

2.) Only the specific part of the complex value is unknown.

I took route 2 so that the above works without any computed (since
"name" is not computed but something else is). This may actually have an
effect on other parts of Terraform configs, however those similar
configs would've simply crashed previously so it shouldn't break any
pre-existing configs.

Given the fix, I don't recommend this be backported.

Fixes #12183

The fix is in flatmap for this but the entire issue is a bit more
complex. Given a schema with a computed set, if you reference it like
this:

    lookup(attr[0], "field")

And "attr" contains a computed set within it, it would panic even though
"field" is available. There were a couple avenues I could've taken to
fix this:

1.) Any complex value containing any unknown value at any point is
entirely unknown.

2.) Only the specific part of the complex value is unknown.

I took route 2 so that the above works without any computed (since
"name" is not computed but something else is). This may actually have an
effect on other parts of Terraform configs, however those similar
configs would've simply crashed previously so it shouldn't break any
pre-existing configs.
@mitchellh mitchellh added this to the Terraform 0.9 milestone Feb 23, 2017
@mitchellh mitchellh requested a review from jbardin February 23, 2017 18:04
Copy link
Member

@jbardin jbardin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

impressive fix/test ratio ;)

@mitchellh
Copy link
Contributor Author

Hah, there were a number of places where this could go wrong :P Wanted to test each

@mitchellh mitchellh merged commit f77646a into master Feb 23, 2017
@mitchellh mitchellh deleted the b-computed-elem branch February 23, 2017 18:27
@ghost
Copy link

ghost commented Apr 16, 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 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panic on apply after destroy
3 participants