From c9e18abca5e3efc7b9f3410e2dad983625dfd926 Mon Sep 17 00:00:00 2001 From: yongruilin Date: Wed, 18 Mar 2026 22:25:32 +0000 Subject: [PATCH] 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 488e7db83b7..bbc1d1d3095 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.WithOrigin("union") @@ -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 4ad415cbcf3..a8132e3036a 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",