From 10473da4f7012953dca9f32bc8630aba32ae3a56 Mon Sep 17 00:00:00 2001 From: Aaron Prindle Date: Wed, 1 Oct 2025 23:50:04 +0000 Subject: [PATCH] chore: add +k8s:immutable tag implementation and test --- .../pkg/api/validate/immutable.go | 42 +--- .../pkg/api/validate/immutable_test.go | 230 +++--------------- .../output_tests/tags/immutable/doc_test.go | 22 +- .../immutable/zz_generated.validations.go | 45 +++- .../item/immutable_transitions/doc_test.go | 8 +- .../zz_generated.validations.go | 14 +- .../tags/item/typedef/doc_test.go | 2 +- .../item/typedef/zz_generated.validations.go | 5 +- .../tags/listmap/multiple_keys/doc_test.go | 30 +-- .../multiple_keys/zz_generated.validations.go | 21 +- .../tags/listmap/single_key/doc_test.go | 30 +-- .../single_key/zz_generated.validations.go | 15 +- .../tags/listset/zz_generated.validations.go | 35 ++- .../primitive_pointers/doc_test.go | 8 +- .../zz_generated.validations.go | 20 +- .../update_validations/primitives/doc_test.go | 8 +- .../primitives/zz_generated.validations.go | 20 +- .../validation-gen/validators/immutable.go | 17 +- 18 files changed, 231 insertions(+), 341 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/api/validate/immutable.go b/staging/src/k8s.io/apimachinery/pkg/api/validate/immutable.go index 5a9d3da2ec2..01a879c98f6 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/validate/immutable.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/validate/immutable.go @@ -19,46 +19,22 @@ package validate import ( "context" - "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/operation" "k8s.io/apimachinery/pkg/util/validation/field" ) -// ImmutableByCompare verifies that the specified value has not changed in the -// course of an update operation. It does nothing if the old value is not -// provided. If the caller needs to compare types that are not trivially -// comparable, they should use ImmutableByReflect instead. +// Immutable verifies that the specified value has not changed in the course of +// an update operation. It does nothing if the old value is not provided. // -// Caution: structs with pointer fields satisfy comparable, but this function -// will only compare pointer values. It does not compare the pointed-to -// values. -func ImmutableByCompare[T comparable](_ context.Context, op operation.Operation, fldPath *field.Path, value, oldValue *T) field.ErrorList { +// This function unconditionally returns a validation error as it +// relies on the default ratcheting mechanism to only be called when a +// change to the field has already been detected. This avoids a redundant +// equivalence check across ratcheting and this function. +func Immutable[T any](_ context.Context, op operation.Operation, fldPath *field.Path, _, _ T) field.ErrorList { if op.Type != operation.Update { return nil } - if value == nil && oldValue == nil { - return nil + return field.ErrorList{ + field.Invalid(fldPath, nil, "field is immutable").WithOrigin("immutable"), } - if value == nil || oldValue == nil || *value != *oldValue { - return field.ErrorList{ - field.Forbidden(fldPath, "field is immutable"), - } - } - return nil -} - -// ImmutableByReflect verifies that the specified value has not changed in -// the course of an update operation. It does nothing if the old value is not -// provided. Unlike ImmutableByCompare, this function can be used with types that are -// not directly comparable, at the cost of performance. -func ImmutableByReflect[T any](_ context.Context, op operation.Operation, fldPath *field.Path, value, oldValue T) field.ErrorList { - if op.Type != operation.Update { - return nil - } - if !equality.Semantic.DeepEqual(value, oldValue) { - return field.ErrorList{ - field.Forbidden(fldPath, "field is immutable"), - } - } - return nil } diff --git a/staging/src/k8s.io/apimachinery/pkg/api/validate/immutable_test.go b/staging/src/k8s.io/apimachinery/pkg/api/validate/immutable_test.go index c9edece2842..60396b0cc64 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/validate/immutable_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/validate/immutable_test.go @@ -25,225 +25,51 @@ import ( "k8s.io/utils/ptr" ) -type StructComparable struct { - S string - I int - B bool -} +func TestImmutable(t *testing.T) { + // The Immutable function relies on validation ratcheting to avoid being + // called when old and new values are equivalent. This unit test only needs + // to confirm two behaviors: + // 1. The function does nothing for non-update operations (e.g., create). + // 2. The function *always* returns an error for update operations, since + // ratcheting should have prevented the call if the values were unchanged. -func TestImmutableByCompare(t *testing.T) { - structA := StructComparable{"abc", 123, true} - structA2 := structA - structB := StructComparable{"xyz", 456, false} - - for _, tc := range []struct { - name string - fn func(operation.Operation, *field.Path) field.ErrorList - fail bool - }{{ - name: "nil both values", - fn: func(op operation.Operation, fld *field.Path) field.ErrorList { - return ImmutableByCompare[int](context.Background(), op, fld, nil, nil) - }, - }, { - name: "nil value", - fn: func(op operation.Operation, fld *field.Path) field.ErrorList { - return ImmutableByCompare(context.Background(), op, fld, nil, ptr.To(123)) - }, - fail: true, - }, { - name: "nil oldValue", - fn: func(op operation.Operation, fld *field.Path) field.ErrorList { - return ImmutableByCompare(context.Background(), op, fld, ptr.To(123), nil) - }, - fail: true, - }, { - name: "int", - fn: func(op operation.Operation, fld *field.Path) field.ErrorList { - return ImmutableByCompare(context.Background(), op, fld, ptr.To(123), ptr.To(123)) - }, - }, { - name: "int fail", - fn: func(op operation.Operation, fld *field.Path) field.ErrorList { - return ImmutableByCompare(context.Background(), op, fld, ptr.To(123), ptr.To(456)) - }, - fail: true, - }, { - name: "string", - fn: func(op operation.Operation, fld *field.Path) field.ErrorList { - return ImmutableByCompare(context.Background(), op, fld, ptr.To("abc"), ptr.To("abc")) - }, - }, { - name: "string fail", - fn: func(op operation.Operation, fld *field.Path) field.ErrorList { - return ImmutableByCompare(context.Background(), op, fld, ptr.To("abc"), ptr.To("xyz")) - }, - fail: true, - }, { - name: "bool", - fn: func(op operation.Operation, fld *field.Path) field.ErrorList { - return ImmutableByCompare(context.Background(), op, fld, ptr.To(true), ptr.To(true)) - }, - }, { - name: "bool fail", - fn: func(op operation.Operation, fld *field.Path) field.ErrorList { - return ImmutableByCompare(context.Background(), op, fld, ptr.To(true), ptr.To(false)) - }, - fail: true, - }, { - name: "same struct", - fn: func(op operation.Operation, fld *field.Path) field.ErrorList { - return ImmutableByCompare(context.Background(), op, fld, ptr.To(structA), ptr.To(structA)) - }, - }, { - name: "equal struct", - fn: func(op operation.Operation, fld *field.Path) field.ErrorList { - return ImmutableByCompare(context.Background(), op, fld, ptr.To(structA), ptr.To(structA2)) - }, - }, { - name: "struct fail", - fn: func(op operation.Operation, fld *field.Path) field.ErrorList { - return ImmutableByCompare(context.Background(), op, fld, ptr.To(structA), ptr.To(structB)) - }, - fail: true, - }} { - t.Run(tc.name, func(t *testing.T) { - errs := tc.fn(operation.Operation{Type: operation.Create}, field.NewPath("")) - if len(errs) != 0 { // Create should always succeed - t.Errorf("case %q (create): expected success: %v", tc.name, errs) - } - errs = tc.fn(operation.Operation{Type: operation.Update}, field.NewPath("")) - if tc.fail && len(errs) == 0 { - t.Errorf("case %q (update): expected failure", tc.name) - } else if !tc.fail && len(errs) != 0 { - t.Errorf("case %q (update): expected success: %v", tc.name, errs) - } - }) - } -} - -type StructNonComparable struct { - S string - SP *string - I int - IP *int - B bool - BP *bool - SS []string - MSS map[string]string -} - -func TestImmutableByReflect(t *testing.T) { - structA := StructNonComparable{ - S: "abc", - SP: ptr.To("abc"), - I: 123, - IP: ptr.To(123), - B: true, - BP: ptr.To(true), - SS: []string{"a", "b", "c"}, - MSS: map[string]string{"a": "b", "c": "d"}, - } - - structA2 := structA - structA2.SP = ptr.To("abc") - structA2.IP = ptr.To(123) - structA2.BP = ptr.To(true) - structA2.SS = []string{"a", "b", "c"} - structA2.MSS = map[string]string{"a": "b", "c": "d"} - - structB := StructNonComparable{ - S: "xyz", - SP: ptr.To("xyz"), - I: 456, - IP: ptr.To(456), - B: false, - BP: ptr.To(false), - SS: []string{"x", "y", "z"}, - MSS: map[string]string{"x": "X", "y": "Y"}, + type simpleStruct struct { + S string } for _, tc := range []struct { name string - fn func(operation.Operation, *field.Path) field.ErrorList - fail bool + fn func(op operation.Operation, fldPath *field.Path) field.ErrorList }{{ - name: "nil both values", + name: "with primitive type", fn: func(op operation.Operation, fld *field.Path) field.ErrorList { - return ImmutableByReflect[*int](context.Background(), op, fld, nil, nil) + return Immutable(context.Background(), op, fld, ptr.To(123), ptr.To(456)) }, }, { - name: "nil value", + name: "with struct type", fn: func(op operation.Operation, fld *field.Path) field.ErrorList { - return ImmutableByReflect(context.Background(), op, fld, nil, ptr.To(123)) - }, - fail: true, - }, { - name: "nil oldValue", - fn: func(op operation.Operation, fld *field.Path) field.ErrorList { - return ImmutableByReflect(context.Background(), op, fld, ptr.To(123), nil) - }, - fail: true, - }, { - name: "int", - fn: func(op operation.Operation, fld *field.Path) field.ErrorList { - return ImmutableByReflect(context.Background(), op, fld, ptr.To(123), ptr.To(123)) + return Immutable(context.Background(), op, fld, &simpleStruct{S: "a"}, &simpleStruct{S: "b"}) }, }, { - name: "int fail", + name: "with nil values", fn: func(op operation.Operation, fld *field.Path) field.ErrorList { - return ImmutableByReflect(context.Background(), op, fld, ptr.To(123), ptr.To(456)) + // Explicitly type the nil to satisfy the generic function signature. + return Immutable[*int](context.Background(), op, fld, nil, nil) }, - fail: true, - }, { - name: "string", - fn: func(op operation.Operation, fld *field.Path) field.ErrorList { - return ImmutableByReflect(context.Background(), op, fld, ptr.To("abc"), ptr.To("abc")) - }, - }, { - name: "string fail", - fn: func(op operation.Operation, fld *field.Path) field.ErrorList { - return ImmutableByReflect(context.Background(), op, fld, ptr.To("abc"), ptr.To("xyz")) - }, - fail: true, - }, { - name: "bool", - fn: func(op operation.Operation, fld *field.Path) field.ErrorList { - return ImmutableByReflect(context.Background(), op, fld, ptr.To(true), ptr.To(true)) - }, - }, { - name: "bool fail", - fn: func(op operation.Operation, fld *field.Path) field.ErrorList { - return ImmutableByReflect(context.Background(), op, fld, ptr.To(true), ptr.To(false)) - }, - fail: true, - }, { - name: "same struct", - fn: func(op operation.Operation, fld *field.Path) field.ErrorList { - return ImmutableByReflect(context.Background(), op, fld, ptr.To(structA), ptr.To(structA)) - }, - }, { - name: "equal struct", - fn: func(op operation.Operation, fld *field.Path) field.ErrorList { - return ImmutableByReflect(context.Background(), op, fld, ptr.To(structA), ptr.To(structA2)) - }, - }, { - name: "struct fail", - fn: func(op operation.Operation, fld *field.Path) field.ErrorList { - return ImmutableByReflect(context.Background(), op, fld, ptr.To(structA), ptr.To(structB)) - }, - fail: true, }} { t.Run(tc.name, func(t *testing.T) { - errs := tc.fn(operation.Operation{Type: operation.Create}, field.NewPath("")) - if len(errs) != 0 { // Create should always succeed - t.Errorf("case %q (create): expected success: %v", tc.name, errs) + // Create operations should never return an error. + errs := tc.fn(operation.Operation{Type: operation.Create}, field.NewPath("field")) + if len(errs) != 0 { + t.Errorf("expected success for create operation, but got errors: %v", errs) } - errs = tc.fn(operation.Operation{Type: operation.Update}, field.NewPath("")) - if tc.fail && len(errs) == 0 { - t.Errorf("case %q (update): expected failure", tc.name) - } else if !tc.fail && len(errs) != 0 { - t.Errorf("case %q (update): expected success: %v", tc.name, errs) + + // Update operations should always return exactly one error. + errs = tc.fn(operation.Operation{Type: operation.Update}, field.NewPath("field")) + if len(errs) == 0 { + t.Errorf("expected a failure for update operation, but got success") + } else if len(errs) > 1 { + t.Errorf("expected exactly one error for update operation, but got %d: %v", len(errs), errs) } }) } diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/immutable/doc_test.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/immutable/doc_test.go index 100e349add7..139f8ecc351 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/immutable/doc_test.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/immutable/doc_test.go @@ -64,16 +64,16 @@ func Test(t *testing.T) { st.Value(&structA).OldValue(&structA2).ExpectValid() st.Value(&structA2).OldValue(&structA).ExpectValid() - st.Value(&structA).OldValue(&structB).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ - field.Forbidden(field.NewPath("stringField"), "immutable"), - field.Forbidden(field.NewPath("stringPtrField"), "immutable"), - field.Forbidden(field.NewPath("structField"), "immutable"), - field.Forbidden(field.NewPath("structPtrField"), "immutable"), - field.Forbidden(field.NewPath("noncomparableStructField"), "immutable"), - field.Forbidden(field.NewPath("noncomparableStructPtrField"), "immutable"), - field.Forbidden(field.NewPath("sliceField"), "immutable"), - field.Forbidden(field.NewPath("mapField"), "immutable"), - field.Forbidden(field.NewPath("immutableField"), "immutable"), - field.Forbidden(field.NewPath("immutablePtrField"), "immutable"), + st.Value(&structA).OldValue(&structB).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByOrigin(), field.ErrorList{ + field.Invalid(field.NewPath("stringField"), nil, "").WithOrigin("immutable"), + field.Invalid(field.NewPath("stringPtrField"), nil, "").WithOrigin("immutable"), + field.Invalid(field.NewPath("structField"), nil, "").WithOrigin("immutable"), + field.Invalid(field.NewPath("structPtrField"), nil, "").WithOrigin("immutable"), + field.Invalid(field.NewPath("noncomparableStructField"), nil, "").WithOrigin("immutable"), + field.Invalid(field.NewPath("noncomparableStructPtrField"), nil, "").WithOrigin("immutable"), + field.Invalid(field.NewPath("sliceField"), nil, "").WithOrigin("immutable"), + field.Invalid(field.NewPath("mapField"), nil, "").WithOrigin("immutable"), + field.Invalid(field.NewPath("immutableField"), nil, "").WithOrigin("immutable"), + field.Invalid(field.NewPath("immutablePtrField"), nil, "").WithOrigin("immutable"), }) } diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/immutable/zz_generated.validations.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/immutable/zz_generated.validations.go index 2d44426e290..1acca1831f4 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/immutable/zz_generated.validations.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/immutable/zz_generated.validations.go @@ -52,7 +52,10 @@ func RegisterValidations(scheme *testscheme.Scheme) error { // Validate_ImmutableType validates an instance of ImmutableType according // to declarative validation rules in the API schema. func Validate_ImmutableType(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *ImmutableType) (errs field.ErrorList) { - errs = append(errs, validate.ImmutableByCompare(ctx, op, fldPath, obj, oldObj)...) + if e := validate.Immutable(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } return errs } @@ -70,7 +73,10 @@ func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field return nil } // call field-attached validations - errs = append(errs, validate.ImmutableByCompare(ctx, op, fldPath, obj, oldObj)...) + if e := validate.Immutable(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } return }(fldPath.Child("stringField"), &obj.StringField, safe.Field(oldObj, func(oldObj *Struct) *string { return &oldObj.StringField }))...) @@ -82,7 +88,10 @@ func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field return nil } // call field-attached validations - errs = append(errs, validate.ImmutableByCompare(ctx, op, fldPath, obj, oldObj)...) + if e := validate.Immutable(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } return }(fldPath.Child("stringPtrField"), obj.StringPtrField, safe.Field(oldObj, func(oldObj *Struct) *string { return oldObj.StringPtrField }))...) @@ -94,7 +103,10 @@ func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field return nil } // call field-attached validations - errs = append(errs, validate.ImmutableByReflect(ctx, op, fldPath, obj, oldObj)...) + if e := validate.Immutable(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } return }(fldPath.Child("structField"), &obj.StructField, safe.Field(oldObj, func(oldObj *Struct) *ComparableStruct { return &oldObj.StructField }))...) @@ -106,7 +118,10 @@ func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field return nil } // call field-attached validations - errs = append(errs, validate.ImmutableByReflect(ctx, op, fldPath, obj, oldObj)...) + if e := validate.Immutable(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } return }(fldPath.Child("structPtrField"), obj.StructPtrField, safe.Field(oldObj, func(oldObj *Struct) *ComparableStruct { return oldObj.StructPtrField }))...) @@ -118,7 +133,10 @@ func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field return nil } // call field-attached validations - errs = append(errs, validate.ImmutableByReflect(ctx, op, fldPath, obj, oldObj)...) + if e := validate.Immutable(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } return }(fldPath.Child("noncomparableStructField"), &obj.NonComparableStructField, safe.Field(oldObj, func(oldObj *Struct) *NonComparableStruct { return &oldObj.NonComparableStructField }))...) @@ -130,7 +148,10 @@ func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field return nil } // call field-attached validations - errs = append(errs, validate.ImmutableByReflect(ctx, op, fldPath, obj, oldObj)...) + if e := validate.Immutable(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } return }(fldPath.Child("noncomparableStructPtrField"), obj.NonComparableStructPtrField, safe.Field(oldObj, func(oldObj *Struct) *NonComparableStruct { return oldObj.NonComparableStructPtrField }))...) @@ -142,7 +163,10 @@ func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field return nil } // call field-attached validations - errs = append(errs, validate.ImmutableByReflect(ctx, op, fldPath, obj, oldObj)...) + if e := validate.Immutable(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } return }(fldPath.Child("sliceField"), obj.SliceField, safe.Field(oldObj, func(oldObj *Struct) []string { return oldObj.SliceField }))...) @@ -154,7 +178,10 @@ func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field return nil } // call field-attached validations - errs = append(errs, validate.ImmutableByReflect(ctx, op, fldPath, obj, oldObj)...) + if e := validate.Immutable(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } return }(fldPath.Child("mapField"), obj.MapField, safe.Field(oldObj, func(oldObj *Struct) map[string]string { return oldObj.MapField }))...) diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/item/immutable_transitions/doc_test.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/item/immutable_transitions/doc_test.go index 467392461e2..f6b86edd8ee 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/item/immutable_transitions/doc_test.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/item/immutable_transitions/doc_test.go @@ -42,13 +42,13 @@ func Test(t *testing.T) { } st.Value(new).OldValue(old).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ - field.Forbidden(field.NewPath("listField").Index(0), "immutable"), - field.Forbidden(field.NewPath("listField").Index(1).Child("stringField"), "immutable"), + field.Invalid(field.NewPath("listField").Index(0), nil, "immutable").WithOrigin("immutable"), + field.Invalid(field.NewPath("listField").Index(1).Child("stringField"), nil, "immutable").WithOrigin("immutable"), }) st.Value(new).OldValue(&Struct{ListField: []Item{}}).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ - field.Forbidden(field.NewPath("listField").Index(0), "immutable"), - field.Forbidden(field.NewPath("listField").Index(1).Child("stringField"), "immutable"), + field.Invalid(field.NewPath("listField").Index(0), nil, "immutable").WithOrigin("immutable"), + field.Invalid(field.NewPath("listField").Index(1).Child("stringField"), nil, "immutable").WithOrigin("immutable"), }) // Test that "c" can change independently diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/item/immutable_transitions/zz_generated.validations.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/item/immutable_transitions/zz_generated.validations.go index 1f59168f698..c385d2efede 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/item/immutable_transitions/zz_generated.validations.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/item/immutable_transitions/zz_generated.validations.go @@ -65,12 +65,18 @@ func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field // 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.Key1 == b.Key1 })...) func() { // cohort {"key1": "a"} - errs = append(errs, validate.SliceItem(ctx, op, fldPath, obj, oldObj, func(item *Item) bool { return item.Key1 == "a" }, validate.DirectEqual, validate.ImmutableByCompare)...) + if e := validate.SliceItem(ctx, op, fldPath, obj, oldObj, func(item *Item) bool { return item.Key1 == "a" }, validate.DirectEqual, validate.Immutable); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } }() func() { // cohort {"key1": "b"} - errs = append(errs, validate.SliceItem(ctx, op, fldPath, obj, oldObj, func(item *Item) bool { return item.Key1 == "b" }, validate.DirectEqual, func(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *Item) field.ErrorList { - return validate.Subfield(ctx, op, fldPath, obj, oldObj, "stringField", func(o *Item) *string { return &o.StringField }, validate.DirectEqualPtr, validate.ImmutableByCompare) - })...) + if e := validate.SliceItem(ctx, op, fldPath, obj, oldObj, func(item *Item) bool { return item.Key1 == "b" }, validate.DirectEqual, func(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *Item) field.ErrorList { + return validate.Subfield(ctx, op, fldPath, obj, oldObj, "stringField", func(o *Item) *string { return &o.StringField }, validate.DirectEqualPtr, validate.Immutable) + }); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } }() return }(fldPath.Child("listField"), obj.ListField, safe.Field(oldObj, func(oldObj *Struct) []Item { return oldObj.ListField }))...) diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/item/typedef/doc_test.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/item/typedef/doc_test.go index 43dc94afe9a..d823a2f361e 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/item/typedef/doc_test.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/item/typedef/doc_test.go @@ -53,7 +53,7 @@ func Test(t *testing.T) { }, } st.Value(newStruct).OldValue(oldStruct).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ - field.Forbidden(field.NewPath("typedefItems").Index(0), "immutable"), + field.Invalid(field.NewPath("typedefItems").Index(0), nil, "immutable").WithOrigin("immutable"), }) // Test nested typedef (typedef of typedef). diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/item/typedef/zz_generated.validations.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/item/typedef/zz_generated.validations.go index b4ccddc9755..98b38b37558 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/item/typedef/zz_generated.validations.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/item/typedef/zz_generated.validations.go @@ -83,7 +83,10 @@ func Validate_ItemList(ctx context.Context, op operation.Operation, fldPath *fie // 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 })...) func() { // cohort {"key": "immutable"} - errs = append(errs, validate.SliceItem(ctx, op, fldPath, obj, oldObj, func(item *Item) bool { return item.Key == "immutable" }, validate.DirectEqual, validate.ImmutableByCompare)...) + if e := validate.SliceItem(ctx, op, fldPath, obj, oldObj, func(item *Item) bool { return item.Key == "immutable" }, validate.DirectEqual, validate.Immutable); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } }() func() { // cohort {"key": "validated"} errs = append(errs, validate.SliceItem(ctx, op, fldPath, obj, oldObj, func(item *Item) bool { return item.Key == "validated" }, validate.DirectEqual, func(ctx context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj *Item) field.ErrorList { diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/listmap/multiple_keys/doc_test.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/listmap/multiple_keys/doc_test.go index 2787b0118a8..c11f88c17c8 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/listmap/multiple_keys/doc_test.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/listmap/multiple_keys/doc_test.go @@ -106,24 +106,24 @@ func TestUpdateCorrelation(t *testing.T) { st.Value(&structA2).OldValue(&structA1).ExpectValid() st.Value(&structA1).OldValue(&structB).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ - field.Forbidden(field.NewPath("listField").Index(0), "immutable"), - field.Forbidden(field.NewPath("listField").Index(1), "immutable"), - field.Forbidden(field.NewPath("listTypedefField").Index(0), "immutable"), - field.Forbidden(field.NewPath("listTypedefField").Index(1), "immutable"), - field.Forbidden(field.NewPath("typedefField").Index(0), "immutable"), - field.Forbidden(field.NewPath("typedefField").Index(1), "immutable"), + field.Invalid(field.NewPath("listField").Index(0), nil, "immutable").WithOrigin("immutable"), + field.Invalid(field.NewPath("listField").Index(1), nil, "immutable").WithOrigin("immutable"), + field.Invalid(field.NewPath("listTypedefField").Index(0), nil, "immutable").WithOrigin("immutable"), + field.Invalid(field.NewPath("listTypedefField").Index(1), nil, "immutable").WithOrigin("immutable"), + field.Invalid(field.NewPath("typedefField").Index(0), nil, "immutable").WithOrigin("immutable"), + field.Invalid(field.NewPath("typedefField").Index(1), nil, "immutable").WithOrigin("immutable"), }) st.Value(&structB).OldValue(&structA1).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ - field.Forbidden(field.NewPath("listField").Index(0), "immutable"), - field.Forbidden(field.NewPath("listField").Index(1), "immutable"), - field.Forbidden(field.NewPath("listField").Index(2), "immutable"), - field.Forbidden(field.NewPath("listTypedefField").Index(0), "immutable"), - field.Forbidden(field.NewPath("listTypedefField").Index(1), "immutable"), - field.Forbidden(field.NewPath("listTypedefField").Index(2), "immutable"), - field.Forbidden(field.NewPath("typedefField").Index(0), "immutable"), - field.Forbidden(field.NewPath("typedefField").Index(1), "immutable"), - field.Forbidden(field.NewPath("typedefField").Index(2), "immutable"), + field.Invalid(field.NewPath("listField").Index(0), nil, "immutable").WithOrigin("immutable"), + field.Invalid(field.NewPath("listField").Index(1), nil, "immutable").WithOrigin("immutable"), + field.Invalid(field.NewPath("listField").Index(2), nil, "immutable").WithOrigin("immutable"), + field.Invalid(field.NewPath("listTypedefField").Index(0), nil, "immutable").WithOrigin("immutable"), + field.Invalid(field.NewPath("listTypedefField").Index(1), nil, "immutable").WithOrigin("immutable"), + field.Invalid(field.NewPath("listTypedefField").Index(2), nil, "immutable").WithOrigin("immutable"), + field.Invalid(field.NewPath("typedefField").Index(0), nil, "immutable").WithOrigin("immutable"), + field.Invalid(field.NewPath("typedefField").Index(1), nil, "immutable").WithOrigin("immutable"), + field.Invalid(field.NewPath("typedefField").Index(2), nil, "immutable").WithOrigin("immutable"), }) } diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/listmap/multiple_keys/zz_generated.validations.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/listmap/multiple_keys/zz_generated.validations.go index f930f28dc92..648aff61e06 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/listmap/multiple_keys/zz_generated.validations.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/listmap/multiple_keys/zz_generated.validations.go @@ -73,9 +73,12 @@ func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field return nil } // call field-attached validations - errs = append(errs, validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, func(a OtherStruct, b OtherStruct) bool { + if e := validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, func(a OtherStruct, b OtherStruct) bool { return a.Key1Field == b.Key1Field && a.Key2Field == b.Key2Field - }, validate.DirectEqual, validate.ImmutableByCompare)...) + }, validate.DirectEqual, validate.Immutable); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } // lists with map semantics require unique keys errs = append(errs, validate.Unique(ctx, op, fldPath, obj, oldObj, func(a OtherStruct, b OtherStruct) bool { return a.Key1Field == b.Key1Field && a.Key2Field == b.Key2Field @@ -91,9 +94,12 @@ func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field return nil } // call field-attached validations - errs = append(errs, validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, func(a OtherTypedefStruct, b OtherTypedefStruct) bool { + if e := validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, func(a OtherTypedefStruct, b OtherTypedefStruct) bool { return a.Key1Field == b.Key1Field && a.Key2Field == b.Key2Field - }, validate.DirectEqual, validate.ImmutableByCompare)...) + }, validate.DirectEqual, validate.Immutable); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } // lists with map semantics require unique keys errs = append(errs, validate.Unique(ctx, op, fldPath, obj, oldObj, func(a OtherTypedefStruct, b OtherTypedefStruct) bool { return a.Key1Field == b.Key1Field && a.Key2Field == b.Key2Field @@ -109,9 +115,12 @@ func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field return nil } // call field-attached validations - errs = append(errs, validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, func(a OtherStruct, b OtherStruct) bool { + if e := validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, func(a OtherStruct, b OtherStruct) bool { return a.Key1Field == b.Key1Field && a.Key2Field == b.Key2Field - }, validate.DirectEqual, validate.ImmutableByCompare)...) + }, validate.DirectEqual, validate.Immutable); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } // call the type's validation function errs = append(errs, Validate_ListType(ctx, op, fldPath, obj, oldObj)...) return diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/listmap/single_key/doc_test.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/listmap/single_key/doc_test.go index a2130348018..49f724db8b9 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/listmap/single_key/doc_test.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/listmap/single_key/doc_test.go @@ -107,24 +107,24 @@ func TestUpdateCorrelation(t *testing.T) { st.Value(&structA2).OldValue(&structA1).ExpectValid() st.Value(&structA1).OldValue(&structB).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ - field.Forbidden(field.NewPath("listField").Index(0), "immutable"), - field.Forbidden(field.NewPath("listField").Index(1), "immutable"), - field.Forbidden(field.NewPath("listTypedefField").Index(0), "immutable"), - field.Forbidden(field.NewPath("listTypedefField").Index(1), "immutable"), - field.Forbidden(field.NewPath("typedefField").Index(0), "immutable"), - field.Forbidden(field.NewPath("typedefField").Index(1), "immutable"), + field.Invalid(field.NewPath("listField").Index(0), nil, "immutable").WithOrigin("immutable"), + field.Invalid(field.NewPath("listField").Index(1), nil, "immutable").WithOrigin("immutable"), + field.Invalid(field.NewPath("listTypedefField").Index(0), nil, "immutable").WithOrigin("immutable"), + field.Invalid(field.NewPath("listTypedefField").Index(1), nil, "immutable").WithOrigin("immutable"), + field.Invalid(field.NewPath("typedefField").Index(0), nil, "immutable").WithOrigin("immutable"), + field.Invalid(field.NewPath("typedefField").Index(1), nil, "immutable").WithOrigin("immutable"), }) st.Value(&structB).OldValue(&structA1).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ - field.Forbidden(field.NewPath("listField").Index(0), "immutable"), - field.Forbidden(field.NewPath("listField").Index(1), "immutable"), - field.Forbidden(field.NewPath("listField").Index(2), "immutable"), - field.Forbidden(field.NewPath("listTypedefField").Index(0), "immutable"), - field.Forbidden(field.NewPath("listTypedefField").Index(1), "immutable"), - field.Forbidden(field.NewPath("listTypedefField").Index(2), "immutable"), - field.Forbidden(field.NewPath("typedefField").Index(0), "immutable"), - field.Forbidden(field.NewPath("typedefField").Index(1), "immutable"), - field.Forbidden(field.NewPath("typedefField").Index(2), "immutable"), + field.Invalid(field.NewPath("listField").Index(0), nil, "immutable").WithOrigin("immutable"), + field.Invalid(field.NewPath("listField").Index(1), nil, "immutable").WithOrigin("immutable"), + field.Invalid(field.NewPath("listField").Index(2), nil, "immutable").WithOrigin("immutable"), + field.Invalid(field.NewPath("listTypedefField").Index(0), nil, "immutable").WithOrigin("immutable"), + field.Invalid(field.NewPath("listTypedefField").Index(1), nil, "immutable").WithOrigin("immutable"), + field.Invalid(field.NewPath("listTypedefField").Index(2), nil, "immutable").WithOrigin("immutable"), + field.Invalid(field.NewPath("typedefField").Index(0), nil, "immutable").WithOrigin("immutable"), + field.Invalid(field.NewPath("typedefField").Index(1), nil, "immutable").WithOrigin("immutable"), + field.Invalid(field.NewPath("typedefField").Index(2), nil, "immutable").WithOrigin("immutable"), }) } diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/listmap/single_key/zz_generated.validations.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/listmap/single_key/zz_generated.validations.go index 2a6d52021ad..172c1667f5a 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/listmap/single_key/zz_generated.validations.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/listmap/single_key/zz_generated.validations.go @@ -71,7 +71,10 @@ func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field return nil } // call field-attached validations - errs = append(errs, validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, func(a OtherStruct, b OtherStruct) bool { return a.KeyField == b.KeyField }, validate.DirectEqual, validate.ImmutableByCompare)...) + if e := validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, func(a OtherStruct, b OtherStruct) bool { return a.KeyField == b.KeyField }, validate.DirectEqual, validate.Immutable); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } // lists with map semantics require unique keys errs = append(errs, validate.Unique(ctx, op, fldPath, obj, oldObj, func(a OtherStruct, b OtherStruct) bool { return a.KeyField == b.KeyField })...) return @@ -85,7 +88,10 @@ func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field return nil } // call field-attached validations - errs = append(errs, validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, func(a OtherTypedefStruct, b OtherTypedefStruct) bool { return a.KeyField == b.KeyField }, validate.DirectEqual, validate.ImmutableByCompare)...) + if e := validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, func(a OtherTypedefStruct, b OtherTypedefStruct) bool { return a.KeyField == b.KeyField }, validate.DirectEqual, validate.Immutable); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } // lists with map semantics require unique keys errs = append(errs, validate.Unique(ctx, op, fldPath, obj, oldObj, func(a OtherTypedefStruct, b OtherTypedefStruct) bool { return a.KeyField == b.KeyField })...) return @@ -99,7 +105,10 @@ func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field return nil } // call field-attached validations - errs = append(errs, validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, func(a OtherStruct, b OtherStruct) bool { return a.KeyField == b.KeyField }, validate.DirectEqual, validate.ImmutableByCompare)...) + if e := validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, func(a OtherStruct, b OtherStruct) bool { return a.KeyField == b.KeyField }, validate.DirectEqual, validate.Immutable); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } // call the type's validation function errs = append(errs, Validate_ListType(ctx, op, fldPath, obj, oldObj)...) return diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/listset/zz_generated.validations.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/listset/zz_generated.validations.go index 55a47c07f2d..dc360fc7ce5 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/listset/zz_generated.validations.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/listset/zz_generated.validations.go @@ -70,7 +70,10 @@ func Validate_ImmutableStruct(ctx context.Context, op operation.Operation, fldPa return nil } // call field-attached validations - errs = append(errs, validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, nil, nil, validate.ImmutableByCompare)...) + if e := validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, nil, nil, validate.Immutable); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } return }(fldPath.Child("sliceComparableField"), obj.SliceComparableField, safe.Field(oldObj, func(oldObj *ImmutableStruct) []ComparableStruct { return oldObj.SliceComparableField }))...) @@ -82,7 +85,10 @@ func Validate_ImmutableStruct(ctx context.Context, op operation.Operation, fldPa return nil } // call field-attached validations - errs = append(errs, validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, validate.DirectEqual, nil, validate.ImmutableByCompare)...) + if e := validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, validate.DirectEqual, nil, validate.Immutable); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } // lists with set semantics require unique values errs = append(errs, validate.Unique(ctx, op, fldPath, obj, oldObj, validate.DirectEqual)...) return @@ -96,7 +102,10 @@ func Validate_ImmutableStruct(ctx context.Context, op operation.Operation, fldPa return nil } // call field-attached validations - errs = append(errs, validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, nil, nil, validate.ImmutableByReflect)...) + if e := validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, nil, nil, validate.Immutable); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } return }(fldPath.Child("sliceNonComparableField"), obj.SliceNonComparableField, safe.Field(oldObj, func(oldObj *ImmutableStruct) []NonComparableStruct { return oldObj.SliceNonComparableField }))...) @@ -108,7 +117,10 @@ func Validate_ImmutableStruct(ctx context.Context, op operation.Operation, fldPa return nil } // call field-attached validations - errs = append(errs, validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, validate.SemanticDeepEqual, nil, validate.ImmutableByReflect)...) + if e := validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, validate.SemanticDeepEqual, nil, validate.Immutable); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } // lists with set semantics require unique values errs = append(errs, validate.Unique(ctx, op, fldPath, obj, oldObj, validate.SemanticDeepEqual)...) return @@ -122,7 +134,10 @@ func Validate_ImmutableStruct(ctx context.Context, op operation.Operation, fldPa return nil } // call field-attached validations - errs = append(errs, validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, nil, nil, validate.ImmutableByCompare)...) + if e := validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, nil, nil, validate.Immutable); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } return }(fldPath.Child("slicePrimitiveField"), obj.SlicePrimitiveField, safe.Field(oldObj, func(oldObj *ImmutableStruct) []int { return oldObj.SlicePrimitiveField }))...) @@ -134,7 +149,10 @@ func Validate_ImmutableStruct(ctx context.Context, op operation.Operation, fldPa return nil } // call field-attached validations - errs = append(errs, validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, validate.DirectEqual, nil, validate.ImmutableByCompare)...) + if e := validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, validate.DirectEqual, nil, validate.Immutable); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } // lists with set semantics require unique values errs = append(errs, validate.Unique(ctx, op, fldPath, obj, oldObj, validate.DirectEqual)...) return @@ -148,7 +166,10 @@ func Validate_ImmutableStruct(ctx context.Context, op operation.Operation, fldPa return nil } // call field-attached validations - errs = append(errs, validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, validate.SemanticDeepEqual, nil, validate.ImmutableByReflect)...) + if e := validate.EachSliceVal(ctx, op, fldPath, obj, oldObj, validate.SemanticDeepEqual, nil, validate.Immutable); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } // lists with set semantics require unique values errs = append(errs, validate.Unique(ctx, op, fldPath, obj, oldObj, validate.SemanticDeepEqual)...) return diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/update_validations/primitive_pointers/doc_test.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/update_validations/primitive_pointers/doc_test.go index 7ad410ae888..fb0e735218e 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/update_validations/primitive_pointers/doc_test.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/update_validations/primitive_pointers/doc_test.go @@ -53,9 +53,9 @@ func Test(t *testing.T) { st.Value(&structA1).OldValue(&structA2).ExpectValid() st.Value(&structA1).OldValue(&structB).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ - field.Forbidden(field.NewPath("sp"), ""), - field.Forbidden(field.NewPath("ip"), ""), - field.Forbidden(field.NewPath("bp"), ""), - field.Forbidden(field.NewPath("fp"), ""), + field.Invalid(field.NewPath("sp"), nil, "").WithOrigin("immutable"), + field.Invalid(field.NewPath("ip"), nil, "").WithOrigin("immutable"), + field.Invalid(field.NewPath("bp"), nil, "").WithOrigin("immutable"), + field.Invalid(field.NewPath("fp"), nil, "").WithOrigin("immutable"), }) } diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/update_validations/primitive_pointers/zz_generated.validations.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/update_validations/primitive_pointers/zz_generated.validations.go index 2bd5d715fa4..0466acb84fb 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/update_validations/primitive_pointers/zz_generated.validations.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/update_validations/primitive_pointers/zz_generated.validations.go @@ -59,7 +59,10 @@ func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field return nil } // call field-attached validations - errs = append(errs, validate.ImmutableByCompare(ctx, op, fldPath, obj, oldObj)...) + if e := validate.Immutable(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } return }(fldPath.Child("sp"), obj.SP, safe.Field(oldObj, func(oldObj *Struct) *string { return oldObj.SP }))...) @@ -71,7 +74,10 @@ func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field return nil } // call field-attached validations - errs = append(errs, validate.ImmutableByCompare(ctx, op, fldPath, obj, oldObj)...) + if e := validate.Immutable(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } return }(fldPath.Child("ip"), obj.IP, safe.Field(oldObj, func(oldObj *Struct) *int { return oldObj.IP }))...) @@ -83,7 +89,10 @@ func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field return nil } // call field-attached validations - errs = append(errs, validate.ImmutableByCompare(ctx, op, fldPath, obj, oldObj)...) + if e := validate.Immutable(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } return }(fldPath.Child("bp"), obj.BP, safe.Field(oldObj, func(oldObj *Struct) *bool { return oldObj.BP }))...) @@ -95,7 +104,10 @@ func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field return nil } // call field-attached validations - errs = append(errs, validate.ImmutableByCompare(ctx, op, fldPath, obj, oldObj)...) + if e := validate.Immutable(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } return }(fldPath.Child("fp"), obj.FP, safe.Field(oldObj, func(oldObj *Struct) *float64 { return oldObj.FP }))...) diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/update_validations/primitives/doc_test.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/update_validations/primitives/doc_test.go index 429c8812868..1a2ac506d3e 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/update_validations/primitives/doc_test.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/update_validations/primitives/doc_test.go @@ -43,9 +43,9 @@ func Test(t *testing.T) { st.Value(&structA).OldValue(&structA).ExpectValid() st.Value(&structA).OldValue(&structB).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ - field.Forbidden(field.NewPath("s"), ""), - field.Forbidden(field.NewPath("i"), ""), - field.Forbidden(field.NewPath("b"), ""), - field.Forbidden(field.NewPath("f"), ""), + field.Invalid(field.NewPath("s"), nil, "").WithOrigin("immutable"), + field.Invalid(field.NewPath("i"), nil, "").WithOrigin("immutable"), + field.Invalid(field.NewPath("b"), nil, "").WithOrigin("immutable"), + field.Invalid(field.NewPath("f"), nil, "").WithOrigin("immutable"), }) } diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/update_validations/primitives/zz_generated.validations.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/update_validations/primitives/zz_generated.validations.go index 4e9df2f2782..531fe8b9af2 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/update_validations/primitives/zz_generated.validations.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/update_validations/primitives/zz_generated.validations.go @@ -59,7 +59,10 @@ func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field return nil } // call field-attached validations - errs = append(errs, validate.ImmutableByCompare(ctx, op, fldPath, obj, oldObj)...) + if e := validate.Immutable(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } return }(fldPath.Child("s"), &obj.S, safe.Field(oldObj, func(oldObj *Struct) *string { return &oldObj.S }))...) @@ -71,7 +74,10 @@ func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field return nil } // call field-attached validations - errs = append(errs, validate.ImmutableByCompare(ctx, op, fldPath, obj, oldObj)...) + if e := validate.Immutable(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } return }(fldPath.Child("i"), &obj.I, safe.Field(oldObj, func(oldObj *Struct) *int { return &oldObj.I }))...) @@ -83,7 +89,10 @@ func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field return nil } // call field-attached validations - errs = append(errs, validate.ImmutableByCompare(ctx, op, fldPath, obj, oldObj)...) + if e := validate.Immutable(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } return }(fldPath.Child("b"), &obj.B, safe.Field(oldObj, func(oldObj *Struct) *bool { return &oldObj.B }))...) @@ -95,7 +104,10 @@ func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field return nil } // call field-attached validations - errs = append(errs, validate.ImmutableByCompare(ctx, op, fldPath, obj, oldObj)...) + if e := validate.Immutable(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + errs = append(errs, e...) + return // do not proceed + } return }(fldPath.Child("f"), &obj.F, safe.Field(oldObj, func(oldObj *Struct) *float64 { return &oldObj.F }))...) diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/validators/immutable.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/validators/immutable.go index a2343f3d304..c4a94154f65 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/validators/immutable.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/validators/immutable.go @@ -18,7 +18,6 @@ package validators import ( "k8s.io/apimachinery/pkg/util/sets" - "k8s.io/code-generator/cmd/validation-gen/util" "k8s.io/gengo/v2/codetags" "k8s.io/gengo/v2/types" ) @@ -46,24 +45,14 @@ func (immutableTagValidator) ValidScopes() sets.Set[Scope] { } var ( - immutableCompareValidator = types.Name{Package: libValidationPkg, Name: "ImmutableByCompare"} - immutableReflectValidator = types.Name{Package: libValidationPkg, Name: "ImmutableByReflect"} + immutableValidator = types.Name{Package: libValidationPkg, Name: "Immutable"} ) func (immutableTagValidator) GetValidations(context Context, _ codetags.Tag) (Validations, error) { var result Validations - if util.IsDirectComparable(util.NonPointer(util.NativeType(context.Type))) { - // This is a minor optimization to just compare primitive values when - // possible. Slices and maps are not comparable, and structs might hold - // pointer fields, which are directly comparable but not what we need. - // - // Note: This compares the pointee, not the pointer itself. - result.AddFunction(Function(immutableTagName, DefaultFlags, immutableCompareValidator)) - } else { - result.AddFunction(Function(immutableTagName, DefaultFlags, immutableReflectValidator)) - } - + // Use ShortCircuit flag so immutable runs in the same group as +k8s:optional. + result.AddFunction(Function(immutableTagName, ShortCircuit, immutableValidator)) return result, nil }