On Wed, Jan 19, 2022 at 4:06 PM Thomas Kettenbach <tkett...@gmail.com>
wrote:

> Hi,
>
> applying a jsonschema check for arbitrary json objects may be used upfront
> to ensure validity of those objects. One candidate could be the
> 'connect.json', so i thought i could give it a try. Please see below for an
> example jsonschema, as a first attempt.
>
> I would not want to create an jira issue for this, as it is really just an
> experiment.
>

I guess the only thing I don't like about this is that the raw json schema
is a poor documentation (it is way too spread out and verbose, compared to,
say, the markdown file I link below) and we don't (afaik) have a good way
to build and publish something readable from the json schema file itself.

If there was a generator of something readable, we could maybe check it
into the repository and then have a ci job that would compare output of the
tool with what is currently checked in there, to ensure it is kept up to
date, maybe?


> Example usage on empty json ( it's just '{}' ):
>
> $ jsonschema -i empty.json qpid-proton-connect-schema.json || echo "Invalid
> json"
> (no output )
> NOTE: empty json is valid to this schema, but it's not very useful.
>
> Example usage on an incorrect json:
>
> $ jsonschema -i broken2.json qpid-proton-connect-schema.json || echo
> "Invalid json"
> : '' is not of type 'integer'
> Invalid json
> NOTE: This check failed, as the file 'broken.json' has an empty string for
> 'heartbeat', according to the schema, that should be an integer.
> NOTE2: Also, the scheme used is 'http', which is not reflected in the
> schema, yet.
>
> BR,
> Thomas
>
>
> ## Appendix -1  file: qpid-proton-connect-schema.json
>
> $ cat qpid-proton-connect-schema.json
> {
>   "$schema": "http://json-schema/draft-07/schema#";,
>   "$id": "qpid-proton_connect-schema",
>   "title": "qpid-proton connect schema",
>   "description": "Schema for the 'connect.json' config file used by
> qpid-proton python binding.",
>   "type": "object",
>   "additionalProperties": true,
>

why not set this to false? to allow some "$comment" flexibility to users?


>   "required": [
>   ],
>   "properties": {
>     "scheme": {
>       "type": "string",
>       "description": "The AMQP scheme to use. Can be 'amqp' or 'amqps'"
>     },
>

the description here suggests an enum, so

    "scheme": {
      "description": "\"amqp\" (no TLS) or \"amqps\"",
      "type": "string",
      "enum": [
        "amqp",
        "amqps"
      ],
      "default": "amqps"
    },

(I am not entirely sure about the default value, but I'll check that later)


>     "host": {
>       "type": "string",
>       "description": "The hostname to connect to."
>     },
>     "heartbeat": {
>       "type": "integer",
>       "description": "Specify a heartbeat interval."
>     },
>

the markdown doc at
https://github.com/apache/qpid-proton/blob/main/docs/connect-config.md does
not have the "heartbeat" key documented


>     "sasl": {
>       "$ref": "#/definitions/sasl_properties"
>     },
>     "user": {
>        "type": "string",
>        "description": "Specify connecting username to AMQP service
> endpoint."
>     },
>     "password": {
>        "type": "string",
>        "description": "Specify the connecting users password."
>     },
>     "tls": {
>        "type": "object",
>        "$ref": "#/definitions/tls_properties"
>     }
>   },
>   "definitions": {
>      "sasl_properties": {
>        "type": "object",
>        "required": [ "sasl_enabled", "mechanisms" ],
>

from reading the markdown I'd think these are all optional, but I can't
argue with fresh experience actually using it


>        "properties": {
>          "sasl_enabled": {
>            "type": "boolean",
>            "description": "Indicates the usage of any SASL auth mechanism."
>          },
>          "mechanisms": {
>            "type": "string",
>            "description": "A space separated list of enabled SASL
> mechanisms."
>          }
>        }
>      },
>      "tls_properties": {
>        "type": "object",
>        "required": [ "ca", "cert", "key", "verify" ],
>

from reading the markdown I'd think these are optional too, but I guess
when I already have tls{} then in practice I have to provide all properties


>        "ca": {
>          "type": "string"
>        },
>        "cert": {
>          "type": "string"
>        },
>        "key": {
>          "type": "string"
>        },
>

all these fields can be, according to the markdown doc, also "null", so
doing "type": ["string", "null"] seems appropriate


>        "verify": {
>          "type": "boolean"
>        }
>     }
>   }
> }
>
> ## Appendix -2  file: broken.json
>
> {
>     "scheme": "http",
>     "host": "example.com",
>     "heartbeat": "",
>     "user": "foo",
>     "password": "bar"
> }


There is a documented requirement that when "scheme" is "amqp", then the
"tls" must not be present. This suggests to me adding

  "$comment": "not-required means forbidden in JSON Schema speak",
  "if": {
    "properties": {
      "scheme": {
        "const": "amqp"
      }
    }
  },
  "then": {
    "not": {
      "required": [
        "tls"
      ]
    }
  }

This way we will at least use some schema-7 features.
-- 
Mit freundlichen Grüßen / Kind regards
Jiri Daněk

Reply via email to