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
Add support of Yaml Schema #318
Conversation
This adds support to the loadSchema function to attempt loading of a schema file containing YAML formatted schemata. Signed-off-by: Yevhen Vydolob <yvydolob@redhat.com>
…rver into yaml-scheme
Signed-off-by: Yevhen Vydolob <yvydolob@redhat.com>
} | ||
|
||
try { | ||
const schemaContent: JSONSchema = yaml.safeLoad(content); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you are doing a simple conversion then I'm concerned that it might not work with k8s yaml schemas. The structure of which has the schema held within it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joshuawilson I'm not sure that understand your point, can you add sample of such k8s yaml scheme?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is a link to the Knative Eventing Subscription schema. Notice that is has structure wrapping the actual schema.
https://github.com/knative/eventing/blob/master/config/core/resources/subscription.yaml
The schema needed for validation doesn't start until line 32. If we include the rest, will validation actually work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, because that file itself is not YAML Scheme, it is k8's CRD. To get actual scheme you need to extract that yaml in to separate yaml file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a fine, I was just hoping to be able to point to the URL for a k8s schema and use it. But I do understand that has become a more generic tool.
This PR based on #191, with resolving merge conflicts and tests.