mirror of
https://github.com/k3s-io/kubernetes.git
synced 2025-08-04 18:00:08 +00:00
Merge pull request #130705 from aaron-prindle/validation-gen-add-metric-and-runtime-verification-upstream
[Declarative Validation] feat: add declarative validation metrics and associated runtime verification tests
This commit is contained in:
commit
21f7eaa8e2
@ -39,7 +39,6 @@ import (
|
|||||||
"k8s.io/apimachinery/pkg/util/sets"
|
"k8s.io/apimachinery/pkg/util/sets"
|
||||||
"k8s.io/apimachinery/pkg/util/validation"
|
"k8s.io/apimachinery/pkg/util/validation"
|
||||||
"k8s.io/apimachinery/pkg/util/validation/field"
|
"k8s.io/apimachinery/pkg/util/validation/field"
|
||||||
fldtest "k8s.io/apimachinery/pkg/util/validation/field/testing"
|
|
||||||
"k8s.io/apimachinery/pkg/util/version"
|
"k8s.io/apimachinery/pkg/util/version"
|
||||||
utilfeature "k8s.io/apiserver/pkg/util/feature"
|
utilfeature "k8s.io/apiserver/pkg/util/feature"
|
||||||
"k8s.io/component-base/featuregate"
|
"k8s.io/component-base/featuregate"
|
||||||
@ -16714,7 +16713,7 @@ func TestValidateReplicationControllerUpdate(t *testing.T) {
|
|||||||
tc.old.ObjectMeta.ResourceVersion = "1"
|
tc.old.ObjectMeta.ResourceVersion = "1"
|
||||||
tc.update.ObjectMeta.ResourceVersion = "1"
|
tc.update.ObjectMeta.ResourceVersion = "1"
|
||||||
errs := ValidateReplicationControllerUpdate(&tc.update, &tc.old, PodValidationOptions{})
|
errs := ValidateReplicationControllerUpdate(&tc.update, &tc.old, PodValidationOptions{})
|
||||||
matcher := fldtest.ErrorMatcher{}.ByType().ByField().ByOrigin().ByDetailSubstring()
|
matcher := field.ErrorMatcher{}.ByType().ByField().ByOrigin().ByDetailSubstring()
|
||||||
matcher.Test(t, tc.expectedErrs, errs)
|
matcher.Test(t, tc.expectedErrs, errs)
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
@ -16864,7 +16863,7 @@ func TestValidateReplicationController(t *testing.T) {
|
|||||||
for k, tc := range errorCases {
|
for k, tc := range errorCases {
|
||||||
t.Run(k, func(t *testing.T) {
|
t.Run(k, func(t *testing.T) {
|
||||||
errs := ValidateReplicationController(&tc.input, PodValidationOptions{})
|
errs := ValidateReplicationController(&tc.input, PodValidationOptions{})
|
||||||
matcher := fldtest.ErrorMatcher{}.ByType().ByField().ByOrigin().ByDetailSubstring()
|
matcher := field.ErrorMatcher{}.ByType().ByField().ByOrigin().ByDetailSubstring()
|
||||||
matcher.Test(t, tc.expectedErrs, errs)
|
matcher.Test(t, tc.expectedErrs, errs)
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
@ -20770,7 +20769,7 @@ func TestValidateEndpointsCreate(t *testing.T) {
|
|||||||
t.Run(k, func(t *testing.T) {
|
t.Run(k, func(t *testing.T) {
|
||||||
errs := ValidateEndpointsCreate(&v.endpoints)
|
errs := ValidateEndpointsCreate(&v.endpoints)
|
||||||
// TODO: set .RequireOriginWhenInvalid() once metadata is done
|
// TODO: set .RequireOriginWhenInvalid() once metadata is done
|
||||||
matcher := fldtest.ErrorMatcher{}.ByType().ByField().ByOrigin()
|
matcher := field.ErrorMatcher{}.ByType().ByField().ByOrigin()
|
||||||
matcher.Test(t, v.expectedErrs, errs)
|
matcher.Test(t, v.expectedErrs, errs)
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
@ -22767,7 +22766,7 @@ func TestValidateTopologySpreadConstraints(t *testing.T) {
|
|||||||
for _, tc := range testCases {
|
for _, tc := range testCases {
|
||||||
t.Run(tc.name, func(t *testing.T) {
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
errs := validateTopologySpreadConstraints(tc.constraints, fieldPath, tc.opts)
|
errs := validateTopologySpreadConstraints(tc.constraints, fieldPath, tc.opts)
|
||||||
matcher := fldtest.ErrorMatcher{}.ByType().ByField().ByOrigin().RequireOriginWhenInvalid()
|
matcher := field.ErrorMatcher{}.ByType().ByField().ByOrigin().RequireOriginWhenInvalid()
|
||||||
matcher.Test(t, tc.wantFieldErrors, errs)
|
matcher.Test(t, tc.wantFieldErrors, errs)
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
@ -35,7 +35,6 @@ import (
|
|||||||
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
|
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
|
||||||
"k8s.io/apimachinery/pkg/util/sets"
|
"k8s.io/apimachinery/pkg/util/sets"
|
||||||
"k8s.io/apimachinery/pkg/util/validation/field"
|
"k8s.io/apimachinery/pkg/util/validation/field"
|
||||||
fieldtesting "k8s.io/apimachinery/pkg/util/validation/field/testing"
|
|
||||||
)
|
)
|
||||||
|
|
||||||
type testConversions struct {
|
type testConversions struct {
|
||||||
@ -1089,7 +1088,7 @@ func TestRegisterValidate(t *testing.T) {
|
|||||||
} else {
|
} else {
|
||||||
results = s.ValidateUpdate(ctx, tc.options, tc.object, tc.oldObject, tc.subresource...)
|
results = s.ValidateUpdate(ctx, tc.options, tc.object, tc.oldObject, tc.subresource...)
|
||||||
}
|
}
|
||||||
matcher := fieldtesting.ErrorMatcher{}.ByType().ByField().ByOrigin()
|
matcher := field.ErrorMatcher{}.ByType().ByField().ByOrigin()
|
||||||
matcher.Test(t, tc.expected, results)
|
matcher.Test(t, tc.expected, results)
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
@ -14,19 +14,16 @@ See the License for the specific language governing permissions and
|
|||||||
limitations under the License.
|
limitations under the License.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
package testing
|
package field
|
||||||
|
|
||||||
import (
|
import (
|
||||||
"fmt"
|
"fmt"
|
||||||
"reflect"
|
"reflect"
|
||||||
"regexp"
|
"regexp"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
|
||||||
|
|
||||||
field "k8s.io/apimachinery/pkg/util/validation/field"
|
|
||||||
)
|
)
|
||||||
|
|
||||||
// ErrorMatcher is a helper for comparing field.Error objects.
|
// ErrorMatcher is a helper for comparing Error objects.
|
||||||
type ErrorMatcher struct {
|
type ErrorMatcher struct {
|
||||||
// TODO(thockin): consider whether type is ever NOT required, maybe just
|
// TODO(thockin): consider whether type is ever NOT required, maybe just
|
||||||
// assume it.
|
// assume it.
|
||||||
@ -42,9 +39,9 @@ type ErrorMatcher struct {
|
|||||||
requireOriginWhenInvalid bool
|
requireOriginWhenInvalid bool
|
||||||
}
|
}
|
||||||
|
|
||||||
// Matches returns true if the two field.Error objects match according to the
|
// Matches returns true if the two Error objects match according to the
|
||||||
// configured criteria.
|
// configured criteria.
|
||||||
func (m ErrorMatcher) Matches(want, got *field.Error) bool {
|
func (m ErrorMatcher) Matches(want, got *Error) bool {
|
||||||
if m.matchType && want.Type != got.Type {
|
if m.matchType && want.Type != got.Type {
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
@ -58,7 +55,7 @@ func (m ErrorMatcher) Matches(want, got *field.Error) bool {
|
|||||||
if want.Origin != got.Origin {
|
if want.Origin != got.Origin {
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
if m.requireOriginWhenInvalid && want.Type == field.ErrorTypeInvalid {
|
if m.requireOriginWhenInvalid && want.Type == ErrorTypeInvalid {
|
||||||
if want.Origin == "" || got.Origin == "" {
|
if want.Origin == "" || got.Origin == "" {
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
@ -72,7 +69,7 @@ func (m ErrorMatcher) Matches(want, got *field.Error) bool {
|
|||||||
|
|
||||||
// Render returns a string representation of the specified Error object,
|
// Render returns a string representation of the specified Error object,
|
||||||
// according to the criteria configured in the ErrorMatcher.
|
// according to the criteria configured in the ErrorMatcher.
|
||||||
func (m ErrorMatcher) Render(e *field.Error) string {
|
func (m ErrorMatcher) Render(e *Error) string {
|
||||||
buf := strings.Builder{}
|
buf := strings.Builder{}
|
||||||
|
|
||||||
comma := func() {
|
comma := func() {
|
||||||
@ -93,7 +90,7 @@ func (m ErrorMatcher) Render(e *field.Error) string {
|
|||||||
comma()
|
comma()
|
||||||
buf.WriteString(fmt.Sprintf("Value=%v", e.BadValue))
|
buf.WriteString(fmt.Sprintf("Value=%v", e.BadValue))
|
||||||
}
|
}
|
||||||
if m.matchOrigin || m.requireOriginWhenInvalid && e.Type == field.ErrorTypeInvalid {
|
if m.matchOrigin || m.requireOriginWhenInvalid && e.Type == ErrorTypeInvalid {
|
||||||
comma()
|
comma()
|
||||||
buf.WriteString(fmt.Sprintf("Origin=%q", e.Origin))
|
buf.WriteString(fmt.Sprintf("Origin=%q", e.Origin))
|
||||||
}
|
}
|
||||||
@ -170,17 +167,25 @@ func (m ErrorMatcher) ByDetailRegexp() ErrorMatcher {
|
|||||||
return m
|
return m
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestIntf lets users pass a testing.T while not coupling this package to Go's
|
||||||
|
// testing package.
|
||||||
|
type TestIntf interface {
|
||||||
|
Helper()
|
||||||
|
Errorf(format string, args ...any)
|
||||||
|
Logf(format string, args ...any)
|
||||||
|
}
|
||||||
|
|
||||||
// Test compares two ErrorLists by the criteria configured in this matcher, and
|
// Test compares two ErrorLists by the criteria configured in this matcher, and
|
||||||
// fails the test if they don't match. If a given "want" error matches multiple
|
// fails the test if they don't match. If a given "want" error matches multiple
|
||||||
// "got" errors, they will all be consumed. This might be OK (e.g. if there are
|
// "got" errors, they will all be consumed. This might be OK (e.g. if there are
|
||||||
// multiple errors on the same field from the same origin) or it might be an
|
// multiple errors on the same field from the same origin) or it might be an
|
||||||
// insufficiently specific matcher, so these will be logged.
|
// insufficiently specific matcher, so these will be logged.
|
||||||
func (m ErrorMatcher) Test(tb testing.TB, want, got field.ErrorList) {
|
func (m ErrorMatcher) Test(tb TestIntf, want, got ErrorList) {
|
||||||
tb.Helper()
|
tb.Helper()
|
||||||
|
|
||||||
remaining := got
|
remaining := got
|
||||||
for _, w := range want {
|
for _, w := range want {
|
||||||
tmp := make(field.ErrorList, 0, len(remaining))
|
tmp := make(ErrorList, 0, len(remaining))
|
||||||
n := 0
|
n := 0
|
||||||
for _, g := range remaining {
|
for _, g := range remaining {
|
||||||
if m.Matches(w, g) {
|
if m.Matches(w, g) {
|
@ -26,6 +26,8 @@ import (
|
|||||||
"k8s.io/apimachinery/pkg/util/sets"
|
"k8s.io/apimachinery/pkg/util/sets"
|
||||||
"k8s.io/apimachinery/pkg/util/validation/field"
|
"k8s.io/apimachinery/pkg/util/validation/field"
|
||||||
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
|
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
|
||||||
|
validationmetrics "k8s.io/apiserver/pkg/validation"
|
||||||
|
"k8s.io/klog/v2"
|
||||||
)
|
)
|
||||||
|
|
||||||
// ValidateDeclaratively validates obj against declarative validation tags
|
// ValidateDeclaratively validates obj against declarative validation tags
|
||||||
@ -106,3 +108,212 @@ func parseSubresourcePath(subresourcePath string) ([]string, error) {
|
|||||||
parts := strings.Split(subresourcePath[1:], "/")
|
parts := strings.Split(subresourcePath[1:], "/")
|
||||||
return parts, nil
|
return parts, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// 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) {
|
||||||
|
logger := klog.FromContext(ctx)
|
||||||
|
mismatchDetails := gatherDeclarativeValidationMismatches(imperativeErrs, declarativeErrs, takeover)
|
||||||
|
for _, detail := range mismatchDetails {
|
||||||
|
// Log information about the mismatch using contextual logger
|
||||||
|
logger.Info(detail)
|
||||||
|
|
||||||
|
// Increment the metric for the mismatch
|
||||||
|
validationmetrics.Metrics.IncDeclarativeValidationMismatchMetric()
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// gatherDeclarativeValidationMismatches compares imperative and declarative validation errors
|
||||||
|
// and returns detailed information about any mismatches found. Errors are compared via type, field, and origin
|
||||||
|
func gatherDeclarativeValidationMismatches(imperativeErrs, declarativeErrs field.ErrorList, takeover bool) []string {
|
||||||
|
var mismatchDetails []string
|
||||||
|
// short circuit here to minimize allocs for usual case of 0 validation errors
|
||||||
|
if len(imperativeErrs) == 0 && len(declarativeErrs) == 0 {
|
||||||
|
return mismatchDetails
|
||||||
|
}
|
||||||
|
// recommendation based on takeover status
|
||||||
|
recommendation := "This difference should not affect system operation since hand written validation is authoritative."
|
||||||
|
if takeover {
|
||||||
|
recommendation = "Consider disabling the DeclarativeValidationTakeover feature gate to keep data persisted in etcd consistent with prior versions of Kubernetes."
|
||||||
|
}
|
||||||
|
fuzzyMatcher := field.ErrorMatcher{}.ByType().ByField().ByOrigin().RequireOriginWhenInvalid()
|
||||||
|
exactMatcher := field.ErrorMatcher{}.Exactly()
|
||||||
|
|
||||||
|
// Dedupe imperative errors of exact error matches as they are
|
||||||
|
// not intended and come from (buggy) duplicate validation calls
|
||||||
|
// This is necessary as without deduping we could get unmatched
|
||||||
|
// imperative errors for cases that are correct (matching)
|
||||||
|
dedupedImperativeErrs := field.ErrorList{}
|
||||||
|
for _, err := range imperativeErrs {
|
||||||
|
found := false
|
||||||
|
for _, existingErr := range dedupedImperativeErrs {
|
||||||
|
if exactMatcher.Matches(existingErr, err) {
|
||||||
|
found = true
|
||||||
|
break
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if !found {
|
||||||
|
dedupedImperativeErrs = append(dedupedImperativeErrs, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
imperativeErrs = dedupedImperativeErrs
|
||||||
|
|
||||||
|
// Create a copy of declarative errors to track remaining ones
|
||||||
|
remaining := make(field.ErrorList, len(declarativeErrs))
|
||||||
|
copy(remaining, declarativeErrs)
|
||||||
|
|
||||||
|
// Match each "covered" imperative error to declarative errors.
|
||||||
|
// We use a fuzzy matching approach to find corresponding declarative errors
|
||||||
|
// for each imperative error marked as CoveredByDeclarative.
|
||||||
|
// As matches are found, they're removed from the 'remaining' list.
|
||||||
|
// They are removed from `remaining` with a "1:many" mapping: for a given
|
||||||
|
// imperative error we mark as matched all matching declarative errors
|
||||||
|
// This allows us to:
|
||||||
|
// 1. Detect imperative errors that should have matching declarative errors but don't
|
||||||
|
// 2. Identify extra declarative errors with no imperative counterpart
|
||||||
|
// Both cases indicate issues with the declarative validation implementation.
|
||||||
|
for _, iErr := range imperativeErrs {
|
||||||
|
if !iErr.CoveredByDeclarative {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
|
||||||
|
tmp := make(field.ErrorList, 0, len(remaining))
|
||||||
|
matchCount := 0
|
||||||
|
|
||||||
|
for _, dErr := range remaining {
|
||||||
|
if fuzzyMatcher.Matches(iErr, dErr) {
|
||||||
|
matchCount++
|
||||||
|
} else {
|
||||||
|
tmp = append(tmp, dErr)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if matchCount == 0 {
|
||||||
|
mismatchDetails = append(mismatchDetails,
|
||||||
|
fmt.Sprintf(
|
||||||
|
"Unexpected difference between hand written validation and declarative validation error results, unmatched error(s) found %s. "+
|
||||||
|
"This indicates an issue with declarative validation. %s",
|
||||||
|
fuzzyMatcher.Render(iErr),
|
||||||
|
recommendation,
|
||||||
|
),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
remaining = tmp
|
||||||
|
}
|
||||||
|
|
||||||
|
// Any remaining unmatched declarative errors are considered "extra"
|
||||||
|
for _, dErr := range remaining {
|
||||||
|
mismatchDetails = append(mismatchDetails,
|
||||||
|
fmt.Sprintf(
|
||||||
|
"Unexpected difference between hand written validation and declarative validation error results, extra error(s) found %s. "+
|
||||||
|
"This indicates an issue with declarative validation. %s",
|
||||||
|
fuzzyMatcher.Render(dErr),
|
||||||
|
recommendation,
|
||||||
|
),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
return mismatchDetails
|
||||||
|
}
|
||||||
|
|
||||||
|
// createDeclarativeValidationPanicHandler returns a function with panic recovery logic
|
||||||
|
// that will increment the panic metric and either log or append errors based on the takeover parameter.
|
||||||
|
func createDeclarativeValidationPanicHandler(ctx context.Context, errs *field.ErrorList, takeover bool) func() {
|
||||||
|
logger := klog.FromContext(ctx)
|
||||||
|
return func() {
|
||||||
|
if r := recover(); r != nil {
|
||||||
|
// Increment the panic metric counter
|
||||||
|
validationmetrics.Metrics.IncDeclarativeValidationPanicMetric()
|
||||||
|
|
||||||
|
const errorFmt = "panic during declarative validation: %v"
|
||||||
|
if takeover {
|
||||||
|
// 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))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// withRecover 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(
|
||||||
|
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 {
|
||||||
|
return func(ctx context.Context, options sets.Set[string], scheme *runtime.Scheme, obj runtime.Object) (errs field.ErrorList) {
|
||||||
|
defer createDeclarativeValidationPanicHandler(ctx, &errs, takeover)()
|
||||||
|
|
||||||
|
return validateFunc(ctx, options, scheme, obj)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// withRecoverUpdate 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(
|
||||||
|
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 {
|
||||||
|
return func(ctx context.Context, options sets.Set[string], scheme *runtime.Scheme, obj, oldObj runtime.Object) (errs field.ErrorList) {
|
||||||
|
defer createDeclarativeValidationPanicHandler(ctx, &errs, takeover)()
|
||||||
|
|
||||||
|
return validateUpdateFunc(ctx, options, scheme, obj, oldObj)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// ValidateDeclarativelyWithRecovery validates obj against declarative validation tags
|
||||||
|
// with panic recovery logic. 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.
|
||||||
|
//
|
||||||
|
// option should contain any validation options that the declarative validation
|
||||||
|
// tags expect.
|
||||||
|
//
|
||||||
|
// takeover determines if panic recovery should return validation errors (true) or
|
||||||
|
// just log warnings (false).
|
||||||
|
//
|
||||||
|
// Returns a field.ErrorList containing any validation errors. An internal error
|
||||||
|
// is included if requestInfo is missing from the context, if version
|
||||||
|
// 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)
|
||||||
|
}
|
||||||
|
|
||||||
|
// ValidateUpdateDeclarativelyWithRecovery validates obj and oldObj against declarative
|
||||||
|
// validation tags with panic recovery logic. 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.
|
||||||
|
//
|
||||||
|
// option should contain any validation options that the declarative validation
|
||||||
|
// tags expect.
|
||||||
|
//
|
||||||
|
// takeover determines if panic recovery should return validation errors (true) or
|
||||||
|
// just log warnings (false).
|
||||||
|
//
|
||||||
|
// Returns a field.ErrorList containing any validation errors. An internal error
|
||||||
|
// is included if requestInfo is missing from the context, if version
|
||||||
|
// 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)
|
||||||
|
}
|
||||||
|
@ -17,8 +17,12 @@ limitations under the License.
|
|||||||
package rest
|
package rest
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"bytes"
|
||||||
"context"
|
"context"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"reflect"
|
||||||
|
"regexp"
|
||||||
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
v1 "k8s.io/api/core/v1"
|
v1 "k8s.io/api/core/v1"
|
||||||
@ -29,8 +33,8 @@ import (
|
|||||||
"k8s.io/apimachinery/pkg/runtime/schema"
|
"k8s.io/apimachinery/pkg/runtime/schema"
|
||||||
"k8s.io/apimachinery/pkg/util/sets"
|
"k8s.io/apimachinery/pkg/util/sets"
|
||||||
"k8s.io/apimachinery/pkg/util/validation/field"
|
"k8s.io/apimachinery/pkg/util/validation/field"
|
||||||
fieldtesting "k8s.io/apimachinery/pkg/util/validation/field/testing"
|
|
||||||
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
|
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
|
||||||
|
"k8s.io/klog/v2"
|
||||||
)
|
)
|
||||||
|
|
||||||
func TestValidateDeclaratively(t *testing.T) {
|
func TestValidateDeclaratively(t *testing.T) {
|
||||||
@ -154,7 +158,7 @@ func TestValidateDeclaratively(t *testing.T) {
|
|||||||
} else {
|
} else {
|
||||||
results = ValidateUpdateDeclaratively(ctx, tc.options, scheme, tc.object, tc.oldObject)
|
results = ValidateUpdateDeclaratively(ctx, tc.options, scheme, tc.object, tc.oldObject)
|
||||||
}
|
}
|
||||||
matcher := fieldtesting.ErrorMatcher{}.ByType().ByField().ByOrigin()
|
matcher := field.ErrorMatcher{}.ByType().ByField().ByOrigin()
|
||||||
matcher.Test(t, tc.expected, results)
|
matcher.Test(t, tc.expected, results)
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
@ -182,3 +186,500 @@ func (p Pod) DeepCopyObject() runtime.Object {
|
|||||||
RestartPolicy: p.RestartPolicy,
|
RestartPolicy: p.RestartPolicy,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestGatherDeclarativeValidationMismatches tests all mismatch
|
||||||
|
// scenarios across imperative and declarative errors for
|
||||||
|
// the gatherDeclarativeValidationMismatches function
|
||||||
|
func TestGatherDeclarativeValidationMismatches(t *testing.T) {
|
||||||
|
replicasPath := field.NewPath("spec").Child("replicas")
|
||||||
|
minReadySecondsPath := field.NewPath("spec").Child("minReadySeconds")
|
||||||
|
selectorPath := field.NewPath("spec").Child("selector")
|
||||||
|
|
||||||
|
errA := field.Invalid(replicasPath, nil, "regular error A")
|
||||||
|
errB := field.Invalid(minReadySecondsPath, -1, "covered error B").WithOrigin("minimum")
|
||||||
|
coveredErrB := field.Invalid(minReadySecondsPath, -1, "covered error B").WithOrigin("minimum")
|
||||||
|
errBWithDiffDetail := field.Invalid(minReadySecondsPath, -1, "covered error B - different detail").WithOrigin("minimum")
|
||||||
|
coveredErrB.CoveredByDeclarative = true
|
||||||
|
errC := field.Invalid(replicasPath, nil, "covered error C").WithOrigin("minimum")
|
||||||
|
coveredErrC := field.Invalid(replicasPath, nil, "covered error C").WithOrigin("minimum")
|
||||||
|
coveredErrC.CoveredByDeclarative = true
|
||||||
|
errCWithDiffOrigin := field.Invalid(replicasPath, nil, "covered error C").WithOrigin("maximum")
|
||||||
|
errD := field.Invalid(selectorPath, nil, "regular error D")
|
||||||
|
|
||||||
|
testCases := []struct {
|
||||||
|
name string
|
||||||
|
imperativeErrors field.ErrorList
|
||||||
|
declarativeErrors field.ErrorList
|
||||||
|
takeover bool
|
||||||
|
expectMismatches bool
|
||||||
|
expectDetailsContaining []string
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "Declarative and imperative return 0 errors - no mismatch",
|
||||||
|
imperativeErrors: field.ErrorList{},
|
||||||
|
declarativeErrors: field.ErrorList{},
|
||||||
|
takeover: false,
|
||||||
|
expectMismatches: false,
|
||||||
|
expectDetailsContaining: []string{},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "Declarative returns multiple errors with different origins, errors match - no mismatch",
|
||||||
|
imperativeErrors: field.ErrorList{
|
||||||
|
errA,
|
||||||
|
coveredErrB,
|
||||||
|
coveredErrC,
|
||||||
|
errD,
|
||||||
|
},
|
||||||
|
declarativeErrors: field.ErrorList{
|
||||||
|
errB,
|
||||||
|
errC,
|
||||||
|
},
|
||||||
|
takeover: false,
|
||||||
|
expectMismatches: false,
|
||||||
|
expectDetailsContaining: []string{},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "Declarative returns multiple errors with different origins, errors don't match - mismatch case",
|
||||||
|
imperativeErrors: field.ErrorList{
|
||||||
|
errA,
|
||||||
|
coveredErrB,
|
||||||
|
coveredErrC,
|
||||||
|
},
|
||||||
|
declarativeErrors: field.ErrorList{
|
||||||
|
errB,
|
||||||
|
errCWithDiffOrigin,
|
||||||
|
},
|
||||||
|
takeover: true,
|
||||||
|
expectMismatches: true,
|
||||||
|
expectDetailsContaining: []string{
|
||||||
|
"Unexpected difference between hand written validation and declarative validation error results",
|
||||||
|
"unmatched error(s) found",
|
||||||
|
"extra error(s) found",
|
||||||
|
"replicas",
|
||||||
|
"Consider disabling the DeclarativeValidationTakeover feature gate to keep data persisted in etcd consistent with prior versions of Kubernetes",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "Declarative and imperative return exactly 1 error, errors match - no mismatch",
|
||||||
|
imperativeErrors: field.ErrorList{
|
||||||
|
coveredErrB,
|
||||||
|
},
|
||||||
|
declarativeErrors: field.ErrorList{
|
||||||
|
errB,
|
||||||
|
},
|
||||||
|
takeover: false,
|
||||||
|
expectMismatches: false,
|
||||||
|
expectDetailsContaining: []string{},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "Declarative and imperative exactly 1 error, errors don't match - mismatch",
|
||||||
|
imperativeErrors: field.ErrorList{
|
||||||
|
coveredErrB,
|
||||||
|
},
|
||||||
|
declarativeErrors: field.ErrorList{
|
||||||
|
errC,
|
||||||
|
},
|
||||||
|
takeover: false,
|
||||||
|
expectMismatches: true,
|
||||||
|
expectDetailsContaining: []string{
|
||||||
|
"Unexpected difference between hand written validation and declarative validation error results",
|
||||||
|
"unmatched error(s) found",
|
||||||
|
"minReadySeconds",
|
||||||
|
"extra error(s) found",
|
||||||
|
"replicas",
|
||||||
|
"This difference should not affect system operation since hand written validation is authoritative",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "Declarative returns 0 errors, imperative returns 1 covered error - mismatch",
|
||||||
|
imperativeErrors: field.ErrorList{
|
||||||
|
coveredErrB,
|
||||||
|
},
|
||||||
|
declarativeErrors: field.ErrorList{},
|
||||||
|
takeover: true,
|
||||||
|
expectMismatches: true,
|
||||||
|
expectDetailsContaining: []string{
|
||||||
|
"Unexpected difference between hand written validation and declarative validation error results",
|
||||||
|
"unmatched error(s) found",
|
||||||
|
"minReadySeconds",
|
||||||
|
"Consider disabling the DeclarativeValidationTakeover feature gate to keep data persisted in etcd consistent with prior versions of Kubernetes",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "Declarative returns 0 errors, imperative returns 1 uncovered error - no mismatch",
|
||||||
|
imperativeErrors: field.ErrorList{
|
||||||
|
errB,
|
||||||
|
},
|
||||||
|
declarativeErrors: field.ErrorList{},
|
||||||
|
takeover: false,
|
||||||
|
expectMismatches: false,
|
||||||
|
expectDetailsContaining: []string{},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "Declarative returns 1 error, imperative returns 0 error - mismatch",
|
||||||
|
imperativeErrors: field.ErrorList{},
|
||||||
|
declarativeErrors: field.ErrorList{
|
||||||
|
errB,
|
||||||
|
},
|
||||||
|
takeover: false,
|
||||||
|
expectMismatches: true,
|
||||||
|
expectDetailsContaining: []string{
|
||||||
|
"Unexpected difference between hand written validation and declarative validation error results",
|
||||||
|
"extra error(s) found",
|
||||||
|
"minReadySeconds",
|
||||||
|
"This difference should not affect system operation since hand written validation is authoritative",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "Declarative returns 1 error, imperative returns 3 matching errors - no mismatch",
|
||||||
|
imperativeErrors: field.ErrorList{
|
||||||
|
coveredErrB,
|
||||||
|
},
|
||||||
|
declarativeErrors: field.ErrorList{
|
||||||
|
errB,
|
||||||
|
errB,
|
||||||
|
errBWithDiffDetail,
|
||||||
|
},
|
||||||
|
takeover: false,
|
||||||
|
expectMismatches: false,
|
||||||
|
expectDetailsContaining: []string{},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tc := range testCases {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
details := gatherDeclarativeValidationMismatches(tc.imperativeErrors, tc.declarativeErrors, tc.takeover)
|
||||||
|
// Check if mismatches were found if expected
|
||||||
|
if tc.expectMismatches && len(details) == 0 {
|
||||||
|
t.Errorf("Expected mismatches but got none")
|
||||||
|
}
|
||||||
|
// Check if details contain expected text
|
||||||
|
detailsStr := strings.Join(details, " ")
|
||||||
|
for _, expectedContent := range tc.expectDetailsContaining {
|
||||||
|
if !strings.Contains(detailsStr, expectedContent) {
|
||||||
|
t.Errorf("Expected details to contain: %q, but they didn't.\nDetails were:\n%s",
|
||||||
|
expectedContent, strings.Join(details, "\n"))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// If we don't expect any details, make sure none provided
|
||||||
|
if len(tc.expectDetailsContaining) == 0 && len(details) > 0 {
|
||||||
|
t.Errorf("Expected no details, but got %d details: %v", len(details), details)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestCompareDeclarativeErrorsAndEmitMismatches tests expected
|
||||||
|
// logging of mismatch information given match & mismatch error conditions.
|
||||||
|
func TestCompareDeclarativeErrorsAndEmitMismatches(t *testing.T) {
|
||||||
|
replicasPath := field.NewPath("spec").Child("replicas")
|
||||||
|
minReadySecondsPath := field.NewPath("spec").Child("minReadySeconds")
|
||||||
|
|
||||||
|
errA := field.Invalid(replicasPath, nil, "regular error A")
|
||||||
|
errB := field.Invalid(minReadySecondsPath, -1, "covered error B").WithOrigin("minimum")
|
||||||
|
coveredErrB := field.Invalid(minReadySecondsPath, -1, "covered error B").WithOrigin("minimum")
|
||||||
|
coveredErrB.CoveredByDeclarative = true
|
||||||
|
|
||||||
|
testCases := []struct {
|
||||||
|
name string
|
||||||
|
imperativeErrs field.ErrorList
|
||||||
|
declarativeErrs field.ErrorList
|
||||||
|
takeover bool
|
||||||
|
expectLogs bool
|
||||||
|
expectedRegex string
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "mismatched errors, log info",
|
||||||
|
imperativeErrs: field.ErrorList{coveredErrB},
|
||||||
|
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",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "matching errors, don't log info",
|
||||||
|
imperativeErrs: field.ErrorList{coveredErrB},
|
||||||
|
declarativeErrs: field.ErrorList{errB},
|
||||||
|
takeover: true,
|
||||||
|
expectLogs: false,
|
||||||
|
expectedRegex: "",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tc := range testCases {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
var buf bytes.Buffer
|
||||||
|
klog.SetOutput(&buf)
|
||||||
|
klog.LogToStderr(false)
|
||||||
|
defer klog.LogToStderr(true)
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
CompareDeclarativeErrorsAndEmitMismatches(ctx, tc.imperativeErrs, tc.declarativeErrs, tc.takeover)
|
||||||
|
|
||||||
|
klog.Flush()
|
||||||
|
logOutput := buf.String()
|
||||||
|
|
||||||
|
if tc.expectLogs {
|
||||||
|
matched, err := regexp.MatchString(tc.expectedRegex, logOutput)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Bad regex: %v", err)
|
||||||
|
}
|
||||||
|
if !matched {
|
||||||
|
t.Errorf("Expected log output to match %q, but got:\n%s", tc.expectedRegex, logOutput)
|
||||||
|
}
|
||||||
|
} else if len(logOutput) > 0 {
|
||||||
|
t.Errorf("Expected no mismatch logs, but found: %s", logOutput)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestWithRecover(t *testing.T) {
|
||||||
|
ctx := context.Background()
|
||||||
|
scheme := runtime.NewScheme()
|
||||||
|
options := sets.New[string]()
|
||||||
|
obj := &runtime.Unknown{}
|
||||||
|
|
||||||
|
testCases := []struct {
|
||||||
|
name string
|
||||||
|
validateFn func(context.Context, sets.Set[string], *runtime.Scheme, runtime.Object) field.ErrorList
|
||||||
|
takeoverEnabled bool
|
||||||
|
wantErrs field.ErrorList
|
||||||
|
expectLogRegex string
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "no panic",
|
||||||
|
validateFn: func(context.Context, sets.Set[string], *runtime.Scheme, runtime.Object) field.ErrorList {
|
||||||
|
return field.ErrorList{
|
||||||
|
field.Invalid(field.NewPath("field"), "value", "reason"),
|
||||||
|
}
|
||||||
|
},
|
||||||
|
takeoverEnabled: false,
|
||||||
|
wantErrs: field.ErrorList{
|
||||||
|
field.Invalid(field.NewPath("field"), "value", "reason"),
|
||||||
|
},
|
||||||
|
expectLogRegex: "",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "panic with takeover disabled",
|
||||||
|
validateFn: func(context.Context, sets.Set[string], *runtime.Scheme, runtime.Object) field.ErrorList {
|
||||||
|
panic("test panic")
|
||||||
|
},
|
||||||
|
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",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "panic with takeover enabled",
|
||||||
|
validateFn: func(context.Context, sets.Set[string], *runtime.Scheme, runtime.Object) field.ErrorList {
|
||||||
|
panic("test panic")
|
||||||
|
},
|
||||||
|
takeoverEnabled: true,
|
||||||
|
wantErrs: field.ErrorList{
|
||||||
|
field.InternalError(nil, fmt.Errorf("panic during declarative validation: test panic")),
|
||||||
|
},
|
||||||
|
expectLogRegex: "",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "nil return, no panic",
|
||||||
|
validateFn: func(context.Context, sets.Set[string], *runtime.Scheme, runtime.Object) field.ErrorList {
|
||||||
|
return nil
|
||||||
|
},
|
||||||
|
takeoverEnabled: false,
|
||||||
|
wantErrs: nil,
|
||||||
|
expectLogRegex: "",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tc := range testCases {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
var buf bytes.Buffer
|
||||||
|
klog.SetOutput(&buf)
|
||||||
|
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)
|
||||||
|
gotErrs := wrapped(ctx, options, scheme, obj)
|
||||||
|
|
||||||
|
klog.Flush()
|
||||||
|
logOutput := buf.String()
|
||||||
|
|
||||||
|
// Compare gotErrs vs. tc.wantErrs
|
||||||
|
if !equalErrorLists(gotErrs, tc.wantErrs) {
|
||||||
|
t.Errorf("withRecover() gotErrs = %#v, want %#v", gotErrs, tc.wantErrs)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check logs if needed
|
||||||
|
if tc.expectLogRegex != "" {
|
||||||
|
matched, err := regexp.MatchString(tc.expectLogRegex, logOutput)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Bad regex: %v", err)
|
||||||
|
}
|
||||||
|
if !matched {
|
||||||
|
t.Errorf("Expected log output %q, but got:\n%s", tc.expectLogRegex, logOutput)
|
||||||
|
}
|
||||||
|
} else if strings.Contains(logOutput, "panic during declarative validation") {
|
||||||
|
t.Errorf("Unexpected panic log found: %s", logOutput)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestWithRecoverUpdate(t *testing.T) {
|
||||||
|
ctx := context.Background()
|
||||||
|
scheme := runtime.NewScheme()
|
||||||
|
options := sets.New[string]()
|
||||||
|
obj := &runtime.Unknown{}
|
||||||
|
oldObj := &runtime.Unknown{}
|
||||||
|
|
||||||
|
testCases := []struct {
|
||||||
|
name string
|
||||||
|
validateFn func(context.Context, sets.Set[string], *runtime.Scheme, runtime.Object, runtime.Object) field.ErrorList
|
||||||
|
takeoverEnabled bool
|
||||||
|
wantErrs field.ErrorList
|
||||||
|
expectLogRegex string
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "no panic",
|
||||||
|
validateFn: func(context.Context, sets.Set[string], *runtime.Scheme, runtime.Object, runtime.Object) field.ErrorList {
|
||||||
|
return field.ErrorList{
|
||||||
|
field.Invalid(field.NewPath("field"), "value", "reason"),
|
||||||
|
}
|
||||||
|
},
|
||||||
|
takeoverEnabled: false,
|
||||||
|
wantErrs: field.ErrorList{
|
||||||
|
field.Invalid(field.NewPath("field"), "value", "reason"),
|
||||||
|
},
|
||||||
|
expectLogRegex: "",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "panic with takeover disabled",
|
||||||
|
validateFn: func(context.Context, sets.Set[string], *runtime.Scheme, runtime.Object, runtime.Object) field.ErrorList {
|
||||||
|
panic("test update panic")
|
||||||
|
},
|
||||||
|
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",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "panic with takeover enabled",
|
||||||
|
validateFn: func(context.Context, sets.Set[string], *runtime.Scheme, runtime.Object, runtime.Object) field.ErrorList {
|
||||||
|
panic("test update panic")
|
||||||
|
},
|
||||||
|
takeoverEnabled: true,
|
||||||
|
wantErrs: field.ErrorList{
|
||||||
|
field.InternalError(nil, fmt.Errorf("panic during declarative validation: test update panic")),
|
||||||
|
},
|
||||||
|
expectLogRegex: "",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "nil return, no panic",
|
||||||
|
validateFn: func(context.Context, sets.Set[string], *runtime.Scheme, runtime.Object, runtime.Object) field.ErrorList {
|
||||||
|
return nil
|
||||||
|
},
|
||||||
|
takeoverEnabled: false,
|
||||||
|
wantErrs: nil,
|
||||||
|
expectLogRegex: "",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tc := range testCases {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
var buf bytes.Buffer
|
||||||
|
klog.SetOutput(&buf)
|
||||||
|
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)
|
||||||
|
gotErrs := wrapped(ctx, options, scheme, obj, oldObj)
|
||||||
|
|
||||||
|
klog.Flush()
|
||||||
|
logOutput := buf.String()
|
||||||
|
|
||||||
|
// Compare gotErrs with wantErrs
|
||||||
|
if !equalErrorLists(gotErrs, tc.wantErrs) {
|
||||||
|
t.Errorf("withRecoverUpdate() gotErrs = %#v, want %#v", gotErrs, tc.wantErrs)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Verify log output
|
||||||
|
if tc.expectLogRegex != "" {
|
||||||
|
matched, err := regexp.MatchString(tc.expectLogRegex, logOutput)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Bad regex: %v", err)
|
||||||
|
}
|
||||||
|
if !matched {
|
||||||
|
t.Errorf("Expected log pattern %q, but got:\n%s", tc.expectLogRegex, logOutput)
|
||||||
|
}
|
||||||
|
} else if strings.Contains(logOutput, "panic during declarative validation") {
|
||||||
|
t.Errorf("Unexpected panic log found: %s", logOutput)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestValidateDeclarativelyWithRecovery(t *testing.T) {
|
||||||
|
ctx := context.Background()
|
||||||
|
scheme := runtime.NewScheme()
|
||||||
|
options := sets.New[string]()
|
||||||
|
obj := &runtime.Unknown{}
|
||||||
|
|
||||||
|
// Simple test for the ValidateDeclarativelyWithRecovery function
|
||||||
|
t.Run("with takeover disabled", func(t *testing.T) {
|
||||||
|
errs := ValidateDeclarativelyWithRecovery(ctx, options, scheme, obj, 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 := ValidateDeclarativelyWithRecovery(ctx, options, scheme, obj, true)
|
||||||
|
if errs == nil {
|
||||||
|
// This is expected to error since the request info is missing
|
||||||
|
t.Errorf("Expected errors but got nil")
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestValidateUpdateDeclarativelyWithRecovery(t *testing.T) {
|
||||||
|
ctx := context.Background()
|
||||||
|
scheme := runtime.NewScheme()
|
||||||
|
options := sets.New[string]()
|
||||||
|
obj := &runtime.Unknown{}
|
||||||
|
oldObj := &runtime.Unknown{}
|
||||||
|
|
||||||
|
// Simple test for the ValidateUpdateDeclarativelyWithRecovery function
|
||||||
|
t.Run("with takeover disabled", func(t *testing.T) {
|
||||||
|
errs := ValidateUpdateDeclarativelyWithRecovery(ctx, options, scheme, obj, oldObj, 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 := ValidateUpdateDeclarativelyWithRecovery(ctx, options, scheme, obj, oldObj, true)
|
||||||
|
if errs == nil {
|
||||||
|
// This is expected to error since the request info is missing
|
||||||
|
t.Errorf("Expected errors but got nil")
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
func equalErrorLists(a, b field.ErrorList) bool {
|
||||||
|
// If both are nil, consider them equal
|
||||||
|
if a == nil && b == nil {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
// If one is nil and the other not, they're different
|
||||||
|
if (a == nil && b != nil) || (a != nil && b == nil) {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
// Both non-nil: do a normal DeepEqual
|
||||||
|
return reflect.DeepEqual(a, b)
|
||||||
|
}
|
||||||
|
88
staging/src/k8s.io/apiserver/pkg/validation/metrics.go
Normal file
88
staging/src/k8s.io/apiserver/pkg/validation/metrics.go
Normal file
@ -0,0 +1,88 @@
|
|||||||
|
/*
|
||||||
|
Copyright 2025 The Kubernetes Authors.
|
||||||
|
|
||||||
|
Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
|
you may not use this file except in compliance with the License.
|
||||||
|
You may obtain a copy of the License at
|
||||||
|
|
||||||
|
http://www.apache.org/licenses/LICENSE-2.0
|
||||||
|
|
||||||
|
Unless required by applicable law or agreed to in writing, software
|
||||||
|
distributed under the License is distributed on an "AS IS" BASIS,
|
||||||
|
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||||
|
See the License for the specific language governing permissions and
|
||||||
|
limitations under the License.
|
||||||
|
*/
|
||||||
|
|
||||||
|
package validation
|
||||||
|
|
||||||
|
import (
|
||||||
|
"k8s.io/component-base/metrics"
|
||||||
|
"k8s.io/component-base/metrics/legacyregistry"
|
||||||
|
)
|
||||||
|
|
||||||
|
const (
|
||||||
|
namespace = "apiserver" // Keep it consistent; apiserver is handling it
|
||||||
|
subsystem = "validation"
|
||||||
|
)
|
||||||
|
|
||||||
|
// ValidationMetrics is the interface for validation metrics.
|
||||||
|
type ValidationMetrics interface {
|
||||||
|
IncDeclarativeValidationMismatchMetric()
|
||||||
|
IncDeclarativeValidationPanicMetric()
|
||||||
|
Reset()
|
||||||
|
}
|
||||||
|
|
||||||
|
var validationMetricsInstance = &validationMetrics{
|
||||||
|
DeclarativeValidationMismatchCounter: metrics.NewCounter(
|
||||||
|
&metrics.CounterOpts{
|
||||||
|
Namespace: namespace,
|
||||||
|
Subsystem: subsystem,
|
||||||
|
Name: "declarative_validation_mismatch_total",
|
||||||
|
Help: "Number of times declarative validation results differed from handwritten validation results for core types.",
|
||||||
|
StabilityLevel: metrics.BETA,
|
||||||
|
},
|
||||||
|
),
|
||||||
|
DeclarativeValidationPanicCounter: metrics.NewCounter(
|
||||||
|
&metrics.CounterOpts{
|
||||||
|
Namespace: namespace,
|
||||||
|
Subsystem: subsystem,
|
||||||
|
Name: "declarative_validation_panic_total",
|
||||||
|
Help: "Number of times declarative validation has panicked during validation.",
|
||||||
|
StabilityLevel: metrics.BETA,
|
||||||
|
},
|
||||||
|
),
|
||||||
|
}
|
||||||
|
|
||||||
|
// Metrics provides access to validation metrics.
|
||||||
|
var Metrics ValidationMetrics = validationMetricsInstance
|
||||||
|
|
||||||
|
func init() {
|
||||||
|
legacyregistry.MustRegister(validationMetricsInstance.DeclarativeValidationMismatchCounter)
|
||||||
|
legacyregistry.MustRegister(validationMetricsInstance.DeclarativeValidationPanicCounter)
|
||||||
|
}
|
||||||
|
|
||||||
|
type validationMetrics struct {
|
||||||
|
DeclarativeValidationMismatchCounter *metrics.Counter
|
||||||
|
DeclarativeValidationPanicCounter *metrics.Counter
|
||||||
|
}
|
||||||
|
|
||||||
|
// Reset resets the validation metrics.
|
||||||
|
func (m *validationMetrics) Reset() {
|
||||||
|
m.DeclarativeValidationMismatchCounter.Reset()
|
||||||
|
m.DeclarativeValidationPanicCounter.Reset()
|
||||||
|
}
|
||||||
|
|
||||||
|
// IncDeclarativeValidationMismatchMetric increments the counter for the declarative_validation_mismatch_total metric.
|
||||||
|
func (m *validationMetrics) IncDeclarativeValidationMismatchMetric() {
|
||||||
|
m.DeclarativeValidationMismatchCounter.Inc()
|
||||||
|
}
|
||||||
|
|
||||||
|
// IncDeclarativeValidationPanicMetric increments the counter for the declarative_validation_panic_total metric.
|
||||||
|
func (m *validationMetrics) IncDeclarativeValidationPanicMetric() {
|
||||||
|
m.DeclarativeValidationPanicCounter.Inc()
|
||||||
|
}
|
||||||
|
|
||||||
|
func ResetValidationMetricsInstance() {
|
||||||
|
validationMetricsInstance.Reset()
|
||||||
|
}
|
150
staging/src/k8s.io/apiserver/pkg/validation/metrics_test.go
Normal file
150
staging/src/k8s.io/apiserver/pkg/validation/metrics_test.go
Normal file
@ -0,0 +1,150 @@
|
|||||||
|
/*
|
||||||
|
Copyright 2025 The Kubernetes Authors.
|
||||||
|
|
||||||
|
Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
|
you may not use this file except in compliance with the License.
|
||||||
|
You may obtain a copy of the License at
|
||||||
|
|
||||||
|
http://www.apache.org/licenses/LICENSE-2.0
|
||||||
|
|
||||||
|
Unless required by applicable law or agreed to in writing, software
|
||||||
|
distributed under the License is distributed on an "AS IS" BASIS,
|
||||||
|
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||||
|
See the License for the specific language governing permissions and
|
||||||
|
limitations under the License.
|
||||||
|
*/
|
||||||
|
|
||||||
|
package validation
|
||||||
|
|
||||||
|
import (
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"k8s.io/component-base/metrics/legacyregistry"
|
||||||
|
"k8s.io/component-base/metrics/testutil"
|
||||||
|
)
|
||||||
|
|
||||||
|
// TestDeclarativeValidationMismatchMetric tests that the mismatch metric correctly increments once
|
||||||
|
func TestDeclarativeValidationMismatchMetric(t *testing.T) {
|
||||||
|
defer legacyregistry.Reset()
|
||||||
|
defer ResetValidationMetricsInstance()
|
||||||
|
|
||||||
|
// Increment the metric once
|
||||||
|
Metrics.IncDeclarativeValidationMismatchMetric()
|
||||||
|
|
||||||
|
expected := `
|
||||||
|
# HELP apiserver_validation_declarative_validation_mismatch_total [BETA] Number of times declarative validation results differed from handwritten validation results for core types.
|
||||||
|
# TYPE apiserver_validation_declarative_validation_mismatch_total counter
|
||||||
|
apiserver_validation_declarative_validation_mismatch_total 1
|
||||||
|
`
|
||||||
|
|
||||||
|
if err := testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(expected), "declarative_validation_mismatch_total"); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestDeclarativeValidationPanicMetric tests that the panic metric correctly increments once
|
||||||
|
func TestDeclarativeValidationPanicMetric(t *testing.T) {
|
||||||
|
defer legacyregistry.Reset()
|
||||||
|
defer ResetValidationMetricsInstance()
|
||||||
|
|
||||||
|
// Increment the metric once
|
||||||
|
Metrics.IncDeclarativeValidationPanicMetric()
|
||||||
|
|
||||||
|
expected := `
|
||||||
|
# HELP apiserver_validation_declarative_validation_panic_total [BETA] Number of times declarative validation has panicked during validation.
|
||||||
|
# TYPE apiserver_validation_declarative_validation_panic_total counter
|
||||||
|
apiserver_validation_declarative_validation_panic_total 1
|
||||||
|
`
|
||||||
|
|
||||||
|
if err := testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(expected), "declarative_validation_panic_total"); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestDeclarativeValidationMismatchMetricMultiple tests that the mismatch metric correctly increments multiple times
|
||||||
|
func TestDeclarativeValidationMismatchMetricMultiple(t *testing.T) {
|
||||||
|
defer legacyregistry.Reset()
|
||||||
|
defer ResetValidationMetricsInstance()
|
||||||
|
|
||||||
|
// Increment the metric three times
|
||||||
|
Metrics.IncDeclarativeValidationMismatchMetric()
|
||||||
|
Metrics.IncDeclarativeValidationMismatchMetric()
|
||||||
|
Metrics.IncDeclarativeValidationMismatchMetric()
|
||||||
|
|
||||||
|
expected := `
|
||||||
|
# HELP apiserver_validation_declarative_validation_mismatch_total [BETA] Number of times declarative validation results differed from handwritten validation results for core types.
|
||||||
|
# TYPE apiserver_validation_declarative_validation_mismatch_total counter
|
||||||
|
apiserver_validation_declarative_validation_mismatch_total 3
|
||||||
|
`
|
||||||
|
|
||||||
|
if err := testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(expected), "declarative_validation_mismatch_total"); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestDeclarativeValidationPanicMetricMultiple tests that the panic metric correctly increments multiple times
|
||||||
|
func TestDeclarativeValidationPanicMetricMultiple(t *testing.T) {
|
||||||
|
defer legacyregistry.Reset()
|
||||||
|
defer ResetValidationMetricsInstance()
|
||||||
|
|
||||||
|
// Increment the metric three times
|
||||||
|
Metrics.IncDeclarativeValidationPanicMetric()
|
||||||
|
Metrics.IncDeclarativeValidationPanicMetric()
|
||||||
|
Metrics.IncDeclarativeValidationPanicMetric()
|
||||||
|
|
||||||
|
expected := `
|
||||||
|
# HELP apiserver_validation_declarative_validation_panic_total [BETA] Number of times declarative validation has panicked during validation.
|
||||||
|
# TYPE apiserver_validation_declarative_validation_panic_total counter
|
||||||
|
apiserver_validation_declarative_validation_panic_total 3
|
||||||
|
`
|
||||||
|
|
||||||
|
if err := testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(expected), "declarative_validation_panic_total"); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestDeclarativeValidationMetricsReset tests that the Reset function correctly resets the metrics to zero
|
||||||
|
func TestDeclarativeValidationMetricsReset(t *testing.T) {
|
||||||
|
defer legacyregistry.Reset()
|
||||||
|
defer ResetValidationMetricsInstance()
|
||||||
|
|
||||||
|
// Increment both metrics
|
||||||
|
Metrics.IncDeclarativeValidationMismatchMetric()
|
||||||
|
Metrics.IncDeclarativeValidationPanicMetric()
|
||||||
|
|
||||||
|
// Reset the metrics
|
||||||
|
Metrics.Reset()
|
||||||
|
|
||||||
|
// Verify they've been reset to zero
|
||||||
|
expected := `
|
||||||
|
# HELP apiserver_validation_declarative_validation_mismatch_total [BETA] Number of times declarative validation results differed from handwritten validation results for core types.
|
||||||
|
# TYPE apiserver_validation_declarative_validation_mismatch_total counter
|
||||||
|
apiserver_validation_declarative_validation_mismatch_total 0
|
||||||
|
# HELP apiserver_validation_declarative_validation_panic_total [BETA] Number of times declarative validation has panicked during validation.
|
||||||
|
# TYPE apiserver_validation_declarative_validation_panic_total counter
|
||||||
|
apiserver_validation_declarative_validation_panic_total 0
|
||||||
|
`
|
||||||
|
|
||||||
|
if err := testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(expected), "declarative_validation_mismatch_total", "declarative_validation_panic_total"); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Increment the metrics again to ensure they're still functional
|
||||||
|
Metrics.IncDeclarativeValidationMismatchMetric()
|
||||||
|
Metrics.IncDeclarativeValidationPanicMetric()
|
||||||
|
|
||||||
|
// Verify they've been incremented correctly
|
||||||
|
expected = `
|
||||||
|
# HELP apiserver_validation_declarative_validation_mismatch_total [BETA] Number of times declarative validation results differed from handwritten validation results for core types.
|
||||||
|
# TYPE apiserver_validation_declarative_validation_mismatch_total counter
|
||||||
|
apiserver_validation_declarative_validation_mismatch_total 1
|
||||||
|
# HELP apiserver_validation_declarative_validation_panic_total [BETA] Number of times declarative validation has panicked during validation.
|
||||||
|
# TYPE apiserver_validation_declarative_validation_panic_total counter
|
||||||
|
apiserver_validation_declarative_validation_panic_total 1
|
||||||
|
`
|
||||||
|
|
||||||
|
if err := testutil.GatherAndCompare(legacyregistry.DefaultGatherer, strings.NewReader(expected), "declarative_validation_mismatch_total", "declarative_validation_panic_total"); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
}
|
@ -122,6 +122,19 @@
|
|||||||
- error_type
|
- error_type
|
||||||
- policy
|
- policy
|
||||||
- policy_binding
|
- policy_binding
|
||||||
|
- name: declarative_validation_mismatch_total
|
||||||
|
subsystem: validation
|
||||||
|
namespace: apiserver
|
||||||
|
help: Number of times declarative validation results differed from handwritten validation
|
||||||
|
results for core types.
|
||||||
|
type: Counter
|
||||||
|
stabilityLevel: BETA
|
||||||
|
- name: declarative_validation_panic_total
|
||||||
|
subsystem: validation
|
||||||
|
namespace: apiserver
|
||||||
|
help: Number of times declarative validation has panicked during validation.
|
||||||
|
type: Counter
|
||||||
|
stabilityLevel: BETA
|
||||||
- name: disabled_metrics_total
|
- name: disabled_metrics_total
|
||||||
help: The count of disabled metrics.
|
help: The count of disabled metrics.
|
||||||
type: Counter
|
type: Counter
|
||||||
|
Loading…
Reference in New Issue
Block a user