diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/validate.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/validate.go index 02f01ce9526..09993eb4563 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/validate.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/validate.go @@ -116,7 +116,7 @@ func CompareDeclarativeErrorsAndEmitMismatches(ctx context.Context, imperativeEr mismatchDetails := gatherDeclarativeValidationMismatches(imperativeErrs, declarativeErrs, takeover) for _, detail := range mismatchDetails { // Log information about the mismatch using contextual logger - logger.Info(detail) + logger.Error(nil, detail) // Increment the metric for the mismatch validationmetrics.Metrics.IncDeclarativeValidationMismatchMetric() @@ -231,20 +231,20 @@ func createDeclarativeValidationPanicHandler(ctx context.Context, errs *field.Er // If takeover is enabled, output as a validation error as authoritative validator panicked and validation should error *errs = append(*errs, field.InternalError(nil, fmt.Errorf(errorFmt, r))) } else { - // if takeover not enabled, log the panic as an info message - logger.Info(fmt.Sprintf(errorFmt, r)) + // if takeover not enabled, log the panic as an error message + logger.Error(nil, fmt.Sprintf(errorFmt, r)) } } } } -// withRecover wraps a validation function with panic recovery logic. +// panicSafeValidateFunc wraps a validation function with panic recovery logic. // It takes a validation function with the ValidateDeclaratively signature // and returns a function with the same signature. // The returned function will execute the wrapped function and handle any panics by // incrementing the panic metric, and logging an error message // if takeover=false, and adding a validation error if takeover=true. -func withRecover( +func panicSafeValidateFunc( validateFunc func(ctx context.Context, options sets.Set[string], scheme *runtime.Scheme, obj runtime.Object) field.ErrorList, takeover bool, ) func(ctx context.Context, options sets.Set[string], scheme *runtime.Scheme, obj runtime.Object) field.ErrorList { @@ -255,13 +255,13 @@ func withRecover( } } -// withRecoverUpdate wraps an update validation function with panic recovery logic. +// panicSafeValidateUpdateFunc wraps an update validation function with panic recovery logic. // It takes a validation function with the ValidateUpdateDeclaratively signature // and returns a function with the same signature. // The returned function will execute the wrapped function and handle any panics by // incrementing the panic metric, and logging an error message // if takeover=false, and adding a validation error if takeover=true. -func withRecoverUpdate( +func panicSafeValidateUpdateFunc( validateUpdateFunc func(ctx context.Context, options sets.Set[string], scheme *runtime.Scheme, obj, oldObj runtime.Object) field.ErrorList, takeover bool, ) func(ctx context.Context, options sets.Set[string], scheme *runtime.Scheme, obj, oldObj runtime.Object) field.ErrorList { @@ -292,7 +292,7 @@ func withRecoverUpdate( // conversion fails, or if a panic occurs during validation when // takeover is true. func ValidateDeclarativelyWithRecovery(ctx context.Context, options sets.Set[string], scheme *runtime.Scheme, obj runtime.Object, takeover bool) field.ErrorList { - return withRecover(ValidateDeclaratively, takeover)(ctx, options, scheme, obj) + return panicSafeValidateFunc(ValidateDeclaratively, takeover)(ctx, options, scheme, obj) } // ValidateUpdateDeclarativelyWithRecovery validates obj and oldObj against declarative @@ -315,5 +315,5 @@ func ValidateDeclarativelyWithRecovery(ctx context.Context, options sets.Set[str // conversion fails, or if a panic occurs during validation when // takeover is true. func ValidateUpdateDeclarativelyWithRecovery(ctx context.Context, options sets.Set[string], scheme *runtime.Scheme, obj, oldObj runtime.Object, takeover bool) field.ErrorList { - return withRecoverUpdate(ValidateUpdateDeclaratively, takeover)(ctx, options, scheme, obj, oldObj) + return panicSafeValidateUpdateFunc(ValidateUpdateDeclaratively, takeover)(ctx, options, scheme, obj, oldObj) } diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/validate_test.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/validate_test.go index 642901d6c93..2e419d46125 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/validate_test.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/validate_test.go @@ -394,8 +394,8 @@ func TestCompareDeclarativeErrorsAndEmitMismatches(t *testing.T) { declarativeErrs: field.ErrorList{errA}, takeover: true, expectLogs: true, - // logs have a prefix of the form - I0309 21:05:33.865030 1926106 validate.go:199] - expectedRegex: "I.*Unexpected difference between hand written validation and declarative validation error results.*Consider disabling the DeclarativeValidationTakeover feature gate to keep data persisted in etcd consistent with prior versions of Kubernetes", + // logs have a prefix of the form - E0309 21:05:33.865030 1926106 validate.go:199] + expectedRegex: "E.*Unexpected difference between hand written validation and declarative validation error results.*Consider disabling the DeclarativeValidationTakeover feature gate to keep data persisted in etcd consistent with prior versions of Kubernetes", }, { name: "matching errors, don't log info", @@ -468,8 +468,8 @@ func TestWithRecover(t *testing.T) { }, takeoverEnabled: false, wantErrs: nil, - // logs have a prefix of the form - I0309 21:05:33.865030 1926106 validate.go:199] - expectLogRegex: "I.*panic during declarative validation: test panic", + // logs have a prefix of the form - E0309 21:05:33.865030 1926106 validate.go:199] + expectLogRegex: "E.*panic during declarative validation: test panic", }, { name: "panic with takeover enabled", @@ -500,8 +500,8 @@ func TestWithRecover(t *testing.T) { klog.LogToStderr(false) defer klog.LogToStderr(true) - // Pass the takeover flag to withRecover instead of relying on the feature gate - wrapped := withRecover(tc.validateFn, tc.takeoverEnabled) + // Pass the takeover flag to panicSafeValidateFunc instead of relying on the feature gate + wrapped := panicSafeValidateFunc(tc.validateFn, tc.takeoverEnabled) gotErrs := wrapped(ctx, options, scheme, obj) klog.Flush() @@ -509,7 +509,7 @@ func TestWithRecover(t *testing.T) { // Compare gotErrs vs. tc.wantErrs if !equalErrorLists(gotErrs, tc.wantErrs) { - t.Errorf("withRecover() gotErrs = %#v, want %#v", gotErrs, tc.wantErrs) + t.Errorf("panicSafeValidateFunc() gotErrs = %#v, want %#v", gotErrs, tc.wantErrs) } // Check logs if needed @@ -562,8 +562,8 @@ func TestWithRecoverUpdate(t *testing.T) { }, takeoverEnabled: false, wantErrs: nil, - // logs have a prefix of the form - I0309 21:05:33.865030 1926106 validate.go:199] - expectLogRegex: "I.*panic during declarative validation: test update panic", + // logs have a prefix of the form - E0309 21:05:33.865030 1926106 validate.go:199] + expectLogRegex: "E.*panic during declarative validation: test update panic", }, { name: "panic with takeover enabled", @@ -594,8 +594,8 @@ func TestWithRecoverUpdate(t *testing.T) { klog.LogToStderr(false) defer klog.LogToStderr(true) - // Pass the takeover flag to withRecoverUpdate instead of relying on the feature gate - wrapped := withRecoverUpdate(tc.validateFn, tc.takeoverEnabled) + // Pass the takeover flag to panicSafeValidateUpdateFunc instead of relying on the feature gate + wrapped := panicSafeValidateUpdateFunc(tc.validateFn, tc.takeoverEnabled) gotErrs := wrapped(ctx, options, scheme, obj, oldObj) klog.Flush() @@ -603,7 +603,7 @@ func TestWithRecoverUpdate(t *testing.T) { // Compare gotErrs with wantErrs if !equalErrorLists(gotErrs, tc.wantErrs) { - t.Errorf("withRecoverUpdate() gotErrs = %#v, want %#v", gotErrs, tc.wantErrs) + t.Errorf("panicSafeValidateUpdateFunc() gotErrs = %#v, want %#v", gotErrs, tc.wantErrs) } // Verify log output