don't lose checks from refresh-only plan #32115
Conversation
jbardin
added
the
1.3-backport
internal/terraform/context_apply.go
Outdated
| @@ -25,6 +25,10 @@ func (c *Context) Apply(plan *plans.Plan, config *configs.Config) (*states.State | |||
|
|
|||
| log.Printf("[DEBUG] Building and walking apply graph for %s plan", plan.UIMode) | |||
|
|
|||
| // FIXME: refresh plans still store outputs as changes, so we can't use | |||
| // Empty() | |||
| possibleRefresh := len(plan.Changes.Resources) == 0 | |||
There was a problem hiding this comment.
Is it worth considering using plan.UIMode == plans.RefreshOnlyMode as the condition here instead of looking at resource changes? The UIMode field does have a "do not use for apply-time logic" warning, so that's rude, but if the goal is detecting a refresh-only plan for this temporary fix maybe that's okay to ignore.
I can't point definitively to an example of how things could go wrong, but my concern is that there might somehow be a non-refresh plan which causes changes to checks without having any resource changes, and we'd be incorrectly recording the checks in state in that case.
There was a problem hiding this comment.
I also can't imagine how that could go wrong, but I agree the UIMode feels slightly more robust. We've committed to using UIMode for the time being because of destroy plan differences, so using it again here isn't going to cause any more problems.
jbardin
force-pushed
the
jbardin/apply-refresh-plan
branch
from
0bc41bc
to
589ff8e
Compare
jbardin
force-pushed
the
jbardin/apply-refresh-plan
branch
from
589ff8e
to
1100eae
Compare
|
Since the original commits ended up included in #32123, I rebased this to only include the final commit from review to clean things up. |
|
Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch. |
|
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
If there are no changes in a given plan, then there is no reason to create and walk a full apply graph since all objects are known. This means that an apply operation of a refresh plan should only entail returning the planned state.
We however do still need the walk to apply output changes which are recorded in the plan, and some vestigial cleanup is done during the apply walk. In the future, copying the planned output values and cleaning up the state could be done outside the walk, but we will leave that for later pending investigation into any other unexpected side effects it may cause.
For now to ensure refreshed checks are stored correctly, copy the planned checks into the final state after the apply walk.