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

providers mirror doesn't use lock file #28274

Closed
ThinkChaos opened this issue Apr 2, 2021 · 6 comments · Fixed by #32742
Closed

providers mirror doesn't use lock file #28274

ThinkChaos opened this issue Apr 2, 2021 · 6 comments · Fixed by #32742
Assignees
Labels
bug explained a Terraform Core team member has described the root cause of this issue in code good first issue

Comments

@ThinkChaos
Copy link

Terraform Version

0.14.9 / 0.15.0-beta2

Expected Behavior

When running terraform providers mirror in a directory with a .terraform.lock.hcl, I'd expect the downloaded providers to be the ones specified in the lock file.

Actual Behavior

The downloaded providers are not the ones specified in the lock file.

Steps to Reproduce

cat > main.tf << EOF
terraform {
  required_version = ">= 0.14"
  required_providers {
    vault = {
      version = "2.18.0" # latest is 2.19.0
    }
  }
}
EOF

terraform providers lock

# Check the lock file contains 2.18.0
cat .terraform.lock.hcl

# Change the constraint to allow newer versions
sed -i 's/"2.18.0"/">= 2.18.0"/' main.tf

# Run `providers mirror` without updating lock file
terraform providers mirror .

# Check the downloaded version
# I'd expect to see 2.18.0 as specified by the lockfile but it's the latest version matching the constraint (2.19.0 at time of writing)
ls -l registry.terraform.io/hashicorp/vault/terraform-provider-vault_*.zip

Thoughts on Fix

I think there are some minor design questions to answer before this can be fixed:

What should happen when there is a lock file but some providers are not in it?

  1. Error: ask user to run providers lock
  2. Ignore the lock file for that provider
  3. Update the lock file

What should happen when the constraint and lock file conflict?

I imagine this would cause an error asking the user to update the lock file.

Besides those questions I believe the fix would look something like this:

In command/providers_mirror.go:

  1. load the lock file with c.lockedDependencies()
  2. check each constraint against the matching locked version
  3. check the locked version is available
  4. use the locked version to set selected (see line 141)

Disclaimer: I'm new to using TF and only took a quick look at the code, so I probably got some things wrong and am missing context.

@ThinkChaos ThinkChaos added bug new new issue not yet triaged labels Apr 2, 2021
@mildwonkey
Copy link
Contributor

Hi @ThinkChaos, thanks for submitting this!
The providers mirror command was originally intended (it might be more accurate to say "imagined") for use pre-init, where one was bundling providers to host (and use) elsewhere. It's main purpose is to use the configuration, so it can be used to pull providers for multiple platforms (and not just what's represented by the local lockfile). If you want to allow newer versions, it's best to specify that directly in the configuration.

I will re-label this as an enhancement request. Thanks again!

@mildwonkey mildwonkey added enhancement and removed bug labels Apr 5, 2021
@ThinkChaos
Copy link
Author

ThinkChaos commented Apr 6, 2021

Thank you for the response.

Reading the docs it is not clear to me that mirroring is meant to be done before init, and tutorials I could find on this subject also use it before (edit: after) init so I'm not alone with this misconception.
Maybe the docs could be clarified as to what purpose it serves and where in the TF cycle it fits?

It's main purpose is to use the configuration, so it can be used to pull providers for multiple platforms (and not just what's represented by the local lockfile).

I'd argue that the lockfile is part of the configuration. And it can contain multiple platforms if you run providers lock with multiple -platform arguments!

If you want to allow newer versions, it's best to specify that directly in the configuration.

The goal is the opposite, but I wasn't quite clear in the original comment so I'll explain a bit more.
When writing the TF configs on dev machines, a lockfile is generated. The configs get copied to a server that uses them to deploy other servers. Unfortunately, the deploying server sometimes fails to contact registry.terraform.io; this is where mirroring comes into play.

To mirror the providers, we use providers mirror -platform=linux_amd64 on the Windows dev machines. If a new version (that also matches the constraint) of a provider was published since we ran init -upgrade, then that version gets mirrored instead of the version we want (the one in the lockfile).
Then when the deploying server runs init (without -upgrade), it cannot find in the mirror we generated the version it expects and the deployment fails.

That's what the example I originally posted reproduces: a new version coming out after the lockfile is generated.

Now that I have a better understanding of the intended role of providers mirror, I see it is intentional it doesn't use the lockfile.
I still think making the command respect the lockfile when present would make generally more sense from a user perspective. And as it wouldn't break current intended usage (no behavior change when there's no lockfile) I see it as a net gain.

Anyways, our current workaround is to use specific versions in our required_providers so that both providers mirror and init resolve the same versions.
This is not ideal as it makes upgrading our providers harder compared to running init -upgrade.

@rkhaled0
Copy link
Contributor

Thank for raising it @ThinkChaos ,

Same for me, I thought that tf providers mirror honors the lock file, but seems not.

It would be great if it takes into consideration the lock file instead of pinning specific versions.

@ohcibi
Copy link

ohcibi commented Feb 10, 2023

It just does not make any sense at all to not respect the lockfile and also not providing any switch for it to enable/disable.

The problem is not only that the mirror download is not respecting the lock file, the problem is that the terraform init -plugin-dir=... command that is supposed to follow is respecting that lockfile and therefore complains about missing versions. The entire section in the documentation is rendered useless by this.

Enhancement label should be removed and replaced by documentation and bugfix labels.

@alisdair alisdair self-assigned this Feb 17, 2023
@alisdair
Copy link
Member

Thanks for all the feedback on this issue. We discussed the issue internally and would be happy to review a PR which meets the following design, which is very close to the proposals in this thread:


Update the terraform providers mirror command to support the dependency lock file. Specific requirements:

  • Load the configuration
  • Read the lock file
  • If the lock file is not present, consider only the configured constraints (same as current behaviour)
  • If the lock file is present:
    • Validate locks against configuration; if validation fails, present the normal "Inconsistent dependency lock file" diagnostic and exit
    • Keeping the same iteration over the configuration required providers, constrain the set of acceptable versions for each provider to the locked version
    • In this mode, output a log message to explain that we are using the lock file instead of a constraint: - Selected v4.54.0 to match dependency lock file

As with all PRs, this should include test coverage, which in the case of this command means adding a new e2e test case.


If anyone would like to work on such a PR, please do go ahead and link to it in this thread.

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug explained a Terraform Core team member has described the root cause of this issue in code good first issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants