From 0ddc786b386ce27fa4dab4bcf205fe57d6a3093d Mon Sep 17 00:00:00 2001 From: David Eads Date: Wed, 12 Jun 2019 15:45:31 -0400 Subject: [PATCH 1/5] protect kubernetes community owned API groups in CRDs --- .../20190612-crd-group-protection.md | 116 ++++++++++++++++++ 1 file changed, 116 insertions(+) create mode 100644 keps/sig-api-machinery/20190612-crd-group-protection.md diff --git a/keps/sig-api-machinery/20190612-crd-group-protection.md b/keps/sig-api-machinery/20190612-crd-group-protection.md new file mode 100644 index 00000000000..51fbddc3783 --- /dev/null +++ b/keps/sig-api-machinery/20190612-crd-group-protection.md @@ -0,0 +1,116 @@ +--- +title: k8s.io Group Protection +authors: + - "@deads2k" +owning-sig: sig-api-machinery +participating-sigs: + - sig-api-machinery +reviewers: + - "@sttts" + - "@jpbetz" + - "@liggitt" +approvers: + - "@liggitt" + - "@sttts" +editor: + name: "@deads2k" +creation-date: 2019-06-12 +last-updated: 2019-06-12 +status: implementable +--- + +# k8s.io Group Protection + +## Table of Contents + +## Summary + +API groups are organized by namespace, similar to java packages. `authorization.k8s.io` is one example. When users create +CRDs, they get to specify an API group and their type will be injected into that group by the kube-apiserver. + +The *.k8s.io or *.kubernetes.io groups are owned by the Kubernetes community and protected by API review (see [What APIs need to be reviewed](https://github.com/kubernetes/community/blob/master/sig-architecture/api-review-process.md#what-apis-need-to-be-reviewed), +to ensure consistency and quality. To avoid confusion in our API groups and prevent accidentally claiming a +space inside of the kubernetes API groups, the kube-apiserver needs to be updated to protect these reserved API groups. + +This KEP proposes adding an `api-approved.kubernetes.io` annotation to CustomResourceDefinition. This is only needed if +the CRD group is `k8s.io`, `kubernetes.io`, or ends with `.k8s.io`, `.kubernetes.io`. The value should be a link to the +pull request where the API has been approved. + +```yaml +metadata: + annotations: + "api-approved.kubernetes.io": "https://github.com/kubernetes/kubernetes/pull/78458" +``` + +## Motivation + +* Prevent accidentally including an unapproved API in community owned API groups + +### Goals + +* Ensure API consistency. +* Prevent accidentally claiming reserved named. + +### Non-Goals + +* Prevent malicious users from claiming reserved names. + +## Proposal + +This KEP proposes adding an `api-approved.kubernetes.io` annotation to CustomResourceDefinition. This is only needed if +the CRD group is `k8s.io`, `kubernetes.io`, or ends with `.k8s.io`, `.kubernetes.io`. The value should be a link to the +pull request where the API has been approved. + +```yaml +metadata: + annotations: + "api-approved.kubernetes.io": "https://github.com/kubernetes/kubernetes/pull/78458" +``` + +This field is used by new kube-apiservers to set the `KubeAPIApproved` condition. + 1. If a new server sees a CRD for a resource in a kube group and sees the annotation, it will set the `KubeAPIApproved` condition to true. + 2. If a new server sees a CRD for a resource in a kube group and does not see the annotation, it will set the `KubeAPIApproved` condition to false. + 3. If a new server sees a CRD for a resource outside a kube group, it does not set the `KubeAPIApproved` condition at all. + +In v1, this annotation will be required in order to create a CRD for a resource in one of the kube API groups. + +### Behavior of new clusters +1. Current CRD for a resource in the kube group already in API is missing valid `api-approved.kubernetes.io` - `KubeAPIApproved` condition will be false. +2. CRD for a resource in the kube group creating via CRD.v1beta1 is missing valid `api-approved.kubernetes.io` - create as normal. This ensures compatibility. `KubeAPIApproved` condition will be false. +3. CRD for a resource in the kube group creating via CRD.v1 is missing valid `api-approved.kubernetes.io` - fail validation and do not store in etcd. +4. CRD for a resource outside the kube group creating via CRD.v1 is contains the `api-approved.kubernetes.io` - fail validation and do not store in etcd. +5. In CRD.v1, remove a required `api-approved.kubernetes.io` - fail validation. +5. In all versions, any update that does not change the `api-approved.kubernetes.io` will go through our current validation rules. + + +### Behavior of old clusters +1. Nothing changes. The old clusters will persist and keep the annotation + +This doesn't actively prevent bad actors from simply setting the annotation, but it does prevent accidentally claiming +an inappropriate name. + +## References + +* [Accidental name in Kanary](https://libraries.io/github/AmadeusITGroup/kanary)) + +### Test Plan + +**blockers for GA:** + +* Document in the release notes. The impact is very low + +### Graduation Criteria + +* the test plan is fully implemented for the respective quality level + +### Upgrade / Downgrade Strategy + +* annotations and conditions are always persisted. If set, they remain consistent. If unset, they also remain consistent. + +### Version Skew Strategy + +* annotations and conditions are always persisted. If set, they remain consistent. If unset, they also remain consistent. + +## Alternatives considered + +## Implementation History From c80a1d12587b1907372d8a7c4d0d0aab6397b66f Mon Sep 17 00:00:00 2001 From: David Eads Date: Wed, 26 Jun 2019 13:50:32 -0400 Subject: [PATCH 2/5] clarify the URL for CRD --- keps/sig-api-machinery/20190612-crd-group-protection.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/keps/sig-api-machinery/20190612-crd-group-protection.md b/keps/sig-api-machinery/20190612-crd-group-protection.md index 51fbddc3783..f79a7369ea9 100644 --- a/keps/sig-api-machinery/20190612-crd-group-protection.md +++ b/keps/sig-api-machinery/20190612-crd-group-protection.md @@ -5,6 +5,7 @@ authors: owning-sig: sig-api-machinery participating-sigs: - sig-api-machinery + - sig-architecture reviewers: - "@sttts" - "@jpbetz" @@ -15,7 +16,7 @@ approvers: editor: name: "@deads2k" creation-date: 2019-06-12 -last-updated: 2019-06-12 +last-updated: 2019-06-26 status: implementable --- @@ -33,8 +34,8 @@ to ensure consistency and quality. To avoid confusion in our API groups and pre space inside of the kubernetes API groups, the kube-apiserver needs to be updated to protect these reserved API groups. This KEP proposes adding an `api-approved.kubernetes.io` annotation to CustomResourceDefinition. This is only needed if -the CRD group is `k8s.io`, `kubernetes.io`, or ends with `.k8s.io`, `.kubernetes.io`. The value should be a link to the -pull request where the API has been approved. +the CRD group is `k8s.io`, `kubernetes.io`, or ends with `.k8s.io`, `.kubernetes.io`. The value should be a link to a +to a URL where the current spec was approved, so updates to the spec should also update the URL. ```yaml metadata: From 4dfe0094203d01d385471735c767d54ce07582b3 Mon Sep 17 00:00:00 2001 From: David Eads Date: Tue, 2 Jul 2019 12:39:40 -0400 Subject: [PATCH 3/5] CRD group protection: add details about how to handle a conflict --- .../sig-api-machinery/20190612-crd-group-protection.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/keps/sig-api-machinery/20190612-crd-group-protection.md b/keps/sig-api-machinery/20190612-crd-group-protection.md index f79a7369ea9..05c329fa157 100644 --- a/keps/sig-api-machinery/20190612-crd-group-protection.md +++ b/keps/sig-api-machinery/20190612-crd-group-protection.md @@ -90,6 +90,16 @@ In v1, this annotation will be required in order to create a CRD for a resource This doesn't actively prevent bad actors from simply setting the annotation, but it does prevent accidentally claiming an inappropriate name. +### What to do if you accidentally put an unapproved API in a protected group +1. Get the current state and future changes approved. For community projects, this is the best choice if the current state + is approvable. +2. If there are structural problems with the API's current state that prevent approval, you have two choices. + 1. restructure in a new version, maintaining a conversion webhook, and plan to stop serving the old version. There are + some cases where this may not work if the changes are not roundtrippable, but they should be rare. + 2. restructure in a new API group. There will be no connection to existing data. This may be disruptive for non-alpha APIs, but these + names are reserved and the process of API review has been in place for some time. The expectation is that this is + the exceptional case of an exceptional case. + ## References * [Accidental name in Kanary](https://libraries.io/github/AmadeusITGroup/kanary)) From 2d591296aa6b5b7fe6cc5806dfc4710acc9882bb Mon Sep 17 00:00:00 2001 From: David Eads Date: Wed, 3 Jul 2019 10:30:28 -0400 Subject: [PATCH 4/5] allow indicating the CRD is unapproved, but remain createable --- .../20190612-crd-group-protection.md | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/keps/sig-api-machinery/20190612-crd-group-protection.md b/keps/sig-api-machinery/20190612-crd-group-protection.md index 05c329fa157..42d170a9065 100644 --- a/keps/sig-api-machinery/20190612-crd-group-protection.md +++ b/keps/sig-api-machinery/20190612-crd-group-protection.md @@ -16,7 +16,7 @@ approvers: editor: name: "@deads2k" creation-date: 2019-06-12 -last-updated: 2019-06-26 +last-updated: 2019-07-03 status: implementable --- @@ -29,7 +29,7 @@ status: implementable API groups are organized by namespace, similar to java packages. `authorization.k8s.io` is one example. When users create CRDs, they get to specify an API group and their type will be injected into that group by the kube-apiserver. -The *.k8s.io or *.kubernetes.io groups are owned by the Kubernetes community and protected by API review (see [What APIs need to be reviewed](https://github.com/kubernetes/community/blob/master/sig-architecture/api-review-process.md#what-apis-need-to-be-reviewed), +The `*.k8s.io` or `*.kubernetes.io` groups are owned by the Kubernetes community and protected by API review (see [What APIs need to be reviewed](https://github.com/kubernetes/community/blob/master/sig-architecture/api-review-process.md#what-apis-need-to-be-reviewed), to ensure consistency and quality. To avoid confusion in our API groups and prevent accidentally claiming a space inside of the kubernetes API groups, the kube-apiserver needs to be updated to protect these reserved API groups. @@ -60,7 +60,9 @@ metadata: This KEP proposes adding an `api-approved.kubernetes.io` annotation to CustomResourceDefinition. This is only needed if the CRD group is `k8s.io`, `kubernetes.io`, or ends with `.k8s.io`, `.kubernetes.io`. The value should be a link to the -pull request where the API has been approved. +pull request where the API has been approved. If the API is unapproved, you may set the annotation to a string starting +with `"unapproved"`. For instance, `"unapproved, temporarily squatting` or `"unapproved, experimental-only"`. This +is discouraged. ```yaml metadata: @@ -68,20 +70,28 @@ metadata: "api-approved.kubernetes.io": "https://github.com/kubernetes/kubernetes/pull/78458" ``` +```yaml +metadata: + annotations: + "api-approved.kubernetes.io": "unapproved, experimental-only" +``` + This field is used by new kube-apiservers to set the `KubeAPIApproved` condition. - 1. If a new server sees a CRD for a resource in a kube group and sees the annotation, it will set the `KubeAPIApproved` condition to true. - 2. If a new server sees a CRD for a resource in a kube group and does not see the annotation, it will set the `KubeAPIApproved` condition to false. - 3. If a new server sees a CRD for a resource outside a kube group, it does not set the `KubeAPIApproved` condition at all. + 1. If a new server sees a CRD for a resource in a kube group and sees the annotation set to a URL, it will set the `KubeAPIApproved` condition to true. + 2. If a new server sees a CRD for a resource in a kube group and sees the annotation set to `"unapproved.*"`, it will set the `KubeAPIApproved` condition to false. + 3. If a new server sees a CRD for a resource in a kube group and does not see the annotation, it will set the `KubeAPIApproved` condition to false. + 4. If a new server sees a CRD for a resource outside a kube group, it does not set the `KubeAPIApproved` condition at all. -In v1, this annotation will be required in order to create a CRD for a resource in one of the kube API groups. +In v1, this annotation will be required in order to create a CRD for a resource in one of the kube API groups. If the `KubeAPIApproved` condition is false, +the condition message will include a link to https://github.com/kubernetes/enhancements/pull/1111 for reference. ### Behavior of new clusters -1. Current CRD for a resource in the kube group already in API is missing valid `api-approved.kubernetes.io` - `KubeAPIApproved` condition will be false. +1. Current CRD for a resource in the kube group already in API is missing valid `api-approved.kubernetes.io` or has set the value to `"unapproved.*"` - `KubeAPIApproved` condition will be false. 2. CRD for a resource in the kube group creating via CRD.v1beta1 is missing valid `api-approved.kubernetes.io` - create as normal. This ensures compatibility. `KubeAPIApproved` condition will be false. 3. CRD for a resource in the kube group creating via CRD.v1 is missing valid `api-approved.kubernetes.io` - fail validation and do not store in etcd. 4. CRD for a resource outside the kube group creating via CRD.v1 is contains the `api-approved.kubernetes.io` - fail validation and do not store in etcd. 5. In CRD.v1, remove a required `api-approved.kubernetes.io` - fail validation. -5. In all versions, any update that does not change the `api-approved.kubernetes.io` will go through our current validation rules. +6. In all versions, any update that does not change the `api-approved.kubernetes.io` will go through our current validation rules. ### Behavior of old clusters @@ -99,6 +109,8 @@ an inappropriate name. 2. restructure in a new API group. There will be no connection to existing data. This may be disruptive for non-alpha APIs, but these names are reserved and the process of API review has been in place for some time. The expectation is that this is the exceptional case of an exceptional case. +3. Indicate that your API is unapproved by setting the `"api-approved.kubernetes.io"` annotation to something starting with + unapproved. For instance, `"unapproved, temporarily squatting` or `"unapproved, experimental-only"`. ## References From 7d8375fec4b9a3b48aad39dbc2cf4059e6ec67d6 Mon Sep 17 00:00:00 2001 From: David Eads Date: Tue, 23 Jul 2019 10:52:31 -0400 Subject: [PATCH 5/5] fix editor tag --- keps/sig-api-machinery/20190612-crd-group-protection.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/keps/sig-api-machinery/20190612-crd-group-protection.md b/keps/sig-api-machinery/20190612-crd-group-protection.md index 42d170a9065..139a5e458a1 100644 --- a/keps/sig-api-machinery/20190612-crd-group-protection.md +++ b/keps/sig-api-machinery/20190612-crd-group-protection.md @@ -13,8 +13,7 @@ reviewers: approvers: - "@liggitt" - "@sttts" -editor: - name: "@deads2k" +editor: "@deads2k" creation-date: 2019-06-12 last-updated: 2019-07-03 status: implementable