Skip to content

Commit

Permalink
Add per version field section to custom resource webhook conversion KEP
Browse files Browse the repository at this point in the history
  • Loading branch information
mbohlool committed Oct 8, 2018
1 parent f70574f commit 794d7b9
Showing 1 changed file with 27 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -128,18 +128,41 @@ type CustomResourceConversionWebhook {
### Defaulting

In case that there is no versions list, a single version with values defaulted to top level version will be created. That means a single version with a name set to spec.version.
All newly added per version fields (schema, additionalPrinterColumns or subresources) will be defaulted to the coresponding top level field except for the first version in the list that will remain empty.
None of the added per version fields (schema, additionalPrinterColumns or subresources) will be defaulted to the coresponding top level field except because that is a backward incompatible change.

### Top level fields to Per-Version fields

### Validation
The new per version fields (schema, additionalPrinterColumns or subresources) that we are going to call X in this section cannot be defaulted to
top level fields because that would be a backward incompatible change.

To keep backward compatibility, the top level fields (schema, additionalPrinterColumns or subresources) stay the same and source of truth for first (top) version. The first item in the versions list must not set any of those fields. The plan is to use unified version list for v1.
Quoting from API [guidelines](https://github.com/kubernetes/community/blob/master/contributors/devel/api_changes.md#backward-compatibility-gotchas):

> A single feature/property cannot be represented using multiple spec fields in the same API version simultaneously
Hence the defaulting either implicit or explicit has the potential to break backward compatibility as we have two sets of fields representing the same feature. Refer to [this document](http://bit.ly/k8s-crd-per-version-defaulting) for more details.

Hence the solution is the two sets of fields cannot be set at the same time. These validation rules need to be applied:

* Either top level X or per version X can be set not both. This rule applies to individual X’s not the whole set. E.g. top level Schema can be set while per version subresources are set.
* Per version X cannot be the same. E.g. if all per version schema are the same, the CRD object will be rejected with an error message asking the user to use the top level schema.

The second validation garantees a clean moving to CRD-v1 (apiextensions.k8s.io/v1). The desired state of CRD-v1 is to only have per-version X and get rid of the top level X. To achieve that, these are conversion rules (and again these rules apply to individual fields not the whole set):

* v1beta1->v1: If top level X is set in v1beta1, then it will be copied to all versions in v1.
* v1beta1->v1: If per version X are set in v1beta1, then they will be used for per version X in v1.
* v1->v1beta1: If all per-version X are the same in v1, they will be copied to top level X in v1beta1
* v1->v1beta1: Otherwise, they will be used as per version X in v1beta1

There is one corner case that if the user of v1beta1 set all per version X the same, then the conversion above is not round trippable. That's the case the above validation protects from.

### Support Level

The feature will be alpha in the first implementation and will have a feature gate that is defaulted to false. The roll-back story with a feature gate is much more clear. if we have the features as alpha in kubernetes release Y (>X where the feature is missing) and we make it beta in kubernetes release Z, it is not safe to use the feature and downgrade from Y to X but the feature is alpha in Y which is fine. It is safe to downgrade from Z to Y (given that we enable the feature gate in Y) and that is desirable as the feature is beta in Z.
*Feature gate:* This is going to be an alpha feature and protected with a feature gate. But if we downgrade from a beta cluster (a cluster that has the feature as beta) to an alpha cluster and stored CRDs has per version fields set, they will be disabled. This is dangerous for Schema and Status field as it makes the status field mutable and validation on CRs will be disabled. Thus the feature gate in alpha cluster only protects adding per version fields not the behaviour. This way even if we downgrade to alpha cluster, per version validations and subresources will be honored.

For webhook conversion itself, the feature gate will also protect the implementation and alpha cluster with disabled feature gate will return error for CRDs with webhook conversion.

The feature will be alpha in the first implementation and will have a feature gate that is defaulted to false. The roll-back story with a feature gate is much more clear. if we have the features as alpha in kubernetes release Y (>X where the feature is missing) and we make it beta in kubernetes release Z, it is not safe to use the feature and downgrade from Y to X but the feature is alpha in Y which is fine. It is safe to downgrade from Z to Y (given that we enable the feature gate in Y) and that is desirable as the feature is beta in Z.
On downgrading from a Z to Y, stored CRDs can have per version fields set. While the feature gate can be off on Y (alpha cluster), it is dangerous to disable per-version Schema Validation or Status subresources as it makes the status field mutable and validation on CRs will be disabled. Thus the feature gate in Y only protects adding per version fields not the actual behaviour. This way even if we downgrade from Z to Y, per version validations and subresources will be honored.

### Rollback

Expand Down

0 comments on commit 794d7b9

Please sign in to comment.