Skip to content

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

caozhuozi
Copy link
Contributor

@caozhuozi caozhuozi commented Apr 12, 2025

fix: #46

NOTE for Reviewers:

  1. I was unable to find any auto generation involved from json schema to Go structs or vice versa in image-spec.
  2. The logic related to the schema in the 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 Role
schema/schema.go schema/schema.go Go metadata definition for the schema
schema/validator.go schema/validator.go Go utility package to validate JSONs
schema/config_test.go schema/config_test.go Use schema/validator.goto test the config json schema with a variety of intentionally common mistakes and edge-cases config JSON examples
schema/example_test.go schema/spec_test.go Markdown examples validation

note:

  1. I renamed schema/spec_test.go (from image-spec) to schema/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.
  2. I’ve taken most of the code directly from the image-spec and temporarily removed any unused code, such as those related to other specs like 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:

  • Top-level fields:
    • descriptor and modelfs are required
    • config is optional
  • In descriptor:
    • Both name and version are required
    • Other fields are optional
  • In modelfs:
    • type must be exactly "layers" (enforced via "enum")
    • diff_ids must be a non-empty array (enforced via "minItems": 1)

@bergwolf
Copy link
Contributor

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.

@caozhuozi
Copy link
Contributor Author

HI, @bergwolf Thank you for reviewing the PR!

Would you add a schema for all the spec?

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. 🙇

@bergwolf
Copy link
Contributor

bergwolf commented Apr 21, 2025

@caozhuozi I'm mostly considering auto-generating .go files from the json schema. Can we do it w/ the current schema?

@caozhuozi
Copy link
Contributor Author

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?

@caozhuozi
Copy link
Contributor Author

caozhuozi commented Apr 21, 2025

@bergwolf

I'm mostly considering auto-generating .go files from the json schema. Can we do it w/ the current schema?

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. ;-)

@bergwolf
Copy link
Contributor

@caozhuozi I see. Thanks for the details! Then, could you add the validation tests so that we make sure the specs-go aligns with the schema? It also ensures that we involve the spec and schema together in future changes.

@caozhuozi
Copy link
Contributor Author

@bergwolf
Sure. I will.

@caozhuozi caozhuozi changed the title add json schema for model config [WIP] add json schema for model config May 2, 2025
@caozhuozi caozhuozi force-pushed the jsonschema branch 7 times, most recently from 6b1334c to 379215b Compare May 3, 2025 03:42
Signed-off-by: caozhuozi <543481992@qq.com>
@caozhuozi caozhuozi changed the title [WIP] add json schema for model config add json schema for model config May 4, 2025
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.

Do we need a json schema?
2 participants