-
Notifications
You must be signed in to change notification settings - Fork 1
Add RFC for schema requirements in cluster apps #55
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
Changes from 2 commits
64bbe8c
c2bba6a
7284043
6c9dc6a
679a970
3a1eedb
636a28a
736151f
9c6e189
f4e634a
afe2672
296d9cd
25e0fa1
314d99e
57af1a0
c0b6eff
02fe1e1
3073423
ac661ee
9ce8eed
bdd7107
6dc44d8
70c15b4
e906823
1bb5ae9
a075697
0d6ba39
83f0c87
9630507
4720417
2b7e16e
9363c0b
9d9a8a5
d424fac
35031cb
4c0f5ae
7163aab
0a3d632
a55d389
c5fdaff
d2a2f35
211488e
172a47b
4b54af0
10637bc
521379e
650c139
d345df7
3ee3e0b
6a26e98
29e256a
0f8706d
6545e82
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,199 @@ | ||||||||||||||
| # Basic requirements for cluster app values schema | ||||||||||||||
|
|
||||||||||||||
| Cluster apps are apps which are used to configure and provision clusters based on Cluster API in the Giant Swarm platform. Like all helm charts, these apps are configured using so-called values YAML files, which are passed to the app as a ConfigMap for cluster creation. The allowed structure of the values YAML is defined by the app's values schema. | ||||||||||||||
|
|
||||||||||||||
| This RFC defines basic requirements for all cluster apps provided by Giant Swarm. | ||||||||||||||
|
|
||||||||||||||
| ## Overview | ||||||||||||||
|
|
||||||||||||||
| - [R1: JSON Schema dialect must be specified](#r1) | ||||||||||||||
| - [R2: Schema must explicitly cover all allowed properties](#r2) | ||||||||||||||
| - [R3: Array item schema must be defined](#r3) | ||||||||||||||
| - [R4: Properties must have a title](#r4) | ||||||||||||||
| - [R5: Properties should have descriptions](#r5) | ||||||||||||||
| - [R6: Properties should provide examples](#r6) | ||||||||||||||
| - [R7: Constrain values as much as possible](#r7) | ||||||||||||||
| - [R8: Required properties must be marked as such](#r8) | ||||||||||||||
|
|
||||||||||||||
| ## Background | ||||||||||||||
|
|
||||||||||||||
| TODO: Some more info regarding the rationale. | ||||||||||||||
|
|
||||||||||||||
| - In-place documentation of properties | ||||||||||||||
| - Better validation to catch configuration errors before they are applied | ||||||||||||||
| - Generated entry forms | ||||||||||||||
| - Generated information display | ||||||||||||||
| - Enabling processes and communication about explicit contracts instead of implicit behaviour | ||||||||||||||
|
|
||||||||||||||
| ## Definitions | ||||||||||||||
|
|
||||||||||||||
| - **Cluster app**: App (as defined by the Giant Swarm app platform) that allows for provisioning a cluster in a Giant Swarm installation. | ||||||||||||||
| - **JSON Schema**: declarative language that allows to annotate and validate documents that are compatible with JSON. | ||||||||||||||
| - The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this document are to be interpreted as described in [RFC 2119](https://www.rfc-editor.org/rfc/rfc2119.html) | ||||||||||||||
|
|
||||||||||||||
| ## Requirements | ||||||||||||||
|
|
||||||||||||||
| Requirements carry identifiers with prefix `R` and a unique number, for easy referencing. | ||||||||||||||
|
|
||||||||||||||
| ### R1: JSON Schema dialect must be specified {#r1} | ||||||||||||||
|
|
||||||||||||||
| Each cluster app's values schema file MUST specify the schema dialect (also called draft) used via the `$schema` keyword on the top level, with the draft URI as a value. | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The meaning and possible values of specific keywords changed over the course of different drafts. Supporting all drafts would increase the complexity of our tooling, e.g.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With the latest restrictions we discussed (e.g. forbid There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, if we make sure that we are indifferent to all the changes between 2019-09 and 2020-12.
|
||||||||||||||
|
|
||||||||||||||
| Example: | ||||||||||||||
|
|
||||||||||||||
| ```json | ||||||||||||||
| { | ||||||||||||||
| "$schema": "https://json-schema.org/draft/2020-12/schema", | ||||||||||||||
| ... | ||||||||||||||
| } | ||||||||||||||
| ``` | ||||||||||||||
|
|
||||||||||||||
| The example below shows an excerpt of a JSON Schema which uses draft 2020-12. | ||||||||||||||
marians marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||
|
|
||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add negative examples, or particularly recommendations regarding |
||||||||||||||
| ### R2: Schema must explicitly cover all allowed properties {#r2} | ||||||||||||||
|
|
||||||||||||||
| All properties that can be used in values MUST be covered in the schema explicitly. | ||||||||||||||
|
|
||||||||||||||
| To enforce this and to disable the use of any undefined properties, the keyword `additionalProperties` SHOULD be set to `false` on all object schemas. | ||||||||||||||
|
|
||||||||||||||
| The `patternProperties` keyword MUST NOT be used. | ||||||||||||||
marians marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||
|
|
||||||||||||||
| ### R3: Array item schema must be defined {#r3} | ||||||||||||||
|
|
||||||||||||||
| The items schema for all array properties must be defined using the `items` keyword. | ||||||||||||||
marians marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||
|
|
||||||||||||||
| ### R4: Properties must have a title {#r4} | ||||||||||||||
|
|
||||||||||||||
| Each property MUST be annotated via the `title` keyword. | ||||||||||||||
|
|
||||||||||||||
| Example: | ||||||||||||||
|
|
||||||||||||||
| ```json | ||||||||||||||
| { | ||||||||||||||
| ... | ||||||||||||||
| "properties": { | ||||||||||||||
| "name": { | ||||||||||||||
| "type": "string", | ||||||||||||||
| "title": "Cluster name", | ||||||||||||||
| ... | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| ``` | ||||||||||||||
|
|
||||||||||||||
| Rationale: These annotations (just like description and examples, see R5 and R6) are mainly useful for users during data entry, but also when reasoning about an cluster configuration. | ||||||||||||||
|
|
||||||||||||||
| Best practices: | ||||||||||||||
|
|
||||||||||||||
| - Ue sentence case. | ||||||||||||||
| - Do not use punctuation. | ||||||||||||||
| - Do not repeat the parent title. | ||||||||||||||
| - Example: if `.controlPlane` is titled `Control plane`, then `.controlPlane.availabilityZones` should be titled `Availability zones`, not `Control plane availability zones`. | ||||||||||||||
|
|
||||||||||||||
| ### R5: Properties should have descriptions {#r5} | ||||||||||||||
|
|
||||||||||||||
| Each property SHOULD be annotated and via the `description` keyword. | ||||||||||||||
|
|
||||||||||||||
| Example: | ||||||||||||||
|
|
||||||||||||||
| ```json | ||||||||||||||
| { | ||||||||||||||
| ... | ||||||||||||||
| "properties": { | ||||||||||||||
| "name": { | ||||||||||||||
| "type": "string", | ||||||||||||||
| "description": "Identifies this cluster uniquely within the installation.", | ||||||||||||||
| ... | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| ``` | ||||||||||||||
|
|
||||||||||||||
| Best practices: | ||||||||||||||
|
|
||||||||||||||
| - Do not repeat the property name or title in the description. | ||||||||||||||
| - Write descriptions between 50 and 200 characters long. | ||||||||||||||
| - Use simple language. | ||||||||||||||
| - Use no formatting syntax, nor hyperlinks. | ||||||||||||||
| - Use no line brakes. | ||||||||||||||
marians marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||
| - Use sentence case and punctuation. | ||||||||||||||
|
|
||||||||||||||
| ### R6: Properties should provide examples {#r6} | ||||||||||||||
|
|
||||||||||||||
| Each property SHOULD provide one or more examples using the `example` keyword. | ||||||||||||||
|
|
||||||||||||||
| Example: | ||||||||||||||
|
|
||||||||||||||
| ```json | ||||||||||||||
| { | ||||||||||||||
| ... | ||||||||||||||
| "properties": { | ||||||||||||||
| "name": { | ||||||||||||||
| "type": "string", | ||||||||||||||
| "examples": ["devel001"], | ||||||||||||||
| ... | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| ``` | ||||||||||||||
|
|
||||||||||||||
| Best practices: | ||||||||||||||
|
|
||||||||||||||
| - Ideally, provide one example. | ||||||||||||||
| - Provide more than one example if you want to highlight that very different values are accepted. | ||||||||||||||
| - Provide no more than five examples | ||||||||||||||
|
||||||||||||||
|
|
||||||||||||||
| TODO | ||||||||||||||
|
|
||||||||||||||
| - We could decide to use the examples for testing purposes, replacing ci-values.yaml and the likes. In that case, we should make it a MUST requirement. | ||||||||||||||
|
|
||||||||||||||
| ### R7: Constrain values as much as possible {#r7} | ||||||||||||||
|
|
||||||||||||||
| Property schema SHOULD explicitly restrict values as much as possible. | ||||||||||||||
|
|
||||||||||||||
| There are several ways this requirement can/has to be fulfilled, depending on the property type and the use case. | ||||||||||||||
|
|
||||||||||||||
| Properties of type `string`: | ||||||||||||||
|
|
||||||||||||||
| - Use of the keywords `minLength` and `maxLength` to specify the valid length. | ||||||||||||||
| - If applicable, use the `pattern` keyword to specify a regular expression for validation. This is useful for example to restrict the value to ASCII characters, or to lowercase etc. | ||||||||||||||
| - Use the `format` keyword to restrict the property to a common string format, like a URL or an email address. Watch out to use only formats which are available in the JSON schema dialect used (see also R1). | ||||||||||||||
|
|
||||||||||||||
| Numeric properties (type `number`, `integer`): | ||||||||||||||
|
|
||||||||||||||
| - Restrict the values range using `minimum` or `exclusiveMaximum` and `maximum` or `exclusiveMaximum`. | ||||||||||||||
marians marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||
| - Use `multipleOf` if the value must be a multiple of a certain number. | ||||||||||||||
|
|
||||||||||||||
| Example: | ||||||||||||||
|
|
||||||||||||||
| ```json | ||||||||||||||
| "diskSizeInGb": { | ||||||||||||||
| "type": "integer", | ||||||||||||||
| "minimum": 10, | ||||||||||||||
| "maximum": 200, | ||||||||||||||
| "multipleOf": 10 | ||||||||||||||
| } | ||||||||||||||
| ``` | ||||||||||||||
|
|
||||||||||||||
| ### R8: Required properties must be marked as such {#r8} | ||||||||||||||
|
|
||||||||||||||
| If a property is required to be set, it MUST be included in the `required` keyword. | ||||||||||||||
|
|
||||||||||||||
| In terms of a lifecycle, this means that the property must be defined when submitting the values for validation or for cluster creation. | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As we forbid empty objects, I suggest adding that to the RFC (related issue).
Suggested change
|
||||||||||||||
| ## TODO | ||||||||||||||
|
|
||||||||||||||
| I haven't gotten to these yet, or I'm not sure about them. | ||||||||||||||
|
|
||||||||||||||
| - Each property must declare the `type`. Not sure this is required by JSON schema. | ||||||||||||||
|
|
||||||||||||||
| - Use of the `default` keyword. As per JSON Schema documentation, it is for annotation only. It is not meant to automatically fill in missing values for validation. | ||||||||||||||
|
|
||||||||||||||
| - How to specify whether a property can be modified or not. `"readOnly": true` mit be the right one for that. | ||||||||||||||
|
|
||||||||||||||
| - Use of the `deprecated` keyword to phase out properties. Documentation says: "The deprecated keyword is a boolean that indicates that the instance value the keyword applies to should not be used and may be removed in the future." | ||||||||||||||
marians marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||
|
|
||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about "How to verify/format JSON schema files" (in your editor) |
||||||||||||||
| ## Resources | ||||||||||||||
|
|
||||||||||||||
| - [Understanding JSON Schema](https://json-schema.org/understanding-json-schema/) | ||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.