feedback

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) {

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
+// validateStructuralCompleteness checks that all value validations in s have
-// outside of value validation.
+// 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
+	var sAdditionalPropertiesSchema *Structural
-	if s.AdditionalProperties != nil && s.AdditionalProperties.Structural != nil {
+	if s.AdditionalProperties != nil {
-		additionalPropertiesSchema = s.AdditionalProperties.Structural
+		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)...)

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,