Skip to content

Commit ff8dd1a

Browse files
committed
resolving KEP comments round kubernetes#6
1 parent 3492567 commit ff8dd1a

File tree

1 file changed

+4
-12
lines changed
  • keps/sig-api-machinery/5073-declarative-validation-with-validation-gen

1 file changed

+4
-12
lines changed

keps/sig-api-machinery/5073-declarative-validation-with-validation-gen/README.md

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1033,26 +1033,18 @@ func TestVersionedValidationByFuzzing(t *testing.T) {
10331033
```
10341034
10351035
###### strategy_test.go vs validation_test.go
1036-
<<[UNRESOLVED should we move current validation_test.go logic to strategy_test.go as it makes more sense generally and aids this project? ]>>
1037-
1038-
10391036
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.
10401037
1041-
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.
1042-
1043-
**Moving the feature gate check and declarative validation logic to `strategy_test.go` could offer several advantages</strong>:
1038+
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. For v1.33 we plan on using this method for the initial small migration where we land the core pieces of `validation-gen`
10441039
1040+
For v1.34 we plan on - **moving the feature gate check and declarative validation logic to `strategy_test.go`**. Doing this has the following benefits:
10451041
* **Reduced Test Duplication:** `validation_test.go` could be simplified, as it would no longer need to handle both hand-written and declarative validation paths.
10461042
* **Clearer Separation of Concerns:** `strategy.go` would be responsible for determining _when_ to validate, while `validation.go` would handle _how_ to validate.
10471043
* **Easier Migration:** Transitioning to a fully declarative model would be smoother, as the core validation logic would already be invoked through the strategy.
10481044
1049-
However, this approach also has potential drawbacks:
1045+
Doing this requires an additional PR to moving existing hand-written validation logic from `validation.go` to `strategy.go` would but it would be straightforward (only moving files). This would be done **in a PR after the initial migration PR in v1.34 but before any additional migration work is done.**
10501046
1051-
* **Additional PRs for Initial Migration:** Moving existing hand-written validation logic from `validation.go` to `strategy.go` would result in additional work via a PR to move the related code but it would be straightforward (only moving files).
1052-
1053-
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.
1054-
1055-
<<[/UNRESOLVED]>>
1047+
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`.
10561048
10571049
###### Error Message Equivalence
10581050

0 commit comments

Comments
 (0)