8000 Allow successful init when constraint matches at least one valid version by dsa0x · Pull Request #37137 · hashicorp/terraform · GitHub
[go: up one dir, main page]

Skip to content

Conversation

dsa0x
Copy link
Member
@dsa0x dsa0x commented May 20, 2025

This makes s slight change to the init command, so that any version from the registry that does not conform to terraform's semver constraints is skipped, allowing the command to use other versions if present. If there is no version deemed valid, the command still appropriately gets an error, but the error would likely tell the user that the registry has no version that matches the requested constraint.

Target Release

1.13.x

Rollback Plan

  • If a change needs to be reverted, we will roll out an update to the code within 7 days.

Changes to Security Controls

Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.

CHANGELOG entry

  • This change is user-facing and I added a changelog entry.
  • This change is not user-facing.

@dsa0x dsa0x force-pushed the sams/log-invalid-reg-vsns branch from 159f1a3 to a095350 Compare May 20, 2025 13:52
@dsa0x dsa0x marked this pull 8000 request as ready for review May 20, 2025 15:07
@dsa0x dsa0x requested a review from a team as a code owner May 20, 2025 15:07
@mildwonkey
Copy link
Contributor

What you do think about including a warning (in the main output, not just logging) when invalid responses were returned from the registry? Something like "the registry returned invalid response X for provider Z so we selected the best of the valid options" (but better words)? I'm picturing a user who expected a provider upgrade and doesn't understand why the version they can see in the registry isn't downloading (it's there in the logs, so this is really just a thought, not something we need immediately or at all!)

Copy link
Contributor
@mildwonkey mildwonkey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation looks solid, but I feel like terraform should emit a warning (not just log the error) when encountering invalid responses from the registry, so it's immediately obvious to the user that there's something going on even if terraform can continue. (Not a merge blocking thought, I'm curious what others thing)

@dsa0x
Copy link
Member Author
dsa0x commented May 20, 2025

What you do think about including a warning (in the main output, not just logging) when invalid responses were returned from the registry?

The issue with doing this is that if a provider has a large number of invalid versions, most of which were skipped, and thus having nothing to do with the final selected version, the output still contains a warning for those versions.

Also, I feel like the provider warnings we have today are indications of things that the user can fix, e.g upgrading to a non-deprecated version or resource. However, in this case, there is nothing the user can do, and they probably do not care, because their constraint still resolved to a version.

@jbardin
Copy link
Member
jbardin commented May 20, 2025

Warnings are usually emitted for something that the practitioner can take action on, but the registry database containing an invalid value isn't something that any Terraform user can fix.

The warnings in this part of the code are meant for a different use case, the registry can return messages about using a specific version, so the user can change the requirements set in the configuration.

The reasoning I gave @dsa0x, was that in the rare case of something failing in the registry which causes the expected version to be absent here, then it would be clear the failure is somewhere between Terraform and the provider release process. Checking the debug logs from the CLI, and contacting the registry would be the next steps regardless of the CLI output.

maybe we could overload the warnings from this code path, but it might not be worth the change.

@mildwonkey
Copy link
Contributor

That's fair, no objections from me!

@dsa0x dsa0x requested review from mildwonkey and a team May 20, 2025 16:09
@dsa0x dsa0x merged commit ede46c0 into main May 21, 2025
8 checks passed
@dsa0x dsa0x deleted the sams/log-invalid-reg-vsns branch May 21, 2025 07:06
Copy link
Contributor

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on th 84EA e 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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0