Input validation / Configuration validation

Proof-of-concept outline (illustrative, not executed): Goal: Demonstrate how the patch changes validation semantics by merging declarative validation with handwritten validation during resource creation. 1) Define a minimal strategy that implements both handwritten and declarative validation: - Validate(ctx, obj) returns no errors (handwritten validation passes). - ValidateDeclaratively(ctx, obj, old, errs, opType, config) checks a declarative rule (for example, requiring a string field to be all lowercase) and appends an error if violated. 2) Create a sample object that would violate the declarative rule but pass handwritten validation: - obj := Foo{ Spec: FooSpec{ Name: "Bar" } } - handwrittenErrs := strategy.Validate(ctx, obj) // returns [] (no errors) - mergedErrs := strategy.ValidateDeclaratively(ctx, obj, nil, handwrittenErrs, operation.Create, config) // returns an error: spec.name must be lowercase 3) Before this patch, the API path would only consider handwritten validation when producing the Admission/Creation error (or would not merge declarative errors in some pathways depending on implementation). - In such a scenario, creating obj with Name "Bar" would be accepted if handwritten validation passed. 4) After this patch, the API path explicitly merges declarative validation results with handwritten results, causing the operation to be rejected if the declarative rule is violated. Minimal illustrative Go-like snippet (conceptual, not production-ready): // Pseudo-types to illustrate the concept type FooStrategy struct{ } func (s *FooStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList { return field.ErrorList{} } func (s *FooStrategy) ValidateDeclaratively(ctx context.Context, obj runtime.Object, old runtime.Object, errs field.ErrorList, opType operation.Type, config DeclarativeValidationConfig) field.ErrorList { fo := obj.(*Foo) if fo.Spec.Name != strings.ToLower(fo.Spec.Name) { return append(errs, field.Invalid(field.NewPath("spec", "name"), fo.Spec.Name, "name must be lowercase")) } return errs } ctx := context.Background() obj := &Foo{Spec: FooSpec{Name: "Bar"}} strategy := &FooStrategy{} handwritten := strategy.Validate(ctx, obj) // empty merged := strategy.ValidateDeclaratively(ctx, obj, nil, handwritten, operation.Create, DeclarativeValidationConfig{}) // contains error for uppercase name if len(merged) > 0 { // creation would be rejected } Note: The PoC is illustrative and focuses on the interaction between handwritten and declarative validations as affected by this patch. In real Kubernetes code, the declarative validation path is wired via DeclarativeValidationStrategy and DeclarativeValidationConfigurer interfaces, and the opType parameter would be operation.Create. The key exploit path being demonstrated is: without this patch, a resource with a violation detectable by declarative validation could slip through due to not merging declarative errors with handwritten errors; with the patch, such errors are merged and cause rejection.

