Error planning tainted resource when it no longer exists #27563
Conversation
The existing context test files are becoming quite unwieldy. Start new ones both to make editing easier, and to help discourage the copy+pasting of older test patterns we no longer need.
Codecov Report
|
The tainted state was checked against `cty.NilVal`, however it was always being set to a null value. The refreshed state value was being shadowed, and not used in the following plan.
jbardin
force-pushed
the
jbardin/refresh-tainted
branch
from
009bdb6 to
cb7a08c
Compare
| diags = diags.Append(refreshDiags) | ||
| if diags.HasErrors() { | ||
| return diags | ||
| } | ||
| instanceRefreshState = s |
There was a problem hiding this comment.
Is the temporary variable s needed here? I don't see the logical difference between this and the prior code, given the shortcut return on error.
There was a problem hiding this comment.
instanceRefreshState is passed to plan below, but instanceRefreshState here is scoped only to this block and shadows the outer variable. The temporary value ensures both that we don't shadow the outer variable, and that the new value is re-assigned. (I could have just as easily declared a temp diagnostics variable and removed the :=)
There was a problem hiding this comment.
Ah, I see now, thanks for clarifying!
|
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 as resolved and limited conversation to collaborators
When a resource returns an error during apply we "taint" the state for that resource, but save it just in case.
The resource would then be removed during the next refresh if it did not exist, and the plan should result in a Create action rather than a replacement. The tainted state however was compared against
cty.NilValrather than checking if it was more generallyNull, which is the normal case after the resource is removed from the state.Fixes #27559
Manual 0.14 backport will be required due to changes in 0.15