diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 65612fc279ef9..a55791b0d9fd7 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -17,6 +17,7 @@ limitations under the License. package features import ( + apiextensionsfeatures "k8s.io/apiextensions-apiserver/pkg/features" "k8s.io/apimachinery/pkg/util/runtime" genericfeatures "k8s.io/apiserver/pkg/features" utilfeature "k8s.io/apiserver/pkg/util/feature" @@ -1191,6 +1192,11 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS genericfeatures.ServerSideFieldValidation: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.29 + // inherited features from apiextensions-apiserver, relisted here to get a conflict if it is changed + // unintentionally on either side: + + apiextensionsfeatures.CRDValidationRatcheting: {Default: false, PreRelease: featuregate.Alpha}, + // features that enable backwards compatibility but are scheduled to be removed // ... HPAScaleToZero: {Default: false, PreRelease: featuregate.Alpha}, diff --git a/staging/src/k8s.io/apiextensions-apiserver/go.mod b/staging/src/k8s.io/apiextensions-apiserver/go.mod index e79e7c3b66207..7013abf8ed1d6 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/go.mod +++ b/staging/src/k8s.io/apiextensions-apiserver/go.mod @@ -6,6 +6,7 @@ go 1.20 require ( github.com/emicklei/go-restful/v3 v3.9.0 + github.com/evanphx/json-patch v5.6.0+incompatible github.com/gogo/protobuf v1.3.2 github.com/google/cel-go v0.16.0 github.com/google/gnostic-models v0.6.8 @@ -49,7 +50,6 @@ require ( github.com/coreos/go-systemd/v22 v22.5.0 // indirect github.com/davecgh/go-spew v1.1.1 // indirect github.com/dustin/go-humanize v1.0.1 // indirect - github.com/evanphx/json-patch v5.6.0+incompatible // indirect github.com/felixge/httpsnoop v1.0.3 // indirect github.com/fsnotify/fsnotify v1.6.0 // indirect github.com/go-logr/logr v1.2.4 // indirect 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 95876883aef22..419ea25e84f61 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 @@ -825,7 +825,7 @@ func validateCustomResourceDefinitionValidation(ctx context.Context, customResou // if validation passed otherwise, make sure we can actually construct a schema validator from this custom resource validation. if len(allErrs) == 0 { - if _, _, err := apiservervalidation.NewSchemaValidator(customResourceValidation); err != nil { + if _, _, err := apiservervalidation.NewSchemaValidator(customResourceValidation.OpenAPIV3Schema); err != nil { allErrs = append(allErrs, field.Invalid(fldPath, "", fmt.Sprintf("error building validator: %v", err))) } } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go index a2e4b7a28c7ce..2a6dcb80a7c75 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go @@ -79,8 +79,6 @@ import ( "k8s.io/klog/v2" "k8s.io/kube-openapi/pkg/spec3" "k8s.io/kube-openapi/pkg/validation/spec" - "k8s.io/kube-openapi/pkg/validation/strfmt" - "k8s.io/kube-openapi/pkg/validation/validate" ) // crdHandler serves the `/apis` endpoint. @@ -739,20 +737,22 @@ func (r *crdHandler) getOrCreateServingInfoFor(uid types.UID, name string) (*crd utilruntime.HandleError(err) return nil, fmt.Errorf("the server could not properly serve the CR schema") } + var internalSchemaProps *apiextensionsinternal.JSONSchemaProps var internalValidationSchema *apiextensionsinternal.CustomResourceValidation if validationSchema != nil { internalValidationSchema = &apiextensionsinternal.CustomResourceValidation{} if err := apiextensionsv1.Convert_v1_CustomResourceValidation_To_apiextensions_CustomResourceValidation(validationSchema, internalValidationSchema, nil); err != nil { return nil, fmt.Errorf("failed to convert CRD validation to internal version: %v", err) } + internalSchemaProps = internalValidationSchema.OpenAPIV3Schema } - validator, _, err := apiservervalidation.NewSchemaValidator(internalValidationSchema) + validator, _, err := apiservervalidation.NewSchemaValidator(internalSchemaProps) if err != nil { return nil, err } var statusSpec *apiextensionsinternal.CustomResourceSubresourceStatus - var statusValidator *validate.SchemaValidator + var statusValidator apiservervalidation.SchemaValidator subresources, err := apiextensionshelpers.GetSubresourcesForVersion(crd, v.Name) if err != nil { utilruntime.HandleError(err) @@ -767,11 +767,10 @@ func (r *crdHandler) getOrCreateServingInfoFor(uid types.UID, name string) (*crd // for the status subresource, validate only against the status schema if internalValidationSchema != nil && internalValidationSchema.OpenAPIV3Schema != nil && internalValidationSchema.OpenAPIV3Schema.Properties != nil { if statusSchema, ok := internalValidationSchema.OpenAPIV3Schema.Properties["status"]; ok { - openapiSchema := &spec.Schema{} - if err := apiservervalidation.ConvertJSONSchemaPropsWithPostProcess(&statusSchema, openapiSchema, apiservervalidation.StripUnsupportedFormatsPostProcess); err != nil { + statusValidator, _, err = apiservervalidation.NewSchemaValidator(&statusSchema) + if err != nil { return nil, err } - statusValidator = validate.NewSchemaValidator(openapiSchema, nil, "", strfmt.Default) } } } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/ratcheting.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/ratcheting.go new file mode 100644 index 0000000000000..6565d83eee53c --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/ratcheting.go @@ -0,0 +1,412 @@ +/* +Copyright 2023 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 validation + +import ( + "reflect" + + "k8s.io/apiserver/pkg/cel/common" + celopenapi "k8s.io/apiserver/pkg/cel/openapi" + "k8s.io/kube-openapi/pkg/validation/spec" + "k8s.io/kube-openapi/pkg/validation/strfmt" + "k8s.io/kube-openapi/pkg/validation/validate" +) + +// schemaArgs are the arguments to constructor for OpenAPI schema validator, +// NewSchemaValidator +type schemaArgs struct { + schema *spec.Schema + root interface{} + path string + knownFormats strfmt.Registry + options []validate.Option +} + +// RatchetingSchemaValidator wraps kube-openapis SchemaValidator to provide a +// ValidateUpdate function which allows ratcheting +type RatchetingSchemaValidator struct { + schemaArgs +} + +func NewRatchetingSchemaValidator(schema *spec.Schema, rootSchema interface{}, root string, formats strfmt.Registry, options ...validate.Option) *RatchetingSchemaValidator { + return &RatchetingSchemaValidator{ + schemaArgs: schemaArgs{ + schema: schema, + root: rootSchema, + path: root, + knownFormats: formats, + options: options, + }, + } +} + +func (r *RatchetingSchemaValidator) Validate(new interface{}) *validate.Result { + sv := validate.NewSchemaValidator(r.schema, r.root, r.path, r.knownFormats, r.options...) + return sv.Validate(new) +} + +func (r *RatchetingSchemaValidator) ValidateUpdate(new, old interface{}) *validate.Result { + return newRatchetingValueValidator(new, old, r.schemaArgs).Validate() +} + +// ratchetingValueValidator represents an invocation of SchemaValidator.ValidateUpdate +// for specific arguments for `old` and `new` +// +// It follows the openapi SchemaValidator down its traversal of the new value +// by injecting validate.Option into each recursive invocation. +// +// A ratchetingValueValidator will be constructed and added to the tree for +// each explored sub-index and sub-property during validation. +// +// It's main job is to keep the old/new values correlated as the traversal +// continues, and postprocess errors according to our ratcheting policy. +// +// ratchetingValueValidator is not thread safe. +type ratchetingValueValidator struct { + // schemaArgs provides the arguments to use in the temporary SchemaValidator + // that is created during a call to Validate. + schemaArgs + + // Currently correlated old value during traversal of the schema/object + oldValue interface{} + + // Value being validated + value interface{} + + // Scratch space below, may change during validation + + // Cached comparison result of DeepEqual of `value` and `thunk.oldValue` + comparisonResult *bool + + // Cached map representation of a map-type list, or nil if not map-type list + mapList common.MapList + + // Children spawned by a call to `Validate` on this object + // key is either a string or an index, depending upon whether `value` is + // a map or a list, respectively. + // + // The list of children may be incomplete depending upon if the internal + // logic of kube-openapi's SchemaValidator short-circuited before + // reaching all of the children. + // + // It should be expected to have an entry for either all of the children, or + // none of them. + children map[interface{}]*ratchetingValueValidator +} + +func newRatchetingValueValidator(newValue, oldValue interface{}, args schemaArgs) *ratchetingValueValidator { + return &ratchetingValueValidator{ + oldValue: oldValue, + value: newValue, + schemaArgs: args, + children: map[interface{}]*ratchetingValueValidator{}, + } +} + +// getValidateOption provides a kube-openapi validate.Option for SchemaValidator +// that injects a ratchetingValueValidator to be used for all subkeys and subindices +func (r *ratchetingValueValidator) getValidateOption() validate.Option { + return func(svo *validate.SchemaValidatorOptions) { + svo.NewValidatorForField = func(field string, schema *spec.Schema, rootSchema interface{}, root string, formats strfmt.Registry, opts ...validate.Option) validate.ValueValidator { + return r.SubPropertyValidator(field, schema, rootSchema, root, formats, opts...) + } + svo.NewValidatorForIndex = func(index int, schema *spec.Schema, rootSchema interface{}, root string, formats strfmt.Registry, opts ...validate.Option) validate.ValueValidator { + return r.SubIndexValidator(index, schema, rootSchema, root, formats, opts...) + } + } +} + +// Validate validates the update from r.oldValue to r.value +// +// During evaluation, a temporary tree of ratchetingValueValidator is built for all +// traversed field paths. It is necessary to build the tree to take advantage of +// DeepEqual checks performed by lower levels of the object during validation without +// greatly modifying `kube-openapi`'s implementation. +// +// The tree, and all cache storage/scratch space for the validation of a single +// call to `Validate` is thrown away at the end of the top-level call +// to `Validate`. +// +// `Validate` will create a node in the tree to for each of the explored children. +// The node's main purpose is to store a lazily computed DeepEqual check between +// the oldValue and the currently passed value. If the check is performed, it +// will be stored in the node to be re-used by a parent node during a DeepEqual +// comparison, if necessary. +// +// This call has a side-effect of populating it's `children` variable with +// the explored nodes of the object tree. +func (r *ratchetingValueValidator) Validate() *validate.Result { + opts := append([]validate.Option{ + r.getValidateOption(), + }, r.options...) + + s := validate.NewSchemaValidator(r.schema, r.root, r.path, r.knownFormats, opts...) + + res := s.Validate(r.value) + + if res.IsValid() { + return res + } + + // Current ratcheting rule is to ratchet errors if DeepEqual(old, new) is true. + if r.CachedDeepEqual() { + newRes := &validate.Result{} + newRes.MergeAsWarnings(res) + return newRes + } + + return res +} + +// SubPropertyValidator overrides the standard validator constructor for sub-properties by +// returning our special ratcheting variant. +// +// If we can correlate an old value, we return a ratcheting validator to +// use for the child. +// +// If the old value cannot be correlated, then default validation is used. +func (r *ratchetingValueValidator) SubPropertyValidator(field string, schema *spec.Schema, rootSchema interface{}, root string, formats strfmt.Registry, options ...validate.Option) validate.ValueValidator { + // Find correlated old value + asMap, ok := r.oldValue.(map[string]interface{}) + if !ok { + return validate.NewSchemaValidator(schema, rootSchema, root, formats, options...) + } + + oldValueForField, ok := asMap[field] + if !ok { + return validate.NewSchemaValidator(schema, rootSchema, root, formats, options...) + } + + return inlineValidator(func(new interface{}) *validate.Result { + childNode := newRatchetingValueValidator(new, oldValueForField, schemaArgs{ + schema: schema, + root: rootSchema, + path: root, + knownFormats: formats, + options: options, + }) + + r.children[field] = childNode + return childNode.Validate() + }) + +} + +// SubIndexValidator overrides the standard validator constructor for sub-indicies by +// returning our special ratcheting variant. +// +// If we can correlate an old value, we return a ratcheting validator to +// use for the child. +// +// If the old value cannot be correlated, then default validation is used. +func (r *ratchetingValueValidator) SubIndexValidator(index int, schema *spec.Schema, rootSchema interface{}, root string, formats strfmt.Registry, options ...validate.Option) validate.ValueValidator { + oldValueForIndex := r.correlateOldValueForChildAtNewIndex(index) + if oldValueForIndex == nil { + // If correlation fails, default to non-ratcheting logic + return validate.NewSchemaValidator(schema, rootSchema, root, formats, options...) + } + + return inlineValidator(func(new interface{}) *validate.Result { + childNode := newRatchetingValueValidator(new, oldValueForIndex, schemaArgs{ + schema: schema, + root: rootSchema, + path: root, + knownFormats: formats, + options: options, + }) + + r.children[index] = childNode + return childNode.Validate() + }) +} + +// If oldValue is not a list, returns nil +// If oldValue is a list takes mapType into account and attempts to find the +// old value with the same index or key, depending upon the mapType. +// +// If listType is map, creates a map representation of the list using the designated +// map-keys and caches it for future calls. +func (r *ratchetingValueValidator) correlateOldValueForChildAtNewIndex(index int) any { + oldAsList, ok := r.oldValue.([]interface{}) + if !ok { + return nil + } + + asList, ok := r.value.([]interface{}) + if !ok { + return nil + } else if len(asList) <= index { + // Cannot correlate out of bounds index + return nil + } + + listType, _ := r.schema.Extensions.GetString("x-kubernetes-list-type") + switch listType { + case "map": + // Look up keys for this index in current object + currentElement := asList[index] + + oldList := r.mapList + if oldList == nil { + oldList = celopenapi.MakeMapList(r.schema, oldAsList) + r.mapList = oldList + } + return oldList.Get(currentElement) + + case "set": + // Are sets correlatable? Only if the old value equals the current value. + // We might be able to support this, but do not currently see a lot + // of value + // (would allow you to add/remove items from sets with ratcheting but not change them) + return nil + case "atomic": + // Atomic lists are not correlatable by item + // Ratcheting is not available on a per-index basis + return nil + default: + // Correlate by-index by default. + // + // Cannot correlate an out-of-bounds index + if len(oldAsList) <= index { + return nil + } + + return oldAsList[index] + } +} + +// CachedDeepEqual is equivalent to reflect.DeepEqual, but caches the +// results in the tree of ratchetInvocationScratch objects on the way: +// +// For objects and arrays, this function will make a best effort to make +// use of past DeepEqual checks performed by this Node's children, if available. +// +// If a lazy computation could not be found for all children possibly due +// to validation logic short circuiting and skipping the children, then +// this function simply defers to reflect.DeepEqual. +func (r *ratchetingValueValidator) CachedDeepEqual() (res bool) { + if r.comparisonResult != nil { + return *r.comparisonResult + } + + defer func() { + r.comparisonResult = &res + }() + + if r.value == nil && r.oldValue == nil { + return true + } else if r.value == nil || r.oldValue == nil { + return false + } + + oldAsArray, oldIsArray := r.oldValue.([]interface{}) + newAsArray, newIsArray := r.value.([]interface{}) + + if oldIsArray != newIsArray { + return false + } else if oldIsArray { + if len(oldAsArray) != len(newAsArray) { + return false + } else if len(r.children) != len(oldAsArray) { + // kube-openapi validator is written to always visit all + // children of a slice, so this case is only possible if + // one of the children could not be correlated. In that case, + // we know the objects are not equal. + // + return false + } + + // Correctly considers map-type lists due to fact that index here + // is only used for numbering. The correlation is stored in the + // childInvocation itself + // + // NOTE: This does not consider sets, since we don't correlate them. + for i := range newAsArray { + // Query for child + child, ok := r.children[i] + if !ok { + // This should not happen + return false + } else if !child.CachedDeepEqual() { + // If one child is not equal the entire object is not equal + return false + } + } + + return true + } + + oldAsMap, oldIsMap := r.oldValue.(map[string]interface{}) + newAsMap, newIsMap := r.value.(map[string]interface{}) + + if oldIsMap != newIsMap { + return false + } else if oldIsMap { + if len(oldAsMap) != len(newAsMap) { + return false + } else if len(oldAsMap) == 0 && len(newAsMap) == 0 { + // Both empty + return true + } else if len(r.children) != len(oldAsMap) { + // If we are missing a key it is because the old value could not + // be correlated to the new, so the objects are not equal. + // + return false + } + + for k := range oldAsMap { + // Check to see if this child was explored during validation + child, ok := r.children[k] + if !ok { + // Child from old missing in new due to key change + // Objects are not equal. + return false + } else if !child.CachedDeepEqual() { + // If one child is not equal the entire object is not equal + return false + } + } + + return true + } + + return reflect.DeepEqual(r.oldValue, r.value) +} + +// A validator which just calls a validate function, and advertises that it +// validates anything +// +// In the future kube-openapi's ValueValidator interface can be simplified +// to be closer to `currentValidator.Options.NewValidator(value, ...).Validate()` +// so that a tree of "validation nodes" can be more formally encoded in the API. +// In that case this class would not be necessary. +type inlineValidator func(new interface{}) *validate.Result + +var _ validate.ValueValidator = inlineValidator(nil) + +func (f inlineValidator) Validate(new interface{}) *validate.Result { + return f(new) +} + +func (f inlineValidator) SetPath(path string) { + // Do nothing + // Unused by kube-openapi +} + +func (f inlineValidator) Applies(source interface{}, valueKind reflect.Kind) bool { + return true +} diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/ratcheting_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/ratcheting_test.go new file mode 100644 index 0000000000000..078bb548cdcb4 --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/ratcheting_test.go @@ -0,0 +1,136 @@ +/* +Copyright 2023 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 validation_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/apiextensions-apiserver/pkg/apiserver/validation" + "k8s.io/kube-openapi/pkg/validation/spec" + "k8s.io/kube-openapi/pkg/validation/strfmt" +) + +var zeroIntSchema *spec.Schema = &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Type: spec.StringOrArray{"number"}, + Minimum: ptr(float64(0)), + Maximum: ptr(float64(0)), + }, +} + +var smallIntSchema *spec.Schema = &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Type: spec.StringOrArray{"number"}, + Maximum: ptr(float64(50)), + }, +} + +var mediumIntSchema *spec.Schema = &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Type: spec.StringOrArray{"number"}, + Minimum: ptr(float64(50)), + Maximum: ptr(float64(10000)), + }, +} + +var largeIntSchema *spec.Schema = &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Type: spec.StringOrArray{"number"}, + Minimum: ptr(float64(10000)), + }, +} + +func TestScalarRatcheting(t *testing.T) { + validator := validation.NewRatchetingSchemaValidator(mediumIntSchema, nil, "", strfmt.Default) + require.True(t, validator.ValidateUpdate(1, 1).IsValid()) + require.False(t, validator.ValidateUpdate(1, 2).IsValid()) +} + +var objectSchema *spec.Schema = &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Type: spec.StringOrArray{"object"}, + Properties: map[string]spec.Schema{ + "zero": *zeroIntSchema, + "small": *smallIntSchema, + "medium": *mediumIntSchema, + "large": *largeIntSchema, + }, + }, +} + +var objectObjectSchema *spec.Schema = &spec.Schema{ + SchemaProps: spec.SchemaProps{ + Type: spec.StringOrArray{"object"}, + Properties: map[string]spec.Schema{ + "nested": *objectSchema, + }, + }, +} + +// Shows scalar fields of objects can be ratcheted +func TestObjectScalarFieldsRatcheting(t *testing.T) { + validator := validation.NewRatchetingSchemaValidator(objectSchema, nil, "", strfmt.Default) + assert.True(t, validator.ValidateUpdate(map[string]interface{}{ + "small": 500, + }, map[string]interface{}{ + "small": 500, + }).IsValid()) + assert.True(t, validator.ValidateUpdate(map[string]interface{}{ + "small": 501, + }, map[string]interface{}{ + "small": 501, + "medium": 500, + }).IsValid()) + assert.False(t, validator.ValidateUpdate(map[string]interface{}{ + "small": 500, + }, map[string]interface{}{ + "small": 501, + }).IsValid()) +} + +// Shows schemas with object fields which themselves are ratcheted can be ratcheted +func TestObjectObjectFieldsRatcheting(t *testing.T) { + validator := validation.NewRatchetingSchemaValidator(objectObjectSchema, nil, "", strfmt.Default) + assert.True(t, validator.ValidateUpdate(map[string]interface{}{ + "nested": map[string]interface{}{ + "small": 500, + }}, map[string]interface{}{ + "nested": map[string]interface{}{ + "small": 500, + }}).IsValid()) + assert.True(t, validator.ValidateUpdate(map[string]interface{}{ + "nested": map[string]interface{}{ + "small": 501, + }}, map[string]interface{}{ + "nested": map[string]interface{}{ + "small": 501, + "medium": 500, + }}).IsValid()) + assert.False(t, validator.ValidateUpdate(map[string]interface{}{ + "nested": map[string]interface{}{ + "small": 500, + }}, map[string]interface{}{ + "nested": map[string]interface{}{ + "small": 501, + }}).IsValid()) +} + +func ptr[T any](v T) *T { + return &v +} 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 fad044d1c6bf6..e0042356ac0c4 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 @@ -22,29 +22,83 @@ import ( "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apiextensions-apiserver/pkg/features" "k8s.io/apimachinery/pkg/util/validation/field" + utilfeature "k8s.io/apiserver/pkg/util/feature" openapierrors "k8s.io/kube-openapi/pkg/validation/errors" "k8s.io/kube-openapi/pkg/validation/spec" "k8s.io/kube-openapi/pkg/validation/strfmt" "k8s.io/kube-openapi/pkg/validation/validate" ) +type SchemaValidator interface { + SchemaCreateValidator + ValidateUpdate(new, old interface{}) *validate.Result +} + +type SchemaCreateValidator interface { + Validate(value interface{}) *validate.Result +} + +// basicSchemaValidator wraps a kube-openapi SchemaCreateValidator to +// support ValidateUpdate. It implements ValidateUpdate by simply validating +// the new value via kube-openapi, ignoring the old value. +type basicSchemaValidator struct { + *validate.SchemaValidator +} + +func (s basicSchemaValidator) ValidateUpdate(new, old interface{}) *validate.Result { + return s.Validate(new) +} + // NewSchemaValidator creates an openapi schema validator for the given CRD validation. -func NewSchemaValidator(customResourceValidation *apiextensions.CustomResourceValidation) (*validate.SchemaValidator, *spec.Schema, error) { +// +// If feature `CRDValidationRatcheting` is disabled, this returns validator which +// validates all `Update`s and `Create`s as a `Create` - without considering old value. +// +// If feature `CRDValidationRatcheting` is enabled - the validator returned +// will support ratcheting unchanged correlatable fields across an update. +func NewSchemaValidator(customResourceValidation *apiextensions.JSONSchemaProps) (SchemaValidator, *spec.Schema, error) { // Convert CRD schema to openapi schema openapiSchema := &spec.Schema{} if customResourceValidation != nil { // TODO: replace with NewStructural(...).ToGoOpenAPI - if err := ConvertJSONSchemaPropsWithPostProcess(customResourceValidation.OpenAPIV3Schema, openapiSchema, StripUnsupportedFormatsPostProcess); err != nil { + if err := ConvertJSONSchemaPropsWithPostProcess(customResourceValidation, openapiSchema, StripUnsupportedFormatsPostProcess); err != nil { return nil, nil, err } } - return validate.NewSchemaValidator(openapiSchema, nil, "", strfmt.Default), openapiSchema, nil + + if utilfeature.DefaultFeatureGate.Enabled(features.CRDValidationRatcheting) { + return NewRatchetingSchemaValidator(openapiSchema, nil, "", strfmt.Default), openapiSchema, nil + } + return basicSchemaValidator{validate.NewSchemaValidator(openapiSchema, nil, "", strfmt.Default)}, openapiSchema, nil +} + +// ValidateCustomResourceUpdate validates the transition of Custom Resource from +// `old` to `new` against the schema in the CustomResourceDefinition. +// Both customResource and old represent a JSON data structures. +// +// If feature `CRDValidationRatcheting` is disabled, this behaves identically to +// ValidateCustomResource(customResource). +func ValidateCustomResourceUpdate(fldPath *field.Path, customResource, old interface{}, validator SchemaValidator) field.ErrorList { + // Additional feature gate check for sanity + if !utilfeature.DefaultFeatureGate.Enabled(features.CRDValidationRatcheting) { + return ValidateCustomResource(nil, customResource, validator) + } else if validator == nil { + return nil + } + + result := validator.ValidateUpdate(customResource, old) + if result.IsValid() { + return nil + } + + return kubeOpenAPIResultToFieldErrors(fldPath, result) } // ValidateCustomResource validates the Custom Resource against the schema in the CustomResourceDefinition. // CustomResource is a JSON data structure. -func ValidateCustomResource(fldPath *field.Path, customResource interface{}, validator *validate.SchemaValidator) field.ErrorList { +func ValidateCustomResource(fldPath *field.Path, customResource interface{}, validator SchemaCreateValidator) field.ErrorList { if validator == nil { return nil } @@ -53,6 +107,11 @@ func ValidateCustomResource(fldPath *field.Path, customResource interface{}, val if result.IsValid() { return nil } + + return kubeOpenAPIResultToFieldErrors(fldPath, result) +} + +func kubeOpenAPIResultToFieldErrors(fldPath *field.Path, result *validate.Result) field.ErrorList { var allErrs field.ErrorList for _, err := range result.Errors { switch err := err.(type) { @@ -287,7 +346,7 @@ func ConvertJSONSchemaPropsWithPostProcess(in *apiextensions.JSONSchemaProps, ou out.VendorExtensible.AddExtension("x-kubernetes-embedded-resource", true) } if len(in.XListMapKeys) != 0 { - out.VendorExtensible.AddExtension("x-kubernetes-list-map-keys", in.XListMapKeys) + out.VendorExtensible.AddExtension("x-kubernetes-list-map-keys", convertSliceToInterfaceSlice(in.XListMapKeys)) } if in.XListType != nil { out.VendorExtensible.AddExtension("x-kubernetes-list-type", *in.XListType) @@ -300,11 +359,19 @@ func ConvertJSONSchemaPropsWithPostProcess(in *apiextensions.JSONSchemaProps, ou if err := apiextensionsv1.Convert_apiextensions_ValidationRules_To_v1_ValidationRules(&in.XValidations, &serializationValidationRules, nil); err != nil { return err } - out.VendorExtensible.AddExtension("x-kubernetes-validations", serializationValidationRules) + out.VendorExtensible.AddExtension("x-kubernetes-validations", convertSliceToInterfaceSlice(serializationValidationRules)) } return nil } +func convertSliceToInterfaceSlice[T any](in []T) []interface{} { + var res []interface{} + for _, v := range in { + res = append(res, v) + } + return res +} + func convertSliceOfJSONSchemaProps(in *[]apiextensions.JSONSchemaProps, out *[]spec.Schema, postProcess PostProcessFunc) error { if in != nil { for _, jsonSchemaProps := range *in { 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 7bbeba3eae541..c6776ebf6a9d0 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 @@ -601,7 +601,7 @@ func TestValidateCustomResource(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - validator, _, err := NewSchemaValidator(&apiextensions.CustomResourceValidation{OpenAPIV3Schema: &tt.schema}) + validator, _, err := NewSchemaValidator(&tt.schema) if err != nil { t.Fatal(err) } @@ -689,7 +689,7 @@ func TestItemsProperty(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - validator, _, err := NewSchemaValidator(&apiextensions.CustomResourceValidation{OpenAPIV3Schema: &tt.args.schema}) + validator, _, err := NewSchemaValidator(&tt.args.schema) if err != nil { t.Fatal(err) } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/features/kube_features.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/features/kube_features.go index a5932281367c3..1844ed8d1eb49 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/features/kube_features.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/features/kube_features.go @@ -22,11 +22,18 @@ import ( ) const ( -// Every feature gate should add method here following this template: -// -// // owner: @username -// // alpha: v1.4 -// MyFeature() bool + // Every feature gate should add method here following this template: + // + // // owner: @username + // // alpha: v1.4 + // MyFeature() bool + + // owner: @alexzielenski + // alpha: v1.28 + // + // Ignores errors raised on unchanged fields of Custom Resources + // across UPDATE/PATCH requests. + CRDValidationRatcheting featuregate.Feature = "CRDValidationRatcheting" ) func init() { @@ -36,4 +43,6 @@ func init() { // defaultKubernetesFeatureGates consists of all known Kubernetes-specific feature keys. // To add a new feature, define a key for it above and add it here. The features will be // available throughout Kubernetes binaries. -var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{} +var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ + CRDValidationRatcheting: {Default: false, PreRelease: featuregate.Alpha}, +} diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/strategy.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/strategy.go index 92a688a56d729..b534c627343ba 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/strategy.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/strategy.go @@ -19,13 +19,12 @@ package customresource import ( "context" - "k8s.io/kube-openapi/pkg/validation/validate" - "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema" "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel" structurallisttype "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/listtype" schemaobjectmeta "k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta" + "k8s.io/apiextensions-apiserver/pkg/apiserver/validation" apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -58,7 +57,7 @@ type customResourceStrategy struct { kind schema.GroupVersionKind } -func NewStrategy(typer runtime.ObjectTyper, namespaceScoped bool, kind schema.GroupVersionKind, schemaValidator, statusSchemaValidator *validate.SchemaValidator, structuralSchemas map[string]*structuralschema.Structural, status *apiextensions.CustomResourceSubresourceStatus, scale *apiextensions.CustomResourceSubresourceScale) customResourceStrategy { +func NewStrategy(typer runtime.ObjectTyper, namespaceScoped bool, kind schema.GroupVersionKind, schemaValidator, statusSchemaValidator validation.SchemaValidator, structuralSchemas map[string]*structuralschema.Structural, status *apiextensions.CustomResourceSubresourceStatus, scale *apiextensions.CustomResourceSubresourceScale) customResourceStrategy { celValidators := map[string]*cel.Validator{} if utilfeature.DefaultFeatureGate.Enabled(features.CustomResourceValidationExpressions) { for name, s := range structuralSchemas { diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/validator.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/validator.go index ae339013afe3f..a158aa3b6dcb1 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/validator.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/registry/customresource/validator.go @@ -28,17 +28,16 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/validation/field" - "k8s.io/kube-openapi/pkg/validation/validate" "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions" - apiservervalidation "k8s.io/apiextensions-apiserver/pkg/apiserver/validation" + apiextensionsvalidation "k8s.io/apiextensions-apiserver/pkg/apiserver/validation" ) type customResourceValidator struct { namespaceScoped bool kind schema.GroupVersionKind - schemaValidator *validate.SchemaValidator - statusSchemaValidator *validate.SchemaValidator + schemaValidator apiextensionsvalidation.SchemaValidator + statusSchemaValidator apiextensionsvalidation.SchemaValidator } func (a customResourceValidator) Validate(ctx context.Context, obj runtime.Object, scale *apiextensions.CustomResourceSubresourceScale) field.ErrorList { @@ -58,7 +57,7 @@ func (a customResourceValidator) Validate(ctx context.Context, obj runtime.Objec var allErrs field.ErrorList allErrs = append(allErrs, validation.ValidateObjectMetaAccessor(accessor, a.namespaceScoped, validation.NameIsDNSSubdomain, field.NewPath("metadata"))...) - allErrs = append(allErrs, apiservervalidation.ValidateCustomResource(nil, u.UnstructuredContent(), a.schemaValidator)...) + allErrs = append(allErrs, apiextensionsvalidation.ValidateCustomResource(nil, u.UnstructuredContent(), a.schemaValidator)...) allErrs = append(allErrs, a.ValidateScaleSpec(ctx, u, scale)...) allErrs = append(allErrs, a.ValidateScaleStatus(ctx, u, scale)...) @@ -70,6 +69,10 @@ func (a customResourceValidator) ValidateUpdate(ctx context.Context, obj, old ru if !ok { return field.ErrorList{field.Invalid(field.NewPath(""), u, fmt.Sprintf("has type %T. Must be a pointer to an Unstructured type", u))} } + oldU, ok := old.(*unstructured.Unstructured) + if !ok { + return field.ErrorList{field.Invalid(field.NewPath(""), old, fmt.Sprintf("has type %T. Must be a pointer to an Unstructured type", u))} + } objAccessor, err := meta.Accessor(obj) if err != nil { return field.ErrorList{field.Invalid(field.NewPath("metadata"), nil, err.Error())} @@ -86,7 +89,7 @@ func (a customResourceValidator) ValidateUpdate(ctx context.Context, obj, old ru var allErrs field.ErrorList allErrs = append(allErrs, validation.ValidateObjectMetaAccessorUpdate(objAccessor, oldAccessor, field.NewPath("metadata"))...) - allErrs = append(allErrs, apiservervalidation.ValidateCustomResource(nil, u.UnstructuredContent(), a.schemaValidator)...) + allErrs = append(allErrs, apiextensionsvalidation.ValidateCustomResourceUpdate(nil, u.UnstructuredContent(), oldU.UnstructuredContent(), a.schemaValidator)...) allErrs = append(allErrs, a.ValidateScaleSpec(ctx, u, scale)...) allErrs = append(allErrs, a.ValidateScaleStatus(ctx, u, scale)...) @@ -103,6 +106,10 @@ func (a customResourceValidator) ValidateStatusUpdate(ctx context.Context, obj, if !ok { return field.ErrorList{field.Invalid(field.NewPath(""), u, fmt.Sprintf("has type %T. Must be a pointer to an Unstructured type", u))} } + oldU, ok := old.(*unstructured.Unstructured) + if !ok { + return field.ErrorList{field.Invalid(field.NewPath(""), old, fmt.Sprintf("has type %T. Must be a pointer to an Unstructured type", u))} + } objAccessor, err := meta.Accessor(obj) if err != nil { return field.ErrorList{field.Invalid(field.NewPath("metadata"), nil, err.Error())} @@ -119,7 +126,7 @@ func (a customResourceValidator) ValidateStatusUpdate(ctx context.Context, obj, var allErrs field.ErrorList allErrs = append(allErrs, validation.ValidateObjectMetaAccessorUpdate(objAccessor, oldAccessor, field.NewPath("metadata"))...) - allErrs = append(allErrs, apiservervalidation.ValidateCustomResource(nil, u.UnstructuredContent(), a.schemaValidator)...) + allErrs = append(allErrs, apiextensionsvalidation.ValidateCustomResourceUpdate(nil, u.UnstructuredContent(), oldU, a.schemaValidator)...) allErrs = append(allErrs, a.ValidateScaleStatus(ctx, u, scale)...) return allErrs diff --git a/staging/src/k8s.io/apiextensions-apiserver/test/integration/ratcheting_test.go b/staging/src/k8s.io/apiextensions-apiserver/test/integration/ratcheting_test.go new file mode 100644 index 0000000000000..cc88e1d8114a5 --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/test/integration/ratcheting_test.go @@ -0,0 +1,1576 @@ +/* +Copyright 2023 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 integration_test + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "strings" + "testing" + "time" + + jsonpatch "github.com/evanphx/json-patch" + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" + "k8s.io/apiextensions-apiserver/pkg/features" + "k8s.io/apiextensions-apiserver/test/integration/fixtures" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/uuid" + "k8s.io/apimachinery/pkg/util/wait" + utilfeature "k8s.io/apiserver/pkg/util/feature" + "k8s.io/client-go/dynamic" + featuregatetesting "k8s.io/component-base/featuregate/testing" +) + +var stringSchema *apiextensionsv1.JSONSchemaProps = &apiextensionsv1.JSONSchemaProps{ + Type: "string", +} + +var stringMapSchema *apiextensionsv1.JSONSchemaProps = &apiextensionsv1.JSONSchemaProps{ + Type: "object", + AdditionalProperties: &apiextensionsv1.JSONSchemaPropsOrBool{ + Schema: stringSchema, + }, +} + +var numberSchema *apiextensionsv1.JSONSchemaProps = &apiextensionsv1.JSONSchemaProps{ + Type: "integer", +} + +var numbersMapSchema *apiextensionsv1.JSONSchemaProps = &apiextensionsv1.JSONSchemaProps{ + Type: "object", + AdditionalProperties: &apiextensionsv1.JSONSchemaPropsOrBool{ + Schema: numberSchema, + }, +} + +type ratchetingTestContext struct { + *testing.T + DynamicClient dynamic.Interface + APIExtensionsClient clientset.Interface +} + +type ratchetingTestOperation interface { + Do(ctx *ratchetingTestContext) error + Description() string +} + +type expectError struct { + op ratchetingTestOperation +} + +func (e expectError) Do(ctx *ratchetingTestContext) error { + err := e.op.Do(ctx) + if err != nil { + return nil + } + return errors.New("expected error") +} + +func (e expectError) Description() string { + return fmt.Sprintf("Expect Error: %v", e.op.Description()) +} + +// apiextensions-apiserver has discovery disabled, so hardcode this mapping +var fakeRESTMapper map[schema.GroupVersionResource]string = map[schema.GroupVersionResource]string{ + myCRDV1Beta1: "MyCoolCRD", +} + +type applyPatchOperation struct { + description string + gvr schema.GroupVersionResource + name string + patch map[string]interface{} +} + +func (a applyPatchOperation) Do(ctx *ratchetingTestContext) error { + // Lookup GVK from discovery + kind, ok := fakeRESTMapper[a.gvr] + if !ok { + return fmt.Errorf("no mapping found for Gvr %v, add entry to fakeRESTMapper", a.gvr) + } + + a.patch["kind"] = kind + a.patch["apiVersion"] = a.gvr.GroupVersion().String() + + if meta, ok := a.patch["metadata"]; ok { + mObj := meta.(map[string]interface{}) + mObj["name"] = a.name + mObj["namespace"] = "default" + } else { + a.patch["metadata"] = map[string]interface{}{ + "name": a.name, + "namespace": "default", + } + } + + _, err := ctx.DynamicClient.Resource(a.gvr).Namespace("default").Apply(context.TODO(), a.name, &unstructured.Unstructured{ + Object: a.patch, + }, metav1.ApplyOptions{ + FieldManager: "manager", + }) + + return err + +} + +func (a applyPatchOperation) Description() string { + return a.description +} + +// Replaces schema used for v1beta1 of crd +type updateMyCRDV1Beta1Schema struct { + newSchema *apiextensionsv1.JSONSchemaProps +} + +func (u updateMyCRDV1Beta1Schema) Do(ctx *ratchetingTestContext) error { + var myCRD *apiextensionsv1.CustomResourceDefinition + var err error = apierrors.NewConflict(schema.GroupResource{}, "", nil) + for apierrors.IsConflict(err) { + myCRD, err = ctx.APIExtensionsClient.ApiextensionsV1().CustomResourceDefinitions().Get(context.TODO(), myCRDV1Beta1.Resource+"."+myCRDV1Beta1.Group, metav1.GetOptions{}) + if err != nil { + return err + } + + // Insert a sentinel property that we can probe to detect when the + // schema takes effect + sch := u.newSchema.DeepCopy() + if sch.Properties == nil { + sch.Properties = map[string]apiextensionsv1.JSONSchemaProps{} + } + + uuidString := string(uuid.NewUUID()) + // UUID string is just hex separated by dashes, which is safe to + // throw into regex like this + pattern := "^" + uuidString + "$" + sentinelName := "__ratcheting_sentinel_field__" + sch.Properties[sentinelName] = apiextensionsv1.JSONSchemaProps{ + Type: "string", + Pattern: pattern, + + // Put MaxLength condition inside AllOf since the string_validator + // in kube-openapi short circuits upon seeing MaxLength, and we + // want both pattern and MaxLength errors + AllOf: []apiextensionsv1.JSONSchemaProps{ + { + MinLength: ptr((int64(1))), // 1 MinLength to prevent empty value from ever being admitted + MaxLength: ptr((int64(0))), // 0 MaxLength to prevent non-empty value from ever being admitted + }, + }, + } + + for _, v := range myCRD.Spec.Versions { + if v.Name != myCRDV1Beta1.Version { + continue + } + v.Schema.OpenAPIV3Schema = sch + } + + _, err = ctx.APIExtensionsClient.ApiextensionsV1().CustomResourceDefinitions().Update(context.TODO(), myCRD, metav1.UpdateOptions{ + FieldManager: "manager", + }) + if err != nil { + return err + } + + // Keep trying to create an invalid instance of the CRD until we + // get an error containing the ResourceVersion we are looking for + // + counter := 0 + return wait.PollUntilContextCancel(context.TODO(), 100*time.Millisecond, true, func(_ context.Context) (done bool, err error) { + counter += 1 + err = applyPatchOperation{ + gvr: myCRDV1Beta1, + name: "sentinel-resource", + patch: map[string]interface{}{ + // Just keep using different values + sentinelName: fmt.Sprintf("invalid %v %v", uuidString, counter), + }}.Do(ctx) + + if err == nil { + return false, errors.New("expected error when creating sentinel resource") + } + + // Check to see if the returned error message contains our + // unique string. UUID should be unique enough to just check + // simple existence in the error. + if strings.Contains(err.Error(), uuidString) { + return true, nil + } + + return false, nil + }) + } + return err +} + +func (u updateMyCRDV1Beta1Schema) Description() string { + return "Update CRD schema" +} + +type patchMyCRDV1Beta1Schema struct { + description string + patch map[string]interface{} +} + +func (p patchMyCRDV1Beta1Schema) Do(ctx *ratchetingTestContext) error { + var err error + patchJSON, err := json.Marshal(p.patch) + if err != nil { + return err + } + + myCRD, err := ctx.APIExtensionsClient.ApiextensionsV1().CustomResourceDefinitions().Get(context.TODO(), myCRDV1Beta1.Resource+"."+myCRDV1Beta1.Group, metav1.GetOptions{}) + if err != nil { + return err + } + + for _, v := range myCRD.Spec.Versions { + if v.Name != myCRDV1Beta1.Version { + continue + } + + jsonSchema, err := json.Marshal(v.Schema.OpenAPIV3Schema) + if err != nil { + return err + } + + merged, err := jsonpatch.MergePatch(jsonSchema, patchJSON) + if err != nil { + return err + } + + var parsed apiextensionsv1.JSONSchemaProps + if err := json.Unmarshal(merged, &parsed); err != nil { + return err + } + + return updateMyCRDV1Beta1Schema{ + newSchema: &parsed, + }.Do(ctx) + } + + return fmt.Errorf("could not find version %v in CRD %v", myCRDV1Beta1.Version, myCRD.Name) +} + +func (p patchMyCRDV1Beta1Schema) Description() string { + return p.description +} + +type ratchetingTestCase struct { + Name string + Disabled bool + Operations []ratchetingTestOperation +} + +func runTests(t *testing.T, cases []ratchetingTestCase) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.CRDValidationRatcheting, true)() + tearDown, apiExtensionClient, dynamicClient, err := fixtures.StartDefaultServerWithClients(t) + if err != nil { + t.Fatal(err) + } + defer tearDown() + + group := myCRDV1Beta1.Group + version := myCRDV1Beta1.Version + resource := myCRDV1Beta1.Resource + kind := fakeRESTMapper[myCRDV1Beta1] + + myCRD := &apiextensionsv1.CustomResourceDefinition{ + ObjectMeta: metav1.ObjectMeta{Name: resource + "." + group}, + Spec: apiextensionsv1.CustomResourceDefinitionSpec{ + Group: group, + Versions: []apiextensionsv1.CustomResourceDefinitionVersion{{ + Name: version, + Served: true, + Storage: true, + Schema: &apiextensionsv1.CustomResourceValidation{ + OpenAPIV3Schema: &apiextensionsv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensionsv1.JSONSchemaProps{ + "content": { + Type: "object", + AdditionalProperties: &apiextensionsv1.JSONSchemaPropsOrBool{ + Schema: &apiextensionsv1.JSONSchemaProps{ + Type: "string", + }, + }, + }, + "num": { + Type: "object", + AdditionalProperties: &apiextensionsv1.JSONSchemaPropsOrBool{ + Schema: &apiextensionsv1.JSONSchemaProps{ + Type: "integer", + }, + }, + }, + }, + }, + }, + }}, + Names: apiextensionsv1.CustomResourceDefinitionNames{ + Plural: resource, + Kind: kind, + ListKind: kind + "List", + }, + Scope: apiextensionsv1.NamespaceScoped, + }, + } + + _, err = fixtures.CreateNewV1CustomResourceDefinition(myCRD, apiExtensionClient, dynamicClient) + if err != nil { + t.Fatal(err) + } + for _, c := range cases { + if c.Disabled { + continue + } + + t.Run(c.Name, func(t *testing.T) { + ctx := &ratchetingTestContext{ + T: t, + DynamicClient: dynamicClient, + APIExtensionsClient: apiExtensionClient, + } + + for i, op := range c.Operations { + t.Logf("Performing Operation: %v", op.Description()) + if err := op.Do(ctx); err != nil { + t.Fatalf("failed %T operation %v: %v\n%v", op, i, err, op) + } + } + + // Reset resources + err := ctx.DynamicClient.Resource(myCRDV1Beta1).Namespace("default").DeleteCollection(context.TODO(), metav1.DeleteOptions{}, metav1.ListOptions{}) + if err != nil { + t.Fatal(err) + } + }) + } +} + +var myCRDV1Beta1 schema.GroupVersionResource = schema.GroupVersionResource{ + Group: "mygroup.example.com", + Version: "v1beta1", + Resource: "mycrds", +} + +var myCRDInstanceName string = "mycrdinstance" + +func TestRatchetingFunctionality(t *testing.T) { + cases := []ratchetingTestCase{ + { + Name: "Minimum Maximum", + Operations: []ratchetingTestOperation{ + updateMyCRDV1Beta1Schema{&apiextensionsv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensionsv1.JSONSchemaProps{ + "hasMinimum": *numberSchema, + "hasMaximum": *numberSchema, + "hasMinimumAndMaximum": *numberSchema, + }, + }}, + applyPatchOperation{ + "Create an object that complies with the schema", + myCRDV1Beta1, + myCRDInstanceName, + map[string]interface{}{ + "hasMinimum": 0, + "hasMaximum": 1000, + "hasMinimumAndMaximum": 50, + }}, + patchMyCRDV1Beta1Schema{ + "Add stricter minimums and maximums that violate the previous object", + map[string]interface{}{ + "properties": map[string]interface{}{ + "hasMinimum": map[string]interface{}{ + "minimum": 10, + }, + "hasMaximum": map[string]interface{}{ + "maximum": 20, + }, + "hasMinimumAndMaximum": map[string]interface{}{ + "minimum": 10, + "maximum": 20, + }, + "noRestrictions": map[string]interface{}{ + "type": "integer", + }, + }, + }}, + applyPatchOperation{ + "Add new fields that validates successfully without changing old ones", + myCRDV1Beta1, + myCRDInstanceName, + map[string]interface{}{ + "noRestrictions": 50, + }}, + expectError{ + applyPatchOperation{ + "Change a single old field to be invalid", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "hasMinimum": 5, + }}, + }, + expectError{ + applyPatchOperation{ + "Change multiple old fields to be invalid", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "hasMinimum": 5, + "hasMaximum": 21, + }}, + }, + applyPatchOperation{ + "Change single old field to be valid", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "hasMinimum": 11, + }}, + applyPatchOperation{ + "Change multiple old fields to be valid", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "hasMaximum": 19, + "hasMinimumAndMaximum": 15, + }}, + }, + }, + { + Name: "Enum", + Operations: []ratchetingTestOperation{ + // Create schema with some enum element + updateMyCRDV1Beta1Schema{&apiextensionsv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensionsv1.JSONSchemaProps{ + "enumField": *stringSchema, + }, + }}, + applyPatchOperation{ + "Create an instance with a soon-to-be-invalid value", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "enumField": "okValueNowBadValueLater", + }}, + patchMyCRDV1Beta1Schema{ + "restrict `enumField` to an enum of A, B, or C", + map[string]interface{}{ + "properties": map[string]interface{}{ + "enumField": map[string]interface{}{ + "enum": []interface{}{ + "A", "B", "C", + }, + }, + "otherField": map[string]interface{}{ + "type": "string", + }, + }, + }}, + applyPatchOperation{ + "An invalid patch with no changes is a noop", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "enumField": "okValueNowBadValueLater", + }}, + applyPatchOperation{ + "Add a new field, and include old value in our patch", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "enumField": "okValueNowBadValueLater", + "otherField": "anythingGoes", + }}, + expectError{ + applyPatchOperation{ + "Set enumField to invalid value D", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "enumField": "D", + }}, + }, + applyPatchOperation{ + "Set to a valid value", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "enumField": "A", + }}, + expectError{ + applyPatchOperation{ + "After setting a valid value, return to the old, accepted value", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "enumField": "okValueNowBadValueLater", + }}, + }, + }, + }, + { + Name: "AdditionalProperties", + Operations: []ratchetingTestOperation{ + updateMyCRDV1Beta1Schema{&apiextensionsv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensionsv1.JSONSchemaProps{ + "nums": *numbersMapSchema, + "content": *stringMapSchema, + }, + }}, + applyPatchOperation{ + "Create an instance", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "nums": map[string]interface{}{ + "num1": 1, + "num2": 1000000, + }, + "content": map[string]interface{}{ + "k1": "some content", + "k2": "other content", + }, + }}, + patchMyCRDV1Beta1Schema{ + "set minimum value for fields with additionalProperties", + map[string]interface{}{ + "properties": map[string]interface{}{ + "nums": map[string]interface{}{ + "additionalProperties": map[string]interface{}{ + "minimum": 1000, + }, + }, + }, + }}, + applyPatchOperation{ + "updating validating field num2 to another validating value, but rachet invalid field num1", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "nums": map[string]interface{}{ + "num1": 1, + "num2": 2000, + }, + }}, + expectError{applyPatchOperation{ + "update field num1 to different invalid value", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "nums": map[string]interface{}{ + "num1": 2, + "num2": 2000, + }, + }}}, + }, + }, + { + Name: "MinProperties MaxProperties", + Operations: []ratchetingTestOperation{ + updateMyCRDV1Beta1Schema{&apiextensionsv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensionsv1.JSONSchemaProps{ + "restricted": { + Type: "object", + AdditionalProperties: &apiextensionsv1.JSONSchemaPropsOrBool{ + Schema: stringSchema, + }, + }, + "unrestricted": { + Type: "object", + AdditionalProperties: &apiextensionsv1.JSONSchemaPropsOrBool{ + Schema: stringSchema, + }, + }, + }, + }}, + applyPatchOperation{ + "Create instance", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "restricted": map[string]interface{}{ + "key1": "hi", + "key2": "there", + }, + }}, + patchMyCRDV1Beta1Schema{ + "set both minProperties and maxProperties to 1 to violate the previous object", + map[string]interface{}{ + "properties": map[string]interface{}{ + "restricted": map[string]interface{}{ + "minProperties": 1, + "maxProperties": 1, + }, + }, + }}, + applyPatchOperation{ + "ratchet violating object 'restricted' around changes to unrelated field", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "restricted": map[string]interface{}{ + "key1": "hi", + "key2": "there", + }, + "unrestricted": map[string]interface{}{ + "key1": "yo", + }, + }}, + expectError{applyPatchOperation{ + "make invalid changes to previously ratcheted invalid field", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "restricted": map[string]interface{}{ + "key1": "changed", + "key2": "there", + }, + "unrestricted": map[string]interface{}{ + "key1": "yo", + }, + }}}, + + patchMyCRDV1Beta1Schema{ + "remove maxProeprties, set minProperties to 2", + map[string]interface{}{ + "properties": map[string]interface{}{ + "restricted": map[string]interface{}{ + "minProperties": 2, + "maxProperties": nil, + }, + }, + }}, + applyPatchOperation{ + "a new value", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "restricted": map[string]interface{}{ + "key1": "hi", + "key2": "there", + "key3": "buddy", + }, + }}, + + expectError{applyPatchOperation{ + "violate new validation by removing keys", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "restricted": map[string]interface{}{ + "key1": "hi", + "key2": nil, + "key3": nil, + }, + }}}, + patchMyCRDV1Beta1Schema{ + "remove minProperties, set maxProperties to 1", + map[string]interface{}{ + "properties": map[string]interface{}{ + "restricted": map[string]interface{}{ + "minProperties": nil, + "maxProperties": 1, + }, + }, + }}, + applyPatchOperation{ + "modify only the other key, ratcheting maxProperties for field `restricted`", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "restricted": map[string]interface{}{ + "key1": "hi", + "key2": "there", + "key3": "buddy", + }, + "unrestricted": map[string]interface{}{ + "key1": "value", + "key2": "value", + }, + }}, + expectError{ + applyPatchOperation{ + "modifying one value in the object with maxProperties restriction, but keeping old fields", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "restricted": map[string]interface{}{ + "key1": "hi", + "key2": "theres", + "key3": "buddy", + }, + }}}, + }, + }, + { + Name: "MinItems", + Operations: []ratchetingTestOperation{ + updateMyCRDV1Beta1Schema{&apiextensionsv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensionsv1.JSONSchemaProps{ + "field": *stringSchema, + "array": { + Type: "array", + Items: &apiextensionsv1.JSONSchemaPropsOrArray{ + Schema: stringSchema, + }, + }, + }, + }}, + applyPatchOperation{ + "Create instance", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "array": []interface{}{"value1", "value2", "value3"}, + }}, + patchMyCRDV1Beta1Schema{ + "change minItems on array to 10, invalidates previous object", + map[string]interface{}{ + "properties": map[string]interface{}{ + "array": map[string]interface{}{ + "minItems": 10, + }, + }, + }}, + applyPatchOperation{ + "keep invalid field `array` unchanged, add new field with ratcheting", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "array": []interface{}{"value1", "value2", "value3"}, + "field": "value", + }}, + expectError{ + applyPatchOperation{ + "modify array element without satisfying property", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "array": []interface{}{"value2", "value2", "value3"}, + }}}, + + expectError{ + applyPatchOperation{ + "add array element without satisfying proeprty", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "array": []interface{}{"value1", "value2", "value3", "value4"}, + }}}, + + applyPatchOperation{ + "make array valid", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "array": []interface{}{"value1", "value2", "value3", "4", "5", "6", "7", "8", "9", "10"}, + }}, + expectError{ + applyPatchOperation{ + "revert to original value", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "array": []interface{}{"value1", "value2", "value3"}, + }}}, + }, + }, + { + Name: "MaxItems", + Operations: []ratchetingTestOperation{ + updateMyCRDV1Beta1Schema{&apiextensionsv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensionsv1.JSONSchemaProps{ + "field": *stringSchema, + "array": { + Type: "array", + Items: &apiextensionsv1.JSONSchemaPropsOrArray{ + Schema: stringSchema, + }, + }, + }, + }}, + applyPatchOperation{ + "create instance", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "array": []interface{}{"value1", "value2", "value3"}, + }}, + patchMyCRDV1Beta1Schema{ + "change maxItems on array to 1, invalidates previous object", + map[string]interface{}{ + "properties": map[string]interface{}{ + "array": map[string]interface{}{ + "maxItems": 1, + }, + }, + }}, + applyPatchOperation{ + "ratchet old value of array through an update to another field", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "array": []interface{}{"value1", "value2", "value3"}, + "field": "value", + }}, + expectError{ + applyPatchOperation{ + "modify array element without satisfying property", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "array": []interface{}{"value2", "value2", "value3"}, + }}}, + + expectError{ + applyPatchOperation{ + "remove array element without satisfying proeprty", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "array": []interface{}{"value1", "value2"}, + }}}, + + applyPatchOperation{ + "change array to valid value that satisfies maxItems", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "array": []interface{}{"value1"}, + }}, + expectError{ + applyPatchOperation{ + "revert to previous invalid ratcheted value", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "array": []interface{}{"value1", "value2", "value3"}, + }}}, + }, + }, + { + Name: "MinLength MaxLength", + Operations: []ratchetingTestOperation{ + updateMyCRDV1Beta1Schema{&apiextensionsv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensionsv1.JSONSchemaProps{ + "minField": *stringSchema, + "maxField": *stringSchema, + "otherField": *stringSchema, + }, + }}, + applyPatchOperation{ + "create instance", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "minField": "value", + "maxField": "valueThatsVeryLongSee", + }}, + patchMyCRDV1Beta1Schema{ + "set minField maxLength to 10, and maxField's minLength to 15", + map[string]interface{}{ + "properties": map[string]interface{}{ + "minField": map[string]interface{}{ + "minLength": 10, + }, + "maxField": map[string]interface{}{ + "maxLength": 15, + }, + }, + }}, + applyPatchOperation{ + "add new field `otherField`, ratcheting `minField` and `maxField`", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "minField": "value", + "maxField": "valueThatsVeryLongSee", + "otherField": "otherValue", + }}, + applyPatchOperation{ + "make minField valid, ratcheting old value for maxField", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "minField": "valuelength13", + "maxField": "valueThatsVeryLongSee", + "otherField": "otherValue", + }}, + applyPatchOperation{ + "make maxField shorter", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "maxField": "l2", + }}, + expectError{ + applyPatchOperation{ + "make maxField too long", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "maxField": "valuewithlength17", + }}}, + expectError{ + applyPatchOperation{ + "revert minFIeld to previously ratcheted value", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "minField": "value", + }}}, + expectError{ + applyPatchOperation{ + "revert maxField to previously ratcheted value", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "maxField": "valueThatsVeryLongSee", + }}}, + }, + }, + { + Name: "Pattern", + Operations: []ratchetingTestOperation{ + updateMyCRDV1Beta1Schema{&apiextensionsv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensionsv1.JSONSchemaProps{ + "field": *stringSchema, + }, + }}, + applyPatchOperation{ + "create instance", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "field": "doesnt abide pattern", + }}, + patchMyCRDV1Beta1Schema{ + "add pattern validation on `field`", + map[string]interface{}{ + "properties": map[string]interface{}{ + "field": map[string]interface{}{ + "pattern": "^[1-9]+$", + }, + "otherField": map[string]interface{}{ + "type": "string", + }, + }, + }}, + applyPatchOperation{ + "add unrelated field, ratcheting old invalid field", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "field": "doesnt abide pattern", + "otherField": "added", + }}, + expectError{applyPatchOperation{ + "change field to invalid value", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "field": "w123", + "otherField": "added", + }}}, + applyPatchOperation{ + "change field to a valid value", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "field": "123", + "otherField": "added", + }}, + }, + }, + { + Name: "Format Addition and Change", + Operations: []ratchetingTestOperation{ + updateMyCRDV1Beta1Schema{&apiextensionsv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensionsv1.JSONSchemaProps{ + "field": *stringSchema, + }, + }}, + applyPatchOperation{ + "create instance", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "field": "doesnt abide any format", + }}, + patchMyCRDV1Beta1Schema{ + "change `field`'s format to `byte", + map[string]interface{}{ + "properties": map[string]interface{}{ + "field": map[string]interface{}{ + "format": "byte", + }, + "otherField": map[string]interface{}{ + "type": "string", + }, + }, + }}, + applyPatchOperation{ + "add unrelated otherField, ratchet invalid old field format", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "field": "doesnt abide any format", + "otherField": "value", + }}, + expectError{applyPatchOperation{ + "change field to an invalid string", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "field": "asd", + }}}, + applyPatchOperation{ + "change field to a valid byte string", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "field": "dGhpcyBpcyBwYXNzd29yZA==", + }}, + patchMyCRDV1Beta1Schema{ + "change `field`'s format to date-time", + map[string]interface{}{ + "properties": map[string]interface{}{ + "field": map[string]interface{}{ + "format": "date-time", + }, + }, + }}, + applyPatchOperation{ + "change otherField, ratchet `field`'s invalid byte format", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "field": "dGhpcyBpcyBwYXNzd29yZA==", + "otherField": "value2", + }}, + applyPatchOperation{ + "change `field` to a valid value", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "field": "2018-11-13T20:20:39+00:00", + "otherField": "value2", + }}, + expectError{ + applyPatchOperation{ + "revert `field` to previously ratcheted value", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "field": "dGhpcyBpcyBwYXNzd29yZA==", + "otherField": "value2", + }}}, + expectError{ + applyPatchOperation{ + "revert `field` to its initial value from creation", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "field": "doesnt abide any format", + }}}, + }, + }, + { + Name: "Map Type List Reordering Grandfathers Invalid Key", + Operations: []ratchetingTestOperation{ + updateMyCRDV1Beta1Schema{&apiextensionsv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensionsv1.JSONSchemaProps{ + "field": { + Type: "array", + XListType: ptr("map"), + XListMapKeys: []string{"name", "port"}, + Items: &apiextensionsv1.JSONSchemaPropsOrArray{ + Schema: &apiextensionsv1.JSONSchemaProps{ + Type: "object", + Required: []string{"name", "port"}, + Properties: map[string]apiextensionsv1.JSONSchemaProps{ + "name": *stringSchema, + "port": *numberSchema, + "field": *stringSchema, + }, + }, + }, + }, + }, + }}, + applyPatchOperation{ + "create instance with three soon-to-be-invalid keys", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "field": []interface{}{ + map[string]interface{}{ + "name": "nginx", + "port": 443, + "field": "value", + }, + map[string]interface{}{ + "name": "etcd", + "port": 2379, + "field": "value", + }, + map[string]interface{}{ + "name": "kube-apiserver", + "port": 6443, + "field": "value", + }, + }, + }}, + patchMyCRDV1Beta1Schema{ + "set `field`'s maxItems to 2, which is exceeded by all of previous object's elements", + map[string]interface{}{ + "properties": map[string]interface{}{ + "field": map[string]interface{}{ + "maxItems": 2, + }, + }, + }}, + applyPatchOperation{ + "reorder invalid objects which have too many properties, but do not modify them or change keys", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "field": []interface{}{ + map[string]interface{}{ + "name": "kube-apiserver", + "port": 6443, + "field": "value", + }, + map[string]interface{}{ + "name": "nginx", + "port": 443, + "field": "value", + }, + map[string]interface{}{ + "name": "etcd", + "port": 2379, + "field": "value", + }, + }, + }}, + expectError{ + applyPatchOperation{ + "attempt to change one of the fields of the items which exceed maxItems", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "field": []interface{}{ + map[string]interface{}{ + "name": "kube-apiserver", + "port": 6443, + "field": "value", + }, + map[string]interface{}{ + "name": "nginx", + "port": 443, + "field": "value", + }, + map[string]interface{}{ + "name": "etcd", + "port": 2379, + "field": "value", + }, + map[string]interface{}{ + "name": "dev", + "port": 8080, + "field": "value", + }, + }, + }}}, + patchMyCRDV1Beta1Schema{ + "Require even numbered port in key, remove maxItems requirement", + map[string]interface{}{ + "properties": map[string]interface{}{ + "field": map[string]interface{}{ + "maxItems": nil, + "items": map[string]interface{}{ + "properties": map[string]interface{}{ + "port": map[string]interface{}{ + "multipleOf": 2, + }, + }, + }, + }, + }, + }}, + + applyPatchOperation{ + "reorder fields without changing anything", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "field": []interface{}{ + map[string]interface{}{ + "name": "nginx", + "port": 443, + "field": "value", + }, + map[string]interface{}{ + "name": "etcd", + "port": 2379, + "field": "value", + }, + map[string]interface{}{ + "name": "kube-apiserver", + "port": 6443, + "field": "value", + }, + }, + }}, + + applyPatchOperation{ + `use "invalid" keys despite changing order and changing sibling fields to the key`, + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "field": []interface{}{ + map[string]interface{}{ + "name": "nginx", + "port": 443, + "field": "value", + }, + map[string]interface{}{ + "name": "etcd", + "port": 2379, + "field": "value", + }, + map[string]interface{}{ + "name": "kube-apiserver", + "port": 6443, + "field": "this is a changed value for an an invalid but grandfathered key", + }, + map[string]interface{}{ + "name": "dev", + "port": 8080, + "field": "value", + }, + }, + }}, + }, + }, + { + Name: "ArrayItems correlate by index", + Operations: []ratchetingTestOperation{ + updateMyCRDV1Beta1Schema{&apiextensionsv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensionsv1.JSONSchemaProps{ + "values": { + Type: "array", + Items: &apiextensionsv1.JSONSchemaPropsOrArray{ + Schema: stringMapSchema, + }, + }, + "otherField": *stringSchema, + }, + }}, + applyPatchOperation{ + "create instance with length 5 values", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "values": []interface{}{ + map[string]interface{}{ + "name": "1", + "key": "value", + }, + map[string]interface{}{ + "name": "2", + "key": "value", + }, + }, + }}, + patchMyCRDV1Beta1Schema{ + "Set minimum length of 6 for values of elements in the items array", + map[string]interface{}{ + "properties": map[string]interface{}{ + "values": map[string]interface{}{ + "items": map[string]interface{}{ + "additionalProperties": map[string]interface{}{ + "minLength": 6, + }, + }, + }, + }, + }}, + expectError{ + applyPatchOperation{ + "change value to one that exceeds minLength", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "values": []interface{}{ + map[string]interface{}{ + "name": "1", + "key": "value", + }, + map[string]interface{}{ + "name": "2", + "key": "bad", + }, + }, + }}}, + applyPatchOperation{ + "add new fields without touching the map", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "values": []interface{}{ + map[string]interface{}{ + "name": "1", + "key": "value", + }, + map[string]interface{}{ + "name": "2", + "key": "value", + }, + }, + "otherField": "hello world", + }}, + // (This test shows an array can be correlated by index with its old value) + applyPatchOperation{ + "add new, valid fields to elements of the array, ratcheting unchanged old fields within the array elements by correlating by index", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "values": []interface{}{ + map[string]interface{}{ + "name": "1", + "key": "value", + }, + map[string]interface{}{ + "name": "2", + "key": "value", + "key2": "valid value", + }, + }, + }}, + expectError{ + applyPatchOperation{ + "reorder the array, preventing index correlation", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "values": []interface{}{ + map[string]interface{}{ + "name": "2", + "key": "value", + "key2": "valid value", + }, + map[string]interface{}{ + "name": "1", + "key": "value", + }, + }, + }}}, + }, + }, + // Features that should not ratchet + { + Name: "AllOf_should_not_ratchet", + }, + { + Name: "OneOf_should_not_ratchet", + }, + { + Name: "AnyOf_should_not_ratchet", + }, + { + Name: "Not_should_not_ratchet", + }, + { + Name: "CEL_transition_rules_should_not_ratchet", + }, + // Future Functionality, disabled tests + { + Name: "CEL Add Change Rule", + // Planned future test. CEL Rules are not yet ratcheted in alpha + // implementation of CRD Validation Ratcheting + Disabled: true, + Operations: []ratchetingTestOperation{ + updateMyCRDV1Beta1Schema{&apiextensionsv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensionsv1.JSONSchemaProps{ + "field": { + Type: "object", + AdditionalProperties: &apiextensionsv1.JSONSchemaPropsOrBool{ + Schema: &apiextensionsv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensionsv1.JSONSchemaProps{ + "stringField": *stringSchema, + "intField": *numberSchema, + "otherIntField": *numberSchema, + }, + }, + }, + }, + }, + }}, + applyPatchOperation{ + "create instance with strings that do not start with k8s", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "field": map[string]interface{}{ + "object1": map[string]interface{}{ + "stringField": "a string", + "intField": 5, + }, + "object2": map[string]interface{}{ + "stringField": "another string", + "intField": 15, + }, + "object3": map[string]interface{}{ + "stringField": "a third string", + "intField": 7, + }, + }, + }}, + patchMyCRDV1Beta1Schema{ + "require that stringField value start with `k8s`", + map[string]interface{}{ + "properties": map[string]interface{}{ + "field": map[string]interface{}{ + "additionalProperties": map[string]interface{}{ + "properties": map[string]interface{}{ + "stringField": map[string]interface{}{ + "x-kubernetes-validations": []interface{}{ + map[string]interface{}{ + "rule": "self.startsWith('k8s')", + "message": "strings must have k8s prefix", + }, + }, + }, + }, + }, + }, + }, + }}, + applyPatchOperation{ + "add a new entry that follows the new rule, ratchet old values", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "field": map[string]interface{}{ + "object1": map[string]interface{}{ + "stringField": "a string", + "intField": 5, + }, + "object2": map[string]interface{}{ + "stringField": "another string", + "intField": 15, + }, + "object3": map[string]interface{}{ + "stringField": "a third string", + "intField": 7, + }, + "object4": map[string]interface{}{ + "stringField": "k8s third string", + "intField": 7, + }, + }, + }}, + applyPatchOperation{ + "modify a sibling to an invalid value, ratcheting the unchanged invalid value", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "field": map[string]interface{}{ + "object1": map[string]interface{}{ + "stringField": "a string", + "intField": 15, + }, + "object2": map[string]interface{}{ + "stringField": "another string", + "intField": 10, + "otherIntField": 20, + }, + }, + }}, + expectError{ + applyPatchOperation{ + "change a previously ratcheted field to an invalid value", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "field": map[string]interface{}{ + "object2": map[string]interface{}{ + "stringField": "a changed string", + }, + "object3": map[string]interface{}{ + "stringField": "a changed third string", + }, + }, + }}}, + patchMyCRDV1Beta1Schema{ + "require that stringField values are also odd length", + map[string]interface{}{ + "properties": map[string]interface{}{ + "field": map[string]interface{}{ + "additionalProperties": map[string]interface{}{ + "stringField": map[string]interface{}{ + "x-kubernetes-validations": []interface{}{ + map[string]interface{}{ + "rule": "self.startsWith('k8s')", + "message": "strings must have k8s prefix", + }, + map[string]interface{}{ + "rule": "len(self) % 2 == 1", + "message": "strings must have odd length", + }, + }, + }, + }, + }, + }, + }}, + applyPatchOperation{ + "have mixed ratcheting of one or two CEL rules, object4 is ratcheted by one rule, object1 is ratcheting 2 rules", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "field": map[string]interface{}{ + "object1": map[string]interface{}{ + "stringField": "a string", // invalid. even number length, no k8s prefix + "intField": 1000, + }, + "object4": map[string]interface{}{ + "stringField": "k8s third string", // invalid. even number length. ratcheted + "intField": 7000, + }, + }, + }}, + expectError{ + applyPatchOperation{ + "swap keys between valuesin the map", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "field": map[string]interface{}{ + "object1": map[string]interface{}{ + "stringField": "k8s third string", + "intField": 1000, + }, + "object4": map[string]interface{}{ + "stringField": "a string", + "intField": 7000, + }, + }, + }}}, + applyPatchOperation{ + "fix keys", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "field": map[string]interface{}{ + "object1": map[string]interface{}{ + "stringField": "k8s a stringy", + "intField": 1000, + }, + "object4": map[string]interface{}{ + "stringField": "k8s third stringy", + "intField": 7000, + }, + }, + }}, + }, + }, + { + // Changing a list to a set should allow you to keep the items the + // same, but if you modify any one item the set must be uniqued + // + // Possibly a future area of improvement. As it stands now, + // SSA implementation is incompatible with ratcheting this field: + // https://github.com/kubernetes/kubernetes/blob/ec9a8ffb237e391ce9ccc58de93ba4ecc2fabf42/staging/src/k8s.io/apimachinery/pkg/util/managedfields/internal/structuredmerge.go#L146-L149 + // + // Throws error trying to interpret an invalid existing `liveObj` + // as a set. + Name: "Change list to set", + Disabled: true, + Operations: []ratchetingTestOperation{ + updateMyCRDV1Beta1Schema{&apiextensionsv1.JSONSchemaProps{ + Type: "object", + Properties: map[string]apiextensionsv1.JSONSchemaProps{ + "values": { + Type: "object", + AdditionalProperties: &apiextensionsv1.JSONSchemaPropsOrBool{ + Schema: &apiextensionsv1.JSONSchemaProps{ + Type: "array", + Items: &apiextensionsv1.JSONSchemaPropsOrArray{ + Schema: numberSchema, + }, + }, + }, + }, + }, + }}, + applyPatchOperation{ + "reate a list of numbers with duplicates using the old simple schema", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "values": map[string]interface{}{ + "dups": []interface{}{1, 2, 2, 3, 1000, 2000}, + }, + }}, + patchMyCRDV1Beta1Schema{ + "change list type to set", + map[string]interface{}{ + "properties": map[string]interface{}{ + "values": map[string]interface{}{ + "additionalProperties": map[string]interface{}{ + "x-kubernetes-list-type": "set", + }, + }, + }, + }}, + expectError{ + applyPatchOperation{ + "change original without removing duplicates", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "values": map[string]interface{}{ + "dups": []interface{}{1, 2, 2, 3, 1000, 2000, 3}, + }, + }}}, + expectError{applyPatchOperation{ + "add another list with duplicates", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "values": map[string]interface{}{ + "dups": []interface{}{1, 2, 2, 3, 1000, 2000}, + "dups2": []interface{}{1, 2, 2, 3, 1000, 2000}, + }, + }}}, + // Can add a valid sibling field + //! Remove this ExpectError if/when we add support for ratcheting + // the type of a list + applyPatchOperation{ + "add a valid sibling field", + myCRDV1Beta1, myCRDInstanceName, map[string]interface{}{ + "values": map[string]interface{}{ + "dups": []interface{}{1, 2, 2, 3, 1000, 2000}, + "otherField": []interface{}{1, 2, 3}, + }, + }}, + // Can remove dups to make valid + //! Normally this woud be valid, but SSA is unable to interpret + // the `liveObj` in the new schema, so fails. Changing + // x-kubernetes-list-type from anything to a set is unsupported by SSA. + applyPatchOperation{ + "remove dups to make list valid", + myCRDV1Beta1, + myCRDInstanceName, + map[string]interface{}{ + "values": map[string]interface{}{ + "dups": []interface{}{1, 3, 1000, 2000}, + "otherField": []interface{}{1, 2, 3}, + }, + }}, + }, + }, + } + + runTests(t, cases) +} + +func ptr[T any](v T) *T { + return &v +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 8a1bc7e73fd47..d2192848fdf9f 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1334,6 +1334,7 @@ k8s.io/apiextensions-apiserver/pkg/controller/openapi/v2 k8s.io/apiextensions-apiserver/pkg/controller/openapiv3 k8s.io/apiextensions-apiserver/pkg/controller/status k8s.io/apiextensions-apiserver/pkg/crdserverscheme +k8s.io/apiextensions-apiserver/pkg/features k8s.io/apiextensions-apiserver/pkg/generated/openapi k8s.io/apiextensions-apiserver/pkg/registry/customresource k8s.io/apiextensions-apiserver/pkg/registry/customresource/tableconvertor