From a5a2cfdb3535f65579aad6bbbbbe344fcce85bf3 Mon Sep 17 00:00:00 2001 From: yongruilin Date: Wed, 5 Nov 2025 01:57:08 +0000 Subject: [PATCH] fix(validation-gen): Correct ratcheting for uncorrelated old values The validation ratcheting logic failed to distinguish between a field that was explicitly nil and a field that was absent in the old object (uncorrelated). safe.Field() returns nil in both scenarios. This caused validation to be incorrectly skipped for oldObj that cannot be found during an update, as the logic treated the (nil, nil) old/new value pair as unchanged. This commit introduces an oldValueCorrelated boolean flag to the generated validation functions. This flag is set to false when the parent of the old object is nil, signaling that a corresponding old value could not be found. The ratcheting check is now conditioned on this flag, ensuring that validation proceeds correctly. --- .../ratcheting/default_behavior/doc_test.go | 6 +- .../zz_generated.validations.go | 138 +++++++++--------- .../output_tests/ratcheting/list/doc_test.go | 6 +- .../list/zz_generated.validations.go | 72 ++++----- .../cmd/validation-gen/validation.go | 26 +++- 5 files changed, 133 insertions(+), 115 deletions(-) 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 eedb927f26a..c4bdcd27387 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 @@ -176,11 +176,11 @@ 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() + }).OldValue(nil).ExpectMatches(field.ErrorMatcher{}.ByType().ByField(), field.ErrorList{ + field.Invalid(field.NewPath("NonComparable"), "", ""), + }) } 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 82fb34499b6..70526d231f9 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 @@ -116,15 +116,15 @@ func Validate_DirectComparableStruct(ctx context.Context, op operation.Operation // field DirectComparableStruct.IntField errs = append(errs, - func(fldPath *field.Path, obj, oldObj *int) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj *int, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && (obj == oldObj || (obj != nil && oldObj != nil && *obj == *oldObj)) { + if oldValueCorrelated && op.Type == operation.Update && (obj == oldObj || (obj != nil && oldObj != nil && *obj == *oldObj)) { return nil } // call field-attached validations errs = append(errs, validate.FixedResult(ctx, op, fldPath, obj, oldObj, false, "field intField")...) return - }(fldPath.Child("intField"), &obj.IntField, safe.Field(oldObj, func(oldObj *DirectComparableStruct) *int { return &oldObj.IntField }))...) + }(fldPath.Child("intField"), &obj.IntField, safe.Field(oldObj, func(oldObj *DirectComparableStruct) *int { return &oldObj.IntField }), oldObj != nil)...) return errs } @@ -137,15 +137,15 @@ func Validate_MixComparableStruct(ctx context.Context, op operation.Operation, f // field MixComparableStruct.NonComparable errs = append(errs, - func(fldPath *field.Path, obj, oldObj []string) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj []string, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { + if oldValueCorrelated && 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 }))...) + }(fldPath.Child("NonComparable"), obj.NonComparable, safe.Field(oldObj, func(oldObj *MixComparableStruct) []string { return oldObj.NonComparable }), oldObj != nil)...) return errs } @@ -165,9 +165,9 @@ func Validate_NestedDirectComparableStruct(ctx context.Context, op operation.Ope // field NestedDirectComparableStruct.DirectComparableStructField errs = append(errs, - func(fldPath *field.Path, obj, oldObj *DirectComparableStruct) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj *DirectComparableStruct, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && (obj == oldObj || (obj != nil && oldObj != nil && *obj == *oldObj)) { + if oldValueCorrelated && op.Type == operation.Update && (obj == oldObj || (obj != nil && oldObj != nil && *obj == *oldObj)) { return nil } // call field-attached validations @@ -177,7 +177,7 @@ func Validate_NestedDirectComparableStruct(ctx context.Context, op operation.Ope return }(fldPath.Child("directComparableStructField"), &obj.DirectComparableStructField, safe.Field(oldObj, func(oldObj *NestedDirectComparableStruct) *DirectComparableStruct { return &oldObj.DirectComparableStructField - }))...) + }), oldObj != nil)...) return errs } @@ -189,9 +189,9 @@ func Validate_NestedNonDirectComparableStruct(ctx context.Context, op operation. // field NestedNonDirectComparableStruct.NonDirectComparableStructField errs = append(errs, - func(fldPath *field.Path, obj, oldObj *NonDirectComparableStruct) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj *NonDirectComparableStruct, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { + if oldValueCorrelated && op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { return nil } // call field-attached validations @@ -201,7 +201,7 @@ func Validate_NestedNonDirectComparableStruct(ctx context.Context, op operation. return }(fldPath.Child("nonDirectComparableStructField"), &obj.NonDirectComparableStructField, safe.Field(oldObj, func(oldObj *NestedNonDirectComparableStruct) *NonDirectComparableStruct { return &oldObj.NonDirectComparableStructField - }))...) + }), oldObj != nil)...) return errs } @@ -213,15 +213,15 @@ func Validate_NonDirectComparableStruct(ctx context.Context, op operation.Operat // field NonDirectComparableStruct.IntPtrField errs = append(errs, - func(fldPath *field.Path, obj, oldObj *int) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj *int, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && (obj == oldObj || (obj != nil && oldObj != nil && *obj == *oldObj)) { + if oldValueCorrelated && op.Type == operation.Update && (obj == oldObj || (obj != nil && oldObj != nil && *obj == *oldObj)) { return nil } // call field-attached validations errs = append(errs, validate.FixedResult(ctx, op, fldPath, obj, oldObj, false, "field intPtrField")...) return - }(fldPath.Child("intPtrField"), obj.IntPtrField, safe.Field(oldObj, func(oldObj *NonDirectComparableStruct) *int { return oldObj.IntPtrField }))...) + }(fldPath.Child("intPtrField"), obj.IntPtrField, safe.Field(oldObj, func(oldObj *NonDirectComparableStruct) *int { return oldObj.IntPtrField }), oldObj != nil)...) return errs } @@ -241,9 +241,9 @@ func Validate_StructEmbedded(ctx context.Context, op operation.Operation, fldPat // field StructEmbedded.DirectComparableStruct errs = append(errs, - func(fldPath *field.Path, obj, oldObj *DirectComparableStruct) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj *DirectComparableStruct, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && (obj == oldObj || (obj != nil && oldObj != nil && *obj == *oldObj)) { + if oldValueCorrelated && op.Type == operation.Update && (obj == oldObj || (obj != nil && oldObj != nil && *obj == *oldObj)) { return nil } // call field-attached validations @@ -251,13 +251,13 @@ func Validate_StructEmbedded(ctx context.Context, op operation.Operation, fldPat // call the type's validation function errs = append(errs, Validate_DirectComparableStruct(ctx, op, fldPath, obj, oldObj)...) return - }(fldPath.Child("directComparableStruct"), &obj.DirectComparableStruct, safe.Field(oldObj, func(oldObj *StructEmbedded) *DirectComparableStruct { return &oldObj.DirectComparableStruct }))...) + }(fldPath.Child("directComparableStruct"), &obj.DirectComparableStruct, safe.Field(oldObj, func(oldObj *StructEmbedded) *DirectComparableStruct { return &oldObj.DirectComparableStruct }), oldObj != nil)...) // field StructEmbedded.NonDirectComparableStruct errs = append(errs, - func(fldPath *field.Path, obj, oldObj *NonDirectComparableStruct) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj *NonDirectComparableStruct, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { + if oldValueCorrelated && op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { return nil } // call field-attached validations @@ -265,13 +265,13 @@ func Validate_StructEmbedded(ctx context.Context, op operation.Operation, fldPat // call the type's validation function errs = append(errs, Validate_NonDirectComparableStruct(ctx, op, fldPath, obj, oldObj)...) return - }(fldPath.Child("nonDirectComparableStruct"), &obj.NonDirectComparableStruct, safe.Field(oldObj, func(oldObj *StructEmbedded) *NonDirectComparableStruct { return &oldObj.NonDirectComparableStruct }))...) + }(fldPath.Child("nonDirectComparableStruct"), &obj.NonDirectComparableStruct, safe.Field(oldObj, func(oldObj *StructEmbedded) *NonDirectComparableStruct { return &oldObj.NonDirectComparableStruct }), oldObj != nil)...) // field StructEmbedded.NestedDirectComparableStructField errs = append(errs, - func(fldPath *field.Path, obj, oldObj *NestedDirectComparableStruct) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj *NestedDirectComparableStruct, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && (obj == oldObj || (obj != nil && oldObj != nil && *obj == *oldObj)) { + if oldValueCorrelated && op.Type == operation.Update && (obj == oldObj || (obj != nil && oldObj != nil && *obj == *oldObj)) { return nil } // call field-attached validations @@ -281,13 +281,13 @@ func Validate_StructEmbedded(ctx context.Context, op operation.Operation, fldPat return }(fldPath.Child("nestedDirectComparableStructField"), &obj.NestedDirectComparableStructField, safe.Field(oldObj, func(oldObj *StructEmbedded) *NestedDirectComparableStruct { return &oldObj.NestedDirectComparableStructField - }))...) + }), oldObj != nil)...) // field StructEmbedded.NestedNonDirectComparableStructField errs = append(errs, - func(fldPath *field.Path, obj, oldObj *NestedNonDirectComparableStruct) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj *NestedNonDirectComparableStruct, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { + if oldValueCorrelated && op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { return nil } // call field-attached validations @@ -297,7 +297,7 @@ func Validate_StructEmbedded(ctx context.Context, op operation.Operation, fldPat return }(fldPath.Child("nestedNonDirectComparableStructField"), &obj.NestedNonDirectComparableStructField, safe.Field(oldObj, func(oldObj *StructEmbedded) *NestedNonDirectComparableStruct { return &oldObj.NestedNonDirectComparableStructField - }))...) + }), oldObj != nil)...) return errs } @@ -309,9 +309,9 @@ func Validate_StructMap(ctx context.Context, op operation.Operation, fldPath *fi // field StructMap.MapKeyField errs = append(errs, - func(fldPath *field.Path, obj, oldObj map[S]string) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj map[S]string, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { + if oldValueCorrelated && op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { return nil } // call field-attached validations @@ -319,13 +319,13 @@ func Validate_StructMap(ctx context.Context, op operation.Operation, fldPath *fi // iterate the map and call the key type's validation function errs = append(errs, validate.EachMapKey(ctx, op, fldPath, obj, oldObj, Validate_S)...) return - }(fldPath.Child("mapKeyField"), obj.MapKeyField, safe.Field(oldObj, func(oldObj *StructMap) map[S]string { return oldObj.MapKeyField }))...) + }(fldPath.Child("mapKeyField"), obj.MapKeyField, safe.Field(oldObj, func(oldObj *StructMap) map[S]string { return oldObj.MapKeyField }), oldObj != nil)...) // field StructMap.MapValueField errs = append(errs, - func(fldPath *field.Path, obj, oldObj map[string]S) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj map[string]S, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { + if oldValueCorrelated && op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { return nil } // call field-attached validations @@ -333,13 +333,13 @@ func Validate_StructMap(ctx context.Context, op operation.Operation, fldPath *fi // iterate the map and call the value type's validation function errs = append(errs, validate.EachMapVal(ctx, op, fldPath, obj, oldObj, validate.DirectEqual, Validate_S)...) return - }(fldPath.Child("mapValueField"), obj.MapValueField, safe.Field(oldObj, func(oldObj *StructMap) map[string]S { return oldObj.MapValueField }))...) + }(fldPath.Child("mapValueField"), obj.MapValueField, safe.Field(oldObj, func(oldObj *StructMap) map[string]S { return oldObj.MapValueField }), oldObj != nil)...) // field StructMap.AliasMapKeyTypeField errs = append(errs, - func(fldPath *field.Path, obj, oldObj AliasMapKeyType) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj AliasMapKeyType, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { + if oldValueCorrelated && op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { return nil } // call field-attached validations @@ -347,13 +347,13 @@ func Validate_StructMap(ctx context.Context, op operation.Operation, fldPath *fi // call the type's validation function errs = append(errs, Validate_AliasMapKeyType(ctx, op, fldPath, obj, oldObj)...) return - }(fldPath.Child("aliasMapKeyTypeField"), obj.AliasMapKeyTypeField, safe.Field(oldObj, func(oldObj *StructMap) AliasMapKeyType { return oldObj.AliasMapKeyTypeField }))...) + }(fldPath.Child("aliasMapKeyTypeField"), obj.AliasMapKeyTypeField, safe.Field(oldObj, func(oldObj *StructMap) AliasMapKeyType { return oldObj.AliasMapKeyTypeField }), oldObj != nil)...) // field StructMap.AliasMapValueTypeField errs = append(errs, - func(fldPath *field.Path, obj, oldObj AliasMapValueType) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj AliasMapValueType, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { + if oldValueCorrelated && op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { return nil } // call field-attached validations @@ -361,7 +361,7 @@ func Validate_StructMap(ctx context.Context, op operation.Operation, fldPath *fi // call the type's validation function errs = append(errs, Validate_AliasMapValueType(ctx, op, fldPath, obj, oldObj)...) return - }(fldPath.Child("aliasMapValueTypeField"), obj.AliasMapValueTypeField, safe.Field(oldObj, func(oldObj *StructMap) AliasMapValueType { return oldObj.AliasMapValueTypeField }))...) + }(fldPath.Child("aliasMapValueTypeField"), obj.AliasMapValueTypeField, safe.Field(oldObj, func(oldObj *StructMap) AliasMapValueType { return oldObj.AliasMapValueTypeField }), oldObj != nil)...) return errs } @@ -373,21 +373,21 @@ func Validate_StructPrimitive(ctx context.Context, op operation.Operation, fldPa // field StructPrimitive.IntField errs = append(errs, - func(fldPath *field.Path, obj, oldObj *int) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj *int, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && (obj == oldObj || (obj != nil && oldObj != nil && *obj == *oldObj)) { + if oldValueCorrelated && op.Type == operation.Update && (obj == oldObj || (obj != nil && oldObj != nil && *obj == *oldObj)) { return nil } // call field-attached validations errs = append(errs, validate.FixedResult(ctx, op, fldPath, obj, oldObj, false, "field intField")...) return - }(fldPath.Child("intField"), &obj.IntField, safe.Field(oldObj, func(oldObj *StructPrimitive) *int { return &oldObj.IntField }))...) + }(fldPath.Child("intField"), &obj.IntField, safe.Field(oldObj, func(oldObj *StructPrimitive) *int { return &oldObj.IntField }), oldObj != nil)...) // field StructPrimitive.IntPtrField errs = append(errs, - func(fldPath *field.Path, obj, oldObj *int) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj *int, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && (obj == oldObj || (obj != nil && oldObj != nil && *obj == *oldObj)) { + if oldValueCorrelated && op.Type == operation.Update && (obj == oldObj || (obj != nil && oldObj != nil && *obj == *oldObj)) { return nil } // call field-attached validations @@ -400,7 +400,7 @@ func Validate_StructPrimitive(ctx context.Context, op operation.Operation, fldPa } errs = append(errs, validate.FixedResult(ctx, op, fldPath, obj, oldObj, false, "field intPtrField")...) return - }(fldPath.Child("intPtrField"), obj.IntPtrField, safe.Field(oldObj, func(oldObj *StructPrimitive) *int { return oldObj.IntPtrField }))...) + }(fldPath.Child("intPtrField"), obj.IntPtrField, safe.Field(oldObj, func(oldObj *StructPrimitive) *int { return oldObj.IntPtrField }), oldObj != nil)...) return errs } @@ -412,9 +412,9 @@ func Validate_StructSlice(ctx context.Context, op operation.Operation, fldPath * // field StructSlice.SliceField errs = append(errs, - func(fldPath *field.Path, obj, oldObj []S) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj []S, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { + if oldValueCorrelated && op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { return nil } // call field-attached validations @@ -422,13 +422,13 @@ func Validate_StructSlice(ctx context.Context, op operation.Operation, fldPath * // iterate the list and call the type's validation function errs = append(errs, validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, nil, nil, Validate_S)...) return - }(fldPath.Child("sliceField"), obj.SliceField, safe.Field(oldObj, func(oldObj *StructSlice) []S { return oldObj.SliceField }))...) + }(fldPath.Child("sliceField"), obj.SliceField, safe.Field(oldObj, func(oldObj *StructSlice) []S { return oldObj.SliceField }), oldObj != nil)...) // field StructSlice.TypeDefSliceField errs = append(errs, - func(fldPath *field.Path, obj, oldObj MySlice) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj MySlice, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { + if oldValueCorrelated && op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { return nil } // call field-attached validations @@ -436,7 +436,7 @@ func Validate_StructSlice(ctx context.Context, op operation.Operation, fldPath * // call the type's validation function errs = append(errs, Validate_MySlice(ctx, op, fldPath, obj, oldObj)...) return - }(fldPath.Child("typedefSliceField"), obj.TypeDefSliceField, safe.Field(oldObj, func(oldObj *StructSlice) MySlice { return oldObj.TypeDefSliceField }))...) + }(fldPath.Child("typedefSliceField"), obj.TypeDefSliceField, safe.Field(oldObj, func(oldObj *StructSlice) MySlice { return oldObj.TypeDefSliceField }), oldObj != nil)...) return errs } @@ -448,9 +448,9 @@ func Validate_StructStruct(ctx context.Context, op operation.Operation, fldPath // field StructStruct.DirectComparableStructField errs = append(errs, - func(fldPath *field.Path, obj, oldObj *DirectComparableStruct) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj *DirectComparableStruct, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && (obj == oldObj || (obj != nil && oldObj != nil && *obj == *oldObj)) { + if oldValueCorrelated && op.Type == operation.Update && (obj == oldObj || (obj != nil && oldObj != nil && *obj == *oldObj)) { return nil } // call field-attached validations @@ -458,13 +458,13 @@ func Validate_StructStruct(ctx context.Context, op operation.Operation, fldPath // call the type's validation function errs = append(errs, Validate_DirectComparableStruct(ctx, op, fldPath, obj, oldObj)...) return - }(fldPath.Child("directComparableStructField"), &obj.DirectComparableStructField, safe.Field(oldObj, func(oldObj *StructStruct) *DirectComparableStruct { return &oldObj.DirectComparableStructField }))...) + }(fldPath.Child("directComparableStructField"), &obj.DirectComparableStructField, safe.Field(oldObj, func(oldObj *StructStruct) *DirectComparableStruct { return &oldObj.DirectComparableStructField }), oldObj != nil)...) // field StructStruct.NonDirectComparableStructField errs = append(errs, - func(fldPath *field.Path, obj, oldObj *NonDirectComparableStruct) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj *NonDirectComparableStruct, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { + if oldValueCorrelated && op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { return nil } // call field-attached validations @@ -472,13 +472,13 @@ func Validate_StructStruct(ctx context.Context, op operation.Operation, fldPath // call the type's validation function errs = append(errs, Validate_NonDirectComparableStruct(ctx, op, fldPath, obj, oldObj)...) return - }(fldPath.Child("nonDirectComparableStructField"), &obj.NonDirectComparableStructField, safe.Field(oldObj, func(oldObj *StructStruct) *NonDirectComparableStruct { return &oldObj.NonDirectComparableStructField }))...) + }(fldPath.Child("nonDirectComparableStructField"), &obj.NonDirectComparableStructField, safe.Field(oldObj, func(oldObj *StructStruct) *NonDirectComparableStruct { return &oldObj.NonDirectComparableStructField }), oldObj != nil)...) // field StructStruct.DirectComparableStructPtr errs = append(errs, - func(fldPath *field.Path, obj, oldObj *DirectComparableStruct) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj *DirectComparableStruct, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && (obj == oldObj || (obj != nil && oldObj != nil && *obj == *oldObj)) { + if oldValueCorrelated && op.Type == operation.Update && (obj == oldObj || (obj != nil && oldObj != nil && *obj == *oldObj)) { return nil } // call field-attached validations @@ -486,13 +486,13 @@ func Validate_StructStruct(ctx context.Context, op operation.Operation, fldPath // call the type's validation function errs = append(errs, Validate_DirectComparableStruct(ctx, op, fldPath, obj, oldObj)...) return - }(fldPath.Child("directComparableStructPtrField"), obj.DirectComparableStructPtr, safe.Field(oldObj, func(oldObj *StructStruct) *DirectComparableStruct { return oldObj.DirectComparableStructPtr }))...) + }(fldPath.Child("directComparableStructPtrField"), obj.DirectComparableStructPtr, safe.Field(oldObj, func(oldObj *StructStruct) *DirectComparableStruct { return oldObj.DirectComparableStructPtr }), oldObj != nil)...) // field StructStruct.NonDirectComparableStructPtr errs = append(errs, - func(fldPath *field.Path, obj, oldObj *NonDirectComparableStruct) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj *NonDirectComparableStruct, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { + if oldValueCorrelated && op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { return nil } // call field-attached validations @@ -500,13 +500,13 @@ func Validate_StructStruct(ctx context.Context, op operation.Operation, fldPath // call the type's validation function errs = append(errs, Validate_NonDirectComparableStruct(ctx, op, fldPath, obj, oldObj)...) return - }(fldPath.Child("nonDirectComparableStructPtrField"), obj.NonDirectComparableStructPtr, safe.Field(oldObj, func(oldObj *StructStruct) *NonDirectComparableStruct { return oldObj.NonDirectComparableStructPtr }))...) + }(fldPath.Child("nonDirectComparableStructPtrField"), obj.NonDirectComparableStructPtr, safe.Field(oldObj, func(oldObj *StructStruct) *NonDirectComparableStruct { return oldObj.NonDirectComparableStructPtr }), oldObj != nil)...) // field StructStruct.DirectComparableStruct errs = append(errs, - func(fldPath *field.Path, obj, oldObj *DirectComparableStruct) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj *DirectComparableStruct, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && (obj == oldObj || (obj != nil && oldObj != nil && *obj == *oldObj)) { + if oldValueCorrelated && op.Type == operation.Update && (obj == oldObj || (obj != nil && oldObj != nil && *obj == *oldObj)) { return nil } // call field-attached validations @@ -514,13 +514,13 @@ func Validate_StructStruct(ctx context.Context, op operation.Operation, fldPath // call the type's validation function errs = append(errs, Validate_DirectComparableStruct(ctx, op, fldPath, obj, oldObj)...) return - }(safe.Value(fldPath, func() *field.Path { return fldPath.Child("DirectComparableStruct") }), &obj.DirectComparableStruct, safe.Field(oldObj, func(oldObj *StructStruct) *DirectComparableStruct { return &oldObj.DirectComparableStruct }))...) + }(safe.Value(fldPath, func() *field.Path { return fldPath.Child("DirectComparableStruct") }), &obj.DirectComparableStruct, safe.Field(oldObj, func(oldObj *StructStruct) *DirectComparableStruct { return &oldObj.DirectComparableStruct }), oldObj != nil)...) // field StructStruct.NonDirectComparableStruct errs = append(errs, - func(fldPath *field.Path, obj, oldObj *NonDirectComparableStruct) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj *NonDirectComparableStruct, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { + if oldValueCorrelated && op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { return nil } // call field-attached validations @@ -528,7 +528,7 @@ func Validate_StructStruct(ctx context.Context, op operation.Operation, fldPath // call the type's validation function errs = append(errs, Validate_NonDirectComparableStruct(ctx, op, fldPath, obj, oldObj)...) return - }(safe.Value(fldPath, func() *field.Path { return fldPath.Child("NonDirectComparableStruct") }), &obj.NonDirectComparableStruct, safe.Field(oldObj, func(oldObj *StructStruct) *NonDirectComparableStruct { return &oldObj.NonDirectComparableStruct }))...) + }(safe.Value(fldPath, func() *field.Path { return fldPath.Child("NonDirectComparableStruct") }), &obj.NonDirectComparableStruct, safe.Field(oldObj, func(oldObj *StructStruct) *NonDirectComparableStruct { return &oldObj.NonDirectComparableStruct }), oldObj != nil)...) return errs } 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 bb5aaeeaead..2621278ee9d 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 @@ -100,8 +100,6 @@ func Test_StructSlice(t *testing.T) { }).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) @@ -113,5 +111,7 @@ func Test_Items(t *testing.T) { Items: []Item{ {Key: "valid1"}, }, - }).ExpectValid() + }).ExpectValidateFalseByPath(map[string][]string{ + "items[0].data": {"field Data"}, + }) } 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 e740378a902..93c0d8aa1bd 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 @@ -64,15 +64,15 @@ func Validate_Item(ctx context.Context, op operation.Operation, fldPath *field.P // field Item.Data errs = append(errs, - func(fldPath *field.Path, obj, oldObj map[string]string) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj map[string]string, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { + if oldValueCorrelated && 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 }))...) + }(fldPath.Child("data"), obj.Data, safe.Field(oldObj, func(oldObj *Item) map[string]string { return oldObj.Data }), oldObj != nil)...) return errs } @@ -84,9 +84,9 @@ func Validate_ItemList(ctx context.Context, op operation.Operation, fldPath *fie // field ItemList.Items errs = append(errs, - func(fldPath *field.Path, obj, oldObj []Item) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj []Item, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { + if oldValueCorrelated && op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { return nil } // call field-attached validations @@ -95,7 +95,7 @@ func Validate_ItemList(ctx context.Context, op operation.Operation, fldPath *fie // 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 }))...) + }(fldPath.Child("items"), obj.Items, safe.Field(oldObj, func(oldObj *ItemList) []Item { return oldObj.Items }), oldObj != nil)...) return errs } @@ -147,9 +147,9 @@ func Validate_StructSlice(ctx context.Context, op operation.Operation, fldPath * // field StructSlice.AtomicSliceStringField errs = append(errs, - func(fldPath *field.Path, obj, oldObj []StringType) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj []StringType, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { + if oldValueCorrelated && op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { return nil } // call field-attached validations @@ -157,13 +157,13 @@ func Validate_StructSlice(ctx context.Context, op operation.Operation, fldPath * return validate.FixedResult(ctx, op, fldPath, obj, oldObj, false, "field AtomicSliceStringField[*]") })...) return - }(fldPath.Child("atomicSliceStringField"), obj.AtomicSliceStringField, safe.Field(oldObj, func(oldObj *StructSlice) []StringType { return oldObj.AtomicSliceStringField }))...) + }(fldPath.Child("atomicSliceStringField"), obj.AtomicSliceStringField, safe.Field(oldObj, func(oldObj *StructSlice) []StringType { return oldObj.AtomicSliceStringField }), oldObj != nil)...) // field StructSlice.AtomicSliceTypeField errs = append(errs, - func(fldPath *field.Path, obj, oldObj IntSliceType) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj IntSliceType, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { + if oldValueCorrelated && op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { return nil } // call field-attached validations @@ -171,13 +171,13 @@ func Validate_StructSlice(ctx context.Context, op operation.Operation, fldPath * return validate.FixedResult(ctx, op, fldPath, obj, oldObj, false, "field AtomicSliceTypeField[*]") })...) return - }(fldPath.Child("atomicSliceTypeField"), obj.AtomicSliceTypeField, safe.Field(oldObj, func(oldObj *StructSlice) IntSliceType { return oldObj.AtomicSliceTypeField }))...) + }(fldPath.Child("atomicSliceTypeField"), obj.AtomicSliceTypeField, safe.Field(oldObj, func(oldObj *StructSlice) IntSliceType { return oldObj.AtomicSliceTypeField }), oldObj != nil)...) // field StructSlice.AtomicSliceComparableField errs = append(errs, - func(fldPath *field.Path, obj, oldObj []ComparableStruct) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj []ComparableStruct, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { + if oldValueCorrelated && op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { return nil } // call field-attached validations @@ -185,13 +185,13 @@ func Validate_StructSlice(ctx context.Context, op operation.Operation, fldPath * return validate.FixedResult(ctx, op, fldPath, obj, oldObj, false, "field AtomicSliceComparableField[*]") })...) return - }(fldPath.Child("atomicSliceComparableField"), obj.AtomicSliceComparableField, safe.Field(oldObj, func(oldObj *StructSlice) []ComparableStruct { return oldObj.AtomicSliceComparableField }))...) + }(fldPath.Child("atomicSliceComparableField"), obj.AtomicSliceComparableField, safe.Field(oldObj, func(oldObj *StructSlice) []ComparableStruct { return oldObj.AtomicSliceComparableField }), oldObj != nil)...) // field StructSlice.AtomicSliceNonComparableField errs = append(errs, - func(fldPath *field.Path, obj, oldObj []NonComparableStruct) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj []NonComparableStruct, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { + if oldValueCorrelated && op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { return nil } // call field-attached validations @@ -201,13 +201,13 @@ func Validate_StructSlice(ctx context.Context, op operation.Operation, fldPath * // iterate the list and call the type's validation function errs = append(errs, validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, nil, nil, Validate_NonComparableStruct)...) return - }(fldPath.Child("atomicSliceNonComparableField"), obj.AtomicSliceNonComparableField, safe.Field(oldObj, func(oldObj *StructSlice) []NonComparableStruct { return oldObj.AtomicSliceNonComparableField }))...) + }(fldPath.Child("atomicSliceNonComparableField"), obj.AtomicSliceNonComparableField, safe.Field(oldObj, func(oldObj *StructSlice) []NonComparableStruct { return oldObj.AtomicSliceNonComparableField }), oldObj != nil)...) // field StructSlice.SetSliceComparableField errs = append(errs, - func(fldPath *field.Path, obj, oldObj []ComparableStruct) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj []ComparableStruct, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { + if oldValueCorrelated && op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { return nil } // call field-attached validations @@ -217,13 +217,13 @@ func Validate_StructSlice(ctx context.Context, op operation.Operation, fldPath * // lists with set semantics require unique values errs = append(errs, validate.Unique(ctx, op, fldPath, obj, oldObj, validate.DirectEqual)...) return - }(fldPath.Child("setSliceComparableField"), obj.SetSliceComparableField, safe.Field(oldObj, func(oldObj *StructSlice) []ComparableStruct { return oldObj.SetSliceComparableField }))...) + }(fldPath.Child("setSliceComparableField"), obj.SetSliceComparableField, safe.Field(oldObj, func(oldObj *StructSlice) []ComparableStruct { return oldObj.SetSliceComparableField }), oldObj != nil)...) // field StructSlice.SetSliceNonComparableField errs = append(errs, - func(fldPath *field.Path, obj, oldObj []NonComparableStruct) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj []NonComparableStruct, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { + if oldValueCorrelated && op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { return nil } // call field-attached validations @@ -235,13 +235,13 @@ func Validate_StructSlice(ctx context.Context, op operation.Operation, fldPath * // iterate the list and call the type's validation function errs = append(errs, validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, validate.SemanticDeepEqual, nil, Validate_NonComparableStruct)...) return - }(fldPath.Child("setSliceNonComparableField"), obj.SetSliceNonComparableField, safe.Field(oldObj, func(oldObj *StructSlice) []NonComparableStruct { return oldObj.SetSliceNonComparableField }))...) + }(fldPath.Child("setSliceNonComparableField"), obj.SetSliceNonComparableField, safe.Field(oldObj, func(oldObj *StructSlice) []NonComparableStruct { return oldObj.SetSliceNonComparableField }), oldObj != nil)...) // field StructSlice.MapSliceComparableField errs = append(errs, - func(fldPath *field.Path, obj, oldObj []ComparableStructWithKey) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj []ComparableStructWithKey, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { + if oldValueCorrelated && op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { return nil } // call field-attached validations @@ -251,13 +251,13 @@ func Validate_StructSlice(ctx context.Context, op operation.Operation, fldPath * // lists with map semantics require unique keys errs = append(errs, validate.Unique(ctx, op, fldPath, obj, oldObj, func(a ComparableStructWithKey, b ComparableStructWithKey) bool { return a.Key == b.Key })...) return - }(fldPath.Child("mapSliceComparableField"), obj.MapSliceComparableField, safe.Field(oldObj, func(oldObj *StructSlice) []ComparableStructWithKey { return oldObj.MapSliceComparableField }))...) + }(fldPath.Child("mapSliceComparableField"), obj.MapSliceComparableField, safe.Field(oldObj, func(oldObj *StructSlice) []ComparableStructWithKey { return oldObj.MapSliceComparableField }), oldObj != nil)...) // field StructSlice.MapSliceNonComparableField errs = append(errs, - func(fldPath *field.Path, obj, oldObj []NonComparableStructWithKey) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj []NonComparableStructWithKey, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { + if oldValueCorrelated && op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { return nil } // call field-attached validations @@ -269,13 +269,13 @@ func Validate_StructSlice(ctx context.Context, op operation.Operation, fldPath * // iterate the list and call the type's validation function errs = append(errs, validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, func(a NonComparableStructWithKey, b NonComparableStructWithKey) bool { return a.Key == b.Key }, validate.SemanticDeepEqual, Validate_NonComparableStructWithKey)...) return - }(fldPath.Child("mapSliceNonComparableField"), obj.MapSliceNonComparableField, safe.Field(oldObj, func(oldObj *StructSlice) []NonComparableStructWithKey { return oldObj.MapSliceNonComparableField }))...) + }(fldPath.Child("mapSliceNonComparableField"), obj.MapSliceNonComparableField, safe.Field(oldObj, func(oldObj *StructSlice) []NonComparableStructWithKey { return oldObj.MapSliceNonComparableField }), oldObj != nil)...) // field StructSlice.MapSlicePtrKeyField errs = append(errs, - func(fldPath *field.Path, obj, oldObj []PtrKeyStruct) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj []PtrKeyStruct, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { + if oldValueCorrelated && op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { return nil } // call field-attached validations @@ -293,13 +293,13 @@ func Validate_StructSlice(ctx context.Context, op operation.Operation, fldPath * return ((a.Key == nil && b.Key == nil) || (a.Key != nil && b.Key != nil && *a.Key == *b.Key)) }, validate.SemanticDeepEqual, Validate_PtrKeyStruct)...) return - }(fldPath.Child("mapSlicePtrKeyField"), obj.MapSlicePtrKeyField, safe.Field(oldObj, func(oldObj *StructSlice) []PtrKeyStruct { return oldObj.MapSlicePtrKeyField }))...) + }(fldPath.Child("mapSlicePtrKeyField"), obj.MapSlicePtrKeyField, safe.Field(oldObj, func(oldObj *StructSlice) []PtrKeyStruct { return oldObj.MapSlicePtrKeyField }), oldObj != nil)...) // field StructSlice.MapSliceMixedKeyField errs = append(errs, - func(fldPath *field.Path, obj, oldObj []MixedKeyStruct) (errs field.ErrorList) { + func(fldPath *field.Path, obj, oldObj []MixedKeyStruct, oldValueCorrelated bool) (errs field.ErrorList) { // don't revalidate unchanged data - if op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { + if oldValueCorrelated && op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { return nil } // call field-attached validations @@ -317,7 +317,7 @@ func Validate_StructSlice(ctx context.Context, op operation.Operation, fldPath * return ((a.Key1 == nil && b.Key1 == nil) || (a.Key1 != nil && b.Key1 != nil && *a.Key1 == *b.Key1)) && a.Key2 == b.Key2 }, validate.SemanticDeepEqual, Validate_MixedKeyStruct)...) return - }(fldPath.Child("mapSliceMixedKeyField"), obj.MapSliceMixedKeyField, safe.Field(oldObj, func(oldObj *StructSlice) []MixedKeyStruct { return oldObj.MapSliceMixedKeyField }))...) + }(fldPath.Child("mapSliceMixedKeyField"), obj.MapSliceMixedKeyField, safe.Field(oldObj, func(oldObj *StructSlice) []MixedKeyStruct { return oldObj.MapSliceMixedKeyField }), oldObj != nil)...) return errs } diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/validation.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/validation.go index ee3e3cda3b7..c755706edb6 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/validation.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/validation.go @@ -1193,7 +1193,7 @@ func (g *genValidations) emitValidationForChild(c *generator.Context, thisChild } sw.Do("// field $.inType|raw$.$.fieldName$\n", targs) sw.Do("errs = append(errs,\n", targs) - sw.Do(" func(fldPath *$.field.Path|raw$, obj, oldObj $.fieldTypePfx$$.fieldType|raw$) (errs $.field.ErrorList|raw$) {\n", targs) + sw.Do(" func(fldPath *$.field.Path|raw$, obj, oldObj $.fieldTypePfx$$.fieldType|raw$, oldValueCorrelated bool) (errs $.field.ErrorList|raw$) {\n", targs) if err := sw.Merge(buf, bufsw); err != nil { panic(fmt.Sprintf("failed to merge buffer: %v", err)) } @@ -1207,10 +1207,28 @@ func (g *genValidations) emitValidationForChild(c *generator.Context, thisChild sw.Do("$.safe.Value|raw$(fldPath, func() *$.field.Path|raw$ { return fldPath.Child(\"$.fieldType|raw$\") }), ", targs) } sw.Do(" $.fieldExprPfx$obj.$.fieldName$, ", targs) + // safe.Field returns a nil if the old object does not have a correlatable + // value, such as a map. + // This is ambiguous with the case where the field exists and is nil. + // This ambiguity is a problem for ratcheting, which needs to distinguish + // these cases. For example, if a required field is removed from a map, + // safe.Field will return nil for the old value, and the new value is also + // nil (because it doesn't exist). Ratcheting would normally allow this, + // but it's a validation failure because a required field is missing. + // + // To solve this, we pass an extra boolean parameter to the validation + // function, indicating whether the old value was correlated. If the old + // value was uncorrelated, it means the field was not present in the old + // object, and we should not apply ratcheting logic. `oldObj != nil` + // provides this bit of information. + // + // This bit is not currently propagated down to deeper levels of + // validation, but since the code generator only ever looks one level + // down, this is sufficient for now. sw.Do(" $.safe.Field|raw$(oldObj, ", targs) sw.Do(" func(oldObj *$.inType|raw$) $.fieldTypePfx$$.fieldType|raw$ {", targs) sw.Do(" return $.fieldExprPfx$oldObj.$.fieldName$", targs) - sw.Do(" }),", targs) + sw.Do(" }), oldObj != nil", targs) sw.Do(" )...)\n", targs) sw.Do("\n", nil) } else { @@ -1257,10 +1275,10 @@ func emitRatchetingCheck(c *generator.Context, t *types.Type, sw *generator.Snip // - obj != nil : handle optional fields which are updated to nil // - oldObj != nil : handle optional fields which are updated from nil // - *obj == *oldObj : compare values - sw.Do("if op.Type == $.operation.Update|raw$ && (obj == oldObj || (obj != nil && oldObj != nil && *obj == *oldObj)) {\n", targs) + sw.Do("if oldValueCorrelated && op.Type == $.operation.Update|raw$ && (obj == oldObj || (obj != nil && oldObj != nil && *obj == *oldObj)) {\n", targs) } else { targs["equality"] = mkSymbolArgs(c, equalityPkgSymbols) - sw.Do("if op.Type == $.operation.Update|raw$ && $.equality.Semantic|raw$.DeepEqual(obj, oldObj) {\n", targs) + sw.Do("if oldValueCorrelated && op.Type == $.operation.Update|raw$ && $.equality.Semantic|raw$.DeepEqual(obj, oldObj) {\n", targs) } sw.Do(" return nil\n", nil) sw.Do("}\n", nil)