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 <thockin@google.com>
This commit is contained in:
yongruilin
2026-03-19 00:08:14 +00:00
committed by Lalit Chauhan
parent 3d39627cd9
commit b6ee759d8e
9 changed files with 291 additions and 15 deletions

View File

@@ -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) {

View File

@@ -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) {

View File

@@ -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{}

View File

@@ -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{

View File

@@ -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
}

View File

@@ -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{}

View File

@@ -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{

View File

@@ -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
}

View File

@@ -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: