chore: consider switching JSON-schema dependency to github.com/santhosh-tekuri/jsonschema

[thaJeztah]

Description

• related: chore: consider switching JSON-schema dependency to github.com/santhosh-tekuri/jsonschema compose-spec/compose-go#749

The github.com/xeipuuv/gojsonschema module used for JSON-schema validation has not been maintained for a long time, and because of that, ended on the "avoid" list in kubernetes; kubernetes/kubernetes#128572 (comment)

The OCI image-spec has now transitioned to a new dependency (github.com/santhosh-tekuri/jsonschema) as replacement in v1.1.1, and we should look if we can follow so that we don't end up with two separate dependencies performing the same task;

• Switch jsonschema validation libraries opencontainers/image-spec#1189

[wilkice]

Hi, I am trying to implement this. But now has a problem. This new dependency: github.com/santhosh-tekuri/jsonschema, will fail current schema test. Because we are defining a dict type in test file

cli/cli/compose/schema/schema_test.go

Line 13 in 2eec746

type dict map[string]any

The new schema validator take dict as an invalid type. Detail see santhosh-tekuri/jsonschema#215.

So I am wondering can we replace all dict in https://github.com/docker/cli/blob/master/cli/compose/schema/schema_test.go to map[string]any. This way we can transfer to new validator dependency.

Also we can wait until https://github.com/santhosh-tekuri/jsonschema fix this, but not sure how long it will takes.

If changing test is ok, I can make PR for this transfer to new dependency.

[thaJeztah]

Thanks for looking at this! In all honesty, I haven't looked closely yet how easy the transition would be (and if the new one provides all functionality we need); I saw the move of other projects to this new module, and (as we tend to have common dependencies between those projects), wanted to explore if we could switch as well.

If you're interested, it may also be worth looking at compose-spec/compose-go#749 first; the compose-spec is a more modern codebase, and if we want to switch, we likely want to make sure the new dependency works for that project (if it does, we can switch docker/cli to match).

cc @ndeloof @glours ☝️ if you agree, that is (I'm not a core maintainer of the compose-spec, so let me know if you think that makes sense)

if changing test is ok, I can make PR for this transfer to new dependency.

I guess this depends a bit; does the new module provide an alternative to maintain existing behavior, and to still have a test covering what's tested? (If so, then I think updating the test should not be a problem).

[ndeloof]

👍

[wilkice]

Thanks for replying.

I guess this depends a bit; does the new module provide an alternative to maintain existing behavior, and to still have a test covering what's tested? (If so, then I think updating the test should not be a problem).

the new module need to modify it's behavior. In many case, provided json data is ok but validator fail. I am trying a PR to fix this. As the switch is still on discussing, So I think currently the change of test code is not needed.

I saw the move of other projects to this new module, and (as we tend to have common dependencies between those projects), wanted to explore if we could switch as well.

got it. I will do some experiment to see if switch is ok on compose-spec.

[wilkice]

Hi @thaJeztah, as the author says he won't fix this issue. From my view, this may lead to some problem later when validating using named type.

So I am favor of not switching to this module, and will not work on this issue.

santhosh-tekuri/jsonschema#215 (comment)