ratcheting-cel: add optionalOldSelf field

This commit is contained in:
Alexander Zielenski 2023-10-24 11:40:59 -07:00
parent 18adc30933
commit 5edb27aa38
7 changed files with 829 additions and 0 deletions

View File

@ -210,6 +210,19 @@ type ValidationRule struct {
// - '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.
//
// If `rule` makes use of the `oldSelf` variable it is implicitly a
// `transition rule`.
//
// By default, the `oldSelf` variable is the same type as `self`.
// When `optionalOldSelf` is true, the `oldSelf` variable is a CEL optional
// variable whose value() is the same type as `self`.
// See the documentation for the `optionalOldSelf` field for details.
//
// Transition rules by default are applied only on UPDATE requests and are
// skipped if an old value could not be found. You can opt a transition
// rule into unconditional evaluation by setting `optionalOldSelf` to true.
//
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.
@ -246,6 +259,24 @@ type ValidationRule struct {
// e.g. for attribute `foo.34$` appears in a list `testList`, the fieldPath could be set to `.testList['foo.34$']`
// +optional
FieldPath string
// optionalOldSelf is used to opt a transition rule into evaluation
// even when the object is first created, or if the old object is
// missing the value.
//
// When enabled `oldSelf` will be a CEL optional whose value will be
// `None` if there is no old value, or when the object is initially created.
//
// You may check for presence of oldSelf using `oldSelf.hasValue()` and
// unwrap it after checking using `oldSelf.value()`. Check the CEL
// documentation for Optional types for more information:
// https://pkg.go.dev/github.com/google/cel-go/cel#OptionalTypes
//
// May not be set unless `oldSelf` is used in `rule`.
//
// +featureGate=CRDValidationRatcheting
// +optional
OptionalOldSelf *bool
}
// JSON represents any valid JSON value.

View File

@ -249,6 +249,19 @@ type ValidationRule struct {
// - '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.
//
// If `rule` makes use of the `oldSelf` variable it is implicitly a
// `transition rule`.
//
// By default, the `oldSelf` variable is the same type as `self`.
// When `optionalOldSelf` is true, the `oldSelf` variable is a CEL optional
// variable whose value() is the same type as `self`.
// See the documentation for the `optionalOldSelf` field for details.
//
// Transition rules by default are applied only on UPDATE requests and are
// skipped if an old value could not be found. You can opt a transition
// rule into unconditional evaluation by setting `optionalOldSelf` to true.
//
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.
@ -285,6 +298,24 @@ type ValidationRule struct {
// e.g. for attribute `foo.34$` appears in a list `testList`, the fieldPath could be set to `.testList['foo.34$']`
// +optional
FieldPath string `json:"fieldPath,omitempty" protobuf:"bytes,5,opt,name=fieldPath"`
// optionalOldSelf is used to opt a transition rule into evaluation
// even when the object is first created, or if the old object is
// missing the value.
//
// When enabled `oldSelf` will be a CEL optional whose value will be
// `None` if there is no old value, or when the object is initially created.
//
// You may check for presence of oldSelf using `oldSelf.hasValue()` and
// unwrap it after checking using `oldSelf.value()`. Check the CEL
// documentation for Optional types for more information:
// https://pkg.go.dev/github.com/google/cel-go/cel#OptionalTypes
//
// May not be set unless `oldSelf` is used in `rule`.
//
// +featureGate=CRDValidationRatcheting
// +optional
OptionalOldSelf *bool `json:"optionalOldSelf,omitempty" protobuf:"bytes,6,opt,name=optionalOldSelf"`
}
// JSON represents any valid JSON value.

View File

@ -249,6 +249,19 @@ type ValidationRule struct {
// - '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.
//
// If `rule` makes use of the `oldSelf` variable it is implicitly a
// `transition rule`.
//
// By default, the `oldSelf` variable is the same type as `self`.
// When `optionalOldSelf` is true, the `oldSelf` variable is a CEL optional
// variable whose value() is the same type as `self`.
// See the documentation for the `optionalOldSelf` field for details.
//
// Transition rules by default are applied only on UPDATE requests and are
// skipped if an old value could not be found. You can opt a transition
// rule into unconditional evaluation by setting `optionalOldSelf` to true.
//
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.
@ -285,6 +298,24 @@ type ValidationRule struct {
// e.g. for attribute `foo.34$` appears in a list `testList`, the fieldPath could be set to `.testList['foo.34$']`
// +optional
FieldPath string `json:"fieldPath,omitempty" protobuf:"bytes,5,opt,name=fieldPath"`
// optionalOldSelf is used to opt a transition rule into evaluation
// even when the object is first created, or if the old object is
// missing the value.
//
// When enabled `oldSelf` will be a CEL optional whose value will be
// `None` if there is no old value, or when the object is initially created.
//
// You may check for presence of oldSelf using `oldSelf.hasValue()` and
// unwrap it after checking using `oldSelf.value()`. Check the CEL
// documentation for Optional types for more information:
// https://pkg.go.dev/github.com/google/cel-go/cel#OptionalTypes
//
// May not be set unless `oldSelf` is used in `rule`.
//
// +featureGate=CRDValidationRatcheting
// +optional
OptionalOldSelf *bool `json:"optionalOldSelf,omitempty" protobuf:"bytes,6,opt,name=optionalOldSelf"`
}
// JSON represents any valid JSON value.

View File

@ -1186,6 +1186,8 @@ func ValidateCustomResourceDefinitionOpenAPISchema(schema *apiextensions.JSONSch
if uncorrelatablePath := ssv.forbidOldSelfValidations(); uncorrelatablePath != nil {
allErrs.CELErrors = append(allErrs.CELErrors, field.Invalid(fldPath.Child("x-kubernetes-validations").Index(i).Child("rule"), schema.XValidations[i].Rule, fmt.Sprintf("oldSelf cannot be used on the uncorrelatable portion of the schema within %v", uncorrelatablePath)))
}
} else if schema.XValidations[i].OptionalOldSelf != nil {
allErrs.CELErrors = append(allErrs.CELErrors, field.Invalid(fldPath.Child("x-kubernetes-validations").Index(i).Child("optionalOldSelf"), *schema.XValidations[i].OptionalOldSelf, "may not be set if oldSelf is not used in rule"))
}
}
}

View File

@ -41,6 +41,7 @@ import (
"k8s.io/apiserver/pkg/cel/environment"
"k8s.io/apiserver/pkg/cel/library"
"k8s.io/utils/pointer"
"k8s.io/utils/ptr"
)
type validationMatch struct {
@ -9650,6 +9651,157 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
required("spec.validation.openAPIV3Schema.properties[f].x-kubernetes-validations[0].messageExpression"),
},
},
{
name: "forbid transition rule on element of list of type atomic when optionalOldSelf is set",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"value": {
Type: "array",
XListType: strPtr("atomic"),
Items: &apiextensions.JSONSchemaPropsOrArray{
Schema: &apiextensions.JSONSchemaProps{
Type: "string",
MaxLength: int64ptr(10),
XValidations: apiextensions.ValidationRules{
{Rule: `self == oldSelf.orValue("")`, OptionalOldSelf: ptr.To(true)},
},
},
},
},
},
},
},
expectedErrors: []validationMatch{
invalid("spec.validation.openAPIV3Schema.properties[value].items.x-kubernetes-validations[0].rule"),
},
},
{
name: "forbid transition rule on element of list defaulting to type atomic when optionalOldSelf is set",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"value": {
Type: "array",
Items: &apiextensions.JSONSchemaPropsOrArray{
Schema: &apiextensions.JSONSchemaProps{
Type: "string",
MaxLength: int64ptr(10),
XValidations: apiextensions.ValidationRules{
{Rule: `self == oldSelf.orValue("")`, OptionalOldSelf: ptr.To(true)},
},
},
},
},
},
},
},
expectedErrors: []validationMatch{
invalid("spec.validation.openAPIV3Schema.properties[value].items.x-kubernetes-validations[0].rule"),
},
},
{
name: "forbid transition rule on element of list of type set when optionalOldSelf is set",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"value": {
Type: "array",
MaxItems: int64ptr(10),
XListType: strPtr("set"),
Items: &apiextensions.JSONSchemaPropsOrArray{
Schema: &apiextensions.JSONSchemaProps{
Type: "string",
MaxLength: int64ptr(10),
XValidations: apiextensions.ValidationRules{
{Rule: `self == oldSelf.orValue("")`, OptionalOldSelf: ptr.To(true)},
},
},
},
},
},
},
},
expectedErrors: []validationMatch{
invalid("spec.validation.openAPIV3Schema.properties[value].items.x-kubernetes-validations[0].rule"),
},
},
{
name: "forbid transition rule on element of map of unrecognized type when optionalOldSelf is set",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"value": {
Type: "object",
XMapType: strPtr("future"),
Properties: map[string]apiextensions.JSONSchemaProps{
"subfield": {
Type: "string",
MaxLength: int64ptr(10),
XValidations: apiextensions.ValidationRules{
{Rule: `self == oldSelf.orValue("")`, OptionalOldSelf: ptr.To(true)},
},
},
},
},
},
},
},
expectedErrors: []validationMatch{
invalid("spec.validation.openAPIV3Schema.properties[value].properties[subfield].x-kubernetes-validations[0].rule"),
unsupported("spec.validation.openAPIV3Schema.properties[value].x-kubernetes-map-type"),
},
},
{
name: "forbid setting optionalOldSelf to true if oldSelf is not used",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"value": {
Type: "string",
MaxLength: int64ptr(10),
XValidations: apiextensions.ValidationRules{
{Rule: `self == "foo"`, OptionalOldSelf: ptr.To(true)},
},
},
},
},
},
expectedErrors: []validationMatch{
invalid("spec.validation.openAPIV3Schema.properties[value].x-kubernetes-validations[0].optionalOldSelf"),
},
},
{
name: "forbid setting optionalOldSelf to false if oldSelf is not used",
opts: validationOptions{requireStructuralSchema: true},
input: apiextensions.CustomResourceValidation{
OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
Type: "object",
Properties: map[string]apiextensions.JSONSchemaProps{
"value": {
Type: "string",
MaxLength: int64ptr(10),
XValidations: apiextensions.ValidationRules{
{Rule: `self == "foo"`, OptionalOldSelf: ptr.To(false)},
},
},
},
},
},
expectedErrors: []validationMatch{
invalid("spec.validation.openAPIV3Schema.properties[value].x-kubernetes-validations[0].optionalOldSelf"),
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {

View File

@ -24,6 +24,7 @@ import (
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation"
apiextensionsfeatures "k8s.io/apiextensions-apiserver/pkg/features"
apiequality "k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/fields"
@ -33,6 +34,7 @@ import (
"k8s.io/apiserver/pkg/registry/generic"
"k8s.io/apiserver/pkg/storage"
"k8s.io/apiserver/pkg/storage/names"
utilfeature "k8s.io/apiserver/pkg/util/feature"
)
// strategy implements behavior for CustomResources.
@ -223,3 +225,60 @@ 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(apiextensionsfeatures.CRDValidationRatcheting) && (oldCRD == nil || (oldCRD != nil && !specHasOptionalOldSelf(&oldCRD.Spec))) {
if newCRD.Spec.Validation != nil {
dropOptionalOldSelfField(newCRD.Spec.Validation.OpenAPIV3Schema)
}
for _, v := range newCRD.Spec.Versions {
if v.Schema != nil {
dropOptionalOldSelfField(v.Schema.OpenAPIV3Schema)
}
}
}
}
// dropOptionalOldSelfField drops field optionalOldSelf from CRD schema
func dropOptionalOldSelfField(schema *apiextensions.JSONSchemaProps) {
if schema == nil {
return
}
for i := range schema.XValidations {
schema.XValidations[i].OptionalOldSelf = nil
}
if schema.AdditionalProperties != nil {
dropOptionalOldSelfField(schema.AdditionalProperties.Schema)
}
for def, jsonSchema := range schema.Properties {
dropOptionalOldSelfField(&jsonSchema)
schema.Properties[def] = jsonSchema
}
if schema.Items != nil {
dropOptionalOldSelfField(schema.Items.Schema)
for i, jsonSchema := range schema.Items.JSONSchemas {
dropOptionalOldSelfField(&jsonSchema)
schema.Items.JSONSchemas[i] = jsonSchema
}
}
}
func specHasOptionalOldSelf(spec *apiextensions.CustomResourceDefinitionSpec) bool {
return validation.HasSchemaWith(spec, schemaHasOptionalOldSelf)
}
func schemaHasOptionalOldSelf(s *apiextensions.JSONSchemaProps) bool {
return validation.SchemaHas(s, func(s *apiextensions.JSONSchemaProps) bool {
for _, v := range s.XValidations {
if v.OptionalOldSelf != nil {
return true
}
}
return false
})
}

View File

@ -20,12 +20,17 @@ import (
"context"
"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"
apiextensionsfeatures "k8s.io/apiextensions-apiserver/pkg/features"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/validation/field"
utilfeature "k8s.io/apiserver/pkg/util/feature"
featuregatetesting "k8s.io/component-base/featuregate/testing"
"k8s.io/utils/pointer"
"k8s.io/utils/ptr"
)
func strPtr(in string) *string {
@ -189,3 +194,521 @@ 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
enableRatcheting bool
crd *apiextensions.CustomResourceDefinition
oldCRD *apiextensions.CustomResourceDefinition
expectedCRD *apiextensions.CustomResourceDefinition
}{
{
name: "Ratcheting, For creation, FG disabled, no OptionalOldSelf, no field drop",
enableRatcheting: false,
crd: &apiextensions.CustomResourceDefinition{},
oldCRD: nil,
expectedCRD: &apiextensions.CustomResourceDefinition{},
},
{
name: "Ratcheting, For creation, FG disabled, set OptionalOldSelf, drop OptionalOldSelf",
enableRatcheting: 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.",
OptionalOldSelf: ptr.To(true),
},
},
Properties: map[string]apiextensions.JSONSchemaProps{
"subRule": {
Type: "object",
XValidations: apiextensions.ValidationRules{
{
Rule: "isTest == true",
Message: "isTest should be true.",
OptionalOldSelf: ptr.To(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.",
},
},
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: "Ratcheting, For creation, FG enabled, set OptionalOldSelf, update with OptionalOldSelf",
enableRatcheting: 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.",
OptionalOldSelf: ptr.To(true),
},
},
Properties: map[string]apiextensions.JSONSchemaProps{
"subRule": {
Type: "object",
XValidations: apiextensions.ValidationRules{
{
Rule: "isTest == true",
Message: "isTest should be true.",
OptionalOldSelf: ptr.To(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.",
OptionalOldSelf: ptr.To(true),
},
},
Properties: map[string]apiextensions.JSONSchemaProps{
"subRule": {
Type: "object",
XValidations: apiextensions.ValidationRules{
{
Rule: "isTest == true",
Message: "isTest should be true.",
OptionalOldSelf: ptr.To(true),
},
},
Properties: map[string]apiextensions.JSONSchemaProps{
"isTest": {
Type: "boolean",
},
},
},
},
},
},
},
},
},
{
name: "Ratcheting, For update, FG disabled, oldCRD OptionalOldSelf in use, don't drop OptionalOldSelfs",
enableRatcheting: 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.",
OptionalOldSelf: ptr.To(true),
},
},
Properties: map[string]apiextensions.JSONSchemaProps{
"subRule": {
Type: "object",
XValidations: apiextensions.ValidationRules{
{
Rule: "isTest == true",
Message: "isTest should be true.",
OptionalOldSelf: ptr.To(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",
Properties: map[string]apiextensions.JSONSchemaProps{
"otherRule": {
Type: "object",
XValidations: apiextensions.ValidationRules{
{
Rule: "self.isTest == true",
Message: "isTest should be true.",
OptionalOldSelf: ptr.To(true),
},
},
},
},
},
},
},
},
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.",
OptionalOldSelf: ptr.To(true),
},
},
Properties: map[string]apiextensions.JSONSchemaProps{
"subRule": {
Type: "object",
XValidations: apiextensions.ValidationRules{
{
Rule: "isTest == true",
Message: "isTest should be true.",
OptionalOldSelf: ptr.To(true),
},
},
Properties: map[string]apiextensions.JSONSchemaProps{
"isTest": {
Type: "boolean",
},
},
},
},
},
},
},
},
},
{
name: "Ratcheting, For update, FG disabled, oldCRD OptionalOldSelf in use, but different from new, don't drop OptionalOldSelfs",
enableRatcheting: 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.",
OptionalOldSelf: ptr.To(true),
},
},
Properties: map[string]apiextensions.JSONSchemaProps{
"subRule": {
Type: "object",
XValidations: apiextensions.ValidationRules{
{
Rule: "isTest == true",
Message: "isTest should be true.",
OptionalOldSelf: ptr.To(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",
Properties: map[string]apiextensions.JSONSchemaProps{
"subRule": {
Type: "object",
XValidations: apiextensions.ValidationRules{
{
Rule: "isTest == true",
Message: "isTest should be true.",
OptionalOldSelf: ptr.To(true),
},
},
},
},
},
},
},
},
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.",
OptionalOldSelf: ptr.To(true),
},
},
Properties: map[string]apiextensions.JSONSchemaProps{
"subRule": {
Type: "object",
XValidations: apiextensions.ValidationRules{
{
Rule: "isTest == true",
Message: "isTest should be true.",
OptionalOldSelf: ptr.To(true),
},
},
Properties: map[string]apiextensions.JSONSchemaProps{
"isTest": {
Type: "boolean",
},
},
},
},
},
},
},
},
},
{
name: "Ratcheting, For update, FG disabled, oldCRD has no OptionalOldSelf, drop OptionalOldSelf",
enableRatcheting: 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.",
OptionalOldSelf: ptr.To(true),
},
},
},
},
},
},
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.",
},
},
},
},
},
},
},
{
name: "Ratcheting, For update, FG enabled, oldCRD has optionalOldSelf, updated to newCRD",
enableRatcheting: 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.",
OptionalOldSelf: ptr.To(true),
},
},
},
},
},
},
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",
OptionalOldSelf: ptr.To(true),
},
},
},
},
},
},
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.",
OptionalOldSelf: ptr.To(true),
},
},
},
},
},
},
},
{
name: "Ratcheting, For update, FG enabled, oldCRD has no OptionalOldSelf, updated to newCRD",
enableRatcheting: 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.",
OptionalOldSelf: ptr.To(true),
},
},
},
},
},
},
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.",
OptionalOldSelf: ptr.To(true),
},
},
},
},
},
},
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, apiextensionsfeatures.CRDValidationRatcheting, tc.enableRatcheting)()
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\n%v", tc.oldCRD, old, diff)
}
if diff := cmp.Diff(tc.expectedCRD, tc.crd); diff != "" {
t.Fatalf("unexpected crd: %v\n%v", tc.crd, diff)
}
})
}
}