github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2026-07-18 04:54:54 +00:00
Code Issues Projects Releases Wiki Activity

refactor: Remove Validate(Update)Declaratively and improve error handling

Browse Source
This commit is contained in:
yongruilin
2025-09-19 19:58:26 +00:00
parent d2ec0308c7
commit 9a7e2291d1
3 changed files with 21 additions and 92 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
pkg/apis/autoscaling/validation/declarative_validation_test.go
View File

@@ -20,9 +20,9 @@ import (
"testing"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/util/validation/field"
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/registry/rest"
"k8s.io/kubernetes/pkg/api/legacyscheme"
apitesting "k8s.io/kubernetes/pkg/api/testing"
"k8s.io/kubernetes/pkg/apis/autoscaling"
@@ -30,9 +30,11 @@ import (
// TestScaleDeclarativeValidation verifies that the validation rules that are applied uniformly to the Scale API.
func TestScaleDeclarativeValidation(t *testing.T) {
apiGroup := "autoscaling"
apiVersion := "v1"
ctx := genericapirequest.WithRequestInfo(genericapirequest.NewDefaultContext(), &genericapirequest.RequestInfo{
APIGroup: "autoscaling",
APIVersion: "v1",
APIGroup: apiGroup,
APIVersion: apiVersion,
Subresource: "scale",
})
@@ -61,7 +63,8 @@ func TestScaleDeclarativeValidation(t *testing.T) {
// All resources that have a scale subresource are expected to test Scale validation against any handwritten
// validation code defined on that resource.
tester := field.ErrorMatcher{}.ByType().ByField().ByOrigin()
tester.Test(t, rest.ValidateDeclaratively(ctx, legacyscheme.Scheme, &tc.input), tc.expectedErrs)
obj, _ := legacyscheme.Scheme.ConvertToVersion(&tc.input, schema.GroupVersion{Group: apiGroup, Version: apiVersion})
tester.Test(t, tc.expectedErrs, legacyscheme.Scheme.Validate(ctx, nil, obj, "scale"))
apitesting.VerifyVersionedValidationEquivalence(t, &tc.input, nil, "scale")
})

37
staging/src/k8s.io/apiserver/pkg/registry/rest/validate.go
View File

@@ -90,28 +90,7 @@ type validationConfigOption struct {
validationIdentifier string
}
// ValidateDeclaratively validates obj against declarative validation tags
// defined in its Go type. It uses the API version extracted from ctx and the
// provided scheme for validation.
//
// The ctx MUST contain requestInfo, which determines the target API for
// validation. The obj is converted to the API version using the provided scheme
// before validation occurs. The scheme MUST have the declarative validation
// registered for the requested resource/subresource.
//
// Returns a field.ErrorList containing any validation errors. An internal error
// is included if requestInfo is missing from the context or if version
// conversion fails.
func ValidateDeclaratively(ctx context.Context, scheme *runtime.Scheme, obj runtime.Object, configOpts ...ValidationConfig) field.ErrorList {
cfg := &validationConfigOption{opType: operation.Create}
for _, o := range configOpts {
o(cfg)
}
return panicSafeValidateFunc(validateDeclaratively, cfg.takeover, cfg.validationIdentifier)(ctx, scheme, obj, nil, cfg)
}
// ValidateUpdateDeclaratively validates obj and oldObj against declarative
// validateDeclaratively validates obj and oldObj against declarative
// validation tags defined in its Go type. It uses the API version extracted from
// ctx and the provided scheme for validation.
//
@@ -123,14 +102,6 @@ func ValidateDeclaratively(ctx context.Context, scheme *runtime.Scheme, obj runt
// Returns a field.ErrorList containing any validation errors. An internal error
// is included if requestInfo is missing from the context or if version
// conversion fails.
func ValidateUpdateDeclaratively(ctx context.Context, scheme *runtime.Scheme, obj, oldObj runtime.Object, configOpts ...ValidationConfig) field.ErrorList {
cfg := &validationConfigOption{opType: operation.Update}
for _, o := range configOpts {
o(cfg)
}
return panicSafeValidateFunc(validateDeclaratively, cfg.takeover, cfg.validationIdentifier)(ctx, scheme, obj, oldObj, cfg)
}
func validateDeclaratively(ctx context.Context, scheme *runtime.Scheme, obj, oldObj runtime.Object, o *validationConfigOption) field.ErrorList {
// Find versionedGroupVersion, which identifies the API version to use for declarative validation.
versionedGroupVersion, subresources, err := requestInfo(ctx, o.subresourceGVKMapper)
@@ -187,9 +158,9 @@ func parseSubresourcePath(subresourcePath string) ([]string, error) {
return parts, nil
}
// CompareDeclarativeErrorsAndEmitMismatches checks for mismatches between imperative and declarative validation
// compareDeclarativeErrorsAndEmitMismatches checks for mismatches between imperative and declarative validation
// and logs + emits metrics when inconsistencies are found
func CompareDeclarativeErrorsAndEmitMismatches(ctx context.Context, imperativeErrs, declarativeErrs field.ErrorList, takeover bool, validationIdentifier string) {
func compareDeclarativeErrorsAndEmitMismatches(ctx context.Context, imperativeErrs, declarativeErrs field.ErrorList, takeover bool, validationIdentifier string) {
logger := klog.FromContext(ctx)
mismatchDetails := gatherDeclarativeValidationMismatches(imperativeErrs, declarativeErrs, takeover)
for _, detail := range mismatchDetails {
@@ -394,7 +365,7 @@ func ValidateDeclarativelyWithMigrationChecks(ctx context.Context, scheme *runti
// Call the panic-safe wrapper with the real validation function.
declarativeErrs := panicSafeValidateFunc(validateDeclaratively, cfg.takeover, cfg.validationIdentifier)(ctx, scheme, obj, oldObj, cfg)
CompareDeclarativeErrorsAndEmitMismatches(ctx, errs, declarativeErrs, takeover, validationIdentifier)
compareDeclarativeErrorsAndEmitMismatches(ctx, errs, declarativeErrs, takeover, validationIdentifier)
if takeover {
errs = append(errs.RemoveCoveredByDeclarative(), declarativeErrs...)

65
staging/src/k8s.io/apiserver/pkg/registry/rest/validate_test.go
View File

@@ -162,12 +162,16 @@ func TestValidateDeclaratively(t *testing.T) {
Subresource: tc.subresource,
})
t.Run(tc.name, func(t *testing.T) {
var results field.ErrorList
if tc.oldObject == nil {
results = ValidateDeclaratively(ctx, scheme, tc.object, WithOptions(tc.options))
} else {
results = ValidateUpdateDeclaratively(ctx, scheme, tc.object, tc.oldObject, WithOptions(tc.options))
cfg := &validationConfigOption{
options: tc.options,
}
if tc.oldObject == nil {
cfg.opType = operation.Create
} else {
cfg.opType = operation.Update
}
results := panicSafeValidateFunc(validateDeclaratively, cfg.takeover, cfg.validationIdentifier)(ctx, scheme, tc.object, tc.oldObject, cfg)
matcher := field.ErrorMatcher{}.ByType().ByField().ByOrigin()
matcher.Test(t, tc.expected, results)
})
@@ -425,7 +429,7 @@ func TestCompareDeclarativeErrorsAndEmitMismatches(t *testing.T) {
defer klog.LogToStderr(true)
ctx := context.Background()
CompareDeclarativeErrorsAndEmitMismatches(ctx, tc.imperativeErrs, tc.declarativeErrs, tc.takeover, "test_validationIdentifier")
compareDeclarativeErrorsAndEmitMismatches(ctx, tc.imperativeErrs, tc.declarativeErrs, tc.takeover, "test_validationIdentifier")
klog.Flush()
logOutput := buf.String()
@@ -632,55 +636,6 @@ func TestWithRecoverUpdate(t *testing.T) {
}
}
func TestValidateDeclarativelyWithRecovery(t *testing.T) {
ctx := context.Background()
scheme := runtime.NewScheme()
var options []string
obj := &runtime.Unknown{}
// Simple test for the ValidateDeclarativelyWithRecovery function
t.Run("with takeover disabled", func(t *testing.T) {
errs := ValidateDeclaratively(ctx, scheme, obj, WithOptions(options), WithTakeover(false))
if errs == nil {
// This is expected to error since the request info is missing
t.Errorf("Expected errors but got nil")
}
})
t.Run("with takeover enabled", func(t *testing.T) {
errs := ValidateDeclaratively(ctx, scheme, obj, WithOptions(options), WithTakeover(true))
if errs == nil {
// This is expected to error since the request info is missioptionsng
t.Errorf("Expected errors but got nil")
}
})
}
func TestValidateUpdateDeclarativelyWithRecovery(t *testing.T) {
ctx := context.Background()
scheme := runtime.NewScheme()
var options []string
obj := &runtime.Unknown{}
oldObj := &runtime.Unknown{}
// Simple test for the ValidateUpdateDeclarativelyWithRecovery function
t.Run("with takeover disabled", func(t *testing.T) {
errs := ValidateUpdateDeclaratively(ctx, scheme, obj, oldObj, WithOptions(options), WithTakeover(false))
if errs == nil {
// This is expected to error since the request info is missing
t.Errorf("Expected errors but got nil")
}
})
t.Run("with takeover enabled", func(t *testing.T) {
errs := ValidateUpdateDeclaratively(ctx, scheme, obj, oldObj, WithOptions(options), WithTakeover(true))
if errs == nil {
// This is expected to error since the request info is missing
t.Errorf("Expected errors but got nil")
}
})
}
func TestRecordDuplicateValidationErrors(t *testing.T) {
ctx := context.Background()