hashicorp / terraform

Terraform enables you to safely and predictably create, change, and improve infrastructure. It is a source-available tool that codifies APIs into declarative configuration files that can be shared amongst team members, treated as code, edited, reviewed, and versioned.
https://www.terraform.io/
Other
41.68k stars 9.41k forks source link

.terraform.lock.hcl contains hashes for manifest.json files. #33599

Open sodul opened 11 months ago

sodul commented 11 months ago

Terraform Version

Terraform v1.4.6
on darwin_arm64

Terraform Configuration Files

not relevant, pretty much any workspace that includes hashicorp providers.

Debug Output

not relevant

Expected Behavior

The .terraform.lock.hcl file should only contain hashes of actual providers binaries.

Actual Behavior

The zh: hash for the manifest.json files are included.

provider "registry.terraform.io/hashicorp/consul" {
   version     = "2.18.0"
   constraints = "2.18.0"
   hashes = [
     "h1:ABHNF3vYoRej5vLiiKR/XYgjhjubsjlVROssc555Vj8=",
     "h1:GIbc9IimARunBuk+GdxgUX0cnZ2WwFW0IZMyT8E88B8=",
     "h1:fpHbgFUF+uyPCG+RLVJbTjpBHR8EZOxZkCh4e6VB7tY=",
     "zh:155f6899c8f1e0162169e4d09f426ea9a9738d74129f0ffc54b7570941c49cd4",
     "zh:241ba909b387c349845b0371321af45ccbb00290edbe1ea0861ad8732951f718",
     "zh:612f52be1fb5dd507b8064c67785313a531faad35873cba53f998f3766473335",
     "zh:67fbd695381b9c5db83ffeeaec1bfdbcc477ccb1e9de1caff76a0186d4c85908",
     "zh:7f88b151c9690a6addccbffe8484f0257344ef55424a9efb025dfddd052a4dc6",
     "zh:8d954f3ffeb72b6c18cc5ae8c3189bb3a8cb66967b2106c7d0163009c12bba15",
     "zh:913774c7eabc6e9078a1bd00347cc539b19a6f6b45dacbd21454dffdc9f4ae43",
     "zh:9517558883f994649695643a6208079ed0445aaa0ac2dee69f88cb044d21c6c9",
     "zh:a0211f596e35bd1b8d4bb9cda321cb1555a427a8d3f6724fe09893168fac9b7e",
     "zh:a70eaa0a88f677f901855a4ab908ddcf961e4afaf3a0147b08faaead57b4fe07",
     "zh:b02f5cd94ab236d988cfec531c56c199e3087803f1b2908a1a2b6da8a57b3751",
     "zh:f3d3efac504c9484a025beb919d22b290aa6dbff256f6e86c1f8ce7817e077e5",
   ]
 }

In this case the last hash is from https://github.com/hashicorp/terraform-provider-consul/releases/download/v2.18.0/terraform-provider-consul_2.18.0_SHA256SUMS

155f6899c8f1e0162169e4d09f426ea9a9738d74129f0ffc54b7570941c49cd4  terraform-provider-consul_2.18.0_freebsd_386.zip
241ba909b387c349845b0371321af45ccbb00290edbe1ea0861ad8732951f718  terraform-provider-consul_2.18.0_darwin_amd64.zip
612f52be1fb5dd507b8064c67785313a531faad35873cba53f998f3766473335  terraform-provider-consul_2.18.0_linux_arm.zip
67fbd695381b9c5db83ffeeaec1bfdbcc477ccb1e9de1caff76a0186d4c85908  terraform-provider-consul_2.18.0_freebsd_amd64.zip
7f88b151c9690a6addccbffe8484f0257344ef55424a9efb025dfddd052a4dc6  terraform-provider-consul_2.18.0_darwin_arm64.zip
8d954f3ffeb72b6c18cc5ae8c3189bb3a8cb66967b2106c7d0163009c12bba15  terraform-provider-consul_2.18.0_linux_386.zip
913774c7eabc6e9078a1bd00347cc539b19a6f6b45dacbd21454dffdc9f4ae43  terraform-provider-consul_2.18.0_freebsd_arm.zip
9517558883f994649695643a6208079ed0445aaa0ac2dee69f88cb044d21c6c9  terraform-provider-consul_2.18.0_windows_amd64.zip
a0211f596e35bd1b8d4bb9cda321cb1555a427a8d3f6724fe09893168fac9b7e  terraform-provider-consul_2.18.0_linux_arm64.zip
a70eaa0a88f677f901855a4ab908ddcf961e4afaf3a0147b08faaead57b4fe07  terraform-provider-consul_2.18.0_linux_amd64.zip
b02f5cd94ab236d988cfec531c56c199e3087803f1b2908a1a2b6da8a57b3751  terraform-provider-consul_2.18.0_windows_386.zip
f3d3efac504c9484a025beb919d22b290aa6dbff256f6e86c1f8ce7817e077e5  terraform-provider-consul_2.18.0_manifest.json

