kubernetes
/
apiserver
mirror of https://github.com/kubernetes/apiserver.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #130754 from aaron-prindle/validation-gen-add-metric-and-runtime-verification-review-comments-upstream 

[Declarative Validation] chore: change Info->Error log level related to declarative validation runtime tests and refactor panic wrapper names

Kubernetes-commit: 020c4b7c655f63cd0ab1b8466492da528961f930
This commit is contained in:
Kubernetes Publisher 2025-03-13 13:28:02 -07:00
parent c31bd9905e 4fe686a430
commit d88392f1f6
4 changed files with 24 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
go.mod
View File

@ -48,7 +48,7 @@ require (
gopkg.in/evanphx/json-patch.v4 v4.12.0
gopkg.in/go-jose/go-jose.v2 v2.6.3
gopkg.in/natefinch/lumberjack.v2 v2.2.1
k8s.io/api v0.0.0-20250313213104-173e173fb2b8
k8s.io/api v0.0.0-20250313213105-aae61a387040
k8s.io/apimachinery v0.0.0-20250313012745-a04ff375cef8
k8s.io/client-go v0.0.0-20250313213513-7faeb317c7cb
k8s.io/component-base v0.0.0-20250313134451-21955e6fb6de

4
go.sum
View File

@ -367,8 +367,8 @@ gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
k8s.io/api v0.0.0-20250313213104-173e173fb2b8 h1:Z7fpBA1zEvgJJjIhketIRekQDQzUp67A3ou5MQZBBFk=
k8s.io/api v0.0.0-20250313213104-173e173fb2b8/go.mod h1:nyT7xKOld0k8IraPSyWNF96YIMppoRBYxtAUIehRQW0=
k8s.io/api v0.0.0-20250313213105-aae61a387040 h1:t4Fsq5t8BkN81dKsvJL0PobJ7UcEONapi8OEL7GKxdo=
k8s.io/api v0.0.0-20250313213105-aae61a387040/go.mod h1:nyT7xKOld0k8IraPSyWNF96YIMppoRBYxtAUIehRQW0=
k8s.io/apimachinery v0.0.0-20250313012745-a04ff375cef8 h1:aZ3lyC/cbDqhg3/RCSobSABNBeUzBWh6LrHeSnIVzVY=
k8s.io/apimachinery v0.0.0-20250313012745-a04ff375cef8/go.mod h1:S2OIkExGqJOXYSYcAJwQ9zWcc6BkBUdTJUu4M7z0cvo=
k8s.io/client-go v0.0.0-20250313213513-7faeb317c7cb h1:VQrMVHfIw2UYOoHRKv3IVq+0zOreSzDJNrqq1cucnUk=

18
pkg/registry/rest/validate.go
View File

@ -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)
}

24
pkg/registry/rest/validate_test.go
View File

@ -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