diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation.go index a0341e75195..b265271555f 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/validation.go @@ -93,7 +93,6 @@ func NewValidator(s *schema.Structural, isResourceRoot bool, perCallLimit uint64 // exist. declType is expected to be a CEL DeclType corresponding to the structural schema. // perCallLimit was added for testing purpose only. Callers should always use const PerCallLimit from k8s.io/apiserver/pkg/apis/cel/config.go as input. func validator(validationSchema, nodeSchema *schema.Structural, isResourceRoot bool, declType *cel.DeclType, perCallLimit uint64) *Validator { - compilationSchema := *nodeSchema compilationSchema.XValidations = validationSchema.XValidations compiledRules, err := Compile(&compilationSchema, declType, perCallLimit, environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true), StoredExpressionsEnvLoader()) @@ -291,7 +290,6 @@ func nestedToStructural(nested *schema.NestedValueValidation) *schema.Structural } return structuralConversion - } func (s *Validator) validate(ctx context.Context, fldPath *field.Path, obj, oldObj interface{}, correlation ratchetingOptions, costBudget int64) (errs field.ErrorList, remainingBudget int64) { diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/complete.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/complete.go index a6a81c12c73..e9213660047 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/complete.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/complete.go @@ -22,8 +22,15 @@ import ( "k8s.io/apimachinery/pkg/util/validation/field" ) -// validateStructuralCompleteness checks that every specified field or array in s is also specified -// outside of value validation. +// validateStructuralCompleteness checks that all value validations in s have +// a structural counterpart so that every value validation applies to a value +// with a known schema: +// - validations for specific properties must have that property (or additionalProperties under an option) structurally defined +// - additionalProperties validations must have additionalProperties defined in the structural portion of the schema corresponding to that node +// - Items validations must have also have a corresponding items structurally +// +// The "structural" portion of the schema refers to all nodes in the +// schema traversible without following any NestedValueValidations. func validateStructuralCompleteness(s *Structural, fldPath *field.Path, opts ValidationOptions) field.ErrorList { if s == nil { return nil @@ -69,17 +76,22 @@ func validateNestedValueValidationCompleteness(v *NestedValueValidation, s *Stru allErrs = append(allErrs, validateValueValidationCompleteness(&v.ValueValidation, s, sPath, vPath, opts)...) allErrs = append(allErrs, validateNestedValueValidationCompleteness(v.Items, s.Items, sPath.Child("items"), vPath.Child("items"), opts)...) - var additionalPropertiesSchema *Structural - if s.AdditionalProperties != nil && s.AdditionalProperties.Structural != nil { - additionalPropertiesSchema = s.AdditionalProperties.Structural + var sAdditionalPropertiesSchema *Structural + if s.AdditionalProperties != nil { + sAdditionalPropertiesSchema = s.AdditionalProperties.Structural } for k, vFld := range v.Properties { if sFld, ok := s.Properties[k]; !ok { - if additionalPropertiesSchema == nil || !opts.AllowValidationPropertiesWithAdditionalProperties { + if sAdditionalPropertiesSchema == nil || !opts.AllowValidationPropertiesWithAdditionalProperties { allErrs = append(allErrs, field.Required(sPath.Child("properties").Key(k), fmt.Sprintf("because it is defined in %s", vPath.Child("properties").Key(k)))) } else { - allErrs = append(allErrs, validateNestedValueValidationCompleteness(&vFld, additionalPropertiesSchema, sPath.Child("additionalProperties"), vPath.Child("properties").Key(k), opts)...) + // Allow validations on specific properties if there exists an + // additionalProperties structural schema specified instead of + // direct properties + // NOTE: This does not allow `additionalProperties: true` structural + // schema to be combined with specific property validations. + allErrs = append(allErrs, validateNestedValueValidationCompleteness(&vFld, sAdditionalPropertiesSchema, sPath.Child("additionalProperties"), vPath.Child("properties").Key(k), opts)...) } } else { allErrs = append(allErrs, validateNestedValueValidationCompleteness(&vFld, &sFld, sPath.Child("properties").Key(k), vPath.Child("properties").Key(k), opts)...) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/validation.go index 9f904efedb3..a9caddb38d7 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/validation.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/validation.go @@ -78,7 +78,7 @@ type ValidationOptions struct { // * additionalProperties at the root is not allowed. func ValidateStructural(fldPath *field.Path, s *Structural) field.ErrorList { return ValidateStructuralWithOptions(fldPath, s, ValidationOptions{ - // This widens the schema for CRDs, so first few releases will still + // This would widen the schema for CRD if set to true, so first few releases will still // not admit any. But it can still be used by libraries and // declarative validation for native types AllowNestedAdditionalProperties: false,