Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

backend/local: Show resource instance drift as part of the plan #26921

Closed
wants to merge 1 commit into from

Conversation

apparentlymart
Copy link
Contributor

For another installment of "I had a little extra time at the end of the week", I noticed a few weeks ago that we now have sufficient information available during planning to detect and report drift explicitly, like we mocked in #15419. This is a prototype of that, mainly focused on seeing what the real UX might be like and how the problem has changed since I filed #15419.


When a user edits Terraform-managed objects outside of Terraform it can often prove confusing because Terraform indicates the need to change something even though the configuration hasn't changed.

We retain enough information in the state that we can actually detect when an object has "drifted" since the last apply, so now we'll include that information in the plan output both just to generally make it more visible (in case the drift wasn't actually intentional and so investigation is warranted) and also to potentially serve as an explanation for other changes that appear in the plan that follows.


One way this problem changed since I wrote up #15419 is that we've made sure that terraform plan will consider changes to root module outputs as needing to be applied, which was actually the main motivation for #15419, and we've also refined how data resources work so they get processed as part of planning now too.

Because of that, I no longer think it's important to be able to "apply" drift into the state in isolation, if that drift doesn't result in changes to the outputs. Applying it would not result in any significant observable side-effect. With that said, this prototype is therefore just a cosmetic UI thing and doesn't cause Terraform to consider drift as changes to be applied, only as extra context to present alongside changes to be applied.

When a user edits Terraform-managed objects outside of Terraform it can
often prove confusing because Terraform indicates the need to change
something even though the configuration hasn't changed.

We retain enough information in the state that we can actually detect when
an object has "drifted" since the last apply, so now we'll include that
information in the plan output both just to generally make it more visible
(in case the drift wasn't actually intentional and so investigation is
warranted) and also to potentially serve as an explanation for other
changes that appear in the plan that follows.
@apparentlymart apparentlymart self-assigned this Nov 14, 2020
Base automatically changed from master to main February 24, 2021 18:01
if statesShowDrift(baseState, priorState) {
// We've detected some drift by refreshing during our planning, then.
renderDrift(baseState, priorState, schemas, ui, colorize)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if we use this approach for rendering drift, there are two non-ideal consequences:

  • terraform plan -out=saved.tfplan and terraform show saved.tfplan will not render the same result (the drift will be dropped)
  • terraform show -json saved.tfplan does not have enough information to render drift

There are a couple of ways that I can think of handling this (either or both of which may be flawed):

  1. Extend the plan file format to store a set of objects similar to ResourceInstanceChange for the drifted resources, computing those from the diff between the two states, and then correspondingly update the plan renderer and JSON plan output
  2. Store the base state in the plan file along with the refreshed state, use those to render the plan

As a potential consumer of the JSON plan, I would much rather see option 1 if possible, so long as the resource_drift objects are semantically equivalent to the existing resource_changes objects. I can then reuse the same logic to do whatever I want with that information. If only option 2 is possible, I'd have to write new logic to diff the state files myself, which is possible but feels like more work.

@apparentlymart
Copy link
Contributor Author

The behavior I prototyped here will now be folded into #28634.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 6, 2021

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 you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants