diff --git a/api/api-rules/violation_exceptions.list b/api/api-rules/violation_exceptions.list index b69a9b2f89c..27ff722cdd0 100644 --- a/api/api-rules/violation_exceptions.list +++ b/api/api-rules/violation_exceptions.list @@ -419,6 +419,7 @@ API rule violation: names_match,k8s.io/apiextensions-apiserver/pkg/apis/apiexten API rule violation: names_match,k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1,JSONSchemaProps,XListType API rule violation: names_match,k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1,JSONSchemaProps,XMapType API rule violation: names_match,k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1,JSONSchemaProps,XPreserveUnknownFields +API rule violation: names_match,k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1,JSONSchemaProps,XValidations API rule violation: names_match,k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1,JSONSchemaPropsOrArray,JSONSchemas API rule violation: names_match,k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1,JSONSchemaPropsOrArray,Schema API rule violation: names_match,k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1,JSONSchemaPropsOrBool,Allows @@ -435,6 +436,7 @@ API rule violation: names_match,k8s.io/apiextensions-apiserver/pkg/apis/apiexten API rule violation: names_match,k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1,JSONSchemaProps,XListType API rule violation: names_match,k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1,JSONSchemaProps,XMapType API rule violation: names_match,k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1,JSONSchemaProps,XPreserveUnknownFields +API rule violation: names_match,k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1,JSONSchemaProps,XValidations API rule violation: names_match,k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1,JSONSchemaPropsOrArray,JSONSchemas API rule violation: names_match,k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1,JSONSchemaPropsOrArray,Schema API rule violation: names_match,k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1,JSONSchemaPropsOrBool,Allows diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types_jsonschema.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types_jsonschema.go index c0ac63e575d..f402c416d66 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types_jsonschema.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/types_jsonschema.go @@ -122,6 +122,80 @@ type JSONSchemaProps struct { // Atomic maps will be entirely replaced when updated. // +optional XMapType *string + + // x-kubernetes-validations -kubernetes-validations describes a list of validation rules written in the CEL expression language. + // This field is an alpha-level. Using this field requires the feature gate `CustomResourceValidationExpressions` to be enabled. + // +patchMergeKey=rule + // +patchStrategy=merge + // +listType=map + // +listMapKey=rule + XValidations ValidationRules +} + +// ValidationRules describes a list of validation rules written in the CEL expression language. +type ValidationRules []ValidationRule + +// ValidationRule describes a validation rule written in the CEL expression language. +type ValidationRule struct { + // Rule represents the expression which will be evaluated by CEL. + // ref: https://github.com/google/cel-spec + // The Rule is scoped to the location of the x-kubernetes-validations extension in the schema. + // The `self` variable in the CEL expression is bound to the scoped value. + // Example: + // - Rule scoped to the root of a resource with a status subresource: {"rule": "self.status.actual <= self.spec.maxDesired"} + // + // If the Rule is scoped to an object with properties, the accessible properties of the object are field selectable + // via `self.field` and field presence can be checked via `has(self.field)`. Null valued fields are treated as + // absent fields in CEL expressions. + // If the Rule is scoped to an object with additionalProperties (i.e. a map) the value of the map + // are accessible via `self[mapKey]`, map containment can be checked via `mapKey in self` and all entries of the map + // are accessible via CEL macros and functions such as `self.all(...)`. + // If the Rule is scoped to an array, the elements of the array are accessible via `self[i]` and also by macros and + // functions. + // If the Rule is scoped to a scalar, `self` is bound to the scalar value. + // Examples: + // - Rule scoped to a map of objects: {"rule": "self.components['Widget'].priority < 10"} + // - Rule scoped to a list of integers: {"rule": "self.values.all(value, value >= 0 && value < 100)"} + // - Rule scoped to a string value: {"rule": "self.startsWith('kube')"} + // + // The `apiVersion`, `kind`, `metadata.name` and `metadata.generateName` are always accessible from the root of the + // object and from any x-kubernetes-embedded-resource annotated objects. No other metadata properties are accessible. + // + // Unknown data preserved in custom resources via x-kubernetes-preserve-unknown-fields is not accessible in CEL + // expressions. This includes: + // - Unknown field values that are preserved by object schemas with x-kubernetes-preserve-unknown-fields. + // - Object properties where the property schema is of an "unknown type". An "unknown type" is recursively defined as: + // - A schema with no type and x-kubernetes-preserve-unknown-fields set to true + // - An array where the items schema is of an "unknown type" + // - An object where the additionalProperties schema is of an "unknown type" + // + // Only property names of the form `[a-zA-Z_.-/][a-zA-Z0-9_.-/]*` are accessible. + // Accessible property names are escaped according to the following rules when accessed in the expression: + // - '__' escapes to '__underscores__' + // - '.' escapes to '__dot__' + // - '-' escapes to '__dash__' + // - '/' escapes to '__slash__' + // - Property names that exactly match a CEL RESERVED keyword escape to '__{keyword}__'. The keywords are: + // "true", "false", "null", "in", "as", "break", "const", "continue", "else", "for", "function", "if", + // "import", "let", "loop", "package", "namespace", "return". + // Examples: + // - Rule accessing a property named "namespace": {"rule": "self.__namespace__ > 0"} + // - Rule accessing a property named "x-prop": {"rule": "self.x__dash__prop > 0"} + // - Rule accessing a property named "redact__d": {"rule": "self.redact__underscores__d > 0"} + // + // Equality on arrays with x-kubernetes-list-type of 'set' or 'map' ignores element order, i.e. [1, 2] == [2, 1]. + // Concatenation on arrays with x-kubernetes-list-type use the semantics of the list type: + // - 'set': `X + Y` performs a union where the array positions of all elements in `X` are preserved and + // non-intersecting elements in `Y` are appended, retaining their partial order. + // - 'map': `X + Y` performs a merge where the array positions of all keys in `X` are preserved but the values + // are overwritten by values in `Y` when the key sets of `X` and `Y` intersect. Elements in `Y` with + // non-intersecting keys are appended, retaining their partial order. + Rule string + // Message represents the message displayed when validation fails. The message is required if the Rule contains + // line breaks. The message must not contain line breaks. + // If unset, the message is "failed rule: {Rule}". + // e.g. "must be a URL with the host matching spec.host" + Message string } // JSON represents any valid JSON value. diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/types_jsonschema.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/types_jsonschema.go index 4433e2446f7..277fd7a124a 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/types_jsonschema.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1/types_jsonschema.go @@ -161,6 +161,80 @@ type JSONSchemaProps struct { // Atomic maps will be entirely replaced when updated. // +optional XMapType *string `json:"x-kubernetes-map-type,omitempty" protobuf:"bytes,43,opt,name=xKubernetesMapType"` + + // x-kubernetes-validations describes a list of validation rules written in the CEL expression language. + // This field is an alpha-level. Using this field requires the feature gate `CustomResourceValidationExpressions` to be enabled. + // +patchMergeKey=rule + // +patchStrategy=merge + // +listType=map + // +listMapKey=rule + XValidations ValidationRules `json:"x-kubernetes-validations,omitempty" patchStrategy:"merge" patchMergeKey:"rule" protobuf:"bytes,44,rep,name=xKubernetesValidations"` +} + +// ValidationRules describes a list of validation rules written in the CEL expression language. +type ValidationRules []ValidationRule + +// ValidationRule describes a validation rule written in the CEL expression language. +type ValidationRule struct { + // Rule represents the expression which will be evaluated by CEL. + // ref: https://github.com/google/cel-spec + // The Rule is scoped to the location of the x-kubernetes-validations extension in the schema. + // The `self` variable in the CEL expression is bound to the scoped value. + // Example: + // - Rule scoped to the root of a resource with a status subresource: {"rule": "self.status.actual <= self.spec.maxDesired"} + // + // If the Rule is scoped to an object with properties, the accessible properties of the object are field selectable + // via `self.field` and field presence can be checked via `has(self.field)`. Null valued fields are treated as + // absent fields in CEL expressions. + // If the Rule is scoped to an object with additionalProperties (i.e. a map) the value of the map + // are accessible via `self[mapKey]`, map containment can be checked via `mapKey in self` and all entries of the map + // are accessible via CEL macros and functions such as `self.all(...)`. + // If the Rule is scoped to an array, the elements of the array are accessible via `self[i]` and also by macros and + // functions. + // If the Rule is scoped to a scalar, `self` is bound to the scalar value. + // Examples: + // - Rule scoped to a map of objects: {"rule": "self.components['Widget'].priority < 10"} + // - Rule scoped to a list of integers: {"rule": "self.values.all(value, value >= 0 && value < 100)"} + // - Rule scoped to a string value: {"rule": "self.startsWith('kube')"} + // + // The `apiVersion`, `kind`, `metadata.name` and `metadata.generateName` are always accessible from the root of the + // object and from any x-kubernetes-embedded-resource annotated objects. No other metadata properties are accessible. + // + // Unknown data preserved in custom resources via x-kubernetes-preserve-unknown-fields is not accessible in CEL + // expressions. This includes: + // - Unknown field values that are preserved by object schemas with x-kubernetes-preserve-unknown-fields. + // - Object properties where the property schema is of an "unknown type". An "unknown type" is recursively defined as: + // - A schema with no type and x-kubernetes-preserve-unknown-fields set to true + // - An array where the items schema is of an "unknown type" + // - An object where the additionalProperties schema is of an "unknown type" + // + // Only property names of the form `[a-zA-Z_.-/][a-zA-Z0-9_.-/]*` are accessible. + // Accessible property names are escaped according to the following rules when accessed in the expression: + // - '__' escapes to '__underscores__' + // - '.' escapes to '__dot__' + // - '-' escapes to '__dash__' + // - '/' escapes to '__slash__' + // - Property names that exactly match a CEL RESERVED keyword escape to '__{keyword}__'. The keywords are: + // "true", "false", "null", "in", "as", "break", "const", "continue", "else", "for", "function", "if", + // "import", "let", "loop", "package", "namespace", "return". + // Examples: + // - Rule accessing a property named "namespace": {"rule": "self.__namespace__ > 0"} + // - Rule accessing a property named "x-prop": {"rule": "self.x__dash__prop > 0"} + // - Rule accessing a property named "redact__d": {"rule": "self.redact__underscores__d > 0"} + // + // Equality on arrays with x-kubernetes-list-type of 'set' or 'map' ignores element order, i.e. [1, 2] == [2, 1]. + // Concatenation on arrays with x-kubernetes-list-type use the semantics of the list type: + // - 'set': `X + Y` performs a union where the array positions of all elements in `X` are preserved and + // non-intersecting elements in `Y` are appended, retaining their partial order. + // - 'map': `X + Y` performs a merge where the array positions of all keys in `X` are preserved but the values + // are overwritten by values in `Y` when the key sets of `X` and `Y` intersect. Elements in `Y` with + // non-intersecting keys are appended, retaining their partial order. + Rule string `json:"rule" protobuf:"bytes,1,opt,name=rule"` + // Message represents the message displayed when validation fails. The message is required if the Rule contains + // line breaks. The message must not contain line breaks. + // If unset, the message is "failed rule: {Rule}". + // e.g. "must be a URL with the host matching spec.host" + Message string `json:"message,omitempty" protobuf:"bytes,2,opt,name=message"` } // JSON represents any valid JSON value. diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types_jsonschema.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types_jsonschema.go index 1837723a080..c9d943c9a8a 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types_jsonschema.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1/types_jsonschema.go @@ -161,6 +161,80 @@ type JSONSchemaProps struct { // Atomic maps will be entirely replaced when updated. // +optional XMapType *string `json:"x-kubernetes-map-type,omitempty" protobuf:"bytes,43,opt,name=xKubernetesMapType"` + + // x-kubernetes-validations describes a list of validation rules written in the CEL expression language. + // This field is an alpha-level. Using this field requires the feature gate `CustomResourceValidationExpressions` to be enabled. + // +patchMergeKey=rule + // +patchStrategy=merge + // +listType=map + // +listMapKey=rule + XValidations ValidationRules `json:"x-kubernetes-validations,omitempty" patchStrategy:"merge" patchMergeKey:"rule" protobuf:"bytes,44,rep,name=xKubernetesValidations"` +} + +// ValidationRules describes a list of validation rules written in the CEL expression language. +type ValidationRules []ValidationRule + +// ValidationRule describes a validation rule written in the CEL expression language. +type ValidationRule struct { + // Rule represents the expression which will be evaluated by CEL. + // ref: https://github.com/google/cel-spec + // The Rule is scoped to the location of the x-kubernetes-validations extension in the schema. + // The `self` variable in the CEL expression is bound to the scoped value. + // Example: + // - Rule scoped to the root of a resource with a status subresource: {"rule": "self.status.actual <= self.spec.maxDesired"} + // + // If the Rule is scoped to an object with properties, the accessible properties of the object are field selectable + // via `self.field` and field presence can be checked via `has(self.field)`. Null valued fields are treated as + // absent fields in CEL expressions. + // If the Rule is scoped to an object with additionalProperties (i.e. a map) the value of the map + // are accessible via `self[mapKey]`, map containment can be checked via `mapKey in self` and all entries of the map + // are accessible via CEL macros and functions such as `self.all(...)`. + // If the Rule is scoped to an array, the elements of the array are accessible via `self[i]` and also by macros and + // functions. + // If the Rule is scoped to a scalar, `self` is bound to the scalar value. + // Examples: + // - Rule scoped to a map of objects: {"rule": "self.components['Widget'].priority < 10"} + // - Rule scoped to a list of integers: {"rule": "self.values.all(value, value >= 0 && value < 100)"} + // - Rule scoped to a string value: {"rule": "self.startsWith('kube')"} + // + // The `apiVersion`, `kind`, `metadata.name` and `metadata.generateName` are always accessible from the root of the + // object and from any x-kubernetes-embedded-resource annotated objects. No other metadata properties are accessible. + // + // Unknown data preserved in custom resources via x-kubernetes-preserve-unknown-fields is not accessible in CEL + // expressions. This includes: + // - Unknown field values that are preserved by object schemas with x-kubernetes-preserve-unknown-fields. + // - Object properties where the property schema is of an "unknown type". An "unknown type" is recursively defined as: + // - A schema with no type and x-kubernetes-preserve-unknown-fields set to true + // - An array where the items schema is of an "unknown type" + // - An object where the additionalProperties schema is of an "unknown type" + // + // Only property names of the form `[a-zA-Z_.-/][a-zA-Z0-9_.-/]*` are accessible. + // Accessible property names are escaped according to the following rules when accessed in the expression: + // - '__' escapes to '__underscores__' + // - '.' escapes to '__dot__' + // - '-' escapes to '__dash__' + // - '/' escapes to '__slash__' + // - Property names that exactly match a CEL RESERVED keyword escape to '__{keyword}__'. The keywords are: + // "true", "false", "null", "in", "as", "break", "const", "continue", "else", "for", "function", "if", + // "import", "let", "loop", "package", "namespace", "return". + // Examples: + // - Rule accessing a property named "namespace": {"rule": "self.__namespace__ > 0"} + // - Rule accessing a property named "x-prop": {"rule": "self.x__dash__prop > 0"} + // - Rule accessing a property named "redact__d": {"rule": "self.redact__underscores__d > 0"} + // + // Equality on arrays with x-kubernetes-list-type of 'set' or 'map' ignores element order, i.e. [1, 2] == [2, 1]. + // Concatenation on arrays with x-kubernetes-list-type use the semantics of the list type: + // - 'set': `X + Y` performs a union where the array positions of all elements in `X` are preserved and + // non-intersecting elements in `Y` are appended, retaining their partial order. + // - 'map': `X + Y` performs a merge where the array positions of all keys in `X` are preserved but the values + // are overwritten by values in `Y` when the key sets of `X` and `Y` intersect. Elements in `Y` with + // non-intersecting keys are appended, retaining their partial order. + Rule string `json:"rule" protobuf:"bytes,1,opt,name=rule"` + // Message represents the message displayed when validation fails. The message is required if the Rule contains + // line breaks. The message must not contain line breaks. + // If unset, the message is "failed rule: {Rule}". + // e.g. "must be a URL with the host matching spec.host" + Message string `json:"message,omitempty" protobuf:"bytes,2,opt,name=message"` } // JSON represents any valid JSON value. diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go index c2ce09e627a..30732d500c4 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation.go @@ -19,11 +19,13 @@ package validation import ( "fmt" "reflect" + "regexp" "strings" "unicode" "unicode/utf8" "k8s.io/apiextensions-apiserver/pkg/apihelpers" + "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel" structuraldefaulting "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/defaulting" apiequality "k8s.io/apimachinery/pkg/api/equality" genericvalidation "k8s.io/apimachinery/pkg/api/validation" @@ -909,6 +911,40 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch } } + 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")) + } 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")) + } 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")) + } 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")) + } + } + + structural, err := structuralschema.NewStructural(schema) + if err == nil { + compResults, err := cel.Compile(structural, isRoot) + if err != nil { + allErrs = append(allErrs, field.InternalError(fldPath.Child("x-kubernetes-validations"), err)) + } else { + for i, cr := range compResults { + 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)) + } else { + allErrs = append(allErrs, field.Invalid(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), schema.XValidations[i], cr.Error.Detail)) + } + } + } + } + } + } + if opts.requireMapListKeysMapSetValidation { allErrs = append(allErrs, validateMapListKeysMapSet(schema, fldPath)...) } @@ -916,6 +952,11 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch return allErrs } +var newlineMatcher = regexp.MustCompile(`[\n\r]+`) // valid newline chars in CEL grammar +func hasNewlines(s string) bool { + return newlineMatcher.MatchString(s) +} + func validateMapListKeysMapSet(schema *apiextensions.JSONSchemaProps, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} @@ -1112,7 +1153,7 @@ func validateSimpleJSONPath(s string, fldPath *field.Path) field.ErrorList { return allErrs } -var allowedFieldsAtRootSchema = []string{"Description", "Type", "Format", "Title", "Maximum", "ExclusiveMaximum", "Minimum", "ExclusiveMinimum", "MaxLength", "MinLength", "Pattern", "MaxItems", "MinItems", "UniqueItems", "MultipleOf", "Required", "Items", "Properties", "ExternalDocs", "Example", "XPreserveUnknownFields"} +var allowedFieldsAtRootSchema = []string{"Description", "Type", "Format", "Title", "Maximum", "ExclusiveMaximum", "Minimum", "ExclusiveMinimum", "MaxLength", "MinLength", "Pattern", "MaxItems", "MinItems", "UniqueItems", "MultipleOf", "Required", "Items", "Properties", "ExternalDocs", "Example", "XPreserveUnknownFields", "XValidations"} func allowedAtRootSchema(field string) bool { for _, v := range allowedFieldsAtRootSchema { @@ -1144,16 +1185,16 @@ func allVersionsSpecifyOpenAPISchema(spec *apiextensions.CustomResourceDefinitio } func specHasDefaults(spec *apiextensions.CustomResourceDefinitionSpec) bool { - return hasSchemaWith(spec, schemaHasDefaults) + return HasSchemaWith(spec, schemaHasDefaults) } func schemaHasDefaults(s *apiextensions.JSONSchemaProps) bool { - return schemaHas(s, func(s *apiextensions.JSONSchemaProps) bool { + return SchemaHas(s, func(s *apiextensions.JSONSchemaProps) bool { return s.Default != nil }) } -func hasSchemaWith(spec *apiextensions.CustomResourceDefinitionSpec, pred func(s *apiextensions.JSONSchemaProps) bool) bool { +func HasSchemaWith(spec *apiextensions.CustomResourceDefinitionSpec, pred func(s *apiextensions.JSONSchemaProps) bool) bool { if spec.Validation != nil && spec.Validation.OpenAPIV3Schema != nil && pred(spec.Validation.OpenAPIV3Schema) { return true } @@ -1165,7 +1206,7 @@ func hasSchemaWith(spec *apiextensions.CustomResourceDefinitionSpec, pred func(s return false } -func schemaHas(s *apiextensions.JSONSchemaProps, pred func(s *apiextensions.JSONSchemaProps) bool) bool { +func SchemaHas(s *apiextensions.JSONSchemaProps, pred func(s *apiextensions.JSONSchemaProps) bool) bool { if s == nil { return false } @@ -1175,60 +1216,60 @@ func schemaHas(s *apiextensions.JSONSchemaProps, pred func(s *apiextensions.JSON } if s.Items != nil { - if s.Items != nil && schemaHas(s.Items.Schema, pred) { + if s.Items != nil && SchemaHas(s.Items.Schema, pred) { return true } for _, s := range s.Items.JSONSchemas { - if schemaHas(&s, pred) { + if SchemaHas(&s, pred) { return true } } } for _, s := range s.AllOf { - if schemaHas(&s, pred) { + if SchemaHas(&s, pred) { return true } } for _, s := range s.AnyOf { - if schemaHas(&s, pred) { + if SchemaHas(&s, pred) { return true } } for _, s := range s.OneOf { - if schemaHas(&s, pred) { + if SchemaHas(&s, pred) { return true } } - if schemaHas(s.Not, pred) { + if SchemaHas(s.Not, pred) { return true } for _, s := range s.Properties { - if schemaHas(&s, pred) { + if SchemaHas(&s, pred) { return true } } if s.AdditionalProperties != nil { - if schemaHas(s.AdditionalProperties.Schema, pred) { + if SchemaHas(s.AdditionalProperties.Schema, pred) { return true } } for _, s := range s.PatternProperties { - if schemaHas(&s, pred) { + if SchemaHas(&s, pred) { return true } } if s.AdditionalItems != nil { - if schemaHas(s.AdditionalItems.Schema, pred) { + if SchemaHas(s.AdditionalItems.Schema, pred) { return true } } for _, s := range s.Definitions { - if schemaHas(&s, pred) { + if SchemaHas(&s, pred) { return true } } for _, d := range s.Dependencies { - if schemaHas(d.Schema, pred) { + if SchemaHas(d.Schema, pred) { return true } } @@ -1249,8 +1290,8 @@ func specHasKubernetesExtensions(spec *apiextensions.CustomResourceDefinitionSpe } func schemaHasKubernetesExtensions(s *apiextensions.JSONSchemaProps) bool { - return schemaHas(s, func(s *apiextensions.JSONSchemaProps) bool { - return s.XEmbeddedResource || s.XPreserveUnknownFields != nil || s.XIntOrString || len(s.XListMapKeys) > 0 || s.XListType != nil + return SchemaHas(s, func(s *apiextensions.JSONSchemaProps) bool { + return s.XEmbeddedResource || s.XPreserveUnknownFields != nil || s.XIntOrString || len(s.XListMapKeys) > 0 || s.XListType != nil || len(s.XValidations) > 0 }) } @@ -1322,12 +1363,12 @@ func schemaHasUnprunedDefaults(schema *apiextensions.JSONSchemaProps) (bool, err // requireAtomicSetType returns true if the old CRD spec as at least one x-kubernetes-list-type=set with non-atomic items type. func requireAtomicSetType(oldCRDSpec *apiextensions.CustomResourceDefinitionSpec) bool { - return !hasSchemaWith(oldCRDSpec, hasNonAtomicSetType) + return !HasSchemaWith(oldCRDSpec, hasNonAtomicSetType) } // hasNonAtomicSetType recurses over the schema and returns whether any list of type "set" as non-atomic item types. func hasNonAtomicSetType(schema *apiextensions.JSONSchemaProps) bool { - return schemaHas(schema, func(schema *apiextensions.JSONSchemaProps) bool { + return SchemaHas(schema, func(schema *apiextensions.JSONSchemaProps) bool { if schema.XListType != nil && *schema.XListType == "set" && schema.Items != nil && schema.Items.Schema != nil { // we don't support schema.Items.JSONSchemas is := schema.Items.Schema switch is.Type { @@ -1344,11 +1385,11 @@ func hasNonAtomicSetType(schema *apiextensions.JSONSchemaProps) bool { } func requireMapListKeysMapSetValidation(oldCRDSpec *apiextensions.CustomResourceDefinitionSpec) bool { - return !hasSchemaWith(oldCRDSpec, hasInvalidMapListKeysMapSet) + return !HasSchemaWith(oldCRDSpec, hasInvalidMapListKeysMapSet) } func hasInvalidMapListKeysMapSet(schema *apiextensions.JSONSchemaProps) bool { - return schemaHas(schema, func(schema *apiextensions.JSONSchemaProps) bool { + return SchemaHas(schema, func(schema *apiextensions.JSONSchemaProps) bool { return len(validateMapListKeysMapSet(schema, field.NewPath(""))) > 0 }) } @@ -1426,7 +1467,7 @@ func specHasInvalidTypes(spec *apiextensions.CustomResourceDefinitionSpec) bool // SchemaHasInvalidTypes returns true if it contains invalid offending openapi-v3 specification. func SchemaHasInvalidTypes(s *apiextensions.JSONSchemaProps) bool { - return schemaHas(s, func(s *apiextensions.JSONSchemaProps) bool { + return SchemaHas(s, func(s *apiextensions.JSONSchemaProps) bool { return len(s.Type) > 0 && !openapiV3Types.Has(s.Type) }) } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go index 538e24f9099..5f2ee8c7313 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go @@ -1767,6 +1767,51 @@ func TestValidateCustomResourceDefinition(t *testing.T) { forbidden("spec", "validation", "openAPIV3Schema", "properties[embedded]", "properties[kind]", "properties[foo]", "x-kubernetes-embedded-resource"), }, }, + { + name: "x-kubernetes-validations access metadata name", + resource: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Group: "group.com", + Versions: singleVersionList, + Subresources: &apiextensions.CustomResourceSubresources{ + Status: &apiextensions.CustomResourceSubresourceStatus{}, + }, + Scope: apiextensions.NamespaceScoped, + Names: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "Plural", + ListKind: "PluralList", + }, + PreserveUnknownFields: pointer.BoolPtr(false), + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + XPreserveUnknownFields: pointer.BoolPtr(true), + XValidations: apiextensions.ValidationRules{ + { + Rule: "size(self.metadata.name) > 3", + }, + }, + Properties: map[string]apiextensions.JSONSchemaProps{ + "metadata": { + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "name": { + Type: "string", + }, + }, + }, + }, + }, + }, + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + }, { name: "defaults with enabled feature gate, unstructural schema", resource: &apiextensions.CustomResourceDefinition{ @@ -3939,6 +3984,78 @@ func TestValidateCustomResourceDefinition(t *testing.T) { invalid("spec", "validation", "openAPIV3Schema", "properties[nested]", "properties[invalid]", "properties[metadata]", "type"), }, }, + { + name: "x-kubernetes-validations should be forbidden under oneOf/anyOf/allOf/not, structural schema", + resource: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "plural.group.com"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Group: "group.com", + Version: "version", + Versions: singleVersionList, + Scope: apiextensions.NamespaceScoped, + Names: apiextensions.CustomResourceDefinitionNames{ + Plural: "plural", + Singular: "singular", + Kind: "Plural", + ListKind: "PluralList", + }, + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "a": { + Type: "number", + Not: &apiextensions.JSONSchemaProps{ + XValidations: apiextensions.ValidationRules{ + { + Rule: "should be forbidden", + }, + }, + }, + AnyOf: []apiextensions.JSONSchemaProps{ + { + XValidations: apiextensions.ValidationRules{ + { + Rule: "should be forbidden", + }, + }, + }, + }, + AllOf: []apiextensions.JSONSchemaProps{ + { + XValidations: apiextensions.ValidationRules{ + { + Rule: "should be forbidden", + }, + }, + }, + }, + OneOf: []apiextensions.JSONSchemaProps{ + { + XValidations: apiextensions.ValidationRules{ + { + Rule: "should be forbidden", + }, + }, + }, + }, + }, + }, + }, + }, + PreserveUnknownFields: pointer.BoolPtr(false), + }, + Status: apiextensions.CustomResourceDefinitionStatus{ + StoredVersions: []string{"version"}, + }, + }, + errors: []validationMatch{ + forbidden("spec", "validation", "openAPIV3Schema", "properties[a]", "not", "x-kubernetes-validations"), + forbidden("spec", "validation", "openAPIV3Schema", "properties[a]", "allOf[0]", "x-kubernetes-validations"), + forbidden("spec", "validation", "openAPIV3Schema", "properties[a]", "anyOf[0]", "x-kubernetes-validations"), + forbidden("spec", "validation", "openAPIV3Schema", "properties[a]", "oneOf[0]", "x-kubernetes-validations"), + }, + }, } for _, tc := range tests { @@ -6104,22 +6221,20 @@ func TestValidateCustomResourceDefinitionUpdate(t *testing.T) { func TestValidateCustomResourceDefinitionValidation(t *testing.T) { tests := []struct { - name string - input apiextensions.CustomResourceValidation - statusEnabled bool - opts validationOptions - wantError bool + name string + input apiextensions.CustomResourceValidation + statusEnabled bool + opts validationOptions + expectedErrors []validationMatch }{ { - name: "empty", - input: apiextensions.CustomResourceValidation{}, - wantError: false, + name: "empty", + input: apiextensions.CustomResourceValidation{}, }, { name: "empty with status", input: apiextensions.CustomResourceValidation{}, statusEnabled: true, - wantError: false, }, { name: "root type without status", @@ -6129,7 +6244,6 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { }, }, statusEnabled: false, - wantError: false, }, { name: "root type having invalid value, with status", @@ -6139,7 +6253,9 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { }, }, statusEnabled: true, - wantError: true, + expectedErrors: []validationMatch{ + invalid("spec.validation.openAPIV3Schema.type"), + }, }, { name: "non-allowed root field with status", @@ -6156,7 +6272,9 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { }, }, statusEnabled: true, - wantError: true, + expectedErrors: []validationMatch{ + invalid("spec.validation.openAPIV3Schema"), + }, }, { name: "all allowed fields at the root of the schema with status", @@ -6164,7 +6282,6 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { OpenAPIV3Schema: validValidationSchema, }, statusEnabled: true, - wantError: false, }, { name: "null type", @@ -6177,7 +6294,9 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { }, }, }, - wantError: true, + expectedErrors: []validationMatch{ + forbidden("spec.validation.openAPIV3Schema.properties[null].type"), + }, }, { name: "nullable at the root", @@ -6187,7 +6306,9 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { Nullable: true, }, }, - wantError: true, + expectedErrors: []validationMatch{ + forbidden("spec.validation.openAPIV3Schema.nullable"), + }, }, { name: "nullable without type", @@ -6200,7 +6321,6 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { }, }, }, - wantError: false, }, { name: "nullable with types", @@ -6226,15 +6346,16 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { }, }, }, - wantError: false, }, { name: "must be structural, but isn't", input: apiextensions.CustomResourceValidation{ OpenAPIV3Schema: &apiextensions.JSONSchemaProps{}, }, - opts: validationOptions{requireStructuralSchema: true}, - wantError: true, + opts: validationOptions{requireStructuralSchema: true}, + expectedErrors: []validationMatch{ + required("spec.validation.openAPIV3Schema.type"), + }, }, { name: "must be structural", @@ -6243,8 +6364,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { Type: "object", }, }, - opts: validationOptions{requireStructuralSchema: true}, - wantError: false, + opts: validationOptions{requireStructuralSchema: true}, }, { name: "require valid types, valid", @@ -6253,8 +6373,7 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { Type: "object", }, }, - opts: validationOptions{requireValidPropertyType: true, requireStructuralSchema: true}, - wantError: false, + opts: validationOptions{requireValidPropertyType: true, requireStructuralSchema: true}, }, { name: "require valid types, invalid", @@ -6263,8 +6382,15 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { Type: "null", }, }, - opts: validationOptions{requireValidPropertyType: true, requireStructuralSchema: true}, - wantError: true, + opts: validationOptions{requireValidPropertyType: true, requireStructuralSchema: true}, + expectedErrors: []validationMatch{ + // Invalid value: "null": must be object at the root + unsupported("spec.validation.openAPIV3Schema.type"), + // Forbidden: type cannot be set to null, use nullable as an alternative + forbidden("spec.validation.openAPIV3Schema.type"), + // Unsupported value: "null": supported values: "array", "boolean", "integer", "number", "object", "string" + invalid("spec.validation.openAPIV3Schema.type"), + }, }, { name: "require valid types, invalid", @@ -6273,8 +6399,11 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { Type: "bogus", }, }, - opts: validationOptions{requireValidPropertyType: true, requireStructuralSchema: true}, - wantError: true, + opts: validationOptions{requireValidPropertyType: true, requireStructuralSchema: true}, + expectedErrors: []validationMatch{ + unsupported("spec.validation.openAPIV3Schema.type"), + invalid("spec.validation.openAPIV3Schema.type"), + }, }, { name: "invalid type with list type extension set", @@ -6284,7 +6413,9 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { XListType: strPtr("set"), }, }, - wantError: true, + expectedErrors: []validationMatch{ + invalid("spec.validation.openAPIV3Schema.type"), + }, }, { name: "unset type with list type extension set", @@ -6293,7 +6424,9 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { XListType: strPtr("set"), }, }, - wantError: true, + expectedErrors: []validationMatch{ + required("spec.validation.openAPIV3Schema.type"), + }, }, { name: "invalid list type extension", @@ -6303,7 +6436,9 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { XListType: strPtr("invalid"), }, }, - wantError: true, + expectedErrors: []validationMatch{ + unsupported("spec.validation.openAPIV3Schema.x-kubernetes-list-type"), + }, }, { name: "invalid list type extension with list map keys extension non-empty", @@ -6314,7 +6449,9 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { XListMapKeys: []string{"key"}, }, }, - wantError: true, + expectedErrors: []validationMatch{ + invalid("spec.validation.openAPIV3Schema.x-kubernetes-list-type"), + }, }, { name: "unset list type extension with list map keys extension non-empty", @@ -6323,7 +6460,9 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { XListMapKeys: []string{"key"}, }, }, - wantError: true, + expectedErrors: []validationMatch{ + required("spec.validation.openAPIV3Schema.x-kubernetes-list-type"), + }, }, { name: "empty list map keys extension with list type extension map", @@ -6333,7 +6472,10 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { XListType: strPtr("map"), }, }, - wantError: true, + expectedErrors: []validationMatch{ + required("spec.validation.openAPIV3Schema.x-kubernetes-list-map-keys"), + required("spec.validation.openAPIV3Schema.items"), + }, }, { name: "no items schema with list type extension map", @@ -6344,7 +6486,9 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { XListMapKeys: []string{"key"}, }, }, - wantError: true, + expectedErrors: []validationMatch{ + required("spec.validation.openAPIV3Schema.items"), + }, }, { name: "multiple schema items with list type extension map", @@ -6364,7 +6508,10 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { }, }, }, - wantError: true, + expectedErrors: []validationMatch{ + forbidden("spec.validation.openAPIV3Schema.items"), + invalid("spec.validation.openAPIV3Schema.items"), + }, }, { name: "non object item with list type extension map", @@ -6380,7 +6527,9 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { }, }, }, - wantError: true, + expectedErrors: []validationMatch{ + invalid("spec.validation.openAPIV3Schema.items.type"), + }, }, { name: "items with key missing from properties with list type extension map", @@ -6396,7 +6545,9 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { }, }, }, - wantError: true, + expectedErrors: []validationMatch{ + invalid("spec.validation.openAPIV3Schema.x-kubernetes-list-map-keys"), + }, }, { name: "items with non scalar key property type with list type extension map", @@ -6417,7 +6568,9 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { }, }, }, - wantError: true, + expectedErrors: []validationMatch{ + invalid("spec.validation.openAPIV3Schema.items.properties[key].type"), + }, }, { name: "duplicate map keys with list type extension map", @@ -6438,7 +6591,9 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { }, }, }, - wantError: true, + expectedErrors: []validationMatch{ + invalid("spec.validation.openAPIV3Schema.x-kubernetes-list-map-keys"), + }, }, { name: "allowed schema with list type extension map", @@ -6462,7 +6617,6 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { }, }, }, - wantError: false, }, { name: "allowed list-type atomic", @@ -6477,7 +6631,6 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { }, }, }, - wantError: false, }, { name: "allowed list-type atomic with non-atomic items", @@ -6493,7 +6646,6 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { }, }, }, - wantError: false, }, { name: "allowed list-type set with scalar items", @@ -6508,7 +6660,6 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { }, }, }, - wantError: false, }, { name: "allowed list-type set with atomic map items", @@ -6527,7 +6678,6 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { }, }, }, - wantError: false, }, { name: "invalid list-type set with non-atomic map items", @@ -6546,8 +6696,10 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { }, }, }, - opts: validationOptions{requireAtomicSetType: true}, - wantError: true, + opts: validationOptions{requireAtomicSetType: true}, + expectedErrors: []validationMatch{ + invalid("spec.validation.openAPIV3Schema.items.x-kubernetes-map-type"), + }, }, { name: "invalid list-type set with unspecified map-type for map items", @@ -6565,8 +6717,10 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { }, }, }, - opts: validationOptions{requireAtomicSetType: true}, - wantError: true, + opts: validationOptions{requireAtomicSetType: true}, + expectedErrors: []validationMatch{ + invalid("spec.validation.openAPIV3Schema.items.x-kubernetes-map-type"), + }, }, { name: "allowed list-type set with atomic list items", @@ -6587,7 +6741,6 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { }, }, }, - wantError: false, }, { name: "allowed list-type set with unspecified list-type in list items", @@ -6607,7 +6760,6 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { }, }, }, - wantError: false, }, { name: "invalid list-type set with with non-atomic list items", @@ -6628,8 +6780,10 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { }, }, }, - opts: validationOptions{requireAtomicSetType: true}, - wantError: true, + opts: validationOptions{requireAtomicSetType: true}, + expectedErrors: []validationMatch{ + invalid("spec.validation.openAPIV3Schema.items.x-kubernetes-list-type"), + }, }, { name: "invalid type with map type extension (granular)", @@ -6639,7 +6793,9 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { XMapType: strPtr("granular"), }, }, - wantError: true, + expectedErrors: []validationMatch{ + invalid("spec.validation.openAPIV3Schema.type"), + }, }, { name: "unset type with map type extension (granular)", @@ -6648,7 +6804,9 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { XMapType: strPtr("granular"), }, }, - wantError: true, + expectedErrors: []validationMatch{ + required("spec.validation.openAPIV3Schema.type"), + }, }, { name: "invalid type with map type extension (atomic)", @@ -6658,7 +6816,9 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { XMapType: strPtr("atomic"), }, }, - wantError: true, + expectedErrors: []validationMatch{ + invalid("spec.validation.openAPIV3Schema.type"), + }, }, { name: "unset type with map type extension (atomic)", @@ -6667,7 +6827,9 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { XMapType: strPtr("atomic"), }, }, - wantError: true, + expectedErrors: []validationMatch{ + required("spec.validation.openAPIV3Schema.type"), + }, }, { name: "invalid map type", @@ -6677,7 +6839,9 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { XMapType: strPtr("badMapType"), }, }, - wantError: true, + expectedErrors: []validationMatch{ + unsupported("spec.validation.openAPIV3Schema.x-kubernetes-map-type"), + }, }, { name: "allowed type with map type extension (granular)", @@ -6687,7 +6851,6 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { XMapType: strPtr("granular"), }, }, - wantError: false, }, { name: "allowed type with map type extension (atomic)", @@ -6697,7 +6860,6 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { XMapType: strPtr("atomic"), }, }, - wantError: false, }, { name: "invalid map with non-required key and no default", @@ -6721,7 +6883,9 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { opts: validationOptions{ requireMapListKeysMapSetValidation: true, }, - wantError: true, + expectedErrors: []validationMatch{ + required("spec.validation.openAPIV3Schema.items.properties[key].default"), + }, }, { name: "allowed map with required key and no default", @@ -6746,7 +6910,6 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { opts: validationOptions{ requireMapListKeysMapSetValidation: true, }, - wantError: false, }, { name: "allowed map with non-required key and default", @@ -6772,7 +6935,6 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { allowDefaults: true, requireMapListKeysMapSetValidation: true, }, - wantError: false, }, { name: "invalid map with nullable key", @@ -6797,7 +6959,10 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { opts: validationOptions{ requireMapListKeysMapSetValidation: true, }, - wantError: true, + expectedErrors: []validationMatch{ + required("spec.validation.openAPIV3Schema.items.properties[key].default"), + forbidden("spec.validation.openAPIV3Schema.items.properties[key].nullable"), + }, }, { name: "invalid map with nullable items", @@ -6822,7 +6987,10 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { opts: validationOptions{ requireMapListKeysMapSetValidation: true, }, - wantError: true, + expectedErrors: []validationMatch{ + forbidden("spec.validation.openAPIV3Schema.items.nullable"), + required("spec.validation.openAPIV3Schema.items.properties[key].default"), + }, }, { name: "valid map with some required, some defaulted, and non-key fields", @@ -6857,7 +7025,9 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { opts: validationOptions{ requireMapListKeysMapSetValidation: true, }, - wantError: true, + expectedErrors: []validationMatch{ + forbidden("spec.validation.openAPIV3Schema.items.properties[b].default"), + }, }, { name: "invalid set with nullable items", @@ -6875,7 +7045,9 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { opts: validationOptions{ requireMapListKeysMapSetValidation: true, }, - wantError: true, + expectedErrors: []validationMatch{ + forbidden("spec.validation.openAPIV3Schema.items.nullable"), + }, }, { name: "allowed set with non-nullable items", @@ -6893,16 +7065,555 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) { opts: validationOptions{ requireMapListKeysMapSetValidation: true, }, - wantError: false, + }, + { + name: "valid x-kubernetes-validations for scalar element", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "subRoot": { + Type: "string", + XValidations: apiextensions.ValidationRules{ + { + Rule: "self.startsWith('s')", + Message: "subRoot should start with 's'.", + }, + { + Rule: "self.endsWith('s')", + Message: "subRoot should end with 's'.", + }, + }, + }, + }, + }, + }, + opts: validationOptions{ + requireStructuralSchema: true, + }, + }, + { + name: "valid x-kubernetes-validations for object", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + XValidations: apiextensions.ValidationRules{ + { + Rule: "self.minReplicas <= self.maxReplicas", + Message: "minReplicas should be no greater than maxReplicas", + }, + }, + Properties: map[string]apiextensions.JSONSchemaProps{ + "minReplicas": { + Type: "integer", + }, + "maxReplicas": { + Type: "integer", + }, + }, + }, + }, + opts: validationOptions{ + requireStructuralSchema: true, + }, + }, + { + name: "invalid x-kubernetes-validations with empty rule", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + XValidations: apiextensions.ValidationRules{ + {Message: "empty rule"}, + }, + }, + }, + expectedErrors: []validationMatch{ + required("spec.validation.openAPIV3Schema.x-kubernetes-validations[0].rule"), + }, + opts: validationOptions{ + requireStructuralSchema: true, + }, + }, + { + name: "valid x-kubernetes-validations with empty validators", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + XValidations: apiextensions.ValidationRules{}, + }, + }, + opts: validationOptions{ + requireStructuralSchema: true, + }, + }, + { + name: "invalid rule in x-kubernetes-validations", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "subRoot": { + Type: "string", + XValidations: apiextensions.ValidationRules{ + { + Rule: "self == true", + Message: "subRoot should be true.", + }, + { + Rule: "self.endsWith('s')", + Message: "subRoot should end with 's'.", + }, + }, + }, + }, + }, + }, + expectedErrors: []validationMatch{ + invalid("spec.validation.openAPIV3Schema.properties[subRoot].x-kubernetes-validations[0].rule"), + }, + opts: validationOptions{ + requireStructuralSchema: true, + }, + }, + { + name: "valid x-kubernetes-validations for nested object under multiple fields", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + XValidations: apiextensions.ValidationRules{ + { + Rule: "self.minReplicas <= self.maxReplicas", + Message: "minReplicas should be no greater than maxReplicas.", + }, + }, + Properties: map[string]apiextensions.JSONSchemaProps{ + "minReplicas": { + Type: "integer", + }, + "maxReplicas": { + Type: "integer", + }, + "subRule": { + Type: "object", + XValidations: apiextensions.ValidationRules{ + { + Rule: "self.isTest == true", + Message: "isTest should be true.", + }, + }, + Properties: map[string]apiextensions.JSONSchemaProps{ + "isTest": { + Type: "boolean", + }, + }, + }, + }, + }, + }, + opts: validationOptions{ + requireStructuralSchema: true, + }, + }, + { + name: "valid x-kubernetes-validations for object of array", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + XValidations: apiextensions.ValidationRules{ + { + Rule: "size(self.nestedObj[0]) == 10", + Message: "size of first element in nestedObj should be equal to 10", + }, + }, + Properties: map[string]apiextensions.JSONSchemaProps{ + "nestedObj": { + Type: "array", + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "array", + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "string", + }, + }, + }, + }, + }, + }, + }, + }, + opts: validationOptions{ + requireStructuralSchema: true, + }, + }, + { + name: "valid x-kubernetes-validations for array", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "array", + XValidations: apiextensions.ValidationRules{ + { + Rule: "size(self) > 0", + Message: "scoped field should contain more than 0 element.", + }, + }, + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "string", + }, + }, + }, + }, + }, + }, + opts: validationOptions{ + requireStructuralSchema: true, + }, + }, + { + name: "valid x-kubernetes-validations for array of object", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "array", + XValidations: apiextensions.ValidationRules{ + { + Rule: "self[0].nestedObj.val > 0", + Message: "val should be greater than 0.", + }, + }, + Items: &apiextensions.JSONSchemaPropsOrArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "nestedObj": { + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "val": { + Type: "integer", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + opts: validationOptions{ + requireStructuralSchema: true, + }, + }, + { + name: "valid x-kubernetes-validations for escaping", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + XValidations: apiextensions.ValidationRules{ + { + Rule: "self.__if__ > 0", + }, + { + Rule: "self.__namespace__ > 0", + }, + { + Rule: "self.self > 0", + }, + { + Rule: "self.int > 0", + }, + }, + Properties: map[string]apiextensions.JSONSchemaProps{ + "if": { + Type: "integer", + }, + "namespace": { + Type: "integer", + }, + "self": { + Type: "integer", + }, + "int": { + Type: "integer", + }, + }, + }, + }, + opts: validationOptions{ + requireStructuralSchema: true, + }, + }, + { + name: "invalid x-kubernetes-validations for escaping", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + XValidations: apiextensions.ValidationRules{ + { + Rule: "self.if > 0", + }, + { + Rule: "self.namespace > 0", + }, + { + Rule: "self.unknownProp > 0", + }, + }, + Properties: map[string]apiextensions.JSONSchemaProps{ + "if": { + Type: "integer", + }, + "namespace": { + Type: "integer", + }, + }, + }, + }, + expectedErrors: []validationMatch{ + invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[0].rule"), + invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[1].rule"), + invalid("spec.validation.openAPIV3Schema.x-kubernetes-validations[2].rule"), + }, + opts: validationOptions{ + requireStructuralSchema: true, + }, + }, + { + name: "valid default with x-kubernetes-validations", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "embedded": { + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "metadata": { + Type: "object", + XEmbeddedResource: true, + Properties: map[string]apiextensions.JSONSchemaProps{ + "name": { + Type: "string", + XValidations: apiextensions.ValidationRules{ + { + Rule: "self == 'singleton'", + }, + }, + Default: jsonPtr("singleton"), + }, + }, + }, + }, + }, + "value": { + Type: "string", + XValidations: apiextensions.ValidationRules{ + { + Rule: "self.startsWith('kube')", + }, + }, + Default: jsonPtr("kube-everything"), + }, + "object": { + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "field1": { + Type: "integer", + }, + "field2": { + Type: "integer", + }, + }, + XValidations: apiextensions.ValidationRules{ + { + Rule: "self.field1 < self.field2", + }, + }, + Default: jsonPtr(map[string]interface{}{"field1": 1, "field2": 2}), + }, + }, + }, + }, + opts: validationOptions{ + requireStructuralSchema: true, + allowDefaults: true, + }, + }, + { + name: "invalid default with x-kubernetes-validations", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "embedded": { + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "metadata": { + Type: "object", + XEmbeddedResource: true, + Properties: map[string]apiextensions.JSONSchemaProps{ + "name": { + Type: "string", + XValidations: apiextensions.ValidationRules{ + { + Rule: "self == 'singleton'", + }, + }, + Default: jsonPtr("nope"), + }, + }, + }, + }, + }, + "value": { + Type: "string", + XValidations: apiextensions.ValidationRules{ + { + Rule: "self.startsWith('kube')", + }, + }, + Default: jsonPtr("nope"), + }, + "object": { + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "field1": { + Type: "integer", + }, + "field2": { + Type: "integer", + }, + }, + XValidations: apiextensions.ValidationRules{ + { + Rule: "self.field1 < self.field2", + }, + }, + Default: jsonPtr(map[string]interface{}{"field1": 2, "field2": 1}), + }, + }, + }, + }, + expectedErrors: []validationMatch{ + invalid("spec.validation.openAPIV3Schema.properties[embedded].properties[metadata].properties[name].default"), + invalid("spec.validation.openAPIV3Schema.properties[value].default"), + invalid("spec.validation.openAPIV3Schema.properties[object].default"), + }, + opts: validationOptions{ + requireStructuralSchema: true, + allowDefaults: true, + }, + }, + { + name: "rule is empty or not specified", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "value": { + Type: "integer", + XValidations: apiextensions.ValidationRules{ + { + Message: "something", + }, + { + Rule: " ", + Message: "something", + }, + }, + }, + }, + }, + }, + expectedErrors: []validationMatch{ + required("spec.validation.openAPIV3Schema.properties[value].x-kubernetes-validations[0].rule"), + required("spec.validation.openAPIV3Schema.properties[value].x-kubernetes-validations[1].rule"), + }, + }, + { + name: "multiline rule with message", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "value": { + Type: "integer", + XValidations: apiextensions.ValidationRules{ + { + Rule: "self >= 0 &&\nself <= 100", + Message: "value must be between 0 and 100 (inclusive)", + }, + }, + }, + }, + }, + }, + }, + { + name: "invalid and required messages", + input: apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "value": { + Type: "integer", + XValidations: apiextensions.ValidationRules{ + { + Rule: "self >= 0 &&\nself <= 100", + }, + { + Rule: "self == 50", + Message: "value requirements:\nmust be >= 0\nmust be <= 100 ", + }, + { + Rule: "self == 50", + Message: " ", + }, + }, + }, + }, + }, + }, + expectedErrors: []validationMatch{ + // message must be specified if rule contains line breaks + required("spec.validation.openAPIV3Schema.properties[value].x-kubernetes-validations[0].message"), + // message must not contain line breaks + invalid("spec.validation.openAPIV3Schema.properties[value].x-kubernetes-validations[1].message"), + // message must be non-empty if specified + invalid("spec.validation.openAPIV3Schema.properties[value].x-kubernetes-validations[2].message"), + }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got := validateCustomResourceDefinitionValidation(&tt.input, tt.statusEnabled, tt.opts, field.NewPath("spec", "validation")) - if !tt.wantError && len(got) > 0 { - t.Errorf("Expected no error, but got: %v", got) - } else if tt.wantError && len(got) == 0 { - t.Error("Expected error, but got none") + + seenErrs := make([]bool, len(got)) + + for _, expectedError := range tt.expectedErrors { + found := false + for i, err := range got { + if expectedError.matches(err) && !seenErrs[i] { + found = true + seenErrs[i] = true + break + } + } + + if !found { + t.Errorf("expected %v at %v, got %v", expectedError.errorType, expectedError.path.String(), got) + } + } + + for i, seen := range seenErrs { + if !seen { + t.Errorf("unexpected error: %v", got[i]) + } } }) } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/compilation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/compilation.go new file mode 100644 index 00000000000..53652f66523 --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/compilation.go @@ -0,0 +1,127 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cel + +import ( + "fmt" + "strings" + "time" + + "github.com/google/cel-go/cel" + "github.com/google/cel-go/checker/decls" + "github.com/google/cel-go/ext" + + expr "google.golang.org/genproto/googleapis/api/expr/v1alpha1" + "google.golang.org/protobuf/proto" + + "k8s.io/apiextensions-apiserver/pkg/apiserver/schema" + celmodel "k8s.io/apiextensions-apiserver/third_party/forked/celopenapi/model" +) + +// ScopedVarName is the variable name assigned to the locally scoped data element of a CEL valid. +const ScopedVarName = "self" + +// CompilationResult represents the cel compilation result for one rule +type CompilationResult struct { + Program cel.Program + Error *Error +} + +// Compile compiles all the XValidations rules (without recursing into the schema) and returns a slice containing a +// CompilationResult for each ValidationRule, or an error. +// Each CompilationResult may contain: +/// - non-nil Program, nil Error: The program was compiled successfully +// - nil Program, non-nil Error: Compilation resulted in an error +// - nil Program, nil Error: The provided rule was empty so compilation was not attempted +func Compile(s *schema.Structural, isResourceRoot bool) ([]CompilationResult, error) { + if len(s.Extensions.XValidations) == 0 { + return nil, nil + } + celRules := s.Extensions.XValidations + + var propDecls []*expr.Decl + var root *celmodel.DeclType + var ok bool + env, err := cel.NewEnv() + if err != nil { + return nil, err + } + reg := celmodel.NewRegistry(env) + scopedTypeName := generateUniqueSelfTypeName() + rt, err := celmodel.NewRuleTypes(scopedTypeName, s, isResourceRoot, reg) + if err != nil { + return nil, err + } + if rt == nil { + return nil, nil + } + opts, err := rt.EnvOptions(env.TypeProvider()) + if err != nil { + return nil, err + } + root, ok = rt.FindDeclType(scopedTypeName) + if !ok { + rootDecl := celmodel.SchemaDeclType(s, isResourceRoot) + if rootDecl == nil { + return nil, fmt.Errorf("rule declared on schema that does not support validation rules type: '%s' x-kubernetes-preserve-unknown-fields: '%t'", s.Type, s.XPreserveUnknownFields) + } + root = rootDecl.MaybeAssignTypeName(scopedTypeName) + } + propDecls = append(propDecls, decls.NewVar(ScopedVarName, root.ExprType())) + opts = append(opts, cel.Declarations(propDecls...)) + opts = append(opts, ext.Strings()) + env, err = env.Extend(opts...) + if err != nil { + return nil, err + } + + // compResults is the return value which saves a list of compilation results in the same order as x-kubernetes-validations rules. + compResults := make([]CompilationResult, len(celRules)) + for i, rule := range celRules { + var compilationResult CompilationResult + if len(strings.TrimSpace(rule.Rule)) == 0 { + // include a compilation result, but leave both program and error nil per documented return semantics of this + // function + } else { + ast, issues := env.Compile(rule.Rule) + if issues != nil { + compilationResult.Error = &Error{ErrorTypeInvalid, "compilation failed: " + issues.String()} + } else if !proto.Equal(ast.ResultType(), decls.Bool) { + compilationResult.Error = &Error{ErrorTypeInvalid, "cel expression must evaluate to a bool"} + } else { + prog, err := env.Program(ast) + if err != nil { + compilationResult.Error = &Error{ErrorTypeInvalid, "program instantiation failed: " + err.Error()} + } else { + compilationResult.Program = prog + } + } + } + + compResults[i] = compilationResult + } + + return compResults, nil +} + +// generateUniqueSelfTypeName creates a placeholder type name to use in a CEL programs for cases +// where we do not wish to expose a stable type name to CEL validator rule authors. For this to effectively prevent +// developers from depending on the generated name (i.e. using it in CEL programs), it must be changed each time a +// CRD is created or updated. +func generateUniqueSelfTypeName() string { + return fmt.Sprintf("selfType%d", time.Now().Nanosecond()) +} diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/compilation_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/compilation_test.go new file mode 100644 index 00000000000..cfda9b25b78 --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/compilation_test.go @@ -0,0 +1,491 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cel + +import ( + "strings" + "testing" + + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" + "k8s.io/apiextensions-apiserver/pkg/apiserver/schema" +) + +type validationMatch struct { + errorType ErrorType + contains string +} + +func invalidError(contains string) validationMatch { + return validationMatch{errorType: ErrorTypeInvalid, contains: contains} +} + +func (v validationMatch) matches(err *Error) bool { + return err.Type == v.errorType && strings.Contains(err.Error(), v.contains) +} + +func TestCelCompilation(t *testing.T) { + cases := []struct { + name string + input schema.Structural + expectedErrors []validationMatch + }{ + { + name: "valid object", + input: schema.Structural{ + Generic: schema.Generic{ + Type: "object", + }, + Properties: map[string]schema.Structural{ + "minReplicas": { + Generic: schema.Generic{ + Type: "integer", + }, + }, + "maxReplicas": { + Generic: schema.Generic{ + Type: "integer", + }, + }, + }, + Extensions: schema.Extensions{ + XValidations: apiextensions.ValidationRules{ + { + Rule: "self.minReplicas < self.maxReplicas", + Message: "minReplicas should be smaller than maxReplicas", + }, + }, + }, + }, + }, + { + name: "valid for string", + input: schema.Structural{ + Generic: schema.Generic{ + Type: "string", + }, + Extensions: schema.Extensions{ + XValidations: apiextensions.ValidationRules{ + { + Rule: "self.startsWith('s')", + Message: "scoped field should start with 's'", + }, + }, + }, + }, + }, + { + name: "valid for byte", + input: schema.Structural{ + Generic: schema.Generic{ + Type: "string", + }, + ValueValidation: &schema.ValueValidation{ + Format: "byte", + }, + Extensions: schema.Extensions{ + XValidations: apiextensions.ValidationRules{ + { + Rule: "string(self).endsWith('s')", + Message: "scoped field should end with 's'", + }, + }, + }, + }, + }, + { + name: "valid for boolean", + input: schema.Structural{ + Generic: schema.Generic{ + Type: "boolean", + }, + Extensions: schema.Extensions{ + XValidations: apiextensions.ValidationRules{ + { + Rule: "self == true", + Message: "scoped field should be true", + }, + }, + }, + }, + }, + { + name: "valid for integer", + input: schema.Structural{ + Generic: schema.Generic{ + Type: "integer", + }, + Extensions: schema.Extensions{ + XValidations: apiextensions.ValidationRules{ + { + Rule: "self > 0", + Message: "scoped field should be greater than 0", + }, + }, + }, + }, + }, + { + name: "valid for number", + input: schema.Structural{ + Generic: schema.Generic{ + Type: "number", + }, + Extensions: schema.Extensions{ + XValidations: apiextensions.ValidationRules{ + { + Rule: "self > 1.0", + Message: "scoped field should be greater than 1.0", + }, + }, + }, + }, + }, + { + name: "valid nested object of object", + input: schema.Structural{ + Generic: schema.Generic{ + Type: "object", + }, + Properties: map[string]schema.Structural{ + "nestedObj": { + Generic: schema.Generic{ + Type: "object", + }, + Properties: map[string]schema.Structural{ + "val": { + Generic: schema.Generic{ + Type: "integer", + }, + ValueValidation: &schema.ValueValidation{ + Format: "int64", + }, + }, + }, + }, + }, + Extensions: schema.Extensions{ + XValidations: apiextensions.ValidationRules{ + { + Rule: "self.nestedObj.val == 10", + Message: "val should be equal to 10", + }, + }, + }, + }, + }, + { + name: "valid nested object of array", + input: schema.Structural{ + Generic: schema.Generic{ + Type: "object", + }, + Properties: map[string]schema.Structural{ + "nestedObj": { + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "string", + }, + }, + }, + }, + }, + Extensions: schema.Extensions{ + XValidations: apiextensions.ValidationRules{ + { + Rule: "size(self.nestedObj[0]) == 10", + Message: "size of first element in nestedObj should be equal to 10", + }, + }, + }, + }, + }, + { + name: "valid nested array of array", + input: schema.Structural{ + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "string", + }, + }, + }, + }, + Extensions: schema.Extensions{ + XValidations: apiextensions.ValidationRules{ + { + Rule: "size(self[0][0]) == 10", + Message: "size of items under items of scoped field should be equal to 10", + }, + }, + }, + }, + }, + { + name: "valid nested array of object", + input: schema.Structural{ + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "object", + }, + Properties: map[string]schema.Structural{ + "nestedObj": { + Generic: schema.Generic{ + Type: "object", + }, + Properties: map[string]schema.Structural{ + "val": { + Generic: schema.Generic{ + Type: "integer", + }, + ValueValidation: &schema.ValueValidation{ + Format: "int64", + }, + }, + }, + }, + }, + }, + Extensions: schema.Extensions{ + XValidations: apiextensions.ValidationRules{ + { + Rule: "self[0].nestedObj.val == 10", + Message: "val under nestedObj under properties under items should be equal to 10", + }, + }, + }, + }, + }, + { + name: "valid map", + input: schema.Structural{ + Generic: schema.Generic{ + Type: "object", + AdditionalProperties: &schema.StructuralOrBool{ + Bool: true, + Structural: &schema.Structural{ + Generic: schema.Generic{ + Type: "boolean", + Nullable: false, + }, + }, + }, + }, + Extensions: schema.Extensions{ + XValidations: apiextensions.ValidationRules{ + { + Rule: "size(self) > 0", + Message: "size of scoped field should be greater than 0", + }, + }, + }, + }, + }, + { + name: "invalid checking for number", + input: schema.Structural{ + Generic: schema.Generic{ + Type: "number", + }, + Extensions: schema.Extensions{ + XValidations: apiextensions.ValidationRules{ + { + Rule: "size(self) == 10", + Message: "size of scoped field should be equal to 10", + }, + }, + }, + }, + expectedErrors: []validationMatch{ + invalidError("compilation failed"), + }, + }, + { + name: "compilation failure", + input: schema.Structural{ + Generic: schema.Generic{ + Type: "integer", + }, + Extensions: schema.Extensions{ + XValidations: apiextensions.ValidationRules{ + { + Rule: "size(self) == 10", + Message: "size of scoped field should be equal to 10", + }, + }, + }, + }, + expectedErrors: []validationMatch{ + invalidError("compilation failed"), + }, + }, + { + name: "valid for escaping", + input: schema.Structural{ + Generic: schema.Generic{ + Type: "object", + }, + Properties: map[string]schema.Structural{ + "namespace": { + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "string", + }, + }, + }, + }, + "if": { + Generic: schema.Generic{ + Type: "integer", + }, + }, + "self": { + Generic: schema.Generic{ + Type: "integer", + }, + }, + }, + Extensions: schema.Extensions{ + XValidations: apiextensions.ValidationRules{ + { + Rule: "size(self.__namespace__[0]) == 10", + Message: "size of first element in nestedObj should be equal to 10", + }, + { + Rule: "self.__if__ == 10", + }, + { + Rule: "self.self == 10", + }, + }, + }, + }, + }, + { + name: "invalid for escaping", + input: schema.Structural{ + Generic: schema.Generic{ + Type: "object", + }, + Properties: map[string]schema.Structural{ + "namespace": { + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "array", + }, + Items: &schema.Structural{ + Generic: schema.Generic{ + Type: "string", + }, + }, + }, + }, + "if": { + Generic: schema.Generic{ + Type: "integer", + }, + }, + "self": { + Generic: schema.Generic{ + Type: "integer", + }, + }, + }, + Extensions: schema.Extensions{ + XValidations: apiextensions.ValidationRules{ + { + Rule: "size(self.namespace[0]) == 10", + Message: "size of first element in nestedObj should be equal to 10", + }, + { + Rule: "self.if == 10", + }, + { + Rule: "self == 10", + }, + }, + }, + }, + expectedErrors: []validationMatch{ + invalidError("undefined field 'namespace'"), + invalidError("undefined field 'if'"), + invalidError("found no matching overload"), + }, + }, + } + + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + compilationResults, err := Compile(&tt.input, false) + if err != nil { + t.Errorf("Expected no error, but got: %v", err) + } + + seenErrs := make([]bool, len(compilationResults)) + + for _, expectedError := range tt.expectedErrors { + found := false + for i, result := range compilationResults { + if expectedError.matches(result.Error) && !seenErrs[i] { + found = true + seenErrs[i] = true + break + } + } + + if !found { + t.Errorf("expected error: %v", expectedError) + } + } + + for i, seen := range seenErrs { + if !seen && compilationResults[i].Error != nil { + t.Errorf("unexpected error: %v", compilationResults[i].Error) + } + } + }) + } +} diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/errors.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/errors.go new file mode 100644 index 00000000000..907ca6ec8f1 --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/errors.go @@ -0,0 +1,47 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cel + +// Error is an implementation of the 'error' interface, which represents a +// XValidation error. +type Error struct { + Type ErrorType + Detail string +} + +var _ error = &Error{} + +// Error implements the error interface. +func (v *Error) Error() string { + return v.Detail +} + +// ErrorType is a machine readable value providing more detail about why +// a XValidation is invalid. +type ErrorType string + +const ( + // ErrorTypeRequired is used to report withNullable values that are not + // provided (e.g. empty strings, null values, or empty arrays). See + // Required(). + ErrorTypeRequired ErrorType = "RuleRequired" + // ErrorTypeInvalid is used to report malformed values + ErrorTypeInvalid ErrorType = "RuleInvalid" + // ErrorTypeInternal is used to report other errors that are not related + // to user input. See InternalError(). + ErrorTypeInternal ErrorType = "InternalError" +) diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/convert.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/convert.go index b7a19d17de3..4b6b1eedf61 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/convert.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/convert.go @@ -18,7 +18,6 @@ package schema import ( "fmt" - "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" ) @@ -247,6 +246,7 @@ func newExtensions(s *apiextensions.JSONSchemaProps) (*Extensions, error) { XListMapKeys: s.XListMapKeys, XListType: s.XListType, XMapType: s.XMapType, + XValidations: s.XValidations, } if s.XPreserveUnknownFields != nil { diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/kubeopenapi.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/kubeopenapi.go index f389ed4e05b..23bffbfb190 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/kubeopenapi.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/kubeopenapi.go @@ -87,6 +87,9 @@ func (x *Extensions) toKubeOpenAPI(ret *spec.Schema) { if x.XMapType != nil { ret.VendorExtensible.AddExtension("x-kubernetes-map-type", *x.XMapType) } + if len(x.XValidations) > 0 { + ret.VendorExtensible.AddExtension("x-kubernetes-validations", x.XValidations) + } } func (v *ValueValidation) toKubeOpenAPI(ret *spec.Schema) { diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/structural.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/structural.go index ebcffdea68f..1948b7ba47a 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/structural.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/structural.go @@ -17,6 +17,7 @@ limitations under the License. package schema import ( + "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" "k8s.io/apimachinery/pkg/runtime" ) @@ -127,6 +128,9 @@ type Extensions struct { // Atomic maps will be entirely replaced when updated. // +optional XMapType *string + + // x-kubernetes-validations describes a list of validation rules for expression validation. + XValidations apiextensions.ValidationRules } // +k8s:deepcopy-gen=true 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 6a8be321b40..e2966341a7b 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 @@ -324,6 +324,9 @@ func validateNestedValueValidation(v *NestedValueValidation, skipAnyOf, skipAllO if v.ForbiddenExtensions.XMapType != nil { allErrs = append(allErrs, field.Forbidden(fldPath.Child("x-kubernetes-map-type"), "must be undefined to be structural")) } + if len(v.ForbiddenExtensions.XValidations) > 0 { + allErrs = append(allErrs, field.Forbidden(fldPath.Child("x-kubernetes-validations"), "must be empty to be structural")) + } // forbid reasoning about metadata because it can lead to metadata restriction we don't want if _, found := v.Properties["metadata"]; found { diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation.go index fdd69c22b6c..14081c94461 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation.go @@ -18,19 +18,9 @@ package validation import ( "encoding/json" - "fmt" "strings" - "github.com/google/cel-go/cel" - "github.com/google/cel-go/checker/decls" - "github.com/google/cel-go/common/types" - "github.com/google/cel-go/ext" - - expr "google.golang.org/genproto/googleapis/api/expr/v1alpha1" - "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" - "k8s.io/apiextensions-apiserver/pkg/apiserver/schema" - "k8s.io/apiextensions-apiserver/third_party/forked/celopenapi/model" "k8s.io/apimachinery/pkg/util/validation/field" openapierrors "k8s.io/kube-openapi/pkg/validation/errors" "k8s.io/kube-openapi/pkg/validation/spec" @@ -264,6 +254,9 @@ func ConvertJSONSchemaPropsWithPostProcess(in *apiextensions.JSONSchemaProps, ou if in.XMapType != nil { out.VendorExtensible.AddExtension("x-kubernetes-map-type", *in.XMapType) } + if len(in.XValidations) != 0 { + out.VendorExtensible.AddExtension("x-kubernetes-validations", in.XValidations) + } return nil } @@ -339,45 +332,3 @@ func convertJSONSchemaPropsOrStringArray(in *apiextensions.JSONSchemaPropsOrStri } return nil } - -// CompileAndValidate provides a sanity check of the CEL validation functionality until we wire in the -// full functionality. -func CompileAndValidate(s *schema.Structural, bindings map[string]interface{}, rule string) (bool, error) { - env, err := cel.NewEnv() - if err != nil { - return false, err - } - reg := model.NewRegistry(env) - rt, err := model.NewRuleTypes("testType", s, reg) - if err != nil { - return false, err - } - opts, err := rt.EnvOptions(env.TypeProvider()) - if err != nil { - return false, err - } - root, ok := rt.FindDeclType("testType") - if !ok { - root = model.SchemaDeclType(s).MaybeAssignTypeName("testType") - } - propDecls := []*expr.Decl{decls.NewVar("self", root.ExprType())} - opts = append(opts, cel.Declarations(propDecls...)) - opts = append(opts, ext.Strings()) - env, err = env.Extend(opts...) - if err != nil { - return false, err - } - ast, issues := env.Compile(rule) - if issues != nil { - return false, fmt.Errorf("issues: %v", issues) - } - prog, err := env.Program(ast) - if err != nil { - return false, err - } - evalResult, _, err := prog.Eval(bindings) - if err != nil { - return false, err - } - return evalResult == types.True, nil -} diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation_test.go index b80ccd42131..2d71ead9e98 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/validation_test.go @@ -23,7 +23,6 @@ import ( "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" apiextensionsfuzzer "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/fuzzer" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - "k8s.io/apiextensions-apiserver/pkg/apiserver/schema" "k8s.io/apimachinery/pkg/api/apitesting/fuzzer" apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/runtime" @@ -485,32 +484,3 @@ func TestItemsProperty(t *testing.T) { }) } } - -// TestCompileAndValidate is a placeholder test of CEL validation that will be removed when the actual functionality -// is wired in. -func TestCompileAndValidate(t *testing.T) { - s := &schema.Structural{ - Generic: schema.Generic{ - Type: "object", - }, - Properties: map[string]schema.Structural{ - "name": { - Generic: schema.Generic{ - Type: "string", - }, - }, - }, - } - bindings := map[string]interface{}{ - "self": map[string]interface{}{ - "name": "kube", - }, - } - result, err := CompileAndValidate(s, bindings, "self.name == 'kube'") - if err != nil { - t.Fatal(err) - } - if !result { - t.Error("Expected expression to evaluate to true") - } -} diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy.go index 708ef2d9b11..421c2119f60 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy.go @@ -28,9 +28,11 @@ import ( "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/apiserver/pkg/features" "k8s.io/apiserver/pkg/registry/generic" "k8s.io/apiserver/pkg/storage" "k8s.io/apiserver/pkg/storage/names" + utilfeature "k8s.io/apiserver/pkg/util/feature" "sigs.k8s.io/structured-merge-diff/v4/fieldpath" ) @@ -77,6 +79,8 @@ func (strategy) PrepareForCreate(ctx context.Context, obj runtime.Object) { break } } + + dropDisabledFields(crd, nil) } // PrepareForUpdate clears fields that are not allowed to be set by end users on update. @@ -105,6 +109,8 @@ func (strategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Object) { break } } + + dropDisabledFields(newCRD, oldCRD) } // Validate validates a new CustomResourceDefinition. @@ -222,3 +228,54 @@ func MatchCustomResourceDefinition(label labels.Selector, field fields.Selector) func CustomResourceDefinitionToSelectableFields(obj *apiextensions.CustomResourceDefinition) fields.Set { return generic.ObjectMetaFieldsSet(&obj.ObjectMeta, true) } + +// dropDisabledFields drops disabled fields that are not used if their associated feature gates +// are not enabled. +func dropDisabledFields(newCRD *apiextensions.CustomResourceDefinition, oldCRD *apiextensions.CustomResourceDefinition) { + if !utilfeature.DefaultFeatureGate.Enabled(features.CustomResourceValidationExpressions) && (oldCRD == nil || (oldCRD != nil && !specHasXValidations(&oldCRD.Spec))) { + if newCRD.Spec.Validation != nil { + dropXValidationsField(newCRD.Spec.Validation.OpenAPIV3Schema) + } + for _, v := range newCRD.Spec.Versions { + if v.Schema != nil { + dropXValidationsField(v.Schema.OpenAPIV3Schema) + } + } + } +} + +// dropXValidationsField drops field XValidations from CRD schema +func dropXValidationsField(schema *apiextensions.JSONSchemaProps) { + if schema == nil { + return + } + schema.XValidations = nil + if schema.AdditionalProperties != nil { + dropXValidationsField(schema.AdditionalProperties.Schema) + } + for def, jsonSchema := range schema.Properties { + dropXValidationsField(&jsonSchema) + schema.Properties[def] = jsonSchema + } + if schema.Items != nil { + dropXValidationsField(schema.Items.Schema) + for i, jsonSchema := range schema.Items.JSONSchemas { + dropXValidationsField(&jsonSchema) + schema.Items.JSONSchemas[i] = jsonSchema + } + } + for def, jsonSchemaPropsOrStringArray := range schema.Dependencies { + dropXValidationsField(jsonSchemaPropsOrStringArray.Schema) + schema.Dependencies[def] = jsonSchemaPropsOrStringArray + } +} + +func specHasXValidations(spec *apiextensions.CustomResourceDefinitionSpec) bool { + return validation.HasSchemaWith(spec, schemaHasXValidations) +} + +func schemaHasXValidations(s *apiextensions.JSONSchemaProps) bool { + return validation.SchemaHas(s, func(s *apiextensions.JSONSchemaProps) bool { + return s.XValidations != nil + }) +} diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy_test.go index ecef7af8dca..bb9b84c5d20 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresourcedefinition/strategy_test.go @@ -19,11 +19,15 @@ package customresourcedefinition import ( "testing" + "github.com/google/go-cmp/cmp" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" + featuregatetesting "k8s.io/component-base/featuregate/testing" "k8s.io/utils/pointer" ) @@ -187,3 +191,483 @@ func TestValidateAPIApproval(t *testing.T) { }) } } + +// TestDropDisabledFields tests if the drop functionality is working fine or not with feature gate switch +func TestDropDisabledFields(t *testing.T) { + testCases := []struct { + name string + enableXValidations bool + crd *apiextensions.CustomResourceDefinition + oldCRD *apiextensions.CustomResourceDefinition + expectedCRD *apiextensions.CustomResourceDefinition + }{ + { + name: "For creation, FG disabled, no XValidations, no field drop", + enableXValidations: false, + crd: &apiextensions.CustomResourceDefinition{}, + oldCRD: nil, + expectedCRD: &apiextensions.CustomResourceDefinition{}, + }, + { + name: "For creation, FG disabled, empty XValidations, no field drop", + enableXValidations: false, + crd: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "foos.sigs.k8s.io", Annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: "valid"}, ResourceVersion: "1"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Validation: &apiextensions.CustomResourceValidation{}, + }, + }, + oldCRD: nil, + expectedCRD: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "foos.sigs.k8s.io", Annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: "valid"}, ResourceVersion: "1"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Validation: &apiextensions.CustomResourceValidation{}, + }, + }, + }, + { + name: "For creation, FG disabled, set XValidations, drop XValidations", + enableXValidations: false, + crd: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "foos.sigs.k8s.io", Annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: "valid"}, ResourceVersion: "1"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + XValidations: apiextensions.ValidationRules{ + { + Rule: "size(self) > 0", + Message: "openAPIV3Schema should contain more than 0 element.", + }, + }, + Dependencies: apiextensions.JSONSchemaDependencies{ + "test": apiextensions.JSONSchemaPropsOrStringArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + XValidations: apiextensions.ValidationRules{ + { + Rule: "size(self) > 0", + Message: "size of scoped field should be greater than 0.", + }, + }, + }, + }, + }, + Properties: map[string]apiextensions.JSONSchemaProps{ + "subRule": { + Type: "object", + XValidations: apiextensions.ValidationRules{ + { + Rule: "isTest == true", + Message: "isTest should be true.", + }, + }, + Properties: map[string]apiextensions.JSONSchemaProps{ + "isTest": { + Type: "boolean", + }, + }, + }, + }, + }, + }, + }, + }, + oldCRD: nil, + expectedCRD: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "foos.sigs.k8s.io", Annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: "valid"}, ResourceVersion: "1"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Dependencies: apiextensions.JSONSchemaDependencies{ + "test": apiextensions.JSONSchemaPropsOrStringArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, + }, + Properties: map[string]apiextensions.JSONSchemaProps{ + "subRule": { + Type: "object", + Properties: map[string]apiextensions.JSONSchemaProps{ + "isTest": { + Type: "boolean", + }, + }, + }, + }, + }, + }, + }, + }, + }, + { + name: "For creation, FG enabled, set XValidations, update with XValidations", + enableXValidations: true, + crd: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "foos.sigs.k8s.io", Annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: "valid"}, ResourceVersion: "1"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + XValidations: apiextensions.ValidationRules{ + { + Rule: "size(self) > 0", + Message: "openAPIV3Schema should contain more than 0 element.", + }, + }, + Dependencies: apiextensions.JSONSchemaDependencies{ + "test": apiextensions.JSONSchemaPropsOrStringArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + XValidations: apiextensions.ValidationRules{ + { + Rule: "size(self) > 0", + Message: "size of scoped field should be greater than 0.", + }, + }, + }, + }, + }, + Properties: map[string]apiextensions.JSONSchemaProps{ + "subRule": { + Type: "object", + XValidations: apiextensions.ValidationRules{ + { + Rule: "isTest == true", + Message: "isTest should be true.", + }, + }, + Properties: map[string]apiextensions.JSONSchemaProps{ + "isTest": { + Type: "boolean", + }, + }, + }, + }, + }, + }, + }, + }, + oldCRD: nil, + expectedCRD: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "foos.sigs.k8s.io", Annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: "valid"}, ResourceVersion: "1"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + XValidations: apiextensions.ValidationRules{ + { + Rule: "size(self) > 0", + Message: "openAPIV3Schema should contain more than 0 element.", + }, + }, + Dependencies: apiextensions.JSONSchemaDependencies{ + "test": apiextensions.JSONSchemaPropsOrStringArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + XValidations: apiextensions.ValidationRules{ + { + Rule: "size(self) > 0", + Message: "size of scoped field should be greater than 0.", + }, + }, + }, + }, + }, + Properties: map[string]apiextensions.JSONSchemaProps{ + "subRule": { + Type: "object", + XValidations: apiextensions.ValidationRules{ + { + Rule: "isTest == true", + Message: "isTest should be true.", + }, + }, + Properties: map[string]apiextensions.JSONSchemaProps{ + "isTest": { + Type: "boolean", + }, + }, + }, + }, + }, + }, + }, + }, + }, + { + name: "For update, FG disabled, oldCRD XValidation in use, don't drop XValidations", + enableXValidations: false, + crd: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "foos.sigs.k8s.io", Annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: "valid"}, ResourceVersion: "1"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + XValidations: apiextensions.ValidationRules{ + { + Rule: "size(self) > 0", + Message: "openAPIV3Schema should contain more than 0 element.", + }, + }, + Dependencies: apiextensions.JSONSchemaDependencies{ + "test": apiextensions.JSONSchemaPropsOrStringArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + XValidations: apiextensions.ValidationRules{ + { + Rule: "size(self) > 0", + Message: "size of scoped field should be greater than 0.", + }, + }, + }, + }, + }, + Properties: map[string]apiextensions.JSONSchemaProps{ + "subRule": { + Type: "object", + XValidations: apiextensions.ValidationRules{ + { + Rule: "isTest == true", + Message: "isTest should be true.", + }, + }, + Properties: map[string]apiextensions.JSONSchemaProps{ + "isTest": { + Type: "boolean", + }, + }, + }, + }, + }, + }, + }, + }, + oldCRD: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "foos.sigs.k8s.io", Annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: "valid"}, ResourceVersion: "1"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + Dependencies: apiextensions.JSONSchemaDependencies{ + "test": apiextensions.JSONSchemaPropsOrStringArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + XValidations: apiextensions.ValidationRules{ + { + Rule: "size(self) > 0", + Message: "size of scoped field should be greater than 0.", + }, + }, + }, + }, + }, + }, + }, + }, + }, + expectedCRD: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "foos.sigs.k8s.io", Annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: "valid"}, ResourceVersion: "1"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + XValidations: apiextensions.ValidationRules{ + { + Rule: "size(self) > 0", + Message: "openAPIV3Schema should contain more than 0 element.", + }, + }, + Dependencies: apiextensions.JSONSchemaDependencies{ + "test": apiextensions.JSONSchemaPropsOrStringArray{ + Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + XValidations: apiextensions.ValidationRules{ + { + Rule: "size(self) > 0", + Message: "size of scoped field should be greater than 0.", + }, + }, + }, + }, + }, + Properties: map[string]apiextensions.JSONSchemaProps{ + "subRule": { + Type: "object", + XValidations: apiextensions.ValidationRules{ + { + Rule: "isTest == true", + Message: "isTest should be true.", + }, + }, + Properties: map[string]apiextensions.JSONSchemaProps{ + "isTest": { + Type: "boolean", + }, + }, + }, + }, + }, + }, + }, + }, + }, + { + name: "For update, FG disabled, oldCRD has no XValidations, drop XValidations", + enableXValidations: false, + crd: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "foos.sigs.k8s.io", Annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: "valid"}, ResourceVersion: "1"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + XValidations: apiextensions.ValidationRules{ + { + Rule: "size(self) > 0", + Message: "openAPIV3Schema should contain more than 0 element.", + }, + }, + }, + }, + }, + }, + oldCRD: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "foos.sigs.k8s.io", Annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: "valid"}, ResourceVersion: "1"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, + }, + }, + expectedCRD: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "foos.sigs.k8s.io", Annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: "valid"}, ResourceVersion: "1"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, + }, + }, + }, + { + name: "For update, FG enabled, oldCRD has XValidations, updated to newCRD", + enableXValidations: true, + crd: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "foos.sigs.k8s.io", Annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: "valid"}, ResourceVersion: "1"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + XValidations: apiextensions.ValidationRules{ + { + Rule: "size(self) > 0", + Message: "openAPIV3Schema should contain more than 0 element.", + }, + }, + }, + }, + }, + }, + oldCRD: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "foos.sigs.k8s.io", Annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: "valid"}, ResourceVersion: "1"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + XValidations: apiextensions.ValidationRules{ + { + Rule: "old data", + Message: "old data", + }, + }, + }, + }, + }, + }, + expectedCRD: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "foos.sigs.k8s.io", Annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: "valid"}, ResourceVersion: "1"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + XValidations: apiextensions.ValidationRules{ + { + Rule: "size(self) > 0", + Message: "openAPIV3Schema should contain more than 0 element.", + }, + }, + }, + }, + }, + }, + }, + { + name: "For update, FG enabled, oldCRD has no XValidations, updated to newCRD", + enableXValidations: true, + crd: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "foos.sigs.k8s.io", Annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: "valid"}, ResourceVersion: "1"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + XValidations: apiextensions.ValidationRules{ + { + Rule: "size(self) > 0", + Message: "openAPIV3Schema should contain more than 0 element.", + }, + }, + }, + }, + }, + }, + oldCRD: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "foos.sigs.k8s.io", Annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: "valid"}, ResourceVersion: "1"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + }, + }, + }, + }, + expectedCRD: &apiextensions.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: "foos.sigs.k8s.io", Annotations: map[string]string{v1beta1.KubeAPIApprovedAnnotation: "valid"}, ResourceVersion: "1"}, + Spec: apiextensions.CustomResourceDefinitionSpec{ + Validation: &apiextensions.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensions.JSONSchemaProps{ + Type: "object", + XValidations: apiextensions.ValidationRules{ + { + Rule: "size(self) > 0", + Message: "openAPIV3Schema should contain more than 0 element.", + }, + }, + }, + }, + }, + }, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CustomResourceValidationExpressions, tc.enableXValidations)() + old := tc.oldCRD.DeepCopy() + + dropDisabledFields(tc.crd, tc.oldCRD) + + // old crd should never be changed + if diff := cmp.Diff(tc.oldCRD, old); diff != "" { + t.Fatalf("old crd changed from %v to %v", tc.oldCRD, old) + } + + if diff := cmp.Diff(tc.expectedCRD, tc.crd); diff != "" { + t.Fatalf("unexpected crd: %v", tc.crd) + } + }) + } +}