Skip to content

Commit

Permalink
Update the KEP
Browse files Browse the repository at this point in the history
  • Loading branch information
@cici37
cici37 committed Jul 28, 2021
1 parent 82505d1 commit 5f2f1e1
Showing 1 changed file with 114 additions and 110 deletions.
224 changes: 114 additions & 110 deletions ...g-api-machinery/NNNN-crd-validation-defaulting-conversion-expressions/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,15 +150,14 @@ Items marked with (R) are required *prior to targeting to a milestone / release*

## Summary

CRDs need direct support for non-trivial validation and defaulting. While
admission webhooks can handle the validation and defaulting use cases that CRDs
cannot handle directly, they signficantly complicate the development and
CRDs need direct support for non-trivial validation. Comparing to the current mechanism of using
admission webhooks to handle the 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 and defaulting use cases can
CRDs such that a much larger portion of validation use cases can
be solved without the use of webhooks. Support for CRD conversions via
the expression language will also be added.
the expression language will also be added in the future.

We will use the [Common Expression Language
(CEL)](https://github.com/google/cel-go). It is sufficiently lightweight and
Expand All @@ -171,19 +170,19 @@ allowing syntax and type errors to be caught at CRD registration time.
### Descriptive, self contained CRDs

This KEP will make CRDs more self contained. Instead of having
validation/defaulting/conversion rules coded into webhooks that must be
registered and upgraded independant of a CRD, the rules will be contained within
validation rules coded into webhooks that must be
registered and upgraded independent of a CRD, the rules will be contained within
the CRD object definition, making them easier to author and introspect by
cluster administrators and users, and eliminating version skew issues that can
happen between a CRD and webhook since they can be registered and
upgraded/rolledback independently.
upgraded/rolled-back independently.

### Webhooks: Development Complexity

Introducing a production grade webhook is a substantial development task.
Beyond authoring the actual core logic that a webhook must perform, the webhook
must be instrumented for monitoring and alerting and integrated with the
packaging/repleases processes for the environments it will be run it.
packaging/releases processes for the environments it will be run it.

The developer must also carefully consider the upgrade and rollback ordering
between the webhook and CRD.
Expand All @@ -194,9 +193,9 @@ Admission webhooks are part of the critical serving path of the kube-apiserver.
Admission webhooks add latency to requests, and large numbers of webhooks
can cause, or contribute to, request timeouts being exceeded.

Webhooks must either be configured as FailPolicy.Fail or FailPolicy.Ignore. If
FailPolicy.Ignore is used, there is potential for requests skip the webhook and
be admitted. If FailPolicy.Fail is used, a webhook outage can result in a
Webhooks must either be configured as `FailPolicy.Fail` or `FailPolicy.Ignore`. If
`FailPolicy.Ignore` is used, there is potential for requests skip the webhook and
be admitted. If `FailPolicy.Fail` is used, a webhook outage can result in a
localized or widespread Kubernetes control plane outage depending on which
objects the webhook is configured to intercept.

Expand All @@ -212,7 +211,7 @@ Provide type checking of custom resources against schemas.
limits ('minimum' and 'maximum' properties) on individual fields and size limits
on maps and lists ('minItems', 'maxItems').

Additionally the API Expression WG is working KEPs that would improve CRD validation:
In addition, the API Expression WG is working on KEPs that would improve CRD validation:

- OpenAPIv3 'formats' which could (and I believe should) be leveraged by
Kubernetes to handle validation of string fields for cases where regex is poorly
Expand All @@ -223,12 +222,6 @@ suited or insufficient.
These improvements are largely complementary to expression support and either
are (or should be) addressed by in separate KEPs.

[Common Expression Language (CEL)](https://github.com/google/cel-go) is an
excellent supplement to the above because it is sufficiently expressive to
satisfy a large set of remaining uses cases that none of the above can solve.
For example, cross-field validation use cases can only be solved using
expressions or webhooks.

### Goals

- Make CRDs more self-contained and declarative
Expand All @@ -246,10 +239,13 @@ expressions or webhooks.

## Proposal

### Validation
[Common Expression Language (CEL)](https://github.com/google/cel-go) is an
excellent supplement to the current validation mechanism because it is sufficiently expressive to
satisfy a large set of remaining uses cases that none of the above can solve.
For example, cross-field validation use cases can only be solved using
expressions or webhooks.

CEL validation expressions will be allowed in CRD structural schemas via the
`x-kubernetes-validator` extension.
`x-kubernetes-validator` extension will be added to CRD structural schemas to allow CEL validation expressions.

```yaml
apiVersion: apiextensions.k8s.io/v1
Expand All @@ -261,8 +257,10 @@ kind: CustomResourceDefinition
properties:
spec:
x-kubernetes-validator:
- rule: "minReplicas <= maxReplicas"
- rule: this.properties.minReplicas <= this.properties.maxReplicas
message: "minReplicas cannot be larger than maxReplicas"
- rule: this.HasField("properties")
message: "there has to be properties field under spec"
type: object
properties:
minReplicas:
Expand All @@ -271,37 +269,114 @@ kind: CustomResourceDefinition
type: integer
```
Each validator may have multiple validation rules.
- Each validator may have multiple validation rules.
Each validation rule has an optional 'message' field for the error message that
- Each validation rule has an optional 'message' field for the error message that
will be surfaced when the validation rule evaluates to false.
The validator will be scoped to the location of the `x-kubernetes-validator`
- The validator will be scoped to the location of the `x-kubernetes-validator`
extension in the schema. In the above example, the validator is scoped to the
'spec' field.

For OpenAPIv3 object types, the expression will have direct access to all the
- For OpenAPIv3 object types, the expression will have direct access to all the
fields of the object the validator is scoped to.
TODO: is there also value to providing access to the scoped object via a variable name like 'this'?

For OpenAPIv3 scalar types (integer, string & boolean), the expression will have
access to the scalar data element the validator is scoped to. TODO: what
variable name will the data element be accessable by? 'this' ?

For OpenAPIv3 list and map types, the expression will have access to the data
- For OpenAPIv3 scalar types (integer, string & boolean), the expression will have
access to the scalar data element the validator is scoped to.


- For OpenAPIv3 list and map types, the expression will have access to the data
element of the list or map.
TODO: what variable name will the data element be accessable by? 'this' ?


TODO: Should the message also be an expression to allow for some basic variable
substitution?

TODO: Should a 'type' field also be required? If it is not 'cel', the validator
`TODO: Should the message also be an expression to allow for some basic variable
substitution?`

`TODO: Should a 'type' field also be required? If it is not 'cel', the validator
could be skipped to future proof against addition of other ways to validate in
the future, or to allow 3rd party validators to have a way to inline their
valiation rules in CRDs.
validation rules in CRDs.`



#### 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.

`this` represents the field `x-kubernetes-validator` scopes to. In above example, the validator is scoped to the
`spec` field. So `this.properties.minReplicas` represents to `minReplicas` field which the rule will apply.
Having `this` will be beneficial for setting up rules like validating if there is a specific field available.

#### Expression lifecycle

When CRDs are written to the kube-apiserver, all expressions will be [parsed and
typechecked](https://github.com/google/cel-go#parse-and-check) and the resulting
program will be cached for later evaluation (CEL evaluation is thread-safe and
side-effect free). Any parsing or type checking errors will cause the CRD write
to fail with a descriptive error.

#### Function library

The function library available to expressions can be augmented using [extension
functions](https://github.com/google/cel-spec/blob/master/doc/langdef.md#extension-functions).

TODO: we need to propose a list of functions to include.

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
confidence will not need to be changed or rethought.
- Support kubernetes specific concepts, like accessing associative lists by key
may be needed, we should review those cases carefully.


### User Stories

TODO: table out a wide range of validation, defaulting and conversion use cases and how
they are supported

### Notes/Constraints/Caveats (Optional)

<!--
What are the caveats to the proposal?
What are some important details that didn't come across above?
Go in to as much detail as necessary here.
This might be a good place to talk about core concepts and how they relate.
-->

### Risks and Mitigations

#### Accidental misuse

TODO: breaking the control plane, overloading the api-server

#### Malicious use

TODO: jailbreaking, bitcoin mining, exfiltration attacks?

<!--
What are the risks of this proposal, and how do we mitigate? Think broadly.
For example, consider both security and how this will impact the larger
Kubernetes ecosystem.

How will security be reviewed, and by whom?

How will UX be reviewed, and by whom?

Consider including folks who also work outside the SIG or subproject.
-->

### Defaulting

### Future Plan


#### Defaulting

The `x-kubernetes-default` extension will be used. The location of the defaulter
determins the scope of defaulter.
Expand Down Expand Up @@ -332,7 +407,7 @@ The 'field' is optional and defaults to the scope if not explicitly set.
If the expression evaluates to null, the field is left unset.
TODO: Any downsides to using null this way?

### Conversion
#### Conversion

Conversion rules will be made available via a ConversionRules strategy. A set of
rules can be provided for each supported source/destination version pair. Each rule will
Expand Down Expand Up @@ -372,77 +447,6 @@ If the expression evaluates to null, the field is left unset.
TODO: demonstrate writing fields into annotations for round trip
TODO: demonstrate string <-> structured (label selector example & ServicePort example)

#### 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 list items and the keys of map entries it traverses.

TODO: What is the exact format? Can we use something a SMD representation?

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.

TODO: show example
TODO: what is the exact format? Do we have something pre-existing that does this?

#### Expression lifecycle

When CRDs are written to the kube-apiserver, all expressions will be [parsed and
typechecked](https://github.com/google/cel-go#parse-and-check) and the resulting
program will be cached for later evaluation (CEL evaluation is thread-safe and
side-effect free). Any parsing or type checking errors will cause the CRD write
to fail with a descriptive error.

#### Function library

The function library available to expressions can be augmented using [extension
functions](https://github.com/google/cel-spec/blob/master/doc/langdef.md#extension-functions).

TODO: we need to propose a list of functions to include.

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
confidence will not need to be changed or rethought.
- Support kubernetes specific concepts, like accessing associative lists by key
may be needed, we should review those cases carefully.


### User Stories

TODO: table out a wide range of validation, defaulting and conversion use cases and how
they are supported

### Notes/Constraints/Caveats (Optional)

<!--
What are the caveats to the proposal?
What are some important details that didn't come across above?
Go in to as much detail as necessary here.
This might be a good place to talk about core concepts and how they relate.
-->

### Risks and Mitigations

#### Accidental misuse

TODO: breaking the control plane, overloading the api-server

#### Malicious use

TODO: jailbreaking, bitcoin mining, exfiltration attacks?

<!--
What are the risks of this proposal, and how do we mitigate? Think broadly.
For example, consider both security and how this will impact the larger
Kubernetes ecosystem.

How will security be reviewed, and by whom?

How will UX be reviewed, and by whom?

Consider including folks who also work outside the SIG or subproject.
-->

## Design Details

Expand Down

0 comments on commit 5f2f1e1

Please sign in to comment.