Skip CEL expression validation if OpenAPIv3 schema is invalid

Co-authored-by: Jordan Liggitt <jordan@liggitt.net>
This commit is contained in:
Joe Betz 2022-07-25 14:25:55 -04:00
parent c06031959f
commit 95b0d44a56
2 changed files with 270 additions and 69 deletions

View File

@ -722,10 +722,35 @@ func validateCustomResourceDefinitionValidation(ctx context.Context, customResou
requireValidPropertyType: opts.requireValidPropertyType,
}
celContext := RootCELContext(schema)
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema, fldPath.Child("openAPIV3Schema"), openAPIV3Schema, true, &opts, celContext)...)
var celContext *CELSchemaContext
var structuralSchemaInitErrs field.ErrorList
if opts.requireStructuralSchema {
if ss, err := structuralschema.NewStructural(schema); err != nil {
// These validation errors overlap with OpenAPISchema validation errors so we keep track of them
// separately and only show them if OpenAPISchema validation does not report any errors.
structuralSchemaInitErrs = append(structuralSchemaInitErrs, field.Invalid(fldPath.Child("openAPIV3Schema"), "", err.Error()))
} else if validationErrors := structuralschema.ValidateStructural(fldPath.Child("openAPIV3Schema"), ss); len(validationErrors) > 0 {
allErrs = append(allErrs, validationErrors...)
} else if validationErrors, err := structuraldefaulting.ValidateDefaults(ctx, fldPath.Child("openAPIV3Schema"), ss, true, opts.requirePrunedDefaults); err != nil {
// this should never happen
allErrs = append(allErrs, field.Invalid(fldPath.Child("openAPIV3Schema"), "", err.Error()))
} else if len(validationErrors) > 0 {
allErrs = append(allErrs, validationErrors...)
} else {
// Only initialize CEL rule validation context if the structural schemas are valid.
// A nil CELSchemaContext indicates that no CEL validation should be attempted.
celContext = RootCELContext(schema)
}
}
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema, fldPath.Child("openAPIV3Schema"), openAPIV3Schema, true, &opts, celContext).AllErrors()...)
if celContext.TotalCost != nil {
if len(allErrs) == 0 && len(structuralSchemaInitErrs) > 0 {
// Structural schema initialization errors overlap with OpenAPISchema validation errors so we only show them
// if there are no OpenAPISchema validation errors.
allErrs = append(allErrs, structuralSchemaInitErrs...)
}
if celContext != nil && celContext.TotalCost != nil {
if celContext.TotalCost.Total > StaticEstimatedCRDCostLimit {
for _, expensive := range celContext.TotalCost.MostExpensive {
costErrorMsg := fmt.Sprintf("contributed to estimated rule cost total exceeding cost limit for entire OpenAPIv3 schema")
@ -736,22 +761,6 @@ func validateCustomResourceDefinitionValidation(ctx context.Context, customResou
allErrs = append(allErrs, field.Forbidden(fldPath.Child("openAPIV3Schema"), costErrorMsg))
}
}
if opts.requireStructuralSchema {
if ss, err := structuralschema.NewStructural(schema); err != nil {
// if the generic schema validation did its job, we should never get an error here. Hence, we hide it if there are validation errors already.
if len(allErrs) == 0 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("openAPIV3Schema"), "", err.Error()))
}
} else if validationErrors := structuralschema.ValidateStructural(fldPath.Child("openAPIV3Schema"), ss); len(validationErrors) > 0 {
allErrs = append(allErrs, validationErrors...)
} else if validationErrors, err := structuraldefaulting.ValidateDefaults(ctx, fldPath.Child("openAPIV3Schema"), ss, true, opts.requirePrunedDefaults); err != nil {
// this should never happen
allErrs = append(allErrs, field.Invalid(fldPath.Child("openAPIV3Schema"), "", err.Error()))
} else {
allErrs = append(allErrs, validationErrors...)
}
}
}
// if validation passed otherwise, make sure we can actually construct a schema validator from this custom resource validation.
@ -765,17 +774,41 @@ func validateCustomResourceDefinitionValidation(ctx context.Context, customResou
var metaFields = sets.NewString("metadata", "kind", "apiVersion")
// OpenAPISchemaErrorList tracks all validation errors reported ValidateCustomResourceDefinitionOpenAPISchema
// with CEL related errors kept separate from schema related errors.
type OpenAPISchemaErrorList struct {
SchemaErrors field.ErrorList
CELErrors field.ErrorList
}
// AppendErrors appends all errors in the provided list with the errors of this list.
func (o *OpenAPISchemaErrorList) AppendErrors(list *OpenAPISchemaErrorList) {
if o == nil || list == nil {
return
}
o.SchemaErrors = append(o.SchemaErrors, list.SchemaErrors...)
o.CELErrors = append(o.CELErrors, list.CELErrors...)
}
// AllErrors returns a list containing both schema and CEL errors.
func (o *OpenAPISchemaErrorList) AllErrors() field.ErrorList {
if o == nil {
return field.ErrorList{}
}
return append(o.SchemaErrors, o.CELErrors...)
}
// ValidateCustomResourceDefinitionOpenAPISchema statically validates
func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSchemaProps, fldPath *field.Path, ssv specStandardValidator, isRoot bool, opts *validationOptions, celContext *CELSchemaContext) field.ErrorList {
allErrs := field.ErrorList{}
func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSchemaProps, fldPath *field.Path, ssv specStandardValidator, isRoot bool, opts *validationOptions, celContext *CELSchemaContext) *OpenAPISchemaErrorList {
allErrs := &OpenAPISchemaErrorList{SchemaErrors: field.ErrorList{}, CELErrors: field.ErrorList{}}
if schema == nil {
return allErrs
}
allErrs = append(allErrs, ssv.validate(schema, fldPath)...)
allErrs.SchemaErrors = append(allErrs.SchemaErrors, ssv.validate(schema, fldPath)...)
if schema.UniqueItems == true {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("uniqueItems"), "uniqueItems cannot be set to true since the runtime complexity becomes quadratic"))
allErrs.SchemaErrors = append(allErrs.SchemaErrors, field.Forbidden(fldPath.Child("uniqueItems"), "uniqueItems cannot be set to true since the runtime complexity becomes quadratic"))
}
// additionalProperties and properties are mutual exclusive because otherwise they
@ -790,7 +823,7 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch
if schema.AdditionalProperties != nil {
if len(schema.Properties) != 0 {
if schema.AdditionalProperties.Allows == false || schema.AdditionalProperties.Schema != nil {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("additionalProperties"), "additionalProperties and properties are mutual exclusive"))
allErrs.SchemaErrors = append(allErrs.SchemaErrors, field.Forbidden(fldPath.Child("additionalProperties"), "additionalProperties and properties are mutual exclusive"))
}
}
// Note: we forbid additionalProperties at resource root, both embedded and top-level.
@ -800,7 +833,7 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch
// we have to forbid defaults inside additionalProperties because pruning without actual value is ambiguous
subSsv = ssv.withForbiddenDefaults("inside additionalProperties applying to object metadata")
}
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.AdditionalProperties.Schema, fldPath.Child("additionalProperties"), subSsv, false, opts, celContext.ChildAdditionalPropertiesContext(schema.AdditionalProperties.Schema))...)
allErrs.AppendErrors(ValidateCustomResourceDefinitionOpenAPISchema(schema.AdditionalProperties.Schema, fldPath.Child("additionalProperties"), subSsv, false, opts, celContext.ChildAdditionalPropertiesContext(schema.AdditionalProperties.Schema)))
}
if len(schema.Properties) != 0 {
@ -819,37 +852,37 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch
}
}
propertySchema := jsonSchema
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&propertySchema, fldPath.Child("properties").Key(property), subSsv, false, opts, celContext.ChildPropertyContext(&propertySchema, property))...)
allErrs.AppendErrors(ValidateCustomResourceDefinitionOpenAPISchema(&propertySchema, fldPath.Child("properties").Key(property), subSsv, false, opts, celContext.ChildPropertyContext(&propertySchema, property)))
}
}
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.Not, fldPath.Child("not"), ssv, false, opts, nil)...)
allErrs.AppendErrors(ValidateCustomResourceDefinitionOpenAPISchema(schema.Not, fldPath.Child("not"), ssv, false, opts, nil))
if len(schema.AllOf) != 0 {
for i, jsonSchema := range schema.AllOf {
allOfSchema := jsonSchema
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&allOfSchema, fldPath.Child("allOf").Index(i), ssv, false, opts, nil)...)
allErrs.AppendErrors(ValidateCustomResourceDefinitionOpenAPISchema(&allOfSchema, fldPath.Child("allOf").Index(i), ssv, false, opts, nil))
}
}
if len(schema.OneOf) != 0 {
for i, jsonSchema := range schema.OneOf {
oneOfSchema := jsonSchema
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&oneOfSchema, fldPath.Child("oneOf").Index(i), ssv, false, opts, nil)...)
allErrs.AppendErrors(ValidateCustomResourceDefinitionOpenAPISchema(&oneOfSchema, fldPath.Child("oneOf").Index(i), ssv, false, opts, nil))
}
}
if len(schema.AnyOf) != 0 {
for i, jsonSchema := range schema.AnyOf {
anyOfSchema := jsonSchema
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&anyOfSchema, fldPath.Child("anyOf").Index(i), ssv, false, opts, nil)...)
allErrs.AppendErrors(ValidateCustomResourceDefinitionOpenAPISchema(&anyOfSchema, fldPath.Child("anyOf").Index(i), ssv, false, opts, nil))
}
}
if len(schema.Definitions) != 0 {
for definition, jsonSchema := range schema.Definitions {
definitionSchema := jsonSchema
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&definitionSchema, fldPath.Child("definitions").Key(definition), ssv, false, opts, nil)...)
allErrs.AppendErrors(ValidateCustomResourceDefinitionOpenAPISchema(&definitionSchema, fldPath.Child("definitions").Key(definition), ssv, false, opts, nil))
}
}
@ -863,84 +896,84 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch
subSsv = subSsv.withForbidOldSelfValidations(fldPath)
}
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.Items.Schema, fldPath.Child("items"), subSsv, false, opts, celContext.ChildItemsContext(schema.Items.Schema))...)
allErrs.AppendErrors(ValidateCustomResourceDefinitionOpenAPISchema(schema.Items.Schema, fldPath.Child("items"), subSsv, false, opts, celContext.ChildItemsContext(schema.Items.Schema)))
if len(schema.Items.JSONSchemas) != 0 {
for i, jsonSchema := range schema.Items.JSONSchemas {
itemsSchema := jsonSchema
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&itemsSchema, fldPath.Child("items").Index(i), subSsv, false, opts, celContext.ChildItemsContext(&itemsSchema))...)
allErrs.AppendErrors(ValidateCustomResourceDefinitionOpenAPISchema(&itemsSchema, fldPath.Child("items").Index(i), subSsv, false, opts, celContext.ChildItemsContext(&itemsSchema)))
}
}
}
if schema.Dependencies != nil {
for dependency, jsonSchemaPropsOrStringArray := range schema.Dependencies {
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(jsonSchemaPropsOrStringArray.Schema, fldPath.Child("dependencies").Key(dependency), ssv, false, opts, nil)...)
allErrs.AppendErrors(ValidateCustomResourceDefinitionOpenAPISchema(jsonSchemaPropsOrStringArray.Schema, fldPath.Child("dependencies").Key(dependency), ssv, false, opts, nil))
}
}
if schema.XPreserveUnknownFields != nil && *schema.XPreserveUnknownFields == false {
allErrs = append(allErrs, field.Invalid(fldPath.Child("x-kubernetes-preserve-unknown-fields"), *schema.XPreserveUnknownFields, "must be true or undefined"))
allErrs.SchemaErrors = append(allErrs.SchemaErrors, field.Invalid(fldPath.Child("x-kubernetes-preserve-unknown-fields"), *schema.XPreserveUnknownFields, "must be true or undefined"))
}
if schema.XMapType != nil && schema.Type != "object" {
if len(schema.Type) == 0 {
allErrs = append(allErrs, field.Required(fldPath.Child("type"), "must be object if x-kubernetes-map-type is specified"))
allErrs.SchemaErrors = append(allErrs.SchemaErrors, field.Required(fldPath.Child("type"), "must be object if x-kubernetes-map-type is specified"))
} else {
allErrs = append(allErrs, field.Invalid(fldPath.Child("type"), schema.Type, "must be object if x-kubernetes-map-type is specified"))
allErrs.SchemaErrors = append(allErrs.SchemaErrors, field.Invalid(fldPath.Child("type"), schema.Type, "must be object if x-kubernetes-map-type is specified"))
}
}
if schema.XMapType != nil && *schema.XMapType != "atomic" && *schema.XMapType != "granular" {
allErrs = append(allErrs, field.NotSupported(fldPath.Child("x-kubernetes-map-type"), *schema.XMapType, []string{"atomic", "granular"}))
allErrs.SchemaErrors = append(allErrs.SchemaErrors, field.NotSupported(fldPath.Child("x-kubernetes-map-type"), *schema.XMapType, []string{"atomic", "granular"}))
}
if schema.XListType != nil && schema.Type != "array" {
if len(schema.Type) == 0 {
allErrs = append(allErrs, field.Required(fldPath.Child("type"), "must be array if x-kubernetes-list-type is specified"))
allErrs.SchemaErrors = append(allErrs.SchemaErrors, field.Required(fldPath.Child("type"), "must be array if x-kubernetes-list-type is specified"))
} else {
allErrs = append(allErrs, field.Invalid(fldPath.Child("type"), schema.Type, "must be array if x-kubernetes-list-type is specified"))
allErrs.SchemaErrors = append(allErrs.SchemaErrors, field.Invalid(fldPath.Child("type"), schema.Type, "must be array if x-kubernetes-list-type is specified"))
}
} else if opts.requireAtomicSetType && schema.XListType != nil && *schema.XListType == "set" && schema.Items != nil && schema.Items.Schema != nil { // by structural schema items are present
is := schema.Items.Schema
switch is.Type {
case "array":
if is.XListType != nil && *is.XListType != "atomic" { // atomic is the implicit default behaviour if unset, hence != atomic is wrong
allErrs = append(allErrs, field.Invalid(fldPath.Child("items").Child("x-kubernetes-list-type"), is.XListType, "must be atomic as item of a list with x-kubernetes-list-type=set"))
allErrs.SchemaErrors = append(allErrs.SchemaErrors, field.Invalid(fldPath.Child("items").Child("x-kubernetes-list-type"), is.XListType, "must be atomic as item of a list with x-kubernetes-list-type=set"))
}
case "object":
if is.XMapType == nil || *is.XMapType != "atomic" { // granular is the implicit default behaviour if unset, hence nil and != atomic are wrong
allErrs = append(allErrs, field.Invalid(fldPath.Child("items").Child("x-kubernetes-map-type"), is.XListType, "must be atomic as item of a list with x-kubernetes-list-type=set"))
allErrs.SchemaErrors = append(allErrs.SchemaErrors, field.Invalid(fldPath.Child("items").Child("x-kubernetes-map-type"), is.XListType, "must be atomic as item of a list with x-kubernetes-list-type=set"))
}
}
}
if schema.XListType != nil && *schema.XListType != "atomic" && *schema.XListType != "set" && *schema.XListType != "map" {
allErrs = append(allErrs, field.NotSupported(fldPath.Child("x-kubernetes-list-type"), *schema.XListType, []string{"atomic", "set", "map"}))
allErrs.SchemaErrors = append(allErrs.SchemaErrors, field.NotSupported(fldPath.Child("x-kubernetes-list-type"), *schema.XListType, []string{"atomic", "set", "map"}))
}
if len(schema.XListMapKeys) > 0 {
if schema.XListType == nil {
allErrs = append(allErrs, field.Required(fldPath.Child("x-kubernetes-list-type"), "must be map if x-kubernetes-list-map-keys is non-empty"))
allErrs.SchemaErrors = append(allErrs.SchemaErrors, field.Required(fldPath.Child("x-kubernetes-list-type"), "must be map if x-kubernetes-list-map-keys is non-empty"))
} else if *schema.XListType != "map" {
allErrs = append(allErrs, field.Invalid(fldPath.Child("x-kubernetes-list-type"), *schema.XListType, "must be map if x-kubernetes-list-map-keys is non-empty"))
allErrs.SchemaErrors = append(allErrs.SchemaErrors, field.Invalid(fldPath.Child("x-kubernetes-list-type"), *schema.XListType, "must be map if x-kubernetes-list-map-keys is non-empty"))
}
}
if schema.XListType != nil && *schema.XListType == "map" {
if len(schema.XListMapKeys) == 0 {
allErrs = append(allErrs, field.Required(fldPath.Child("x-kubernetes-list-map-keys"), "must not be empty if x-kubernetes-list-type is map"))
allErrs.SchemaErrors = append(allErrs.SchemaErrors, field.Required(fldPath.Child("x-kubernetes-list-map-keys"), "must not be empty if x-kubernetes-list-type is map"))
}
if schema.Items == nil {
allErrs = append(allErrs, field.Required(fldPath.Child("items"), "must have a schema if x-kubernetes-list-type is map"))
allErrs.SchemaErrors = append(allErrs.SchemaErrors, field.Required(fldPath.Child("items"), "must have a schema if x-kubernetes-list-type is map"))
}
if schema.Items != nil && schema.Items.Schema == nil {
allErrs = append(allErrs, field.Invalid(fldPath.Child("items"), schema.Items, "must only have a single schema if x-kubernetes-list-type is map"))
allErrs.SchemaErrors = append(allErrs.SchemaErrors, field.Invalid(fldPath.Child("items"), schema.Items, "must only have a single schema if x-kubernetes-list-type is map"))
}
if schema.Items != nil && schema.Items.Schema != nil && schema.Items.Schema.Type != "object" {
allErrs = append(allErrs, field.Invalid(fldPath.Child("items").Child("type"), schema.Items.Schema.Type, "must be object if parent array's x-kubernetes-list-type is map"))
allErrs.SchemaErrors = append(allErrs.SchemaErrors, field.Invalid(fldPath.Child("items").Child("type"), schema.Items.Schema.Type, "must be object if parent array's x-kubernetes-list-type is map"))
}
if schema.Items != nil && schema.Items.Schema != nil && schema.Items.Schema.Type == "object" {
@ -948,64 +981,72 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch
for _, k := range schema.XListMapKeys {
if s, ok := schema.Items.Schema.Properties[k]; ok {
if s.Type == "array" || s.Type == "object" {
allErrs = append(allErrs, field.Invalid(fldPath.Child("items").Child("properties").Key(k).Child("type"), schema.Items.Schema.Type, "must be a scalar type if parent array's x-kubernetes-list-type is map"))
allErrs.SchemaErrors = append(allErrs.SchemaErrors, field.Invalid(fldPath.Child("items").Child("properties").Key(k).Child("type"), schema.Items.Schema.Type, "must be a scalar type if parent array's x-kubernetes-list-type is map"))
}
} else {
allErrs = append(allErrs, field.Invalid(fldPath.Child("x-kubernetes-list-map-keys"), schema.XListMapKeys, "entries must all be names of item properties"))
allErrs.SchemaErrors = append(allErrs.SchemaErrors, field.Invalid(fldPath.Child("x-kubernetes-list-map-keys"), schema.XListMapKeys, "entries must all be names of item properties"))
}
if _, ok := keys[k]; ok {
allErrs = append(allErrs, field.Invalid(fldPath.Child("x-kubernetes-list-map-keys"), schema.XListMapKeys, "must not contain duplicate entries"))
allErrs.SchemaErrors = append(allErrs.SchemaErrors, field.Invalid(fldPath.Child("x-kubernetes-list-map-keys"), schema.XListMapKeys, "must not contain duplicate entries"))
}
keys[k] = struct{}{}
}
}
}
if opts.requireMapListKeysMapSetValidation {
allErrs.SchemaErrors = append(allErrs.SchemaErrors, validateMapListKeysMapSet(schema, fldPath)...)
}
if len(schema.XValidations) > 0 {
for i, rule := range schema.XValidations {
trimmedRule := strings.TrimSpace(rule.Rule)
trimmedMsg := strings.TrimSpace(rule.Message)
if len(trimmedRule) == 0 {
allErrs = append(allErrs, field.Required(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), "rule is not specified"))
allErrs.SchemaErrors = append(allErrs.SchemaErrors, field.Required(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), "rule is not specified"))
} else if len(rule.Message) > 0 && len(trimmedMsg) == 0 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("x-kubernetes-validations").Index(i).Child("message"), rule.Message, "message must be non-empty if specified"))
allErrs.SchemaErrors = append(allErrs.SchemaErrors, field.Invalid(fldPath.Child("x-kubernetes-validations").Index(i).Child("message"), rule.Message, "message must be non-empty if specified"))
} else if hasNewlines(trimmedMsg) {
allErrs = append(allErrs, field.Invalid(fldPath.Child("x-kubernetes-validations").Index(i).Child("message"), rule.Message, "message must not contain line breaks"))
allErrs.SchemaErrors = append(allErrs.SchemaErrors, field.Invalid(fldPath.Child("x-kubernetes-validations").Index(i).Child("message"), rule.Message, "message must not contain line breaks"))
} else if hasNewlines(trimmedRule) && len(trimmedMsg) == 0 {
allErrs = append(allErrs, field.Required(fldPath.Child("x-kubernetes-validations").Index(i).Child("message"), "message must be specified if rule contains line breaks"))
allErrs.SchemaErrors = append(allErrs.SchemaErrors, field.Required(fldPath.Child("x-kubernetes-validations").Index(i).Child("message"), "message must be specified if rule contains line breaks"))
}
}
if celContext != nil {
// If any schema related validation errors have been found at this level or deeper, skip CEL expression validation.
// Invalid OpenAPISchemas are not always possible to convert into valid CEL DeclTypes, and can lead to CEL
// validation error messages that are not actionable (will go away once the schema errors are resolved) and that
// are difficult for CEL expression authors to understand.
if len(allErrs.SchemaErrors) == 0 && celContext != nil {
typeInfo, err := celContext.TypeInfo()
if err != nil {
allErrs = append(allErrs, field.InternalError(fldPath.Child("x-kubernetes-validations"), fmt.Errorf("internal error: failed to construct type information for x-kubernetes-validations rules: %s", err)))
allErrs.CELErrors = append(allErrs.CELErrors, field.InternalError(fldPath.Child("x-kubernetes-validations"), fmt.Errorf("internal error: failed to construct type information for x-kubernetes-validations rules: %s", err)))
} else if typeInfo == nil {
allErrs = append(allErrs, field.InternalError(fldPath.Child("x-kubernetes-validations"), fmt.Errorf("internal error: failed to retrieve type information for x-kubernetes-validations")))
allErrs.CELErrors = append(allErrs.CELErrors, field.InternalError(fldPath.Child("x-kubernetes-validations"), fmt.Errorf("internal error: failed to retrieve type information for x-kubernetes-validations")))
} else {
compResults, err := cel.Compile(typeInfo.Schema, typeInfo.DeclType, cel.PerCallLimit)
if err != nil {
allErrs = append(allErrs, field.InternalError(fldPath.Child("x-kubernetes-validations"), err))
allErrs.CELErrors = append(allErrs.CELErrors, field.InternalError(fldPath.Child("x-kubernetes-validations"), err))
} else {
for i, cr := range compResults {
expressionCost := getExpressionCost(cr, celContext)
if expressionCost > StaticEstimatedCostLimit {
costErrorMsg := getCostErrorMessage("estimated rule cost", expressionCost, StaticEstimatedCostLimit)
allErrs = append(allErrs, field.Forbidden(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), costErrorMsg))
allErrs.CELErrors = append(allErrs.CELErrors, field.Forbidden(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), costErrorMsg))
}
if celContext.TotalCost != nil {
celContext.TotalCost.ObserveExpressionCost(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), expressionCost)
}
if cr.Error != nil {
if cr.Error.Type == cel.ErrorTypeRequired {
allErrs = append(allErrs, field.Required(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), cr.Error.Detail))
allErrs.CELErrors = append(allErrs.CELErrors, field.Required(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), cr.Error.Detail))
} else {
allErrs = append(allErrs, field.Invalid(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), schema.XValidations[i], cr.Error.Detail))
allErrs.CELErrors = append(allErrs.CELErrors, field.Invalid(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), schema.XValidations[i], cr.Error.Detail))
}
}
if cr.TransitionRule {
if uncorrelatablePath := ssv.forbidOldSelfValidations(); uncorrelatablePath != nil {
allErrs = append(allErrs, field.Invalid(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), schema.XValidations[i].Rule, fmt.Sprintf("oldSelf cannot be used on the uncorrelatable portion of the schema within %v", uncorrelatablePath)))
allErrs.CELErrors = append(allErrs.CELErrors, field.Invalid(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), schema.XValidations[i].Rule, fmt.Sprintf("oldSelf cannot be used on the uncorrelatable portion of the schema within %v", uncorrelatablePath)))
}
}
}
@ -1014,10 +1055,6 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch
}
}
if opts.requireMapListKeysMapSetValidation {
allErrs = append(allErrs, validateMapListKeysMapSet(schema, fldPath)...)
}
return allErrs
}

