Check lock file diagnostics in mirror command

[jbardin]

The lock file diagnostic were not being checked in the mirror command, so an incomplete or broken lock file might cause the cli to crash.

Add -lock-file flag to providers mirror command, so that a user can proceed to create a mirror with an incomplete lock file when running init is not an option.

Fixes #35318

[apparentlymart]

This does seem plausible, but since we added support for using the lock file in #32742 it seems like we have created ourselves a bit of a chicken/egg problem that would be exacerbated by this change.

The original goal of terraform providers lock was to be an escape hatch for someone who has configured Terraform CLI to exclusively use a mirror for provider installation, so that they could use this command to update their mirror with new packages taken from their origin registries. Without that extra step, terraform init would fail because the provider isn't yet available in the mirror.

If we were to merge this change it seems like we'd break that use-case, because any newly-added provider would be absent from the lock file, terraform providers mirror would fail telling the user to run terraform init -upgrade, and then terraform init -upgrade would fail because the necessary provider package isn't available in the mirror yet.

#32742 already slightly broke that use-case as we saw in #35318, but the change proposed in this PR seems like it would break it entirely.

I can think of a couple different compromises that we could take from here:

• Make terraform providers mirror command use the lock file opportunistically when it contains an entry specifically for the provider under consideration, but to treat any problems with the dependency lock file as if the relevant entry in the file wasn't present at all.

• Do what you proposed in this PR, but also add a new option to terraform providers mirror that restores the pre-bugfix: terraform providers mirror command should honor terraform lock file #32742 behavior of ignoring the lock file entirely so that someone who is using that command with the goal it was originally intended for can use the new option to force it to work. Perhaps terraform providers mirror -upgrade would be a plausible name for it, to match the option of the same name on terraform init that also effectively means "ignore the lock file".

  (If we take this option then we should probably mention it as an option in the error message about the lock file being inconsistent. I'm not sure though whether we should prioritize terraform init -upgrade or terraform providers mirror -upgrade as the primary solution listed first, since of course it depends on what the person was trying to achieve. The latter solution is more suitable for someone using terraform providers mirror for its original intended purpose.)

[jbardin]

Thanks @apparentlymart, that matches my what I was looking into myself. I was intending this to work like the former of your options, but inspecting more closely it seems that the lock file was not expected to be there or complete.

I don't think this patch makes things any worse, the error it's surfacing would happen regardless, it just guards the panic. I guess you could say it might "work" in that the mirror files would be created, but terraform would still exit with an error. There's not really any good way to deal with a partial lock file however, without making this an explicit option somehow.

[apparentlymart]

Ah yes, I suppose you are right that this is just replacing the panic with an error.

I was imagining that the current code would allow you to change the version constraint for a provider that's already tracked in the lock file and then use terraform providers mirror to update the mirror to contain a package that matches the new version constraint, because the consistency check result was ignored.

But of course that doesn't hold because the subsequent logic would still use the selection from the dependency lock file anyway, so you'd end up just populating the mirror with whatever version was previously selected regardless of what's in the configuration. Although that case would now also be an error, the error isn't actually replacing a useful behavior.

[jbardin]

I'm kind of inclined to add an option to use the lock file, rather than add an option to ignore it. Seeing how we've had both behaviors in releases already, either change is breaking in some way. I do think we need to add an option of some sort, so I guess it's just a matter of choosing between reverting to the original intended behavior along with something like -use-lock-file, or keeping the current behavior with a -ignore-lock-file flag 🤔

[jbardin]

OK, proposing that we add a -lock-file flag, which defaults to true in order to maintain the current behavior. This can be now overridden with -lock-file=false. If this looks good, I'll add the docs and help text output to match.

[nfagerlund]

• Tested this to reproduce the bug, verify that the "diags not panic" change works, and verify that the new flag switches lock behaviors. All looks solid!
• I approve the new flag name, and I think the default makes sense.

[github-actions]

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

[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.