From 6af4a0491446b0d84dc443deebf79b395b5c0351 Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Sat, 6 Nov 2021 15:23:28 -0400 Subject: [PATCH 01/15] Clarify escaping, root field access and support for int-or-string, embedded and unknown fields --- .../README.md | 99 +++++++++++++------ 1 file changed, 70 insertions(+), 29 deletions(-) diff --git a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md index cdd2782a474..2dd972960be 100644 --- a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md +++ b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md @@ -101,9 +101,7 @@ CRDs validation, they significantly complicate the development and operability of CRDs. This KEP proposes that an inline expression language be integrated directly into CRDs such that a -much larger portion of validation use cases can be solved without the use of webhooks. When -selecting an exression language, we want to be sure that it can support defaulting and CRD -conversion in the future. +much larger portion of validation use cases can be solved without the use of webhooks. This KEP proposes the adoption of [Common Expression Language (CEL)](https://github.com/google/cel-go). It is sufficiently lightweight and safe to be run directly @@ -237,10 +235,12 @@ will be surfaced when the validation rule evaluates to false. extension in the schema. In the above example, the validator is scoped to the `spec` field. `self` will be used to represent the name of the field which the validator is scoped to. - - Consideration under adding the representative of scoped filed name: There would be composition problem while generating CRD with tools like `controller-gen`. - When trying to add validation as a marker comment to a field, the validation rule will - be hard to define without the actual field name. As the example showing below. When we want to put cel validation on ToySpec, the field name as `spec` has not - been identified yet which makes rule hard to define. + + - Consideration under adding the representative of scoped filed name: There would be composition + problem while generating CRD with tools like `controller-gen`. When trying to add validation as + a marker comment to a field, the validation rule will be hard to define without the actual field + name. As the example showing below. When we want to put cel validation on ToySpec, the field + name as `spec` has not been identified yet which makes rule hard to define. ```azure // +kubebuilder:validation:XValidator= @@ -283,14 +283,31 @@ like the `all` macro, e.g. `self.all(listItem, )` or `self.all(mapKey Strategy](https://kubernetes.io/docs/reference/using-api/server-side-apply/#merge-strategy) for details. - The use of "old" is congruent with how `AdmissionReview` identifies the existing object as - `oldObject`. To avoid name collisions `old` will be treated the same as a CEL + `oldSelf`. To avoid name collisions `old` will be treated the same as a CEL keyword for escaping purposes (see below). - xref [analysis of possible interactions with immutability and validation](https://github.com/kubernetes/enhancements/tree/master/keps/sig-api-machinery/1101-immutable-fields#openapi-extension-x-kubernetes-immutable). -- If a object property collides with a CEL keyword (see RESERVED in [CEL Syntax](https://github.com/google/cel-spec/blob/master/doc/langdef.md#syntax)), +- If a object property name is a CEL keyword (see RESERVED in [CEL Syntax](https://github.com/google/cel-spec/blob/master/doc/langdef.md#syntax)), it will be escaped by prepending a _ prefix. To prevent this from causing a subsequent collision, all properties with a `_` prefix will always be prefixed by `__` (generally, N+1 the existing number of `_`s). + +- If a object property name is a CEL language identifier (`int`, `uint`, `double`, `bool`, `string`, + `bytes`, `list`, `map`, `null_type`, `type`, see [CEL language + identifiers](https://github.com/google/cel-spec/blob/master/doc/langdef.md#values)) it is not + accessible as a root variable and must be accessed via `self`, .e.g. `seft.int`. + +- Rules may be written at the root of an object, and may make field selection into any fields + declared in the OpenAPIv3 schema of the CRD. This includes selection of fields in both the `spec` + and `status` in the same expression, e.g. `status.quantity <= spec.maxQuantity`. Because CRDs only + allow the `name` and `generatedName` to be declared in the `metadata` of an object, these are the + only metadata fields that may be validated using CEL validator rules. For example, + `metadata.name.endsWith('mySuffix')` is allowed, but only when the OpenAPIv3 schema explicitly + declares `metadata` as a field in the root object and declares the `name` field within `metadata`. + `size(metadata.labels) < 3` however, it not allowed. The limit on which `metadata` fields may be + validated is an intentional design choice (that aims to keep metadata behavior uniform across + types) and applies to all validation mechanisms (e.g. the OpenAPIV3 `maxItems` restriction), not + just CEL validator rules. - We plan to allow access to the current state of the object to allow validation rules to check the new value against the current value, e.g. for immutability checks (for validation racheting we would @@ -311,7 +328,6 @@ exact indices of the list items and the keys of map entries it traverses. A field *pattern* is a path to all nodes in the data tree that match the pattern. I.e. it may wildcard list item and map keys. - #### Expression lifecycle When CRDs are written to the kube-apiserver, all expressions will be [parsed and @@ -330,7 +346,6 @@ List of functions to include for the initial release: - Regular Expressions - Some Standard Definitions - Considerations: - The functions will become VERY difficult to change as this feature matures. We should limit ourselves initially to functions that we have a high level of @@ -475,24 +490,50 @@ coverage of interactions in these dimensions: Types: -| OpenAPIv3 type | CEL type | -| --------------------------------------- | ------------------------------ | -| 'object' with Properties | object / "message type" | -| 'object' with AdditionalProperties | map | -| 'array | list | -| 'array' with x-kubernetes-list-type=map | list with map based Equality & unique key guarantees | -| 'array' with x-kubernetes-list-type=set | list with set based Equality & unique entry guarantees | -| 'boolean' | boolean | -| 'number' (all formats) | double | -| 'integer' (all formats) | int (64) | -| 'null' | null | -| 'string' | string | -| 'string' with format=byte | bytes | -| 'string' with format=binary | bytes | -| 'string' with format=date | timestamp (protobuf.Timestamp) | -| 'string' with format=datetime | timestamp (protobuf.Timestamp) | - -xref: [CEL types](https://github.com/google/cel-spec/blob/master/doc/langdef.md#values), [OpenAPI types](https://swagger.io/specification/#data-types). +| OpenAPIv3 type | CEL type | +| -------------------------------------------------- | -------------------------------------------------------------------------------------------------------------- | +| 'object' with Properties | object / "message type" | +| 'object' with AdditionalProperties | map | +| 'object' with x-kubernetes-embedded-type | | +| 'object' with x-kubernetes-preserve-unknown-fields | | +| x-kubernetes-embedded-int-or-string | object with 'intVal' (type: int) and 'strVal' (type: string) fields | +| 'array | list | +| 'array' with x-kubernetes-list-type=map | list with map based Equality & unique key guarantees | +| 'array' with x-kubernetes-list-type=set | list with set based Equality & unique entry guarantees | +| 'boolean' | boolean | +| 'number' (all formats) | double | +| 'integer' (all formats) | int (64) | +| 'null' | null_type | +| 'string' | string | +| 'string' with format=byte | string (containing base64 encoded data) | +| 'string' with format=binary | bytes | +| 'string' with format=date | timestamp (google.protobuf.Timestamp) | +| 'string' with format=datetime | timestamp (google.protobuf.Timestamp) | +| 'string' with format=duration | duration (google.protobuf.Duration) | + +xref: [CEL types](https://github.com/google/cel-spec/blob/master/doc/langdef.md#values), [OpenAPI +types](https://swagger.io/specification/#data-types), [Kubernetes Structural Schemas](https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#specifying-a-structural-schema). + +Although `x-kubernetes-preserve-unknown-fields` allows custom resources to contain values without +corresponding schema information, we will not provide validation support of these "schemaless" +values. Reasons for this include: + +- Without schema information, types (e.g. `map` vs. `object`), formats (e.g. plain `string` + vs. `date`) and list types (plain `list` vs. `set`) are not available, and this feature depends + on this schema information to integrate custom resources values with CEL. + +- The contents of unknown fields are unvalidated. So it is possible for the contents to be any + possible values. This significantly complicates the authoring of validation rules and makes them + far more difficult write safely. Specifically, all unchecked field selection and assumptions about + value types will result in runtime errors when the values contents of the unknown data does not + match the expectations of the validation rule author. None of the benefits of static type checking + at CRD registration time would apply. + +- For objects with both `x-kubernetes-embedded-type` and `x-kubernetes-preserve-unknown-fields` set to `true`, + if we were to allow for validation of embedded resource while the validation rules that are part of + the embedded resource are not enforced, we would be allowing for developers to use CEL to validate + the contents of embedded types without allowing developers to write validation rules directly on the + embedded type to validate it. We don't want to do that. Implementation: From c3bc36fadc51ad1f7c1a8e1700e8beeaa231e172 Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Mon, 8 Nov 2021 12:15:17 -0800 Subject: [PATCH 02/15] Update keps/sig-api-machinery/2876-crd-validation-expression-language/README.md Co-authored-by: Jordan Liggitt --- .../2876-crd-validation-expression-language/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md index 2dd972960be..620f4a59afe 100644 --- a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md +++ b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md @@ -236,7 +236,7 @@ extension in the schema. In the above example, the validator is scoped to the `spec` field. `self` will be used to represent the name of the field which the validator is scoped to. - - Consideration under adding the representative of scoped filed name: There would be composition + - Consideration under adding the representative of scoped field name: There would be composition problem while generating CRD with tools like `controller-gen`. When trying to add validation as a marker comment to a field, the validation rule will be hard to define without the actual field name. As the example showing below. When we want to put cel validation on ToySpec, the field From 36184df50227ea90cb6064e8eeb651f8137edd57 Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Mon, 8 Nov 2021 12:15:48 -0800 Subject: [PATCH 03/15] Update keps/sig-api-machinery/2876-crd-validation-expression-language/README.md Co-authored-by: Jordan Liggitt --- .../2876-crd-validation-expression-language/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md index 620f4a59afe..8a5f638f553 100644 --- a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md +++ b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md @@ -295,7 +295,7 @@ like the `all` macro, e.g. `self.all(listItem, )` or `self.all(mapKey - If a object property name is a CEL language identifier (`int`, `uint`, `double`, `bool`, `string`, `bytes`, `list`, `map`, `null_type`, `type`, see [CEL language identifiers](https://github.com/google/cel-spec/blob/master/doc/langdef.md#values)) it is not - accessible as a root variable and must be accessed via `self`, .e.g. `seft.int`. + accessible as a root variable and must be accessed via `self`, .e.g. `self.int`. - Rules may be written at the root of an object, and may make field selection into any fields declared in the OpenAPIv3 schema of the CRD. This includes selection of fields in both the `spec` From 566b0eabc3867fad6a402a5f63c0028406ab37ad Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Mon, 8 Nov 2021 18:34:52 -0500 Subject: [PATCH 04/15] Address 'byte' representation and escaping of weird property names --- .../README.md | 32 +++++++++++-------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md index 8a5f638f553..af3fbb6221a 100644 --- a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md +++ b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md @@ -283,13 +283,13 @@ like the `all` macro, e.g. `self.all(listItem, )` or `self.all(mapKey Strategy](https://kubernetes.io/docs/reference/using-api/server-side-apply/#merge-strategy) for details. - The use of "old" is congruent with how `AdmissionReview` identifies the existing object as - `oldSelf`. To avoid name collisions `old` will be treated the same as a CEL + `oldObject`. To avoid name collisions `old` will be treated the same as a CEL keyword for escaping purposes (see below). - xref [analysis of possible interactions with immutability and validation](https://github.com/kubernetes/enhancements/tree/master/keps/sig-api-machinery/1101-immutable-fields#openapi-extension-x-kubernetes-immutable). - If a object property name is a CEL keyword (see RESERVED in [CEL Syntax](https://github.com/google/cel-spec/blob/master/doc/langdef.md#syntax)), - it will be escaped by prepending a _ prefix. To prevent this from causing a subsequent collision, all properties with a `_` prefix will always be + it will be escaped by prepending a _ prefix. To prevent this from causing a subsequent collision, properties named with a CEL keyword and a `_` prefix will be prefixed by `__` (generally, N+1 the existing number of `_`s). - If a object property name is a CEL language identifier (`int`, `uint`, `double`, `bool`, `string`, @@ -297,17 +297,22 @@ like the `all` macro, e.g. `self.all(listItem, )` or `self.all(mapKey identifiers](https://github.com/google/cel-spec/blob/master/doc/langdef.md#values)) it is not accessible as a root variable and must be accessed via `self`, .e.g. `self.int`. +- If a object property name contains characters not allowed in CEL identifiers it is escaped using these rules: + - `.` (period) is escaped as `__dot__` + - `-` (slash) is escaped as `__slash__` + - ` ` (space) is escaped as `__space__` + - `__` (2 underscores) is escaped as `__underscores__` + - Rules may be written at the root of an object, and may make field selection into any fields - declared in the OpenAPIv3 schema of the CRD. This includes selection of fields in both the `spec` - and `status` in the same expression, e.g. `status.quantity <= spec.maxQuantity`. Because CRDs only - allow the `name` and `generatedName` to be declared in the `metadata` of an object, these are the - only metadata fields that may be validated using CEL validator rules. For example, - `metadata.name.endsWith('mySuffix')` is allowed, but only when the OpenAPIv3 schema explicitly - declares `metadata` as a field in the root object and declares the `name` field within `metadata`. - `size(metadata.labels) < 3` however, it not allowed. The limit on which `metadata` fields may be - validated is an intentional design choice (that aims to keep metadata behavior uniform across - types) and applies to all validation mechanisms (e.g. the OpenAPIV3 `maxItems` restriction), not - just CEL validator rules. + declared in the OpenAPIv3 schema of the CRD as well as `apiVersion`, `kind`, `metadata.name` and + `metadata.generateName`. This includes selection of fields in both the `spec` and `status` in the + same expression, e.g. `status.quantity <= spec.maxQuantity`. Because CRDs only allow the `name` + and `generatName` to be declared in the `metadata` of an object, these are the only metadata + fields that may be validated using CEL validator rules. For example, + `metadata.name.endsWith('mySuffix')` is allowed, but `size(metadata.labels) < 3` it not + allowed. The limit on which `metadata` fields may be validated is an intentional design choice + (that aims to keep metadata behavior uniform across types) and applies to all validation + mechanisms (e.g. the OpenAPIV3 `maxItems` restriction), not just CEL validator rules. - We plan to allow access to the current state of the object to allow validation rules to check the new value against the current value, e.g. for immutability checks (for validation racheting we would @@ -505,8 +510,7 @@ Types: | 'integer' (all formats) | int (64) | | 'null' | null_type | | 'string' | string | -| 'string' with format=byte | string (containing base64 encoded data) | -| 'string' with format=binary | bytes | +| 'string' with format=byte (base64 encoded) | bytes | | 'string' with format=date | timestamp (google.protobuf.Timestamp) | | 'string' with format=datetime | timestamp (google.protobuf.Timestamp) | | 'string' with format=duration | duration (google.protobuf.Duration) | From f1bec78f6840bee98ebf160ecbc7e1768b1cca51 Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Mon, 8 Nov 2021 23:02:37 -0500 Subject: [PATCH 05/15] Describe how to handle characters not allowed in CEL identifiers in escaping rules --- .../README.md | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md index af3fbb6221a..af1f0187299 100644 --- a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md +++ b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md @@ -297,11 +297,16 @@ like the `all` macro, e.g. `self.all(listItem, )` or `self.all(mapKey identifiers](https://github.com/google/cel-spec/blob/master/doc/langdef.md#values)) it is not accessible as a root variable and must be accessed via `self`, .e.g. `self.int`. -- If a object property name contains characters not allowed in CEL identifiers it is escaped using these rules: - - `.` (period) is escaped as `__dot__` - - `-` (slash) is escaped as `__slash__` - - ` ` (space) is escaped as `__space__` - - `__` (2 underscores) is escaped as `__underscores__` +- If a object property name contains characters not allowed in CEL identifiers (`[a-zA-Z_][a-zA-Z0-9_]*`) it is escaped using these rules: + - Property names starting with a number are prefixed by `_`. Property names prefixed with `_` + followed by a number are prefixed with `__` and the number. + - `__` (2 underscores) is escaped as `__underscores__` (and is used as the escape char for the below rules) + - All characters except `[a-zA-Z0-9]` are escaped either as `__{symbolName}__` or `__0x{unicodeHex}__`, the recognized symbol names are: + - `dot` (`.`) + - `dash` (`-`) + - `space` (` `) + - `dollar` (`$`) + - `slash` (`/`) - Rules may be written at the root of an object, and may make field selection into any fields declared in the OpenAPIv3 schema of the CRD as well as `apiVersion`, `kind`, `metadata.name` and From 0843c097a49f8093f07f05102d7e3257f87ec6ef Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Tue, 9 Nov 2021 09:20:20 -0800 Subject: [PATCH 06/15] Update keps/sig-api-machinery/2876-crd-validation-expression-language/README.md Co-authored-by: Jordan Liggitt --- .../2876-crd-validation-expression-language/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md index af1f0187299..cd48f517bea 100644 --- a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md +++ b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md @@ -506,7 +506,7 @@ Types: | 'object' with AdditionalProperties | map | | 'object' with x-kubernetes-embedded-type | | | 'object' with x-kubernetes-preserve-unknown-fields | | -| x-kubernetes-embedded-int-or-string | object with 'intVal' (type: int) and 'strVal' (type: string) fields | +| x-kubernetes-int-or-string | object with 'intVal' (type: int) and 'strVal' (type: string) fields | | 'array | list | | 'array' with x-kubernetes-list-type=map | list with map based Equality & unique key guarantees | | 'array' with x-kubernetes-list-type=set | list with set based Equality & unique entry guarantees | From 71308bb16651669df71f7e45a2a722baea1a1b44 Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Tue, 9 Nov 2021 15:58:52 -0500 Subject: [PATCH 07/15] update escaping rules --- .../README.md | 46 +++++++++++++------ 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md index cd48f517bea..63ec58a6b01 100644 --- a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md +++ b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md @@ -226,6 +226,22 @@ kind: CustomResourceDefinition type: integer ``` +Example Validation Rules: + +| Rule | Purpose | +| ---------------- | ------------ | +| `minReplicas <= replicas <= maxReplicas` | Validate that the three fields defining replicas are ordered appropriately | +| `'Available' in stateCounts` | Validate the 'Available' key exists in a map | +| `(size(list1) == 0) != (size(list2) == 0)` | Validate that one of two lists is non-empty, but not both | +| `created + ttl < expiry` | Validate that 'expiry' date is after a 'create' date plus a 'ttl' duration | +| `health.startsWith('ok')` | Validate a 'health' string field has the prefix 'ok' | +| `widgets.exists(w, w.key == 'x' && w.foo < 10)` | Validate that the 'foo' property of a listMap item with a key 'x' is less than 10 | +| `type(limit) == string ? limit == '100%' : limit == 1000` | Validate an int-or-string field for both the the int and string cases | +| `metadata.name == 'singleton` | Validate that an object's name matches a specific value (making it a singleton) | +| `set1.all(e, !(e in set2))` | Validate that two listSets are disjoint | +| `size(names) == size(details) && names.all(n, n in details)` | Validate the 'details' map is keyed by the items in the names listSet | + + - Each validator may have multiple validation rules. - Each validation rule has an optional 'message' field for the error message that @@ -297,22 +313,25 @@ like the `all` macro, e.g. `self.all(listItem, )` or `self.all(mapKey identifiers](https://github.com/google/cel-spec/blob/master/doc/langdef.md#values)) it is not accessible as a root variable and must be accessed via `self`, .e.g. `self.int`. -- If a object property name contains characters not allowed in CEL identifiers (`[a-zA-Z_][a-zA-Z0-9_]*`) it is escaped using these rules: - - Property names starting with a number are prefixed by `_`. Property names prefixed with `_` - followed by a number are prefixed with `__` and the number. - - `__` (2 underscores) is escaped as `__underscores__` (and is used as the escape char for the below rules) - - All characters except `[a-zA-Z0-9]` are escaped either as `__{symbolName}__` or `__0x{unicodeHex}__`, the recognized symbol names are: - - `dot` (`.`) - - `dash` (`-`) - - `space` (` `) - - `dollar` (`$`) - - `slash` (`/`) +- Only property names of the form `[a-zA-Z_.-/][a-zA-Z0-9_.-/]*` are accessible. + If a property name is "self" or matches with a [reserved language identifier](https://github.com/google/cel-spec/blob/v0.6.0/doc/langdef.md#values) + (`int`, `uint`, `double`, `bool`, `string`, `bytes`, `list`, `map`, `null_type`, `type`), it is + not escaped, but it is excluded from the bound variables and can only be accessed via + "self.{property name}". All other accessible property names are escaped according to the following rules + when accessed in the expression: + - '__' escapes to '__underscores__' + - '.' escapes to '__dot__' + - '-' escapes to '__dash__' + - '/' escapes to '__slash__' + - CEL RESERVED keywords escape to '__{keyword}__'. The keywords are: "true", "false", "null", + "in", "as", "break", "const", "continue", "else", "for", "function", "if", "import", "let", + "loop", "package", "namespace", "return". - Rules may be written at the root of an object, and may make field selection into any fields declared in the OpenAPIv3 schema of the CRD as well as `apiVersion`, `kind`, `metadata.name` and `metadata.generateName`. This includes selection of fields in both the `spec` and `status` in the same expression, e.g. `status.quantity <= spec.maxQuantity`. Because CRDs only allow the `name` - and `generatName` to be declared in the `metadata` of an object, these are the only metadata + and `generateName` to be declared in the `metadata` of an object, these are the only metadata fields that may be validated using CEL validator rules. For example, `metadata.name.endsWith('mySuffix')` is allowed, but `size(metadata.labels) < 3` it not allowed. The limit on which `metadata` fields may be validated is an intentional design choice @@ -506,7 +525,7 @@ Types: | 'object' with AdditionalProperties | map | | 'object' with x-kubernetes-embedded-type | | | 'object' with x-kubernetes-preserve-unknown-fields | | -| x-kubernetes-int-or-string | object with 'intVal' (type: int) and 'strVal' (type: string) fields | +| x-kubernetes-int-or-string | dynamic object that is either an int or a string, `type(value)` can be used to check the type | | 'array | list | | 'array' with x-kubernetes-list-type=map | list with map based Equality & unique key guarantees | | 'array' with x-kubernetes-list-type=set | list with set based Equality & unique entry guarantees | @@ -594,9 +613,10 @@ developers to test their validation rules. #### Beta +- x-kubernetes-int-or-string is upgraded to use a union type of just int or string, not a dynamic type (CEL go support is planned in lates 2021) - Understanding of upper bounds of CPU/memory usage and appropriate limits set to prevent abuse. - Build-in macro/function library is comprehensive and stable (any changes to this will be a breaking change) -- CEL numeric comparison issue is resolved +- CEL numeric comparison issue is resolved (e.g. ability to compare ints to doubles) ## Production Readiness Review Questionnaire From 8bce013a9a0bf549f6e958ca6dadce598240f143 Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Thu, 11 Nov 2021 14:46:07 -0500 Subject: [PATCH 08/15] remove access to object fields as root bound variables --- .../README.md | 76 ++++++++----------- 1 file changed, 32 insertions(+), 44 deletions(-) diff --git a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md index 63ec58a6b01..3116da94774 100644 --- a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md +++ b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md @@ -11,7 +11,6 @@ - [Goals](#goals) - [Non-Goals](#non-goals) - [Proposal](#proposal) - - [Field paths and field patterns](#field-paths-and-field-patterns) - [Expression lifecycle](#expression-lifecycle) - [Function library](#function-library) - [User Stories](#user-stories) @@ -140,7 +139,7 @@ suited or insufficient. These improvements are largely complementary to expression support and either are (or should be) addressed by in separate KEPs. -For use cases cannot be covered by build-in validation support: +For use cases that cannot be covered by build-in validation support: - Admission Webhooks: have validating admission webhook for further validation - Custom validators: write custom checks in several languages such as Rego @@ -216,7 +215,7 @@ kind: CustomResourceDefinition properties: spec: x-kubernetes-validations: - - rule: "minReplicas <= maxReplicas" + - rule: "self.minReplicas <= self.maxReplicas" message: "minReplicas cannot be larger than maxReplicas" type: object properties: @@ -228,18 +227,18 @@ kind: CustomResourceDefinition Example Validation Rules: -| Rule | Purpose | -| ---------------- | ------------ | -| `minReplicas <= replicas <= maxReplicas` | Validate that the three fields defining replicas are ordered appropriately | -| `'Available' in stateCounts` | Validate the 'Available' key exists in a map | -| `(size(list1) == 0) != (size(list2) == 0)` | Validate that one of two lists is non-empty, but not both | -| `created + ttl < expiry` | Validate that 'expiry' date is after a 'create' date plus a 'ttl' duration | -| `health.startsWith('ok')` | Validate a 'health' string field has the prefix 'ok' | -| `widgets.exists(w, w.key == 'x' && w.foo < 10)` | Validate that the 'foo' property of a listMap item with a key 'x' is less than 10 | -| `type(limit) == string ? limit == '100%' : limit == 1000` | Validate an int-or-string field for both the the int and string cases | -| `metadata.name == 'singleton` | Validate that an object's name matches a specific value (making it a singleton) | -| `set1.all(e, !(e in set2))` | Validate that two listSets are disjoint | -| `size(names) == size(details) && names.all(n, n in details)` | Validate the 'details' map is keyed by the items in the names listSet | +| Rule | Purpose | +| ---------------- | ------------ | +| `self.minReplicas <= self.replicas <= self.maxReplicas` | Validate that the three fields defining replicas are ordered appropriately | +| `'Available' in self.stateCounts` | Validate that an entry with the 'Available' key exists in a map | +| `(size(self.list1) == 0) != (size(self.list2) == 0)` | Validate that one of two lists is non-empty, but not both | +| `has(self.expired) && self.created + self.ttl < self.expired` | Validate that 'expired' date is after a 'create' date plus a 'ttl' duration | +| `self.health.startsWith('ok')` | Validate a 'health' string field has the prefix 'ok' | +| `self.widgets.exists(w, w.key == 'x' && w.foo < 10)` | Validate that the 'foo' property of a listMap item with a key 'x' is less than 10 | +| `type(self.limit) == string ? self.limit == '100%' : self.limit == 1000` | Validate an int-or-string field for both the the int and string cases | +| `self.metadata.name == 'singleton` | Validate that an object's name matches a specific value (making it a singleton) | +| `self.set1.all(e, !(e in self.set2))` | Validate that two listSets are disjoint | +| `size(self.names) == size(self.details) && self.names.all(n, n in self.details)` | Validate the 'details' map is keyed by the items in the 'names' listSet | - Each validator may have multiple validation rules. @@ -279,12 +278,13 @@ is scoped to. It will cause a lot of keywords to be reserved and users have to memorize those variable when writing rules. - Using other names like `this`, `me`, `value`, `_`. The name should be self-explanatory, less chance of conflict and easy to be picked up. -- For OpenAPIv3 object types, the expression will have direct access to all the -fields of the object the validator is scoped to. + +- For OpenAPIv3 object types, the expression may use field selection to access all the + properties of the object the validator is scoped to, e.g. `self.field == 10`. - For OpenAPIv3 scalar types (integer, string & boolean), the expression will have access to the -scalar data element the validator is scoped to. The data element will be accessible to CEL -expressions via `self`, e.g. `len(self) > 10`. + scalar data element the validator is scoped to. The data element will be accessible to CEL + expressions via `self`, e.g. `len(self) > 10`. - For OpenAPIv3 list and map types, the expression will have access to the data element of the list or map. These will be accessible to CEL via `self`. The elements of a map or list can be validated using the CEL support for collections @@ -292,26 +292,20 @@ like the `all` macro, e.g. `self.all(listItem, )` or `self.all(mapKey )`. - For immutability use case, validator will have access to the existing version of the object. This - will be accessible to CEL via the `old` identifier. + will be accessible to CEL via the `oldSelf` identifier. - This will only be available on mergable collection types such as objects (unless `x-kubernetes-map-type=atomic`), maps with `x-kubernetes-map-type=granular` and lists with `x-kubernetes-list-type` set to `set` or `map`. See [Merge Strategy](https://kubernetes.io/docs/reference/using-api/server-side-apply/#merge-strategy) for details. - The use of "old" is congruent with how `AdmissionReview` identifies the existing object as - `oldObject`. To avoid name collisions `old` will be treated the same as a CEL - keyword for escaping purposes (see below). + `oldObject`. - xref [analysis of possible interactions with immutability and validation](https://github.com/kubernetes/enhancements/tree/master/keps/sig-api-machinery/1101-immutable-fields#openapi-extension-x-kubernetes-immutable). - If a object property name is a CEL keyword (see RESERVED in [CEL Syntax](https://github.com/google/cel-spec/blob/master/doc/langdef.md#syntax)), it will be escaped by prepending a _ prefix. To prevent this from causing a subsequent collision, properties named with a CEL keyword and a `_` prefix will be prefixed by `__` (generally, N+1 the existing number of `_`s). - -- If a object property name is a CEL language identifier (`int`, `uint`, `double`, `bool`, `string`, - `bytes`, `list`, `map`, `null_type`, `type`, see [CEL language - identifiers](https://github.com/google/cel-spec/blob/master/doc/langdef.md#values)) it is not - accessible as a root variable and must be accessed via `self`, .e.g. `self.int`. - Only property names of the form `[a-zA-Z_.-/][a-zA-Z0-9_.-/]*` are accessible. If a property name is "self" or matches with a [reserved language identifier](https://github.com/google/cel-spec/blob/v0.6.0/doc/langdef.md#values) @@ -330,10 +324,10 @@ like the `all` macro, e.g. `self.all(listItem, )` or `self.all(mapKey - Rules may be written at the root of an object, and may make field selection into any fields declared in the OpenAPIv3 schema of the CRD as well as `apiVersion`, `kind`, `metadata.name` and `metadata.generateName`. This includes selection of fields in both the `spec` and `status` in the - same expression, e.g. `status.quantity <= spec.maxQuantity`. Because CRDs only allow the `name` + same expression, e.g. `self.status.quantity <= self.spec.maxQuantity`. Because CRDs only allow the `name` and `generateName` to be declared in the `metadata` of an object, these are the only metadata fields that may be validated using CEL validator rules. For example, - `metadata.name.endsWith('mySuffix')` is allowed, but `size(metadata.labels) < 3` it not + `self.metadata.name.endsWith('mySuffix')` is allowed, but `size(self.metadata.labels) < 3` it not allowed. The limit on which `metadata` fields may be validated is an intentional design choice (that aims to keep metadata behavior uniform across types) and applies to all validation mechanisms (e.g. the OpenAPIV3 `maxItems` restriction), not just CEL validator rules. @@ -349,14 +343,6 @@ like the `all` macro, e.g. `self.all(listItem, )` or `self.all(mapKey ([xref](https://github.com/jinmmin/cel-go/blob/a661c99f8e27676c70fc00f4f328476ca4dcdb7f/cel/program.go#L265)) during CRD update to bound complexity. -#### Field paths and field patterns - -A field path is a patch to a single node in the data tree. I.e. it specifies the -exact indices of the list items and the keys of map entries it traverses. - -A field *pattern* is a path to all nodes in the data tree that match the pattern. I.e. -it may wildcard list item and map keys. - #### Expression lifecycle When CRDs are written to the kube-apiserver, all expressions will be [parsed and @@ -584,18 +570,20 @@ The initial changes made in the type integration will be: - We couldn't have both: (a) equality of string keys, which implies a canonical ordering of key fields, and (b) ability for developers to successfully lookup a map entry regardless of how they order the keys in the string representation -So instead of treating "associative lists" as maps in CEL, we will continue to treat them as lists, but override equality to ignore object order (i.e. use map equality semantics) and introduce utility functions to make the list representation easy to use in CEL. E.g. instead of: +So instead of treating "associative lists" as maps in CEL, we will continue to treat them as lists, but override equality to ignore object order (i.e. use map equality semantics). + +Looking up entiries by keys is available primarily via the `exists_one` and `filter` macros. Examples: ``` -associativeList.filter(e, e.key1 == 'a' && e.key2 == 'b').all(e, e.val == 100) -``` +// exists_one() and exists() behave similarly if all map keys are checked, but exsists_one() has slightly stricter +// semantics, which make it preferable -We plan to add a `get()` builtin function (exact name and semantics TBD) that allows for lookup of a single map entry: +// To check if the map contains a entry with a particular key: +associativeList.exists_one(e, e.key1 == 'a' && e.key2 == 'b') +// To lookup a map entry by key and check if some condition is met on the other fields of the entry: +associativeList.exists_one(e, e.key1 == 'a' && e.key2 == 'b' && e.val == 100) ``` -associativeList.get(e, e.key1 == 'a' && e.key2 == 'b').val == 100 -``` - ### Test Plan From 645a7ec0e3bb7bf29d63f6afe3829e9919eb1a16 Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Fri, 12 Nov 2021 11:06:20 -0800 Subject: [PATCH 09/15] Clean up examples Co-authored-by: Jordan Liggitt --- .../2876-crd-validation-expression-language/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md index 3116da94774..36a68bac786 100644 --- a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md +++ b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md @@ -235,8 +235,8 @@ Example Validation Rules: | `has(self.expired) && self.created + self.ttl < self.expired` | Validate that 'expired' date is after a 'create' date plus a 'ttl' duration | | `self.health.startsWith('ok')` | Validate a 'health' string field has the prefix 'ok' | | `self.widgets.exists(w, w.key == 'x' && w.foo < 10)` | Validate that the 'foo' property of a listMap item with a key 'x' is less than 10 | -| `type(self.limit) == string ? self.limit == '100%' : self.limit == 1000` | Validate an int-or-string field for both the the int and string cases | -| `self.metadata.name == 'singleton` | Validate that an object's name matches a specific value (making it a singleton) | +| `type(self) == string ? self == '100%' : self == 1000` | Validate an int-or-string field for both the the int and string cases | +| `self.metadata.name == 'singleton'` | Validate that an object's name matches a specific value (making it a singleton) | | `self.set1.all(e, !(e in self.set2))` | Validate that two listSets are disjoint | | `size(self.names) == size(self.details) && self.names.all(n, n in self.details)` | Validate the 'details' map is keyed by the items in the 'names' listSet | @@ -575,7 +575,7 @@ So instead of treating "associative lists" as maps in CEL, we will continue to t Looking up entiries by keys is available primarily via the `exists_one` and `filter` macros. Examples: ``` -// exists_one() and exists() behave similarly if all map keys are checked, but exsists_one() has slightly stricter +// exists_one() and exists() behave similarly if all map keys are checked, but exists_one() has slightly stricter // semantics, which make it preferable // To check if the map contains a entry with a particular key: From e73e7e35df1f2227c3b3328e3b8494241a4fb578 Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Fri, 12 Nov 2021 14:41:42 -0500 Subject: [PATCH 10/15] Add more examples, remove obsolete info, add note about oldSelf in graduation criteria --- .../README.md | 29 +++++++++---------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md index 36a68bac786..53f13cf1e29 100644 --- a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md +++ b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md @@ -232,6 +232,7 @@ Example Validation Rules: | `self.minReplicas <= self.replicas <= self.maxReplicas` | Validate that the three fields defining replicas are ordered appropriately | | `'Available' in self.stateCounts` | Validate that an entry with the 'Available' key exists in a map | | `(size(self.list1) == 0) != (size(self.list2) == 0)` | Validate that one of two lists is non-empty, but not both | +| `!('MY_ENV' in self.envars) || self['MY_ENV'].matches('^[a-zA-Z]*$')` | Validate the value of a map for a specific key, if it is in the map | | `has(self.expired) && self.created + self.ttl < self.expired` | Validate that 'expired' date is after a 'create' date plus a 'ttl' duration | | `self.health.startsWith('ok')` | Validate a 'health' string field has the prefix 'ok' | | `self.widgets.exists(w, w.key == 'x' && w.foo < 10)` | Validate that the 'foo' property of a listMap item with a key 'x' is less than 10 | @@ -257,7 +258,7 @@ is scoped to. name. As the example showing below. When we want to put cel validation on ToySpec, the field name as `spec` has not been identified yet which makes rule hard to define. - ```azure + ``` // +kubebuilder:validation:XValidator= type ToySpec struct { fieldSample string `json:"fieldSample"` @@ -307,30 +308,27 @@ like the `all` macro, e.g. `self.all(listItem, )` or `self.all(mapKey it will be escaped by prepending a _ prefix. To prevent this from causing a subsequent collision, properties named with a CEL keyword and a `_` prefix will be prefixed by `__` (generally, N+1 the existing number of `_`s). -- Only property names of the form `[a-zA-Z_.-/][a-zA-Z0-9_.-/]*` are accessible. - If a property name is "self" or matches with a [reserved language identifier](https://github.com/google/cel-spec/blob/v0.6.0/doc/langdef.md#values) - (`int`, `uint`, `double`, `bool`, `string`, `bytes`, `list`, `map`, `null_type`, `type`), it is - not escaped, but it is excluded from the bound variables and can only be accessed via - "self.{property name}". All other accessible property names are escaped according to the following rules - when accessed in the expression: +- Only property names of the form `[a-zA-Z_.-/][a-zA-Z0-9_.-/]*` are accessible and are escaped + according to the following rules when accessed in the expression: - '__' escapes to '__underscores__' - '.' escapes to '__dot__' - '-' escapes to '__dash__' - '/' escapes to '__slash__' - - CEL RESERVED keywords escape to '__{keyword}__'. The keywords are: "true", "false", "null", - "in", "as", "break", "const", "continue", "else", "for", "function", "if", "import", "let", - "loop", "package", "namespace", "return". + - Property names that match a CEL RESERVED keyword exactly escape to '__{keyword}__'. The + keywords are: "true", "false", "null", "in", "as", "break", "const", "continue", "else", "for", + "function", "if", "import", "let", "loop", "package", "namespace", "return". - Rules may be written at the root of an object, and may make field selection into any fields declared in the OpenAPIv3 schema of the CRD as well as `apiVersion`, `kind`, `metadata.name` and `metadata.generateName`. This includes selection of fields in both the `spec` and `status` in the - same expression, e.g. `self.status.quantity <= self.spec.maxQuantity`. Because CRDs only allow the `name` - and `generateName` to be declared in the `metadata` of an object, these are the only metadata - fields that may be validated using CEL validator rules. For example, + same expression, e.g. `self.status.quantity <= self.spec.maxQuantity`. Because CRDs only allow the + `name` and `generateName` to be declared in the `metadata` of an object, these are the only + metadata fields that may be validated using CEL validator rules. For example, `self.metadata.name.endsWith('mySuffix')` is allowed, but `size(self.metadata.labels) < 3` it not allowed. The limit on which `metadata` fields may be validated is an intentional design choice - (that aims to keep metadata behavior uniform across types) and applies to all validation - mechanisms (e.g. the OpenAPIV3 `maxItems` restriction), not just CEL validator rules. + (that aims to allow for generic access to labels and annotations across all kinds) and applies to + all validation mechanisms (e.g. the OpenAPIV3 `maxItems` restriction), not just CEL validator + rules. xref rule 4 in [specifying a structural schema](https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#specifying-a-structural-schema). - We plan to allow access to the current state of the object to allow validation rules to check the new value against the current value, e.g. for immutability checks (for validation racheting we would @@ -601,6 +599,7 @@ developers to test their validation rules. #### Beta +- Resolve topic of what support we should provide for access to the previous versions of object (ie. 'oldSelf' feature) - x-kubernetes-int-or-string is upgraded to use a union type of just int or string, not a dynamic type (CEL go support is planned in lates 2021) - Understanding of upper bounds of CPU/memory usage and appropriate limits set to prevent abuse. - Build-in macro/function library is comprehensive and stable (any changes to this will be a breaking change) From 49d894e8267851a633a224e922be1dfeeb94204a Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Fri, 12 Nov 2021 16:20:15 -0500 Subject: [PATCH 11/15] Add example for listMap value checking for a specific key --- .../2876-crd-validation-expression-language/README.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md index 53f13cf1e29..445327a0563 100644 --- a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md +++ b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md @@ -232,12 +232,13 @@ Example Validation Rules: | `self.minReplicas <= self.replicas <= self.maxReplicas` | Validate that the three fields defining replicas are ordered appropriately | | `'Available' in self.stateCounts` | Validate that an entry with the 'Available' key exists in a map | | `(size(self.list1) == 0) != (size(self.list2) == 0)` | Validate that one of two lists is non-empty, but not both | -| `!('MY_ENV' in self.envars) || self['MY_ENV'].matches('^[a-zA-Z]*$')` | Validate the value of a map for a specific key, if it is in the map | +| `!('MY_KEY' in self.map1) || self['MY_KEY].matches('^[a-zA-Z]*$')` | Validate the value of a map for a specific key, if it is in the map | +| `self.envars.filter(e, e.name = 'MY_ENV').all(e, e.value.matches('^[a-zA-Z]*$')` | Validate the 'value' field of a listMap entry where key field 'name' is 'MY_ENV' | | `has(self.expired) && self.created + self.ttl < self.expired` | Validate that 'expired' date is after a 'create' date plus a 'ttl' duration | | `self.health.startsWith('ok')` | Validate a 'health' string field has the prefix 'ok' | | `self.widgets.exists(w, w.key == 'x' && w.foo < 10)` | Validate that the 'foo' property of a listMap item with a key 'x' is less than 10 | -| `type(self) == string ? self == '100%' : self == 1000` | Validate an int-or-string field for both the the int and string cases | -| `self.metadata.name == 'singleton'` | Validate that an object's name matches a specific value (making it a singleton) | +| `type(self) == string ? self == '100%' : self == 1000` | Validate an int-or-string field for both the the int and string cases | +| `self.metadata.name == 'singleton'` | Validate that an object's name matches a specific value (making it a singleton) | | `self.set1.all(e, !(e in self.set2))` | Validate that two listSets are disjoint | | `size(self.names) == size(self.details) && self.names.all(n, n in self.details)` | Validate the 'details' map is keyed by the items in the 'names' listSet | From 988e26da075d9d92b7598058b8b20a6f2ffff1be Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Mon, 15 Nov 2021 21:11:15 -0500 Subject: [PATCH 12/15] Add follow up issues found during 1.23 alpha implementation to beta graduation criteria --- .../2876-crd-validation-expression-language/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md index 445327a0563..6de70b33e6e 100644 --- a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md +++ b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md @@ -605,6 +605,8 @@ developers to test their validation rules. - Understanding of upper bounds of CPU/memory usage and appropriate limits set to prevent abuse. - Build-in macro/function library is comprehensive and stable (any changes to this will be a breaking change) - CEL numeric comparison issue is resolved (e.g. ability to compare ints to doubles) +- [Reduce noise of invalid data messages reported from cel.UnstructuredToVal](https://github.com/kubernetes/kubernetes/issues/106440) +- [Benchmark cel.UnstructuredToVal and optimize away repeated wrapper object construction](https://github.com/kubernetes/kubernetes/issues/106438) ## Production Readiness Review Questionnaire From 02e96028209b4f56793f0004cf635c02813b95be Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Tue, 16 Nov 2021 12:26:42 -0500 Subject: [PATCH 13/15] Update keps/sig-api-machinery/2876-crd-validation-expression-language/README.md Co-authored-by: Chris Bandy --- .../2876-crd-validation-expression-language/README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md index 6de70b33e6e..bd3e86543da 100644 --- a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md +++ b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md @@ -311,11 +311,11 @@ like the `all` macro, e.g. `self.all(listItem, )` or `self.all(mapKey - Only property names of the form `[a-zA-Z_.-/][a-zA-Z0-9_.-/]*` are accessible and are escaped according to the following rules when accessed in the expression: - - '__' escapes to '__underscores__' - - '.' escapes to '__dot__' - - '-' escapes to '__dash__' - - '/' escapes to '__slash__' - - Property names that match a CEL RESERVED keyword exactly escape to '__{keyword}__'. The + - `__` escapes to `__underscores__` + - `.` escapes to `__dot__` + - `-` escapes to `__dash__` + - `/` escapes to `__slash__` + - Property names that match a CEL RESERVED keyword exactly escape to `__{keyword}__`. The keywords are: "true", "false", "null", "in", "as", "break", "const", "continue", "else", "for", "function", "if", "import", "let", "loop", "package", "namespace", "return". From 4adef382ded9e13bce61ce59047d771de7638a7e Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Tue, 16 Nov 2021 12:28:30 -0500 Subject: [PATCH 14/15] Remove obsolete escaping rule info --- .../README.md | 48 +++++++++---------- .../kep.yaml | 3 +- 2 files changed, 24 insertions(+), 27 deletions(-) diff --git a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md index bd3e86543da..511c4c36aa7 100644 --- a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md +++ b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md @@ -229,7 +229,7 @@ Example Validation Rules: | Rule | Purpose | | ---------------- | ------------ | -| `self.minReplicas <= self.replicas <= self.maxReplicas` | Validate that the three fields defining replicas are ordered appropriately | +| `self.minReplicas <= self.replicas && self.replicas <= self.maxReplicas` | Validate that the three fields defining replicas are ordered appropriately | | `'Available' in self.stateCounts` | Validate that an entry with the 'Available' key exists in a map | | `(size(self.list1) == 0) != (size(self.list2) == 0)` | Validate that one of two lists is non-empty, but not both | | `!('MY_KEY' in self.map1) || self['MY_KEY].matches('^[a-zA-Z]*$')` | Validate the value of a map for a specific key, if it is in the map | @@ -305,10 +305,6 @@ like the `all` macro, e.g. `self.all(listItem, )` or `self.all(mapKey - xref [analysis of possible interactions with immutability and validation](https://github.com/kubernetes/enhancements/tree/master/keps/sig-api-machinery/1101-immutable-fields#openapi-extension-x-kubernetes-immutable). -- If a object property name is a CEL keyword (see RESERVED in [CEL Syntax](https://github.com/google/cel-spec/blob/master/doc/langdef.md#syntax)), - it will be escaped by prepending a _ prefix. To prevent this from causing a subsequent collision, properties named with a CEL keyword and a `_` prefix will be - prefixed by `__` (generally, N+1 the existing number of `_`s). - - Only property names of the form `[a-zA-Z_.-/][a-zA-Z0-9_.-/]*` are accessible and are escaped according to the following rules when accessed in the expression: - `__` escapes to `__underscores__` @@ -504,32 +500,32 @@ coverage of interactions in these dimensions: Types: -| OpenAPIv3 type | CEL type | -| -------------------------------------------------- | -------------------------------------------------------------------------------------------------------------- | -| 'object' with Properties | object / "message type" | -| 'object' with AdditionalProperties | map | -| 'object' with x-kubernetes-embedded-type | | -| 'object' with x-kubernetes-preserve-unknown-fields | | -| x-kubernetes-int-or-string | dynamic object that is either an int or a string, `type(value)` can be used to check the type | -| 'array | list | -| 'array' with x-kubernetes-list-type=map | list with map based Equality & unique key guarantees | -| 'array' with x-kubernetes-list-type=set | list with set based Equality & unique entry guarantees | -| 'boolean' | boolean | -| 'number' (all formats) | double | -| 'integer' (all formats) | int (64) | -| 'null' | null_type | -| 'string' | string | -| 'string' with format=byte (base64 encoded) | bytes | -| 'string' with format=date | timestamp (google.protobuf.Timestamp) | -| 'string' with format=datetime | timestamp (google.protobuf.Timestamp) | -| 'string' with format=duration | duration (google.protobuf.Duration) | +| OpenAPIv3 type | CEL type | +| -------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------- | +| 'object' with Properties | object / "message type" | +| 'object' with AdditionalProperties | map | +| 'object' with x-kubernetes-embedded-type | object / "message type", 'apiVersion', 'kind', 'metadata.name' and 'metadata.generateName' are implicitly included in schema | +| 'object' with x-kubernetes-preserve-unknown-fields | object / "message type", unknown fields are NOT accessible in CEL expression | +| x-kubernetes-int-or-string | dynamic object that is either an int or a string, `type(value)` can be used to check the type | +| 'array | list | +| 'array' with x-kubernetes-list-type=map | list with map based Equality & unique key guarantees | +| 'array' with x-kubernetes-list-type=set | list with set based Equality & unique entry guarantees | +| 'boolean' | boolean | +| 'number' (all formats) | double | +| 'integer' (all formats) | int (64) | +| 'null' | null_type | +| 'string' | string | +| 'string' with format=byte (base64 encoded) | bytes | +| 'string' with format=date | timestamp (google.protobuf.Timestamp) | +| 'string' with format=datetime | timestamp (google.protobuf.Timestamp) | +| 'string' with format=duration | duration (google.protobuf.Duration) | xref: [CEL types](https://github.com/google/cel-spec/blob/master/doc/langdef.md#values), [OpenAPI types](https://swagger.io/specification/#data-types), [Kubernetes Structural Schemas](https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/#specifying-a-structural-schema). Although `x-kubernetes-preserve-unknown-fields` allows custom resources to contain values without -corresponding schema information, we will not provide validation support of these "schemaless" -values. Reasons for this include: +corresponding schema information, we will not provide access to these "schemaless" values in CEL +expressions. Reasons for this include: - Without schema information, types (e.g. `map` vs. `object`), formats (e.g. plain `string` vs. `date`) and list types (plain `list` vs. `set`) are not available, and this feature depends diff --git a/keps/sig-api-machinery/2876-crd-validation-expression-language/kep.yaml b/keps/sig-api-machinery/2876-crd-validation-expression-language/kep.yaml index 4213931fada..e85b7419c34 100644 --- a/keps/sig-api-machinery/2876-crd-validation-expression-language/kep.yaml +++ b/keps/sig-api-machinery/2876-crd-validation-expression-language/kep.yaml @@ -6,7 +6,7 @@ authors: - "@DangerOnTheRanger" - "@leilajal" owning-sig: sig-api-machinery -status: implementable +status: implemented creation-date: 2021-05-26 reviewers: - "@deads2k" @@ -16,6 +16,7 @@ reviewers: approvers: - "@deads2k" - "@lavalamp" + - "@liggitt" ##### WARNING !!! ###### # prr-approvers has been moved to its own location From b2a668453d718dd251283bdc5b92a6c47cb36233 Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Fri, 19 Nov 2021 15:17:47 -0500 Subject: [PATCH 15/15] Address feedback, clarify beta usage and adoption expectations --- .../README.md | 58 +++++++++++-------- 1 file changed, 34 insertions(+), 24 deletions(-) diff --git a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md index 511c4c36aa7..ddee452fa7d 100644 --- a/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md +++ b/keps/sig-api-machinery/2876-crd-validation-expression-language/README.md @@ -227,20 +227,20 @@ kind: CustomResourceDefinition Example Validation Rules: -| Rule | Purpose | -| ---------------- | ------------ | -| `self.minReplicas <= self.replicas && self.replicas <= self.maxReplicas` | Validate that the three fields defining replicas are ordered appropriately | -| `'Available' in self.stateCounts` | Validate that an entry with the 'Available' key exists in a map | -| `(size(self.list1) == 0) != (size(self.list2) == 0)` | Validate that one of two lists is non-empty, but not both | -| `!('MY_KEY' in self.map1) || self['MY_KEY].matches('^[a-zA-Z]*$')` | Validate the value of a map for a specific key, if it is in the map | -| `self.envars.filter(e, e.name = 'MY_ENV').all(e, e.value.matches('^[a-zA-Z]*$')` | Validate the 'value' field of a listMap entry where key field 'name' is 'MY_ENV' | -| `has(self.expired) && self.created + self.ttl < self.expired` | Validate that 'expired' date is after a 'create' date plus a 'ttl' duration | -| `self.health.startsWith('ok')` | Validate a 'health' string field has the prefix 'ok' | -| `self.widgets.exists(w, w.key == 'x' && w.foo < 10)` | Validate that the 'foo' property of a listMap item with a key 'x' is less than 10 | -| `type(self) == string ? self == '100%' : self == 1000` | Validate an int-or-string field for both the the int and string cases | -| `self.metadata.name == 'singleton'` | Validate that an object's name matches a specific value (making it a singleton) | -| `self.set1.all(e, !(e in self.set2))` | Validate that two listSets are disjoint | -| `size(self.names) == size(self.details) && self.names.all(n, n in self.details)` | Validate the 'details' map is keyed by the items in the 'names' listSet | +| Rule | Purpose | +| ---------------- | ------------ | +| `self.minReplicas <= self.replicas && self.replicas <= self.maxReplicas` | Validate that the three fields defining replicas are ordered appropriately | +| `'Available' in self.stateCounts` | Validate that an entry with the 'Available' key exists in a map | +| `(self.list1.size() == 0) != self.list2.size() == 0)` | Validate that one of two lists is non-empty, but not both | +| `!('MY_KEY' in self.map1) \|\| self['MY_KEY'].matches('^[a-zA-Z]*$')` | Validate the value of a map for a specific key, if it is in the map | +| `self.envars.filter(e, e.name = 'MY_ENV').all(e, e.value.matches('^[a-zA-Z]*$')` | Validate the 'value' field of a listMap entry where key field 'name' is 'MY_ENV' | +| `has(self.expired) && self.created + self.ttl < self.expired` | Validate that 'expired' date is after a 'create' date plus a 'ttl' duration | +| `self.health.startsWith('ok')` | Validate a 'health' string field has the prefix 'ok' | +| `self.widgets.exists(w, w.key == 'x' && w.foo < 10)` | Validate that the 'foo' property of a listMap item with a key 'x' is less than 10 | +| `type(self) == string ? self == '100%' : self == 1000` | Validate an int-or-string field for both the the int and string cases | +| `self.metadata.name == 'singleton'` | Validate that an object's name matches a specific value (making it a singleton) | +| `self.set1.all(e, !(e in self.set2))` | Validate that two listSets are disjoint | +| `self.names.size() == self.details.size() && self.names.all(n, n in self.details)` | Validate the 'details' map is keyed by the items in the 'names' listSet | - Each validator may have multiple validation rules. @@ -286,7 +286,7 @@ is scoped to. - For OpenAPIv3 scalar types (integer, string & boolean), the expression will have access to the scalar data element the validator is scoped to. The data element will be accessible to CEL - expressions via `self`, e.g. `len(self) > 10`. + expressions via `self`, e.g. `self.size() > 10`. - For OpenAPIv3 list and map types, the expression will have access to the data element of the list or map. These will be accessible to CEL via `self`. The elements of a map or list can be validated using the CEL support for collections @@ -321,7 +321,7 @@ like the `all` macro, e.g. `self.all(listItem, )` or `self.all(mapKey same expression, e.g. `self.status.quantity <= self.spec.maxQuantity`. Because CRDs only allow the `name` and `generateName` to be declared in the `metadata` of an object, these are the only metadata fields that may be validated using CEL validator rules. For example, - `self.metadata.name.endsWith('mySuffix')` is allowed, but `size(self.metadata.labels) < 3` it not + `self.metadata.name.endsWith('mySuffix')` is allowed, but `self.metadata.labels.size() < 3` it not allowed. The limit on which `metadata` fields may be validated is an intentional design choice (that aims to allow for generic access to labels and annotations across all kinds) and applies to all validation mechanisms (e.g. the OpenAPIV3 `maxItems` restriction), not just CEL validator @@ -502,7 +502,7 @@ Types: | OpenAPIv3 type | CEL type | | -------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------- | -| 'object' with Properties | object / "message type" | +| 'object' with Properties | object / "message type" (`type()` evaluates to `selfType.path.to.object.from.self` | | 'object' with AdditionalProperties | map | | 'object' with x-kubernetes-embedded-type | object / "message type", 'apiVersion', 'kind', 'metadata.name' and 'metadata.generateName' are implicitly included in schema | | 'object' with x-kubernetes-preserve-unknown-fields | object / "message type", unknown fields are NOT accessible in CEL expression | @@ -513,6 +513,7 @@ Types: | 'boolean' | boolean | | 'number' (all formats) | double | | 'integer' (all formats) | int (64) | +| | uint (64) | | 'null' | null_type | | 'string' | string | | 'string' with format=byte (base64 encoded) | bytes | @@ -567,17 +568,25 @@ The initial changes made in the type integration will be: So instead of treating "associative lists" as maps in CEL, we will continue to treat them as lists, but override equality to ignore object order (i.e. use map equality semantics). -Looking up entiries by keys is available primarily via the `exists_one` and `filter` macros. Examples: +Looking up and iterating over entiries is available via the `exists`, `exists_one` and `all` macros: ``` -// exists_one() and exists() behave similarly if all map keys are checked, but exists_one() has slightly stricter -// semantics, which make it preferable +// Since there is already a guarantee that "associative lists" only one entry +// exists in the list for each key, `exists` can be used to check if a map contains a particular key instead of `exists_one`. +// Note that `exists_one` can also be used, but handles any errors encountered more strictly. +// See the CEL language spec how errors are handled by `exists_one` and `exists` for more details. -// To check if the map contains a entry with a particular key: -associativeList.exists_one(e, e.key1 == 'a' && e.key2 == 'b') +// Check if an "associative list" contains an entry for a key: +associativeList.exists(e, e.key1 == 'a' && e.key2 == 'b') -// To lookup a map entry by key and check if some condition is met on the other fields of the entry: -associativeList.exists_one(e, e.key1 == 'a' && e.key2 == 'b' && e.val == 100) +// To validate a map contains an entry with a particular key and that some condition is met on the other fields of the entry: +associativeList.exists(e, e.key1 == 'a' && e.key2 == 'b' && e.val == 100) + +// To check the value for a particular key meets some condition (but also allow the entry to be absent): +associativeList.all(e, e.key1 == 'a' && e.key2 == 'b' && e.val == 100) + +// To check some condition on all entries of an "associative list": +associativeList.all(e, e.val == 100) ``` ### Test Plan @@ -603,6 +612,7 @@ developers to test their validation rules. - CEL numeric comparison issue is resolved (e.g. ability to compare ints to doubles) - [Reduce noise of invalid data messages reported from cel.UnstructuredToVal](https://github.com/kubernetes/kubernetes/issues/106440) - [Benchmark cel.UnstructuredToVal and optimize away repeated wrapper object construction](https://github.com/kubernetes/kubernetes/issues/106438) +- Demonstrate adoption and successful feature usage in the community ## Production Readiness Review Questionnaire