fix(terraform): support direct OpenTofu registry h1 hashes fetch (#9213) - #9611
prajaktaukirde wants to merge 5 commits into
Conversation
| } | ||
|
|
||
| urlVersions := fmt.Sprintf("%s://%s/v1/providers/%s/%s/versions", scheme, t.provider.Hostname, t.provider.Namespace, t.provider.Type) | ||
| req, err := http.NewRequestWithContext(context.Background(), "GET", urlVersions, nil) |
There was a problem hiding this comment.
Could I ask you to use the Updatecli custom httpclient from https://github.com/updatecli/updatecli/blob/main/pkg/core/httpclient/main.go
It will setup a few things like retry, user agent, etc.
There was a problem hiding this comment.
Hi @olblak, thank you for the feedback!
I have updated the implementation to import and use the custom Updatecli HTTP client (httpclient.NewRetryClient()) for querying the OpenTofu registry. The updated commit has been pushed to the branch.
loispostula
left a comment
There was a problem hiding this comment.
Built and tested this locally against registry.opentofu.org/hashicorp/random 3.9.0 (15
published platforms). The approach works: before, the lock has 1 h1 and tofu init
adds the other 14; with this branch updatecli writes all 15 and tofu init leaves them
alone. Fixes #9213.
1. Doesn't compile. Provider.Hostname is svchost.Hostname, not string:
main.go:230:23: cannot use t.provider.Hostname (variable of string type
svchost.Hostname) as string value in argument to strings.HasPrefix
Needs a string(...) conversion. CI never ran here (fork PR), hence not caught.
2. Empty hashes return as success. If packages is empty, the function returns
(nil, nil), the caller only falls back on err != nil, and Apply writes
hashes = [], emptying the block. Needs a len(uniqueHashes) == 0 guard.
3. spec.Platforms is silently ignored. Ran with platforms: [darwin_arm64], got
all 15. Probably the right lock file, but the field is documented as controlling this
and ReportConfig still reports it. Needs a log line and a docs update.
Minor:
- The localhost http downgrade (228-231) is unreachable in production, the only caller gates on the opentofu hostname. An unexported
registryBaseURLfield set inNew()would let the test inject instead, and makeshttpClientmockable. - Hostname is hardcoded, so self-hosted registries get nothing.
v.Platforms[0]is arbitrary, prefer an entry fromspec.Platforms.http.MethodGet, andslices.Sort/slices.Compactoversort.Stringsplus the manual dedup.
Tests cover the happy path only, and call the unexported function directly, so the routing condition in getProviderHashes is never exercised. Worth adding the hostname gate, non-200, and empty packages.
|
@prajaktaukirde Without pressure, would you have some time to look at @loispostula comment? |
Fix #9213
This PR fixes the issue where running UpdateCli against the OpenTofu provider registry (
registry.opentofu.org) only records the requested platform'sh1hashes. Because the OpenTofu registry servesh1hashes for all published platforms directly in its download metadata response (under thepackagesdictionary), a subsequent localtofu initexpands the lock file'sh1set to match the full platform list, leading to a perpetual mismatch and a "dirty" lock file.To resolve this, we:
registry.opentofu.org.packagesdictionary containing bothh1andzhhashes for all published platforms.tofu init's behavior exactly, avoiding any binary downloads.tfupdatedownloader behaviour in case of failure.Test
To test this pull request, you can run the following commands: