Skip to content

Commit

Permalink
Forbid CEL transition rules on unmergeable CRD subschemas.
Browse files Browse the repository at this point in the history
Transition rules (i.e. validation rules whose expressions reference
existing state) are not allowed on schemas, or the descendants of
schemas, that are unmergeable according to server-side apply
semantics. Today, this means that only objects with map-type
"granular" (or unspecified) and arrays with list-type "map" support
transition rules on their property/item subschemas.
  • Loading branch information
benluddy committed Mar 3, 2022
1 parent f757ab1 commit fedaa23
Show file tree
Hide file tree
Showing 5 changed files with 680 additions and 57 deletions.
Expand Up @@ -654,6 +654,10 @@ type specStandardValidator interface {
// insideResourceMeta returns true when validating either TypeMeta or ObjectMeta, from an embedded resource or on the top-level.
insideResourceMeta() bool
withInsideResourceMeta() specStandardValidator

// forbidOldSelfValidations returns the path to the first ancestor of the visited path that can't be safely correlated between two revisions of an object, or nil if there is no such path
forbidOldSelfValidations() *field.Path
withForbidOldSelfValidations(path *field.Path) specStandardValidator
}

// validateCustomResourceDefinitionValidation statically validates
Expand Down Expand Up @@ -776,9 +780,14 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch
for property, jsonSchema := range schema.Properties {
subSsv := ssv

// defensively assumes that a future map type is uncorrelatable
if schema.XMapType != nil && (*schema.XMapType != "granular" && *schema.XMapType != "atomic") {
subSsv = subSsv.withForbidOldSelfValidations(fldPath)
}

if (isRoot || schema.XEmbeddedResource) && metaFields.Has(property) {
// we recurse into the schema that applies to ObjectMeta.
subSsv = ssv.withInsideResourceMeta()
subSsv = subSsv.withInsideResourceMeta()
if isRoot {
subSsv = subSsv.withForbiddenDefaults(fmt.Sprintf("in top-level %s", property))
}
Expand Down Expand Up @@ -814,10 +823,19 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch
}

if schema.Items != nil {
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.Items.Schema, fldPath.Child("items"), ssv, false, opts)...)
subSsv := ssv

// we can only correlate old/new items for "map" and "set" lists, and correlation of
// "set" elements by identity is not supported for cel (x-kubernetes-validations)
// rules. an unset list type defaults to "atomic".
if schema.XListType == nil || *schema.XListType != "map" {
subSsv = subSsv.withForbidOldSelfValidations(fldPath)
}

allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(schema.Items.Schema, fldPath.Child("items"), subSsv, false, opts)...)
if len(schema.Items.JSONSchemas) != 0 {
for i, jsonSchema := range schema.Items.JSONSchemas {
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("items").Index(i), ssv, false, opts)...)
allErrs = append(allErrs, ValidateCustomResourceDefinitionOpenAPISchema(&jsonSchema, fldPath.Child("items").Index(i), subSsv, false, opts)...)
}
}
}
Expand Down Expand Up @@ -940,6 +958,15 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch
allErrs = append(allErrs, 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)))
} else {
// todo: remove when transition rule validation is implemented
allErrs = append(allErrs, field.Invalid(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), schema.XValidations[i].Rule, "validation of rules containing oldSelf is not yet implemented"))

}
}
}
}
}
Expand Down Expand Up @@ -1006,10 +1033,11 @@ func validateMapListKeysMapSet(schema *apiextensions.JSONSchemaProps, fldPath *f
}

type specStandardValidatorV3 struct {
allowDefaults bool
disallowDefaultsReason string
isInsideResourceMeta bool
requireValidPropertyType bool
allowDefaults bool
disallowDefaultsReason string
isInsideResourceMeta bool
requireValidPropertyType bool
uncorrelatableOldSelfValidationPath *field.Path
}

func (v *specStandardValidatorV3) withForbiddenDefaults(reason string) specStandardValidator {
Expand All @@ -1029,6 +1057,21 @@ func (v *specStandardValidatorV3) insideResourceMeta() bool {
return v.isInsideResourceMeta
}

func (v *specStandardValidatorV3) withForbidOldSelfValidations(path *field.Path) specStandardValidator {
if v.uncorrelatableOldSelfValidationPath != nil {
// oldSelf validations are already forbidden. preserve the highest-level path
// causing oldSelf validations to be forbidden
return v
}
clone := *v
clone.uncorrelatableOldSelfValidationPath = path
return &clone
}

func (v *specStandardValidatorV3) forbidOldSelfValidations() *field.Path {
return v.uncorrelatableOldSelfValidationPath
}

// validate validates against OpenAPI Schema v3.
func (v *specStandardValidatorV3) validate(schema *apiextensions.JSONSchemaProps, fldPath *field.Path) field.ErrorList {
allErrs := field.ErrorList{}
Expand Down

0 comments on commit fedaa23

Please sign in to comment.