command/show: Disable plan state lineage checks

[alisdair]

When showing a saved plan, we do not need to check the state lineage against current state, because the plan cannot be applied. This is relevant when plan and apply specify a -state argument to choose a non-default state file. In this case, the stored prior state in the plan will not match the default state file, so a lineage check will always error.

Fixes #30195

[apparentlymart]

I see that you marked this a draft and so I'm responding here with some more woolly thoughts that I'm not intending as review so much as thinking aloud about how this command is currently designed, what the implications are of that, and thus what the implications of this change might be.

I spent a little time glaring at the terraform show code myself here, because I shared the intuition you spoke of in a different channel earlier that it feels weird that this command ought to need to go through the backend machinery at all, because everything it needs to do its work should in principle be neatly encapsulated inside the plan file.

Reading through the steps in that command (without your proposed modifications) I see the following high-level operations, focusing on the case where there's a plan file specified:

1. Load the plan file from disk. The odd indirection through Meta.PlanFile here seems questionable given that it makes the command silently ignore the given argument if it's a directory, rather than returning an error, but nonetheless it seems mostly harmless for what we're discussing here.
2. Construct a fake "backend operation" with the plan file along with the current configuration. Passing the current configuration here is very questionable, because a plan file has a configuration snapshot embedded inside of it, but that turns out not to matter because LocalRun in the next step will ignore ConfigDir if it's given a plan file.
3. Ask the backend to instantiate a LocalRun, which was a recent tweak I'd made to the backend API to avoid drastic changes after we shrank the scope of the terraform.Context API, so we'd still have somewhere to hang all of the implicitly-detected things that we used to do as part of creating a Terraform Core context. That does a bunch of work to fetch the latest state snapshot from the configured backend, but I think the main relevant behavior it's relying on here is that for historical reasons the backend is responsible taking the ContextOpts that was prepared earlier by Meta.backendCLIOpts, which includes the provider factories we'll need in a later step to fetch the schemas, and passing them in to terraform.NewContext to put a valid, functional terraform.Context in LocalRun.Core.
   • Note that if we pop down into Meta.providerFactories, which is ultimately responsible for building that set of factories, it's relying only on the dependency lock file on disk and not on the configuration or plan file at all. That's a relatively recent change; in v1.0 and earlier it'd been using a weird mix of dependency lock file, configuration, and state. It's a known issue that it uses the dependency lock file on disk rather than the one that's embedded in the plan file, but there's some logic somewhere (I wasn't able to find it quickly while walking through here) which verifies that the locks in the plan file match the locks on local disk, and so those two are equivalent as long as that check is happening in the terraform show codepath, which I didn't verify.
4. From the LocalRun's Core (a terraform.Context) we call terraform.Context.Schemas, which seems to be the main point of all of this: we need the schemas in order to render the plan, because the plan renderer relies on it for type information. That does finally use the configuration and state to figure out which provider schemas it should actually read, and so we do end up eventually getting the right answer, but as far as I can tell not relying on anything that came from the backend in particular; we just used the backend as a really heavy-handed way to construct a terraform.Context using information that command.Meta already knew.

With all of this said, I'm sure there are some lurking extra things that I skimmed over while I was reading here, because the process of backend instantiation, operation instantiation, and LocalRun creation is a lot of code. What you started to do in your PR here has the advantage of not greatly changing the architecture, with the Backend still being responsible for all the same things it was before, but of course at the cost of adding yet another weird argument to the not-so-well-encapsulated backend.Operation concept.

I think something shallow like you're attempting here is the best answer for a patch release, since significant refactoring would be too risky and disruptive, but based on my read above it does at least seem like there ought to be a variant of what terraform show does that skips the backend steps and instead instantiates a terraform.Context directly itself using information already known by Meta, and calls Schemas on that using the configuration and state serialized inside the plan file. I'm not trying to compel you to work on that, of course! But I wanted to share the results of my spelunking here in case it helps with a more tactical solution for now and also might give some ideas for a potential simplification we could pursue in a follow up PR targeted at a later minor release, if (and only if) you're interested in that.

[alisdair]

Thank you so much for that additional detail, Martin! I was struggling to find a way to rework the LocalRun interface, and hadn't considered taking the restructuring back to the command implementation itself. I'll continue to look into that and hopefully find a neater way forward.

I agree that this shallow fix is a reasonable one to consider for a patch release, so I'm marking this as ready for review now.

[apparentlymart]

This feels like a good, focused (and thus low-risk) solution to the problem for the v1.1 series!

I think it would be nice to try to tidy this up a bit in the main branch for v1.2 if we can (stuff like what we were discussing in the PR comments above) but the priority of doing that is independent of the priority of fixing this bug, so we can discuss that elsewhere. 😀

[github-actions]

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.