From 64bbe8cfe9863522c8eb0cbbd7a6f7a6224f973c Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Fri, 25 Nov 2022 16:53:58 +0100 Subject: [PATCH 01/53] Add RFC for basic schema requirements in cluster apps --- cluster-app-values-schema/README.md | 188 ++++++++++++++++++++++++++++ 1 file changed, 188 insertions(+) create mode 100644 cluster-app-values-schema/README.md diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md new file mode 100644 index 00000000..b723b95a --- /dev/null +++ b/cluster-app-values-schema/README.md @@ -0,0 +1,188 @@ +# 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. + +## 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. + +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. + +### 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. + +### R3: Array item schema must be defined {#r3} + +The items schema for all array properties must be defined using the `items` keyword. + +### 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. +- 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`. +- 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. + + +## 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." + +## Resources + +- [Understanding JSON Schema](https://json-schema.org/understanding-json-schema/) From c2bba6aa0f900e9da9ae2d23c3cb76a4122e935d Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Thu, 1 Dec 2022 14:07:21 +0100 Subject: [PATCH 02/53] Add TOC --- cluster-app-values-schema/README.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index b723b95a..4d42fe9b 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -4,6 +4,17 @@ Cluster apps are apps which are used to configure and provision clusters based o 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. From 7284043c75be9fda3a9bef1151cb19d99832a737 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Thu, 8 Dec 2022 16:13:47 +0100 Subject: [PATCH 03/53] Maintain uppercase MUST --- cluster-app-values-schema/README.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 4d42fe9b..90fbf98b 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -60,7 +60,7 @@ The `patternProperties` keyword MUST NOT be used. ### R3: Array item schema must be defined {#r3} -The items schema for all array properties must be defined using the `items` keyword. +The items schema for all array properties MUST be defined using the `items` keyword. ### R4: Properties must have a title {#r4} @@ -162,7 +162,7 @@ Properties of type `string`: Numeric properties (type `number`, `integer`): - Restrict the values range using `minimum` or `exclusiveMaximum` and `maximum` or `exclusiveMaximum`. -- Use `multipleOf` if the value must be a multiple of a certain number. +- Use `multipleOf` if the value has to be a multiple of a certain number. Example: @@ -181,7 +181,6 @@ If a property is required to be set, it MUST be included in the `required` keywo In terms of a lifecycle, this means that the property must be defined when submitting the values for validation or for cluster creation. - ## TODO I haven't gotten to these yet, or I'm not sure about them. From 6c9dc6aeb17b86acb9b9c80529968b43a4a79e5e Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Thu, 8 Dec 2022 16:14:40 +0100 Subject: [PATCH 04/53] Cahnge "below" to "above" --- cluster-app-values-schema/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 90fbf98b..5c714511 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -48,7 +48,7 @@ Example: } ``` -The example below shows an excerpt of a JSON Schema which uses draft 2020-12. +The example above shows an excerpt of a JSON Schema which uses draft 2020-12. ### R2: Schema must explicitly cover all allowed properties {#r2} From 679a970cc95f31555ac98a3841ea9b051fcce1f5 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Thu, 8 Dec 2022 16:15:25 +0100 Subject: [PATCH 05/53] Change "brakes" to "breaks" --- cluster-app-values-schema/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 5c714511..30b420ba 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -115,7 +115,7 @@ Best practices: - Write descriptions between 50 and 200 characters long. - Use simple language. - Use no formatting syntax, nor hyperlinks. -- Use no line brakes. +- Use no line breaks. - Use sentence case and punctuation. ### R6: Properties should provide examples {#r6} From 3a1eedbb714b99320ea43a5be25f84221feb27f2 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Thu, 8 Dec 2022 20:57:47 +0100 Subject: [PATCH 06/53] Mention enum to list valid values --- cluster-app-values-schema/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 30b420ba..854cdcf6 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -158,6 +158,7 @@ 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). +- Use `enum` if only certain known values can be valid. Numeric properties (type `number`, `integer`): From 636a28a3eee64c9309f54c456dbfc03954f4b3d4 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Thu, 8 Dec 2022 21:23:13 +0100 Subject: [PATCH 07/53] Extended R8 on required properties --- cluster-app-values-schema/README.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 854cdcf6..ecdac948 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -176,11 +176,15 @@ Example: } ``` -### R8: Required properties must be marked as such {#r8} +### R8: Required properties {#r8} -If a property is required to be set, it MUST be included in the `required` keyword. +If a property is required to be set by the creator of a cluster, 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. +Properties that have default values defined in the app's `values.yaml` file MUST NOT be set as required, as here the default values are applied in case no user value is given. + +Properties that get default values assigned via some external mechanism (e.g. an admission controller) also MUST NOT be set as required. An example here would be the name of a cluster or node pool, where a unique name would be generated in case none is given. + +Note: If property of type object named `a` has required properties, this does not indicate that `a` itself must be defined in the payload. However it indicates that if `a` is defined, the required properties must be defined, too. ## TODO From 736151fe49438b7c64ed031541132d1aa1b43fd9 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Thu, 8 Dec 2022 21:26:14 +0100 Subject: [PATCH 08/53] Replace payload with instance --- cluster-app-values-schema/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index ecdac948..d6cf30cb 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -184,7 +184,7 @@ Properties that have default values defined in the app's `values.yaml` file MUST Properties that get default values assigned via some external mechanism (e.g. an admission controller) also MUST NOT be set as required. An example here would be the name of a cluster or node pool, where a unique name would be generated in case none is given. -Note: If property of type object named `a` has required properties, this does not indicate that `a` itself must be defined in the payload. However it indicates that if `a` is defined, the required properties must be defined, too. +Note: If property of type object named `a` has required properties, this does not indicate that `a` itself must be defined in the instance. However it indicates that if `a` is defined, the required properties must be defined, too. ## TODO From 9c6e189eb4106e409e65d8ac76d6355d00b399ed Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Thu, 8 Dec 2022 21:44:15 +0100 Subject: [PATCH 09/53] Update TODO list --- cluster-app-values-schema/README.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index d6cf30cb..43cf9c85 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -190,13 +190,15 @@ Note: If property of type object named `a` has required properties, this does no 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. +- Each property must declare the `type`. Pretty sure that this is required by JSON schema. + +- Should schemas have the `$id` property defined? And if yes, to what? - 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." +- 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.". Additional information could be given via a `$comment` in the right place. ## Resources From f4e634a4c9a20eaf1fd7a3c8c17bfcdd59556a84 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Mon, 12 Dec 2022 12:25:17 +0100 Subject: [PATCH 10/53] Add requirement regarding anyOf/oneOf --- cluster-app-values-schema/README.md | 32 +++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 43cf9c85..b1cd786f 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -186,6 +186,38 @@ Properties that get default values assigned via some external mechanism (e.g. an Note: If property of type object named `a` has required properties, this does not indicate that `a` itself must be defined in the instance. However it indicates that if `a` is defined, the required properties must be defined, too. +### R9: Avoid `anyOf` and `oneOf` {#r9} + +A cluster app schema SHALL NOT make use of the `anyOf` or `oneOf` keyword. + +If using `anyOf` or `oneOf` cannot be avoided, the desired subschema SHOULD be the first in the sequence. + +In JSON Schema, the `anyOf` and `oneOf` keyword defines an array of possible subschemas for a property. For a user interface which is generated based on the schema, this creates a high degree of complexity. + +At Giant Swarm, our user interface for cluster creation will not support `anyOf` nor `oneOf` to full extent. Instead, we are going to select only one of the defined schemas for data input, using simply the first schema that is not deprecated. + +Let's consider this example schema: + +```json +"replicas": { + "anyOf": [ + { + "type": "string", + "deprecated": true, + "$comment": "Supported until v1.23.4" + }, + { + "type": "integer", + "maximum": 100, + "title": "Size" + }, + {...} + ] +} +``` + +Here, the user interface would use the second subschema (type: integer) and ignore all others. + ## TODO I haven't gotten to these yet, or I'm not sure about them. From afe26720cf991b4b7fa1088abebb00141a696a31 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Tue, 10 Jan 2023 13:30:32 +0100 Subject: [PATCH 11/53] Add link to R9 --- cluster-app-values-schema/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index b1cd786f..b2f9ed86 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -14,6 +14,7 @@ This RFC defines basic requirements for all cluster apps provided by Giant Swarm - [R6: Properties should provide examples](#r6) - [R7: Constrain values as much as possible](#r7) - [R8: Required properties must be marked as such](#r8) +- [R9: Avoid `anyOf` and `oneOf`](#r9) ## Background From 296d9cd5ee50223b390727c8bcb7da4642eff1e5 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Tue, 10 Jan 2023 13:42:20 +0100 Subject: [PATCH 12/53] Add R10 on how to use deprecated=true --- cluster-app-values-schema/README.md | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index b2f9ed86..c79eb7e3 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -15,6 +15,7 @@ This RFC defines basic requirements for all cluster apps provided by Giant Swarm - [R7: Constrain values as much as possible](#r7) - [R8: Required properties must be marked as such](#r8) - [R9: Avoid `anyOf` and `oneOf`](#r9) +- [R10: Use `deprecated` to phase out properties](#r10) ## Background @@ -205,7 +206,7 @@ Let's consider this example schema: { "type": "string", "deprecated": true, - "$comment": "Supported until v1.23.4" + "$comment": "to be removed in the next major version, please use the integer type instead" }, { "type": "integer", @@ -219,6 +220,19 @@ Let's consider this example schema: Here, the user interface would use the second subschema (type: integer) and ignore all others. +## R10: Use `deprecated` to phase out properties {#r10} + +To indicate that a property is supposed to be removed in a future version, the property SHOULD carry the `deprecated` key with the value `true`. + +As a consequence, user interfaces for cluster creation SHALL NOT display the according form field for data entry. However, providing the property within values YAML will not cause any failure. + +In addition, it is RECOMMENDED to add a `$comment` key to the property, with information regarding + +- which property will replace the deprecated one (if any) +- when the property will be removed + +Note that `$comment` content is not intended for display in any UI nor processing in any tool. It is mainly targeting schema developers and anyone using the schema itself as a source of information. + ## TODO I haven't gotten to these yet, or I'm not sure about them. @@ -231,8 +245,6 @@ I haven't gotten to these yet, or I'm not sure about them. - 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.". Additional information could be given via a `$comment` in the right place. - ## Resources - [Understanding JSON Schema](https://json-schema.org/understanding-json-schema/) From 25e0fa142430b7a792aa087209b4fb9fa03b32ce Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Tue, 10 Jan 2023 13:50:38 +0100 Subject: [PATCH 13/53] Clarify description requirements --- cluster-app-values-schema/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index c79eb7e3..07576b88 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -96,6 +96,8 @@ Best practices: Each property SHOULD be annotated and via the `description` keyword. +Description content MUST NOT include any line breaks. Also it MUST NOT contain formatting code like e.g. Markdown or HTML. + Example: ```json @@ -116,8 +118,6 @@ 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 breaks. - Use sentence case and punctuation. ### R6: Properties should provide examples {#r6} From 314d99e0e8962069103f5089829a12cbef088ac4 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Tue, 10 Jan 2023 13:50:49 +0100 Subject: [PATCH 14/53] Remove line on patternProperties --- cluster-app-values-schema/README.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 07576b88..b9d1e17f 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -58,8 +58,6 @@ All properties that can be used in values MUST be covered in the schema explicit 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. - ### R3: Array item schema must be defined {#r3} The items schema for all array properties MUST be defined using the `items` keyword. From 57af1a08a2fa9cb2b320a67df9ea30812824c8e5 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Tue, 10 Jan 2023 13:51:30 +0100 Subject: [PATCH 15/53] Fix exclusiveMaximum -> exclusiveMinimum --- cluster-app-values-schema/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index b9d1e17f..4d43b9f7 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -162,7 +162,7 @@ Properties of type `string`: Numeric properties (type `number`, `integer`): -- Restrict the values range using `minimum` or `exclusiveMaximum` and `maximum` or `exclusiveMaximum`. +- Restrict the values range using `minimum` or `exclusiveMinimum` and `maximum` or `exclusiveMaximum`. - Use `multipleOf` if the value has to be a multiple of a certain number. Example: From c0b6eff35cee0f1ae645c4f185042ac63cd72f61 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Tue, 10 Jan 2023 13:53:40 +0100 Subject: [PATCH 16/53] Require draft 2020-12 --- cluster-app-values-schema/README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 4d43b9f7..2ba1d5a9 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -6,7 +6,7 @@ This RFC defines basic requirements for all cluster apps provided by Giant Swarm ## Overview -- [R1: JSON Schema dialect must be specified](#r1) +- [R1: JSON Schema dialect (draft 2020-12) 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) @@ -37,10 +37,12 @@ TODO: Some more info regarding the rationale. Requirements carry identifiers with prefix `R` and a unique number, for easy referencing. -### R1: JSON Schema dialect must be specified {#r1} +### R1: JSON Schema dialect (draft 2020-12) 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. +The draft URI MUST BE `https://json-schema.org/draft/2020-12/schema`. + Example: ```json @@ -50,8 +52,6 @@ Example: } ``` -The example above shows an excerpt of a JSON Schema which uses draft 2020-12. - ### R2: Schema must explicitly cover all allowed properties {#r2} All properties that can be used in values MUST be covered in the schema explicitly. From 02fe1e1d620e44878e7dd3fd0eaa1acf050a9a2d Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Wed, 11 Jan 2023 08:11:01 +0100 Subject: [PATCH 17/53] Add TODO --- cluster-app-values-schema/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 2ba1d5a9..935930ea 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -237,6 +237,8 @@ I haven't gotten to these yet, or I'm not sure about them. - Each property must declare the `type`. Pretty sure that this is required by JSON schema. +- How to label values, e. g. in the case of the service priority label. See https://github.com/giantswarm/roadmap/issues/1181#issuecomment-1359344451 + - Should schemas have the `$id` property defined? And if yes, to what? - 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. From 30734231763ed4c7da3742dfb632547f8d4aa0be Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Mon, 16 Jan 2023 14:13:37 +0100 Subject: [PATCH 18/53] Update and improve requirements regarding title (R4) --- cluster-app-values-schema/README.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 935930ea..7b963d57 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -66,6 +66,8 @@ The items schema for all array properties MUST be defined using the `items` keyw Each property MUST be annotated via the `title` keyword. +The `title` keyword provides a user friendly label for the property, to be displayed in a user interface instead of the technical property name. The title should be chosen to match the wording used in documentation, especially in prosa text (as opposed to configuration examples). + Example: ```json @@ -81,14 +83,12 @@ Example: } ``` -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: +Additional requirements apply to the title value: -- 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`. +- The title MUST be written using sentence case capitalization. Right: "Cluster name", Wrong: "Cluster Name". +- The title MUST NOT contain punctuation marks. +- If the title annotates a property that is part of another object, the title SHOULD NOT include the parent property name, to avoid repetition. + - Example: with an object `/controlPlane` that is titled `Control plane`, the property `/controlPlane/availabilityZones` should be titled `Availability zones`, not `Control plane availability zones`. ### R5: Properties should have descriptions {#r5} From ac661eeb79f76d81811b14edef7cc450d14f0d1c Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Mon, 16 Jan 2023 14:19:02 +0100 Subject: [PATCH 19/53] Update description requirements (R5) --- cluster-app-values-schema/README.md | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 7b963d57..c1fe49a1 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -85,7 +85,7 @@ Example: Additional requirements apply to the title value: -- The title MUST be written using sentence case capitalization. Right: "Cluster name", Wrong: "Cluster Name". +- The title MUST be written using sentence case capitalization. Correct: "Cluster name", incorrect: "Cluster Name". - The title MUST NOT contain punctuation marks. - If the title annotates a property that is part of another object, the title SHOULD NOT include the parent property name, to avoid repetition. - Example: with an object `/controlPlane` that is titled `Control plane`, the property `/controlPlane/availabilityZones` should be titled `Availability zones`, not `Control plane availability zones`. @@ -94,7 +94,7 @@ Additional requirements apply to the title value: Each property SHOULD be annotated and via the `description` keyword. -Description content MUST NOT include any line breaks. Also it MUST NOT contain formatting code like e.g. Markdown or HTML. + Example: @@ -111,12 +111,14 @@ Example: } ``` -Best practices: +If a description is given, additional requirements apply to the value: -- Do not repeat the property name or title in the description. -- Write descriptions between 50 and 200 characters long. -- Use simple language. -- Use sentence case and punctuation. +- Description content MUST NOT include any line breaks. +- Descriptions MUST NOT contain formatting code like e.g. Markdown or HTML. +- Descriptions MUST use sentence case capitalization and punctuation. +- The description SHOULD NOT repeat the property name or title. Correct: "Identifies this cluster uniquely within the installation.", incorrect: "Cluster name identifies this cluster uniquely within the installation.". +- Descriptions SHOULD be between 50 and 200 characters long. +- Descriptions SHOULD be written in simple language. ### R6: Properties should provide examples {#r6} From 9ce8eed058dabff9edb88ba0d5249e6d275a8e40 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Mon, 16 Jan 2023 14:19:28 +0100 Subject: [PATCH 20/53] Fix: example -> examples --- cluster-app-values-schema/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index c1fe49a1..4aeadc38 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -122,7 +122,7 @@ If a description is given, additional requirements apply to the value: ### R6: Properties should provide examples {#r6} -Each property SHOULD provide one or more examples using the `example` keyword. +Each property SHOULD provide one or more examples using the `examples` keyword. Example: From bdd7107b76ff5338a8674a03d319a85198a8cb92 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Tue, 17 Jan 2023 12:48:23 +0100 Subject: [PATCH 21/53] Add R11 on providing distinct string values --- cluster-app-values-schema/README.md | 33 +++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 4aeadc38..2c0c29ec 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -16,6 +16,7 @@ This RFC defines basic requirements for all cluster apps provided by Giant Swarm - [R8: Required properties must be marked as such](#r8) - [R9: Avoid `anyOf` and `oneOf`](#r9) - [R10: Use `deprecated` to phase out properties](#r10) +- [R11: Provide valid string values where possible](#r11) ## Background @@ -233,6 +234,38 @@ In addition, it is RECOMMENDED to add a `$comment` key to the property, with inf Note that `$comment` content is not intended for display in any UI nor processing in any tool. It is mainly targeting schema developers and anyone using the schema itself as a source of information. +## R11: Provide valid string values where possible {#r11} + +For string properties, in some cases only a few values are considered valid. In this case, the schema SHOULD specify these selectable values in one of the following forms: + +1. Using `enum` in case the selectable value does not require a more user-friendly label. +2. Using `oneOf` with a combination of `title` and `const`, in case a user-friendly label is needed in addition to the value. See below for more details. + +As an example of the `enum` method, we use a string property with two possible values. The values `active` and `inactive` are considered to be speaking for themselves. + +```json +"autoRefresh": { + "type": "string", + "title": "Auto refresh", + "enum": ["active", "inactive"] +} +``` + +However, if the string values are not considered self-explanatory, the `oneOf` method can be used to provide user-friendly labels in addition to the axctual value. Here, `oneOf` is an array of tuples, in which `title` contains the user-friendly description and `const` provides the value. + +```json +"imagePullPolicy": { + "type": "string", + "title": "Pull policy", + "description": "Whether the container image should be pulled from a registry", + "oneOf": [ + {"const": "IfNotPresent", "title": "Pull image if not present in the node"}, + {"const": "Always", "title": "Always pull up-to-date image"}, + {"const": "Never", "title": "Never pull image, use image available locally"} + ] +} +``` + ## TODO I haven't gotten to these yet, or I'm not sure about them. From 6dc44d8a595e418f09f3dd0d5485bc44fcae05d8 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Thu, 19 Jan 2023 11:44:58 +0100 Subject: [PATCH 22/53] Update requirement regarding type --- cluster-app-values-schema/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 2c0c29ec..62837b7b 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -272,7 +272,7 @@ I haven't gotten to these yet, or I'm not sure about them. - Each property must declare the `type`. Pretty sure that this is required by JSON schema. -- How to label values, e. g. in the case of the service priority label. See https://github.com/giantswarm/roadmap/issues/1181#issuecomment-1359344451 +- `type` must have exactly one value. - Should schemas have the `$id` property defined? And if yes, to what? From 70c15b480fce43b42826ac356648b9825d8475c8 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Thu, 19 Jan 2023 11:46:27 +0100 Subject: [PATCH 23/53] Add details to title requirements --- cluster-app-values-schema/README.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 62837b7b..662fe483 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -87,7 +87,7 @@ Example: Additional requirements apply to the title value: - The title MUST be written using sentence case capitalization. Correct: "Cluster name", incorrect: "Cluster Name". -- The title MUST NOT contain punctuation marks. +- The title MUST NOT contain punctuation marks, leading or trailing white space, control characters, tabs, nor multiple whitespaces in a row. - If the title annotates a property that is part of another object, the title SHOULD NOT include the parent property name, to avoid repetition. - Example: with an object `/controlPlane` that is titled `Control plane`, the property `/controlPlane/availabilityZones` should be titled `Availability zones`, not `Control plane availability zones`. @@ -95,8 +95,6 @@ Additional requirements apply to the title value: Each property SHOULD be annotated and via the `description` keyword. - - Example: ```json From e906823ac681a9637b68a30b9dbbaee45fa3fbfa Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Thu, 19 Jan 2023 11:46:57 +0100 Subject: [PATCH 24/53] Add details to description requirements --- cluster-app-values-schema/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 662fe483..0ae771a9 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -113,7 +113,7 @@ Example: If a description is given, additional requirements apply to the value: - Description content MUST NOT include any line breaks. -- Descriptions MUST NOT contain formatting code like e.g. Markdown or HTML. +- Descriptions MUST NOT contain formatting code like e.g. Markdown or HTML, leading or trailing white space, control characters, tabs, nor multiple whitespaces in a row. - Descriptions MUST use sentence case capitalization and punctuation. - The description SHOULD NOT repeat the property name or title. Correct: "Identifies this cluster uniquely within the installation.", incorrect: "Cluster name identifies this cluster uniquely within the installation.". - Descriptions SHOULD be between 50 and 200 characters long. From 1bb5ae948d36951b0f1bd85af7c5404889cbd5f2 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Mon, 30 Jan 2023 13:53:17 +0100 Subject: [PATCH 25/53] Require draft 2019-09 instead of 202-12 --- cluster-app-values-schema/README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 0ae771a9..27fa1381 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -6,7 +6,7 @@ This RFC defines basic requirements for all cluster apps provided by Giant Swarm ## Overview -- [R1: JSON Schema dialect (draft 2020-12) must be specified](#r1) +- [R1: JSON Schema dialect (draft 2019-09) 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) @@ -38,17 +38,17 @@ TODO: Some more info regarding the rationale. Requirements carry identifiers with prefix `R` and a unique number, for easy referencing. -### R1: JSON Schema dialect (draft 2020-12) must be specified {#r1} +### R1: JSON Schema dialect (draft 2019-09) 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. -The draft URI MUST BE `https://json-schema.org/draft/2020-12/schema`. +The draft URI MUST be `https://json-schema.org/draft/2019-09/schema`. Example: ```json { - "$schema": "https://json-schema.org/draft/2020-12/schema", + "$schema": "https://json-schema.org/draft/2019-09/schema", ... } ``` From a0756979983944ea98d7d5e7be5666fdcc8617d9 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Fri, 3 Feb 2023 13:54:17 +0100 Subject: [PATCH 26/53] Add TODO items --- cluster-app-values-schema/README.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 27fa1381..ee2ecec0 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -278,6 +278,23 @@ I haven't gotten to these yet, or I'm not sure about them. - How to specify whether a property can be modified or not. `"readOnly": true` mit be the right one for that. +- Avoid recursion. No use of dynamicRef, dynamicAnchor, RecursiveRef + +- Not, AllOf, AnyOf, OneOf must only be used for constraints. No use of `type`, `properties` etc. in the sub-schema. + +- If, Then, Else cannot be used + +- DependentRequired, DependentSchemas: to be evaluated. + +- UnevaluatedProperties, UnevaluatedItems cannot be used. + +- AdditionalItems, PrefixItems, for tuple validation cannot be used. + +- All array items must be of the same type. `contains` cannot be used. + +- contentEncoding in combination with contentSchema: to be defined. + - see "username:password" in base 64 example + ## Resources - [Understanding JSON Schema](https://json-schema.org/understanding-json-schema/) From 0d6ba390ee053cbc404274fbb56612ff3748e01d Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Fri, 3 Feb 2023 14:46:06 +0100 Subject: [PATCH 27/53] Clarify examples requirements --- cluster-app-values-schema/README.md | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index ee2ecec0..ed7a2d1a 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -138,15 +138,11 @@ Example: } ``` -Best practices: +An example can give users easy-to-understand guideance on how to use a property. Ideally, examples SHOULD be valid cases, fulfilling the contstraints of the property. -- 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 +Multiple examples can be provided. Per property, there SHOULD be at least one example. Additional examples should only be provided to indicate the range of different values possible. There SHOULD NOT be more than five examples per property. -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. +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} From 83f0c87157340103a0044dcedd394caf4a47ddba Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Fri, 3 Feb 2023 15:02:08 +0100 Subject: [PATCH 28/53] Clarify requirements for string and numeric field restrictions --- cluster-app-values-schema/README.md | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index ed7a2d1a..3da5c61d 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -150,17 +150,9 @@ 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`: +String properties SHOULD be constrained by at least one of the keywords `const`, `enum`, `pattern`, `minLength`, or `maxLength`, or `format`. -- 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). -- Use `enum` if only certain known values can be valid. - -Numeric properties (type `number`, `integer`): - -- Restrict the values range using `minimum` or `exclusiveMinimum` and `maximum` or `exclusiveMaximum`. -- Use `multipleOf` if the value has to be a multiple of a certain number. +Numeric properties (type `number`, `integer`) SHOULD be constrained by at least one of the keywords `minimum` or `exclusiveMinimum`, `maximum` or `exclusiveMaximum`. Example: From 9630507b62cd93ab6cf5182964d5b7c4e9c17941 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Sun, 5 Feb 2023 11:46:23 +0100 Subject: [PATCH 29/53] Add type requirements --- cluster-app-values-schema/README.md | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 3da5c61d..340161d3 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -7,6 +7,7 @@ This RFC defines basic requirements for all cluster apps provided by Giant Swarm ## Overview - [R1: JSON Schema dialect (draft 2019-09) must be specified](#r1) +- [R12: A single type must be declared](#r12) - [R2: Schema must explicitly cover all allowed properties](#r2) - [R3: Array item schema must be defined](#r3) - [R4: Properties must have a title](#r4) @@ -53,6 +54,28 @@ Example: } ``` +### R12: A single type must be declared {#r12} + +For each property, including the root level schema, the `type` keyword MUST be present, and there MUST be a single value for the `type` keyword. + +While JSON Schema draft 2019-09 allows multiple values for the `type` keyword, for cluster-apps this would complicate the generation of user interfaces. Hence we require a single type to be specified. + +For validation, there is no difference between an array with a single member or a single string. The two following examples can be considered identical. + +```json +{ + "type": "string", + "title": "Name" +} +``` + +```json +{ + "type": ["string"], + "title": "Name" +} +``` + ### R2: Schema must explicitly cover all allowed properties {#r2} All properties that can be used in values MUST be covered in the schema explicitly. @@ -256,10 +279,6 @@ However, if the string values are not considered self-explanatory, the `oneOf` m I haven't gotten to these yet, or I'm not sure about them. -- Each property must declare the `type`. Pretty sure that this is required by JSON schema. - -- `type` must have exactly one value. - - Should schemas have the `$id` property defined? And if yes, to what? - 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. From 4720417d14952996c818bcfa989dcf71961ec64a Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Sun, 5 Feb 2023 12:00:55 +0100 Subject: [PATCH 30/53] Add requirement to void recursion --- cluster-app-values-schema/README.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 340161d3..70ef94e5 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -18,6 +18,7 @@ This RFC defines basic requirements for all cluster apps provided by Giant Swarm - [R9: Avoid `anyOf` and `oneOf`](#r9) - [R10: Use `deprecated` to phase out properties](#r10) - [R11: Provide valid string values where possible](#r11) +- [R13: Avoid recursion](#r13) ## Background @@ -275,6 +276,10 @@ However, if the string values are not considered self-explanatory, the `oneOf` m } ``` +### R13: Avoid recursion {#r13} + +The JSON Schema keywords `dynamicRef`, `dynamicAnchor`, and `recursiveRef` MUST NOT be used. + ## TODO I haven't gotten to these yet, or I'm not sure about them. @@ -285,8 +290,6 @@ I haven't gotten to these yet, or I'm not sure about them. - How to specify whether a property can be modified or not. `"readOnly": true` mit be the right one for that. -- Avoid recursion. No use of dynamicRef, dynamicAnchor, RecursiveRef - - Not, AllOf, AnyOf, OneOf must only be used for constraints. No use of `type`, `properties` etc. in the sub-schema. - If, Then, Else cannot be used From 2b7e16ecf1885e4337e185bbba149610ede55832 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Sun, 5 Feb 2023 12:01:43 +0100 Subject: [PATCH 31/53] Add requirement to avoid if, then, else --- cluster-app-values-schema/README.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 70ef94e5..454f72b7 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -19,6 +19,7 @@ This RFC defines basic requirements for all cluster apps provided by Giant Swarm - [R10: Use `deprecated` to phase out properties](#r10) - [R11: Provide valid string values where possible](#r11) - [R13: Avoid recursion](#r13) +- [R14: Avoid logical constructs using `if`, `then`, `else`](#r14) ## Background @@ -280,6 +281,10 @@ However, if the string values are not considered self-explanatory, the `oneOf` m The JSON Schema keywords `dynamicRef`, `dynamicAnchor`, and `recursiveRef` MUST NOT be used. +### R14: Avoid logical constructs using `if`, `then`, `else` {#r14} + +The JSON Schema keywords `if`, `then`, and `else` MUST NOT be used. + ## TODO I haven't gotten to these yet, or I'm not sure about them. @@ -292,8 +297,6 @@ I haven't gotten to these yet, or I'm not sure about them. - Not, AllOf, AnyOf, OneOf must only be used for constraints. No use of `type`, `properties` etc. in the sub-schema. -- If, Then, Else cannot be used - - DependentRequired, DependentSchemas: to be evaluated. - UnevaluatedProperties, UnevaluatedItems cannot be used. From 9363c0b68bac4487211b8d24798d02652fbf2b78 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Sun, 5 Feb 2023 12:02:10 +0100 Subject: [PATCH 32/53] Add requirement to avoid unevaluated properties/items --- cluster-app-values-schema/README.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 454f72b7..8acdbf22 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -20,6 +20,7 @@ This RFC defines basic requirements for all cluster apps provided by Giant Swarm - [R11: Provide valid string values where possible](#r11) - [R13: Avoid recursion](#r13) - [R14: Avoid logical constructs using `if`, `then`, `else`](#r14) +- [R15: Avoid `unevaluatedProperties`, `unevaluatedItems`](#r15) ## Background @@ -285,6 +286,10 @@ The JSON Schema keywords `dynamicRef`, `dynamicAnchor`, and `recursiveRef` MUST The JSON Schema keywords `if`, `then`, and `else` MUST NOT be used. +### R15: Avoid `unevaluatedProperties`, `unevaluatedItems` {#r15} + +The JSON Schema keywords `unevaluatedProperties` , `unevaluatedItems` MUST NOT be used. + ## TODO I haven't gotten to these yet, or I'm not sure about them. @@ -299,8 +304,6 @@ I haven't gotten to these yet, or I'm not sure about them. - DependentRequired, DependentSchemas: to be evaluated. -- UnevaluatedProperties, UnevaluatedItems cannot be used. - - AdditionalItems, PrefixItems, for tuple validation cannot be used. - All array items must be of the same type. `contains` cannot be used. From 9d9a8a5f3f468eca15bbf74330eb533c3351d8d7 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Sun, 5 Feb 2023 12:02:28 +0100 Subject: [PATCH 33/53] Add requirement to avoid tuple validation --- cluster-app-values-schema/README.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 8acdbf22..89100213 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -21,6 +21,7 @@ This RFC defines basic requirements for all cluster apps provided by Giant Swarm - [R13: Avoid recursion](#r13) - [R14: Avoid logical constructs using `if`, `then`, `else`](#r14) - [R15: Avoid `unevaluatedProperties`, `unevaluatedItems`](#r15) +- [R16: Avoid tuple validation](#r16) ## Background @@ -290,6 +291,10 @@ The JSON Schema keywords `if`, `then`, and `else` MUST NOT be used. The JSON Schema keywords `unevaluatedProperties` , `unevaluatedItems` MUST NOT be used. +### R16: Avoid tuple validation {#r16} + +The JSON Schema keywords `additionalItems` and `prefixItems` MUST NOT be used. + ## TODO I haven't gotten to these yet, or I'm not sure about them. @@ -304,8 +309,6 @@ I haven't gotten to these yet, or I'm not sure about them. - DependentRequired, DependentSchemas: to be evaluated. -- AdditionalItems, PrefixItems, for tuple validation cannot be used. - - All array items must be of the same type. `contains` cannot be used. - contentEncoding in combination with contentSchema: to be defined. From d424fac72c3341c3f1e69e2eb5ed9397098e830c Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Sun, 5 Feb 2023 12:12:23 +0100 Subject: [PATCH 34/53] Add requirement regarding 'contains' --- cluster-app-values-schema/README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 89100213..57c9398a 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -21,7 +21,7 @@ This RFC defines basic requirements for all cluster apps provided by Giant Swarm - [R13: Avoid recursion](#r13) - [R14: Avoid logical constructs using `if`, `then`, `else`](#r14) - [R15: Avoid `unevaluatedProperties`, `unevaluatedItems`](#r15) -- [R16: Avoid tuple validation](#r16) +- [R16: Array items and tuple validation](#r16) ## Background @@ -291,9 +291,11 @@ The JSON Schema keywords `if`, `then`, and `else` MUST NOT be used. The JSON Schema keywords `unevaluatedProperties` , `unevaluatedItems` MUST NOT be used. -### R16: Avoid tuple validation {#r16} +### R16: Array items and tuple validation {#r16} -The JSON Schema keywords `additionalItems` and `prefixItems` MUST NOT be used. +All array items MUST be of the same type. + +The JSON Schema keywords `contains`, `additionalItems` and `prefixItems` MUST NOT be used. ## TODO @@ -309,8 +311,6 @@ I haven't gotten to these yet, or I'm not sure about them. - DependentRequired, DependentSchemas: to be evaluated. -- All array items must be of the same type. `contains` cannot be used. - - contentEncoding in combination with contentSchema: to be defined. - see "username:password" in base 64 example From 35031cbc06fb4ae9009f4f4a1f8186165320b7bc Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Mon, 6 Feb 2023 10:30:51 +0100 Subject: [PATCH 35/53] Fix casing --- cluster-app-values-schema/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 57c9398a..2c4364b7 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -309,7 +309,7 @@ I haven't gotten to these yet, or I'm not sure about them. - Not, AllOf, AnyOf, OneOf must only be used for constraints. No use of `type`, `properties` etc. in the sub-schema. -- DependentRequired, DependentSchemas: to be evaluated. +- dependentRequired, dependentSchemas: to be evaluated. - contentEncoding in combination with contentSchema: to be defined. - see "username:password" in base 64 example From 4c0f5ae829378d50a53f70a161f96d833882db23 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Tue, 7 Feb 2023 10:41:17 +0100 Subject: [PATCH 36/53] Refine R9 (allOf, anyOf) --- cluster-app-values-schema/README.md | 57 +++++++++++++++++++++++------ 1 file changed, 45 insertions(+), 12 deletions(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 2c4364b7..5f2e99c1 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -15,7 +15,7 @@ This RFC defines basic requirements for all cluster apps provided by Giant Swarm - [R6: Properties should provide examples](#r6) - [R7: Constrain values as much as possible](#r7) - [R8: Required properties must be marked as such](#r8) -- [R9: Avoid `anyOf` and `oneOf`](#r9) +- [R9: Use `anyOf` and `oneOf` only for specific purposes](#r9) - [R10: Use `deprecated` to phase out properties](#r10) - [R11: Provide valid string values where possible](#r11) - [R13: Avoid recursion](#r13) @@ -202,17 +202,53 @@ Properties that get default values assigned via some external mechanism (e.g. an Note: If property of type object named `a` has required properties, this does not indicate that `a` itself must be defined in the instance. However it indicates that if `a` is defined, the required properties must be defined, too. -### R9: Avoid `anyOf` and `oneOf` {#r9} +### R9: Use `anyOf` and `oneOf` only for specific purposes {#r9} -A cluster app schema SHALL NOT make use of the `anyOf` or `oneOf` keyword. +The keywords `anyOf` and `oneOf` allow definition of multiple subschemas, where the payload must match the constraints of either one (`oneOf`) or any number of (`anyOf`) subschemas. For user interface generation, this creates great complications, hence we strongly restrict the use of these features. -If using `anyOf` or `oneOf` cannot be avoided, the desired subschema SHOULD be the first in the sequence. +A cluster app MAY only make use of the `anyOf` or `oneOf` keyword in the following ways: -In JSON Schema, the `anyOf` and `oneOf` keyword defines an array of possible subschemas for a property. For a user interface which is generated based on the schema, this creates a high degree of complexity. +1. to specify validation constraints via subschemas +2. to declare one or more subschemas as deprecated -At Giant Swarm, our user interface for cluster creation will not support `anyOf` nor `oneOf` to full extent. Instead, we are going to select only one of the defined schemas for data input, using simply the first schema that is not deprecated. +#### (1) Specifying validation constraints via subschemas -Let's consider this example schema: +The idea here is that each subschema only defines constraints for the validation of the payload. + +In this case, the subschemas MUST NOT contain any of the following keywords: + +- `type` declaration +- annotations (`title`, `description`, `examples`) +- `properties`, `patternProperties`, `additionalProperties` +- `items`, `additionalItems` + +The following examples shows a string property with two subschemas, where each one has a different validation pattern. + +```json +"diskSize": { + "type": "string", + "title": "Volume size", + "examples": ["10 GB", "10000 MB"], + "oneOf": [ + { + "pattern": "^[0-9]+ GB$" + }, + { + "pattern": "^[0-9]+ MB$" + } + ] +} +``` + +#### (2) Declaring a subschema as deprecated + +If the schema of a property changes over time, while keeping the name of the property, the use of subschemas in combination with the `deprecated` keyword can help phase out an old schema and introduce a new one. + +As an exception to the rules defined in (1) above, the subschemas MAY use the JSON Schema keywords forbidden under (1) if `"deprecated": true` is present in all except one of the subschemas. + +In the case of a generated user interface, only the non-deprecated subschema will be applied. All other subschemas will be ignored. + +Example: ```json "replicas": { @@ -226,14 +262,11 @@ Let's consider this example schema: "type": "integer", "maximum": 100, "title": "Size" - }, - {...} + } ] } ``` -Here, the user interface would use the second subschema (type: integer) and ignore all others. - ## R10: Use `deprecated` to phase out properties {#r10} To indicate that a property is supposed to be removed in a future version, the property SHOULD carry the `deprecated` key with the value `true`. @@ -307,7 +340,7 @@ I haven't gotten to these yet, or I'm not sure about them. - How to specify whether a property can be modified or not. `"readOnly": true` mit be the right one for that. -- Not, AllOf, AnyOf, OneOf must only be used for constraints. No use of `type`, `properties` etc. in the sub-schema. +- Clarify use of `Not`. - dependentRequired, dependentSchemas: to be evaluated. From 7163aabe9685b66722c15e1847ea6bbc867a2309 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Mon, 13 Feb 2023 13:45:07 +0100 Subject: [PATCH 37/53] Boolean properties don't need examples --- cluster-app-values-schema/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 5f2e99c1..f4507975 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -148,7 +148,7 @@ If a description is given, additional requirements apply to the value: ### R6: Properties should provide examples {#r6} -Each property SHOULD provide one or more examples using the `examples` keyword. +Each property (except for type `boolean`) SHOULD provide one or more examples using the `examples` keyword. Example: From 0a3d6326751c7476e9026eb162978600d635c094 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Mon, 13 Feb 2023 14:15:22 +0100 Subject: [PATCH 38/53] Change draft to 2020-12 --- cluster-app-values-schema/README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index f4507975..c4a6e2eb 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -6,7 +6,7 @@ This RFC defines basic requirements for all cluster apps provided by Giant Swarm ## Overview -- [R1: JSON Schema dialect (draft 2019-09) must be specified](#r1) +- [R1: JSON Schema dialect (draft 2020-12) must be specified](#r1) - [R12: A single type must be declared](#r12) - [R2: Schema must explicitly cover all allowed properties](#r2) - [R3: Array item schema must be defined](#r3) @@ -43,17 +43,17 @@ TODO: Some more info regarding the rationale. Requirements carry identifiers with prefix `R` and a unique number, for easy referencing. -### R1: JSON Schema dialect (draft 2019-09) must be specified {#r1} +### R1: JSON Schema dialect (draft 2020-12) 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. -The draft URI MUST be `https://json-schema.org/draft/2019-09/schema`. +The draft URI MUST be `https://json-schema.org/draft/2020-12/schema`. Example: ```json { - "$schema": "https://json-schema.org/draft/2019-09/schema", + "$schema": "https://json-schema.org/draft/2020-12/schema", ... } ``` @@ -62,7 +62,7 @@ Example: For each property, including the root level schema, the `type` keyword MUST be present, and there MUST be a single value for the `type` keyword. -While JSON Schema draft 2019-09 allows multiple values for the `type` keyword, for cluster-apps this would complicate the generation of user interfaces. Hence we require a single type to be specified. +While JSON Schema draft 2020-12 allows multiple values for the `type` keyword, for cluster-apps this would complicate the generation of user interfaces. Hence we require a single type to be specified. For validation, there is no difference between an array with a single member or a single string. The two following examples can be considered identical. From a55d389812c39ce67f76ac859c8250fdad174445 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Mon, 13 Feb 2023 14:26:50 +0100 Subject: [PATCH 39/53] Update section numbering --- cluster-app-values-schema/README.md | 44 ++++++++++++++--------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index c4a6e2eb..0c5bd350 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -7,17 +7,17 @@ This RFC defines basic requirements for all cluster apps provided by Giant Swarm ## Overview - [R1: JSON Schema dialect (draft 2020-12) must be specified](#r1) -- [R12: A single type must be declared](#r12) -- [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) -- [R9: Use `anyOf` and `oneOf` only for specific purposes](#r9) -- [R10: Use `deprecated` to phase out properties](#r10) -- [R11: Provide valid string values where possible](#r11) +- [R2: A single type must be declared](#r2) +- [R3: Schema must explicitly cover all allowed properties](#r3) +- [R4: Array item schema must be defined](#r4) +- [R5: Properties must have a title](#r5) +- [R6: Properties should have descriptions](#r6) +- [R7: Properties should provide examples](#r7) +- [R8: Constrain values as much as possible](#r8) +- [R9: Required properties must be marked as such](#r9) +- [R10: Use `anyOf` and `oneOf` only for specific purposes](#r10) +- [R11: Use `deprecated` to phase out properties](#r11) +- [R12: Provide valid string values where possible](#r12) - [R13: Avoid recursion](#r13) - [R14: Avoid logical constructs using `if`, `then`, `else`](#r14) - [R15: Avoid `unevaluatedProperties`, `unevaluatedItems`](#r15) @@ -58,7 +58,7 @@ Example: } ``` -### R12: A single type must be declared {#r12} +### R2: A single type must be declared {#r2} For each property, including the root level schema, the `type` keyword MUST be present, and there MUST be a single value for the `type` keyword. @@ -80,17 +80,17 @@ For validation, there is no difference between an array with a single member or } ``` -### R2: Schema must explicitly cover all allowed properties {#r2} +### R3: Schema must explicitly cover all allowed properties {#r3} 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. -### R3: Array item schema must be defined {#r3} +### R4: Array item schema must be defined {#r4} The items schema for all array properties MUST be defined using the `items` keyword. -### R4: Properties must have a title {#r4} +### R5: Properties must have a title {#r5} Each property MUST be annotated via the `title` keyword. @@ -118,7 +118,7 @@ Additional requirements apply to the title value: - If the title annotates a property that is part of another object, the title SHOULD NOT include the parent property name, to avoid repetition. - Example: with an object `/controlPlane` that is titled `Control plane`, the property `/controlPlane/availabilityZones` should be titled `Availability zones`, not `Control plane availability zones`. -### R5: Properties should have descriptions {#r5} +### R6: Properties should have descriptions {#r6} Each property SHOULD be annotated and via the `description` keyword. @@ -146,7 +146,7 @@ If a description is given, additional requirements apply to the value: - Descriptions SHOULD be between 50 and 200 characters long. - Descriptions SHOULD be written in simple language. -### R6: Properties should provide examples {#r6} +### R7: Properties should provide examples {#r7} Each property (except for type `boolean`) SHOULD provide one or more examples using the `examples` keyword. @@ -171,7 +171,7 @@ Multiple examples can be provided. Per property, there SHOULD be at least one ex 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} +### R8: Constrain values as much as possible {#r8} Property schema SHOULD explicitly restrict values as much as possible. @@ -192,7 +192,7 @@ Example: } ``` -### R8: Required properties {#r8} +### R9: Required properties {#r9} If a property is required to be set by the creator of a cluster, it MUST be included in the `required` keyword. @@ -202,7 +202,7 @@ Properties that get default values assigned via some external mechanism (e.g. an Note: If property of type object named `a` has required properties, this does not indicate that `a` itself must be defined in the instance. However it indicates that if `a` is defined, the required properties must be defined, too. -### R9: Use `anyOf` and `oneOf` only for specific purposes {#r9} +### R10: Use `anyOf` and `oneOf` only for specific purposes {#r10} The keywords `anyOf` and `oneOf` allow definition of multiple subschemas, where the payload must match the constraints of either one (`oneOf`) or any number of (`anyOf`) subschemas. For user interface generation, this creates great complications, hence we strongly restrict the use of these features. @@ -267,7 +267,7 @@ Example: } ``` -## R10: Use `deprecated` to phase out properties {#r10} +## R11: Use `deprecated` to phase out properties {#r11} To indicate that a property is supposed to be removed in a future version, the property SHOULD carry the `deprecated` key with the value `true`. @@ -280,7 +280,7 @@ In addition, it is RECOMMENDED to add a `$comment` key to the property, with inf Note that `$comment` content is not intended for display in any UI nor processing in any tool. It is mainly targeting schema developers and anyone using the schema itself as a source of information. -## R11: Provide valid string values where possible {#r11} +## R12: Provide valid string values where possible {#r12} For string properties, in some cases only a few values are considered valid. In this case, the schema SHOULD specify these selectable values in one of the following forms: From c5fdaffdb7d77652b627699283483eee3eee229f Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Mon, 13 Feb 2023 14:42:42 +0100 Subject: [PATCH 40/53] Add R17 on common schema structure --- cluster-app-values-schema/README.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 0c5bd350..78acf59e 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -22,6 +22,7 @@ This RFC defines basic requirements for all cluster apps provided by Giant Swarm - [R14: Avoid logical constructs using `if`, `then`, `else`](#r14) - [R15: Avoid `unevaluatedProperties`, `unevaluatedItems`](#r15) - [R16: Array items and tuple validation](#r16) +- [R17: Common schema structure](#r17) ## Background @@ -330,6 +331,26 @@ All array items MUST be of the same type. The JSON Schema keywords `contains`, `additionalItems` and `prefixItems` MUST NOT be used. +### R17: Common schema structure {#r17} + +For cluster apps, we aim to use a common schema structure among the providers we support. This structure accounts for the necessary differences that can occur between providers, as some concepts and implementations are provider-specific. + +Each cluster app values schema MUST offer the following root level properties: + +| Property name | Property type | Description | +|-|-|-| +| `metadata` | object | Descriptive settings like name, description, cluster labels (e. g. service priority), owner organization | +| `connectivity` | object | Settings related to connectivity and networking, and defining how the cluster reaches the outside world and how it can be reached. | +| `controlPlane` | object | Configuration of the cluster's control plane, Etcd, API server and more. | +| `nodePools` | array | Configuration of node pools (groups of worker nodes), regardless of their implentation flavour. | + +In addition, the cluster app values schema SHOULD offer the following root level properties: + +| Property name | title | description | +|-|-|-| +| `internal` | object | Settings which are not supposed to be configured by end users, and which won't be exposed via user interfaces. Also experimental features that undergo schema changes. | +| `providerSpecific` | object | Configuration specific to the infrastructure provider. | + ## TODO I haven't gotten to these yet, or I'm not sure about them. From d2a2f35533adbca6978b0b57e5fb208ee0fc2b21 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Wed, 1 Mar 2023 15:02:05 +0100 Subject: [PATCH 41/53] Rephrase and refine R3 --- cluster-app-values-schema/README.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 78acf59e..958b94d6 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -8,7 +8,7 @@ This RFC defines basic requirements for all cluster apps provided by Giant Swarm - [R1: JSON Schema dialect (draft 2020-12) must be specified](#r1) - [R2: A single type must be declared](#r2) -- [R3: Schema must explicitly cover all allowed properties](#r3) +- [R3: Explicitly cover all allowed properties](#r3) - [R4: Array item schema must be defined](#r4) - [R5: Properties must have a title](#r5) - [R6: Properties should have descriptions](#r6) @@ -81,11 +81,13 @@ For validation, there is no difference between an array with a single member or } ``` -### R3: Schema must explicitly cover all allowed properties {#r3} +### R3: Explicitly cover all allowed properties {#r3} 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. +To enforce this and to disable the use of any undefined properties, the keyword `additionalProperties` MUST be set to `false` on the schema's root object. + +For objects deeper in the schema hierarchy, keyword `additionalProperties` SHOULD be set to `false`, too. ### R4: Array item schema must be defined {#r4} From 211488e2a0870908006a17437a520d3cbfcfd542 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Wed, 1 Mar 2023 15:02:25 +0100 Subject: [PATCH 42/53] Extend R17 for compatibility reasons --- cluster-app-values-schema/README.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 958b94d6..c88f146a 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -353,6 +353,14 @@ In addition, the cluster app values schema SHOULD offer the following root level | `internal` | object | Settings which are not supposed to be configured by end users, and which won't be exposed via user interfaces. Also experimental features that undergo schema changes. | | `providerSpecific` | object | Configuration specific to the infrastructure provider. | +For compatibility reasons, the schema MAY have the following properties in the root level: + +- `managementCluster` +- `baseDomain` +- `provider` + +The schema MUST NOT define any other properties on the root level, in addition to the ones mentioned above. + ## TODO I haven't gotten to these yet, or I'm not sure about them. From 172a47b56db63edfd73eb005cebed5b6300e3a8f Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Wed, 1 Mar 2023 17:30:27 +0100 Subject: [PATCH 43/53] R17: Add /cluster-shared to allowed properties --- cluster-app-values-schema/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index c88f146a..50aeb2db 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -358,6 +358,7 @@ For compatibility reasons, the schema MAY have the following properties in the r - `managementCluster` - `baseDomain` - `provider` +- `cluster-shared` The schema MUST NOT define any other properties on the root level, in addition to the ones mentioned above. From 4b54af058bb07944cbfdf9805c008cb6c17a10e5 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Thu, 2 Mar 2023 10:14:29 +0100 Subject: [PATCH 44/53] Fix table headings Co-authored-by: Moritz Gottschling --- cluster-app-values-schema/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 50aeb2db..e52edd9b 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -348,7 +348,7 @@ Each cluster app values schema MUST offer the following root level properties: In addition, the cluster app values schema SHOULD offer the following root level properties: -| Property name | title | description | +| Property name | Property type | Description | |-|-|-| | `internal` | object | Settings which are not supposed to be configured by end users, and which won't be exposed via user interfaces. Also experimental features that undergo schema changes. | | `providerSpecific` | object | Configuration specific to the infrastructure provider. | From 10637bcace9fc4c62136438d84cc6c7e4efb11aa Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Thu, 9 Mar 2023 09:39:48 +0100 Subject: [PATCH 45/53] Update R7 on examples --- cluster-app-values-schema/README.md | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index e52edd9b..6cd607b0 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -12,7 +12,7 @@ This RFC defines basic requirements for all cluster apps provided by Giant Swarm - [R4: Array item schema must be defined](#r4) - [R5: Properties must have a title](#r5) - [R6: Properties should have descriptions](#r6) -- [R7: Properties should provide examples](#r7) +- [R7: Some properties should provide examples](#r7) - [R8: Constrain values as much as possible](#r8) - [R9: Required properties must be marked as such](#r9) - [R10: Use `anyOf` and `oneOf` only for specific purposes](#r10) @@ -149,9 +149,9 @@ If a description is given, additional requirements apply to the value: - Descriptions SHOULD be between 50 and 200 characters long. - Descriptions SHOULD be written in simple language. -### R7: Properties should provide examples {#r7} +### R7: Some properties should provide examples {#r7} -Each property (except for type `boolean`) SHOULD provide one or more examples using the `examples` keyword. +Properties of type `string` SHOULD provide at least one value example using the `examples` keyword. Example: @@ -161,6 +161,7 @@ Example: "properties": { "name": { "type": "string", + "pattern": "^[a-z0-9]{5,10}$", "examples": ["devel001"], ... } @@ -168,11 +169,9 @@ Example: } ``` -An example can give users easy-to-understand guideance on how to use a property. Ideally, examples SHOULD be valid cases, fulfilling the contstraints of the property. +An example can give users easy-to-understand guideance on how to use a property. Ideally, examples SHOULD be valid cases, fulfilling the contstraints of the property (if given). -Multiple examples can be provided. Per property, there SHOULD be at least one example. Additional examples should only be provided to indicate the range of different values possible. There SHOULD NOT be more than five examples per property. - -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. +Multiple examples can be provided. Additional examples should only be provided to indicate the range of different values possible. There SHOULD NOT be more than five examples per property. ### R8: Constrain values as much as possible {#r8} From 521379edae476e9438d9987c9ad0d47ffacd79c4 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Thu, 9 Mar 2023 09:40:36 +0100 Subject: [PATCH 46/53] Add missing condition to R7 --- cluster-app-values-schema/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 6cd607b0..5d1a5441 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -151,7 +151,7 @@ If a description is given, additional requirements apply to the value: ### R7: Some properties should provide examples {#r7} -Properties of type `string` SHOULD provide at least one value example using the `examples` keyword. +Properties of type `string` SHOULD provide at least one value example using the `examples` keyword, if the property is restricted either using `pattern` or `format`. Example: From 650c1397676ec7ddce0de81f6951ce78bfc759f9 Mon Sep 17 00:00:00 2001 From: Marian Steinbach Date: Thu, 9 Mar 2023 09:49:06 +0100 Subject: [PATCH 47/53] Fix typos Co-authored-by: Moritz Gottschling --- cluster-app-values-schema/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 5d1a5441..35d7f72f 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -169,7 +169,7 @@ Example: } ``` -An example can give users easy-to-understand guideance on how to use a property. Ideally, examples SHOULD be valid cases, fulfilling the contstraints of the property (if given). +An example can give users easy-to-understand guidance on how to use a property. Ideally, examples SHOULD be valid cases, fulfilling the constraints of the property (if given). Multiple examples can be provided. Additional examples should only be provided to indicate the range of different values possible. There SHOULD NOT be more than five examples per property. From d345df7fe535f059519fdb3ccc85b8fbabf6c3f9 Mon Sep 17 00:00:00 2001 From: Moritz Gottschling Date: Tue, 11 Apr 2023 09:59:37 +0200 Subject: [PATCH 48/53] Add R18 for empty defaults --- cluster-app-values-schema/README.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 35d7f72f..a71562d4 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -23,6 +23,7 @@ This RFC defines basic requirements for all cluster apps provided by Giant Swarm - [R15: Avoid `unevaluatedProperties`, `unevaluatedItems`](#r15) - [R16: Array items and tuple validation](#r16) - [R17: Common schema structure](#r17) +- [R18: Avoid using empty values in defaults](#r17) ## Background @@ -361,6 +362,20 @@ For compatibility reasons, the schema MAY have the following properties in the r The schema MUST NOT define any other properties on the root level, in addition to the ones mentioned above. +### R18: Avoid using empty values in defaults + +If a property specifies a default via the default keyword, then the default MUST not be an empty value. + +The definition of an empty value differs by type. +| type | empty value | +|---------|---------------------------------------| +| Boolean | `false` | +| String | `""` | +| Integer | `0` | +| Number | `0` or everthing that's equal (`0.0`) | +| Array | `[]` | +| Object | `{}` | + ## TODO I haven't gotten to these yet, or I'm not sure about them. From 3ee3e0b2ff1fdfa975233e1f7e13e5274df224ae Mon Sep 17 00:00:00 2001 From: Moritz Gottschling Date: Tue, 11 Apr 2023 10:02:28 +0200 Subject: [PATCH 49/53] Fix r18 heading --- cluster-app-values-schema/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index a71562d4..520be53d 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -362,7 +362,7 @@ For compatibility reasons, the schema MAY have the following properties in the r The schema MUST NOT define any other properties on the root level, in addition to the ones mentioned above. -### R18: Avoid using empty values in defaults +### R18: Avoid using empty values in defaults {#r18} If a property specifies a default via the default keyword, then the default MUST not be an empty value. From 6a26e980177b214c6002133739e3c3bdb70d5440 Mon Sep 17 00:00:00 2001 From: Moritz Gottschling Date: Wed, 12 Apr 2023 14:48:04 +0200 Subject: [PATCH 50/53] Adapt common schema structure requirements to cluster-aws --- cluster-app-values-schema/README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index 520be53d..fe856351 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -344,7 +344,7 @@ Each cluster app values schema MUST offer the following root level properties: | `metadata` | object | Descriptive settings like name, description, cluster labels (e. g. service priority), owner organization | | `connectivity` | object | Settings related to connectivity and networking, and defining how the cluster reaches the outside world and how it can be reached. | | `controlPlane` | object | Configuration of the cluster's control plane, Etcd, API server and more. | -| `nodePools` | array | Configuration of node pools (groups of worker nodes), regardless of their implentation flavour. | +| `nodePools` | array or object | Configuration of node pools (groups of worker nodes), regardless of their implentation flavour. | In addition, the cluster app values schema SHOULD offer the following root level properties: @@ -359,6 +359,8 @@ For compatibility reasons, the schema MAY have the following properties in the r - `baseDomain` - `provider` - `cluster-shared` +- `defaultMachinePools` +- `kubectlImage` The schema MUST NOT define any other properties on the root level, in addition to the ones mentioned above. From 29e256acc2029d4eb8ab80fb9be533213a81e186 Mon Sep 17 00:00:00 2001 From: Andreas Sommer Date: Mon, 17 Apr 2023 10:41:55 +0200 Subject: [PATCH 51/53] Add recommendations for default values (merge vs. replace) --- cluster-app-values-schema/README.md | 148 ++++++++++++++++++++++++++++ 1 file changed, 148 insertions(+) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index fe856351..aa589b1f 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -24,6 +24,7 @@ This RFC defines basic requirements for all cluster apps provided by Giant Swarm - [R16: Array items and tuple validation](#r16) - [R17: Common schema structure](#r17) - [R18: Avoid using empty values in defaults](#r17) +- [Recommendations that are not requirements](#recommendations) ## Background @@ -378,6 +379,153 @@ The definition of an empty value differs by type. | Array | `[]` | | Object | `{}` | +## Recommendations + +### Default values + +The [`default` keyword in JSON Schema is not meant to fill missing values](https://json-schema.org/understanding-json-schema/reference/generic.html#annotations), but we support using it in that way. Helm only validates based on JSON Schema, but does not take default values from `values.schema.json` for rendering (as of v3.11.2). Therefore, we [generate `values.yaml` automatically](https://github.com/search?q=repo%3Agiantswarm%2Fdevctl%20ensure-generated&type=code) from `values.schema.json` in order to explicitly specify the default values. This means that `values.yaml` is autogenerated and must not be edited manually. + +Due to our merge strategy for [app configuration](https://docs.giantswarm.io/getting-started/app-platform/app-configuration/) which [uses the mergo library](https://github.com/giantswarm/app/tree/master/pkg/values), we can support two modes of allowing customers to use defaults – _merge_ or _replace_. Combining both is not possible. + +#### Allow replacing the default value + +Replacing makes sense for node pools and subnets, for example. If the customer wants to specify their own, our default value should not get considered or merged with their configuration. + +If the key where you want a default value is an `object`, you should introduce and fall back to a new key `internal.defaultAbcDef`. Example in the schema: + +```json +"internal": { + "description": "For Giant Swarm internal use only, not stable, or not supported by UIs.", + "properties": { + "defaultMachinePools": { + "default": { + "def00": { + "customNodeLabels": [ + "label=default" + ], + "instanceType": "m5.xlarge", + "minSize": 3 + } + }, + "patternProperties": { + "^[a-z0-9]{5,10}$": { + "$ref": "#/$defs/machinePool" + } + }, + "title": "Default node pool", + "type": "object" + } + }, + "title": "Internal", + "type": "object" +}, +"nodePools": { + "description": "Node pools of the cluster. If not specified, this defaults to the value of `internal.defaultMachinePools`.", + "patternProperties": { + "^[a-z0-9]{5,10}$": { + "$ref": "#/$defs/machinePool" + } + }, + "title": "Node pools", + "type": "object" +}, +``` + +And then in the Helm template, fall back to the default if no explicit value is defined: + +```text +{{ range $name, $value := .Values.nodePools | default .Values.internal.defaultMachinePools }} +apiVersion: cluster.x-k8s.io/v1beta1 +kind: MachinePool +[...] +{{ end }} +``` + +This means that happa cannot easily determine that a default exists. We may solve this later if needed. + +In the rare case that a customer, or Giant Swarm engineers for our own test installations, want to override only a specific part such as the `maxSize` in the defaults, the whole default value must be copied into the configuration: + +```yaml +nodePools: + # BEGIN part copied from default value + def00: + customNodeLabels: + - label=default + instanceType: m5.xlarge + minSize: 3 + # END part copied from default value + + # This particular item is added: + maxSize: 5 +``` + +#### Allow merging with default value + +Merging makes sense if our default values should prevail even if the customer wants to add customization on top. + +For example, our images are hosted on registries controlled by Giant Swarm, such as Docker Hub or Azure Container Registry. The default could therefore look like this: + +```json +"containerRegistries": { + "default": { + "docker.io": [ + { + "endpoint": "registry-1.docker.io" + }, + { + "endpoint": "giantswarm.azurecr.io" + } + ] + }, + "title": "Container registries", + "type": "object", + "...": "..." +}, +``` + +Or the same in the autogenerated `values.yaml`: + +```yaml +containerRegistries: + docker.io: + - endpoint: registry-1.docker.io + - endpoint: giantswarm.azurecr.io +``` + +Customers might want to add their own image registries. Since `containerRegistries` is an `object`, the following extra secret gets merged: + +```yaml +containerRegistries: + my-harbor-installation.customer.com: + - endpoint: my-harbor-installation.customer.com + auth: foobar +``` + +And the result is: + +```yaml +containerRegistries: + docker.io: + - endpoint: registry-1.docker.io + - endpoint: giantswarm.azurecr.io + + my-harbor-installation.customer.com: + - endpoint: my-harbor-installation.customer.com + auth: foobar +``` + +The above only works for type `object` since patches get merged even if nested. Unfortunately, merging lists (`array`) is not supported. Those will get replaced by the highest priority config. In the example, we want to allow adding credentials to the registry endpoints. That means the patch needs to copy-paste the whole value for the key `docker.io` again, and add any extras such as credentials: + +```yaml +containerRegistries: + docker.io: + - endpoint: registry-1.docker.io + username: abc + password: def + - endpoint: giantswarm.azurecr.io + auth: foo +``` + ## TODO I haven't gotten to these yet, or I'm not sure about them. From 0f8706dd8cd78510a014868dd5b01266d36c52a2 Mon Sep 17 00:00:00 2001 From: Andreas Sommer Date: Tue, 18 Apr 2023 11:37:14 +0200 Subject: [PATCH 52/53] Add recommendations for node pools --- cluster-app-values-schema/README.md | 56 +++++++++++++++++++++++++---- 1 file changed, 49 insertions(+), 7 deletions(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index aa589b1f..cc721983 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -23,8 +23,9 @@ This RFC defines basic requirements for all cluster apps provided by Giant Swarm - [R15: Avoid `unevaluatedProperties`, `unevaluatedItems`](#r15) - [R16: Array items and tuple validation](#r16) - [R17: Common schema structure](#r17) -- [R18: Avoid using empty values in defaults](#r17) -- [Recommendations that are not requirements](#recommendations) +- [R18: Avoid using empty values in defaults](#r18) +- [R19: Default values](#r19) +- [R20: Node pools configuration (`nodePools`)](#r20) ## Background @@ -379,15 +380,13 @@ The definition of an empty value differs by type. | Array | `[]` | | Object | `{}` | -## Recommendations - -### Default values +## R19: Usage of default values {#r19} The [`default` keyword in JSON Schema is not meant to fill missing values](https://json-schema.org/understanding-json-schema/reference/generic.html#annotations), but we support using it in that way. Helm only validates based on JSON Schema, but does not take default values from `values.schema.json` for rendering (as of v3.11.2). Therefore, we [generate `values.yaml` automatically](https://github.com/search?q=repo%3Agiantswarm%2Fdevctl%20ensure-generated&type=code) from `values.schema.json` in order to explicitly specify the default values. This means that `values.yaml` is autogenerated and must not be edited manually. Due to our merge strategy for [app configuration](https://docs.giantswarm.io/getting-started/app-platform/app-configuration/) which [uses the mergo library](https://github.com/giantswarm/app/tree/master/pkg/values), we can support two modes of allowing customers to use defaults – _merge_ or _replace_. Combining both is not possible. -#### Allow replacing the default value +### Allow replacing the default value Replacing makes sense for node pools and subnets, for example. If the customer wants to specify their own, our default value should not get considered or merged with their configuration. @@ -459,7 +458,7 @@ nodePools: maxSize: 5 ``` -#### Allow merging with default value +### Allow merging with default value Merging makes sense if our default values should prevail even if the customer wants to add customization on top. @@ -526,6 +525,49 @@ containerRegistries: auth: foo ``` +## R20: Node pools configuration (`nodePools`) {#r20} + +The root-level value of `nodePools` should be an `object`. Some cluster templates still use `array` and should be adapted over time. Rationale: based on customer requests, we offer the ability to merge several configurations to form the final node pools. For example, kustomize with base and dev/stag/prod-specific patches could be used to create several _extra configs_. Our app platform then merges those together. + +Example: + +```sh +# Base +nodePools: + standard: + instanceType: "m5.xlarge" + minSize: 3 + maxSize: 5 +``` + +```sh +# Development +nodePools: + standard: + # Cheaper instances for non-production workloads + instanceType: "t3.medium" + + # Customer may need another GPU-enabled pool for... stuff + gpu: + instanceType: "super.gpu.extralarge" + minSize: 1 + maxSize: 1 +``` + +Customers expect that these get merged together. The app platform only supports merging of objects, not arrays. + +Cluster charts should fall back to a default node pool if the customer has not specified `nodePool` _at all_. The naming convention for the fallback value is `internal.nodePools`. In the Helm templates, defaulting can be solved like this: + +```text +{{ range $name, $value := .Values.nodePools | default .Values.internal.nodePools }} +apiVersion: cluster.x-k8s.io/v1beta1 +kind: MachinePool +# [...] +{{ end }} +``` + +Taken together, these recommendations lead to least surprise. And the customer could even explicitly define "no node pools wanted" with `nodePools: {}`. + ## TODO I haven't gotten to these yet, or I'm not sure about them. From 6545e82c6cf87b65860bf799eed005d19e51b2dd Mon Sep 17 00:00:00 2001 From: Andreas Sommer Date: Tue, 18 Apr 2023 14:50:32 +0200 Subject: [PATCH 53/53] Link to gitops-template example of using different environments and merging configs together with the base --- cluster-app-values-schema/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cluster-app-values-schema/README.md b/cluster-app-values-schema/README.md index cc721983..8aede472 100644 --- a/cluster-app-values-schema/README.md +++ b/cluster-app-values-schema/README.md @@ -527,7 +527,7 @@ containerRegistries: ## R20: Node pools configuration (`nodePools`) {#r20} -The root-level value of `nodePools` should be an `object`. Some cluster templates still use `array` and should be adapted over time. Rationale: based on customer requests, we offer the ability to merge several configurations to form the final node pools. For example, kustomize with base and dev/stag/prod-specific patches could be used to create several _extra configs_. Our app platform then merges those together. +The root-level value of `nodePools` should be an `object`. Some cluster templates still use `array` and should be adapted over time. Rationale: based on customer requests, we offer the ability to merge several configurations to form the final node pools. For example, [kustomize with base and dev/stag/prod-specific patches](https://github.com/giantswarm/gitops-template/tree/main/bases/environments/stages) could be used to create several _extra configs_. Our app platform then merges those together. Example: