diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 1173090b10a37..4bd4e08af5f60 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -17,7 +17,6 @@ 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" @@ -1212,11 +1211,6 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS genericfeatures.UnauthenticatedHTTP2DOSMitigation: {Default: false, PreRelease: featuregate.Beta}, - // 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 6e7fb81a12355..ceb7f9739e52b 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/go.mod +++ b/staging/src/k8s.io/apiextensions-apiserver/go.mod @@ -6,7 +6,6 @@ go 1.20 require ( github.com/emicklei/go-restful/v3 v3.9.0 - github.com/evanphx/json-patch v4.12.0+incompatible github.com/gogo/protobuf v1.3.2 github.com/google/cel-go v0.16.1 github.com/google/gnostic-models v0.6.8 @@ -50,6 +49,7 @@ 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 875b3e6de079f..1c23eb5ce3077 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 @@ -831,7 +831,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.OpenAPIV3Schema); err != nil { + if _, _, err := apiservervalidation.NewSchemaValidator(customResourceValidation); 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 2a6dcb80a7c75..a2e4b7a28c7ce 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,6 +79,8 @@ 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. @@ -737,22 +739,20 @@ 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(internalSchemaProps) + validator, _, err := apiservervalidation.NewSchemaValidator(internalValidationSchema) if err != nil { return nil, err } var statusSpec *apiextensionsinternal.CustomResourceSubresourceStatus - var statusValidator apiservervalidation.SchemaValidator + var statusValidator *validate.SchemaValidator subresources, err := apiextensionshelpers.GetSubresourcesForVersion(crd, v.Name) if err != nil { utilruntime.HandleError(err) @@ -767,10 +767,11 @@ 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 { - statusValidator, _, err = apiservervalidation.NewSchemaValidator(&statusSchema) - if err != nil { + openapiSchema := &spec.Schema{} + if err := apiservervalidation.ConvertJSONSchemaPropsWithPostProcess(&statusSchema, openapiSchema, apiservervalidation.StripUnsupportedFormatsPostProcess); 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 deleted file mode 100644 index 6565d83eee53c..0000000000000 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/ratcheting.go +++ /dev/null @@ -1,412 +0,0 @@ -/* -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 deleted file mode 100644 index 078bb548cdcb4..0000000000000 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/validation/ratcheting_test.go +++ /dev/null @@ -1,136 +0,0 @@ -/* -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 e0042356ac0c4..fad044d1c6bf6 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,83 +22,29 @@ 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. -// -// 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) { +func NewSchemaValidator(customResourceValidation *apiextensions.CustomResourceValidation) (*validate.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, openapiSchema, StripUnsupportedFormatsPostProcess); err != nil { + if err := ConvertJSONSchemaPropsWithPostProcess(customResourceValidation.OpenAPIV3Schema, openapiSchema, StripUnsupportedFormatsPostProcess); err != nil { return nil, nil, err } } - - 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) + return validate.NewSchemaValidator(openapiSchema, nil, "", strfmt.Default), openapiSchema, nil } // 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 SchemaCreateValidator) field.ErrorList { +func ValidateCustomResource(fldPath *field.Path, customResource interface{}, validator *validate.SchemaValidator) field.ErrorList { if validator == nil { return nil } @@ -107,11 +53,6 @@ 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) { @@ -346,7 +287,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", convertSliceToInterfaceSlice(in.XListMapKeys)) + out.VendorExtensible.AddExtension("x-kubernetes-list-map-keys", in.XListMapKeys) } if in.XListType != nil { out.VendorExtensible.AddExtension("x-kubernetes-list-type", *in.XListType) @@ -359,19 +300,11 @@ 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", convertSliceToInterfaceSlice(serializationValidationRules)) + out.VendorExtensible.AddExtension("x-kubernetes-validations", 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 c6776ebf6a9d0..7bbeba3eae541 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(&tt.schema) + validator, _, err := NewSchemaValidator(&apiextensions.CustomResourceValidation{OpenAPIV3Schema: &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(&tt.args.schema) + validator, _, err := NewSchemaValidator(&apiextensions.CustomResourceValidation{OpenAPIV3Schema: &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 1844ed8d1eb49..a5932281367c3 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,18 +22,11 @@ import ( ) const ( - // 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" +// Every feature gate should add method here following this template: +// +// // owner: @username +// // alpha: v1.4 +// MyFeature() bool ) func init() { @@ -43,6 +36,4 @@ 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{ - CRDValidationRatcheting: {Default: false, PreRelease: featuregate.Alpha}, -} +var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{} 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 b534c627343ba..92a688a56d729 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,12 +19,13 @@ 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" @@ -57,7 +58,7 @@ type customResourceStrategy struct { kind schema.GroupVersionKind } -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 { +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 { 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 c39ad15270a07..9c04e447bc533 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,16 +28,17 @@ 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" - apiextensionsvalidation "k8s.io/apiextensions-apiserver/pkg/apiserver/validation" + apiservervalidation "k8s.io/apiextensions-apiserver/pkg/apiserver/validation" ) type customResourceValidator struct { namespaceScoped bool kind schema.GroupVersionKind - schemaValidator apiextensionsvalidation.SchemaValidator - statusSchemaValidator apiextensionsvalidation.SchemaValidator + schemaValidator *validate.SchemaValidator + statusSchemaValidator *validate.SchemaValidator } func (a customResourceValidator) Validate(ctx context.Context, obj runtime.Object, scale *apiextensions.CustomResourceSubresourceScale) field.ErrorList { @@ -57,7 +58,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, apiextensionsvalidation.ValidateCustomResource(nil, u.UnstructuredContent(), a.schemaValidator)...) + allErrs = append(allErrs, apiservervalidation.ValidateCustomResource(nil, u.UnstructuredContent(), a.schemaValidator)...) allErrs = append(allErrs, a.ValidateScaleSpec(ctx, u, scale)...) allErrs = append(allErrs, a.ValidateScaleStatus(ctx, u, scale)...) @@ -69,10 +70,6 @@ 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())} @@ -89,7 +86,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, apiextensionsvalidation.ValidateCustomResourceUpdate(nil, u.UnstructuredContent(), oldU.UnstructuredContent(), a.schemaValidator)...) + allErrs = append(allErrs, apiservervalidation.ValidateCustomResource(nil, u.UnstructuredContent(), a.schemaValidator)...) allErrs = append(allErrs, a.ValidateScaleSpec(ctx, u, scale)...) allErrs = append(allErrs, a.ValidateScaleStatus(ctx, u, scale)...) @@ -106,10 +103,6 @@ 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())} @@ -126,9 +119,9 @@ func (a customResourceValidator) ValidateStatusUpdate(ctx context.Context, obj, var allErrs field.ErrorList allErrs = append(allErrs, validation.ValidateObjectMetaAccessorUpdate(objAccessor, oldAccessor, field.NewPath("metadata"))...) - if status, hasStatus := u.UnstructuredContent()["status"]; hasStatus { - allErrs = append(allErrs, apiextensionsvalidation.ValidateCustomResourceUpdate(nil, status, oldU.UnstructuredContent()["status"], a.statusSchemaValidator)...) - } + + allErrs = append(allErrs, apiservervalidation.ValidateCustomResource(nil, u.UnstructuredContent(), 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 deleted file mode 100644 index cc88e1d8114a5..0000000000000 --- a/staging/src/k8s.io/apiextensions-apiserver/test/integration/ratcheting_test.go +++ /dev/null @@ -1,1576 +0,0 @@ -/* -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 16d5788624dfc..7238c39cbca07 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1337,7 +1337,6 @@ 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