Steps to Reproduce

terraform init

Additional Context

This is not a very serious bug, but the fact is that the lock is referencing hash of files are not invalid. We noticed that while reviewing an update of our providers and that for hashicorp providers there was one zh: line that remained constant, but not for third party providers.

Something that terraform could do when parsing the SHA256SUM files is to filter out the non zip files. If it turns out that the manifest.json checksum are actually intentionally provided and used, then ignore this report.

References

No response

apparentlymart commented 11 months ago

Hi @sodul,

Terraform does not make any assumptions about what the filenames in your SHA256SUMS file might mean, or try to guess which ones are really packages and which ones aren't. Instead, Terraform expects that you will only include valid package checksums in that file, and exclude everything else.

Given that, I'm now wondering how you have ended up with a manifest file listed in your package checksums. Were you following some documentation that suggested you should do that? If so, I think we should correct that documentation so that the generated checksums file will be correct, rather than changing Terraform's behavior.

The filenames listed in the checksums file are not meaningful to Terraform and to start making assumptions about how they are named now would be a potential breaking change for existing published providers that might use different filenames (since the specific filenames chosen have been, so far, only a private detail chosen by the provider package build process).

sodul commented 11 months ago

@apparentlymart we do not have any custom logic for the checksum, we just call terraform providers lock to generate our .terraform.lock.hcl files.

I think the issue is that for the Hashicorp providers the *_SHA256SUMS file generated during the release include the _manifest.json file, and the registry blindly provides this back to terraform itself. Then terraform just trusts whatever the registry is claiming, which is fine I guess. Possibly the bug is in the registry where it could filter out the checksum for files that are not the actual provider binaries.

We have noticed this pattern with official Hashicorp providers, third party providers do not have that issue.

BTW, since we only care about a limited number of platforms it would be quite nice if terraform providers lock could avoid including the zh: hashes for all the non relevant platforms, that would take care of that extraneous _manifest.json checksum and limit storage and diff bloat.

apparentlymart commented 11 months ago

Ahh, sorry for the misunderstanding. I thought you were reporting that a provider you created was being treated in this way, but I see now that you were referring to hashicorp/consul as a concrete example, and that wasn't just a placeholder to avoid disclosing the true name of the provider.

In that case, I think the problem here is that the release process for the official providers seems to be incorrectly including the manifest file in the set of files it considers when calculating the SHA256SUMS file. The fix in that case would be to change the provider release process to fix this problem, although that is not something we'd change in this codebase because the release processes for the official providers belong to their own codebases. Since this slight misbehavior seems common across them all, I assume they all have some shared dependency that we could change to fix this for all of them at once, at least moving forward with new releases.

Thanks for reporting this, and sorry for the misunderstanding!


I do also acknowledge your addendum about only recording a subset of the checksums. Let's keep that a separate concern because it would require changing the provider registry protocol to explicitly distinguish between checksums for each target platform, rather than just reporting them all together with no indication of which is which.

(While as humans we can infer that the filename containing linux_amd64 is probably a package for linux_amd64, those names are not fixed as part of the protocol and so to depend on them would itself be a protocol change.)

The existing issue #27264 is already covering an improvement to the checksum management that would require changing the registry protocol -- to be able to return more than just zh:-type checksums -- so I think it's probably best to treat this as an additional requirement to consider when the registry team is ready to pick that up, so we can make just one protocol change that deals with both of these concerns at once.

radeksimko commented 4 months ago

I have filed https://github.com/hashicorp/ghaction-terraform-provider-release/issues/81 at the request of the team that takes care of releasing our providers, or setting the conventions for releasing them to be more specific.