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

Add validation for generated json metafiles #174

Merged

Conversation

kislerdm
Copy link
Contributor

@kislerdm kislerdm commented Jan 17, 2024

Resolves #152

What changed

  • Added the program which validates json metadata files for modules and providers by performing a set of checks. It reports all found errors to stderr and terminates with the exit code 1 if the file is invalid, otherwise, it terminates with the exit code 0 without any messages.

Why do we need it

  • To validate correctness of updated json metafiles

Examples

Provider

image

Module

image

@kislerdm kislerdm marked this pull request as draft January 17, 2024 23:37
@kislerdm
Copy link
Contributor Author

kislerdm commented Jan 17, 2024

Hey @janosdebugs @Yantrio @cam72cam! I'd appreciate your quick look over the change to see I'm going in the right direction with the implementation. Thanks!

@Yantrio
Copy link
Member

Yantrio commented Jan 18, 2024

I think the implemtnation here is pretty solid, but I think it may be best to have an external type of validation here instead of using the same code that we're generating with.

Maybe it could be as simple as piping it into JQ and ensuring it's valid json?

@janosdebugs you raised the initial issue, when you mention "valid" json, do you mean that it's valid json, or valid to our format and what we expected?

@ghost
Copy link

ghost commented Jan 18, 2024

@Yantrio as I wrote in the related issue, I would like the JSON to be valid from the perspective of the registry, ideally. However, this may also be covered by #164.

@kislerdm
Copy link
Contributor Author

@janosdebugs @Yantrio Hey folks! Thanks for the feedback. On its basis, I'll remove introduced code and setup a github action which will read modified metafile using jq, and implement the content validation as part of #164.

@ghost
Copy link

ghost commented Jan 18, 2024

To clarify: I'm not unhappy with a Go solution, it feels more robust than shell scripting around it. That being said, I don't know the registry very well, so I'll have to rely on the more experienced colleagues for judgement here.

@kislerdm kislerdm force-pushed the 152-add-validation-for-module-provider-json branch from e6c065a to 9b420b9 Compare January 30, 2024 15:49
Signed-off-by: Dmitry Kisler <[email protected]>
Signed-off-by: Dmitry Kisler <[email protected]>
Signed-off-by: Dmitry Kisler <[email protected]>
Signed-off-by: Dmitry Kisler <[email protected]>
Signed-off-by: Dmitry Kisler <[email protected]>
Signed-off-by: Dmitry Kisler <[email protected]>
@kislerdm kislerdm marked this pull request as ready for review January 30, 2024 22:39
@kislerdm
Copy link
Contributor Author

@janosdebugs @Yantrio hey guys! The validator is ready for review.

@Yantrio Yantrio requested review from Yantrio and a user January 31, 2024 10:29
Copy link
Member

@Yantrio Yantrio left a comment

Choose a reason for hiding this comment

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

thanks for this @kislerdm , I've added 2 tiny tiny nitpicks but overall im happy to approve.

.github/workflows/validate-json-module.yml Outdated Show resolved Hide resolved
.github/workflows/validate-json-provider.yml Outdated Show resolved Hide resolved
kislerdm and others added 2 commits January 31, 2024 14:41
Co-authored-by: James Humphries <[email protected]>
Signed-off-by: Dmitry Kisler <[email protected]>
Co-authored-by: James Humphries <[email protected]>
Signed-off-by: Dmitry Kisler <[email protected]>
@kislerdm
Copy link
Contributor Author

@Yantrio good catch, thank you James! 😄

@kislerdm
Copy link
Contributor Author

kislerdm commented Jan 31, 2024

@janosdebugs Hey Janos! Could you please review the changes? Thanks!

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Very nice and thorough work, thank you!

@Yantrio Yantrio merged commit 4b3824b into opentofu:main Feb 2, 2024
1 check passed
@kislerdm kislerdm deleted the 152-add-validation-for-module-provider-json branch February 2, 2024 13:11
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.

Check for valid JSON on PRs
2 participants