helper/schema,terraform: handle computed primitives in diffs #9618
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes #3309 There are two primary changes, one to how helper/schema creates diffs and one to how Terraform compares diffs. Both require careful understanding. == 1. helper/schema Changes helper/schema, given any primitive field (string, int, bool, etc.) _used to_ create a basic diff when given a computed new value (i.e. from an unkown interpolation). This would put in the plan that the old value is whatever the old value was, and the new value was the actual interpolation. For example, from #3309, the diff showed the following: ``` ~ module.test.aws_eip.test-instance.0 instance: "<INSTANCE ID>" => "${element(aws_instance.test-instance.*.id, count.index)}" ``` Then, when running `apply`, the diff would be realized and you would get a diff mismatch error because it would realize the final value is the same and remove it from the diff. **The change:** `helper/schema` now marks unknown primitive values with `NewComputed` set to true. Semantically this is correct for the diff to have this information. == 2. Terraform Diff.Same Changes Next, the way Terraform compares diffs needed to be updated Specifically, the case where the diff from the plan had a NewComputed primitive and the diff from the apply _no longer has that value_. This is possible if the computed value ended up being the same as the old value. This is allowed to pass through. Together, these fix #3309.
mitchellh
commented
Oct 26, 2016
| New: "${var.foo}", | ||
| Old: "", | ||
| New: "${var.foo}", | ||
| NewComputed: true, |
There was a problem hiding this comment.
This here is an actual test CHANGE versus a new change. The old behavior I believe to be incorrect since there was no way to tell that when applying, the EXACT value of ${var.foo} wasn't expected... the diff comparison would simply never work.
mitchellh
changed the title
jbardin
approved these changes
Oct 28, 2016
|
Thanks! |
mitchellh
added a commit
that referenced
this pull request
Nov 1, 2016
For #9618, we added the ability to ignore old diffs that were computed and removed (because the ultimate value ended up being the same). This ended up breaking computed list/set logic. The correct behavior, as is evident by how the other "skip" logics work, is to set `ok = true` so that the remainder of the logic can run which handles stuff such as computed lists and sets.
gusmat
pushed a commit
to gusmat/terraform
that referenced
this pull request
Dec 6, 2016
For hashicorp#9618, we added the ability to ignore old diffs that were computed and removed (because the ultimate value ended up being the same). This ended up breaking computed list/set logic. The correct behavior, as is evident by how the other "skip" logics work, is to set `ok = true` so that the remainder of the logic can run which handles stuff such as computed lists and sets.
|
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
locked and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #3309
There are two primary changes, one to how helper/schema creates diffs
and one to how Terraform compares diffs. Both require careful
understanding.
1. helper/schema Changes
helper/schema, given any primitive field (string, int, bool, etc.)
used to create a basic diff when given a computed new value (i.e. from
an unkown interpolation). This would put in the plan that the old value
is whatever the old value was, and the new value was the actual
interpolation. For example, from #3309, the diff showed the following:
Then, when running
apply, the diff would be realized and you would geta diff mismatch error because it would realize the final value is the
same and remove it from the diff.
The change:
helper/schemanow marks unknown primitive values withNewComputedset to true. Semantically this is correct for the diff tohave this information.
2. Terraform Diff.Same Changes
Next, the way Terraform compares diffs needed to be updated
Specifically, the case where the diff from the plan had a NewComputed
primitive and the diff from the apply no longer has that value. This
is possible if the computed value ended up being the same as the old
value. This is allowed to pass through.
Together, these fix #3309.