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

refactor(jsonschema): move schema generator from types to here #1219

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

wass3r
Copy link
Collaborator

@wass3r wass3r commented Oct 30, 2024

go-vela/types has been made obsolete. it was responsible for generating the jsonschema that allows for things like IDE validation feedback on Vela YAML pipelines. it was generated via https://github.com/go-vela/types/tree/main/cmd/schema and utilized github.com/alecthomas/jsonschema. schemastore hosts a reference to the published artifact.

github.com/alecthomas/jsonschema has been archived in favor of github.com/invopop/jsonschema, so we're taking the opportunity to upgrade. in the process we're moving from jsonschema draft 4 (2013) to jsonschema 2020-12 (2022) as that is the spec supported by the new library. there are no substantial differences in the generated schema.

the struct tags do most of the heavy lifting for determining the final schema, but there are some necessary customziations due to custom types/unmarshaling we utilize. the library provides some means of doing so by adding certain methods to structs to hook into the generation process. overall, it seems a bit cleaner.

summary of changes

  • move schema generation to this repo
  • change to use github.com/invopop/jsonschema
  • instead of one function handling customization, utilizing provided hooks
  • created separate schema package for generating the schema on the fly and to potentially leverage in other places like the CLI for pipeline validation
  • added tests and mechanism to validate known good/bad pipelines against schema
  • modified CI to utilize the above (see test run)

some notes

jsonschema package

i took the opportunity to look around for alternatives to see what was out there. i think i would have liked to explore using https://github.com/swaggest/jsonschema-go although it doesn't support 2020-12 yet. most notably, it drops the need for separate jsonschema struct tag, instead relying on json and other "direct" tags - the invopop/jsonschema maintainers even reference that as something they wish to move toward.

jsonschema validation

i was thinking about adding in validation. the most notable package for this is https://github.com/santhosh-tekuri/jsonschema (and we're using their provided CLI for the tests that are ran via shell), however it can currently only read in actual files. you provide it with a filename reference which can be a remote URL or local file and tries to resolve it.

i found https://github.com/kaptinlin/jsonschema which seems to address that aspect, but it looks almost too new. however, i did try to use it but ran into some issues where it would claim some valid pipelines were invalid.

i decided to not pursue built-in validation at this time, but it might be worth considering in the future. especially when it comes to validating pipelines using the schema in the Vela CLI. there, we could use the previous library by writing a temp file of the schema (or loading the remote, published schema) and loading the pipeline YAML to validate. generating schema in memory and passing that to the validator seems a bit nicer than utilizing file system or pulling it in remotely 🤷🏼

schemastore

once merged and new schema is published we need to provide a PR to https://github.com/SchemaStore/schemastore to provide the new link. we could fast track a little by generating and uploading schema to the last release manually for now since we just reference the latest release for the schema.json artifact 🤔

Copy link

codecov bot commented Oct 30, 2024

Codecov Report

Attention: Patch coverage is 19.28251% with 180 lines in your changes missing coverage. Please review.

Project coverage is 56.62%. Comparing base (f463fc9) to head (d31e911).

Files with missing lines Patch % Lines
compiler/types/yaml/ruleset.go 0.00% 74 Missing ⚠️
compiler/types/yaml/ulimit.go 0.00% 22 Missing ⚠️
compiler/types/yaml/volume.go 0.00% 22 Missing ⚠️
compiler/types/raw/map.go 0.00% 17 Missing ⚠️
compiler/types/raw/slice.go 0.00% 14 Missing ⚠️
compiler/types/yaml/secret.go 0.00% 13 Missing ⚠️
compiler/types/yaml/stage.go 0.00% 9 Missing ⚠️
schema/pipeline.go 82.69% 6 Missing and 3 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1219      +/-   ##
==========================================
- Coverage   56.87%   56.62%   -0.26%     
==========================================
  Files         599      600       +1     
  Lines       32856    33079     +223     
==========================================
+ Hits        18687    18730      +43     
- Misses      13535    13712     +177     
- Partials      634      637       +3     
Files with missing lines Coverage Δ
compiler/types/yaml/template.go 73.68% <ø> (ø)
compiler/types/yaml/stage.go 91.00% <0.00%> (-9.00%) ⬇️
schema/pipeline.go 82.69% <82.69%> (ø)
compiler/types/yaml/secret.go 87.74% <0.00%> (-8.04%) ⬇️
compiler/types/raw/slice.go 77.77% <0.00%> (-22.23%) ⬇️
compiler/types/raw/map.go 77.88% <0.00%> (-15.22%) ⬇️
compiler/types/yaml/ulimit.go 78.21% <0.00%> (-21.79%) ⬇️
compiler/types/yaml/volume.go 77.31% <0.00%> (-22.69%) ⬇️
compiler/types/yaml/ruleset.go 63.00% <0.00%> (-37.00%) ⬇️

@wass3r wass3r changed the title refactor(jsonschema): move jsonschema generator from types to here refactor(jsonschema): move schema generator from types to here Oct 30, 2024
@wass3r wass3r marked this pull request as ready for review November 1, 2024 01:54
@wass3r wass3r requested a review from a team as a code owner November 1, 2024 01:54
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