oam-dev / spec

Open Application Model (OAM).
https://oam.dev
Other
3.03k stars 246 forks source link

Question regarding schema's schema #485

Open lachieh opened 1 year ago

lachieh commented 1 year ago

I was recently experimenting with validating an OAM ApplicationConfiguration file and thought it best to use the actual schema to validate it. I'm using ajv to validate in the browser and I pulled the schema json file from this repo. When running a validation for the first time, I noticed that ajv was having a bit of difficulty parsing the schema. Specifically, it didn't know how to handle this section: https://github.com/oam-dev/spec/blob/e0991c5d0e666333ebbfa19f3490320bf671d8c0/schema/application_configuration.schema.json#L57-L81

Are the variables, scopes and components supposed to be nested inside of a properties object? Making this change locally resolves the issue for ajv, though I'm not sure if this is an issue with the schema file's schema, or with ajv itself.

Additionally, the $id is not a valid URI (it's missing a / after http:) though I'm not sure if this is critical.

https://github.com/oam-dev/spec/blob/e0991c5d0e666333ebbfa19f3490320bf671d8c0/schema/application_configuration.schema.json#L3

I'm happy to contribute a PR with these changes.

lachieh commented 1 year ago

In doing a bit more investigation, I believe there a some changes to the schema file that need to be made.

According to JSONSchema Draft 07, the additionalProperties field on should be a fully valid JSONSchema on its own, but at the moment it looks like it is a list of properties. additionalProperties is for validating extra keys of any name.

  1. definitions.operationalConfigurationSpec should be a valid JSONSchema. This means:
    • adding "type": "object", "required": [] and "additionalProperties": true (this last one could be false to disallow extra fields on spec though looking at other examples in the world, this would break compatibility)
    • adding a "properties" object, and moving variables, scopes and components inside this new object
  2. remove spec.additionalProperties
  3. adding spec.$ref and set the value to "#/definitions/operationalConfigurationSpec"
Expand for diff of changes ```diff { "$schema": "http://json-schema.org/draft-07/schema#", "$id": "http://oam.dev/v1/oam.application_configuration.schema.json", "title": "Open Application Model Application Configuration JSON Schema", "type": "object", "properties": { "apiVersion": { "type": "string", "description": "The specific version of the Open Application Model specification in use. This version of the specification covers apiVersions in core.oam.dev/v1alpha1." }, "kind": { "type": "string", "description": "For an application configuration schematic, must be ApplicationConfiguration.", "enum": ["ApplicationConfiguration"] }, "metadata": { "type": "object", "description": "Application configuration metadata.", "properties": { "name": { "type": "string" }, "labels": { "type": "object", "description": "A set of string key/value pairs used as arbitrary labels on this application configuration.", "additionalProperties": { "type": "string" } }, "annotations": { "type": "object", "description": "A set of string key/value pairs used as arbitrary annotations on this application configuration.", "additionalProperties": { "type": "string" } } } }, "spec": { "type": "object", "description": "A container for exposing application configuration attributes.", - "$ref": "#/definitions/operationalConfigurationSpec" + "additionalProperties": { + "$ref": "#/definitions/operationalConfigurationSpec" + } } }, "required": ["apiVersion", "kind", "metadata", "spec"], "additionalProperties": false, "definitions": { "operationalConfigurationSpec": { + "type": "object", + "required": [], + "additionalProperties": true - "variables": { - "type": "array", - "description": "Variables that can be referenced in parameter values and properties.", - "items": { - "$ref": "#/definitions/opconfigVariable" - } - }, - "scopes": { - "type": "array", - "description": "Application scope definitions.", - "items": { - "$ref": "#/definitions/applicationScope" - } - }, - "components": { - "type": "array", - "description": "Component instance definitions.", - "items": { - "$ref": "#/definitions/componentInstance" - } - } + "properties": { + "variables": { + "type": "array", + "description": "Variables that can be referenced in parameter values and properties.", + "items": { + "$ref": "#/definitions/opconfigVariable" + } + }, + "scopes": { + "type": "array", + "description": "Application scope definitions.", + "items": { + "$ref": "#/definitions/applicationScope" + } + }, + "components": { + "type": "array", + "description": "Component instance definitions.", + "items": { + "$ref": "#/definitions/componentInstance" + } + } + }, }, "opconfigVariable": { "type": "object", "description": "The Variables section defines variables that may be used elsewhere in the application configuration. The variable section provides a way for an application operator to specify common values that can be substituted into multiple other locations in this configuration (using the [fromVariable(VARNAME)] syntax).", "properties": { "name": { "type": "string", "description": "The parameter's name. Must be unique per configuration.", "$comment": "Some systems have upper bounds for name length. Do we limit here?", "maxLength": 128 }, "value": { "type": "string", "description": "The scalar value." } }, "required": ["name", "value"], "additionalProperties": false }, "applicationScope": { "type": "object", "description": "The scope section defines application scopes that will be created with this application configuration.", "properties": { "name": { "type": "string", "description": "The name of the application scope. Must be unique to the deployment environment." }, "type": { "type": "string", "description": "The fully-qualified GROUP/VERSION.KIND name of the application scope." }, "properties": { "type": "object", "description": "The properties attached to this scope.", "additionalProperties": { "$ref": "#/definitions/propertiesObject" } } }, "required": ["name", "type"], "additionalProperties": false }, "componentInstance": { "type": "object", "description": "This section defines the instances of components to create with this application configuration.", "properties": { "componentName": { "type": "string", "description": "The name of the component to create an instance of." }, "instanceName": { "type": "string", "description": "The name of the instance of this component." }, "parameterValues": { "type": "array", "description": "Overrides of parameters that are exposed by the application scope type defined in 'type'.", "items": { "$ref": "#/definitions/parameterValue" } }, "traits": { "type": "array", "description": "Specifies the traits to attach to this component instance.", "items": { "$ref": "#/definitions/trait" } } }, "required": ["componentName", "instanceName"], "additionalProperties": false }, "parameterValue": { "type": "object", "description": "Values supplied to parameters that are used to override the parameters exposed by other types.", "properties": { "name": { "type": "string", "description": "The name of the component parameter to provide a value for.", "$comment": "Some systems have upper bounds for name length. Do we limit here?", "maxLength": 128 }, "value": { "type": "string", "description": "The value of this parameter." } }, "required": ["name"], "additionalProperties": false }, "trait": { "type": "object", "description": "The trait section defines traits that will be used in a component instance.", "properties": { "name": { "type": "string", "description": "The name of the trait instance. Must be unique to the deployment environment." }, "properties": { "type": "object", "description": "Overrides of parameters that are exposed by the trait type defined in 'type'.", "additionalProperties": { "$ref": "#/definitions/propertiesObject" } } }, "required": ["name"], "additionalProperties": false }, "propertiesObject": { "type": "object", "description": "A properties object (for trait and scope configuration) is an object whose structure is determined by the trait or scope property schema. It may be a simple value, or it may be a complex object.", "properties": {}, "additionalProperties": true } } } ```
lachieh commented 9 months ago

Any input here? Would it be beneficial to recreate the JSON Schema file from the latest version of the spec?