Skip to content

[KEP-5073] Update KEP with section of validation ratcheting #5292

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[KEP-5073] Update KEP with section of validation ratcheting #5292

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 46 additions & 3 deletions keps/sig-api-machinery/5073-declarative-validation-with-validation-gen/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@
- [Scale-Type Subresources](#scale-type-subresources)
- [Streaming Subresources](#streaming-subresources)
- [Ratcheting](#ratcheting)
- [Core Principles](#core-principles)
- [Default Ratcheting Behavior](#default-ratcheting-behavior)
- [Definition of Semantic Equivalence](#definition-of-semantic-equivalence)
- [Ratcheting and Cross-Field Validation](#ratcheting-and-cross-field-validation)
- [Test Plan](#test-plan)
- [Prerequisite testing updates](#prerequisite-testing-updates)
- [Unit tests](#unit-tests)
Expand Down Expand Up @@ -1024,10 +1028,49 @@ The streamed data does not require declarative validation, as it is not structur

### Ratcheting

TODO: Document and explain how:
As Kubernetes APIs evolve, validation rules change. To minimize disruption for users with existing objects created under older rules, declarative validation will incorporate **Validation Ratcheting**. This mechanism aims to selectively bypass new or changed validation rules during object updates (`UPDATE`, `PATCH`, `APPLY`) for fields that have not been modified from their previously persisted state.

- Add general purpose ratcheting to automatically skip validation of unchanged fields
- Catalog and handle complex cases where strict equality checks are not sufficient (lots of non-trivial cases exist today)
#### Core Principles

The design adheres to the following core principles:

1. **Stored data is considered valid:** Any object successfully persisted was once considered valid. Subsequent apiservers must not retroactively invalidate stored objects. (Implication: fixing validation bugs post-release is challenging).
2. **Unchanged fields do not cause update rejections:** An `UPDATE` operation must not fail validation due to fields that were not modified in that operation. (Rationale: HTTP 4xx errors imply a client request problem).
3. **Semantic deep-equal is always sufficient to elide re-validation:** Kubernetes API objects adhere to canonical semantic equivalence rules (`equality.Semantic.DeepEqual`). If a deserialized object satisfies that equivalence with its prior state, re-validation can be bypassed.
* **Subtle:** List elements might individually bypass re-validation, but the list itself might still be re-validated (e.g., if reordered).

Ratcheting is the **default behavior** during `UPDATE` operations.

#### Default Ratcheting Behavior

The default mechanism handles common cases by skipping re-validation if a field hasn't changed based on semantic equivalence.

##### Definition of Semantic Equivalence

"Semantic equivalence" builds on `k8s.io/apimachinery/pkg/api/equality.Semantic.DeepEqual` (similar to `reflect.DeepEqual` but `nil` and empty slices/maps are equivalent). The table below outlines the behavior:

| Value type | Semantic Equivalence | Ratcheting | CRD Comparison (KEP-4008) |
| :---------------------------------- | :------------------------------------------------------------- | :----------------------------------------------------------------------------- | :------------------------------------------------------------------------------------------------------------------ |
| Scalars (int, string, etc.) | direct equality of the value | revalidate the value if changed | same |
| Pointers | equivalence of the pointee | revalidate the value if changed | same |
| Struct | all fields are equivalent | revalidate the struct if any field changed | same |
| Struct fields<br/>`structType=granular` | - | revalidate changed fields | same |
| Struct fields<br/>`structType=atomic` | - | revalidate changed fields | [Issue #131566](https://github.com/kubernetes/kubernetes/issues/131566) (Alignment needed) |
| Slices | all elements are equivalent and the is order unchanged | revalidate the slice if any element changed or order changed | `listType=map`: no validate when re-order<br/>`listType=set`: re-validate when re-order<br/>`listType=atomic`: re-validate when re-order |
| Slice values<br/>`listType=atomic` | - | validate items which are not found (by value) in the old slice | Validate all elements (CRDs ratcheting may be expanded to match in-tree ratcheting) |
| Slice values<br/>`listType=map` | - | (re)validate items which are not found (by key) in the old slice or are changed | same |
Copy link
Contributor

Choose a reason for hiding this comment

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

if order of a slice that is a map changes, but all the keys and values are the same, do we skip validation? I would expect us to skip.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it would skip for the validation on the individual element. But should we still validate the listmap as a whole? If it is declared with "immutable", we consider the order as part of the API and validate the order?

| Slice values<br/>`listType=set` | - | validate items which are not found (by value) in the old slice | Validate all elements (CRDs ratcheting may be expanded to match in-tree ratcheting) |
| Maps | all elements are equivalent | revalidate the map if any element changed | same |
| Map values<br/>`mapType=granular` | - | (re)validate items which are not found (by key) in the old map | same |
| Map values<br/>`mapType=atomic` | - | (re)validate items which are not found (by key) in the old map | [Issue #131566](https://github.com/kubernetes/kubernetes/issues/131566) (Alignment needed) |

**Note on Atomic Types:** The behavior for `structType=atomic` and `mapType=atomic` intentionally deviates from strict atomic re-validation. Only the specific sub-fields or key-value pairs *that were actually modified* are re-validated. This prioritizes user experience but requires alignment with CRD behavior (tracked in Issue #131566).

#### Ratcheting and Cross-Field Validation

A challenge arises if a cross-field validation rule (e.g. `X < Y`) is defined on a common ancestor struct, and an unrelated field (e.g. `Z`) within that same ancestor is modified. This change to `Z` makes the common ancestor “changed” overall, triggering re-validation of the `X < Y` rule. If this rule was recently evolved (e.g., made stricter), it might now fail even if `X` and `Y` themselves are not modified by the user’s update. This could violate the principle “Unchanged fields do not cause update rejections.”

For the initial implementation, this behavior will be documented. Furthermore, cross-field validation rules themselves must incorporate internal ratcheting logic. For instance, generated code for dedicated cross-field tags (like `+k8s:unionMember`) will be designed to only act upon changes to the specific fields they govern and were designed to validate.
Comment on lines +1071 to +1073
Copy link
Contributor

Choose a reason for hiding this comment

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

Please build us an example as soon as we have enough merged so we can see how painful (or not) it seems to be.


### Test Plan

Expand Down