diff --git a/pkg/api/testing/validation.go b/pkg/api/testing/validation.go index 69ad77caec4ee..d6ce1051065a4 100644 --- a/pkg/api/testing/validation.go +++ b/pkg/api/testing/validation.go @@ -23,6 +23,7 @@ import ( "strconv" "testing" + "k8s.io/apimachinery/pkg/api/operation" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" runtimetest "k8s.io/apimachinery/pkg/runtime/testing" @@ -39,12 +40,6 @@ import ( "sigs.k8s.io/randfill" ) -// ValidateFunc is a function that runs validation. -type ValidateFunc func(ctx context.Context, obj runtime.Object) field.ErrorList - -// ValidateUpdateFunc is a function that runs update validation. -type ValidateUpdateFunc func(ctx context.Context, obj, old runtime.Object) field.ErrorList - // VerifyVersionedValidationEquivalence tests that all versions of an API return equivalent validation errors. // It accepts optional configuration to handle path normalization across API versions where structures differ. func VerifyVersionedValidationEquivalence(t *testing.T, obj, old runtime.Object, testConfigs ...ValidationTestConfig) { @@ -286,7 +281,7 @@ func WithMinEmulationVersion(v *version.Version) ValidationTestConfig { // guaranteeing a safe migration. It also checks the errors against an expected set. // It compares errors by field, origin and type; all three should match to be called equivalent. // It also make sure all versions of the given API returns equivalent errors. -func VerifyValidationEquivalence(t *testing.T, ctx context.Context, obj runtime.Object, validateFn ValidateFunc, expectedErrs field.ErrorList, testConfigs ...ValidationTestConfig) { +func VerifyValidationEquivalence(t *testing.T, ctx context.Context, obj runtime.Object, strategy rest.RESTCreateStrategy, expectedErrs field.ErrorList, testConfigs ...ValidationTestConfig) { t.Helper() opts := &validationOption{} for _, testcfg := range testConfigs { @@ -294,7 +289,15 @@ func VerifyValidationEquivalence(t *testing.T, ctx context.Context, obj runtime. } verifyValidationEquivalence(t, expectedErrs, func(c context.Context) field.ErrorList { - return validateFn(c, obj) + errs := strategy.Validate(c, obj) + if dv, ok := strategy.(rest.DeclarativeValidationStrategy); ok { + var config rest.DeclarativeValidationConfig + if vc, ok := strategy.(rest.DeclarativeValidationConfigurer); ok { + config = vc.DeclarativeValidationConfig(c, obj, nil) + } + errs = dv.ValidateDeclaratively(c, obj, nil, errs, operation.Create, config) + } + return errs }, ctx, opts) VerifyVersionedValidationEquivalence(t, obj, nil, testConfigs...) } @@ -317,7 +320,7 @@ func VerifyValidationEquivalence(t *testing.T, ctx context.Context, obj runtime. // guaranteeing a safe migration. It also checks the errors against an expected set. // It compares errors by field, origin and type; all three should match to be called equivalent. // It also make sure all versions of the given API returns equivalent errors. -func VerifyUpdateValidationEquivalence(t *testing.T, ctx context.Context, obj, old runtime.Object, validateUpdateFn ValidateUpdateFunc, expectedErrs field.ErrorList, testConfigs ...ValidationTestConfig) { +func VerifyUpdateValidationEquivalence(t *testing.T, ctx context.Context, obj, old runtime.Object, strategy rest.RESTUpdateStrategy, expectedErrs field.ErrorList, testConfigs ...ValidationTestConfig) { t.Helper() opts := &validationOption{} for _, testcfg := range testConfigs { @@ -325,7 +328,15 @@ func VerifyUpdateValidationEquivalence(t *testing.T, ctx context.Context, obj, o } verifyValidationEquivalence(t, expectedErrs, func(c context.Context) field.ErrorList { - return validateUpdateFn(c, obj, old) + errs := strategy.ValidateUpdate(c, obj, old) + if dv, ok := strategy.(rest.DeclarativeValidationStrategy); ok { + var config rest.DeclarativeValidationConfig + if vc, ok := strategy.(rest.DeclarativeValidationConfigurer); ok { + config = vc.DeclarativeValidationConfig(c, obj, old) + } + errs = dv.ValidateDeclaratively(c, obj, old, errs, operation.Update, config) + } + return errs }, ctx, opts) VerifyVersionedValidationEquivalence(t, obj, old, testConfigs...) } diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/create.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/create.go index 87b2c9d81bfdb..8755f60a341bd 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/create.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/create.go @@ -22,6 +22,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/api/operation" "k8s.io/apimachinery/pkg/api/validate/content" genericvalidation "k8s.io/apimachinery/pkg/api/validation" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -126,7 +127,11 @@ func BeforeCreate(strategy RESTCreateStrategy, ctx context.Context, obj runtime. strategy.PrepareForCreate(ctx, obj) - if errs := strategy.Validate(ctx, obj); len(errs) > 0 { + errs := strategy.Validate(ctx, obj) + if dv, ok := strategy.(DeclarativeValidationStrategy); ok { + errs = dv.ValidateDeclaratively(ctx, obj, nil, errs, operation.Create, declarativeValidationOptions(ctx, strategy, obj)) + } + if len(errs) > 0 { return errors.NewInvalid(kind.GroupKind(), objectMeta.GetName(), errs) } @@ -146,6 +151,14 @@ func BeforeCreate(strategy RESTCreateStrategy, ctx context.Context, obj runtime. return nil } +func declarativeValidationOptions(ctx context.Context, strategy RESTCreateStrategy, obj runtime.Object) DeclarativeValidationConfig { + var config DeclarativeValidationConfig + if vc, ok := strategy.(DeclarativeValidationConfigurer); ok { + config = vc.DeclarativeValidationConfig(ctx, obj, nil) + } + return config +} + // CheckGeneratedNameError checks whether an error that occurred creating a resource is due // to generation being unable to pick a valid name. func CheckGeneratedNameError(ctx context.Context, strategy RESTCreateStrategy, err error, obj runtime.Object) error { diff --git a/staging/src/k8s.io/apiserver/pkg/registry/rest/update.go b/staging/src/k8s.io/apiserver/pkg/registry/rest/update.go index 32de351aa7ae9..d2b4f68f3ba88 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/update.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/update.go @@ -22,6 +22,7 @@ import ( "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/api/operation" genericvalidation "k8s.io/apimachinery/pkg/api/validation" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -151,6 +152,9 @@ func BeforeUpdate(strategy RESTUpdateStrategy, ctx context.Context, obj, old run } errs = append(errs, strategy.ValidateUpdate(ctx, obj, old)...) + if dv, ok := strategy.(DeclarativeValidationStrategy); ok { + errs = dv.ValidateDeclaratively(ctx, obj, old, errs, operation.Update, declarativeValidationUpdateOptions(ctx, strategy, obj, old)) + } if len(errs) > 0 { RecordDuplicateValidationErrors(ctx, kind.GroupKind(), errs) return errors.NewInvalid(kind.GroupKind(), objectMeta.GetName(), errs) @@ -165,6 +169,14 @@ func BeforeUpdate(strategy RESTUpdateStrategy, ctx context.Context, obj, old run return nil } +func declarativeValidationUpdateOptions(ctx context.Context, strategy RESTUpdateStrategy, obj, old runtime.Object) DeclarativeValidationConfig { + var config DeclarativeValidationConfig + if vc, ok := strategy.(DeclarativeValidationConfigurer); ok { + config = vc.DeclarativeValidationConfig(ctx, obj, old) + } + return config +} + // TransformFunc is a function to transform and return newObj type TransformFunc func(ctx context.Context, newObj runtime.Object, oldObj runtime.Object) (transformedNewObj runtime.Object, err error) 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 f0e355e54f920..5af704ef57195 100644 --- a/staging/src/k8s.io/apiserver/pkg/registry/rest/validate.go +++ b/staging/src/k8s.io/apiserver/pkg/registry/rest/validate.go @@ -34,57 +34,63 @@ import ( "k8s.io/klog/v2" ) -// ValidationConfig defines how a declarative validation request may be configured. -type ValidationConfig func(*validationConfigOption) - -// WithOptions sets the validation options. -// Options should contain any validation options that the declarative validation -// tags expect. These often correspond to feature gates. -func WithOptions(options []string) ValidationConfig { - return func(config *validationConfigOption) { - config.options = options - } +// DeclarativeValidationStrategy defines how a strategy may opt-in to declarative validation. +// +// When strategies implements ValidateDeclaratively and handwritten validation (Validate / ValidateUpdate), +// the errors of both are merged and migration checks are performed. +type DeclarativeValidationStrategy interface { + // ValidateDeclaratively runs declarative validation, merges the declarative validation errors with any + // validationErrs returned from the strategy's Validate / ValidateUpdate functions (which implement hand-written validation) + // and performs migration checks. + ValidateDeclaratively(ctx context.Context, obj, oldObj runtime.Object, validationErrs field.ErrorList, opType operation.Type, config DeclarativeValidationConfig) field.ErrorList } -// WithSubresourceMapper sets the subresource mapper for validation. -// This should be used when registering validation for polymorphic subresources like /scale. -// -// For example, the deployments/scale subresource mapper might map from: +// DeclarativeValidation is an implementation of DeclarativeValidationStrategy that +// provides a convenient way for a strategy to opt-in to declarative validation. // -// group: apps, version: v1, subresource=scale +// For example: // -// to a target of: +// type podStrategy struct { +// rest.DeclarativeValidation +// names.NameGenerator +// } +// var Strategy = podStrategy{rest.DeclarativeValidation{Scheme: legacyscheme.Scheme}, names.SimpleNameGenerator} // -// group: autoscaling, version: v1, kind=Scale -// -// When set, the group version in the requestInfo of the ctx provided to a declarative validation -// request will be passed to the subresource mapper to find the group version kind of the subresource. -// Declarative validation will then convert the object to the subresource group version kind and validate it. -// -// Note that the target of the mapping contains no subresource part since the mapper is expected to -// map to the group version kind of the subresource. -func WithSubresourceMapper(subresourceMapper GroupVersionKindProvider) ValidationConfig { - return func(config *validationConfigOption) { - config.subresourceGVKMapper = subresourceMapper - } +// Once a strategy opts-in this way, any generated declarative validation code is run automatically. +type DeclarativeValidation struct { + *runtime.Scheme } -// WithNormalizationRules sets the normalization rules for validation. -func WithNormalizationRules(rules []field.NormalizationRule) ValidationConfig { - return func(config *validationConfigOption) { - config.normalizationRules = rules - } +func (d DeclarativeValidation) ValidateDeclaratively(ctx context.Context, obj, oldObj runtime.Object, validationErrs field.ErrorList, opType operation.Type, config DeclarativeValidationConfig) field.ErrorList { + return ValidateDeclarativelyWithMigrationChecks(ctx, d.Scheme, obj, oldObj, validationErrs, opType, config) } -// WithDeclarativeEnforcement marks the validation configuration to indicate that it includes -// declarative validations that should follow the fine-grained Validation Lifecycle. -// When set, declarative validation is always executed regardless of feature gates. -// Authority is determined by individual tag prefixes (+k8s:alpha, +k8s:beta) and the -// DeclarativeValidationBeta safety switch. -func WithDeclarativeEnforcement() ValidationConfig { - return func(config *validationConfigOption) { - config.declarativeEnforcement = true - } +// DeclarativeValidationConfigurer defines how a strategy may opt-in to configuration of declarative validation. +type DeclarativeValidationConfigurer interface { + // DeclarativeValidationConfig configures declarative validation for a single request. + DeclarativeValidationConfig(ctx context.Context, obj, oldObj runtime.Object) DeclarativeValidationConfig +} + +// DeclarativeValidationConfig holds configuration for declarative validation. +// Strategies that need to customize declarative validation behavior implement +// DeclarativeValidationConfigurer and return this struct. +type DeclarativeValidationConfig struct { + // Options contains validation options that declarative validation tags + // expect. These often correspond to feature gates. + Options []string + + // DeclarativeEnforcement indicates that declarative validations should + // follow the fine-grained Validation Lifecycle. When set, declarative + // validation is always executed regardless of feature gates. + DeclarativeEnforcement bool + + // NormalizationRules are applied to field paths when comparing + // handwritten and declarative validation errors. + NormalizationRules []field.NormalizationRule + + // SubresourceGVKMapper maps a subresource request to the GVK of the + // subresource type for polymorphic subresources like /scale. + SubresourceGVKMapper GroupVersionKindProvider } type allDeclarativeEnforcedKeyType struct{} @@ -100,13 +106,12 @@ func WithAllDeclarativeEnforcedForTest(ctx context.Context) context.Context { return context.WithValue(ctx, allDeclarativeEnforcedKey, true) } -type validationConfigOption struct { - opType operation.Type - options []string - subresourceGVKMapper GroupVersionKindProvider - validationIdentifier string - normalizationRules []field.NormalizationRule - declarativeEnforcement bool +// ValidationConfigOption is the internal configuration used by +// ValidateDeclarativelyWithMigrationChecks. It is exported for use in tests. +type ValidationConfigOption struct { + OpType operation.Type + ValidationIdentifier string + DeclarativeValidationConfig } // validateDeclaratively validates obj and oldObj against declarative @@ -121,9 +126,9 @@ type validationConfigOption struct { // 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, oldObj runtime.Object, o *validationConfigOption) field.ErrorList { +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. - ... [truncated]