feat(validation-gen): Refactor testscheme and add ratcheting bug tests

Refactors the ValidationTester in testscheme to make update validation implicit. Calling .OldValue() now automatically triggers update validation, removing the need to pass an explicit operation type and simplifying the test-writing API.

This is applied to new tests that demonstrate a validation ratcheting bug where validation is incorrectly skipped when comparing a nil old value with a new value. The tests cover list items and struct fields to ensure the fix will be verified.

staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/ratcheting/default_behavior/doc.go

@@ -142,3 +142,12 @@ type NestedNonDirectComparableStruct struct {
 	// +k8s:validateFalse="field nonDirectComparableStructField"
 	NonDirectComparableStructField NonDirectComparableStruct `json:"nonDirectComparableStructField"`
 }
+
+type MixComparableStruct struct {
+	TypeMeta int
+
+	Primitive string `json:"Primitive"`
+
+	// +k8s:validateFalse="field NonComparable"
+	NonComparable []string `json:"NonComparable"`
+}

staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/ratcheting/default_behavior/doc_test.go

@@ -175,3 +175,12 @@ func Test_StructEmbedded(t *testing.T) {
 
 	st.Value(mkTest()).OldValue(mkTest()).ExpectValid()
 }
+
+// This test is to prove the bug of ratcheting behavior mistakenly skip validation on nil vs not found.
+// TODO: update this test once the ratcheting behavior is fixed.
+func Test_Mix(t *testing.T) {
+	st := localSchemeBuilder.Test(t)
+	st.Value(&MixComparableStruct{
+		Primitive: "a",
+	}).OldValue(nil).ExpectValid()
+}

staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/ratcheting/default_behavior/zz_generated.validations.go

@@ -38,6 +38,14 @@ func init() { localSchemeBuilder.Register(RegisterValidations) }
 // RegisterValidations adds validation functions to the given scheme.
 // Public to allow building arbitrary schemes.
 func RegisterValidations(scheme *testscheme.Scheme) error {
+	// type MixComparableStruct
+	scheme.AddValidationFunc((*MixComparableStruct)(nil), func(ctx context.Context, op operation.Operation, obj, oldObj interface{}) field.ErrorList {
+		switch op.Request.SubresourcePath() {
+		case "/":
+			return Validate_MixComparableStruct(ctx, op, nil /* fldPath */, obj.(*MixComparableStruct), safe.Cast[*MixComparableStruct](oldObj))
+		}
+		return field.ErrorList{field.InternalError(nil, fmt.Errorf("no validation found for %T, subresource: %v", obj, op.Request.SubresourcePath()))}
+	})
 	// type StructEmbedded
 	scheme.AddValidationFunc((*StructEmbedded)(nil), func(ctx context.Context, op operation.Operation, obj, oldObj interface{}) field.ErrorList {
 		switch op.Request.SubresourcePath() {
@@ -121,6 +129,27 @@ func Validate_DirectComparableStruct(ctx context.Context, op operation.Operation
 	return errs
 }
 
+// Validate_MixComparableStruct validates an instance of MixComparableStruct according
+// to declarative validation rules in the API schema.
+func Validate_MixComparableStruct(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *MixComparableStruct) (errs field.ErrorList) {
+	// field MixComparableStruct.TypeMeta has no validation
+	// field MixComparableStruct.Primitive has no validation
+
+	// field MixComparableStruct.NonComparable
+	errs = append(errs,
+		func(fldPath *field.Path, obj, oldObj []string) (errs field.ErrorList) {
+			// don't revalidate unchanged data
+			if op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) {
+				return nil
+			}
+			// call field-attached validations
+			errs = append(errs, validate.FixedResult(ctx, op, fldPath, obj, oldObj, false, "field NonComparable")...)
+			return
+		}(fldPath.Child("NonComparable"), obj.NonComparable, safe.Field(oldObj, func(oldObj *MixComparableStruct) []string { return oldObj.NonComparable }))...)
+
+	return errs
+}
+
 // Validate_MySlice validates an instance of MySlice according
 // to declarative validation rules in the API schema.
 func Validate_MySlice(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj MySlice) (errs field.ErrorList) {

staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/ratcheting/list/doc.go

@@ -104,3 +104,18 @@ type MixedKeyStruct struct {
 	Key2 string  `json:"key2"`
 	Data string  `json:"data"`
 }
+
+type Item struct {
+	Key string `json:"key"`
+
+	// +k8s:validateFalse="field Data"
+	Data map[string]string `json:"data"`
+}
+
+type ItemList struct {
+	TypeMeta int
+
+	// +k8s:listType=map
+	// +k8s:listMapKey=key
+	Items []Item `json:"items"`
+}

staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/ratcheting/list/doc_test.go

@@ -99,3 +99,19 @@ func Test_StructSlice(t *testing.T) {
 		MapSliceMixedKeyField:      []MixedKeyStruct{{Key1: ptr.To("a"), Key2: "1", Data: "A"}, {Key1: ptr.To("b"), Key2: "2", Data: "B"}},
 	}).ExpectValid()
 }
+
+// This test is to prove the bug of ratcheting behavior mistakenly skip validation on nil vs not found.
+// TODO: update this test once the ratcheting behavior is fixed.
+func Test_Items(t *testing.T) {
+	st := localSchemeBuilder.Test(t)
+
+	st.Value(&ItemList{
+		Items: []Item{
+			{Key: "valid2"},
+		},
+	}).OldValue(&ItemList{
+		Items: []Item{
+			{Key: "valid1"},
+		},
+	}).ExpectValid()
+}

staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/ratcheting/list/zz_generated.validations.go

@@ -38,6 +38,14 @@ func init() { localSchemeBuilder.Register(RegisterValidations) }
 // RegisterValidations adds validation functions to the given scheme.
 // Public to allow building arbitrary schemes.
 func RegisterValidations(scheme *testscheme.Scheme) error {
+	// type ItemList
+	scheme.AddValidationFunc((*ItemList)(nil), func(ctx context.Context, op operation.Operation, obj, oldObj interface{}) field.ErrorList {
+		switch op.Request.SubresourcePath() {
+		case "/":
+			return Validate_ItemList(ctx, op, nil /* fldPath */, obj.(*ItemList), safe.Cast[*ItemList](oldObj))
+		}
+		return field.ErrorList{field.InternalError(nil, fmt.Errorf("no validation found for %T, subresource: %v", obj, op.Request.SubresourcePath()))}
+	})
 	// type StructSlice
 	scheme.AddValidationFunc((*StructSlice)(nil), func(ctx context.Context, op operation.Operation, obj, oldObj interface{}) field.ErrorList {
 		switch op.Request.SubresourcePath() {
@@ -49,6 +57,49 @@ func RegisterValidations(scheme *testscheme.Scheme) error {
 	return nil
 }
 
+// Validate_Item validates an instance of Item according
+// to declarative validation rules in the API schema.
+func Validate_Item(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *Item) (errs field.ErrorList) {
+	// field Item.Key has no validation
+
+	// field Item.Data
+	errs = append(errs,
+		func(fldPath *field.Path, obj, oldObj map[string]string) (errs field.ErrorList) {
+			// don't revalidate unchanged data
+			if op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) {
+				return nil
+			}
+			// call field-attached validations
+			errs = append(errs, validate.FixedResult(ctx, op, fldPath, obj, oldObj, false, "field Data")...)
+			return
+		}(fldPath.Child("data"), obj.Data, safe.Field(oldObj, func(oldObj *Item) map[string]string { return oldObj.Data }))...)
+
+	return errs
+}
+
+// Validate_ItemList validates an instance of ItemList according
+// to declarative validation rules in the API schema.
+func Validate_ItemList(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *ItemList) (errs field.ErrorList) {
+	// field ItemList.TypeMeta has no validation
+
+	// field ItemList.Items
+	errs = append(errs,
+		func(fldPath *field.Path, obj, oldObj []Item) (errs field.ErrorList) {
+			// don't revalidate unchanged data
+			if op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) {
+				return nil
+			}
+			// call field-attached validations
+			// lists with map semantics require unique keys
+			errs = append(errs, validate.Unique(ctx, op, fldPath, obj, oldObj, func(a Item, b Item) bool { return a.Key == b.Key })...)
+			// iterate the list and call the type's validation function
+			errs = append(errs, validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, func(a Item, b Item) bool { return a.Key == b.Key }, validate.SemanticDeepEqual, Validate_Item)...)
+			return
+		}(fldPath.Child("items"), obj.Items, safe.Field(oldObj, func(oldObj *ItemList) []Item { return oldObj.Items }))...)
+
+	return errs
+}
+
 // Validate_MixedKeyStruct validates an instance of MixedKeyStruct according
 // to declarative validation rules in the API schema.
 func Validate_MixedKeyStruct(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *MixedKeyStruct) (errs field.ErrorList) {

staging/src/k8s.io/code-generator/cmd/validation-gen/testscheme/doc.go

@@ -34,7 +34,9 @@ limitations under the License.
 // by generated test fixtures.
 //
 // For example, to test by hand. The testschema provides utilities to create a value and assert
-// that the expected errors are returned when the value is validated:
+// that the expected errors are returned when the value is validated.
+// Note that if `OldValue()` or `OldValueFuzzed()` is called on the `ValidationTester`, subsequent
+// validation calls will implicitly use update validation.
 //
 //	 func Test(t *testing.T) {
 //		  st := localSchemeBuilder.Test(t)

staging/src/k8s.io/code-generator/cmd/validation-gen/testscheme/testscheme.go

@@ -245,16 +245,18 @@ type ValidationTester struct {
 	*ValidationTestBuilder
 	value        any
 	oldValue     any
+	isUpdate     bool
 	options      []string
 	subresources []string
 }
 
-// OldValue sets the oldValue for this ValidationTester. When oldValue is set to
-// a non-nil value, update validation will be used to test validation.
+// OldValue sets the oldValue for this ValidationTester. When oldValue is set,
+// update validation will be used to test validation.
 // oldValue must be the same type as value.
 // Returns ValidationTester to support call chaining.
 func (v *ValidationTester) OldValue(oldValue any) *ValidationTester {
 	v.oldValue = oldValue
+	v.isUpdate = true
 	return v
 }
 
@@ -263,6 +265,7 @@ func (v *ValidationTester) OldValue(oldValue any) *ValidationTester {
 func (v *ValidationTester) OldValueFuzzed(oldValue any) *ValidationTester {
 	randfiller().Fill(oldValue)
 	v.oldValue = oldValue
+	v.isUpdate = true
 	return v
 }
 
@@ -370,11 +373,8 @@ func (v *ValidationTester) ExpectMatches(matcher field.ErrorMatcher, expected fi
 }
 
 func (v *ValidationTester) validate() field.ErrorList {
-	var errs field.ErrorList
-	if v.oldValue == nil {
-		errs = v.s.Validate(context.Background(), v.options, v.value, v.subresources...)
-	} else {
-		errs = v.s.ValidateUpdate(context.Background(), v.options, v.value, v.oldValue, v.subresources...)
+	if v.isUpdate {
+		return v.s.ValidateUpdate(context.Background(), v.options, v.value, v.oldValue, v.subresources...)
 	}
-	return errs
+	return v.s.Validate(context.Background(), v.options, v.value, v.subresources...)
 }