-
Notifications
You must be signed in to change notification settings - Fork 10
add json schema for model config #48
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
base: main
Are you sure you want to change the base?
Conversation
Would you add a schema for all the spec? It makes sense to add them in one shot, better having a script to auto-generate the go files. |
HI, @bergwolf Thank you for reviewing the PR!
Are you referring to the manifest specification? Initially, I thought yes, but then I wondered given that the manifest spec itself follows the OCI manifest format, would it be redundant? Hope I understand the question correctly. 🙇 |
@caozhuozi I'm mostly considering auto-generating .go files from the json schema. Can we do it w/ the current schema? |
I'm not sure if this behavior is consistent with the current OCI image-spec. I'll look into it and confirm in the following comment. Im also thinking generating a JSON schema from Go structs might also be a reasonable alternative? What do you think? |
I've checked out the image-spec JSON schema files, and it looks like they were added manually. There doesn't seem to be any auto generation in both directions. For example, see the commit history of config-schema.json: https://github.com/opencontainers/image-spec/commits/main/schema/config-schema.json The only automation thing I've found related to the JSON schema files is a validation process. If you look at the Makefile and the spec_test.go, you can see it validates some example JSONs against the schema using the github.com/santhosh-tekuri/jsonschema/v5 package (see https://github.com/opencontainers/image-spec/blob/main/schema/validator.go#L27). Of course, I'm not saying generating code wouldn't be a good idea. ;-) |
@caozhuozi I see. Thanks for the details! Then, could you add the validation tests so that we make sure the |
@bergwolf |
6b1334c
to
379215b
Compare
Signed-off-by: caozhuozi <543481992@qq.com>
fix: #46
NOTE for Reviewers:
image-spec
repo primarily focuses on (example) validation and Go testing.This PR attempts to align with that validation and testing logic from image-spec.
Go files introduced
To mirror the structure of the
image-spec
repo, I’ve introduced the following Go files with some minor adjustments based on my own understanding:model-spec
image-spec
schema/schema.go
schema/schema.go
schema/validator.go
schema/validator.go
schema/config_test.go
schema/config_test.go
schema/validator.go
to test the config json schema with a variety of intentionally common mistakes and edge-cases config JSON examplesschema/example_test.go
schema/spec_test.go
note:
schema/spec_test.go
(from image-spec) toschema/example_test.go
in this repo. In image-spec,spec_test.go
validates the examples embedded in Markdown files, but the name is a bit ambiguous. The renaming helps clarify the purpose of the file here.layout
.NOTE for the JSON Schema file
For the JSON Schema definition in
config-schema.json
, I have embedded validation rules directly within the schema to make it self-contained and ensure consistent validation behavior across different languages.This differs from
image-spec
, where additional validation rules are implemented in Go code (schema/validator.go
) rather than encoded in the JSON schema definition itself.For example, there is an environment variable regex validation in image-spec for the config. This validation is performed in Go code:: https://github.com/opencontainers/image-spec/blob/main/schema/validator.go#L180-L185
But this validation is not explicitly defined in the JSON Schema:
https://github.com/opencontainers/image-spec/blob/main/schema/config-schema.json#L41-L45
I don't think it is a good idea. Thats why I changed it in this repo.
Validation Rules Enforced in This Schema
I embedded the following validation rules directly into the JSON Schema:
descriptor
andmodelfs
are requiredconfig
is optionaldescriptor
:name
andversion
are requiredmodelfs
:type
must be exactly"layers"
(enforced via"enum"
)diff_ids
must be a non-empty array (enforced via"minItems": 1
)