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

Add jsonschema dump of API protocols #13254

Open
phlax opened this issue Sep 24, 2020 · 23 comments
Open

Add jsonschema dump of API protocols #13254

phlax opened this issue Sep 24, 2020 · 23 comments
Labels

Comments

@phlax
Copy link
Member

phlax commented Sep 24, 2020

description

In order to create a vscode plugin (#13078) we need to dump the protobuf protocols to jsonschema

I have created a container for testing https://github.com/chrusty/protoc-gen-jsonschema - and have it installed and working there with the provided examples etc

Im not totally clear how/where to install this within bazel so it is available to protoc - and also how to trigger the actual protoc commands for the envoy API - any help/pointers with these tasks would be much appreciated

refs

@phlax phlax added enhancement Feature requests. Not bugs or questions. triage Issue requires triage labels Sep 24, 2020
@mattklein123 mattklein123 added area/docs help wanted Needs help! and removed enhancement Feature requests. Not bugs or questions. triage Issue requires triage labels Sep 24, 2020
@mattklein123
Copy link
Member

cc @htuch @lizan for bazel help.

@htuch
Copy link
Member

htuch commented Sep 25, 2020

I think you could structure this as a Bazel aspect, and then use an approach similar to

def api_proto_plugin_aspect(tool_label, aspect_impl, use_type_db = False):
, we trigger this in places like
bazel build "${BAZEL_BUILD_OPTIONS[@]}" @envoy_api_canonical//:"${API_VERSION}"_protos --aspects \
. Maybe easier to use something like https://github.com/stackb/rules_proto, which claims to have support for protoc plugins, this didn't exist when we were first developing protodoc.

One thing you might want to think about is if you can enhance this jsonschema plugin to take into account protoc-gen-validate constraints. It's probably not needed for a vscode plugin MVP, but ultimately might be useful.

@phlax
Copy link
Member Author

phlax commented Sep 25, 2020

One thing you might want to think about is if you can enhance this jsonschema plugin

so if i understand this correctly this would add constraints so eg:

if somewhere it specified a tcp/ip port, normally we would just be able to specify an int but with this we can ensure it was in the acceptable port range

@htuch
Copy link
Member

htuch commented Sep 25, 2020

@phlax right; we already have these constraints in the API protos in PGV format.

@dio
Copy link
Member

dio commented Sep 25, 2020

Internally, we use https://github.com/pseudomuto/protoc-gen-doc since it can generate "IR" of a proto as *.json, so we can transform it into whatever we want (I think making a jsonschema with additional information from this *.json is not hard, but yeah integrating it with the current build system probably a challenge). As a bonus, it also spits out information if a field is annotated by PGV: https://github.com/pseudomuto/protoc-gen-doc/tree/master/extensions/envoyproxy_validate (FWIW adding more extensions is not difficult, e.g. we needed to consider fields annotated by google.api.field_behavior).

@mattklein123
Copy link
Member

The one part of the JSON schema that I'm not sure how we are going to manage is all of the Any/typed_config stuff. Is this somehow possible within raw JSON schema or are we going to need something more complicated for vscode?

Basically, we want to know all of the built-in extensions and allow auto complete of typed_config, and then switch to the right schema for the sub-config.

@phlax
Copy link
Member Author

phlax commented Sep 25, 2020

somehow possible within raw JSON schema or are we going to need something more complicated for vscode?

its a while since i used it - but yep - i think it it has a refs system which iirc is for this kind of purpose.

if its not possible to do something directly with jsonschema i think we can mangle the output and typed_configs after

not sure enough yet how the protobuf to jsonschema plugin works which i guess is the key question

likewise - i need to clarify how multiple schemas can be combined, and whether they need to be

@phlax
Copy link
Member Author

phlax commented Sep 25, 2020

...in terms of vscode (as opposed to the typed_configs bit more generally) we have the kubernetes example in upstream, so that should be quite useful to template whats needed - i got it working today on my local install, which was nice

also - see comments about packaging in #13078

@phlax
Copy link
Member Author

phlax commented Sep 26, 2020

in terms of typed_config - this should be useful i think https://json-schema.org/understanding-json-schema/reference/conditionals.html

@phlax
Copy link
Member Author

phlax commented Sep 26, 2020

im thinking the best way to figure the typed_config pattern will be to create an example snippet - initially i guess just with yaml - and then write a jsonschema for it. We could then test that in vscode also to see how it works

from there we can work out how that translates in terms of protobuf -> jsonschema

ill grab a snippet shortly...

@phlax
Copy link
Member Author

phlax commented Sep 26, 2020

i created this repo for playing with snippets/schemas

https://github.com/phlax/typed-config-testing

@phlax
Copy link
Member Author

phlax commented Sep 26, 2020

so after some playing with jsonschema etc, it seems like we can specify a number of typed_config, and then include the schema accordingly - which i think was the minimum requirement.

afaict we cant include schema dynamically - ie enter a url in the "@type" field and it retrieves the schema from that url. Not sure if this is correct, but i couldnt see a way to do it.

the jsonschema pattern is something like this

{
    "properties": {
        "name": {
            "type": "string",
            "description": "The name of this config."
        },
        "typed_config": {
            "type": "object",
            "description": "Extensible typed config.",
            "allOf": [
                {"properties": {
                    "@type": {
                        "type": "string",
                        "description": "set me to change the config in this section",
                        "enum": [
                            "some.typed_config.type1",
                            "some.typed_config.type2"
                        ]
                    },
                    "common_property": {
                        "type": "string",
                        "description": "A property that is common to all config types."
                    },
                }},
                {"oneOf": [
                    {
                        "if": {
                            "properties": {
                                "@type": { "const": "some.typed_config.type1"}
                            }
                        },
                        "then": {
                            "$ref": "https://phlax.github.io/typed-config-testing/type1-schema.json#/definitions/type1Properties"
                        }
                    },
                    {
                        "if": {
                            "properties": {
                                "@type": { "const": "some.typed_config.type2"}
                            }
                        },
                        "then": {
                            "$ref": "https://phlax.github.io/typed-config-testing/type2-schema.json#/definitions/type2Properties"
                        }
                    },
                    {"$ref": "https://phlax.github.io/typed-config-testing/default-schema.json#/definitions/defaultProperties"}
                ]}
            ]
        }
}

in this example there are 2 config "types" - some.typed_config.type1 and some.typed_config.type2.

if you select either one it will load the defined schema and constrain that block

if you dont specify any "@type" or something thats not defined, it loads default properties/constraints

@htuch
Copy link
Member

htuch commented Sep 27, 2020

This looks like a great solution. I think static is fine; what we can do is ensure that for Envoy, we include all the built-in extension type URLs, and provide folks consuming Envoy the ability to specify additional typed configs at build time.

@mattklein123
Copy link
Member

Agreed this is awesome. This is really going to be amazing, especially when we get this everywhere we allow typed_config today which is quite a few places.

I think static is fine; what we can do is ensure that for Envoy, we include all the built-in extension type URLs, and provide folks consuming Envoy the ability to specify additional typed configs at build time.

+1. Once we get the POC working, we should figure out how to automate the entire process, so we guarantee that all extensions/schemas are emitted at build time so we don't miss anyway. Note this is related to #13167. It would be nice to fix this at roughly the same time. This way we will always show in the docs what typed_config values we support in the stock build.

Thank you @phlax this is great and highly appreciated!

@phlax
Copy link
Member Author

phlax commented Sep 28, 2020

The one part of the JSON schema that I'm not sure how we are going to manage is all of the Any/typed_config stuff.

iiuc the concern about Any, then i think that jsonschema "additionalProperties": true allows non-specified properties to be valid

@phlax
Copy link
Member Author

phlax commented Sep 28, 2020

one slightly tangential thought ive been having is about deprecated configs

ideally it would emit a warning, but not sure if/how this can be expressed in jsonschema - or how this would work in terms of autocomplete.

i guess it could just put a DEPRECATED: prefix in the property description

we could also have a strict mode that didnt allow deprecations

@phlax
Copy link
Member Author

phlax commented Sep 28, 2020

@Freyert
Copy link

Freyert commented Dec 29, 2021

@phlax if you would like an "alpha" tester I would love to try this out. Any pointers on getting it up and dumping that you can share? (I'm a total envoy novice).

@jcchavezs
Copy link
Contributor

jcchavezs commented Jan 19, 2022

I just created this tool for generating the json schemas from proto https://github.com/jcchavezs/envoy-config-schema. Does that look like something you would like to adopt?

cc @dio

@unicell
Copy link

unicell commented Jan 21, 2022

@jcchavezs Would it be more useful if you publish json schemas for all resources in a single file I think? That way it'll be easier to configure in IDE / Editor for validation.

@jcchavezs
Copy link
Contributor

Hi @unicell that soudns reasonable, my only question would be: do we need all schemas? As of now I am just publishing Bootstrap schemas for v2 and v3 because that was mainly what I wanted but if you could confirm what are the schemas you are looking for here I could make that work.

@phlax
Copy link
Member Author

phlax commented Jan 24, 2022

@jcchavezs i got quite far with this (ie generating vscode schemas)

i hit a few snags mangling the proto info, but mostly got it working

see #15100 for my earlier WIP attempts

@unicell
Copy link

unicell commented Jan 26, 2022

@jcchavezs My main use case is to config validation, auto completion. Basically what vscode-yaml can offer, but I'm using coc-yaml (the coc fork of it).

Currently I'm using https://gist.githubusercontent.com/linux-china/66da28383a096a4675a44f1d212ba41e/raw/60f351d268d14ebcd128caae3e280425005b58c5/envoy-schema.json but that's not versioned.

I'm hoping to have full Envoy jsonschema available in a single file, versioned tracked. Similar to https://github.com/yannh/kubernetes-json-schema Notice that it has schema for each Kubernetes release, and it has all.json as well as individual resource types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants