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

Fixes #410: Add provider warnings #1123

Merged
merged 3 commits into from
Oct 14, 2024
Merged

Conversation

abstractionfactory
Copy link
Contributor

@abstractionfactory abstractionfactory commented Oct 11, 2024

This PR adds the ability to include warnings for providers. This will translate to the (undocumented) "warnings" field in the V1 API response, which OpenTofu processes. This PR also marks all HashiCorp providers that are archived as deprecated, indicating an official replacement where found/available.

Fixes #410: Add provider warnings
Fixes #751: Deprecate maas/terraform-provider-maas
Related to: opentofu/libregistry#39

Example output in the versions response:

{
  "versions": [
    //...
  ],
  "warnings": [
    "This provider is deprecated."
  ]
}

Fixes #751: Deprecate maas/terraform-provider-maas

Signed-off-by: AbstractionFactory <[email protected]>
@Yantrio
Copy link
Member

Yantrio commented Oct 11, 2024

I'll approve but im unsure about this failing check. It looks broken to me. can you look into it please @abstractionfactory ?

@abstractionfactory
Copy link
Contributor Author

@Yantrio just checked, that one is indeed broken and should be removed. It tries to validate the /versions endpoint against a schema for the individual versions. Safe to ignore.

@tom-reinders
Copy link
Contributor

tom-reinders commented Oct 11, 2024

@abstractionfactory Is not just the github action broken for multiple files?

This https://github.com/opentofu/registry/actions/runs/11287627799/workflow?pr=1123#L42 should just be

for path in ${CHANGED_FILES}; do

as per examples in actions README https://github.com/tj-actions/changed-files?tab=readme-ov-file#using-local-git-directory-

@abstractionfactory
Copy link
Contributor Author

@tom-reinders I don't believe so, I checked the verification application and it doesn't actually work when passed a versions endpoint file.

@tom-reinders
Copy link
Contributor

But the error in the action is trying to open all the files at ones in https://github.com/opentofu/registry/blob/main/src/cmd/validate/main.go#L93.
The only way for the validate command to get all path at ones in stead of seperated by the foreach is if the foreach is broken and it is.

@tom-reinders
Copy link
Contributor

This

          for path in "$CHANGED_FILES"
          do
            /tmp/validate/run provider "$path"
          done

Does the same as this

/tmp/validate/run provider "$CHANGED_FILES"

Anything in between the " is 1 item in the foreach for path in "one path" "second path" and because the action is putting the full content of CHANGED_FILES in between the first set of " it sees it as one path.

@abstractionfactory
Copy link
Contributor Author

So it's broken in more than one place. Wonderful. :D

@ollevche

This comment was marked as duplicate.

@ollevche
Copy link
Member

ollevche commented Oct 14, 2024

GitHub doesn't re-trigger provider validation because the latest commit has no changes to provider paths. Ideally, we would want to show final checks for the diff with the main (or other branch to merge to), because currently it is the way to bypass required validations.

Here is the throwaway commit to validate the check run just fine for this particular PR: https://github.com/opentofu/registry/actions/runs/11326232308/job/31494751924

@tom-reinders
Copy link
Contributor

If you update the command to accept multiple paths than also update the command call in the GitHub action from

for path in "$CHANGED_FILES"
do
  /tmp/validate/run provider "$path"
done

to

/tmp/validate/run provider "$CHANGED_FILES"

to prevent people from making the mistake of thinking that the for loop really works what it does not.

@ollevche
Copy link
Member

Copy link
Contributor

@tom-reinders tom-reinders left a comment

Choose a reason for hiding this comment

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

LGTM!

@abstractionfactory abstractionfactory merged commit 90e90c1 into main Oct 14, 2024
1 check passed
@abstractionfactory abstractionfactory deleted the provider-warnings branch October 14, 2024 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deprecate maas/terraform-provider-maas Add the ability to add provider warnings
5 participants