Skip to content

Commit d95710f

Browse files
yongruilink8s-publishing-bot
authored andcommitted
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). Kubernetes-commit: c9e18abca5e3efc7b9f3410e2dad983625dfd926
1 parent 729062d commit d95710f

2 files changed

Lines changed: 13 additions & 7 deletions

File tree

pkg/api/validate/union.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package validate
1919
import (
2020
"context"
2121
"fmt"
22+
"reflect"
2223
"strings"
2324

2425
"k8s.io/apimachinery/pkg/api/operation"
@@ -106,9 +107,10 @@ func DiscriminatedUnion[T any, D ~string](_ context.Context, op operation.Operat
106107
len(isSetFns), len(union.members))),
107108
}
108109
}
110+
hasOldValue := !reflect.ValueOf(oldObj).IsNil()
109111
var changed bool
110112
discriminatorValue := discriminatorExtractor(obj)
111-
if op.Type == operation.Update {
113+
if op.Type == operation.Update && hasOldValue {
112114
oldDiscriminatorValue := discriminatorExtractor(oldObj)
113115
changed = discriminatorValue != oldDiscriminatorValue
114116
}
@@ -117,7 +119,7 @@ func DiscriminatedUnion[T any, D ~string](_ context.Context, op operation.Operat
117119
member := union.members[i]
118120
isDiscriminatedMember := string(discriminatorValue) == member.discriminatorValue
119121
newIsSet := fieldIsSet(obj)
120-
if op.Type == operation.Update && !changed {
122+
if op.Type == operation.Update && hasOldValue && !changed {
121123
oldIsSet := fieldIsSet(oldObj)
122124
changed = changed || newIsSet != oldIsSet
123125
}
@@ -131,7 +133,7 @@ func DiscriminatedUnion[T any, D ~string](_ context.Context, op operation.Operat
131133
}
132134
// If the union discriminator and membership is unchanged, we don't need to
133135
// re-validate.
134-
if op.Type == operation.Update && !changed {
136+
if op.Type == operation.Update && hasOldValue && !changed {
135137
return nil
136138
}
137139
return errs.WithOrigin("union")
@@ -195,11 +197,12 @@ func unionValidate[T any](op operation.Operation, fldPath *field.Path,
195197
}
196198
}
197199

200+
hasOldValue := !reflect.ValueOf(oldObj).IsNil()
198201
var specifiedFields []string
199202
var changed bool
200203
for i, fieldIsSet := range isSetFns {
201204
newIsSet := fieldIsSet(obj)
202-
if op.Type == operation.Update && !changed {
205+
if op.Type == operation.Update && hasOldValue && !changed {
203206
oldIsSet := fieldIsSet(oldObj)
204207
changed = changed || newIsSet != oldIsSet
205208
}
@@ -209,7 +212,7 @@ func unionValidate[T any](op operation.Operation, fldPath *field.Path,
209212
}
210213

211214
// If the union membership is unchanged, we don't need to re-validate.
212-
if op.Type == operation.Update && !changed {
215+
if op.Type == operation.Update && hasOldValue && !changed {
213216
return nil
214217
}
215218

pkg/api/validate/union_test.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,9 +186,12 @@ func TestUnionRatcheting(t *testing.T) {
186186
expected field.ErrorList
187187
}{
188188
{
189-
name: "both nil",
189+
name: "old nil - no ratcheting",
190190
oldStruct: nil,
191-
newStruct: nil,
191+
newStruct: &testStruct{},
192+
expected: field.ErrorList{
193+
field.Invalid(nil, "", "must specify one of: `m1`, `m2`"),
194+
}.WithOrigin("union"),
192195
},
193196
{
194197
name: "both empty struct",

0 commit comments

Comments
 (0)