diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/ratcheting/default_behavior/doc.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/ratcheting/default_behavior/doc.go index f8bcd0b1959..f20a4b553e5 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/ratcheting/default_behavior/doc.go +++ b/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"` +} diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/ratcheting/default_behavior/doc_test.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/ratcheting/default_behavior/doc_test.go index 9d840c97d13..eedb927f26a 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/ratcheting/default_behavior/doc_test.go +++ b/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() +} diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/ratcheting/default_behavior/zz_generated.validations.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/ratcheting/default_behavior/zz_generated.validations.go index 9dff33bad34..82fb34499b6 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/ratcheting/default_behavior/zz_generated.validations.go +++ b/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) { diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/ratcheting/list/doc.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/ratcheting/list/doc.go index a46f406b160..edd0dcca9fe 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/ratcheting/list/doc.go +++ b/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"` +} diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/ratcheting/list/doc_test.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/ratcheting/list/doc_test.go index 08e3b9c9d8c..bb5aaeeaead 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/ratcheting/list/doc_test.go +++ b/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() +} diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/ratcheting/list/zz_generated.validations.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/ratcheting/list/zz_generated.validations.go index 08c0f731d7f..e740378a902 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/ratcheting/list/zz_generated.validations.go +++ b/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) { diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/testscheme/doc.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/testscheme/doc.go index 5d0f21e1672..618fb7f72cd 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/testscheme/doc.go +++ b/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) diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/testscheme/testscheme.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/testscheme/testscheme.go index 0f4fe1849b4..5261c73dfe9 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/testscheme/testscheme.go +++ b/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...) }