From 9a39e5c49faecdf0be53bbdec2d2751799aa774d Mon Sep 17 00:00:00 2001 From: yongruilin Date: Wed, 18 Mar 2026 22:25:32 +0000 Subject: [PATCH 1/6] Fix union validation ratcheting when oldObj is nil When oldObj is nil (e.g. new map entry added during update), union ratcheting incorrectly treats nil old and empty new as unchanged membership, skipping validation entirely. Fix by checking reflect.ValueOf(oldObj).IsNil() and disabling ratcheting when oldObj is nil, so the new value is fully validated. This affects Union, DiscriminatedUnion, and ZeroOrOneOfUnion (via unionValidate). --- .../k8s.io/apimachinery/pkg/api/validate/union.go | 13 ++++++++----- .../apimachinery/pkg/api/validate/union_test.go | 7 +++++-- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/api/validate/union.go b/staging/src/k8s.io/apimachinery/pkg/api/validate/union.go index 03f45f8661d..0bf552fd40d 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/validate/union.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/validate/union.go @@ -19,6 +19,7 @@ package validate import ( "context" "fmt" + "reflect" "strings" "k8s.io/apimachinery/pkg/api/operation" @@ -106,9 +107,10 @@ func DiscriminatedUnion[T any, D ~string](_ context.Context, op operation.Operat len(isSetFns), len(union.members))), } } + hasOldValue := !reflect.ValueOf(oldObj).IsNil() var changed bool discriminatorValue := discriminatorExtractor(obj) - if op.Type == operation.Update { + if op.Type == operation.Update && hasOldValue { oldDiscriminatorValue := discriminatorExtractor(oldObj) changed = discriminatorValue != oldDiscriminatorValue } @@ -117,7 +119,7 @@ func DiscriminatedUnion[T any, D ~string](_ context.Context, op operation.Operat member := union.members[i] isDiscriminatedMember := string(discriminatorValue) == member.discriminatorValue newIsSet := fieldIsSet(obj) - if op.Type == operation.Update && !changed { + if op.Type == operation.Update && hasOldValue && !changed { oldIsSet := fieldIsSet(oldObj) changed = changed || newIsSet != oldIsSet } @@ -131,7 +133,7 @@ func DiscriminatedUnion[T any, D ~string](_ context.Context, op operation.Operat } // If the union discriminator and membership is unchanged, we don't need to // re-validate. - if op.Type == operation.Update && !changed { + if op.Type == operation.Update && hasOldValue && !changed { return nil } return errs @@ -195,11 +197,12 @@ func unionValidate[T any](op operation.Operation, fldPath *field.Path, } } + hasOldValue := !reflect.ValueOf(oldObj).IsNil() var specifiedFields []string var changed bool for i, fieldIsSet := range isSetFns { newIsSet := fieldIsSet(obj) - if op.Type == operation.Update && !changed { + if op.Type == operation.Update && hasOldValue && !changed { oldIsSet := fieldIsSet(oldObj) changed = changed || newIsSet != oldIsSet } @@ -209,7 +212,7 @@ func unionValidate[T any](op operation.Operation, fldPath *field.Path, } // If the union membership is unchanged, we don't need to re-validate. - if op.Type == operation.Update && !changed { + if op.Type == operation.Update && hasOldValue && !changed { return nil } diff --git a/staging/src/k8s.io/apimachinery/pkg/api/validate/union_test.go b/staging/src/k8s.io/apimachinery/pkg/api/validate/union_test.go index db9fb7231e4..981575a4785 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/validate/union_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/validate/union_test.go @@ -186,9 +186,12 @@ func TestUnionRatcheting(t *testing.T) { expected field.ErrorList }{ { - name: "both nil", + name: "old nil - no ratcheting", oldStruct: nil, - newStruct: nil, + newStruct: &testStruct{}, + expected: field.ErrorList{ + field.Invalid(nil, "", "must specify one of: `m1`, `m2`"), + }.WithOrigin("union"), }, { name: "both empty struct", From 863ed56ac0e21e84cfbcb3d1c7115de65fa6d992 Mon Sep 17 00:00:00 2001 From: yongruilin Date: Wed, 18 Mar 2026 22:26:00 +0000 Subject: [PATCH 2/6] Add nil OldValue test coverage for union doc_tests Add tests for Union, DiscriminatedUnion, and ZeroOrOneOfUnion validating that nil oldObj (new map entry or newly-set pointer field during update) does not skip validation via ratcheting. --- .../tags/union/union/discriminated/simple/doc_test.go | 6 ++++++ .../tags/union/union/undiscriminated/simple/doc_test.go | 6 ++++++ .../tags/zerooroneof/zerooroneof/simple/doc_test.go | 8 ++++++++ 3 files changed, 20 insertions(+) diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/discriminated/simple/doc_test.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/discriminated/simple/doc_test.go index bd2885ffb04..e7d41558f29 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/discriminated/simple/doc_test.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/discriminated/simple/doc_test.go @@ -41,4 +41,10 @@ func Test(t *testing.T) { // Test validation ratcheting st.Value(&Struct{D: DM2, M1: &M1{}, M2: &M2{}}).OldValue(&Struct{D: DM2, M1: &M1{}, M2: &M2{}}).ExpectValid() st.Value(&Struct{D: DM1}).OldValue(&Struct{D: DM1}).ExpectValid() + + // Test update with nil old value (simulates newly-set pointer field during update). + // Discriminated union validation should still detect mismatches even though oldObj is nil. + st.Value(&Struct{D: DM1}).OldValue(nil).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ + field.Invalid(field.NewPath("m1"), nil, "must be specified when"), + }.WithOrigin("union")) } diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/undiscriminated/simple/doc_test.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/undiscriminated/simple/doc_test.go index f00b41b546f..3ca566be821 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/undiscriminated/simple/doc_test.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/undiscriminated/simple/doc_test.go @@ -52,4 +52,10 @@ func Test(t *testing.T) { st.Value(&Struct{}).OldValue(&Struct{}).ExpectValid() st.Value(&Struct{M1: &M1{}, M2: &M2{}}).OldValue(&Struct{M1: &M1{}, M2: &M2{}}).ExpectValid() st.Value(&Struct{M3: "a string", M2: &M2{}}).OldValue(&Struct{M3: "different string", M2: &M2{}}).ExpectValid() + + // Test update with nil old value (simulates new map entry or newly-set pointer field during update). + // Union validation should still detect the empty union even though oldObj is nil. + st.Value(&Struct{}).OldValue(nil).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ + field.Invalid(nil, nil, "must specify one of").WithOrigin("union"), + }) } diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/zerooroneof/zerooroneof/simple/doc_test.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/zerooroneof/zerooroneof/simple/doc_test.go index 01a92457af9..6764fa06545 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/zerooroneof/zerooroneof/simple/doc_test.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/zerooroneof/zerooroneof/simple/doc_test.go @@ -51,4 +51,12 @@ func Test(t *testing.T) { st.Value(&Struct{}).OldValue(&Struct{}).ExpectValid() st.Value(&Struct{M1: &M1{}, M2: &M2{}}).OldValue(&Struct{M1: &M1{}, M2: &M2{}}).ExpectValid() st.Value(&Struct{M3: "a string", M2: &M2{}}).OldValue(&Struct{M3: "different string", M2: &M2{}}).ExpectValid() + + // Test update with nil old value (simulates new map entry added during update). + // Empty union is still valid for zeroOrOneOf, even with nil old value. + st.Value(&Struct{}).OldValue((*Struct)(nil)).ExpectValid() + // Multiple members set should still be caught even with nil old value. + st.Value(&Struct{M1: &M1{}, M2: &M2{}}).OldValue((*Struct)(nil)).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ + field.Invalid(nil, nil, "must specify at most one of").WithOrigin("zeroOrOneOf"), + }) } From 7b708cc7c644f8604c4e172c814a1a6cb0fcc1f4 Mon Sep 17 00:00:00 2001 From: yongruilin Date: Wed, 18 Mar 2026 22:26:41 +0000 Subject: [PATCH 3/6] Add DRA test for device attribute with no value set Test that declarative validation catches a DeviceAttribute with no value fields set, which was the original bug scenario where union ratcheting incorrectly skipped validation for new map entries. --- .../resource/resourceslice/declarative_validation_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/registry/resource/resourceslice/declarative_validation_test.go b/pkg/registry/resource/resourceslice/declarative_validation_test.go index 307efe7f448..6bc6adcbe1e 100644 --- a/pkg/registry/resource/resourceslice/declarative_validation_test.go +++ b/pkg/registry/resource/resourceslice/declarative_validation_test.go @@ -284,6 +284,13 @@ func TestDeclarativeValidateUpdate(t *testing.T) { field.Invalid(field.NewPath("spec", "devices").Index(0).Child("attributes").Key("test.io/multiple"), "", "may have only one of the following fields set: bool, int, string, version"), }, }, + "invalid update: device attribute no value": { + old: mkResourceSliceWithDevices(), + update: mkResourceSliceWithDevices(tweakDeviceAttribute("test.io/empty", resource.DeviceAttribute{})), + expectedErrs: field.ErrorList{ + field.Invalid(field.NewPath("spec", "devices").Index(0).Child("attributes").Key("test.io/empty"), "", "").WithOrigin("union").MarkAlpha(), + }, + }, // spec.sharedCounters "valid update: at limit shared counters": { old: mkResourceSliceWithSharedCounters(), From 3d39627cd9815f39fdf8243a60cd0768538e1b1f Mon Sep 17 00:00:00 2001 From: yongruilin Date: Wed, 18 Mar 2026 23:45:09 +0000 Subject: [PATCH 4/6] Use IsZero instead of IsNil for union ratcheting check - Use reflect.ValueOf(oldObj).IsZero() instead of IsNil() so union validation works with non-nilable T (e.g. value types) - Remove hasOldValue guard from inner loop conditionals; only check at the final ratcheting skip point - Add doc comments explaining T is "any" rather than "comparable" because union members can be slices - Add value-type subtests for Union and DiscriminatedUnion Co-authored-by: Tim Hockin --- .../apimachinery/pkg/api/validate/union.go | 18 ++-- .../pkg/api/validate/union_test.go | 88 +++++++++++++------ 2 files changed, 74 insertions(+), 32 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/api/validate/union.go b/staging/src/k8s.io/apimachinery/pkg/api/validate/union.go index 0bf552fd40d..32a2671f8dc 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/validate/union.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/validate/union.go @@ -61,6 +61,10 @@ type UnionValidationOptions struct { // )...) // return errs // } +// +// Note that T is "any", rather than "comparable", because union-members can be +// slices, meaning T might be a struct with a slice, meaning it is not +// comparable. func Union[T any](_ context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj T, union *UnionMembership, isSetFns ...ExtractorFn[T, bool]) field.ErrorList { options := UnionValidationOptions{ ErrorForEmpty: func(fldPath *field.Path, allFields []string) *field.Error { @@ -99,6 +103,10 @@ func Union[T any](_ context.Context, op operation.Operation, fldPath *field.Path // // It is not an error for the discriminatorValue to be unknown. That must be // validated on its own. +// +// Note that T is "any", rather than "comparable", because union-members can be +// slices, meaning T might be a struct with a slice, meaning it is not +// comparable. func DiscriminatedUnion[T any, D ~string](_ context.Context, op operation.Operation, fldPath *field.Path, obj, oldObj T, union *UnionMembership, discriminatorExtractor ExtractorFn[T, D], isSetFns ...ExtractorFn[T, bool]) (errs field.ErrorList) { if len(union.members) != len(isSetFns) { return field.ErrorList{ @@ -107,10 +115,10 @@ func DiscriminatedUnion[T any, D ~string](_ context.Context, op operation.Operat len(isSetFns), len(union.members))), } } - hasOldValue := !reflect.ValueOf(oldObj).IsNil() + hasOldValue := !reflect.ValueOf(oldObj).IsZero() // because T is any, rather than comparable var changed bool discriminatorValue := discriminatorExtractor(obj) - if op.Type == operation.Update && hasOldValue { + if op.Type == operation.Update { oldDiscriminatorValue := discriminatorExtractor(oldObj) changed = discriminatorValue != oldDiscriminatorValue } @@ -119,7 +127,7 @@ func DiscriminatedUnion[T any, D ~string](_ context.Context, op operation.Operat member := union.members[i] isDiscriminatedMember := string(discriminatorValue) == member.discriminatorValue newIsSet := fieldIsSet(obj) - if op.Type == operation.Update && hasOldValue && !changed { + if op.Type == operation.Update && !changed { oldIsSet := fieldIsSet(oldObj) changed = changed || newIsSet != oldIsSet } @@ -197,12 +205,12 @@ func unionValidate[T any](op operation.Operation, fldPath *field.Path, } } - hasOldValue := !reflect.ValueOf(oldObj).IsNil() + hasOldValue := !reflect.ValueOf(oldObj).IsZero() // because T is any, rather than comparable var specifiedFields []string var changed bool for i, fieldIsSet := range isSetFns { newIsSet := fieldIsSet(obj) - if op.Type == operation.Update && hasOldValue && !changed { + if op.Type == operation.Update && !changed { oldIsSet := fieldIsSet(oldObj) changed = changed || newIsSet != oldIsSet } diff --git a/staging/src/k8s.io/apimachinery/pkg/api/validate/union_test.go b/staging/src/k8s.io/apimachinery/pkg/api/validate/union_test.go index 981575a4785..011937481ce 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/validate/union_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/validate/union_test.go @@ -67,18 +67,34 @@ func TestUnion(t *testing.T) { members = append(members, NewUnionMember(f)) } - // Create mock extractors that return predefined values instead of - // actually extracting from the object. - extractors := make([]ExtractorFn[*testMember, bool], len(tc.fieldValues)) - for i, val := range tc.fieldValues { - extractors[i] = func(_ *testMember) bool { return val } - } + t.Run("pointer", func(t *testing.T) { + // Create mock extractors that return predefined values instead of + // actually extracting from the object. + extractors := make([]ExtractorFn[*testMember, bool], len(tc.fieldValues)) + for i, val := range tc.fieldValues { + extractors[i] = func(_ *testMember) bool { return val } + } - got := Union(context.Background(), operation.Operation{}, nil, &testMember{}, nil, - NewUnionMembership(members...), extractors...) - if !reflect.DeepEqual(got, tc.expected) { - t.Errorf("got %v want %v", got, tc.expected) - } + got := Union(context.Background(), operation.Operation{}, nil, &testMember{}, nil, + NewUnionMembership(members...), extractors...) + if !reflect.DeepEqual(got, tc.expected) { + t.Errorf("got %v want %v", got, tc.expected) + } + }) + t.Run("value", func(t *testing.T) { + // Create mock extractors that return predefined values instead of + // actually extracting from the object. + extractors := make([]ExtractorFn[testMember, bool], len(tc.fieldValues)) + for i, val := range tc.fieldValues { + extractors[i] = func(_ testMember) bool { return val } + } + + got := Union(context.Background(), operation.Operation{}, nil, testMember{}, testMember{}, + NewUnionMembership(members...), extractors...) + if !reflect.DeepEqual(got, tc.expected) { + t.Errorf("got %v want %v", got, tc.expected) + } + }) }) } } @@ -131,26 +147,44 @@ func TestDiscriminatedUnion(t *testing.T) { } for _, tc := range testCases { + members := []UnionMember{} + for _, f := range tc.fields { + members = append(members, NewDiscriminatedUnionMember(f[0], f[1])) + } + t.Run(tc.name, func(t *testing.T) { - members := []UnionMember{} - for _, f := range tc.fields { - members = append(members, NewDiscriminatedUnionMember(f[0], f[1])) - } + t.Run("pointer", func(t *testing.T) { + discriminatorExtractor := func(_ *testMember) string { return tc.discriminatorValue } - discriminatorExtractor := func(_ *testMember) string { return tc.discriminatorValue } + // Create mock extractors that return predefined values instead of + // actually extracting from the object. + extractors := make([]ExtractorFn[*testMember, bool], len(tc.fieldValues)) + for i, val := range tc.fieldValues { + extractors[i] = func(_ *testMember) bool { return val } + } - // Create mock extractors that return predefined values instead of - // actually extracting from the object. - extractors := make([]ExtractorFn[*testMember, bool], len(tc.fieldValues)) - for i, val := range tc.fieldValues { - extractors[i] = func(_ *testMember) bool { return val } - } + got := DiscriminatedUnion(context.Background(), operation.Operation{}, nil, &testMember{}, nil, + NewDiscriminatedUnionMembership(tc.discriminatorField, members...), discriminatorExtractor, extractors...) + if !reflect.DeepEqual(got, tc.expected) { + t.Errorf("got %v want %v", got.ToAggregate(), tc.expected.ToAggregate()) + } + }) + t.Run("value", func(t *testing.T) { + discriminatorExtractor := func(_ testMember) string { return tc.discriminatorValue } - got := DiscriminatedUnion(context.Background(), operation.Operation{}, nil, &testMember{}, nil, - NewDiscriminatedUnionMembership(tc.discriminatorField, members...), discriminatorExtractor, extractors...) - if !reflect.DeepEqual(got, tc.expected) { - t.Errorf("got %v want %v", got.ToAggregate(), tc.expected.ToAggregate()) - } + // Create mock extractors that return predefined values instead of + // actually extracting from the object. + extractors := make([]ExtractorFn[testMember, bool], len(tc.fieldValues)) + for i, val := range tc.fieldValues { + extractors[i] = func(_ testMember) bool { return val } + } + + got := DiscriminatedUnion(context.Background(), operation.Operation{}, nil, testMember{}, testMember{}, + NewDiscriminatedUnionMembership(tc.discriminatorField, members...), discriminatorExtractor, extractors...) + if !reflect.DeepEqual(got, tc.expected) { + t.Errorf("got %v want %v", got.ToAggregate(), tc.expected.ToAggregate()) + } + }) }) } } From b6ee759d8ea61a00d391f44b7fcffd06ff158009 Mon Sep 17 00:00:00 2001 From: yongruilin Date: Thu, 19 Mar 2026 00:08:14 +0000 Subject: [PATCH 5/6] Add slice and map union member support with tests - Code generator: use len() != 0 for slice/map member extractors instead of != nil, so empty slice/map are treated as "not set" - Add slice and map members to union test types (both discriminated and undiscriminated) - Add test coverage for nil vs empty, ratcheting, and nil oldObj with slice/map members Co-authored-by: Tim Hockin --- .../pkg/api/validate/union_test.go | 124 ++++++++++++++++-- .../pkg/api/validate/zeroorone_test.go | 4 +- .../union/union/discriminated/simple/doc.go | 10 ++ .../union/discriminated/simple/doc_test.go | 44 +++++++ .../simple/zz_generated.validations.go | 49 ++++++- .../union/union/undiscriminated/simple/doc.go | 8 ++ .../union/undiscriminated/simple/doc_test.go | 14 ++ .../simple/zz_generated.validations.go | 49 ++++++- .../cmd/validation-gen/validators/union.go | 4 +- 9 files changed, 291 insertions(+), 15 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/api/validate/union_test.go b/staging/src/k8s.io/apimachinery/pkg/api/validate/union_test.go index 011937481ce..783ace1f6d4 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/validate/union_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/validate/union_test.go @@ -190,8 +190,10 @@ func TestDiscriminatedUnion(t *testing.T) { } type testStruct struct { - M1 *m1 `json:"m1"` - M2 *m2 `json:"m2"` + M1 *m1 `json:"m1"` + M2 *m2 `json:"m2"` + M3 []string `json:"m3"` + M4 map[string]string `json:"m4"` } type m1 struct{} @@ -210,6 +212,18 @@ var extractors = []ExtractorFn[*testStruct, bool]{ } return s.M2 != nil }, + func(s *testStruct) bool { + if s == nil { + return false + } + return len(s.M3) != 0 + }, + func(s *testStruct) bool { + if s == nil { + return false + } + return len(s.M4) != 0 + }, } func TestUnionRatcheting(t *testing.T) { @@ -224,7 +238,7 @@ func TestUnionRatcheting(t *testing.T) { oldStruct: nil, newStruct: &testStruct{}, expected: field.ErrorList{ - field.Invalid(nil, "", "must specify one of: `m1`, `m2`"), + field.Invalid(nil, "", "must specify one of: `m1`, `m2`, `m3`, `m4`"), }.WithOrigin("union"), }, { @@ -253,14 +267,40 @@ func TestUnionRatcheting(t *testing.T) { M2: &m2{}, }, expected: field.ErrorList{ - field.Invalid(nil, "{m1, m2}", "must specify exactly one of: `m1`, `m2`"), - }, + field.Invalid(nil, "{m1, m2}", "must specify exactly one of: `m1`, `m2`, `m3`, `m4`"), + }.WithOrigin("union"), + }, + { + name: "slice member ratcheting: unchanged membership", + oldStruct: &testStruct{M3: []string{"a"}}, + newStruct: &testStruct{M3: []string{"b"}}, + }, + { + name: "map member ratcheting: unchanged membership", + oldStruct: &testStruct{M4: map[string]string{"k": "v1"}}, + newStruct: &testStruct{M4: map[string]string{"k": "v2"}}, + }, + { + name: "empty slice is not set", + oldStruct: nil, + newStruct: &testStruct{M3: []string{}}, + expected: field.ErrorList{ + field.Invalid(nil, "", "must specify one of: `m1`, `m2`, `m3`, `m4`"), + }.WithOrigin("union"), + }, + { + name: "empty map is not set", + oldStruct: nil, + newStruct: &testStruct{M4: map[string]string{}}, + expected: field.ErrorList{ + field.Invalid(nil, "", "must specify one of: `m1`, `m2`, `m3`, `m4`"), + }.WithOrigin("union"), }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - members := []UnionMember{NewUnionMember("m1"), NewUnionMember("m2")} + members := []UnionMember{NewUnionMember("m1"), NewUnionMember("m2"), NewUnionMember("m3"), NewUnionMember("m4")} got := Union(context.Background(), operation.Operation{Type: operation.Update}, nil, tc.newStruct, tc.oldStruct, NewUnionMembership(members...), extractors...) if !reflect.DeepEqual(got, tc.expected) { @@ -271,9 +311,11 @@ func TestUnionRatcheting(t *testing.T) { } type testDiscriminatedStruct struct { - D string `json:"d"` - M1 *m1 `json:"m1"` - M2 *m2 `json:"m2"` + D string `json:"d"` + M1 *m1 `json:"m1"` + M2 *m2 `json:"m2"` + M3 []string `json:"m3"` + M4 map[string]string `json:"m4"` } var testDiscriminatorExtractor = func(s *testDiscriminatedStruct) string { @@ -295,6 +337,18 @@ var testDiscriminatedExtractors = []ExtractorFn[*testDiscriminatedStruct, bool]{ } return s.M2 != nil }, + func(s *testDiscriminatedStruct) bool { + if s == nil { + return false + } + return len(s.M3) != 0 + }, + func(s *testDiscriminatedStruct) bool { + if s == nil { + return false + } + return len(s.M4) != 0 + }, } func TestDiscriminatedUnionRatcheting(t *testing.T) { @@ -366,11 +420,61 @@ func TestDiscriminatedUnionRatcheting(t *testing.T) { field.Invalid(field.NewPath("m2"), "", "must be specified when `d` is \"m2\""), }, }, + { + name: "slice member ratcheting: unchanged membership", + oldStruct: &testDiscriminatedStruct{ + D: "m3", + M3: []string{"a"}, + }, + newStruct: &testDiscriminatedStruct{ + D: "m3", + M3: []string{"b"}, + }, + }, + { + name: "map member ratcheting: unchanged membership", + oldStruct: &testDiscriminatedStruct{ + D: "m4", + M4: map[string]string{"k": "v1"}, + }, + newStruct: &testDiscriminatedStruct{ + D: "m4", + M4: map[string]string{"k": "v2"}, + }, + }, + { + name: "empty slice is not set", + oldStruct: &testDiscriminatedStruct{ + D: "m3", + M3: []string{"a"}, + }, + newStruct: &testDiscriminatedStruct{ + D: "m3", + M3: []string{}, + }, + expected: field.ErrorList{ + field.Invalid(field.NewPath("m3"), "", "must be specified when `d` is \"m3\""), + }.WithOrigin("union"), + }, + { + name: "empty map is not set", + oldStruct: &testDiscriminatedStruct{ + D: "m4", + M4: map[string]string{"k": "v"}, + }, + newStruct: &testDiscriminatedStruct{ + D: "m4", + M4: map[string]string{}, + }, + expected: field.ErrorList{ + field.Invalid(field.NewPath("m4"), "", "must be specified when `d` is \"m4\""), + }.WithOrigin("union"), + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - members := []UnionMember{NewDiscriminatedUnionMember("m1", "m1"), NewDiscriminatedUnionMember("m2", "m2")} + members := []UnionMember{NewDiscriminatedUnionMember("m1", "m1"), NewDiscriminatedUnionMember("m2", "m2"), NewDiscriminatedUnionMember("m3", "m3"), NewDiscriminatedUnionMember("m4", "m4")} got := DiscriminatedUnion(context.Background(), operation.Operation{Type: operation.Update}, nil, tc.newStruct, tc.oldStruct, NewDiscriminatedUnionMembership("d", members...), testDiscriminatorExtractor, testDiscriminatedExtractors...) if !reflect.DeepEqual(got, tc.expected) { diff --git a/staging/src/k8s.io/apimachinery/pkg/api/validate/zeroorone_test.go b/staging/src/k8s.io/apimachinery/pkg/api/validate/zeroorone_test.go index 7222c5cd7a2..686a1c11a64 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/validate/zeroorone_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/validate/zeroorone_test.go @@ -119,7 +119,7 @@ func TestZeroOrOneOfUnionRatcheting(t *testing.T) { M2: &m2{}, }, expected: field.ErrorList{ - field.Invalid(nil, "{m1, m2}", "must specify at most one of: `m1`, `m2`").WithOrigin("zeroOrOneOf"), + field.Invalid(nil, "{m1, m2}", "must specify at most one of: `m1`, `m2`, `m3`, `m4`").WithOrigin("zeroOrOneOf"), }, }, { @@ -142,7 +142,7 @@ func TestZeroOrOneOfUnionRatcheting(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - members := []UnionMember{NewUnionMember("m1"), NewUnionMember("m2")} + members := []UnionMember{NewUnionMember("m1"), NewUnionMember("m2"), NewUnionMember("m3"), NewUnionMember("m4")} got := ZeroOrOneOfUnion(context.Background(), operation.Operation{Type: operation.Update}, nil, tc.newStruct, tc.oldStruct, NewUnionMembership(members...), extractors...) if !reflect.DeepEqual(got, tc.expected) { diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/discriminated/simple/doc.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/discriminated/simple/doc.go index 178676a9edc..1b1ae5c123e 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/discriminated/simple/doc.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/discriminated/simple/doc.go @@ -38,6 +38,14 @@ type Struct struct { // +k8s:unionMember // +k8s:optional M2 *M2 `json:"m2"` + + // +k8s:unionMember + // +k8s:optional + M3 []string `json:"m3"` + + // +k8s:unionMember + // +k8s:optional + M4 map[string]string `json:"m4"` } type D string @@ -45,6 +53,8 @@ type D string const ( DM1 D = "M1" DM2 D = "M2" + DM3 D = "M3" + DM4 D = "M4" ) type M1 struct{} diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/discriminated/simple/doc_test.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/discriminated/simple/doc_test.go index e7d41558f29..86c315efc77 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/discriminated/simple/doc_test.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/discriminated/simple/doc_test.go @@ -29,6 +29,8 @@ func Test(t *testing.T) { st.Value(&Struct{D: DM1, M1: &M1{}}).ExpectValid() st.Value(&Struct{D: DM2, M2: &M2{}}).ExpectValid() + st.Value(&Struct{D: DM3, M3: []string{"a"}}).ExpectValid() + st.Value(&Struct{D: DM4, M4: map[string]string{"k": "v"}}).ExpectValid() st.Value(&Struct{D: DM2, M1: &M1{}, M2: &M2{}}).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ field.Invalid(field.NewPath("m1"), nil, "may only be specified when"), @@ -38,10 +40,52 @@ func Test(t *testing.T) { field.Invalid(field.NewPath("m1"), nil, "must be specified when"), }) + // Slice member: discriminator matches but slice is nil + st.Value(&Struct{D: DM3}).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ + field.Invalid(field.NewPath("m3"), nil, "must be specified when"), + }.WithOrigin("union")) + // Slice member: discriminator matches but slice is empty (len==0, not nil) + st.Value(&Struct{D: DM3, M3: []string{}}).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ + field.Invalid(field.NewPath("m3"), nil, "must be specified when"), + }.WithOrigin("union")) + + // Slice member: discriminator doesn't match but slice is set + st.Value(&Struct{D: DM1, M1: &M1{}, M3: []string{"a"}}).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ + field.Invalid(field.NewPath("m3"), nil, "may only be specified when"), + }.WithOrigin("union")) + + // Map member: discriminator matches but map is nil + st.Value(&Struct{D: DM4}).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ + field.Invalid(field.NewPath("m4"), nil, "must be specified when"), + }.WithOrigin("union")) + // Map member: discriminator matches but map is empty (len==0, not nil) + st.Value(&Struct{D: DM4, M4: map[string]string{}}).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ + field.Invalid(field.NewPath("m4"), nil, "must be specified when"), + }.WithOrigin("union")) + + // Map member: discriminator doesn't match but map is set + st.Value(&Struct{D: DM1, M1: &M1{}, M4: map[string]string{"k": "v"}}).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ + field.Invalid(field.NewPath("m4"), nil, "may only be specified when"), + }.WithOrigin("union")) + // Test validation ratcheting st.Value(&Struct{D: DM2, M1: &M1{}, M2: &M2{}}).OldValue(&Struct{D: DM2, M1: &M1{}, M2: &M2{}}).ExpectValid() st.Value(&Struct{D: DM1}).OldValue(&Struct{D: DM1}).ExpectValid() + // Slice member ratcheting: unchanged membership + st.Value(&Struct{D: DM3, M3: []string{"a"}}).OldValue(&Struct{D: DM3, M3: []string{"b"}}).ExpectValid() + // Slice member ratcheting: changed from set to unset + st.Value(&Struct{D: DM3}).OldValue(&Struct{D: DM3, M3: []string{"a"}}).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ + field.Invalid(field.NewPath("m3"), nil, "must be specified when"), + }.WithOrigin("union")) + + // Map member ratcheting: unchanged membership + st.Value(&Struct{D: DM4, M4: map[string]string{"k": "v1"}}).OldValue(&Struct{D: DM4, M4: map[string]string{"k": "v2"}}).ExpectValid() + // Map member ratcheting: changed from set to unset + st.Value(&Struct{D: DM4}).OldValue(&Struct{D: DM4, M4: map[string]string{"k": "v"}}).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ + field.Invalid(field.NewPath("m4"), nil, "must be specified when"), + }.WithOrigin("union")) + // Test update with nil old value (simulates newly-set pointer field during update). // Discriminated union validation should still detect mismatches even though oldObj is nil. st.Value(&Struct{D: DM1}).OldValue(nil).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/discriminated/simple/zz_generated.validations.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/discriminated/simple/zz_generated.validations.go index f54d8805d4c..545db7341df 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/discriminated/simple/zz_generated.validations.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/discriminated/simple/zz_generated.validations.go @@ -25,6 +25,7 @@ import ( context "context" fmt "fmt" + equality "k8s.io/apimachinery/pkg/api/equality" operation "k8s.io/apimachinery/pkg/api/operation" safe "k8s.io/apimachinery/pkg/api/safe" validate "k8s.io/apimachinery/pkg/api/validate" @@ -48,7 +49,7 @@ func RegisterValidations(scheme *testscheme.Scheme) error { return nil } -var unionMembershipFor_k8s_io_code_generator_cmd_validation_gen_output_tests_tags_union_union_discriminated_simple_Struct_ = validate.NewDiscriminatedUnionMembership("d", validate.NewDiscriminatedUnionMember("m1", "M1"), validate.NewDiscriminatedUnionMember("m2", "M2")) +var unionMembershipFor_k8s_io_code_generator_cmd_validation_gen_output_tests_tags_union_union_discriminated_simple_Struct_ = validate.NewDiscriminatedUnionMembership("d", validate.NewDiscriminatedUnionMember("m1", "M1"), validate.NewDiscriminatedUnionMember("m2", "M2"), validate.NewDiscriminatedUnionMember("m3", "M3"), validate.NewDiscriminatedUnionMember("m4", "M4")) // Validate_Struct validates an instance of Struct according // to declarative validation rules in the API schema. @@ -68,6 +69,16 @@ func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field return false } return obj.M2 != nil + }, func(obj *Struct) bool { + if obj == nil { + return false + } + return len(obj.M3) != 0 + }, func(obj *Struct) bool { + if obj == nil { + return false + } + return len(obj.M4) != 0 })...) // field Struct.TypeMeta has no validation @@ -109,5 +120,41 @@ func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field return }(fldPath.Child("m2"), obj.M2, safe.Field(oldObj, func(oldObj *Struct) *M2 { return oldObj.M2 }), oldObj != nil)...) + // field Struct.M3 + errs = append(errs, + func(fldPath *field.Path, obj, oldObj []string, oldValueCorrelated bool) (errs field.ErrorList) { + // don't revalidate unchanged data + if oldValueCorrelated && op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { + return nil + } + // call field-attached validations + earlyReturn := false + if e := validate.OptionalSlice(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + earlyReturn = true + } + if earlyReturn { + return // do not proceed + } + return + }(fldPath.Child("m3"), obj.M3, safe.Field(oldObj, func(oldObj *Struct) []string { return oldObj.M3 }), oldObj != nil)...) + + // field Struct.M4 + errs = append(errs, + func(fldPath *field.Path, obj, oldObj map[string]string, oldValueCorrelated bool) (errs field.ErrorList) { + // don't revalidate unchanged data + if oldValueCorrelated && op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { + return nil + } + // call field-attached validations + earlyReturn := false + if e := validate.OptionalMap(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + earlyReturn = true + } + if earlyReturn { + return // do not proceed + } + return + }(fldPath.Child("m4"), obj.M4, safe.Field(oldObj, func(oldObj *Struct) map[string]string { return oldObj.M4 }), oldObj != nil)...) + return errs } diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/undiscriminated/simple/doc.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/undiscriminated/simple/doc.go index 37fc82a85ad..8cc80bbe5de 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/undiscriminated/simple/doc.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/undiscriminated/simple/doc.go @@ -45,6 +45,14 @@ type Struct struct { // +k8s:unionMember // +k8s:optional M4 *string `json:"m4"` + + // +k8s:unionMember + // +k8s:optional + M5 []string `json:"m5"` + + // +k8s:unionMember + // +k8s:optional + M6 map[string]string `json:"m6"` } type M1 struct{} diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/undiscriminated/simple/doc_test.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/undiscriminated/simple/doc_test.go index 3ca566be821..8d7156d97d5 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/undiscriminated/simple/doc_test.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/undiscriminated/simple/doc_test.go @@ -34,6 +34,16 @@ func Test(t *testing.T) { st.Value(&Struct{M2: &M2{}}).ExpectValid() st.Value(&Struct{M3: "a string"}).ExpectValid() st.Value(&Struct{M4: ptr.To("a string")}).ExpectValid() + st.Value(&Struct{M5: []string{"a string"}}).ExpectValid() + st.Value(&Struct{M6: map[string]string{"k": "v"}}).ExpectValid() + + // Empty slice/map are "not set" (same as nil for union membership) + st.Value(&Struct{M5: []string{}}).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ + field.Invalid(nil, nil, "must specify one of"), + }.WithOrigin("union")) + st.Value(&Struct{M6: map[string]string{}}).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ + field.Invalid(nil, nil, "must specify one of"), + }.WithOrigin("union")) st.Value(&Struct{M1: &M1{}, M2: &M2{}}).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ field.Invalid(nil, nil, "must specify exactly one of"), @@ -53,6 +63,10 @@ func Test(t *testing.T) { st.Value(&Struct{M1: &M1{}, M2: &M2{}}).OldValue(&Struct{M1: &M1{}, M2: &M2{}}).ExpectValid() st.Value(&Struct{M3: "a string", M2: &M2{}}).OldValue(&Struct{M3: "different string", M2: &M2{}}).ExpectValid() + // Slice/map member ratcheting: unchanged membership, different values + st.Value(&Struct{M5: []string{"a"}}).OldValue(&Struct{M5: []string{"b"}}).ExpectValid() + st.Value(&Struct{M6: map[string]string{"k": "v1"}}).OldValue(&Struct{M6: map[string]string{"k": "v2"}}).ExpectValid() + // Test update with nil old value (simulates new map entry or newly-set pointer field during update). // Union validation should still detect the empty union even though oldObj is nil. st.Value(&Struct{}).OldValue(nil).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/undiscriminated/simple/zz_generated.validations.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/undiscriminated/simple/zz_generated.validations.go index 80c81dc083d..76eb02c919b 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/undiscriminated/simple/zz_generated.validations.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/undiscriminated/simple/zz_generated.validations.go @@ -25,6 +25,7 @@ import ( context "context" fmt "fmt" + equality "k8s.io/apimachinery/pkg/api/equality" operation "k8s.io/apimachinery/pkg/api/operation" safe "k8s.io/apimachinery/pkg/api/safe" validate "k8s.io/apimachinery/pkg/api/validate" @@ -48,7 +49,7 @@ func RegisterValidations(scheme *testscheme.Scheme) error { return nil } -var unionMembershipFor_k8s_io_code_generator_cmd_validation_gen_output_tests_tags_union_union_undiscriminated_simple_Struct_ = validate.NewUnionMembership(validate.NewUnionMember("m1"), validate.NewUnionMember("m2"), validate.NewUnionMember("m3"), validate.NewUnionMember("m4")) +var unionMembershipFor_k8s_io_code_generator_cmd_validation_gen_output_tests_tags_union_union_undiscriminated_simple_Struct_ = validate.NewUnionMembership(validate.NewUnionMember("m1"), validate.NewUnionMember("m2"), validate.NewUnionMember("m3"), validate.NewUnionMember("m4"), validate.NewUnionMember("m5"), validate.NewUnionMember("m6")) // Validate_Struct validates an instance of Struct according // to declarative validation rules in the API schema. @@ -74,6 +75,16 @@ func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field return false } return obj.M4 != nil + }, func(obj *Struct) bool { + if obj == nil { + return false + } + return len(obj.M5) != 0 + }, func(obj *Struct) bool { + if obj == nil { + return false + } + return len(obj.M6) != 0 })...) // field Struct.TypeMeta has no validation @@ -151,5 +162,41 @@ func Validate_Struct(ctx context.Context, op operation.Operation, fldPath *field return }(fldPath.Child("m4"), obj.M4, safe.Field(oldObj, func(oldObj *Struct) *string { return oldObj.M4 }), oldObj != nil)...) + // field Struct.M5 + errs = append(errs, + func(fldPath *field.Path, obj, oldObj []string, oldValueCorrelated bool) (errs field.ErrorList) { + // don't revalidate unchanged data + if oldValueCorrelated && op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { + return nil + } + // call field-attached validations + earlyReturn := false + if e := validate.OptionalSlice(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + earlyReturn = true + } + if earlyReturn { + return // do not proceed + } + return + }(fldPath.Child("m5"), obj.M5, safe.Field(oldObj, func(oldObj *Struct) []string { return oldObj.M5 }), oldObj != nil)...) + + // field Struct.M6 + errs = append(errs, + func(fldPath *field.Path, obj, oldObj map[string]string, oldValueCorrelated bool) (errs field.ErrorList) { + // don't revalidate unchanged data + if oldValueCorrelated && op.Type == operation.Update && equality.Semantic.DeepEqual(obj, oldObj) { + return nil + } + // call field-attached validations + earlyReturn := false + if e := validate.OptionalMap(ctx, op, fldPath, obj, oldObj); len(e) != 0 { + earlyReturn = true + } + if earlyReturn { + return // do not proceed + } + return + }(fldPath.Child("m6"), obj.M6, safe.Field(oldObj, func(oldObj *Struct) map[string]string { return oldObj.M6 }), oldObj != nil)...) + return errs } diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/validators/union.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/validators/union.go index 49e7aa5d60a..c7da756dbdf 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/validators/union.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/validators/union.go @@ -325,8 +325,10 @@ func createMemberExtractor(ptrType *types.Type, member *types.Member) FunctionLi } nt := util.NativeType(member.Type) switch nt.Kind { - case types.Pointer, types.Map, types.Slice: + case types.Pointer: extractor.Body = fmt.Sprintf("if obj == nil {return false}; return obj.%s != nil", member.Name) + case types.Map, types.Slice: + extractor.Body = fmt.Sprintf("if obj == nil {return false}; return len(obj.%s) != 0", member.Name) case types.Builtin: extractor.Body = fmt.Sprintf("if obj == nil {return false}; var z %s; return obj.%s != z", member.Type, member.Name) default: From d8a562b6476439fda82827fe1641dc2e307bc52d Mon Sep 17 00:00:00 2001 From: Lalit Chauhan Date: Fri, 20 Mar 2026 08:20:51 +0000 Subject: [PATCH 6/6] Fix backport differences for 1.35 (remove WithOrigin and MarkAlpha) --- .../declarative_validation_test.go | 2 +- .../pkg/api/validate/union_test.go | 12 ++++++------ .../union/discriminated/simple/doc_test.go | 18 +++++++++--------- .../union/undiscriminated/simple/doc_test.go | 6 +++--- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/pkg/registry/resource/resourceslice/declarative_validation_test.go b/pkg/registry/resource/resourceslice/declarative_validation_test.go index 6bc6adcbe1e..e19de1a046d 100644 --- a/pkg/registry/resource/resourceslice/declarative_validation_test.go +++ b/pkg/registry/resource/resourceslice/declarative_validation_test.go @@ -288,7 +288,7 @@ func TestDeclarativeValidateUpdate(t *testing.T) { old: mkResourceSliceWithDevices(), update: mkResourceSliceWithDevices(tweakDeviceAttribute("test.io/empty", resource.DeviceAttribute{})), expectedErrs: field.ErrorList{ - field.Invalid(field.NewPath("spec", "devices").Index(0).Child("attributes").Key("test.io/empty"), "", "").WithOrigin("union").MarkAlpha(), + field.Invalid(field.NewPath("spec", "devices").Index(0).Child("attributes").Key("test.io/empty"), "", ""), }, }, // spec.sharedCounters diff --git a/staging/src/k8s.io/apimachinery/pkg/api/validate/union_test.go b/staging/src/k8s.io/apimachinery/pkg/api/validate/union_test.go index 783ace1f6d4..42eec5eee89 100644 --- a/staging/src/k8s.io/apimachinery/pkg/api/validate/union_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/api/validate/union_test.go @@ -239,7 +239,7 @@ func TestUnionRatcheting(t *testing.T) { newStruct: &testStruct{}, expected: field.ErrorList{ field.Invalid(nil, "", "must specify one of: `m1`, `m2`, `m3`, `m4`"), - }.WithOrigin("union"), + }, }, { name: "both empty struct", @@ -268,7 +268,7 @@ func TestUnionRatcheting(t *testing.T) { }, expected: field.ErrorList{ field.Invalid(nil, "{m1, m2}", "must specify exactly one of: `m1`, `m2`, `m3`, `m4`"), - }.WithOrigin("union"), + }, }, { name: "slice member ratcheting: unchanged membership", @@ -286,7 +286,7 @@ func TestUnionRatcheting(t *testing.T) { newStruct: &testStruct{M3: []string{}}, expected: field.ErrorList{ field.Invalid(nil, "", "must specify one of: `m1`, `m2`, `m3`, `m4`"), - }.WithOrigin("union"), + }, }, { name: "empty map is not set", @@ -294,7 +294,7 @@ func TestUnionRatcheting(t *testing.T) { newStruct: &testStruct{M4: map[string]string{}}, expected: field.ErrorList{ field.Invalid(nil, "", "must specify one of: `m1`, `m2`, `m3`, `m4`"), - }.WithOrigin("union"), + }, }, } @@ -454,7 +454,7 @@ func TestDiscriminatedUnionRatcheting(t *testing.T) { }, expected: field.ErrorList{ field.Invalid(field.NewPath("m3"), "", "must be specified when `d` is \"m3\""), - }.WithOrigin("union"), + }, }, { name: "empty map is not set", @@ -468,7 +468,7 @@ func TestDiscriminatedUnionRatcheting(t *testing.T) { }, expected: field.ErrorList{ field.Invalid(field.NewPath("m4"), "", "must be specified when `d` is \"m4\""), - }.WithOrigin("union"), + }, }, } diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/discriminated/simple/doc_test.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/discriminated/simple/doc_test.go index 86c315efc77..b23f3eddb29 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/discriminated/simple/doc_test.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/discriminated/simple/doc_test.go @@ -43,30 +43,30 @@ func Test(t *testing.T) { // Slice member: discriminator matches but slice is nil st.Value(&Struct{D: DM3}).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ field.Invalid(field.NewPath("m3"), nil, "must be specified when"), - }.WithOrigin("union")) + }) // Slice member: discriminator matches but slice is empty (len==0, not nil) st.Value(&Struct{D: DM3, M3: []string{}}).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ field.Invalid(field.NewPath("m3"), nil, "must be specified when"), - }.WithOrigin("union")) + }) // Slice member: discriminator doesn't match but slice is set st.Value(&Struct{D: DM1, M1: &M1{}, M3: []string{"a"}}).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ field.Invalid(field.NewPath("m3"), nil, "may only be specified when"), - }.WithOrigin("union")) + }) // Map member: discriminator matches but map is nil st.Value(&Struct{D: DM4}).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ field.Invalid(field.NewPath("m4"), nil, "must be specified when"), - }.WithOrigin("union")) + }) // Map member: discriminator matches but map is empty (len==0, not nil) st.Value(&Struct{D: DM4, M4: map[string]string{}}).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ field.Invalid(field.NewPath("m4"), nil, "must be specified when"), - }.WithOrigin("union")) + }) // Map member: discriminator doesn't match but map is set st.Value(&Struct{D: DM1, M1: &M1{}, M4: map[string]string{"k": "v"}}).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ field.Invalid(field.NewPath("m4"), nil, "may only be specified when"), - }.WithOrigin("union")) + }) // Test validation ratcheting st.Value(&Struct{D: DM2, M1: &M1{}, M2: &M2{}}).OldValue(&Struct{D: DM2, M1: &M1{}, M2: &M2{}}).ExpectValid() @@ -77,18 +77,18 @@ func Test(t *testing.T) { // Slice member ratcheting: changed from set to unset st.Value(&Struct{D: DM3}).OldValue(&Struct{D: DM3, M3: []string{"a"}}).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ field.Invalid(field.NewPath("m3"), nil, "must be specified when"), - }.WithOrigin("union")) + }) // Map member ratcheting: unchanged membership st.Value(&Struct{D: DM4, M4: map[string]string{"k": "v1"}}).OldValue(&Struct{D: DM4, M4: map[string]string{"k": "v2"}}).ExpectValid() // Map member ratcheting: changed from set to unset st.Value(&Struct{D: DM4}).OldValue(&Struct{D: DM4, M4: map[string]string{"k": "v"}}).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ field.Invalid(field.NewPath("m4"), nil, "must be specified when"), - }.WithOrigin("union")) + }) // Test update with nil old value (simulates newly-set pointer field during update). // Discriminated union validation should still detect mismatches even though oldObj is nil. st.Value(&Struct{D: DM1}).OldValue(nil).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ field.Invalid(field.NewPath("m1"), nil, "must be specified when"), - }.WithOrigin("union")) + }) } diff --git a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/undiscriminated/simple/doc_test.go b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/undiscriminated/simple/doc_test.go index 8d7156d97d5..466be2575f8 100644 --- a/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/undiscriminated/simple/doc_test.go +++ b/staging/src/k8s.io/code-generator/cmd/validation-gen/output_tests/tags/union/union/undiscriminated/simple/doc_test.go @@ -40,10 +40,10 @@ func Test(t *testing.T) { // Empty slice/map are "not set" (same as nil for union membership) st.Value(&Struct{M5: []string{}}).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ field.Invalid(nil, nil, "must specify one of"), - }.WithOrigin("union")) + }) st.Value(&Struct{M6: map[string]string{}}).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ field.Invalid(nil, nil, "must specify one of"), - }.WithOrigin("union")) + }) st.Value(&Struct{M1: &M1{}, M2: &M2{}}).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ field.Invalid(nil, nil, "must specify exactly one of"), @@ -70,6 +70,6 @@ func Test(t *testing.T) { // Test update with nil old value (simulates new map entry or newly-set pointer field during update). // Union validation should still detect the empty union even though oldObj is nil. st.Value(&Struct{}).OldValue(nil).ExpectMatches(field.ErrorMatcher{}.ByType().ByField().ByDetailSubstring().ByOrigin(), field.ErrorList{ - field.Invalid(nil, nil, "must specify one of").WithOrigin("union"), + field.Invalid(nil, nil, "must specify one of"), }) }