View File

@ -7598,6 +7598,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "forbid transition rule on element of list of type atomic",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -7624,6 +7625,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "forbid transition rule on element of list defaulting to type atomic",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -7649,6 +7651,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "allow transition rule on list of type atomic",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -7695,6 +7698,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "forbid transition rule on element of list of type set",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -7722,6 +7726,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "allow transition rule on list of type set",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -7746,6 +7751,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "allow transition rule on element of list of type map",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -7773,6 +7779,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "allow transition rule on list of type map",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -7801,6 +7808,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "allow transition rule on element of map of type granular",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -7824,6 +7832,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "forbid transition rule on element of map of unrecognized type",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -7851,6 +7860,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "allow transition rule on element of map defaulting to type granular",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -7873,6 +7883,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "allow transition rule on map of type granular",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -7890,6 +7901,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "allow transition rule on map defaulting to type granular",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -7906,6 +7918,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "allow transition rule on element of map of type atomic",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -7928,6 +7941,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "allow transition rule on map of type atomic",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -7945,6 +7959,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "forbid double-nested rule with no limit set",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -7982,6 +7997,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "forbid double-nested rule with one limit set",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -8019,6 +8035,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "allow double-nested rule with three limits set",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -8052,6 +8069,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "allow double-nested rule with one limit set on outermost array",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -8084,6 +8102,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "check for cardinality of 1 under root object",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -8101,6 +8120,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "forbid validation rules where cost total exceeds total limit",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -8151,8 +8171,142 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
forbidden("spec.validation.openAPIV3Schema"),
},
},
{
name: "skip CEL expression validation when OpenAPIv3 schema is an invalid structural schema",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
// illegal to have both Properties and AdditionalProperties
Properties: map[string]apiextensions.JSONSchemaProps{
"field": {
Type: "integer",
},
},
AdditionalProperties: &apiextensions.JSONSchemaPropsOrBool{
Schema: &apiextensions.JSONSchemaProps{
Type: "string",
},
},
XValidations: apiextensions.ValidationRules{
{Rule: "self.invalidFieldName > 50"}, // invalid CEL rule
},
},
},
expectedErrors: []validationMatch{
forbidden("spec.validation.openAPIV3Schema.additionalProperties"), // illegal to have both properties and additional properties
forbidden("spec.validation.openAPIV3Schema.additionalProperties"), // structural schema rule: illegal to have additional properties at root
// Error for invalid CEL rule is NOT expected here because CEL rules are not checked when the schema is invalid
},
},
{
name: "skip CEL expression validation when OpenAPIv3 schema is an invalid structural schema at level below",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"field": {
Type: "object",
// illegal to have both Properties and AdditionalProperties
Properties: map[string]apiextensions.JSONSchemaProps{
"field": {
Type: "integer",
},
},
AdditionalProperties: &apiextensions.JSONSchemaPropsOrBool{
Schema: &apiextensions.JSONSchemaProps{
Type: "string",
},
},
},
},
XValidations: apiextensions.ValidationRules{
{Rule: "self.invalidFieldName > 50"},
},
},
},
expectedErrors: []validationMatch{
forbidden("spec.validation.openAPIV3Schema.properties[field].additionalProperties"),
},
},
{
// So long at the schema information accessible to the CEL expression is valid, the expression should be validated.
name: "do not skip when OpenAPIv3 schema is an invalid structural schema in a separate part of the schema tree",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"a": {
Type: "object",
// illegal to have both Properties and AdditionalProperties
Properties: map[string]apiextensions.JSONSchemaProps{
"field": {
Type: "integer",
},
},
AdditionalProperties: &apiextensions.JSONSchemaPropsOrBool{
Schema: &apiextensions.JSONSchemaProps{
Type: "string",
},
},
},
"b": {
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"field": {
Type: "integer",
},
},
XValidations: apiextensions.ValidationRules{
{Rule: "self.invalidFieldName > 50"},
},
},
},
},
},
expectedErrors: []validationMatch{
forbidden("spec.validation.openAPIV3Schema.properties[a].additionalProperties"),
invalid("spec.validation.openAPIV3Schema.properties[b].x-kubernetes-validations[0].rule"),
},
},
{
// So long at the schema information accessible to the CEL expression is valid, the expression should be validated.
name: "do not skip CEL expression validation when OpenAPIv3 schema is an invalid structural schema at level above",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"a": {
Type: "object",
// illegal to have both Properties and AdditionalProperties
Properties: map[string]apiextensions.JSONSchemaProps{
"b": {
Type: "integer",
XValidations: apiextensions.ValidationRules{
{Rule: "self == 'abc'"},
},
},
},
AdditionalProperties: &apiextensions.JSONSchemaPropsOrBool{
Schema: &apiextensions.JSONSchemaProps{
Type: "string",
},
},
},
},
},
},
expectedErrors: []validationMatch{
forbidden("spec.validation.openAPIV3Schema.properties[a].additionalProperties"),
invalid("spec.validation.openAPIV3Schema.properties[a].properties[b].x-kubernetes-validations[0].rule"),
},
},
{
name: "x-kubernetes-validations rule validated for escaped property name",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -8172,6 +8326,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "x-kubernetes-validations rule validated under array items",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -8196,6 +8351,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "x-kubernetes-validations rule validated under array items, parent has rule",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -8222,6 +8378,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "x-kubernetes-validations rule validated under additionalProperties",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -8246,6 +8403,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "x-kubernetes-validations rule validated under additionalProperties, parent has rule",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -8273,6 +8431,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "x-kubernetes-validations rule validated under unescaped property name",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -8292,6 +8451,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "x-kubernetes-validations rule validated under unescaped property name, parent has rule",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -8314,6 +8474,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "x-kubernetes-validations rule validated under escaped property name",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -8333,6 +8494,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "x-kubernetes-validations rule validated under escaped property name, parent has rule",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -8355,6 +8517,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "x-kubernetes-validations rule validated under unescapable property name",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -8374,6 +8537,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
},
{
name: "x-kubernetes-validations rule validated under unescapable property name, parent has rule",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
@ -8814,7 +8978,7 @@ func TestCelContext(t *testing.T) {
disallowDefaultsReason: opts.disallowDefaultsReason,
requireValidPropertyType: opts.requireValidPropertyType,
}
errors := ValidateCustomResourceDefinitionOpenAPISchema(tt.schema, field.NewPath("openAPIV3Schema"), openAPIV3Schema, true, &opts, celContext)
errors := ValidateCustomResourceDefinitionOpenAPISchema(tt.schema, field.NewPath("openAPIV3Schema"), openAPIV3Schema, true, &opts, celContext).AllErrors()
if len(errors) != 0 {
t.Errorf("Expected no validate errors but got %v", errors)
}