wait_for_fulfillment / source_dest_check type regression #6005
Comments
|
found similar issue with disable_api_termination: ~ module.data_nodes.postgresql.aws_instance.postgresql |
|
One of the reasons it happens is mapstructure library. It converts true/false to 1/0: https://github.com/mitchellh/mapstructure/blob/d2dd0262208475919e1a362f675cfc0e7c10e905/mapstructure.go#L61 |
|
This is making terraform really hard to use, made a lot worse because of the way that the dependencies work in terraform. Given: The way the deps in terraform work here using element() means that all instances of aws_instance.worker are considered dependencies of any aws_route53_record.worker instance, even though actually only aws_instance.worker[i] is really a dependency. Because of the integer casting bug, we're unable to use aws_route53_record at all because trying to use any instance of that with -target also then tries to rebuild every aws_instance_worker due to the integer casting bug. |
|
To demonstrate further: |
|
This one is affecting us pretty heavily. We want to be able to ignore certain changes on ec2 instances that would otherwise cause a new resource and have the flexibility to manually taint them at our leisure at a later time. |
|
Hey folks, apologies for all the trouble here! This was a tricky one to run down since I didn't originally realize that In order to reproduce, we need a config that:
Like this: resource "aws_instance" "worker" {
instance_type = "t2.micro"
ami = "ami-534d5d32"
source_dest_check = false
lifecycle {
ignore_changes = ["instance_type"]
}
}After applying that, if we change the And errors in applies: I'm digging in to a fix for this now! Everybody who reported issues: let me know if this sounds like it could be true for your error case. If you feel like you have a repro that does not involve the above scenario - see if you can get a consistent set of steps for it and let me know! |
|
Same for me, using ignore_changes with user_data |
|
Yep, we're using ignore_changes also. |
For a long time now, the diff logic has relied on the behavior of
`mapstructure.WeakDecode` to determine how various primitives are
converted into strings. The `schema.DiffString` function is used for
all primitive field types: TypeBool, TypeInt, TypeFloat, and TypeString.
The `mapstructure` library's string representation of booleans is "0"
and "1", which differs from `strconv.FormatBool`'s "false" and "true"
(which is used in writing out boolean fields to the state).
Because of this difference, diffs have long had the potential for
cosmetically odd but semantically neutral output like:
"true" => "1"
"false" => "0"
So long as `mapstructure.Decode` or `strconv.ParseBool` are used to
interpret these strings, there's no functional problem.
We had our first clear functional problem with #6005 and friends, where
users noticed diffs like the above showing up unexpectedly and causing
troubles when `ignore_changes` was in play.
This particular bug occurs down in Terraform core's EvalIgnoreChanges.
There, the diff is modified to account for ignored attributes, and
special logic attempts to handle properly the situation where the
ignored attribute was going to trigger a resource replacement. That
logic relies on the string representations of the Old and New fields in
the diff to be the same so that it filters properly.
So therefore, we now get a bug when a diff includes `Old: "0", New:
"false"` since the strings do not match, and `ignore_changes` is not
properly handled.
Here, we introduce `TypeBool`-specific normalizing into `finalizeDiff`.
I spiked out a full `diffBool` function, but figuring out which pieces
of `diffString` to duplicate there got hairy. This seemed like a simpler
and more direct solution.
Fixes #6005 (and potentially others!)
For a long time now, the diff logic has relied on the behavior of
`mapstructure.WeakDecode` to determine how various primitives are
converted into strings. The `schema.DiffString` function is used for
all primitive field types: TypeBool, TypeInt, TypeFloat, and TypeString.
The `mapstructure` library's string representation of booleans is "0"
and "1", which differs from `strconv.FormatBool`'s "false" and "true"
(which is used in writing out boolean fields to the state).
Because of this difference, diffs have long had the potential for
cosmetically odd but semantically neutral output like:
"true" => "1"
"false" => "0"
So long as `mapstructure.Decode` or `strconv.ParseBool` are used to
interpret these strings, there's no functional problem.
We had our first clear functional problem with hashicorp#6005 and friends, where
users noticed diffs like the above showing up unexpectedly and causing
troubles when `ignore_changes` was in play.
This particular bug occurs down in Terraform core's EvalIgnoreChanges.
There, the diff is modified to account for ignored attributes, and
special logic attempts to handle properly the situation where the
ignored attribute was going to trigger a resource replacement. That
logic relies on the string representations of the Old and New fields in
the diff to be the same so that it filters properly.
So therefore, we now get a bug when a diff includes `Old: "0", New:
"false"` since the strings do not match, and `ignore_changes` is not
properly handled.
Here, we introduce `TypeBool`-specific normalizing into `finalizeDiff`.
I spiked out a full `diffBool` function, but figuring out which pieces
of `diffString` to duplicate there got hairy. This seemed like a simpler
and more direct solution.
Fixes hashicorp#6005 (and potentially others!)
|
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
Having upgraded from terraform 6.8 to 6.14
terraform plannow reports:The relevant lines from the resource are:
These lines have not changed since the last apply.
If we try
terraform applyit reports an error:The text was updated successfully, but these errors were encountered: