Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CR webhook conversion documentation #10986

Merged
merged 5 commits into from
Nov 29, 2018
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,7 @@ weight: 30
{{% capture overview %}}
This page explains how to add versioning information to
[CustomResourceDefinitions](/docs/reference/generated/kubernetes-api/{{< param "version" >}}/#customresourcedefinition-v1beta1-apiextensions), to indicate the stability
level of your CustomResourceDefinitions. It also describes how to upgrade an
object from one version to another.

{{< note >}}
**Note**: All specified versions must use the same schema. The is no schema
conversion between versions.
{{< /note >}}
level of your CustomResourceDefinitions or advance your API to a new version with conversion between API Onbjects. It also describes how to upgrade an object from one version to another.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: Onbjects

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Could the overview be simplified to:

This page explains how to add versioning information to
[CustomResourceDefinitions](/docs/reference/generated/kubernetes-api/{{< param "version" >}}/#customresourcedefinition-v1beta1-apiextensions).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kbhawkey I am fine with that. @liggitt wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no really conversion "between API objects". The object is always the same, the repesentation differs.


{{% /capture %}}

Expand All @@ -36,10 +30,10 @@ conversion between versions.
## Overview

The CustomResourceDefinition API supports a `versions` field that you can use to
support multiple versions of custom resources that you have developed, and
indicate the stability of a given custom resource. All versions must currently
use the same schema, so if you need to add a field, you must add it to all
versions.
support multiple versions of custom resources that you have developed. Versions
can have different schemas with a conversion webhook to convert CRs between versions.
mbohlool marked this conversation as resolved.
Show resolved Hide resolved
These conversion should follow [API conventions](https://github.com/kubernetes/community/blob/master/contributors/devel/api-conventions.md) wherever applicable. Specifically
mbohlool marked this conversation as resolved.
Show resolved Hide resolved
a set of useful gotchas and suggestions (such as roundtrip-ability) can be found in the [api change documentation](https://github.com/kubernetes/community/blob/master/contributors/devel/api_changes.md).

mbohlool marked this conversation as resolved.
Show resolved Hide resolved
{{< note >}}
Earlier iterations included a `version` field instead of `versions`. The
Expand All @@ -49,8 +43,9 @@ match the first item in the `versions` field.

## Specify multiple versions

This example shows a CustomResourceDefinition with two versions. The comments in
the YAML provide more context.
This example shows a CustomResourceDefinition with two versions. For the first
example, the assumption is all versions share the same schema with no conversion
between them. The comments in the YAML provide more context.

```yaml
apiVersion: apiextensions.k8s.io/v1beta1
Expand All @@ -71,6 +66,10 @@ spec:
- name: v1
served: true
storage: false
conversion:
# a None strategy will assume same schema for all versions and only set apiVersion
# field of the CRs to the proper value on conversion.
strategy: None
# either Namespaced or Cluster
scope: Namespaced
names:
Expand Down Expand Up @@ -144,6 +143,115 @@ version sort order is `v1`, followed by `v1beta1`. This causes the kubectl
command to use `v1` as the default version unless the provided object specifies
the version.

## Webhook conversion

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the first thing in this section should be a prominent note that the feature is alpha in 1.13, is not enabled by default, what enabling an alpha feature means, and how to enable it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you provide a link to the Kubernetes Versioning doc?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because this algorithm follows Kubernetes versioning, it may be helpful to link to the actual versioning doc, or incorporate that content to a new page in the k8s documentation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could add a label to indicate that the feature is alpha, such as:

{{< feature-state for_k8s_version="v1.13" state="alpha" >}}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mbohlool, what do you think about adding the feature state button before the note below? It is not necessary, but makes it clear that webhook conversion is an alpha feature.

The above example has a None converter between versions which only set `apiVersion` field
mbohlool marked this conversation as resolved.
Show resolved Hide resolved
on conversion and do not change the rest of the object. API server supports webhook conversion that
mbohlool marked this conversation as resolved.
Show resolved Hide resolved
calls an external service in case of a conversion is required, e.g. when:
mbohlool marked this conversation as resolved.
Show resolved Hide resolved

* CR is requested in a different version than stored version.
* CR list request contains objects of different versions than the request version.
mbohlool marked this conversation as resolved.
Show resolved Hide resolved
* Watch is created in one version but the changed object is stored in another version.
* CR PUT request is in a different version than storage version.

To cover all of these cases and to optimize conversion on API Server side, the conversion requests
mbohlool marked this conversation as resolved.
Show resolved Hide resolved
comes with a list of objects to be converted. These conversions should happened independent on these
mbohlool marked this conversation as resolved.
Show resolved Hide resolved
objects because grouping those CRs together in one request could have no meaning other than optimization.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: These conversions happen independently.

nit: The grouping of custom resources ...
What grouping? The order of custom resources listed in the conversion request?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: "the conversion requests may contain multiple objects in order to minimize the calls to the webhook"

I think you can drop the sentence "The grouping of custom resources does not represent any relation between them."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i've revised this paragraph. please look at it again.

### Write a conversion webhook server

Please refer to the implementation of the [CR conversion webhook
server](https://github.com/kubernetes/kubernetes/blob/v1.13.0-beta.1/test/images/crd-conversion-webhook/main.go)
mbohlool marked this conversation as resolved.
Show resolved Hide resolved
that is validated in a Kubernetes e2e test. The webhook handles the
`conversionReview` requests sent by the apiservers, and sends back
mbohlool marked this conversation as resolved.
Show resolved Hide resolved
its decision wrapped in `conversionResponse`. Note that the request
mbohlool marked this conversation as resolved.
Show resolved Hide resolved
contains a list of CRs that need to be converted independently without
changing the order of objects.
The example server is organized in a way to be reused for other conversions. Most of the common codes are located in the [framework file]((https://github.com/kubernetes/kubernetes/blob/v1.10.0-beta.1/test/images/crd-conversion-webhook/converter/framework.go)) that leaves only [one function]((https://github.com/kubernetes/kubernetes/blob/v1.10.0-beta.1/test/images/crd-conversion-webhook/converter/example-converter.go#L29-L80)) to be implemented for different conversions.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: common code is


{{< note >}}
**Note:** The example conversion webhook server leaves the `ClientAuth` field
[empty](https://github.com/kubernetes/kubernetes/blob/v1.10.0-beta.1/test/images/crd-conversion-webhook/config.go#L48-L49),
mbohlool marked this conversation as resolved.
Show resolved Hide resolved
which defaults to `NoClientCert`. This means that the webhook server does not
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs a bigger warning. It is highly discouraged to run a webhook this way in production.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is client auth essential for conversion? for audit and some types of admission, I'd agree, but I'm not sure it is required in all cases for a conversion service

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove the Note:. The template includes this text.
Should webhook be Webhook?

Also, is this PR missing the preview?

authenticate the identity of the clients, supposedly apiservers. If you need
mutual TLS or other ways to authenticate the clients, see
how to [authenticate apiservers](/docs/reference/access-authn-authz/extensible-admission-controllers/#authenticate-apiservers).
{{< /note >}}

### Deploy the conversion webhook service

Documentation for deploying the conversion webhook is the same as [admission webhook example service](/docs/reference/access-authn-authz/extensible-admission-controllers/#deploy_the_admission_webhook_service).
mbohlool marked this conversation as resolved.
Show resolved Hide resolved
The assumption for next sections is that the conversion webhook server is deployed to a service named `exampleConversionWebhookServer` in `default` namespace.
mbohlool marked this conversation as resolved.
Show resolved Hide resolved

{{< note >}}
**Note:** When the webhook server is deployed into the Kubernetes cluster as a
service, it has to expose its service on the 443 port. The communication
mbohlool marked this conversation as resolved.
Show resolved Hide resolved
between the API server and the webhook service may fail if a different port
is used.
{{< /note >}}

### Configure CustomResourceDefinition to use conversion webhooks

The `None` Conversion example can be extended to use the conversion webhook by modifying `conversion`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lower-case conversion

section of the `spec`:

```yaml
apiVersion: apiextensions.k8s.io/v1beta1
kind: CustomResourceDefinition
metadata:
# name must match the spec fields below, and be in the form: <plural>.<group>
name: crontabs.example.com
spec:
# group name to use for REST API: /apis/<group>/<version>
group: example.com
# list of versions supported by this CustomResourceDefinition
versions:
- name: v1beta1
# Each version can be enabled/disabled by Served flag.
served: true
# One and only one version must be marked as the storage version.
storage: true
- name: v1
served: true
storage: false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add some per version schema to demonstrate capability of webhook conversion?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had something here, it got lost in my revisions. Nice suggestion.

conversion:
# a Webhook strategy instruct API server to call an external webhook for any conversion between CRs.
strategy: Webhook
# webhookClientConfig is required when strategy is `Webhook` and it configure the webhook endpoint to be
# called by APIServer.
webhookClientConfig:
service:
namespace: default
name: exampleConversionWebhookServer
caBundle: <pem encoded ca cert that signs the server cert used by the webhook>
# either Namespaced or Cluster
scope: Namespaced
names:
# plural name to be used in the URL: /apis/<group>/<version>/<plural>
plural: crontabs
# singular name to be used as an alias on the CLI and for display
singular: crontab
# kind is normally the CamelCased singular type. Your resource manifests use this.
kind: CronTab
# shortNames allow shorter string to match your resource on the CLI
shortNames:
- ct
```

{{< note >}}
**Note:** When using `clientConfig.service`, the server cert must be valid for
`<svc_name>.<svc_namespace>.svc`.
mbohlool marked this conversation as resolved.
Show resolved Hide resolved
{{< /note >}}

You can save the CustomResourceDefinition in a YAML file, then use
`kubectl apply` to apply it.

```shell
kubectl apply -f my-versioned-crontab-with-conversion.yaml
```

Make sure the conversion service is up and running before applying new changes.

## Writing, reading, and updating versioned CustomResourceDefinition objects

When an object is written, it is persisted at the version designated as the
Expand Down