From cc3f0dd648ee6f1a580a7cb7f20d72d103754fe9 Mon Sep 17 00:00:00 2001 From: Andi Li Date: Wed, 22 Jul 2020 20:32:37 +0000 Subject: [PATCH 01/20] Convert CSI snapshot KEP into new style --- .../README.md} | 0 .../177-volume-snapshot-validation/kep.yaml | 54 +++++++++++++++++++ 2 files changed, 54 insertions(+) rename keps/sig-storage/{20190709-csi-snapshot.md => 177-volume-snapshot-validation/README.md} (100%) create mode 100644 keps/sig-storage/177-volume-snapshot-validation/kep.yaml diff --git a/keps/sig-storage/20190709-csi-snapshot.md b/keps/sig-storage/177-volume-snapshot-validation/README.md similarity index 100% rename from keps/sig-storage/20190709-csi-snapshot.md rename to keps/sig-storage/177-volume-snapshot-validation/README.md diff --git a/keps/sig-storage/177-volume-snapshot-validation/kep.yaml b/keps/sig-storage/177-volume-snapshot-validation/kep.yaml new file mode 100644 index 00000000000..079507fa625 --- /dev/null +++ b/keps/sig-storage/177-volume-snapshot-validation/kep.yaml @@ -0,0 +1,54 @@ +title: CSI Snapshot +kep-number: 177 +authors: + - "@jingxu97" + - "@xing-yang" +owning-sig: sig-storage +participating-sigs: + - sig-storage +reviewers: + - "@msau42" + - "@saad-ali" + - "@thockin" +approvers: + - "@msau42" + - "@saad-ali" + - "@thockin" +prr-approvers: + - TBD +editor: TBD +creation-date: 2019-07-09 +last-updated: 2020-07-22 +status: implementable +see-also: + - n/a +replaces: + - n/a +superseded-by: + - n/a + +# The target maturity stage in the current dev cycle for this KEP. +stage: beta + +# The most recent milestone for which work toward delivery of this KEP has been +# done. This can be the current (upcoming) milestone, if it is being actively +# worked on. +latest-milestone: "v1.20" + +# The milestone at which this feature was, or is targeted to be, at each stage. +milestone: + alpha: "" + beta: "v1.17" + stable: "v1.20" + +# The following PRR answers are required at alpha release +# List the feature gate name and the components for which it must be enabled +feature-gates: + - name: VolumeSnapshotDataSource + components: + - kube-apiserver +disable-supported: true + +# The following PRR answers are required at beta release +metrics: + From b3b20d4bf567de6450a6ea181f9f2bb7b857b56a Mon Sep 17 00:00:00 2001 From: Andi Li Date: Wed, 22 Jul 2020 20:32:58 +0000 Subject: [PATCH 02/20] Add tightening validation via webhook and crd schema KEP --- .../tighten-validation-webhook-crd.md | 799 ++++++++++++++++++ 1 file changed, 799 insertions(+) create mode 100644 keps/sig-storage/177-volume-snapshot-validation/tighten-validation-webhook-crd.md diff --git a/keps/sig-storage/177-volume-snapshot-validation/tighten-validation-webhook-crd.md b/keps/sig-storage/177-volume-snapshot-validation/tighten-validation-webhook-crd.md new file mode 100644 index 00000000000..ed4fcee4424 --- /dev/null +++ b/keps/sig-storage/177-volume-snapshot-validation/tighten-validation-webhook-crd.md @@ -0,0 +1,799 @@ +--- +title: CSI Snapshot +authors: + - "@andili99" + - "@yuxiangqian" +owning-sig: sig-storage +participating-sigs: + - sig-storage +reviewers: + - "@msau42" + - "@liggit" + - "@xing-yang" + - "@mattcary" +approvers: + - "@msau42" + - "@liggit" + - "@xing-yang" + - "@mattcary" +editor: TBD +creation-date: 2020-07-22 +last-updated: 2020-07-22 +status: provisional +see-also: + - n/a +replaces: + - n/a +superseded-by: + - n/a +--- + + +# KEP-1900: Add additional validation to volume snapshot objects + + + + +- [KEP-1900: Add additional validation to volume snapshot objects](#kep-1900-add-additional-validation-to-volume-snapshot-objects) + - [Release Signoff Checklist](#release-signoff-checklist) + - [Summary](#summary) + - [Motivation](#motivation) + - [Background on Admission webhooks](#background-on-admission-webhooks) + - [Goals](#goals) + - [Non-Goals](#non-goals) + - [Proposal](#proposal) + - [Validating Scenarios](#validating-scenarios) + - [VolumeSnapshot](#volumesnapshot) + - [VolumeSnapshotContent](#volumesnapshotcontent) + - [Authentication](#authentication) + - [Timeout](#timeout) + - [Idempotency/Deadlock](#idempotencydeadlock) + - [User Stories (Optional)](#user-stories-optional) + - [Story 1](#story-1) + - [Notes/Constraints/Caveats (Optional)](#notesconstraintscaveats-optional) + - [Risks and Mitigations](#risks-and-mitigations) + - [Backwards compatibility](#backwards-compatibility) + - [Current Controller validation of OneOf semantic](#current-controller-validation-of-oneof-semantic) + - [Handling VolumeSnapshot.](#handling-volumesnapshot) + - [Handling VolumeSnapshotContent](#handling-volumesnapshotcontent) + - [Design Details](#design-details) + - [Deployment](#deployment) + - [Kubernetes API Server Configuration](#kubernetes-api-server-configuration) + - [Test Plan](#test-plan) + - [Graduation Criteria](#graduation-criteria) + - [Upgrade / Downgrade Strategy](#upgrade--downgrade-strategy) + - [Version Skew Strategy](#version-skew-strategy) + - [Production Readiness Review Questionnaire](#production-readiness-review-questionnaire) + - [Feature Enablement and Rollback](#feature-enablement-and-rollback) + - [Rollout, Upgrade and Rollback Planning](#rollout-upgrade-and-rollback-planning) + - [Monitoring Requirements](#monitoring-requirements) + - [Dependencies](#dependencies) + - [Scalability](#scalability) + - [Troubleshooting](#troubleshooting) + - [Implementation History](#implementation-history) + - [Drawbacks](#drawbacks) + - [Alternatives](#alternatives) + - [Infrastructure Needed (Optional)](#infrastructure-needed-optional) + + +## Release Signoff Checklist + + + +Items marked with (R) are required *prior to targeting to a milestone / release*. + +- [ ] (R) Enhancement issue in release milestone, which links to KEP dir in [kubernetes/enhancements] (not the initial KEP PR) +- [ ] (R) KEP approvers have approved the KEP status as `implementable` +- [ ] (R) Design details are appropriately documented +- [ ] (R) Test plan is in place, giving consideration to SIG Architecture and SIG Testing input +- [ ] (R) Graduation criteria is in place +- [ ] (R) Production readiness review completed +- [ ] Production readiness review approved +- [ ] "Implementation History" section is up-to-date for milestone +- [ ] User-facing documentation has been created in [kubernetes/website], for publication to [kubernetes.io] +- [ ] Supporting documentation—e.g., additional design documents, links to mailing list discussions/SIG meetings, relevant PRs/issues, release notes + + + +[kubernetes.io]: https://kubernetes.io/ +[kubernetes/enhancements]: https://git.k8s.io/enhancements +[kubernetes/kubernetes]: https://git.k8s.io/kubernetes +[kubernetes/website]: https://git.k8s.io/website + +## Summary + +Tighten validation on `VolumeSnapshot` and `VolumeSnapshotContent` by updating the CRD validation schema and providing a webhook server to enforce immutability (until builtin immutable fields for CRD is available). + +This KEP will list the new validation rules. It will also provide the release plan to ensure backwards compatibility. As well, it will outline the deployment plan of the webhook server. + +This tightening of the validation on volume snapshot objects is considered a change to the volume snapshot API. Choosing not to install the webhook server and participate in the 2-phase release process can cause future problems when upgrading from v1beta1 to V1 volumesnapshot API if there are currently persisted objects which fail the new stricter validation. Potential impacts include being unable to delete invalid snapshot objects. + +## Motivation + +VolumeSnapshot feature has been on the BETA stage in Kubernetes OSS community since Kubernetes version 1.17. The community has identified a gap in lacking validation when CR(custom resource), i.e, VolumeSnapshot, VolumeSnapshotContent, are created [issue](https://github.com/kubernetes-csi/external-snapshotter/issues/187). This gap will need to be resolved before the feature can be brought to GA. + +### Background on Admission webhooks + +Admission webhooks are HTTP callbacks to intercept requests to the API server. They could be validating webhooks and mutating webhooks(details). Admission webhooks have been released in BETA since K8s v1.9 and GA in v1.16. Following prerequisites are needed to be able to use this feature: + +- K8s version, v1.9+ to use admissionregistration.k8s.io/v1beta1 or v1.16+ to use admissionregistration.k8s.io/v1 (Note that volume snapshot moved to BETA in v1.16) +- Corresponding admission controllers(MutatingAdmissionWebhook, ValidatingAdmissionWebhook) is enabled. (in v1.18+, both will be enabled by default, with mutating precedes validating) +- API admissionregistration.k8s.io/v1beta1 or admissionregistration.k8s.io/v1 is enabled.(Prefer v1 over v1beta1) + +Admission controllers have been in common use in kubernetes for a long time. Admission webhooks are the new, preferred way to control admission, especially for external (out-of-tree) components like the CSI external snapshotter. + +A webhook server receives AdmissionReview(definition) requests from API server, and responses with a response of the same type to either admit/deny the request. Following simplified diagram shows the workflow. +(Note that the mutatting webhooks will be invoked BEFORE validating). + +The webhook server will expose an HTTP endpoint such that to allow the API server to send AdmissionReview requests. Webhook server providers can dynamically(details) configure what type of resources and what type of admission webhooks via creating CRs of type ValidatingWebhookConfiguration and/or MutatingWebhookConfiguration. It’s worth thinking to provide a generic webhook server in csi-repo. At this stage, the proposal is to create a repo under CSI org, however only implement the validating webhooks for VolumeSnapshot. + +CRD validation is preferred over webhook validation due to their lower complexity, however CRD validation schema is unable to enforce immutability or provide ratcheting validation. + +### Goals + +- Provide an updated CRD schema to validate fields +- Prevent: + - Invalid VolumeSnapshot/VolumeSnapshotContent from creation and update + - Invalid updates on immutable fields, i.e., VolumeSnapshot.Spec.Source +- Provide a pre-built image which can be used to deploy the server +- Provide a way to deploy the webhook server in cluster +- Provide a way to authenticate the webhook server to the API server via TLS +- Provide a release process to safely tighten the validation and move towards the ideal state of using builtin CRD validation while maintaining backwards compatibility + +### Non-Goals + +- Provide a way to authenticate the API server to the webhook server + +## Proposal + +Tighten the validation on Volume Snapshot objects. The following fields will begin to enforce immutability: `VolumeSnapshot.Spec.Source` and `VolumeSnapshotContent.Spec.Source`. The following fields will begin to enforce `oneOf`: `VolumeSnapshot.Spec.Source` and `VolumeSnapshotContent.Spec.Source`). The following fields will begin to enforce non-empty strings: `spec.VolumeSnapshotClassName`. More details are in the Validating Scenarios section. + +Due to backwards compatibility concerns, the tightening will occur in two phases. + +1. The first phase is webhook-only, and will use ratcheting validation combined with a reconciliation method to delete or fix currently persisted objects which are invalid under the new (strict) validation rules. +2. The second phase occurs once all invalid objects are cleared from the cluster. The CRD schema validation will be tightened and the webhook will stick around to enforce immutability until immutable fields come to CRDs. (or the crd upgrade could wait until immutable fields are available to do in one go) + +The phases come in separate releases to allow users / cluster admin the opportunity to clean their cluster of any invalid objects. More details are in the Risks and Mitigations section. + +The server will perform validation on Volume Snapshot objects when CREATE and UPDATE requests are made to the api server for `VolumeSnapshot` and `VolumeSnapshotContent` objects. The webhooks will only use validating webhooks, which are read-only. An image will be built and example `Deployment` and `Service` yaml files will be provided. Example configuration files for the `ValidatingWebhookConfiguration` will be provided, to be used to register the webhooks on the API server. + +### Validating Scenarios + +The following is a list of fields which will get checked when a CREATE or UPDATE operation is sent to the API server. Some validation is already enforced by the CRD schema definition, for example some required fields and enums. + +All of the validation desired can be achieved by updating the CRDs to take advantage of the OpenApi v3 schema validation. In particular, the `oneOf` and `minLength` fields can be used. However, there is a desire for some fields to be immutable, which is not yet supported by CRDs. See KEP https://github.com/kubernetes/enhancements/blob/master/keps/sig-api-machinery/20190603-immutable-fields.md for the timeline for immutable fields to come to CRDs. + +Note that we may want to consider other fields to be marked as immutable. + +#### VolumeSnapshot + +| Operation | Field | Reason | HTTP RCode | +| :-------: | :--------------------------: | :-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------: | :--------: | +| CREATE | spec.Source | Exactly one of PersistentVolumeClaimName (Dynamic) or VolumeSnapshotContentName (Pre-provisioned) should be specified. | 400 | +| UPDATE | spec.Source | Immutable, no updates allowed. If the user has specified an incorrect source, they must delete and remake the snapshot. The webhook validation server will not be able to guarantee that only incorrect sources are allowed to be updated. | 400 | +| CREATE | spec.VolumeSnapshotClassName | Must not be the empty string. Can be unset (to use the default snapshot class, if it is set. If the default snapshot class is not set or there is more than 1 default class, then the hook will allow the creation but the snapshot will fail.), or set to a non-empty string (the snapshot class). | 400 | +| UPDATE | spec.VolumeSnapshotClassName | Same restrictions as CREATE. We won’t restrict updating by making this field immutable (only applying the same restrictions as creation) but this field should only be changed by those who know exactly what they are doing. | 400 | + +#### VolumeSnapshotContent +| Operation | Field | Reason | HTTP RCode | +| :-------: | :--------------------: | :---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------: | :--------: | +| CREATE | spec.Source | Exactly one of VolumeHandle (dynamic snapshot created by controller) or SnapshotHandle (pre-provisioned snapshot created by CA) should be specified | 400 | +| UPDATE | spec.Source | Immutable, no updates allowed. | 400 | +| CREATE | spec.VolumeSnapshotRef | Must have both name and namespace fields set. Preprovisioned: This is the reference to the yet to be created VolumeSnapshot object which should bind to this VolumeSnapshotContent. https://github.com/kubernetes-csi/external-snapshotter/blob/097b1fc7d7cd6576182ca34512c14de1c84b2127/pkg/apis/volumesnapshot/v1beta1/types.go#L270. Dynamic: This is the reference to the VS object which triggered the creation of this VSContent. It also has the UID field, but this is set by the controller. | 400 | +| UPDATE | spec.VolumeSnapshotRef | Immutable, no updates allowed, once it's UID has been set. | 400 | + +### Authentication + +There are two directions to authentication. Authenticating the webhook server is legitimate, and authenticating the k8s api server is legitimate. + +The API server authenticates the webhook server through TLS certificates and HTTPS. This is required, and an example method of deploying the webhook server with HTTPS will be provided. + +Authentication on incoming requests to the webhook server is configurable however out of scope of this document. It’s the user’s responsibility in general to configure the webhook service and the API server if authentication is required(details). The web server implementation, however, should allow users to configure whether authentication is required or not. If no authentication config is specified, the webhook server should default to “NoClientCert”, which effectively will not authenticate the identity of the clients. + +### Timeout + +Webhooks add latency to each API server call, thus setting up a reasonable timeout for each AdmissionReview request from the webhook server side is critical. The default timeout is 10 seconds if not specified. When an AdmissionReview request sent to the webhook server timed out, `failurePolicy`(default to `Fail` which is equivalent to disallow) will be triggered. + +In the ValidatingWebhookConfiguration yaml example, a default timeout of two seconds is provided, cluster admins who wish to change the timeout may change the value of `timeoutSeconds` + +To avoid migration pain it is recommended to start with a `failurePolicy` value of `Ignore`, changing it to `Fail` only after the webhook is confirmed to have been installed successfully. + +### Idempotency/Deadlock + +Since only validating webhooks will be introduced in this version, idempotency/deadlock are not relevant. + +### User Stories (Optional) + + + +#### Story 1 +CA can deploy the webhook server. +Users can create and update snapshot objects with confidence invalid updates will be rejected. + +Following are some typical scenarios we are aiming to prevent: +- Creation of invalid CRs +- Reject if a VolumeSnapshot CR does not have a legit VolumeSnapshotSource, i.e., missing both PersistentVolumeName and VolumeSnapshotContentName. +- Reject if a VolumeSnapshotContent CR does not have a legit VolumeSnapshotContentSource, i.e., both VolumeHandle and SnapshotHandle have been specified +- Updating immutable fields +- Reject updates to VolumeSnapshot’s VolumeSnapshotSource +- Reject updates to VolumeSnapshotContent’s VolumeSnapshotContentSource +- Reject updates to VolumeSnapshotContent’s volume snapshot ref after binding + +### Notes/Constraints/Caveats (Optional) + + + + +### Risks and Mitigations + + + +#### Backwards compatibility + +There is a backwards compatibility issue involved when tightening the validation on snapshot objects. Since the feature is already in beta we are committed to more backward compatibility guarantee than alpha. + +There are two perspectives. +- client perspective: API requests that previously succeeded now fail (these are okay) + - create: invalid things get rejected rather than persisted + - update: invalid transitions now get rejected +- API server perspective for existing persisted data + - do not tighten validation (via schema or webhook) in a way that would prevent a no-op update (this must not be violated) + - reason: removing a finalizer is done via update, causes problems with delete + - making a previously optional field required in the schema blocks update of previously persisted data that omitted the field (unless the update populates the newly required field, or you specify a schema default) + +To tackle the backwards compatibility problem, this KEP proposes the following release process. + +Begin with validating webhook only enforcement. The webhook will perform the following validation +- one release with ratcheting validation (via webhook) and controller removal of invalid objects + - webhook is strict on create + - webhook is strict on updates where the existing object passes strict validation + - webhook is relaxed on updates where the existing object fails strict validation (allows finalizer removal, status update, deletion, etc) + - some reconciliation process in volume snapshot controller (this is the hard part) that ensures invalid data is fixed or removed + +Once we are sure no invalid data is persisted, can switch to CRD schema-enforced validation with validating webhooks for immutability in a subsequent release. + +We are unsure of the specifics of the reconciliation process in which the controller will remove or fix invalid objects. + +#### Current Controller validation of OneOf semantic + +##### Handling [VolumeSnapshot](https://github.com/kubernetes-csi/external-snapshotter/blob/master/pkg/common-controller/snapshot_controller.go#L192). + +If the object violates oneOf semantic: Update the VS status to “SnapshotValidationError” and issue an event. + +Note: +- If the VS object has been updated AFTER binding to a VSC, binding from VS->VSC will be lost. +- Deletion of an invalid resource is not blocked by that check as the deletion workflow happens before validation(code). This is to ensure that a user can delete an invalid VS resource. + +##### Handling [VolumeSnapshotContent](https://github.com/kubernetes-csi/external-snapshotter/blob/master/pkg/common-controller/snapshot_controller.go#L91) + +If the object violates oneOf semantic: Update the VSC status to “ContentValidationError” and issue an event. + +## Design Details + + + +### Deployment + +There are two main steps to setup validation for the snapshot objects. The kubernetes API server must be configured to connect to the webhook server, and the webhook server must be deployed and reachable. Make sure to take a look at the prerequisites before deploying. + +### Kubernetes API Server Configuration + +The API server must be configured to connect to the webhook server for certain API requests. This is done by creating a ValidatingWebhookConfiguration object. For a more thorough explanation of each field refer to the documentation. An example yaml file is provided below. The value of timeoutSeconds will affect the latency of snapshot creation, and must be considered carefully as it will affect the time the application may be frozen for. + +``` +apiVersion: admissionregistration.k8s.io/v1 +kind: ValidatingWebhookConfiguration +metadata: + name: "webhook-validation.storage.sigs.k8s.io" +webhooks: +- name: "snapshot.webhook-validation.storage.sigs.k8s.io" + rules: + - apiGroups: ["snapshot.storage.k8s.io"] + apiVersions: ["v1beta1"] + operations: ["CREATE", "UPDATE"] + resources: ["VolumeSnapshot", "VolumeSnapshotContent"] + scope: "*" + clientConfig: + service: + namespace: "kube-system" + name: "snapshot-validation-service" + caBundle: "Ci0tLS0tQk...<`caBundle` is a PEM encoded CA bundle which will be used to validate the webhook's server certificate.>...tLS0K" + admissionReviewVersions: ["v1", "v1beta1"] + sideEffects: None + failurePolicy: Ignore # We recommend switching to Fail only after successful installation of the server and webhook. + timeoutSeconds: 2 # This will affect the latency and performance. Finetune this value based on your application's tolerance. +``` + +Webhook Server Deployment +The recommended deployment mode for the webhook server is within the cluster to minimize network latency. For high-availability we recommend using a Deployment and Service to deploy the validation server. Some example yaml files are provided, and should be changed to suit the Cluster Admin’s needs. + +``` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: snapshot-validation-deployment + labels: + app: snapshot-validation +spec: + replicas: 3 + selector: + matchLabels: + app: snapshot-validation + template: + metadata: + labels: + app: snapshot-validation + spec: + containers: + - name: snapshot-validation + image: snapshot-validation:xxx # change the image if you wish to use your own custom validation server image + ports: + - containerPort: 443 # change the port as needed +``` + +``` +apiVersion: v1 +kind: Service +metadata: + name: snapshot-validation-service + namespace: default # Don't use the default namespace. Choose an appropriate one. +spec: + selector: + app: snapshot-validation + ports: + - protocol: TCP + port: 443 # Change if needed + targetPort: 443 # Change if the webserver image expects a different port +``` + +### Test Plan +There will be unit testing on the webserver to ensure that the correct policy gets enforced. + +There is a cluster available for testing in the external snapshotter repository. We will deploy the webhook validation server and test the e2e functionality. + + + +### Graduation Criteria + + + +### Upgrade / Downgrade Strategy + + + +### Version Skew Strategy + + + +## Production Readiness Review Questionnaire + + + +### Feature Enablement and Rollback + +* **How can this feature be enabled / disabled in a live cluster?** + - [ ] Feature gate (also fill in values in `kep.yaml`) + - Feature gate name: VolumeSnapshotDataSource (overall feature gate) + - Components depending on the feature gate: + - [ ] Other + - Describe the mechanism: Create or delete the `validatingwebhookconfiguration` object. Once we reach phase two of the release with validating via CRDs, the feature cannot be disabled. + - Will enabling / disabling the feature require downtime of the control + plane? No (Phase 1) + - Will enabling / disabling the feature require downtime or reprovisioning + of a node? (Do not assume `Dynamic Kubelet Config` feature is enabled). No + +* **Does enabling the feature change any default behavior?** + Currently some validation is not fully enforced. This will tighten the validation to be in line with what is intended. + +* **Can the feature be disabled once it has been enabled (i.e. can we roll back + the enablement)?** + In phase one, the feature can be disabled by removing the webhook. However once we update the CRDs users will not easily be able to disable it once they have upgraded the + +* **What happens if we reenable the feature if it was previously rolled back?** Nothing special. + +* **Are there any tests for feature enablement/disablement?** + The e2e framework does not currently support enabling or disabling feature + gates. However, unit tests in each component dealing with managing data, created + with and without the feature, are necessary. At the very least, think about + conversion tests if API types are being modified. + +### Rollout, Upgrade and Rollback Planning + +_This section must be completed when targeting beta graduation to a release._ + +* **How can a rollout fail? Can it impact already running workloads?** + Try to be as paranoid as possible - e.g., what if some components will restart + mid-rollout? + +* **What specific metrics should inform a rollback?** + +* **Were upgrade and rollback tested? Was the upgrade->downgrade->upgrade path tested?** + Describe manual testing that was done and the outcomes. + Longer term, we may want to require automated upgrade/rollback tests, but we + are missing a bunch of machinery and tooling and can't do that now. + +* **Is the rollout accompanied by any deprecations and/or removals of features, APIs, +fields of API types, flags, etc.?** + Even if applying deprecation policies, they may still surprise some users. + +### Monitoring Requirements + +_This section must be completed when targeting beta graduation to a release._ + +* **How can an operator determine if the feature is in use by workloads?** + Ideally, this should be a metric. Operations against the Kubernetes API (e.g., + checking if there are objects with field X set) may be a last resort. Avoid + logs or events for this purpose. + +* **What are the SLIs (Service Level Indicators) an operator can use to determine +the health of the service?** + - [ ] Metrics + - Metric name: + - [Optional] Aggregation method: + - Components exposing the metric: + - [ ] Other (treat as last resort) + - Details: + +* **What are the reasonable SLOs (Service Level Objectives) for the above SLIs?** + At a high level, this usually will be in the form of "high percentile of SLI + per day <= X". It's impossible to provide comprehensive guidance, but at the very + high level (needs more precise definitions) those may be things like: + - per-day percentage of API calls finishing with 5XX errors <= 1% + - 99% percentile over day of absolute value from (job creation time minus expected + job creation time) for cron job <= 10% + - 99,9% of /health requests per day finish with 200 code + +* **Are there any missing metrics that would be useful to have to improve observability +of this feature?** + Describe the metrics themselves and the reasons why they weren't added (e.g., cost, + implementation difficulties, etc.). + +### Dependencies + +_This section must be completed when targeting beta graduation to a release._ + +* **Does this feature depend on any specific services running in the cluster?** + Think about both cluster-level services (e.g. metrics-server) as well + as node-level agents (e.g. specific version of CRI). Focus on external or + optional services that are needed. For example, if this feature depends on + a cloud provider API, or upon an external software-defined storage or network + control plane. + + For each of these, fill in the following—thinking about running existing user workloads + and creating new ones, as well as about cluster-level services (e.g. DNS): + - [Dependency name] + - Usage description: + - Impact of its outage on the feature: + - Impact of its degraded performance or high-error rates on the feature: + + +### Scalability + +_For alpha, this section is encouraged: reviewers should consider these questions +and attempt to answer them._ + +_For beta, this section is required: reviewers must answer these questions._ + +_For GA, this section is required: approvers should be able to confirm the +previous answers based on experience in the field._ + +* **Will enabling / using this feature result in any new API calls?** + Describe them, providing: + - API call type (e.g. PATCH pods) + - estimated throughput + - originating component(s) (e.g. Kubelet, Feature-X-controller) + focusing mostly on: + - components listing and/or watching resources they didn't before + - API calls that may be triggered by changes of some Kubernetes resources + (e.g. update of object X triggers new updates of object Y) + - periodic API calls to reconcile state (e.g. periodic fetching state, + heartbeats, leader election, etc.) + +* **Will enabling / using this feature result in introducing new API types?** + Describe them, providing: + - API type + - Supported number of objects per cluster + - Supported number of objects per namespace (for namespace-scoped objects) + +* **Will enabling / using this feature result in any new calls to the cloud +provider?** + +* **Will enabling / using this feature result in increasing size or count of +the existing API objects?** + Describe them, providing: + - API type(s): + - Estimated increase in size: (e.g., new annotation of size 32B) + - Estimated amount of new objects: (e.g., new Object X for every existing Pod) + +* **Will enabling / using this feature result in increasing time taken by any +operations covered by [existing SLIs/SLOs]?** + Think about adding additional work or introducing new steps in between + (e.g. need to do X to start a container), etc. Please describe the details. + +* **Will enabling / using this feature result in non-negligible increase of +resource usage (CPU, RAM, disk, IO, ...) in any components?** + Things to keep in mind include: additional in-memory state, additional + non-trivial computations, excessive access to disks (including increased log + volume), significant amount of data sent and/or received over network, etc. + This through this both in small and large cases, again with respect to the + [supported limits]. + +### Troubleshooting + +The Troubleshooting section currently serves the `Playbook` role. We may consider +splitting it into a dedicated `Playbook` document (potentially with some monitoring +details). For now, we leave it here. + +_This section must be completed when targeting beta graduation to a release._ + +* **How does this feature react if the API server and/or etcd is unavailable?** + +* **What are other known failure modes?** + For each of them, fill in the following information by copying the below template: + - [Failure mode brief description] + - Detection: How can it be detected via metrics? Stated another way: + how can an operator troubleshoot without logging into a master or worker node? + - Mitigations: What can be done to stop the bleeding, especially for already + running user workloads? + - Diagnostics: What are the useful log messages and their required logging + levels that could help debug the issue? + Not required until feature graduated to beta. + - Testing: Are there any tests for failure mode? If not, describe why. + +* **What steps should be taken if SLOs are not being met to determine the problem?** + +[supported limits]: https://git.k8s.io/community//sig-scalability/configs-and-limits/thresholds.md +[existing SLIs/SLOs]: https://git.k8s.io/community/sig-scalability/slos/slos.md#kubernetes-slisslos + +## Implementation History + + + +## Drawbacks + + + +## Alternatives + +Sig-storage wide webhook design. This was not accepted because the scope would be too big. + +Wait until immutable fields for crds are implemented. +This [KEP](https://github.com/kubernetes/enhancements/blob/master/keps/sig-api-machinery/20190603-immutable-fields.md) tracks the feature. + + +## Infrastructure Needed (Optional) + + From 4c8a898367638c182d4b9998d7f3e1d661e2292c Mon Sep 17 00:00:00 2001 From: Andi Li Date: Thu, 23 Jul 2020 18:37:40 +0000 Subject: [PATCH 03/20] Rename directory --- .../README.md | 0 .../kep.yaml | 0 .../tighten-validation-webhook-crd.md | 0 3 files changed, 0 insertions(+), 0 deletions(-) rename keps/sig-storage/{177-volume-snapshot-validation => 177-volume-snapshot}/README.md (100%) rename keps/sig-storage/{177-volume-snapshot-validation => 177-volume-snapshot}/kep.yaml (100%) rename keps/sig-storage/{177-volume-snapshot-validation => 177-volume-snapshot}/tighten-validation-webhook-crd.md (100%) diff --git a/keps/sig-storage/177-volume-snapshot-validation/README.md b/keps/sig-storage/177-volume-snapshot/README.md similarity index 100% rename from keps/sig-storage/177-volume-snapshot-validation/README.md rename to keps/sig-storage/177-volume-snapshot/README.md diff --git a/keps/sig-storage/177-volume-snapshot-validation/kep.yaml b/keps/sig-storage/177-volume-snapshot/kep.yaml similarity index 100% rename from keps/sig-storage/177-volume-snapshot-validation/kep.yaml rename to keps/sig-storage/177-volume-snapshot/kep.yaml diff --git a/keps/sig-storage/177-volume-snapshot-validation/tighten-validation-webhook-crd.md b/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md similarity index 100% rename from keps/sig-storage/177-volume-snapshot-validation/tighten-validation-webhook-crd.md rename to keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md From ae1cfab09d59aafff22f7147c0a5e6b5c6cb38b8 Mon Sep 17 00:00:00 2001 From: Andi Li Date: Thu, 23 Jul 2020 19:29:50 +0000 Subject: [PATCH 04/20] Fix some issues --- .../177-volume-snapshot/tighten-validation-webhook-crd.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md b/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md index ed4fcee4424..f6e9125b206 100644 --- a/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md +++ b/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md @@ -1,5 +1,5 @@ --- -title: CSI Snapshot +title: CSI Snapshot Webhook authors: - "@andili99" - "@yuxiangqian" @@ -216,7 +216,7 @@ CRD validation is preferred over webhook validation due to their lower complexit - Prevent: - Invalid VolumeSnapshot/VolumeSnapshotContent from creation and update - Invalid updates on immutable fields, i.e., VolumeSnapshot.Spec.Source -- Provide a pre-built image which can be used to deploy the server +- Provide a pre-built image which can be used to deploy the webhook *server* - Provide a way to deploy the webhook server in cluster - Provide a way to authenticate the webhook server to the API server via TLS - Provide a release process to safely tighten the validation and move towards the ideal state of using builtin CRD validation while maintaining backwards compatibility @@ -231,8 +231,8 @@ Tighten the validation on Volume Snapshot objects. The following fields will beg Due to backwards compatibility concerns, the tightening will occur in two phases. -1. The first phase is webhook-only, and will use ratcheting validation combined with a reconciliation method to delete or fix currently persisted objects which are invalid under the new (strict) validation rules. -2. The second phase occurs once all invalid objects are cleared from the cluster. The CRD schema validation will be tightened and the webhook will stick around to enforce immutability until immutable fields come to CRDs. (or the crd upgrade could wait until immutable fields are available to do in one go) +1. The first phase is webhook-only, and will use [ratcheting validation](#backwards-compatibility) combined with a reconciliation method to delete or fix currently persisted objects which are invalid under the new (strict) validation rules. +2. The second phase occurs once all invalid objects are cleared *from* the cluster. The CRD schema validation will be tightened and the webhook will stick around to enforce immutability until immutable fields come to CRDs. (or the crd upgrade could wait until immutable fields are available to do in one go) The phases come in separate releases to allow users / cluster admin the opportunity to clean their cluster of any invalid objects. More details are in the Risks and Mitigations section. From 423cb9cd9e7d74e0ca4650ade55df56dda7f90d5 Mon Sep 17 00:00:00 2001 From: Andi Li Date: Thu, 23 Jul 2020 20:23:24 +0000 Subject: [PATCH 05/20] Fix list indent. Fix wording. --- keps/sig-storage/177-volume-snapshot/kep.yaml | 2 +- .../tighten-validation-webhook-crd.md | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/keps/sig-storage/177-volume-snapshot/kep.yaml b/keps/sig-storage/177-volume-snapshot/kep.yaml index 079507fa625..ad94b3d1349 100644 --- a/keps/sig-storage/177-volume-snapshot/kep.yaml +++ b/keps/sig-storage/177-volume-snapshot/kep.yaml @@ -15,7 +15,7 @@ approvers: - "@saad-ali" - "@thockin" prr-approvers: - - TBD + - "@deads2k" editor: TBD creation-date: 2019-07-09 last-updated: 2020-07-22 diff --git a/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md b/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md index f6e9125b206..215c68ca282 100644 --- a/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md +++ b/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md @@ -216,7 +216,7 @@ CRD validation is preferred over webhook validation due to their lower complexit - Prevent: - Invalid VolumeSnapshot/VolumeSnapshotContent from creation and update - Invalid updates on immutable fields, i.e., VolumeSnapshot.Spec.Source -- Provide a pre-built image which can be used to deploy the webhook *server* +- Provide a pre-built image which can be used to deploy the webhook server - Provide a way to deploy the webhook server in cluster - Provide a way to authenticate the webhook server to the API server via TLS - Provide a release process to safely tighten the validation and move towards the ideal state of using builtin CRD validation while maintaining backwards compatibility @@ -345,13 +345,13 @@ There are two perspectives. To tackle the backwards compatibility problem, this KEP proposes the following release process. Begin with validating webhook only enforcement. The webhook will perform the following validation -- one release with ratcheting validation (via webhook) and controller removal of invalid objects - - webhook is strict on create - - webhook is strict on updates where the existing object passes strict validation - - webhook is relaxed on updates where the existing object fails strict validation (allows finalizer removal, status update, deletion, etc) +- one release with ratcheting validation using the webhook server + - webhook is strict on create + - webhook is strict on updates where the existing object passes strict validation + - webhook is relaxed on updates where the existing object fails strict validation (allows finalizer removal, status update, deletion, etc) - some reconciliation process in volume snapshot controller (this is the hard part) that ensures invalid data is fixed or removed -Once we are sure no invalid data is persisted, can switch to CRD schema-enforced validation with validating webhooks for immutability in a subsequent release. +Once we are sure no invalid data is persisted, we can switch to CRD schema-enforced validation with validating webhooks for immutability in a subsequent release. We are unsure of the specifics of the reconciliation process in which the controller will remove or fix invalid objects. From 1917f946ac3dcaa2846c3244c979596931bda7ff Mon Sep 17 00:00:00 2001 From: Andi Li Date: Tue, 28 Jul 2020 18:02:29 +0000 Subject: [PATCH 06/20] Address comments Fix typo Fix wrong date for beta of snapshot Change to reference release tag Add `VolumeSnapshotContent.spec.VolumeSnapshotRef` to list of immutable fields with note that it is immutable only after UID is set. --- .../tighten-validation-webhook-crd.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md b/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md index 215c68ca282..747ff1b8e7b 100644 --- a/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md +++ b/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md @@ -197,14 +197,14 @@ VolumeSnapshot feature has been on the BETA stage in Kubernetes OSS community si Admission webhooks are HTTP callbacks to intercept requests to the API server. They could be validating webhooks and mutating webhooks(details). Admission webhooks have been released in BETA since K8s v1.9 and GA in v1.16. Following prerequisites are needed to be able to use this feature: -- K8s version, v1.9+ to use admissionregistration.k8s.io/v1beta1 or v1.16+ to use admissionregistration.k8s.io/v1 (Note that volume snapshot moved to BETA in v1.16) +- K8s version, v1.9+ to use admissionregistration.k8s.io/v1beta1 or v1.16+ to use admissionregistration.k8s.io/v1 (Note that volume snapshot moved to BETA in v1.17) - Corresponding admission controllers(MutatingAdmissionWebhook, ValidatingAdmissionWebhook) is enabled. (in v1.18+, both will be enabled by default, with mutating precedes validating) - API admissionregistration.k8s.io/v1beta1 or admissionregistration.k8s.io/v1 is enabled.(Prefer v1 over v1beta1) Admission controllers have been in common use in kubernetes for a long time. Admission webhooks are the new, preferred way to control admission, especially for external (out-of-tree) components like the CSI external snapshotter. A webhook server receives AdmissionReview(definition) requests from API server, and responses with a response of the same type to either admit/deny the request. Following simplified diagram shows the workflow. -(Note that the mutatting webhooks will be invoked BEFORE validating). +(Note that the mutating webhooks will be invoked BEFORE validating). The webhook server will expose an HTTP endpoint such that to allow the API server to send AdmissionReview requests. Webhook server providers can dynamically(details) configure what type of resources and what type of admission webhooks via creating CRs of type ValidatingWebhookConfiguration and/or MutatingWebhookConfiguration. It’s worth thinking to provide a generic webhook server in csi-repo. At this stage, the proposal is to create a repo under CSI org, however only implement the validating webhooks for VolumeSnapshot. @@ -227,7 +227,9 @@ CRD validation is preferred over webhook validation due to their lower complexit ## Proposal -Tighten the validation on Volume Snapshot objects. The following fields will begin to enforce immutability: `VolumeSnapshot.Spec.Source` and `VolumeSnapshotContent.Spec.Source`. The following fields will begin to enforce `oneOf`: `VolumeSnapshot.Spec.Source` and `VolumeSnapshotContent.Spec.Source`). The following fields will begin to enforce non-empty strings: `spec.VolumeSnapshotClassName`. More details are in the Validating Scenarios section. +Tighten the validation on Volume Snapshot objects. The following fields will begin to enforce immutability: `VolumeSnapshot.Spec.Source`, `VolumeSnapshotContent.spec.VolumeSnapshotRef`*, and `VolumeSnapshotContent.Spec.Source`. The following fields will begin to enforce `oneOf`: `VolumeSnapshot.Spec.Source` and `VolumeSnapshotContent.Spec.Source`). The following fields will begin to enforce non-empty strings: `spec.VolumeSnapshotClassName`. More details are in the Validating Scenarios section. + +*Immutable only after the UID has been set. Due to backwards compatibility concerns, the tightening will occur in two phases. @@ -357,7 +359,7 @@ We are unsure of the specifics of the reconciliation process in which the contro #### Current Controller validation of OneOf semantic -##### Handling [VolumeSnapshot](https://github.com/kubernetes-csi/external-snapshotter/blob/master/pkg/common-controller/snapshot_controller.go#L192). +##### Handling [VolumeSnapshot](https://github.com/kubernetes-csi/external-snapshotter/blob/v2.1.1/pkg/common-controller/snapshot_controller.go#L192). If the object violates oneOf semantic: Update the VS status to “SnapshotValidationError” and issue an event. @@ -365,7 +367,7 @@ Note: - If the VS object has been updated AFTER binding to a VSC, binding from VS->VSC will be lost. - Deletion of an invalid resource is not blocked by that check as the deletion workflow happens before validation(code). This is to ensure that a user can delete an invalid VS resource. -##### Handling [VolumeSnapshotContent](https://github.com/kubernetes-csi/external-snapshotter/blob/master/pkg/common-controller/snapshot_controller.go#L91) +##### Handling [VolumeSnapshotContent](https://github.com/kubernetes-csi/external-snapshotter/blob/v2.1.1/pkg/common-controller/snapshot_controller.go#L91) If the object violates oneOf semantic: Update the VSC status to “ContentValidationError” and issue an event. From 8ed2bd83e16a4b390330db4e2552fa05b58e4560 Mon Sep 17 00:00:00 2001 From: Andi Li Date: Tue, 28 Jul 2020 18:11:43 +0000 Subject: [PATCH 07/20] Fix toc --- .../tighten-validation-webhook-crd.md | 86 ++++++++++--------- 1 file changed, 44 insertions(+), 42 deletions(-) diff --git a/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md b/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md index 747ff1b8e7b..db219df9368 100644 --- a/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md +++ b/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md @@ -101,46 +101,45 @@ tags, and then generate with `hack/update-toc.sh`. --> -- [KEP-1900: Add additional validation to volume snapshot objects](#kep-1900-add-additional-validation-to-volume-snapshot-objects) - - [Release Signoff Checklist](#release-signoff-checklist) - - [Summary](#summary) - - [Motivation](#motivation) - - [Background on Admission webhooks](#background-on-admission-webhooks) - - [Goals](#goals) - - [Non-Goals](#non-goals) - - [Proposal](#proposal) - - [Validating Scenarios](#validating-scenarios) - - [VolumeSnapshot](#volumesnapshot) - - [VolumeSnapshotContent](#volumesnapshotcontent) - - [Authentication](#authentication) - - [Timeout](#timeout) - - [Idempotency/Deadlock](#idempotencydeadlock) - - [User Stories (Optional)](#user-stories-optional) - - [Story 1](#story-1) - - [Notes/Constraints/Caveats (Optional)](#notesconstraintscaveats-optional) - - [Risks and Mitigations](#risks-and-mitigations) - - [Backwards compatibility](#backwards-compatibility) - - [Current Controller validation of OneOf semantic](#current-controller-validation-of-oneof-semantic) - - [Handling VolumeSnapshot.](#handling-volumesnapshot) - - [Handling VolumeSnapshotContent](#handling-volumesnapshotcontent) - - [Design Details](#design-details) - - [Deployment](#deployment) - - [Kubernetes API Server Configuration](#kubernetes-api-server-configuration) - - [Test Plan](#test-plan) - - [Graduation Criteria](#graduation-criteria) - - [Upgrade / Downgrade Strategy](#upgrade--downgrade-strategy) - - [Version Skew Strategy](#version-skew-strategy) - - [Production Readiness Review Questionnaire](#production-readiness-review-questionnaire) - - [Feature Enablement and Rollback](#feature-enablement-and-rollback) - - [Rollout, Upgrade and Rollback Planning](#rollout-upgrade-and-rollback-planning) - - [Monitoring Requirements](#monitoring-requirements) - - [Dependencies](#dependencies) - - [Scalability](#scalability) - - [Troubleshooting](#troubleshooting) - - [Implementation History](#implementation-history) - - [Drawbacks](#drawbacks) - - [Alternatives](#alternatives) - - [Infrastructure Needed (Optional)](#infrastructure-needed-optional) +- [Release Signoff Checklist](#release-signoff-checklist) +- [Summary](#summary) +- [Motivation](#motivation) + - [Background on Admission webhooks](#background-on-admission-webhooks) + - [Goals](#goals) + - [Non-Goals](#non-goals) +- [Proposal](#proposal) + - [Validating Scenarios](#validating-scenarios) + - [VolumeSnapshot](#volumesnapshot) + - [VolumeSnapshotContent](#volumesnapshotcontent) + - [Authentication](#authentication) + - [Timeout](#timeout) + - [Idempotency/Deadlock](#idempotencydeadlock) + - [User Stories (Optional)](#user-stories-optional) + - [Story 1](#story-1) + - [Notes/Constraints/Caveats (Optional)](#notesconstraintscaveats-optional) + - [Risks and Mitigations](#risks-and-mitigations) + - [Backwards compatibility](#backwards-compatibility) + - [Current Controller validation of OneOf semantic](#current-controller-validation-of-oneof-semantic) + - [Handling VolumeSnapshot.](#handling-volumesnapshot) + - [Handling VolumeSnapshotContent](#handling-volumesnapshotcontent) +- [Design Details](#design-details) + - [Deployment](#deployment) + - [Kubernetes API Server Configuration](#kubernetes-api-server-configuration) + - [Test Plan](#test-plan) + - [Graduation Criteria](#graduation-criteria) + - [Upgrade / Downgrade Strategy](#upgrade--downgrade-strategy) + - [Version Skew Strategy](#version-skew-strategy) +- [Production Readiness Review Questionnaire](#production-readiness-review-questionnaire) + - [Feature Enablement and Rollback](#feature-enablement-and-rollback) + - [Rollout, Upgrade and Rollback Planning](#rollout-upgrade-and-rollback-planning) + - [Monitoring Requirements](#monitoring-requirements) + - [Dependencies](#dependencies) + - [Scalability](#scalability) + - [Troubleshooting](#troubleshooting) +- [Implementation History](#implementation-history) +- [Drawbacks](#drawbacks) +- [Alternatives](#alternatives) +- [Infrastructure Needed (Optional)](#infrastructure-needed-optional) ## Release Signoff Checklist @@ -359,7 +358,9 @@ We are unsure of the specifics of the reconciliation process in which the contro #### Current Controller validation of OneOf semantic -##### Handling [VolumeSnapshot](https://github.com/kubernetes-csi/external-snapshotter/blob/v2.1.1/pkg/common-controller/snapshot_controller.go#L192). +##### Handling VolumeSnapshot. + +See code [here](https://github.com/kubernetes-csi/external-snapshotter/blob/v2.1.1/pkg/common-controller/snapshot_controller.go#L192). If the object violates oneOf semantic: Update the VS status to “SnapshotValidationError” and issue an event. @@ -367,8 +368,9 @@ Note: - If the VS object has been updated AFTER binding to a VSC, binding from VS->VSC will be lost. - Deletion of an invalid resource is not blocked by that check as the deletion workflow happens before validation(code). This is to ensure that a user can delete an invalid VS resource. -##### Handling [VolumeSnapshotContent](https://github.com/kubernetes-csi/external-snapshotter/blob/v2.1.1/pkg/common-controller/snapshot_controller.go#L91) +##### Handling VolumeSnapshotContent +See code [here](https://github.com/kubernetes-csi/external-snapshotter/blob/v2.1.1/pkg/common-controller/snapshot_controller.go#L91). If the object violates oneOf semantic: Update the VSC status to “ContentValidationError” and issue an event. ## Design Details From 81a815366f175d1526f9d9b56e04d6f23539b8a2 Mon Sep 17 00:00:00 2001 From: Andi Li Date: Fri, 31 Jul 2020 16:43:11 +0000 Subject: [PATCH 08/20] Add webhook workflow diagram --- .../tighten-validation-webhook-crd.md | 2 ++ .../177-volume-snapshot/webhook-workflow.png | Bin 0 -> 27341 bytes 2 files changed, 2 insertions(+) create mode 100644 keps/sig-storage/177-volume-snapshot/webhook-workflow.png diff --git a/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md b/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md index db219df9368..41a314bd7ac 100644 --- a/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md +++ b/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md @@ -205,6 +205,8 @@ Admission controllers have been in common use in kubernetes for a long time. Adm A webhook server receives AdmissionReview(definition) requests from API server, and responses with a response of the same type to either admit/deny the request. Following simplified diagram shows the workflow. (Note that the mutating webhooks will be invoked BEFORE validating). +![Webhook workflow diagram](./webhook-workflow.png) + The webhook server will expose an HTTP endpoint such that to allow the API server to send AdmissionReview requests. Webhook server providers can dynamically(details) configure what type of resources and what type of admission webhooks via creating CRs of type ValidatingWebhookConfiguration and/or MutatingWebhookConfiguration. It’s worth thinking to provide a generic webhook server in csi-repo. At this stage, the proposal is to create a repo under CSI org, however only implement the validating webhooks for VolumeSnapshot. CRD validation is preferred over webhook validation due to their lower complexity, however CRD validation schema is unable to enforce immutability or provide ratcheting validation. diff --git a/keps/sig-storage/177-volume-snapshot/webhook-workflow.png b/keps/sig-storage/177-volume-snapshot/webhook-workflow.png new file mode 100644 index 0000000000000000000000000000000000000000..65c50b68699b10e30fb209eb3c4b0270d37dba1c GIT binary patch literal 27341 zcmYhibwE@7`#%f_3MwEiEihU@N*a`q?ndIKOS(sil!SChH=~j65D5|K(F~9pjlc#n z82k?2_viaO|8D18@2lf=y-$>enmqnPs)raD82E|`vRW7zSgsftn2osifnVxBP-SCa zIHV}bzSi+wJnSNLwSpPu`B{wIkOudq)%9Y9wSX2Am4YJ~`E zT#L#2#nI=FUnn=C_k&iCbGzHo`mpMoGjUxAbgw(`>ip1uJn+bW%wNrSAKr7Qzj~ea z%kp7K4)cZeG0N;-W0pLPf65t~OTY%F)X46G39A)Ub0XZo=j1v}+TND(vTu?iJJ1Qv zK18=A{$t-v>UZokAx22iGuQXMnooo=u;U&^eCTCxC4GqX0QiLsmj+9|;F&81E)7n| zJ?yv-y_gtS@~_vP2tULL5xeISbXD0S_-9*45~8taxN;rtf_nace>)}=jNc~n^uYNk z)Y($J)AS=&Tx0JJ!H$#uOW`kNT$=C?f6w`k{@&oC@_wo1SsTIMYiv$GP`ybBx`4&z9W zA1*Q09JKJ`(o)ZkvRe1$Tb{fC`=b#xN#QFmIHM~mHbF>{#*PHZR>nNwgO-dLHRX(0wb<=CcBYwkrd#cZujt(D3?+stvuEl2u zTwI`=`cCabuJVE}=N`sOb=)qo2H(=(T9}Jedt7~moBG36Z;SF!p2O`;*Q+bkicK zYoNB2V@ini;LZtT);RF8e_TmQYPRZ`Uhm(^4WB9BqUUl0elK;`|3*dLuIuMLzwL_8 z1s}6I2W>W4BQAQ{&|VoyWj01!XEy^=sMo)2;R<8nuXjVJ5BW!T^?Y~ZJsvKqZ`7vwTGkoRUa&v z{mDI+YRb;kTSe?uhFj0n9-OUQrzM}%`uarmwoR0aAU@ST&JCFHU)A@GkKR-|Mn~5M zs57iXwxFJGYw&cJ@~v0UxfkS;>c=zSpl$f=rQ_+j?aD<+AAF})YOL$bq!!Omk7Tc< z)|g!!VplY~zp6Mz?h@0iy$@cZ=b=Uf2opG9&O<^NRpIDKZ?22fpWoZp8v7kui9uy<$X;vNwidwzw$4SMp_i7 zAJm^23?9w9?c<@AY@)?ACK);Exj}wF1gwkcnfi}ev&UKO*2TmYI^L|fW3h^8HL8uX zqgXB{&*;pP1KPx`@v^a1jx`Znai#Xuplt3GBirYuHp{Ci*SBqY0V)zIrUkSY1;YPiSMJO6v zUarfxtpzfVS%tuv-?*UH3+8gCEx9P3bq)T`5n$;(TC2pH;VZk?x z{!-`qQX0N$wY#@I0Z5A6&eJ{WhsP(@2z1lW$nG-6~)?41jYpQ0!av77CENj2;JS3TBPtdw#a5O9WN&735?S8}hPeZxy zQ&U@5toA>ZQhwW5-Ht7i;}r+%T}!M;oe^OZ0ND;JvNshsTfS;aPP%lvU-)4O_5)^# z*0KTB6!3Uo({7>4X1GG%`JvPavX+af?zB`-ZojcJ(`+u-R2-?^@t#>bxwuBzM)EIQg{*gs=XVY`tD2Y=kRP=kKSjz()!feRqX6I2CW zc61U4qFq+$_|w&tpUwo+`{>O;s~bxQYPbYPEpec#K*{*21fM=P)60GT{}MU&SkXTm z!8h`@kCIAlREOV!{qR)vW-El6+HuPXGh^~)kGd#&I*X%b!j<#VZ(_^iKT((_gJwBh zcZ0lV>g^<`ScjE^AuX}pM!xtVjux8G>Sfwc3JR8Vt|+y%KgBrQ67S@hupp;CEk|Us zYU5Wg2he9Ax9F!8hPsRub2Ff-kLI)IJA#2~U^6?3|HXx)4SlQ6ZThX@uf?P^VXjJ+ zxyA$EsH;}Rd62pT2Xa3@)7tWv5-V$e6iM`~pv)R~z(~iBpkTpj2TH-Bv@12hrDIU; z_L2T(O`hkr%d5&6pHWg0&uXx1b+I4{M$3fic_qak4L})6^huxJ&-Jlu@Z>CtKk715Y7PZ)6;tZHQhHQ|t-6%kB(Z4P#p)cbJ9!b!6o*TLDD1sV z5ZmG__mbI^IqjE2s~}fA^E6<8c98Dh2uk*8V7MHGHD}83>_Yg7Diu!1{=jpcbZR8i zS&(ZkX#zQq3*XEcx5@7Jbk_T9MA8<;Hb&N;21gVSAeUT>f*=oYe5UTk7AbF~+**nv zM^wyEU9tg(<7|TpnWhfE%krH15`3J=Rh5I739V3}!Kvz0Vhyc*p2n)%N&Hd{4!x)% z!-CjlqTu%EMS^OxU}R6})oIM98~wq`=E)XzwL%4#ct)QeFLhJYMs_3~t)50{t2O{q zW%ll7Ymv7&|Ks^npXzgmu_9s1h9PXmMs!jqa0S0F^7y16jFhZa4uc)nScs6R@ZEH) zOIH7zL=nX_Y}NJR8>eU}yD0ov8>&3ME67z;jESfDJd{o@x!;}=UwvJ*(QUc{^kc3> zA!Ac@1j^`Y6yL17R5d<*)g&naF;`i)(Lshz?{~*a@=M&%V)+b~H|Ru_9+zztvZbdP z8kq(jibN%G7{{-augAfqxr8pth##yYeYfaGG{D?Cf!*T-zcxA)>(`z1q$KfjxbG*4 zShu&krY+G`>l0TX_aq$BbV2!PeufR*53nA3MeWsIIBAor2G6)~jj5#W zD7FSrGGhm9&26cT?&Ozc_bY1hF>99{n;7+xhHJ~JY4S&96>DjV4?_rNE3Lx*+&1>_ zqQSMZ1#Z8lhDlZtq4dvj%i`@JLokqwi3kNpczI)f3yzxMV*8|~z;mm|NbM1*Z@VAf zMDN-k}C8WlOx1{5ZwrAWId%o^MI$z8_+M_rd=f!2A&$ zwaI2-#i1cEqK2>uH+yp!lHx&Bsk?T4292%wn1xzP%hET%BTssPm&zEJ#2b~IJMC{7 zGL33f-5aTNVOOi0F=aRFoNMa&z5bFJcXGL>rEd(K5PnJNG2> zp#>208B9 z#$S8iICo#0HoBPYeLgrp{gJiudk7Z?=dJ~xNXLR(iYj$Nb#~>3rFPAIe^?iQ>*L3H zmp}IUO~Q8re<1X9p3hhDaJvz|7`6}>i;X7!Y;nRN#Fa~&fanr8+NQk7{l1C*gD{!O zD>&W_L|%84MAGa{;4b@iUlU%sr||S-^Yp0yZNP!n-m|Xv@@eE&^thy!GJUIOx$au$ zF5kYNy2bAs`y{q;?@^BKY-}~H4Car-!t^O`CuQ*!NePLH$2nVq#w9O~XLn_?i&4Fb zn=end=Kf9ucQTY5x>QCfFZRj2MgGQ*FWw>(E)t)&zJHc@UyKP^iSjX{k(BZOWN`UUTzoMr(bekf3At z@fY{qb7WNuWy!2lWh4DeKykFehH8`l6_GIZQD=;@LVJ(CXeG4B2skval-to*?wFdw zrN}m^bYF^@MPa^Eu_3AVQgr5kN(h1iBQnR+Y36dGcS^2$y~=xWuxrku2lYm%%&HgU zLQk4EJOV0Sd=jFFH<{bLf|OuEqLowCW{dYtd3TXjuYFTfb;t8RH|c3@*|69R4L}kf z>KNLu^ssO*3T@S|0-huolDwu6@k1`l)A`bG@&}8KOS6uS3RcOMph+%j%l<0G>eo_P z7Wa(H$rm~!(}ny!dbp5hDz@7G)MYKQ3{Mc9xyM(nzO*)4gA(UD4%`Apr=#a`4IsNW zVFn5UuWFPyrSuyZcUn~m5lad9MBzdyKzkiY#kWwRYE~-Qv8PWs9Zah$2Lr+;H}v+>`5?Ao!==*R>C` zeO{Ldn3#$u)TEsqQO~R--^3cR7SFlwJ%R`R=8*fw9N7I%FQ@cE4;;*8@}kkS+Dx+t z;UnxwL4aM;gHyAdU0BVXS)n}b=GEStGk0Sj(|eiY8X&*3L8>TOAEp#25Axn+(U&W1 ztU3~&w&ZRDu{)GVau{(RJxC%K&hu|vcITj?(earvdZWrjg_e-OC~baif9=rx+beac z*%q1OX0i)25hflV20fx3Zqy`dqvmIR!jlL6C)))gcNLDf!nY9m`_Rr3Fq-;y>gv&1 z(jKa$O-7<3h0HwZj~!*TJPFiM!tD$~rw)v!ta+`Mi61`OQ{Jkr-jJ9DS4d@UE6-e) zqRf(q_q%q{$twgBv-L?Fw`xW`=a%gY+9M8>PNC;n-9Y!5*%XYRM3l za>bxlPP!*uYSAUfH7r}rkFt;M3r^UGyS0aXy=wR~|E5m7Y?vYLM|p6R4_(?HUy?WC zz7XXk)Yz^cx^_#(p;v7)(Vv<8v~zx##kfhRyh3CcCU~DDypRK;hQ0&qo3x zsh>n95&Y|I^mL2878vgnOX6HRh|*2oze~MvZN2ECBepfgzR*X4H8Jt2HS%Q2zbV#g zTW0k#_d}Q+makLR1%C2ld&=Q5S%nb=akC4Ql;)GPKOK&YU{#yrKK77F|LH5XE?RKa zaWSJg`nTEjUf;!ZyT!@O1r)w_))d?Pc8U6tG53h z(rvOB@rE%}mHLfrVuNos6Z$yk)64-;AzrzJC_KZM_=E4!w^kQLngtJDMOGENgEHR) z_2V!a+XnVN=7KY|z9b?=NHc$&DZ8V}zI&TaUP@Irm^=7}5=i-uGn-t_95c;fcKtqq z5YNrAEBsRC=W!~7w~z~n`jgz;-(p9?HDy^rL%w--J%dHxI(ijF#}}9`7MAdp)_cJZrv(vf(` z%GGkD;i!M~#}avJ_4vt6O_1D?@meF7ecMMdV*iMD;2=3N!49Q!V*;efRG?a;v!W{a znX0;EneNJZw?y;@zcZI*`^BC}dG_f*dqjJ)=47kV)o}Q{oci(fjh7)3{dopTBjnf7 z%dVtE|1XwBMIvnSdxK@N)o>=VJ>lu*iWs=!tY>enibY5+P+SRqO4h^uRRhYMR^sqs z-GU8MF*w&s{*?5Qar>@d<*wEwhg>HWQESzKdZ6tpf!s}f{aYXgR^qDA8=9?PpbKjB z^eA6}qpwNvQ*a?!Z8M}nHCS-%wKAz}`DzP>$4qUThaZ`U_vU48s)ih1JZ#%5kYbTAYrh{xIyV zYfya608sa1w~`(NwD|@iagFuNMADm;_O+ngTuP(!Qd>a~giF;{lX2TGoiWw50Ar!3 z#6orZ-IHH@2>sfD(f7I1wLwOPOkW^FqDq9ByVAeVUmS>9l?=ahOVE8%77_zb+7{!e z5edg14NTvpP`er=RwJHp*149~<_AZ}lQ*1e&IbKR(iW^#S_gfNepk`#m3tOo^e|rRsJ#f`CpqlRnM*}KX{TSUl zFp{mi28I7zt!053Vb?6*MD=L%1M{{Hl2V0dL$w>u;hIF0H^L+F?xTB60-U}XO3ffKF+K`O-4~w-0nr+CyXpSkA zRY_V)?XfuHHf8qiwk`$cxTlKHLis`i-D@|@Z1{Kx)r)>h|Ap@e>z>2TC=`n z^46-ljmB}~hk>`n;nU&O3s(@mkTW)OQ6~Ms%`uZTsLu~u((ln6ibnwA!0l|BLbEFp zr7fNv@kklm{uJ+1kJ*Uh=f`rxo%T)SA=6jLT;UEr=QoI*4D6rDRM-5-xvaox_uZ-D zxZ5Uy5^$sX5)CkvJDrZZ4ntiT7s&%}TPY3ubh!%{_xYQ8KoM^1^4FO`MZ$dSywLiU z)&Du&0Wi-eX6~;OTt-(6(83z}U*()L2NP$s5~ln7xVTW#K-?H;fC6_}q>n>Oeoj1p zJ$c0}%HhTre!;GdsL%_UZf`t1u^m98)`Psc)$Bv2Jr=ixi<;K%UwZXl$rJJ19xbXb z(%Xkr+TQjr%>n?8H@lzp<#Ax+@8zM}!AN!Mq1>2WKkstxb3>0J@Tv6d<~XsWTo{O) z7>C}?B=I@LGYS&=d#bAMjZ9A|d=*87ddCGhKK@o@h0mm=ws!6FJ#$c{r(m{vRl2XbaOh-$ z$lwur}bp+SJlidki82=hYjermR_z^vvNXMq`uiU2ijSf*FF{n`;A_7mBK6iXxiuYx$nK>=paP*&WFdIb235$j5{Op`PEE!YggoTzFsa|(1RQ_60_S#Jf$G($rxz-<@1#5G>4Ygxf)tZ7 zji#r6?09vVO)=q3x+fKRo|X1Lccg~C%N!G(@efP@#_`z5-x@sgac6=m*C4@d$bBzCN6x)6w)|q=xIgw?;9&53^Z&4N6sv(g zJ-;AB1zSZbd16(m*0|n`D--bpq~pjB;#ATd03X0jl~r8GNJ?*2)W~C){LR2aI-?{% zjF1#mPrDMAYinFVe4#<9E<|9NH0wcIP+j7h?7jZecS$kmd!cZ%i3Un;v9$i;ur8T-;Rj6npY_AuUj6^yvWB zZboNxzh48#-P+;rJ_Rt~9D^Lu>!?i7SVK4&a@xai`1${>Q5ak$G@-X@C2w2IG=;54 zz*0WQNbNVy?ds3|e*{XtMQ18ce~bEyD&Gy@b2g@w;Em(1-Rj!Te5k&upVwOa%4eQ~ znENBYIeQ}hZ@H3h@)$m1d%ATyl1|gH@%Ud<`fi4|<30YFe#>iSY9#gLHpc|mygU(p z1uR);JYDs_23*RJ2=QbckpJED-0_j$a9N_TLK=Q`c0m09{pevhk{+DS6P&;kY!y7T zz-0@x_uZ&q#dS7hy3}Q<{_T78gaIXbofjq5m}g7l|0!LlOSJ!T<;`Og^B?mwtdT z=JS5cVsVpuqa{{zvKuZ{OrI*2 z5?bTub6SpupVb;PxA_0j8Ow+Y@FR}awoH}LYzckn{@_0roh%s42o7CDpp6wlE)56V zzzOfw{)s)KMST_|X^7$&Vc+l!h2h}RV0b7fqC?+4V;Pn1( z8~4C9XsEMZCDqBuiS0>O%>p@h-4aH~Jq&DuwSdn}8-eA6TY}@eFFWvo?cS?3jIqcW zS1=#S0UnV={bt_lUlJ3h0es+5dr_mIEAO){z7E2FpIwXzJJb}GpbQjDli~e}2~d3> zCqp5C#6iV}DBMHPNm8QSbB?DcM8~Dib&P-=Ax3sq<7l{rtTaXWExTC|>x*`A1bMp5 z6VLiaeVRHf?MygD%*V-1!)BIi1yA%JhzoVnkUzIkz!&$IB8>(@I(eszxn7hXh`;#a01Vd5Y zY%G!(z^-#*aw?+twNR1exnj35p{>hlHo?Ho(z6b-C&KTK-qZ{Dk1f4O=4p}UWvU^& zd$NU(VlOMUn>eW%O#t`}@YMa!0*0qze)#KtCp0C8_SD7rA)(H)q5B+0FK3F8>>qq0 zVGnU>Uim!J)Nana$eR3k$a?8MbF+$jn5DPZ`S)pyi7{nWZmGp$FSP|Xsh>wUfIY7> zUN9)}Th(u-BGEfz{$n>%jY??e7#wbAxOohabOMatxE$)e$}XLINkwHUhNOV1nf_R#IlGTM}1 zJa*h-#%Sf%)8|J4kIbeIEofB#*-j>LD^6PEGIDN^;W&4AZyC-8AVXHbDh1U8c~Aks$Y?pWtfWxCXlJ$2efv-UZ4R zn%a#A^BnW38nM}3vD|bbBwuhyB)V!?dS1WX{vP&@DdpuSy71zAh|Xb{LzxRrdAR}# ztO#haEW*|$zWI+wCNB_n>z{vCFc-~J8+IPGrb6@qh2M{|G;xCy*VmQxj zz2|6D=5y!6t;TQ04e=5;yB>cc>|JuXb7(Luysi3tt76A{TjDk573dcS z(G<)hFe<5eXfj$NRA{}eGFDmyhjw~MHPw`81fP{DFPY{kQe!=siHo{NLa(@N7%{83 zf!5|}Di4}$ljv3ds~v;-j!Q?qa-^Rw7_ z9Gp9c_v*O~fyKgF&h&tdP6xryr8XoRR7&k_B}-8)OVL%qrAHYS5Q!c>M~@~qz9szR z7qyydc4jPPnG{y+wN=JFzj%EwFCIu;t}mOJzEV9J&YzQp(R8XibWyIPtFDwTT?c}4 zTP_U&6$yXpeK~lQ>MWSW za^H_DQ9q!PsQw#`CX&~8L=$3~`35gE}{^DwY{>^sJt@y|3p z-?4%Mr$BZ~=YNuNzKO!24{BB@iMb5}d41GYo%$q$!}2IGLiUG#z~mIQ-@^EKwFh=A zYZS*)Oim;|jGKoI1z8vmtZY>Rw%{BZHzS)hFj=&$ziIm&5j) z7P#M-^1)x}l#yvZ#^d3DL8@QKTYgkFj6KSoq{`fF(rY5K0rj^1-3P=EYGL~p0P&Idi-$-u}^qZdK&gYydC=fqE z_zvaAOR^`QPd#02xWNg?r+&os!m5{Nc|op)AfkR8{*!PF?%7t_PBj z&?1q%7+7Cfc$120vVtOEx5xuD6H>tnBX>@O#@{@?+%{5oc=V@{=VBQId_Vw5) z&VlbuS8Pw=h}{C%jvTxO+U878RYsEe+>5EbZyX=6$3uOYsp1!osV(XRwa=?J>=uMW zJr_V~%Yt><^sUtAH%5^-ukIX8%0}f2B8$WQC}TBAP;Qhi+={B>&0bn)!q0s7T;uIl zpzedC9lbH83kIAHB5>me9!?dk8SM7BF_42B>&itsDUL<>dU}{YfjJZ zJ#&TiRp}N-e=fPo>mMz(kf`{suayd3ez&&|`*hS;X>yR*ykbEdvM;*M0QNp8b?VI( zBY-e%$4m3cyGi^(_@;~PbeJ_B^Y@x#yzljTr0iXg)1V}p8{xo|ViShH$k*<#rVgSf z?0maV`-d#~RdKbD%i|}lN9&n0lMal!2ou#pp~I#t7B6n$BJXK^mua}Wa30Xo@X{zj zuCXw%YdAlO9o86>90Z!w7!A-CS$CBqOxb9U%NP4k?lM@;Xerv6xG8()LAuP;vD3Ko zflm5dGVfmUq}0RVXFWO6s5fL7Pg-k8T3=WGgXT6S`xB#3e_xs}BFn7UHt{83o%=JG zds=QnC|WYZyyiLb!+^1wx-m^L@|V(AtVOk3!m#7sL#$V8`OlR1T&olo6(~cNTEtvn zPRF0$DNaFS3pXP+c8(U{T-CaZP-AyaPrYQkXRaC?P7aI4by)&w_l526P&2MNC2;OOP#J!OrJuSMI5M1TUTbMGkra~;UTP&&J(cDs?lDw`9lfB ziQSt+;~rb&S%gHb2ii}8@xVvs60>cj&Tc<@f8scZACz$lsJh~E?uvJYDHQnPr($IQv!Mgs z{Lj`UQ)LMsL7n5g4~>{ub6I7QPhS?CC@jap4rVxoNecLZs_~{vs2Y$A4RiCmN{cPsE8>?jBHT|@G9%k`i>z2zb=qq={Umbqhi)XH6x)$%TLXOIk1T8@d z8URBKI;T7FU|{*EDP(e3d)F+{9<;*%tmZK@U97=Chl@|k3HC|*MwbKGZ#j~Ne|%C6 zeZIP#m1U|OGu{a;-af2N@ks6vE79o!(Y%eWCW6`Jij^#%tjZ)l^Q_d`aoprBs z-5TFLPl=22l(G`~bidM0_YzFs9LeG?9k^h+=n-0Q{l44YKI;)|@+T%PO*ZWe`^n>b zBrI-0MXh8%#(!$XVOA=Oy78fuVl!N~g`WdK|G7K?MpJNn2?Ei6ppTdu``UiSH79_2 z4%*;Rod?HN>SW$Yy=Emy0rvJ|0wyM(w>kCNkTR@S^tjpByZvE~pbh_;aL~J+>No^X-^gdoBNZx z>Q?dqVR5%DH)$68ph~l8#MH^0r>!rHu6EJi`@mUWwS?l^?pL9^JoLBC;+Cym?3c*+G0Ko;u%0y9>3{Rm8QhRaR$)WZhX5@cQ-#CS_hc9e391ceXJ zqobBErtag?K-pY=SoK&ULpD`mT%C?XA&Rkc)J?OLCsv3yZkW&~Tuux`Vy|Lxob;5Y zHv>|F%<#;RhOi`_7$CG(I_1It~`WYa*;V`DZAmy_7z;`z*J35qj+g zo$v~j^8iUA+NXlsd78y;WAXw~I(<^+4r>zoRuCKQG0dU_poAnQ2uC-35Bcw4Qpps?tDAaQs86aIHXAzOnk z&@&#bT0)~P9d&G{6)V?*k&E=fu8v{zd+&Q^J{%!(LubV>s>tLf9_sbtS8w zs!BcIRJv;SAl3C_>z|ybvxl@dx)k(Q&^Ex$Qn#LgzP*1OG5zbxU@=bRG~wlrSICl; zkU;GHC8XUva|e4>qZFAR*cfj6cmZCP`cLZvSsy8o$15PF^huO7K36b0Wij~MJ$Y|R z){5^f^Q}zLgAYVm5DnFjuE(E0ZAyAHa6|{=0~(#i_#@YOz+|6*9QVw1)M5%WQe@e3 zcw4}7{Zd`V(xbqeQF8wGi^#_qSdn*6!Qsq|Idf&NTp5perv_iua2#%#1H zbCYL#x7#F8bt9NICvkIRLHEMbc1-J0n^8G@04kWee^)h#NQh>5`N_^9gZ`C1?-w5x z_yhhn=)98?6ae%YBnjCj(sx8(7K~l&4VCz*a3Z&+9FYRSC@VyRMC&-H_(Iqd`2TgX z!0;l&Uy_(|e*|%lqZPCFXz(8f&0>1Zs8rY(B6CKNYe8pikRjnP43zd?agI7QK!=KK z=gQhPFD#H?TM^KQ|aQRV}U>xl$vq%5FKulbY_@^_T zk#}0ynYBPzC-K5itlqsVZhSanVtMCYIc2ls5GjQ0mCt|AbLdyAxeGuC3MA6LNYq&i z^d4D(EWddjEtvv?3ssA%4rczNf4ObPnNjvAcuJ|Qy*r|jeP0=w$I4wMHP>{1wu=hv z$cHkDD?;}wU}Dg6x$cThU>4&8m0DWIhka^WlL@IkEntv5xgY;+_c_#IA3DKAC2iTL ze!h`Ey!04C@(TViTWvz7vUt0y5FBymk|sY|VZIeNRJ=rqjn!FUGYu}Fck$B4s&CBj znSGbHb+C4fL#{+S#B_&N^8WfBH@Z8%Dz1Y%$wtRI$MnnX%(WjY>05G#0oEe3-G&YM z)vNdW7{EEptkQMC-=k8;>tX=s;kI0~k_iT+GCy0u0y#OD*cC5KUyY8QFVq#W74f~J z5dMTAs=Wq%hq~P!IX1+(=(s#xI$#9iA(XPAo%2Yiwk%WrJ2_LvnWRhfcZr8hKK<#v z%K}lmLwHMbSeCe0o{e{~;vAPk1vFR{CINk3A?Z|)L&NR0tF#bhV$?HR@PpA5{0iDjll!ckBO63*caMtcXwF6-LL za&?ne-i*6Ry+aNdS0xzRmz^~T^RV$z>RJQN; z#CxdG!9t+W{faV5K{^)MX%k%~)8E58@T)(c4Fxst0|y`a#Q9TFa2+RfjEocF8qXD! zEheXQ`th%-M7(EHg1D`g?*EBJPS6AWJfrQEXRP)Ej1Wgw2QAH)hnX<@A-hH|WQO1O zzpwH-So|6-X($lxUV6mYE}|_^ljtY9F}!mX*!uV|)LM=D{Si^CV8^#E2cIgKs7W*j z(z1gom`0*@n?kUp%rJkp?fU5j?vvIriyzK?FAu~oB#n~wB6qhB`nx@G*&Sud&A z3<|=r@|UpHDD}_uq9neEEy;Vg__KSCr|{0H$a<6?|LN_H%zgXuNMMMg3QiCSG|IF? zg^EUPc$@nfu|eF3IV(h}M)R)uv&%>M`@nt@Rx_xhMTHLOk|&WetD6%&an{xHo@qWT zk%0WT%pG2g()}I%y?nv?1+yR6l<2zzhZhjeY5OeHw!4t6?IalBS8Mf0 z6DeIK&(#HW0mZpq;rj=*-s_%oQxN5%4+jwokt;s{hM* zuZGW=Ts3AsC&8+v{B|i#f{s`5>~Pk{*FH73d)gxgTg6oY6U&GDRFiYsJ5(;hnw`3! zkqi?9rL#8mI8(>Z{cBYqk_ z_Q#Ws(SNEx_d3ZQzr__O!NQtTyn5xe@;FikuVVcC-lPKHsc#0WrMoQoe5~AkxUzGz z+jYyvBT*xMDMz3r35nxTJpXf4L~0~MMw;zlKaZ@|j^*vC&DtO9KKd z)qPXp@(e*p6V1XfVq)&F*@$a0xihsf^>@^u&yFniG0n?e2_OZw!WIg=CA~)AzO4LS znySW}%KfG|n1Ncu^=#Bd+$5dcDA$_+ya z42iR+#g^S##A2j*&z`c%LTCuzc@EDe<1A@3H03@Y78IEvZ6)6JQ0W(mVgtpg#`ZfmmLrS1UHint;GSrt^%f za_a!mFS{uiHQ`olNlSVbY6o54&i~W|);`d)U*)4TjtQG$O1WcA|B$6DFJk_ryjLgq zN+3vG8oBd!%$X+RiVP^t_R!+Ccf`d%G$*+QHH~@e90V-9hQb`aZfevEXBoZGY^<_( z0vYifo^)x5(k5_NANvT=E5R*i2CmwMms3oBi_3V6AZ$wM5Y4^#5mtPtM9DvB&m+l3 z4}GuituyrPQ5||4lJoe;QOZYv0n}f-oAQETAACRhKsM8E05_@5bRqLvA}H~wn{|f= zTDbd3q`W!OiCS$L5>gS*d0QZ01PO9?Jog=M4*ouS0d00H9XvF@k;Sfo?X>#88}gJ= zx!qKJ;K871@RSfKH2`fS_Q{EQ)9TsHQUra#2O!8dNdOu4&5K`W&fsJeIzdJO%<8`l@7zSJh zQT@iszHX|l-23^kfd+E7CEQ}V7j47NbWor;QKQ@XsNmB6D)ijXL$6Lf>z~V3PX>@j zJvhu21iFVc9%+SPZn8!w!}kU5u26a`+IYNXSj#jS9)kC!Lw?QqqSwcl9ObeyCgvR~ zXf71Z?bM{S0F2F97y#x;h2yx$vFcRm%B(`oqL9pm`YF8lG98t}9B|@k=BBe<0h<}x zP~0uNWNWuOn0IFYjn;mZb#9ki#mi)pf-Npf{BybxpFHU=d+oOukR^~&Imj#)+%^ed zN^7wSvCzvuX(kDzCEC+lnYz$Xx<1+c7=Fb1Zg*$7*N>x9^oKf16PgGazU5(yy}m+) zTtee(XxdrbzQZ9&nUf~s%S`BVCRZ4{D=^@pUqU~za_`UpUv@pJTCQ z?#AdG1j8@6_qehRnJdw4cWfxAn4{!0@~Gq4p;5}~Q=jmUm{=4^Y|}H6H?ZdFoq(qe zfpz2#F`h2iNwEOFYsX3A{k4lEDI}t6*S%CO@uOkKl2l`dca;8EF^7b~Z&w9AlP-0b zg2R>B{6UoAY-;I}*S?_)5=Mt1NDjc^h&Bt1aTCI|;9heR=iJ7I) zKJwck44L-kFtq7Z53YRVtp=+*3+i{Sarh*=7qQh2JfCW_yrE_>AZ8JaGOdg5 z$naUABT9Kr5HY1P$LmiczjhR1RYBjC7Gv5$j|Yo$>L830|A!(EmVg)zy&)1tpI=D3 zsJ}6LROD?c-k4Qt*+HaL+>z$SP2YyT-4+wa*F{YrcXvTlAyl8x$ zpSQy)kE<W)MiR`DPCj-HuIAu z?RKxdv4dsd{m)28|JyPkAr!p(m* zZVB$a!S@#4R(xg?VuA;(F#9Pe@E_IN5O3bYU#-V|T{D~q&m)tbf;-KI?VGdF(jR>f zJ7jBLGr?2juNLYN6_T0iC%x#I_Sr!)P>9N{0p(^S+lYCnWTeVjjg%;uD9JCXvS6!a z{u7|BE3=TdK9Ff0k#$1=T-SJ_)`hrA^~ivZIp0iSc2-eGIiVA!E{X2IPuzn${sWV`bR zPtvGVRNklN9gh~30}FHLfQl-lfnk@V7#pA7)LKmR+@pEGQ5*%d$EU+*I_CH8iYlf+cjpYLOexcYcaz`wO zE<84;Ig!1JP%>C&-!HR19I*$DF55+`aN+g^KMgtGp#uY2OCJCmz&MnpfY^7my1d1f zt}hAl8Y%D#obasEVLhHn`Mk?-7G~9&NYY_}nSW0hOa>JvZ%oi62>!puoGNWGc!-tF?m#FFxx5`U&x-2ofCXvXA zU-<#>q!5XJfoF4=d5689`H&@-$s_1y&&Uxra6d#ewID$yri5*v&IH3{Nv=VQgkEZPo<7LAkefz4eeNkc==)J`s{Ix@jP!M{67&>hf3B zqjol7;E=Q79iI9uPR7@7Uz(rYeCB8AZl^3K$JC>5@9I4uyNlTP!bb)7k7g#wBnOw$ zOoVw5yEtETv3y3c!vH|rV?_3_0A}}G-U5Oa*4%otWxsQ169gdK7)lyF404gqvJ~Sc zSGN{B@tOzvna^GKHU_N_P09R|GeOjkhFuw=$vHn5R`1?6)7E0#B^|D4Up8XM$81KA z@q_ht^TO#E%{U~>LqH?JoDaC$uQG$pqPgmX71d*lVQ6Vk6v0Fif!;i$eWP-Ui&^ zb60|jv6Mdiq@X(@eLno|1kabzee6^|Bit*|QxY!{9cG`f0H3p=5am3p-uWBXhLpUN z)2$zdSoZmwgE^Ty6xMy_1OL89-`-ov*z@A>1p>+Veit_TU%C%kQ$&&!Vn#*P{o#4OIUE>C|^;iv^7 zzWa)IbV_`2pWf3)>un@{)?W`&*p|n+6AhfeK++;eA_7?FLyJ}4#Z|$4#!YfTo3{Py zmoevW`=uctmx9W2f98KJ@LBjP8-kvM1Ix&vWm*e zl;dQE{K+m8@n*OSqTfT8r@`=%TYJU}dYL*ViN*+P61bfBJv1m%aBsb7tnunKLuL`5menSKIn% zF|=rLG8mvZmqwK`RES^z{xM(Dibl^-EibafBCV&Npf#-ME^%P&85_9gbVzuPlj zvU)vpZsLz-C$%Is-b5Yd{hPWZTRM90A%lIN=6S_I1jb4)s-;S{x{ZtH-Vvj060(1+ zXVEFtbR8NTF4SYgzf;36!Ifr>LXI2l(xe97>!95cpGpXzYQZQo)#1(Sd`Ps$9ignv z>x9_Hu65tn_~_h?7QmtWO8JuqJw4RK0T|#?%W-xyz*;&i6WC6(ZEm0|_Rs-}MM>UU4rjme2i0qTn0{FMw4 z`cG`YR?UTPT+v|O^#Sz>bLoUYcq4BRyMV_8U)3^?%fNdPqK!3`WB+HSI3nAu@5`u* z+73c6QIe@OHORU&bN?vY(}2^tR<%akvPsL9JQ5t4ucRgACz{w+^TVAV)~Up02>`N@ zo-V_VVYAwWqYPGIsl95VO`7~KFg_Zhq6P=Mv zmwXdA%6DKgMnw)NqTw|U_mqMU*Q;hAZ!D}BR7hWx$f+r ziTYF)m-)(`-uRyB5-I)$-D+&GNI?E>ya-X5uqZ_;r>lY7QKU6vH(CzC-UZHZE)NNO zj(J)rzTL$axlOFQ3oAkozvC6Oru?unsTR@>e_(ipQ=|K!vG*}U8^&9mfe<@6^R#>M zSu)n6q^#?0F}qUGTSvoLyW#)8@niDOKAK_#%r$ z750jq?&yzfRVywJ6XjX^ZbWsG{Z_rs2Qj8w^w2^rL$(7I`#kVg)iPI(gE#>TcA^^*;j-5AV@Sjj)kIX16w&>-8p6aE1Xu7P)H zs1i2~yZMxG4mGj#5X8Uf&$n8?UxRqH(;Xy}kB}`f9ytd%+Y3RCFe}Xsq6(d?G z9U2%?fm%EMj)+prB1>=A(>=XcA zV)R8#<iGBOcyd z*fv?!WBAZ$xra=SE-wPA^aXDE6K)SjliKC#q3su`xJ#g?+#4%YiS#wTvMcruG1M%J zKJF_P_vmIS4igx>1tK5#DweqGf`^(wM|9|RCn(!vm*J>2_&fRXK(G&&3AnE z_HvDg`Hw5fQ9XG!kKanZc(CC0cyA!TCj6wOR(-N2-NFAJ5P~$h{~5bo{G5bqUCgLD zH8MtExyw`~_Bo`v?Onr*Q9lXuY{z2Z1@3`U{JQ}yp55$Qb~Swa&bqqso`0A~OH=7s z)F;0l+1;q82LL8VW+ol|xHHFuL)Epq^C|JOclLv_d6I;vT22-p3ra3J2_Y6=i|)5U zny{P(ZAT}1kzt7nYYG^>5iw*&uZvk$y9-m_4@9$ni_{ZCWCRbU;2~EGu+*}?BYY~o z9lKVq>+L*7tu|s6x!i|+9m@-kc$hZWXpIEvr63Uu`6N#_gwJe-2rYOv>n{e(QnK z)WNz-z?01OuXMx-PY33hV}|jEu-1&^q=5;`7(sF4IaUB2*tuHolf1 zF}aX_xmsWHhO#JkxTH)~YI0|^)XTl6+?g>~Abado0c&8ZpP4rR*m9^9T;Qi@j+%N2 zukpK&o3N$J%$mZMV>zSi$}BGi-ZdZofb0v>@E0>!tG_D>PaRCxLotH+Yv_s)YOHi6 zr*vk#+f3IibKQ2rE3MA>z%)~5E)ZUkVKkcaPvjKwE-B=iFS4QfWj?uOKW>BJR@SQe zAYbC66F>Vn7;6Cg$_@wEEzX>{l@4l(W<1`{j<(9n31c3i-5;{V#a3Ul&VnSF7SFOY zixJr;ZMN+`jKnEOl1}J0z8x1A{=5O9*6AAv+DInk-B!e{2jCSE81=YM?igE=p7tiq zG}%TbBS9fTpoa(HMO3pxK{NwOnm1*1RrnfPj?Eap@USp>|4!(2s8TD$Uc6SG71AUh z4YL{vaJAi^a=KaAFK#{*%oe8m0736WAZ5H=#o4O^SLzMqH2J8Tu6jK&(y0@(U-y4> z863TWx>cLJ^qBKzQp?}+rdP{#Y~*H~P~>r<-^UsWzw&!u1KY;1xG=tA?KfC6@uG}v zO+Ohv%56Jb->yhEzSVi&Lyn5HvmoMaBj72zPQff-s}mG)&}L;%1DwpUAM-t>?gHhH zmrIx;LoaQorc2DZyWN2%i~O4%k7lD^$_LnO7TMqXfsqV6j0>1sB9P`E$p+*jE_Km;FeLO8 zUXaT@th2Xg29}Li;tLw-Sz5-I;MqMsiCckY!ppcX|6yP74x3<%V{OCBXsYT=%W(0! z7}CyW4^3Ychu%U7ap+4<{ChK0$fWoa30 z9am*EnC>y91(r`H%nxj7!?`%+1!F$IYq}r&%*|~Tug)HiY<#v;hr~}feimT`UQQ-0 z)Bz<}Yq<>P?IEE;X_C7YKxIqZOSr+PcR;(Alie1cx1P_A95j`=l8;b#QH^Vs5;SpiFs_FE2z}h~t0HV951l@TP?$a5^7Xy+Gf-;KLO5QN|1#(?j~kQ~VFKj?DJ9;I zU$G+b2W?S{8~#ppasqRH_w1_Y_HH^erN>=oO0`8!9CJnkAWh1v!Ad-;S~%8+7qQ%T zGn5339NZTTm6S&my01R=p5`Ucve0G1ZirsXkZ`08S%~iE_jwR6^1^=Ni^_&lpJR8u zA|YSMf$J3plWUU@40inU3<9Js;o1MelUXpfGope+^?CFH?T+{cibtRyf z10#Le@|`Bg&;~iAh!b)gQ}qLViD0pdr_Y4aLQ|y1W_WR2nwz_}=}T&UPMhh-k-B$5 zR_xvoaO@jyRO-q=`YUo^<_<3}P?cq0ArzfMl{JNS2OKM@p<6#6;KY4+N~1kA*@Bq2>(xY9c|d%AELgb|c7Q@otg=PPEdf>Pjo! zjR#Ou49erEom_81hB{F)i91kHn;i{1Y0z5)97#k@HKeY=P;WM982jJvGKU7#vfEUU z8ew8YMyBBBK9+NY+e7BVi>GIh%P0yrmJjhl*$F;^0Fr#)I#(|=KwU(RC?MSdhzde% zx`rX6Xg`_)NP2O5+ZbSL0`W%fW%gMc5x@6O6duBanHaIE-a?UXu8?%qO$r%X$_bF|bqpone=9VDXXg|j#;dXJ~ zeuY7$-eN~2p2Lu?k{wmi-n>Kts`f6rE`rk`6e`K!lHw1FogaiU{Qp<<^k8D@?;0RF zJ+o$566_O>b!xP(>6(G|_LLsUYelMQ+?>xRu;odWVt0AX>0JH0S<8GECuf z2+E`+$ZBr?sJlDnUI|9$ZI|V)PCvs%1Q7u@zspQAf1BlqNFu-I`XDVNX7KfxUqiPR z2|+Zki{;Ty#L>>#BD4=c2%($O1uan*#lM25E=IF?yFa{Je^>UT_EMoJK#c~ucj|i4 zm!TKs7Vhwf&&3_>+&$VcUSk5MwYLnb)-Msfyh50$B-btZngGnx|Ftyc0>LPk3)D!o zQ`ur04Cf1Q5);}imI$U$3r^6%U_nk|5-RApKSC`-2igwsSbBuE<9ZVDzfA}TYDuSS zwbLxO)H1&xF$K+Q-vP3Zve&05*tLMHH(p=SoADN3@V0Ir z03MnU>w2jnhLE7Z44?uGt=pXbIg2?RJKN1Zw6Phl*`&!OAN37fx&<~GJ`eac44{b! zTtouN=mc^lEI;mVCJph}d)%dF^X!DTGP64~(4p6Zdy`%4om#uhiAbiCwQ;W3ld32P zvQ`1`)l^@ua&y|8TeIaO`M(j_CriAeZ#F3&&%kseU`c6wlL8Bchq#K(TWkCRmq|?g zy*~J6WRehMNf9P;Ch{lBKZvb$#(%%xgWnq-%}1s#kcH>GeSOe;>7N!momZtR{uHpa zt}j<++&G6GFkG*{SCunEKro~fs4hq!9#d@0sSrL=cvMRz2p2}8$$b5U;|JTssHF6T zHoB}lhQ7x|v^vn()A-o+fyO3nob#q>Y$F4qtrn>Xz21mDXP6*-Gt?r_flHy_9a7?* zMsJrDH>yNGXtpsSk<(fbwN1(iNW;vHg`cNaZu+%n1II;r5j8lpPC$^=Nz6C!44igt zhO%S!eZ(>~uu@c|eZ#~{LqtyCw6HU3-Gf;|tKe)3=8G=#&$Kp1iH?3#5CV`Cfrjfz zY9uLlUD8JrWMh6Y2G9_^+@Pv2k=R)+u#gcb+|j_s(7gB)?*Bd)t?RaGwKuI<(=Rop zpD2S|$XVB=E!Qy)jhJXu%@*Y{d-JUnE5erSOSd9=dElRVlTgQqx&_0PRx2*Zfhcjx z$F3G3_2dF`;OfZ6n{WzMfe^lzX!#K=3|98DZ~1gSM4mD%V%Q;LpwW_+H*BJ)R6gcw zF-mrdMAc0Em>8nG5oqP#jUyn~D{HsLHJFL$0QCljCN&Hc=WSpG)EuegJ@?rxSlH=G zC@bz3u1kggVg4kLMk%A3M#<_lbzH7YE2er6RycH*&{I5~lk@@sz~P$ED;AHsuO^qp za-iLj*31H#g{?K{#^=JH}%msCdl3m)+!0qn~MNS$%8NvB=hVI!X&x>d8jx zJ{-a37XG!6KTTktxTW!)RSja`o#z3GlD<~3Zaa7y#TUkPNxf2WdI{#^rwv)9hh3+J<$vw!Ij-E zWIH%L6koeh?=~BY^BZCW#JqvU3~v#l;Qcy zJ4?@${ZE_2^fwK|O)G0X^DdkD(EOgs{3VMUx+uvI+kr0&` z{(4RZO?Z7nikdOJXm@`bi<*&nl8%C&xEwc2&i8hzm`!2Y@R+7O=X|8ez|+m7p1D`2 znZm~{?{#DvW#c+6$pN5Vb;Zk{()J;lVZm(5!0c!G(v>f{eO-{6m zX!it?fsk(p{1N=^lqyl=($^+*1ckpaO$vmf9{AJ8=IJxHb}4GlsQWKU>T1v6V@YQ$ zhz<9pHTO#8XDyt9F&B+5Li7F&6(b?L7DS`kn^BEItDxYa6mtpoXT5hAYtg3F=y%(B z7UY6=)xN%|i#>AQ!=f7B^0t){L?EH><*mlQ!}X8dr3LNhj4$ zOcU398@g1V@NZSKxeUVw0#vF>K%E2eYV1aRobZg*#vAl#cfBW;16|sTUFBojrp!;o zXf;Kj^M+cbu$2l9+PB0|c8FDPW%s0UnLAGHEC@TIUAW&a$sUrWy^CmCbA`0SMk|1= z`);EClR_SPk+Bf8SzSa!uVB-x+IU7P;I7OTdPqy*OerUTOBAhLa%@pcN~%3%QXH9e z>&@e^;Jhzt!%v!~9F)8bS}zdqJ+FaPX6bQHcv35X(JN3rEFw*0cOA@-d1p1UY_2k&DOLlE zi?(Vcubtan$W~&H2^~J$Wl4^`>tsJhY@(yMpU}h@%N@z;Ns)W#YoSm<_Qy}nrpIPo zGZ6E(U8L!JCr}|W5B7c|dB{5t|7GZS>5lLbpTgPQsyg$iTB-16Sdi7P7#^yS?t>SD zL@roUhDrh3s(h?6Z0#Mm^kq_9U`kq=DSk1qIP&Z(+nPOSGxFYrk$g zu=cbL%vQfDk$v?&iK3ZBcKgAqlS!&eyuKTayn+mjucr>+mr5KJVhesy`={OAJ$ zpgguIk%dnCsYXf?ah#}BD{@8^-*i2!1g$;i+F;RF?((!oakeINvoZpNnc&)E_+#D> zt69@14LE5j+I=8+#0Jj6D|!Dj=3q@3U7j?`nD^}gW9$S6Q_x8EW9rh~Ijuc^CWK>W zV+)1RT^CEvb#c@KD83lz7KD#E53*Su%Ehiu_I3pZMq5pkg?=C%1>dh7OV(bS9Axw@ zH;u@BBTb9E&y&#p5z#VE?q1*=+P-?z$ode9y@!}a7sVK=1mVl_ctJRa!}H8S-nc;V zxN2I?_p}|W-lhPLBYT`3mQn|Y6^3sVk{%hd`-gAwOy;Y6VD+qZuQ8z7aHTl?u4Q!F zC2`FLxJOV$E=i1D@&^fEaeAXAjLBukzsI6AsFq%#zGS$uTceTQq+!PxZw2Q&?7MB` zO00Q5Xpu+w821(5gj8a=8!b~(s4`LAS*6FZSAoUdawnl=-PhR#Q`B=wj7JGLX<5-=mS;FoxO?oy_t04k!3YasJwur_(swUQqZTZRr1M7na{DU>}{bliH`N07+r-*3WS<|rVQoNu!{CR9jUEn2)_-8fjI z<272o{!Z!}LPlg_bop*C4FblLI<7L(Tqxp{H+ERh#M@>*!=LoKq2I%&+M$|A^JJ4V z^dV#mEfGO>BGiJ$Q1~*&)xd0zL77xr!VQZku&Va%@?%@{${%q1@m-Ev zc;mH+-R^9m`48BZ!qIO1Cg)en%W*j#sf=D_nbt3gCVq*8-i2O|4rX7dQcXiNI)xlq zH>!PG$~ohBZ;#Ro()MoykQ>r(srqADn!{fZ2a`H0B}cmBWWt4IL-g6=$nBg<^Dlc;rMK&N7o6P4Po+QrOu;Y=GIUHWBDf0(wPQ zYR2mEeGnixa6VpNd-j4OUL-+nmRt^j+Hrm|TNXU%Kn?{aC>-nq1Dr?_WvFd23I@GmYO9s!AbKV3l0<7a;0S>ZY_Cw#WtPec*+;cbO$^)6HubT$7X>Vq5kpCCvYK>P{#z)OJHKue{mq=!;N% zaO)A4)f`jv6ue$JBji;s02h{?bV*FZj}d9cs0gRO+l{*jtzTMHfGK)wOJVd3Q*#-8 z$^gIW0GV$>CJm(h3T^yqV;VmLF#WLw%^vqsPM2`@qO%zS8IJSv%ei0c4vSeg+K>1u zuEqnCepyIPRd4R=AD(#)djI8P)g29k{Y)^B_lq5r^Bgz=~%Mt*(_3NYE zaX^2(Hre2Q$#dMEq5THqA{U4;!4IUV19jOoZ#4LFx`Utoi*q3x^iZ7&^ap3NK=UCY z;Bx$z!JmQj1OfPEycpJbsSk+~gj6Fal}|3~wcppk=0nOm04|!Dt6w5V?3WvJoWfhL zRT@K%nZTAAIxo-XPRVimDep-cK2R>hsFn&}aPx0}pNXJ_yd{B4>+cl6L;s>T@$7nd zOv7G7nL^lGz9Y@JY{9R?=iQ&RTEMe+xckFk=Lgr$kKF7U?@_b|Y5FndnrF|U&ojBA zbpx~ZAAFN2Xnr9mx-zYu;0A}~_d2$Wa?Gesok*H3?fj7A=5&|5`sK*T?e`*N8INSr z@RV8We3LfTbFEkN1syn^i~dpt%l(Tl1H`ariGD}kRN{&?5B=hKq5xd6P$CI}m18&D$L>&W@tPGZBUOb-gtS4- z=Y?n?|96J*$Sxuzf9-Fi@t<(TYw!^##hAuW&R}1f(p~G(v=f27&F0F#X$=UnCnD%K~Wx?Isg&LiLErci=r8!6n< z)g<2^5<*!rpbGUhNIS;5cjlPbqxQ39mr|}z&|muq`($+Z_~%&yw?%>DH{p|MD&MoL zp6Lw5{SbiO^y#y5qCaO?zT-i#f_3vB%0X;zaYVu5`GS|Ono%krE%9Edw_)|P0_*|N zd%=AZs8VBgCqbi7<<;YZaoHz;zA51$flC=;PL(9Ju9Uu`E0jOkw&Qp#YQ0ExzFEo> zCoL~Y(Q}{2lJog)Avfc>u1|gvv^SVuAjMdyT*B|qsk56KS~dv3s#0r7cE{zsJnrw~ zbZ>YPf1a#D3@A2hQvRAHjwkHfMXHDU*1ByEg+oOwe^HhyPZX)WOsZVghusc4&*O2f zN@mQ3h^#N2iEGJzyunM?%mnQ5ZU4_>QRaO&-Q9cvrLncbIhpx6^>fB7pwf_D+TNV! z!JIm^6sEmd+Apk12O)n(-XC#JZAA{Y(8|n*x?r7&(}LM?<2Zp~25VY~l#=`$7P6~@ zv6u7IFMDb=7|P#(jA;V7P#|(I2!XPzW7gpHi*l@(zE$Bh!C}YW zvgBuhgT;F*$vr66nxtX^)7qwA>9_O;LDr{ltoh?|z>0Im z@zs_yen0eKrXOJR=VDGd3F)vjL-0=RfXp3xJ^X$f1~&jh7bNzP->+B*o71|O7xtZm z5c}J8?xLBSMYsP1YvoAQ*n8ch`AW~`x{Vk6n^9U&-Vxf2vu4q@k6VT>mi(E;yfMY) z8O;IDvTh@4og&_!m2IA$C#(Q&&EtZ-kscLZUnPWJhpHsSLIU|Kwv47;>HA$n%t>EM z>lSY&cZqS%LBs1@Y{uEg7bmH~Dru%CQ!+;wjwwpAEcIFR@82>q-e>*FSNdL@QT!o+ zaza4ur^FXeZkDr1!lBA-INuz~FFTvFb9lDRPxI|$+u3-~4^Ek1V6u_ES;XyP%Kmb} zYd}x-X8lI8rDzbYBa*{ff=duqa@E0(hCr^?aP! z2)VLHMf@?D#&Z9e0{Z5)d|)4m{Me3(5dKyBz@`(5Gdr83%Q7cudP=czvisht;LDpQdY_<(3U7C_?TH z#Jc+hc6!OB^sS4A+#)UCNx#gge~;ro`)tarFk3~uQEqZpsoNR!AtHow_?J`0zuK@_ z$p@?s*PlVDE5Ph_8^&k$USmE$%Iwd5vj8@;fV-69M0}P>(fkAchN^jmiET|D0O{M`ug_g0`8j>CP9Y$Tdw9;DS1@|vr5htOb1HuX6m$5$ z*^&M~0&YpX0bGH(j{$WZxM>HRwD$iAd-u1nb4tI@Cg!vRd&CZF;En_M5m>lE>VSR& zamhO<-_qYNz{hbUqEST`O#fuxrH?o1r7Hfs7~&ndJWY|<3!W0FDrzc}$vu1ZKi)9* Ak^lez literal 0 HcmV?d00001 From ccec5b5ebab4e16a7322dd54eacca3b10a9be0e3 Mon Sep 17 00:00:00 2001 From: Andi Li Date: Fri, 31 Jul 2020 16:44:25 +0000 Subject: [PATCH 09/20] Fix typo --- .../177-volume-snapshot/tighten-validation-webhook-crd.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md b/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md index 41a314bd7ac..3567f38b99d 100644 --- a/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md +++ b/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md @@ -228,7 +228,7 @@ CRD validation is preferred over webhook validation due to their lower complexit ## Proposal -Tighten the validation on Volume Snapshot objects. The following fields will begin to enforce immutability: `VolumeSnapshot.Spec.Source`, `VolumeSnapshotContent.spec.VolumeSnapshotRef`*, and `VolumeSnapshotContent.Spec.Source`. The following fields will begin to enforce `oneOf`: `VolumeSnapshot.Spec.Source` and `VolumeSnapshotContent.Spec.Source`). The following fields will begin to enforce non-empty strings: `spec.VolumeSnapshotClassName`. More details are in the Validating Scenarios section. +Tighten the validation on Volume Snapshot objects. The following fields will begin to enforce immutability: `VolumeSnapshot.Spec.Source`, `VolumeSnapshotContent.spec.VolumeSnapshotRef`*, and `VolumeSnapshotContent.Spec.Source`. The following fields will begin to enforce `oneOf`: `VolumeSnapshot.Spec.Source` and `VolumeSnapshotContent.Spec.Source`). The following fields will begin to enforce non-empty strings: `VolumeSnapshot.Spec.VolumeSnapshotClassName`. More details are in the Validating Scenarios section. *Immutable only after the UID has been set. From d2eb66e3dfad7ada06de656c61715486041ffd81 Mon Sep 17 00:00:00 2001 From: Andi Li Date: Thu, 6 Aug 2020 18:56:12 +0000 Subject: [PATCH 10/20] Clarify users will be responsible for fixing invalid objects. Add info on deploying TLS. --- .../tighten-validation-webhook-crd.md | 40 ++++++++++++------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md b/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md index 3567f38b99d..7eea263d0f0 100644 --- a/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md +++ b/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md @@ -228,14 +228,12 @@ CRD validation is preferred over webhook validation due to their lower complexit ## Proposal -Tighten the validation on Volume Snapshot objects. The following fields will begin to enforce immutability: `VolumeSnapshot.Spec.Source`, `VolumeSnapshotContent.spec.VolumeSnapshotRef`*, and `VolumeSnapshotContent.Spec.Source`. The following fields will begin to enforce `oneOf`: `VolumeSnapshot.Spec.Source` and `VolumeSnapshotContent.Spec.Source`). The following fields will begin to enforce non-empty strings: `VolumeSnapshot.Spec.VolumeSnapshotClassName`. More details are in the Validating Scenarios section. - -*Immutable only after the UID has been set. +Tighten the validation on Volume Snapshot objects. The following fields will begin to enforce immutability: `VolumeSnapshot.Spec.Source`, and `VolumeSnapshotContent.Spec.Source`. The following fields will begin to enforce `oneOf`: `VolumeSnapshot.Spec.Source` and `VolumeSnapshotContent.Spec.Source`). The following fields will begin to enforce non-empty strings: `VolumeSnapshot.Spec.VolumeSnapshotClassName`. `VolumeSnapshotContent.spec.VolumeSnapshotRef`will be Immutable only after the UID has been set. More details are in the Validating Scenarios section. Due to backwards compatibility concerns, the tightening will occur in two phases. -1. The first phase is webhook-only, and will use [ratcheting validation](#backwards-compatibility) combined with a reconciliation method to delete or fix currently persisted objects which are invalid under the new (strict) validation rules. -2. The second phase occurs once all invalid objects are cleared *from* the cluster. The CRD schema validation will be tightened and the webhook will stick around to enforce immutability until immutable fields come to CRDs. (or the crd upgrade could wait until immutable fields are available to do in one go) +1. The first phase is webhook-only, and will use [ratcheting validation](#backwards-compatibility). It will be the user's responsibility to clean up invalid objects. The controller will not be able to automatically fix invalid objects. The controller will not automatically delete invalid objects to avoid data loss. +2. The second phase occurs once all invalid objects are cleared from the cluster. The CRD schema validation will be tightened and the webhook will stick around to enforce immutability until immutable fields come to CRDs. (or the crd upgrade could wait until immutable fields are available to do in one go) This will be accompanied by a version change to make it clear the CRD is using different validation. The phases come in separate releases to allow users / cluster admin the opportunity to clean their cluster of any invalid objects. More details are in the Risks and Mitigations section. @@ -352,11 +350,11 @@ Begin with validating webhook only enforcement. The webhook will perform the fol - webhook is strict on create - webhook is strict on updates where the existing object passes strict validation - webhook is relaxed on updates where the existing object fails strict validation (allows finalizer removal, status update, deletion, etc) - - some reconciliation process in volume snapshot controller (this is the hard part) that ensures invalid data is fixed or removed + - some reconciliation process in volume snapshot controller that ensures invalid data is fixed or removed. This process will be entirely user managed. The webhook and controllers will not take any automatic action to reconcile invalid objects. Once we are sure no invalid data is persisted, we can switch to CRD schema-enforced validation with validating webhooks for immutability in a subsequent release. -We are unsure of the specifics of the reconciliation process in which the controller will remove or fix invalid objects. +If users do not completely remove their invalid objects before upgrading their CRD definition, it should be possible to downgrade the CRD definition to allow invalid objects to get deleted. #### Current Controller validation of OneOf semantic @@ -386,13 +384,15 @@ proposal will be implemented, this is the place to discuss them. ### Deployment -There are two main steps to setup validation for the snapshot objects. The kubernetes API server must be configured to connect to the webhook server, and the webhook server must be deployed and reachable. Make sure to take a look at the prerequisites before deploying. +There are two main steps to setup validation for the snapshot objects. The kubernetes API server must be configured to connect to the webhook server, and the webhook server must be deployed and reachable. Make sure to take a look at the [prerequisites](#background-on-admission-webhooks) before deploying. + +A sample script will be provided which will handle the deployment of TLS certificates. It is not considered production ready and users are encouraged to use their own certificate management process. The demo will create certificates as a secret in the cluster and mount them as a volume. The `ValidatingWebhookConfiguration` will need to be updated with the CA bundle. ### Kubernetes API Server Configuration The API server must be configured to connect to the webhook server for certain API requests. This is done by creating a ValidatingWebhookConfiguration object. For a more thorough explanation of each field refer to the documentation. An example yaml file is provided below. The value of timeoutSeconds will affect the latency of snapshot creation, and must be considered carefully as it will affect the time the application may be frozen for. -``` +```yaml apiVersion: admissionregistration.k8s.io/v1 kind: ValidatingWebhookConfiguration metadata: @@ -403,13 +403,14 @@ webhooks: - apiGroups: ["snapshot.storage.k8s.io"] apiVersions: ["v1beta1"] operations: ["CREATE", "UPDATE"] - resources: ["VolumeSnapshot", "VolumeSnapshotContent"] + resources: ["volumesnapshots", "volumesnapshotcontents"] scope: "*" clientConfig: service: - namespace: "kube-system" + namespace: "default" name: "snapshot-validation-service" - caBundle: "Ci0tLS0tQk...<`caBundle` is a PEM encoded CA bundle which will be used to validate the webhook's server certificate.>...tLS0K" + path: "/path/to/webhook" + caBundle: "LS0tLS...base64 encoded of public key...LS0K" admissionReviewVersions: ["v1", "v1beta1"] sideEffects: None failurePolicy: Ignore # We recommend switching to Fail only after successful installation of the server and webhook. @@ -419,7 +420,7 @@ webhooks: Webhook Server Deployment The recommended deployment mode for the webhook server is within the cluster to minimize network latency. For high-availability we recommend using a Deployment and Service to deploy the validation server. Some example yaml files are provided, and should be changed to suit the Cluster Admin’s needs. -``` +```yaml apiVersion: apps/v1 kind: Deployment metadata: @@ -438,12 +439,21 @@ spec: spec: containers: - name: snapshot-validation - image: snapshot-validation:xxx # change the image if you wish to use your own custom validation server image + image: image:xxx # change the image to released image or if you wish to use your own custom validation server image + args: ['webhook', '--tls-cert-file=/etc/webhook/certs/cert.pem', '--tls-private-key-file=/etc/webhook/certs/key.pem'] # Change args as needed ports: - containerPort: 443 # change the port as needed + volumeMounts: + - name: webhook-certs + mountPath: /etc/webhook/certs + readOnly: true + volumes: + - name: webhook-certs + secret: + secretName: snapshot-validation-secret ``` -``` +```yaml apiVersion: v1 kind: Service metadata: From 40f84aa4326bf6c25a470d377a2bdb244eb4ec55 Mon Sep 17 00:00:00 2001 From: Andi Li Date: Tue, 11 Aug 2020 18:19:02 +0000 Subject: [PATCH 11/20] Address some comments 1. provisional -> implementable 2. Remove reference to waiting for immutable fields and make clear there is no set timeline for the feature to be available. 3. Expand abbreviations 4. Indent lists --- .../tighten-validation-webhook-crd.md | 48 ++++++++++--------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md b/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md index 7eea263d0f0..ef4c46da87a 100644 --- a/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md +++ b/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md @@ -19,7 +19,7 @@ approvers: editor: TBD creation-date: 2020-07-22 last-updated: 2020-07-22 -status: provisional +status: implementable see-also: - n/a replaces: @@ -182,11 +182,11 @@ Items marked with (R) are required *prior to targeting to a milestone / release* ## Summary -Tighten validation on `VolumeSnapshot` and `VolumeSnapshotContent` by updating the CRD validation schema and providing a webhook server to enforce immutability (until builtin immutable fields for CRD is available). +Tighten validation on `VolumeSnapshot` and `VolumeSnapshotContent` by updating the CRD validation schema and providing a webhook server to enforce immutability. -This KEP will list the new validation rules. It will also provide the release plan to ensure backwards compatibility. As well, it will outline the deployment plan of the webhook server. +This KEP will list the new validation rules. It will also provide the release plan to ensure backwards compatibility. As well, it will outline the deployment plan of the webhook server. The webhook server is deployed separately from the snapshot controll -This tightening of the validation on volume snapshot objects is considered a change to the volume snapshot API. Choosing not to install the webhook server and participate in the 2-phase release process can cause future problems when upgrading from v1beta1 to V1 volumesnapshot API if there are currently persisted objects which fail the new stricter validation. Potential impacts include being unable to delete invalid snapshot objects. +This tightening of the validation on volume snapshot objects is considered a change to the volume snapshot API. Choosing not to install the webhook server and participate in the 2-phase release process can cause future problems when upgrading from v1beta1 to V1 volumesnapshot API if there are currently persisted objects which fail the new stricter validation. Potential impacts include being unable to delete invalid snapshot objects. It should be possible to downgrade the CRD definition as a workaround. ## Motivation @@ -207,7 +207,7 @@ A webhook server receives AdmissionReview(definition) requests from API server, ![Webhook workflow diagram](./webhook-workflow.png) -The webhook server will expose an HTTP endpoint such that to allow the API server to send AdmissionReview requests. Webhook server providers can dynamically(details) configure what type of resources and what type of admission webhooks via creating CRs of type ValidatingWebhookConfiguration and/or MutatingWebhookConfiguration. It’s worth thinking to provide a generic webhook server in csi-repo. At this stage, the proposal is to create a repo under CSI org, however only implement the validating webhooks for VolumeSnapshot. +The webhook server will expose an HTTP endpoint such that to allow the API server to send AdmissionReview requests. Webhook server providers can dynamically(details) configure what type of resources and what type of admission webhooks via creating CRs of type ValidatingWebhookConfiguration and/or MutatingWebhookConfiguration. CRD validation is preferred over webhook validation due to their lower complexity, however CRD validation schema is unable to enforce immutability or provide ratcheting validation. @@ -233,7 +233,7 @@ Tighten the validation on Volume Snapshot objects. The following fields will beg Due to backwards compatibility concerns, the tightening will occur in two phases. 1. The first phase is webhook-only, and will use [ratcheting validation](#backwards-compatibility). It will be the user's responsibility to clean up invalid objects. The controller will not be able to automatically fix invalid objects. The controller will not automatically delete invalid objects to avoid data loss. -2. The second phase occurs once all invalid objects are cleared from the cluster. The CRD schema validation will be tightened and the webhook will stick around to enforce immutability until immutable fields come to CRDs. (or the crd upgrade could wait until immutable fields are available to do in one go) This will be accompanied by a version change to make it clear the CRD is using different validation. +2. The second phase occurs once all invalid objects are cleared from the cluster. The CRD schema validation will be tightened and the webhook will stick around to enforce immutability until immutable fields come to CRDs. This will be accompanied by a version change to make it clear the CRD is using different validation. The phases come in separate releases to allow users / cluster admin the opportunity to clean their cluster of any invalid objects. More details are in the Risks and Mitigations section. @@ -243,7 +243,9 @@ The server will perform validation on Volume Snapshot objects when CREATE and UP The following is a list of fields which will get checked when a CREATE or UPDATE operation is sent to the API server. Some validation is already enforced by the CRD schema definition, for example some required fields and enums. -All of the validation desired can be achieved by updating the CRDs to take advantage of the OpenApi v3 schema validation. In particular, the `oneOf` and `minLength` fields can be used. However, there is a desire for some fields to be immutable, which is not yet supported by CRDs. See KEP https://github.com/kubernetes/enhancements/blob/master/keps/sig-api-machinery/20190603-immutable-fields.md for the timeline for immutable fields to come to CRDs. +All of the validation desired can be achieved by updating the CRDs to take advantage of the OpenApi v3 schema validation. In particular, the `oneOf` and `minLength` fields can be used. + +There is a desire for some fields to be immutable, which is not yet supported by CRDs. See the immutable fields [KEP](https://github.com/kubernetes/enhancements/blob/master/keps/sig-api-machinery/20190603-immutable-fields.md) for the latest updates. As of August 2020, the KEP is provisional and has no clear timeline for when immutable fields will come to CRDs. Note that we may want to consider other fields to be marked as immutable. @@ -257,12 +259,12 @@ Note that we may want to consider other fields to be marked as immutable. | UPDATE | spec.VolumeSnapshotClassName | Same restrictions as CREATE. We won’t restrict updating by making this field immutable (only applying the same restrictions as creation) but this field should only be changed by those who know exactly what they are doing. | 400 | #### VolumeSnapshotContent -| Operation | Field | Reason | HTTP RCode | -| :-------: | :--------------------: | :---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------: | :--------: | -| CREATE | spec.Source | Exactly one of VolumeHandle (dynamic snapshot created by controller) or SnapshotHandle (pre-provisioned snapshot created by CA) should be specified | 400 | -| UPDATE | spec.Source | Immutable, no updates allowed. | 400 | -| CREATE | spec.VolumeSnapshotRef | Must have both name and namespace fields set. Preprovisioned: This is the reference to the yet to be created VolumeSnapshot object which should bind to this VolumeSnapshotContent. https://github.com/kubernetes-csi/external-snapshotter/blob/097b1fc7d7cd6576182ca34512c14de1c84b2127/pkg/apis/volumesnapshot/v1beta1/types.go#L270. Dynamic: This is the reference to the VS object which triggered the creation of this VSContent. It also has the UID field, but this is set by the controller. | 400 | -| UPDATE | spec.VolumeSnapshotRef | Immutable, no updates allowed, once it's UID has been set. | 400 | +| Operation | Field | Reason | HTTP RCode | +| :-------: | :--------------------: | :---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------: | :--------: | +| CREATE | spec.Source | Exactly one of VolumeHandle (dynamic snapshot created by controller) or SnapshotHandle (pre-provisioned snapshot created by cluster admin) should be specified | 400 | +| UPDATE | spec.Source | Immutable, no updates allowed. | 400 | +| CREATE | spec.VolumeSnapshotRef | Must have both name and namespace fields set. Preprovisioned: This is the reference to the yet to be created VolumeSnapshot object which should bind to this VolumeSnapshotContent. https://github.com/kubernetes-csi/external-snapshotter/blob/097b1fc7d7cd6576182ca34512c14de1c84b2127/pkg/apis/volumesnapshot/v1beta1/types.go#L270. Dynamic: This is the reference to the VolumeSnapshot object which triggered the creation of this VolumeSnapshotContent. It also has the UID field, but this is set by the controller. | 400 | +| UPDATE | spec.VolumeSnapshotRef | Immutable, no updates allowed, once it's UID has been set. | 400 | ### Authentication @@ -294,17 +296,17 @@ bogged down. --> #### Story 1 -CA can deploy the webhook server. +Cluster admin can deploy the webhook server. Users can create and update snapshot objects with confidence invalid updates will be rejected. Following are some typical scenarios we are aiming to prevent: - Creation of invalid CRs -- Reject if a VolumeSnapshot CR does not have a legit VolumeSnapshotSource, i.e., missing both PersistentVolumeName and VolumeSnapshotContentName. -- Reject if a VolumeSnapshotContent CR does not have a legit VolumeSnapshotContentSource, i.e., both VolumeHandle and SnapshotHandle have been specified + - Reject if a VolumeSnapshot CR does not have a legit VolumeSnapshotSource, i.e., missing both PersistentVolumeName and VolumeSnapshotContentName. + - Reject if a VolumeSnapshotContent CR does not have a legit VolumeSnapshotContentSource, i.e., both VolumeHandle and SnapshotHandle have been specified - Updating immutable fields -- Reject updates to VolumeSnapshot’s VolumeSnapshotSource -- Reject updates to VolumeSnapshotContent’s VolumeSnapshotContentSource -- Reject updates to VolumeSnapshotContent’s volume snapshot ref after binding + - Reject updates to VolumeSnapshot’s VolumeSnapshotSource + - Reject updates to VolumeSnapshotContent’s VolumeSnapshotContentSource + - Reject updates to VolumeSnapshotContent’s volume snapshot ref after binding ### Notes/Constraints/Caveats (Optional) @@ -362,11 +364,11 @@ If users do not completely remove their invalid objects before upgrading their C See code [here](https://github.com/kubernetes-csi/external-snapshotter/blob/v2.1.1/pkg/common-controller/snapshot_controller.go#L192). -If the object violates oneOf semantic: Update the VS status to “SnapshotValidationError” and issue an event. +If the object violates oneOf semantic: Update the VolumeSnapshot status to “SnapshotValidationError” and issue an event. Note: -- If the VS object has been updated AFTER binding to a VSC, binding from VS->VSC will be lost. -- Deletion of an invalid resource is not blocked by that check as the deletion workflow happens before validation(code). This is to ensure that a user can delete an invalid VS resource. +- If the VolumeSnapshot object has been updated AFTER binding to a VSC, binding from VolumeSnapshot->VSC will be lost. +- Deletion of an invalid resource is not blocked by that check as the deletion workflow happens before validation(code). This is to ensure that a user can delete an invalid VolumeSnapshot resource. ##### Handling VolumeSnapshotContent @@ -386,7 +388,7 @@ proposal will be implemented, this is the place to discuss them. There are two main steps to setup validation for the snapshot objects. The kubernetes API server must be configured to connect to the webhook server, and the webhook server must be deployed and reachable. Make sure to take a look at the [prerequisites](#background-on-admission-webhooks) before deploying. -A sample script will be provided which will handle the deployment of TLS certificates. It is not considered production ready and users are encouraged to use their own certificate management process. The demo will create certificates as a secret in the cluster and mount them as a volume. The `ValidatingWebhookConfiguration` will need to be updated with the CA bundle. +A sample script will be provided which will handle the deployment of TLS certificates. It is not considered production ready and users are encouraged to use their own certificate management process. The demo will create certificates as a secret in the cluster and mount them as a volume. The `ValidatingWebhookConfiguration` will need to be updated with the cluster admin bundle. ### Kubernetes API Server Configuration From b62957f10bb144ba8c9e0e6e2a230f6f7891164d Mon Sep 17 00:00:00 2001 From: Andi Li Date: Wed, 12 Aug 2020 15:21:29 -0400 Subject: [PATCH 12/20] More comments --- .../tighten-validation-webhook-crd.md | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md b/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md index ef4c46da87a..32c30882e6c 100644 --- a/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md +++ b/keps/sig-storage/177-volume-snapshot/tighten-validation-webhook-crd.md @@ -228,16 +228,18 @@ CRD validation is preferred over webhook validation due to their lower complexit ## Proposal -Tighten the validation on Volume Snapshot objects. The following fields will begin to enforce immutability: `VolumeSnapshot.Spec.Source`, and `VolumeSnapshotContent.Spec.Source`. The following fields will begin to enforce `oneOf`: `VolumeSnapshot.Spec.Source` and `VolumeSnapshotContent.Spec.Source`). The following fields will begin to enforce non-empty strings: `VolumeSnapshot.Spec.VolumeSnapshotClassName`. `VolumeSnapshotContent.spec.VolumeSnapshotRef`will be Immutable only after the UID has been set. More details are in the Validating Scenarios section. +Tighten the validation on Volume Snapshot objects. Please see the tables below for detailed information. Due to backwards compatibility concerns, the tightening will occur in two phases. -1. The first phase is webhook-only, and will use [ratcheting validation](#backwards-compatibility). It will be the user's responsibility to clean up invalid objects. The controller will not be able to automatically fix invalid objects. The controller will not automatically delete invalid objects to avoid data loss. -2. The second phase occurs once all invalid objects are cleared from the cluster. The CRD schema validation will be tightened and the webhook will stick around to enforce immutability until immutable fields come to CRDs. This will be accompanied by a version change to make it clear the CRD is using different validation. +1. The first phase is webhook-only, and will use [ratcheting validation](#backwards-compatibility). It will be the user's responsibility to clean up invalid objects which already existed before the webhook was enabled. Invalid objects are those which fail the new, stricter validation. The controller will not be able to automatically fix invalid objects. +2. The second phase can occur once all invalid objects are cleared from the cluster. It will be the cluster admin's responsibility to check and detect when it is safe to move to the second phase. The CRD schema validation will be tightened and the webhook will stick around to enforce immutability until immutable fields come to CRDs. This will be accompanied by a version change to make it clear the CRD is using different validation. The phases come in separate releases to allow users / cluster admin the opportunity to clean their cluster of any invalid objects. More details are in the Risks and Mitigations section. -The server will perform validation on Volume Snapshot objects when CREATE and UPDATE requests are made to the api server for `VolumeSnapshot` and `VolumeSnapshotContent` objects. The webhooks will only use validating webhooks, which are read-only. An image will be built and example `Deployment` and `Service` yaml files will be provided. Example configuration files for the `ValidatingWebhookConfiguration` will be provided, to be used to register the webhooks on the API server. +The webhook server will perform validation on Volume Snapshot objects when CREATE and UPDATE requests are made to the api server for `VolumeSnapshot` and `VolumeSnapshotContent` objects. The webhooks will only use validating webhooks, which are read-only. An image will be built and example `Deployment` and `Service` yaml files will be provided. Example configuration files for the `ValidatingWebhookConfiguration` will be provided, to be used to register the webhooks on the API server. + +The webhook will be developed inside the [external-snapshotter](https://github.com/kubernetes-csi/external-snapshotter) repository. ### Validating Scenarios @@ -268,7 +270,7 @@ Note that we may want to consider other fields to be marked as immutable. ### Authentication -There are two directions to authentication. Authenticating the webhook server is legitimate, and authenticating the k8s api server is legitimate. +There are two directions to authentication. Authenticating the identity of the webhook server, and authenticating the identitiy of the kubernetes api server. The API server authenticates the webhook server through TLS certificates and HTTPS. This is required, and an example method of deploying the webhook server with HTTPS will be provided. @@ -278,9 +280,9 @@ Authentication on incoming requests to the webhook server is configurable howeve Webhooks add latency to each API server call, thus setting up a reasonable timeout for each AdmissionReview request from the webhook server side is critical. The default timeout is 10 seconds if not specified. When an AdmissionReview request sent to the webhook server timed out, `failurePolicy`(default to `Fail` which is equivalent to disallow) will be triggered. -In the ValidatingWebhookConfiguration yaml example, a default timeout of two seconds is provided, cluster admins who wish to change the timeout may change the value of `timeoutSeconds` +In the ValidatingWebhookConfiguration yaml example, a default timeout of two seconds is provided, cluster admins who wish to change the timeout may change the value of `timeoutSeconds`. -To avoid migration pain it is recommended to start with a `failurePolicy` value of `Ignore`, changing it to `Fail` only after the webhook is confirmed to have been installed successfully. +To avoid migration pain it is recommended to start with a `failurePolicy` value of `Ignore`, changing it to `Fail` only after the webhook is confirmed to have been installed successfully. Choosing `Ignore` means that it would be possible invalid objects can get created/updated in the system. ### Idempotency/Deadlock @@ -352,7 +354,7 @@ Begin with validating webhook only enforcement. The webhook will perform the fol - webhook is strict on create - webhook is strict on updates where the existing object passes strict validation - webhook is relaxed on updates where the existing object fails strict validation (allows finalizer removal, status update, deletion, etc) - - some reconciliation process in volume snapshot controller that ensures invalid data is fixed or removed. This process will be entirely user managed. The webhook and controllers will not take any automatic action to reconcile invalid objects. + - The user will need to delete or fix all invalid objects. The webhook and controllers will not take any automatic action to reconcile invalid objects. Once we are sure no invalid data is persisted, we can switch to CRD schema-enforced validation with validating webhooks for immutability in a subsequent release. @@ -471,9 +473,9 @@ spec: ``` ### Test Plan -There will be unit testing on the webserver to ensure that the correct policy gets enforced. +There will be unit testing on the webserver in the same repository to ensure that the correct policy gets enforced. -There is a cluster available for testing in the external snapshotter repository. We will deploy the webhook validation server and test the e2e functionality. +Since the webhook is developed in the external-snapshotter repository, and does not test any csi driver, it would not be a good fit for e2e tests to go under the kubernetes core repository. Hence the plan for e2e tests is to add a new test job in external-snapshotter repo that brings up a kind cluster, installs crds and the webhook, and then runs validation tests.