Fix union validation ratcheting when oldObj is nil

yongruilin · k8s-publishing-bot · commit a1282302f0a1 · 2026-03-18T22:25:32.000Z
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: 9a39e5c49faecdf0be53bbdec2d2751799aa774d
diff --git a/pkg/api/validate/union.go b/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/pkg/api/validate/union_test.go b/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",

Original file line number	Diff line number	Diff line change
`@@ -19,6 +19,7 @@ package validate`
`19`	`19`	`import (`
`20`	`20`	`"context"`
`21`	`21`	`"fmt"`
	`22`	`+ "reflect"`
`22`	`23`	`"strings"`
`23`	`24`
`24`	`25`	`"k8s.io/apimachinery/pkg/api/operation"`
`@@ -106,9 +107,10 @@ func DiscriminatedUnion[T any, D ~string](_ context.Context, op operation.Operat`
`106`	`107`	`len(isSetFns), len(union.members))),`
`107`	`108`	`}`
`108`	`109`	`}`
	`110`	`+ hasOldValue := !reflect.ValueOf(oldObj).IsNil()`
`109`	`111`	`var changed bool`
`110`	`112`	`discriminatorValue := discriminatorExtractor(obj)`
`111`		`- if op.Type == operation.Update {`
	`113`	`+ if op.Type == operation.Update && hasOldValue {`
`112`	`114`	`oldDiscriminatorValue := discriminatorExtractor(oldObj)`
`113`	`115`	`changed = discriminatorValue != oldDiscriminatorValue`
`114`	`116`	`}`
`@@ -117,7 +119,7 @@ func DiscriminatedUnion[T any, D ~string](_ context.Context, op operation.Operat`
`117`	`119`	`member := union.members[i]`
`118`	`120`	`isDiscriminatedMember := string(discriminatorValue) == member.discriminatorValue`
`119`	`121`	`newIsSet := fieldIsSet(obj)`
`120`		`- if op.Type == operation.Update && !changed {`
	`122`	`+ if op.Type == operation.Update && hasOldValue && !changed {`
`121`	`123`	`oldIsSet := fieldIsSet(oldObj)`
`122`	`124`	`changed = changed \|\| newIsSet != oldIsSet`
`123`	`125`	`}`
`@@ -131,7 +133,7 @@ func DiscriminatedUnion[T any, D ~string](_ context.Context, op operation.Operat`
`131`	`133`	`}`
`132`	`134`	`// If the union discriminator and membership is unchanged, we don't need to`
`133`	`135`	`// re-validate.`
`134`		`- if op.Type == operation.Update && !changed {`
	`136`	`+ if op.Type == operation.Update && hasOldValue && !changed {`
`135`	`137`	`return nil`
`136`	`138`	`}`
`137`	`139`	`return errs`
`@@ -195,11 +197,12 @@ func unionValidate[T any](op operation.Operation, fldPath *field.Path,`
`195`	`197`	`}`
`196`	`198`	`}`
`197`	`199`
	`200`	`+ hasOldValue := !reflect.ValueOf(oldObj).IsNil()`
`198`	`201`	`var specifiedFields []string`
`199`	`202`	`var changed bool`
`200`	`203`	`for i, fieldIsSet := range isSetFns {`
`201`	`204`	`newIsSet := fieldIsSet(obj)`
`202`		`- if op.Type == operation.Update && !changed {`
	`205`	`+ if op.Type == operation.Update && hasOldValue && !changed {`
`203`	`206`	`oldIsSet := fieldIsSet(oldObj)`
`204`	`207`	`changed = changed \|\| newIsSet != oldIsSet`
`205`	`208`	`}`
`@@ -209,7 +212,7 @@ func unionValidate[T any](op operation.Operation, fldPath *field.Path,`
`209`	`212`	`}`
`210`	`213`
`211`	`214`	`// 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 {`
`213`	`216`	`return nil`
`214`	`217`	`}`
`215`	`218`