You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
-[Phase1: Initialization (Responsibility of Contributors Implementing the KEP)](#phase1-initialization-responsibility-of-contributors-implementing-the-kep)
50
50
-[Phase2: Scaling the Migration (Responsibility of Contributors Implementing the KEP and broader community)](#phase2-scaling-the-migration-responsibility-of-contributors-implementing-the-kep-and-broader-community)
51
51
-[Phase3: Finalization and GA (Core Team and community)](#phase3-finalization-and-ga-core-team-and-community)
52
52
-[Tagging and Validating Against Internal vs Versioned Types](#tagging-and-validating-against-internal-vs-versioned-types)
53
53
-[Handling Zero Values in Declarative Validation](#handling-zero-values-in-declarative-validation)
54
54
-[Difficulties with <code>+required</code> and <code>+default</code>](#difficulties-with-required-and-default)
55
-
-[Proposed Solutions](#proposed-solutions)
55
+
-[Proposed Solutions](#proposed-solutions-1)
56
56
-[Addressing the Problem with Valid Zero Values Using the Linter](#addressing-the-problem-with-valid-zero-values-using-the-linter)
57
57
-[Ratcheting](#ratcheting)
58
58
-[Test Plan](#test-plan)
@@ -145,7 +145,7 @@ Declarative validation will also benefit Kubernetes users:
145
145
146
146
* Adding new fields and associated validation rules becomes a simpler process of adding IDL tags to the field definition, rather than writing and maintaining validation functions. This reduces potential inconsistencies and bugs. Testing is also streamlined as centralized rules and frameworks enable the use of test fixtures across k8s types making creating tests simpler and faster to implement for contributors.
147
147
* Creating k8s APIs becomes faster as developers can focus on the API’s structure and behavior, leaving the validation boilerplate to be generated automatically. This speeds up development and encourages experimentation with new APIs.
148
-
* Validation is performed on versioned types (assuming recommended approach is from design below), so error message and attributed field path is more likely to be relevant to the user. (Though no known cases of misattributed field errors have yet occurred)
148
+
* Validation is performed on versioned types (assuming recommended approach is used from design below), so error message and attributed field path is more likely to be relevant to the user. (ex: some fields in autoscaling/v2 will be mis-identified with current approach but resolved w/ Declarative Validation + versioned types)
149
149
150
150
Additionally Declarative Validation can be built upon to support features such as:
151
151
@@ -160,7 +160,7 @@ Please feel free to try out the [prototype](https://github.com/jpbetz/kubernetes
160
160
161
161
* Eliminate 90% of net-new hand-written validation within 5 kube releases (target start: v1.33)
162
162
* Convert 50% of existing hand-written validation within 5 kube releases (target start: v1.33)
163
-
*If we lose steam and abandon this, we can roll it back relatively easily.
163
+
*Migrate in such a way that if contributors lose steam and abandon this, we can roll it back relatively easily.
164
164
*`types.go` files become the de-facto IDL of Kubernetes for native types. It is worth noting that `+enum` support, `+default` support and similar enhancements all moved our API development forward in this direction. This enhancement is an attempt to continue that story arc.
165
165
* API Validations are readable and manageable. The IDL should be a joy to use for API authors, reviewers and maintainers. Common complex relationships have simple-to-express idioms in the IDL.
166
166
* Reduce API development costs for API authors and reviewers. Reduce long term maintainer costs. Reclaiming time from core project contributors.
@@ -169,7 +169,6 @@ Please feel free to try out the [prototype](https://github.com/jpbetz/kubernetes
169
169
* Enable development of linters and other API tool chains that use API validation rules and metadata. Further reducing development effort and risk of incorrect validation rules.
170
170
* Retain native (or nearly native) performance.
171
171
* Improve testing rigor by being vastly easier to test.
172
-
* Migrate in such a way that if contributors lose steam and abandon this, we can roll it back relatively easily.
173
172
174
173
### Non-Goals
175
174
@@ -217,14 +216,14 @@ The previous Declarative Validation proposal ([KEP-4153](https://github.com/kube
217
216
218
217
In order to properly support the User Stories for “Kubernetes developer wishes to add a field to an existing API version” and “Kubernetes developer adds a new version an API” it is important that `validation-gen` and the associated tooling for IDL tags have robust UX that immediately notifies users when tags are not used properly. To support this `validation-gen` will have options for validators subject to when they register so that validation authors can express how their associated IDL tag should be used and the framework will error if a user uses an IDL tag incorrectly. See this related WIP PR [here](https://github.com/jpbetz/kubernetes/pull/89) adding such functionality to the prototype for an example of what this might look like.
219
218
220
-
The goal is that when a user makes a mistake in authoring IDL tags we give a meaning error. Users are not expected to know the underlying system or be an insider on the project to successfully use Declarative Validation.
219
+
The goal is that when a user makes a mistake in authoring IDL tags we give a meaningful error. Users are not expected to know the underlying system or be an insider on the project to successfully use Declarative Validation.
221
220
222
221
### Introduce new validation tests and test framework
223
222
224
223
New validation tests will be created as part of the migration process for contributors migrating fields using `validation-gen`. Additionally, a test framework and associated migration test utilities will be created as part of `validation-gen` to leverage the new centralized validation rules and to ensure validation consistency across hand-written and declarative validation rules. See the [Test Plan](?tab=t.0#bookmark=id.a86sekfgbuen) section for more details.
225
224
226
225
#### New Validations Vs Migrating Validations
227
-
`validation-gen`
226
+
FIXME...
228
227
229
228
#### New Validation Tests
230
229
As part of the process for migrating fields, contributors will scrutinize and improve as needed the current validation tests. No field can go thru migration without a robust test for the field being migrated, which proves that it is validated correctly before the change and after. Many existing tests are not sufficient to verify 100% equivalency and need retooling. This allows us to de-risk migration problems by scrutinizing the current tests and enhancing them.
@@ -501,87 +500,86 @@ The below rules are currently implemented or are very similar to an existing val
Shared types present a challenge. For example, different Kubernetes resources have different validation rules for `metadata.name` and `metadata.generateName`. But all resources share the `ObjectMeta` type.
724
722
723
+
#### `subfield` IDL Tag
724
+
725
+
To handle this case, we provide an IDL tag - `k8s:subfield(<field-json-name>)` which can be used to specify a `subfield` validation to add to parent which validates against the the `subfield` value:
726
+
727
+
```go
728
+
typeStructstruct {
729
+
// +k8s:subfield(name)=+k8s:format=dns-label
730
+
metav1.ObjectMeta
731
+
}
732
+
```
733
+
734
+
This will also support chaining of subfield calls with other validators (including subfield) which allows for setting subfield validations on arbitrarily deep nested fields of shared structs. An exaggerated example showcasing this is below: \
<<[UNRESOLVED is it ok that `validation-gen`'s currently design only supports one-deep typedef support? There are potential solutions to extend this, see below]>>
@@ -747,7 +763,7 @@ type Struct struct {
747
763
748
764
In the above example, FooField would be validated as a DNS label and have at least 4 characters which is expected. What might also be expected though is that BarField would be validated as a DNS label, have at least 4 characters, and have no more than 16 characters. INCORRECT! Due to Go's type system, the relationship of type Foo -> string is represented, but type Bar -> type Foo -> string is flattened to type Bar -> string. This leads to a currently open question around the severity of this potential UX issue as well potential solutions on how this could be mitigated if needed.
749
765
750
-
##### Proposed Options
766
+
##### Proposed Solutions
751
767
752
768
1.**Have it well documented that `validation-gen`only support one-deep typedef:
753
769
* Pro: Doesn't require any additional architectural changes to `validation-gen`.
@@ -763,22 +779,7 @@ In the above example, FooField would be validated as a DNS label and have at lea
763
779
* Pro: Better UX for users as they are notified not to use IDL tags that might lead to unintended outcomes when adding IDL tags
764
780
* Con: Requires implementing the chain of typedefs logic.
765
781
766
-
#### `subfield` IDL Tag
767
-
768
-
To handle this case, we provide an IDL tag - `k8s:subfield(<field-json-name>)` which can be used to specify a `subfield` validation to add to parent which validates against the the `subfield` value:
769
-
770
-
```go
771
-
typeStructstruct {
772
-
// +k8s:subfield(name)=+k8s:format=dns-label
773
-
metav1.ObjectMeta
774
-
}
775
-
```
776
-
777
-
This will also support chaining of subfield calls with other validators (including subfield) which allows for setting subfield validations on arbitrarily deep nested fields of shared structs. An exaggerated example showcasing this is below: \
* A new tag could be introduced to signify that a zero value is permissible, even with a default.
896
897
* <strong>Drawback:</strong> Adds complexity by introducing another tag and complicates the mental model.
897
898
3. <strong>Compile-Time or Runtime Default Value Check:</strong>
898
-
* <strong>Compile-Time Check:</strong> During code generation, the <code>+default</code> tag could be parsed, and if it refers to a zero value, validation logic could be adjusted accordingly.
899
+
* <strong>Compile-Time Check:</strong> During code generation, the `+default` tag could be parsed, and if it refers to a zero value, validation logic could be adjusted accordingly.
899
900
* <strong>Drawback:</strong> Complex implementation, requires more information to be available during code generation.
900
901
* <strong>Runtime Check:</strong> Validation logic could check if the provided default value is a zero value and skip certain checks.
901
902
* <strong>Drawback:</strong> Considered overly-complicated ("gross") and potentially impacts performance.
903
+
* <strong>Benefit:</strong> Closest to correct.
904
+
4. <strong>Make `+optional` on non-pointer fields be advisory:</strong>
905
+
* If we find an optional string field, the optional tag can be used as documentation, but the actual validation will rely on the format-checking (e.g. dns-label). To an end user this means that what used to be a "field is required" error now becomes a "not a dns-label" error. Only slightly worse.
902
906
903
907
#### Addressing the Problem with Valid Zero Values Using the Linter
<<[UNRESOLVED should we move current validation_test.go logic to strategy_test.go as it makes more sense generally and aids this project? ]>>
1027
+
1022
1028
1023
1029
Currently, validation logic and associated tests are logically split across `validation.go` and `strategy.go`. The hand-written validation functions and associated tests reside in `validation.go` and `validation_test.go` while `strategy.go` determines when to invoke these functions during API object creation and updates. With the introduction of declarative validation (controlled by the `DeclarativeValidation` feature gate) the current logic split is worth considering moving from `validation_test.go` to `strategy_test.go` as the current logic split in the test is unfavorable for the migration as it requires additional plumbing work.
1024
1030
1025
1031
The current approach in the prototype experiment to migrate ReplicationController involves directly injecting declarative validation and equivalency tests into `validation_test.go`. This is achieved by conditionally appending calls to `rest.ValidateDeclaratively` within the existing test cases, based on the `DeclarativeValidation` feature gate's status. This allows for a direct comparison of the outputs between hand-written and declarative validation within the same test framework.
1026
1032
1027
-
**Moving the feature gate check and declarative validation logic to <code>strategy_test.go</code> could offer several advantages</strong>:
1033
+
**Moving the feature gate check and declarative validation logic to `strategy_test.go` could offer several advantages</strong>:
1028
1034
1029
1035
* **Reduced Test Duplication:** `validation_test.go` could be simplified, as it would no longer need to handle both hand-written and declarative validation paths.
1030
1036
* **Clearer Separation of Concerns:** `strategy.go` would be responsible for determining _when_ to validate, while `validation.go` would handle _how_ to validate.
@@ -1036,6 +1042,8 @@ However, this approach also has potential drawbacks:
1036
1042
1037
1043
Given the low complexity of moving this code prior to the changes, the enhanced logic split of moving the code, and the reduced work for the migration that moving this code would have currently the plan for Declarative Validation is that the current `validation_test.go` tests are moved to `strategy_test.go` in a PR prior to the migration PR.
1038
1044
1045
+
<<[/UNRESOLVED]>>
1046
+
1039
1047
###### Error Message Equivalence
1040
1048
1041
1049
Some error messages will be phrased differently but preserve the same semantic meaning when converted to the declarative validation system. For our testing to check if errors are changed we have two options:
@@ -1071,7 +1079,7 @@ To ensure that Declarative Validation and some of the identified potential perfo
1071
1079
1072
1080
##### DeclarativeValidation
1073
1081
1074
-
* `validation-gen` code generator supports the full set of necessary IDL tags for 1:1 porting of handwritten validation to declarative validation
1082
+
* `validation-gen` code generator supports the full set of necessary IDL tags for 1:1 porting of handwritten validation to declarative validation
1075
1083
* Have plumbed all previous validation_test.go unit tests to run against declarative validation schemas.
1076
1084
* All Unit and Integration tests pass with no errors or only well-understood exceptional errors sourced from a file in the repository
1077
1085
* Linter finalized with complete set of linter rules
0 commit comments