Check set equality for service account scope changes #1130
Conversation
google/resource_compute_instance.go
Outdated
| nList := n.([]interface{}) | ||
| scopesChange := len(oList) != len(nList) | ||
| // service_account has MaxItems: 1 | ||
| if len(oList) == 1 && len(nList) == 1 { |
There was a problem hiding this comment.
You can skip these checks if scopesChange is already true, right? So you could make this scopesChange && len(oList) == 1? It seems a little clearer to me that way, but I'm not an expert here.
There was a problem hiding this comment.
Ok I went for extreme clarity here, let me know what you think
google/resource_compute_instance.go
Outdated
| // Attributes which can only be changed if the instance is stopped | ||
| if d.HasChange("machine_type") || d.HasChange("min_cpu_platform") || d.HasChange("service_account") { | ||
| if d.HasChange("machine_type") || d.HasChange("min_cpu_platform") || d.HasChange("service_account.0.email") || scopesChange { |
There was a problem hiding this comment.
My extremely mild preference is for scopesChange to be first in this list, on the principle that people only read the first item or two in a list, and if the first two items are the same sort of thing, even more so.
| } | ||
|
|
||
| create_timeout = 5 | ||
|
|
||
| metadata_startup_script = "echo Hello" |
There was a problem hiding this comment.
I remember making this one different on purpose - it was essential to catching some issues with metadata_startup_script vs metadata.startup-script a few weeks ago. Did you know about all that already?
There was a problem hiding this comment.
Yeah, I think you changed it for the _basic test, which meant that the _update one now had a change on metadata_startup_script, which is a ForceNew attribute. This updates the _update test to match so the only things that change are updatable fields.
Signed-off-by: Modular Magician <[email protected]>
|
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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks! |
ghost
locked and limited conversation to collaborators
See https://github.com/hashicorp/terraform/issues/17411#issuecomment-368122192 for context.
Fixes #1108.