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

Migrate to santhosh-tekuri/jsonschema #168

Merged
merged 11 commits into from
Jan 23, 2023
Merged

Conversation

yannh
Copy link
Owner

@yannh yannh commented Jan 22, 2023

This migrates the JSON validation from https://github.com/xeipuuv/gojsonschema (unmaintained) to https://github.com/santhosh-tekuri/jsonschema . The "error messages with json-pointers to exact location" feature seems quite nice. It would be nice to also be able to solve xeipuuv/gojsonschema#322

Unit & Acceptance tests pass nicely but I've added a test for using the "non standalone" schemas (that contain HTTP references) and that currently fails. Tracking here santhosh-tekuri/jsonschema#102

While it would be a breaking change, Kubeconform uses the standalone files by default, and I am not sure if anyone uses the non-standalone schemas nor why someone would use those with Kubeconform, so I might release this as a breaking change, unless the feature gets implemented in jsonschema.

@yannh
Copy link
Owner Author

yannh commented Jan 22, 2023

FYI @eyarz since I know you're using kubeconform quite a bit - this is swapping the kubeconform engine 🥳 tests pass fine (despite minor differences in error messages) and performance seems on par. Just a heads up if you can think of something that might break - so far it seems a perfectly good replacement. We'd be using a better maintained JSON schema validation lib, which might enable new features.

@yannh
Copy link
Owner Author

yannh commented Jan 22, 2023

I wonder if they have interest in adding HTTP references https://github.com/santhosh-tekuri/jsonschema/blob/56baa8bb9ba78e8da42c113f788b8b17b2565f15/loader.go#L32

@eyarz
Copy link
Contributor

eyarz commented Jan 23, 2023

@yannh thank you for the heads up!
we will also check it from our side and keep you posted if we will find something.

@yannh yannh changed the title Try migrating to new JSON validation library Migrate to santhosh-tekuri/jsonschema Jan 23, 2023
@yannh
Copy link
Owner Author

yannh commented Jan 23, 2023

@eyarz I think this is good to merge now, passes all tests and additional tests I added. There might be slight differences in the output of errors. I won't make a new release just yet 👍

@yannh yannh merged commit ee7c498 into master Jan 23, 2023
@eyarz
Copy link
Contributor

eyarz commented Jan 24, 2023

yes, this change will break how we parse and view kubeconform output.
we pinned kubcomform go version so it will not break Datree CLI.

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.

2 participants