diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 8c4d8253b0d97..e5d0ba2eebc34 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -904,6 +904,7 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS genericfeatures.WarningHeaders: {Default: true, PreRelease: featuregate.GA, LockToDefault: true}, // remove in 1.24 genericfeatures.OpenAPIEnums: {Default: false, PreRelease: featuregate.Alpha}, genericfeatures.CustomResourceValidationExpressions: {Default: false, PreRelease: featuregate.Alpha}, + genericfeatures.FieldValidation: {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/pkg/apiserver/customresource_handler.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go index c317dd4d98b9c..451b44324a8b3 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 @@ -841,12 +841,13 @@ func (r *crdHandler) getOrCreateServingInfoFor(uid types.UID, name string) (*crd // CRDs explicitly do not support protobuf, but some objects returned by the API server do negotiatedSerializer := unstructuredNegotiatedSerializer{ - typer: typer, - creator: creator, - converter: safeConverter, - structuralSchemas: structuralSchemas, - structuralSchemaGK: kind.GroupKind(), - preserveUnknownFields: crd.Spec.PreserveUnknownFields, + typer: typer, + creator: creator, + converter: safeConverter, + structuralSchemas: structuralSchemas, + structuralSchemaGK: kind.GroupKind(), + preserveUnknownFields: crd.Spec.PreserveUnknownFields, + persistStrictDecodingErrors: true, } var standardSerializers []runtime.SerializerInfo for _, s := range negotiatedSerializer.SupportedMediaTypes() { @@ -1032,9 +1033,10 @@ type unstructuredNegotiatedSerializer struct { creator runtime.ObjectCreater converter runtime.ObjectConvertor - structuralSchemas map[string]*structuralschema.Structural // by version - structuralSchemaGK schema.GroupKind - preserveUnknownFields bool + structuralSchemas map[string]*structuralschema.Structural // by version + structuralSchemaGK schema.GroupKind + preserveUnknownFields bool + persistStrictDecodingErrors bool } func (s unstructuredNegotiatedSerializer) SupportedMediaTypes() []runtime.SerializerInfo { @@ -1077,7 +1079,7 @@ func (s unstructuredNegotiatedSerializer) EncoderForVersion(encoder runtime.Enco } func (s unstructuredNegotiatedSerializer) DecoderToVersion(decoder runtime.Decoder, gv runtime.GroupVersioner) runtime.Decoder { - d := schemaCoercingDecoder{delegate: decoder, validator: unstructuredSchemaCoercer{structuralSchemas: s.structuralSchemas, structuralSchemaGK: s.structuralSchemaGK, preserveUnknownFields: s.preserveUnknownFields}} + d := schemaCoercingDecoder{delegate: decoder, validator: unstructuredSchemaCoercer{structuralSchemas: s.structuralSchemas, structuralSchemaGK: s.structuralSchemaGK, preserveUnknownFields: s.preserveUnknownFields, persistStrictDecodingErrors: s.persistStrictDecodingErrors}} return versioning.NewCodec(nil, d, runtime.UnsafeObjectConvertor(Scheme), Scheme, Scheme, unstructuredDefaulter{ delegate: Scheme, structuralSchemas: s.structuralSchemas, @@ -1237,6 +1239,9 @@ func (d schemaCoercingDecoder) Decode(data []byte, defaults *schema.GroupVersion } if u, ok := obj.(*unstructured.Unstructured); ok { if err := d.validator.apply(u); err != nil { + if runtime.IsStrictDecodingError(err) { + return obj, gvk, err + } return nil, gvk, err } } @@ -1296,9 +1301,10 @@ type unstructuredSchemaCoercer struct { dropInvalidMetadata bool repairGeneration bool - structuralSchemas map[string]*structuralschema.Structural - structuralSchemaGK schema.GroupKind - preserveUnknownFields bool + structuralSchemas map[string]*structuralschema.Structural + structuralSchemaGK schema.GroupKind + preserveUnknownFields bool + persistStrictDecodingErrors bool } func (v *unstructuredSchemaCoercer) apply(u *unstructured.Unstructured) error { @@ -1321,10 +1327,12 @@ func (v *unstructuredSchemaCoercer) apply(u *unstructured.Unstructured) error { if err != nil { return err } + + pruned := map[string]bool{} if gv.Group == v.structuralSchemaGK.Group && kind == v.structuralSchemaGK.Kind { if !v.preserveUnknownFields { - // TODO: switch over pruning and coercing at the root to schemaobjectmeta.Coerce too - structuralpruning.Prune(u.Object, v.structuralSchemas[gv.Version], false) + // TODO: switch over pruning and coercing at the root to schemaobjectmeta.Coerce too + pruned = structuralpruning.Prune(u.Object, v.structuralSchemas[gv.Version], false) structuraldefaulting.PruneNonNullableNullsWithoutDefaults(u.Object, v.structuralSchemas[gv.Version]) } if err := schemaobjectmeta.Coerce(nil, u.Object, v.structuralSchemas[gv.Version], false, v.dropInvalidMetadata); err != nil { @@ -1348,6 +1356,17 @@ func (v *unstructuredSchemaCoercer) apply(u *unstructured.Unstructured) error { return err } } + // collect all the strict decoding errors and return them + if len(pruned) > 0 && v.persistStrictDecodingErrors { + allStrictErrs := make([]error, len(pruned)) + i := 0 + for unknownField, _ := range pruned { + allStrictErrs[i] = fmt.Errorf("unknown field: %s", unknownField) + i++ + } + err := runtime.NewStrictDecodingError(allStrictErrs) + return err + } return nil } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/pruning/algorithm.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/pruning/algorithm.go index a1fd711c6a56e..9a17b22b7979c 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/pruning/algorithm.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/pruning/algorithm.go @@ -23,7 +23,8 @@ import ( // Prune removes object fields in obj which are not specified in s. It skips TypeMeta and ObjectMeta fields // if XEmbeddedResource is set to true, or for the root if isResourceRoot=true, i.e. it does not // prune unknown metadata fields. -func Prune(obj interface{}, s *structuralschema.Structural, isResourceRoot bool) { +// It returns the set of fields that it prunes. +func Prune(obj interface{}, s *structuralschema.Structural, isResourceRoot bool) map[string]bool { if isResourceRoot { if s == nil { s = &structuralschema.Structural{} @@ -34,7 +35,7 @@ func Prune(obj interface{}, s *structuralschema.Structural, isResourceRoot bool) s = &clone } } - prune(obj, s) + return prune(obj, s) } var metaFields = map[string]bool{ @@ -43,19 +44,22 @@ var metaFields = map[string]bool{ "metadata": true, } -func prune(x interface{}, s *structuralschema.Structural) { +func prune(x interface{}, s *structuralschema.Structural) map[string]bool { if s != nil && s.XPreserveUnknownFields { - skipPrune(x, s) - return + return skipPrune(x, s) } + pruning := map[string]bool{} switch x := x.(type) { case map[string]interface{}: if s == nil { for k := range x { + if !metaFields[k] { + pruning[k] = true + } delete(x, k) } - return + return pruning } for k, v := range x { if s.XEmbeddedResource && metaFields[k] { @@ -63,31 +67,48 @@ func prune(x interface{}, s *structuralschema.Structural) { } prop, ok := s.Properties[k] if ok { - prune(v, &prop) + pruned := prune(v, &prop) + for k, b := range pruned { + pruning[k] = b + } } else if s.AdditionalProperties != nil { - prune(v, s.AdditionalProperties.Structural) + pruned := prune(v, s.AdditionalProperties.Structural) + for k, b := range pruned { + pruning[k] = b + } } else { + if !metaFields[k] { + pruning[k] = true + } delete(x, k) } } case []interface{}: if s == nil { for _, v := range x { - prune(v, nil) + pruned := prune(v, nil) + for k, b := range pruned { + pruning[k] = b + } } - return + return pruning } for _, v := range x { - prune(v, s.Items) + pruned := prune(v, s.Items) + for k, b := range pruned { + pruning[k] = b + } } default: // scalars, do nothing } + return pruning } -func skipPrune(x interface{}, s *structuralschema.Structural) { +func skipPrune(x interface{}, s *structuralschema.Structural) map[string]bool { + pruning := map[string]bool{} if s == nil { - return + return pruning } switch x := x.(type) { @@ -97,16 +118,26 @@ func skipPrune(x interface{}, s *structuralschema.Structural) { continue } if prop, ok := s.Properties[k]; ok { - prune(v, &prop) + pruned := prune(v, &prop) + for k, b := range pruned { + pruning[k] = b + } } else if s.AdditionalProperties != nil { - prune(v, s.AdditionalProperties.Structural) + pruned := prune(v, s.AdditionalProperties.Structural) + for k, b := range pruned { + pruning[k] = b + } } } case []interface{}: for _, v := range x { - skipPrune(v, s.Items) + skipPruned := skipPrune(v, s.Items) + for k, b := range skipPruned { + pruning[k] = b + } } default: // scalars, do nothing } + return pruning } diff --git a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go index 9660282c48bc1..b273b32cb4d3e 100644 --- a/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go +++ b/staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/types.go @@ -544,6 +544,16 @@ type CreateOptions struct { // as defined by https://golang.org/pkg/unicode/#IsPrint. // +optional FieldManager string `json:"fieldManager,omitempty" protobuf:"bytes,3,name=fieldManager"` + + // fieldValidation determines how the server should respond to + // unknown/duplicate fields. + // TODO: Do we still need a protobuf tag if protobuf is not supported? + // Valid values are: + // - Ignore: ignore's unknown/duplicate fields + // - Strict: fail the request on unknown/duplicate fields + // - Warn: respond with a warning, but successfully serve the request. + // +optional + FieldValidation string `json:"fieldValidation,omitempty"` } // +k8s:conversion-gen:explicit-from=net/url.Values @@ -577,6 +587,16 @@ type PatchOptions struct { // types (JsonPatch, MergePatch, StrategicMergePatch). // +optional FieldManager string `json:"fieldManager,omitempty" protobuf:"bytes,3,name=fieldManager"` + + // fieldValidation determines how the server should respond to + // unknown/duplicate fields. + // TODO: Do we still need a protobuf tag if protobuf is not supported? + // Valid values are: + // - Ignore: ignore's unknown/duplicate fields + // - Strict: fail the request on unknown/duplicate fields + // - Warn: respond with a warning, but successfully serve the request. + // +optional + FieldValidation string `json:"fieldValidation,omitempty"` } // ApplyOptions may be provided when applying an API object. @@ -632,6 +652,16 @@ type UpdateOptions struct { // as defined by https://golang.org/pkg/unicode/#IsPrint. // +optional FieldManager string `json:"fieldManager,omitempty" protobuf:"bytes,2,name=fieldManager"` + + // fieldValidation determines how the server should respond to + // unknown/duplicate fields. + // TODO: Do we still need a protobuf tag if protobuf is not supported? + // Valid values are: + // - Ignore: ignore's unknown/duplicate fields + // - Strict: fail the request on unknown/duplicate fields + // - Warn: respond with a warning, but successfully serve the request. + // +optional + FieldValidation string `json:"fieldValidation,omitempty"` } // Preconditions must be fulfilled before an operation (update, delete, etc.) is carried out. diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/converter.go b/staging/src/k8s.io/apimachinery/pkg/runtime/converter.go index 4a6cc68574a45..9f5a5824e18ba 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/converter.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/converter.go @@ -103,6 +103,9 @@ type unstructuredConverter struct { mismatchDetection bool // comparison is the default test logic used to compare comparison conversion.Equalities + // fieldValidationDirective indicates what to do with unknown + // fields. It defaults to ignoring unkown fields. + fieldValidationDirective FieldValidationDirective } // NewTestUnstructuredConverter creates an UnstructuredConverter that accepts JSON typed maps and translates them @@ -115,6 +118,22 @@ func NewTestUnstructuredConverter(comparison conversion.Equalities) Unstructured } } +// FieldValidationDirective indicates what the apiserver should +// do with unknown fields (ignore, strict/error, or warn). +type FieldValidationDirective int + +const ( + IgnoreFieldValidation FieldValidationDirective = iota + StrictFieldValidation + WarnFieldValidation +) + +// SetFieldValidationDirective sets the directive for what the converter should do with +// unknown fields. +func (c *unstructuredConverter) SetFieldValidationDirective(directive FieldValidationDirective) { + c.fieldValidationDirective = directive +} + // FromUnstructured converts an object from map[string]interface{} representation into a concrete type. // It uses encoding/json/Unmarshaler if object implements it or reflection if not. func (c *unstructuredConverter) FromUnstructured(u map[string]interface{}, obj interface{}) error { @@ -123,7 +142,7 @@ func (c *unstructuredConverter) FromUnstructured(u map[string]interface{}, obj i if t.Kind() != reflect.Ptr || value.IsNil() { return fmt.Errorf("FromUnstructured requires a non-nil pointer to an object, got %v", t) } - err := fromUnstructured(reflect.ValueOf(u), value.Elem()) + err := c.fromUnstructured(reflect.ValueOf(u), value.Elem()) if c.mismatchDetection { newObj := reflect.New(t.Elem()).Interface() newErr := fromUnstructuredViaJSON(u, newObj) @@ -145,7 +164,7 @@ func fromUnstructuredViaJSON(u map[string]interface{}, obj interface{}) error { return json.Unmarshal(data, obj) } -func fromUnstructured(sv, dv reflect.Value) error { +func (c *unstructuredConverter) fromUnstructured(sv, dv reflect.Value) error { sv = unwrapInterface(sv) if !sv.IsValid() { dv.Set(reflect.Zero(dv.Type())) @@ -213,13 +232,15 @@ func fromUnstructured(sv, dv reflect.Value) error { switch dt.Kind() { case reflect.Map: - return mapFromUnstructured(sv, dv) + err := c.mapFromUnstructured(sv, dv) + return err case reflect.Slice: - return sliceFromUnstructured(sv, dv) + return c.sliceFromUnstructured(sv, dv) case reflect.Ptr: - return pointerFromUnstructured(sv, dv) + return c.pointerFromUnstructured(sv, dv) case reflect.Struct: - return structFromUnstructured(sv, dv) + err := c.structFromUnstructured(sv, dv) + return err case reflect.Interface: return interfaceFromUnstructured(sv, dv) default: @@ -275,7 +296,7 @@ func unwrapInterface(v reflect.Value) reflect.Value { return v } -func mapFromUnstructured(sv, dv reflect.Value) error { +func (c *unstructuredConverter) mapFromUnstructured(sv, dv reflect.Value) error { st, dt := sv.Type(), dv.Type() if st.Kind() != reflect.Map { return fmt.Errorf("cannot restore map from %v", st.Kind()) @@ -293,7 +314,7 @@ func mapFromUnstructured(sv, dv reflect.Value) error { for _, key := range sv.MapKeys() { value := reflect.New(dt.Elem()).Elem() if val := unwrapInterface(sv.MapIndex(key)); val.IsValid() { - if err := fromUnstructured(val, value); err != nil { + if err := c.fromUnstructured(val, value); err != nil { return err } } else { @@ -308,7 +329,7 @@ func mapFromUnstructured(sv, dv reflect.Value) error { return nil } -func sliceFromUnstructured(sv, dv reflect.Value) error { +func (c *unstructuredConverter) sliceFromUnstructured(sv, dv reflect.Value) error { st, dt := sv.Type(), dv.Type() if st.Kind() == reflect.String && dt.Elem().Kind() == reflect.Uint8 { // We store original []byte representation as string. @@ -341,14 +362,14 @@ func sliceFromUnstructured(sv, dv reflect.Value) error { } dv.Set(reflect.MakeSlice(dt, sv.Len(), sv.Cap())) for i := 0; i < sv.Len(); i++ { - if err := fromUnstructured(sv.Index(i), dv.Index(i)); err != nil { + if err := c.fromUnstructured(sv.Index(i), dv.Index(i)); err != nil { return err } } return nil } -func pointerFromUnstructured(sv, dv reflect.Value) error { +func (c *unstructuredConverter) pointerFromUnstructured(sv, dv reflect.Value) error { st, dt := sv.Type(), dv.Type() if st.Kind() == reflect.Ptr && sv.IsNil() { @@ -358,39 +379,96 @@ func pointerFromUnstructured(sv, dv reflect.Value) error { dv.Set(reflect.New(dt.Elem())) switch st.Kind() { case reflect.Ptr, reflect.Interface: - return fromUnstructured(sv.Elem(), dv.Elem()) + return c.fromUnstructured(sv.Elem(), dv.Elem()) default: - return fromUnstructured(sv, dv.Elem()) + return c.fromUnstructured(sv, dv.Elem()) } } -func structFromUnstructured(sv, dv reflect.Value) error { - st, dt := sv.Type(), dv.Type() - if st.Kind() != reflect.Map { - return fmt.Errorf("cannot restore struct from: %v", st.Kind()) - } - +// flattenedFields takes a value and returns all inlined fields as top +// level fields in order to determine which fields are invalid. +func flattenedFields(dv reflect.Value) map[reflect.Value]reflect.Value { + m := map[reflect.Value]reflect.Value{} + dt := dv.Type() for i := 0; i < dt.NumField(); i++ { fieldInfo := fieldInfoFromField(dt, i) fv := dv.Field(i) if len(fieldInfo.name) == 0 { - // This field is inlined. - if err := fromUnstructured(sv, fv); err != nil { - return err + //inlined, recurse + inlinedFields := flattenedFields(fv) + for k, v := range inlinedFields { + m[k] = v } } else { - value := unwrapInterface(sv.MapIndex(fieldInfo.nameValue)) - if value.IsValid() { - if err := fromUnstructured(value, fv); err != nil { - return err - } - } else { - fv.Set(reflect.Zero(fv.Type())) + m[fieldInfo.nameValue] = fv + } + } + return m +} + +func (c *unstructuredConverter) structFromUnstructured(sv, dv reflect.Value) error { + st := sv.Type() + if st.Kind() != reflect.Map { + return fmt.Errorf("cannot restore struct from: %v", st.Kind()) + } + + // TODO: benchmark whether this flatten step actually is less performant + // and thus we need to only conditionally do it for non-ignore case + dtFieldsForFieldName := flattenedFields(dv) + var strictDecodingErr error + if c.fieldValidationDirective != IgnoreFieldValidation { + fieldNameStrings := map[string]struct{}{} + for nameValue := range dtFieldsForFieldName { + fieldNameStrings[nameValue.String()] = struct{}{} + } + + // for every field in sv confirm that it exists in the + // flattened fields set of dv. + // If not, add it to the slice of unknown fields. + unknownFields := []reflect.Value{} + for _, key := range sv.MapKeys() { + if _, ok := fieldNameStrings[key.String()]; !ok { + unknownFields = append(unknownFields, key) + } + } + + // if there are unknown fields in sv + // return the decoding error immediately for the strict directive, + // but for the warn directive, save the error and return it after conversion. + if len(unknownFields) > 0 { + allStrictErrs := make([]error, len(unknownFields)) + for i, unknownField := range unknownFields { + allStrictErrs[i] = fmt.Errorf("unknown field: %s", unknownField.String()) + } + strictDecodingErr = NewStrictDecodingError(allStrictErrs) + if c.fieldValidationDirective == StrictFieldValidation { + return strictDecodingErr } } } - return nil + + for fieldName, fv := range dtFieldsForFieldName { + value := unwrapInterface(sv.MapIndex(fieldName)) + if value.IsValid() { + if err := c.fromUnstructured(value, fv); err != nil { + return err + } + } else { + fv.Set(reflect.Zero(fv.Type())) + } + } + return strictDecodingErr +} + +func deleteFromKeys(name string, keys *[]reflect.Value) { + for i := len(*keys) - 1; i >= 0; i-- { + if name == (*keys)[i].String() { + // Delete from keys + *keys = append((*keys)[:i], (*keys)[i+1:]...) + return + } + } } func interfaceFromUnstructured(sv, dv reflect.Value) error { diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/codec_factory.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/codec_factory.go index e55ab94d1475b..ceca12fac7aff 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/codec_factory.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/codec_factory.go @@ -159,7 +159,9 @@ func DisableStrict(options *CodecFactoryOptions) { // TODO: allow other codecs to be compiled in? // TODO: accept a scheme interface func NewCodecFactory(scheme *runtime.Scheme, mutators ...CodecFactoryOptionsMutator) CodecFactory { - options := CodecFactoryOptions{Pretty: true} + // default to strict decoding, callers of decode are responsible for + // distinguishing fatal decoding errors from strict decoding errors. + options := CodecFactoryOptions{Pretty: true, Strict: true} for _, fn := range mutators { fn(&options) } diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/versioning/versioning.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/versioning/versioning.go index 718c5dfb7df75..a8d43493e9064 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/versioning/versioning.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/versioning/versioning.go @@ -134,9 +134,11 @@ func (c *codec) Decode(data []byte, defaultGVK *schema.GroupVersionKind, into ru } obj, gvk, err := c.decoder.Decode(data, defaultGVK, decodeInto) - if err != nil { + if err != nil && !runtime.IsStrictDecodingError(err) { return nil, gvk, err } + // save the strictDecodingError and the caller decide what to do with it + strictDecodingErr := err if d, ok := obj.(runtime.NestedObjectDecoder); ok { if err := d.DecodeNestedObjects(runtime.WithoutVersionDecoder{c.decoder}); err != nil { @@ -153,14 +155,14 @@ func (c *codec) Decode(data []byte, defaultGVK *schema.GroupVersionKind, into ru // Short-circuit conversion if the into object is same object if into == obj { - return into, gvk, nil + return into, gvk, strictDecodingErr } if err := c.convertor.Convert(obj, into, c.decodeVersion); err != nil { return nil, gvk, err } - return into, gvk, nil + return into, gvk, strictDecodingErr } // perform defaulting if requested @@ -172,7 +174,7 @@ func (c *codec) Decode(data []byte, defaultGVK *schema.GroupVersionKind, into ru if err != nil { return nil, gvk, err } - return out, gvk, nil + return out, gvk, strictDecodingErr } // Encode ensures the provided object is output in the appropriate group and version, invoking diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/types.go b/staging/src/k8s.io/apimachinery/pkg/runtime/types.go index 31359f35f4512..d8d8bf633ed28 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/types.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/types.go @@ -41,9 +41,12 @@ type TypeMeta struct { } const ( - ContentTypeJSON string = "application/json" - ContentTypeYAML string = "application/yaml" - ContentTypeProtobuf string = "application/vnd.kubernetes.protobuf" + ContentTypeJSON string = "application/json" + ContentTypeJSONPatch string = "application/json-patch+json" + ContentTypeJSONMergePatch string = "application/merge-patch+json" + ContentTypeJSONStrategicMergePatch string = "application/strategic-merge-patch+json" + ContentTypeYAML string = "application/yaml" + ContentTypeProtobuf string = "application/vnd.kubernetes.protobuf" ) // RawExtension is used to hold extensions in external versions. diff --git a/staging/src/k8s.io/apimachinery/pkg/util/net/http.go b/staging/src/k8s.io/apimachinery/pkg/util/net/http.go index 42d66d3164ab3..2106ccf9c2047 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/net/http.go +++ b/staging/src/k8s.io/apimachinery/pkg/util/net/http.go @@ -556,6 +556,10 @@ type WarningHeader struct { Text string } +func (w WarningHeader) String() string { + return fmt.Sprintf("%03d %s %s", w.Code, w.Agent, w.Text) +} + // ParseWarningHeaders extract RFC2616 14.46 warnings headers from the specified set of header values. // Multiple comma-separated warnings per header are supported. // If errors are encountered on a header, the remainder of that header are skipped and subsequent headers are parsed. diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go index 3f0ee9c5c74b4..1a9cbb85c5ab0 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go @@ -4351,9 +4351,9 @@ func TestUpdateChecksAPIVersion(t *testing.T) { } } -// runRequest is used by TestDryRun since it runs the test twice in a -// row with a slightly different URL (one has ?dryRun, one doesn't). -func runRequest(t *testing.T, path, verb string, data []byte, contentType string) *http.Response { +// runRequest is used by TestDryRun and TestFieldValidation since it runs the test +// twice in a row with a slightly different URL (one has ?dryRun, one doesn't). +func runRequest(t testing.TB, path, verb string, data []byte, contentType string) *http.Response { request, err := http.NewRequest(verb, path, bytes.NewBuffer(data)) if err != nil { t.Fatalf("unexpected error: %v", err) @@ -4388,6 +4388,116 @@ func (storage *SimpleRESTStorageWithDeleteCollection) DeleteCollection(ctx conte return nil, nil } +func TestFieldValidation(t *testing.T) { + strictDecoderErr := "strict decoder error for" + badRequestErr := "fieldValidation parameter only supports content types" + strictFieldValidation := "?fieldValidation=strict" + // TODO: add test cases for yaml validation, multiple fieldValidation values + tests := []struct { + name string + path string + verb string + data []byte + queryParams string + contentType string + errContains string + }{ + + {name: "post-unknown-strict-validation", path: "/namespaces/default/simples", verb: "POST", data: []byte(`{"kind":"Simple","apiVersion":"test.group/version","metadata":{"creationTimestamp":null},"other":"bar","unknown":"baz"}`), queryParams: strictFieldValidation, errContains: strictDecoderErr}, + {name: "put-unknown-strict-validation", path: "/namespaces/default/simples/id", verb: "PUT", data: []byte(`{"kind": "Simple", "apiVersion": "test.group/version", "metadata": {"name": "id", "creationTimestamp": null}, "other": "bar", "unknown": "baz"}`), queryParams: strictFieldValidation, errContains: strictDecoderErr}, + {name: "post-unknown-ignore-validation", path: "/namespaces/default/simples", verb: "POST", data: []byte(`{"kind":"Simple","apiVersion":"test.group/version","metadata":{"creationTimestamp":null},"other":"bar","unknown":"baz"}`)}, + {name: "put-unknown-ignore-validation", path: "/namespaces/default/simples/id", verb: "PUT", data: []byte(`{"kind": "Simple", "apiVersion": "test.group/version", "metadata": {"name": "id", "creationTimestamp": null}, "other": "bar", "unknown": "baz"}`)}, + {name: "post-unknown-strict-vaidation-json", path: "/namespaces/default/simples", verb: "POST", data: []byte(`{"kind":"Simple","apiVersion":"test.group/version","metadata":{"creationTimestamp":null},"other":"bar","unknown":"baz"}`), queryParams: strictFieldValidation, errContains: strictDecoderErr, contentType: runtime.ContentTypeJSON}, + {name: "post-unknown-strict-validation-protobuf", path: "/namespaces/default/simples", verb: "POST", data: []byte(`{"kind":"Simple","apiVersion":"test.group/version","metadata":{"creationTimestamp":null},"other":"bar","unknown":"baz"}`), queryParams: strictFieldValidation, errContains: badRequestErr, contentType: runtime.ContentTypeProtobuf}, + + // TODO: there's a bug currently where query params are being stripped so this test does not pass yet. + //{path: "/namespaces/default/simples/id", verb: "PATCH", data: []byte(`{"labels":{"foo":"bar"}}`), contentType: "application/merge-patch+json; charset=UTF-8", errContains: notImplementedErr}, + } + + server := httptest.NewServer(handle(map[string]rest.Storage{ + "simples": &SimpleRESTStorageWithDeleteCollection{ + SimpleRESTStorage{ + item: genericapitesting.Simple{ + ObjectMeta: metav1.ObjectMeta{ + Name: "id", + Namespace: "", + UID: "uid", + }, + Other: "bar", + }, + }, + }, + "simples/subsimple": &SimpleXGSubresourceRESTStorage{ + item: genericapitesting.SimpleXGSubresource{ + SubresourceInfo: "foo", + }, + itemGVK: testGroup2Version.WithKind("SimpleXGSubresource"), + }, + })) + defer server.Close() + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + baseURL := server.URL + "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + response := runRequest(t, baseURL+test.path+test.queryParams, test.verb, test.data, test.contentType) + buf := new(bytes.Buffer) + buf.ReadFrom(response.Body) + // TODO: better way of doing than string comparison since we are getting a response instead of a regular go error? + if response.StatusCode != http.StatusBadRequest && !strings.Contains(buf.String(), test.errContains) { + t.Fatalf("unexpected response: %#v, errContains: %#v", response, test.errContains) + } + }) + } + +} + +func BenchmarkFieldValidation(b *testing.B) { + benchmarks := []struct { + name string + queryParam string + }{ + {"nonstrict simples", ""}, + {"strict simples", "?fieldValidation=Strict"}, + } + server := httptest.NewServer(handle(map[string]rest.Storage{ + "simples": &SimpleRESTStorageWithDeleteCollection{ + SimpleRESTStorage{ + item: genericapitesting.Simple{ + ObjectMeta: metav1.ObjectMeta{ + Name: "id", + Namespace: "", + UID: "uid", + }, + Other: "bar", + }, + }, + }, + "simples/subsimple": &SimpleXGSubresourceRESTStorage{ + item: genericapitesting.SimpleXGSubresource{ + SubresourceInfo: "foo", + }, + itemGVK: testGroup2Version.WithKind("SimpleXGSubresource"), + }, + })) + defer server.Close() + + for _, bm := range benchmarks { + b.Run(bm.name, func(b *testing.B) { + for n := 0; n < b.N; n++ { + postPath := "/namespaces/default/simples" + postData := []byte(`{"kind":"Simple","apiVersion":"test.group/version","metadata":{"creationTimestamp":null},"other":"bar"}`) + basePostURL := server.URL + "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + _ = runRequest(b, basePostURL+postPath+bm.queryParam, "POST", postData, "") + + putPath := "/namespaces/default/simples/id" + putData := []byte(`{"kind": "Simple", "apiVersion": "test.group/version", "metadata": {"name": "id", "creationTimestamp": null}, "other": "bar", "unknown": "baz"}`) + basePutURL := server.URL + "/" + prefix + "/" + testGroupVersion.Group + "/" + testGroupVersion.Version + _ = runRequest(b, basePutURL+putPath+bm.queryParam, "PUT", putData, "") + } + }) + + } +} + func TestDryRunDisabled(t *testing.T) { defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.DryRun, false)() diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go index 51ad05bf16baf..b7541ab90f8bc 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/create.go @@ -43,6 +43,7 @@ import ( "k8s.io/apiserver/pkg/registry/rest" "k8s.io/apiserver/pkg/util/dryrun" utilfeature "k8s.io/apiserver/pkg/util/feature" + "k8s.io/apiserver/pkg/warning" utiltrace "k8s.io/utils/trace" ) @@ -92,8 +93,6 @@ func createHandler(r rest.NamedCreater, scope *RequestScope, admit admission.Int return } - decoder := scope.Serializer.DecoderToVersion(s.Serializer, scope.HubGroupVersion) - body, err := limitedReadBody(req, scope.MaxRequestBodyBytes) if err != nil { scope.err(err, w, req) @@ -116,14 +115,27 @@ func createHandler(r rest.NamedCreater, scope *RequestScope, admit admission.Int defaultGVK := scope.Kind original := r.New() - trace.Step("About to convert to expected version") - obj, gvk, err := decoder.Decode(body, &defaultGVK, original) + + validationDirective, err := fieldValidation(req) if err != nil { - err = transformDecodeError(scope.Typer, err, original, gvk, body) scope.err(err, w, req) return } + decoder := scope.Serializer.DecoderToVersion(s.Serializer, scope.HubGroupVersion) + trace.Step("About to convert to expected version") + obj, gvk, err := decoder.Decode(body, &defaultGVK, original) + if err != nil { + if !runtime.IsStrictDecodingError(err) || validationDirective == runtime.StrictFieldValidation { + err = transformDecodeError(scope.Typer, err, original, gvk, body) + scope.err(err, w, req) + return + } + if validationDirective == runtime.WarnFieldValidation { + warning.AddWarning(req.Context(), "", err.Error()) + } + } + objGV := gvk.GroupVersion() if !scope.AcceptsGroupVersion(objGV) { err = errors.NewBadRequest(fmt.Sprintf("the API version in the data (%s) does not match the expected API version (%v)", objGV.String(), gv.String())) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go index 1138bc0aee3bc..cd4b19db8fe25 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/patch.go @@ -50,6 +50,7 @@ import ( "k8s.io/apiserver/pkg/registry/rest" "k8s.io/apiserver/pkg/util/dryrun" utilfeature "k8s.io/apiserver/pkg/util/feature" + "k8s.io/apiserver/pkg/warning" utiltrace "k8s.io/utils/trace" ) @@ -140,6 +141,12 @@ func PatchResource(r rest.Patcher, scope *RequestScope, admit admission.Interfac } gv := scope.Kind.GroupVersion() + validationDirective, err := fieldValidation(req) + if err != nil { + scope.err(err, w, req) + return + } + codec := runtime.NewCodec( scope.Serializer.EncoderForVersion(s.Serializer, gv), scope.Serializer.DecoderToVersion(s.Serializer, scope.HubGroupVersion), @@ -190,15 +197,16 @@ func PatchResource(r rest.Patcher, scope *RequestScope, admit admission.Interfac } p := patcher{ - namer: scope.Namer, - creater: scope.Creater, - defaulter: scope.Defaulter, - typer: scope.Typer, - unsafeConvertor: scope.UnsafeConvertor, - kind: scope.Kind, - resource: scope.Resource, - subresource: scope.Subresource, - dryRun: dryrun.IsDryRun(options.DryRun), + namer: scope.Namer, + creater: scope.Creater, + defaulter: scope.Defaulter, + typer: scope.Typer, + unsafeConvertor: scope.UnsafeConvertor, + kind: scope.Kind, + resource: scope.Resource, + subresource: scope.Subresource, + dryRun: dryrun.IsDryRun(options.DryRun), + validationDirective: validationDirective, objectInterfaces: scope, @@ -251,15 +259,16 @@ type mutateObjectUpdateFunc func(ctx context.Context, obj, old runtime.Object) e // moved into this type. type patcher struct { // Pieces of RequestScope - namer ScopeNamer - creater runtime.ObjectCreater - defaulter runtime.ObjectDefaulter - typer runtime.ObjectTyper - unsafeConvertor runtime.ObjectConvertor - resource schema.GroupVersionResource - kind schema.GroupVersionKind - subresource string - dryRun bool + namer ScopeNamer + creater runtime.ObjectCreater + defaulter runtime.ObjectDefaulter + typer runtime.ObjectTyper + unsafeConvertor runtime.ObjectConvertor + resource schema.GroupVersionResource + kind schema.GroupVersionKind + subresource string + dryRun bool + validationDirective runtime.FieldValidationDirective objectInterfaces admission.ObjectInterfaces @@ -291,7 +300,7 @@ type patcher struct { } type patchMechanism interface { - applyPatchToCurrentObject(currentObject runtime.Object) (runtime.Object, error) + applyPatchToCurrentObject(requextContext context.Context, currentObject runtime.Object) (runtime.Object, error) createNewObject() (runtime.Object, error) } @@ -301,7 +310,7 @@ type jsonPatcher struct { fieldManager *fieldmanager.FieldManager } -func (p *jsonPatcher) applyPatchToCurrentObject(currentObject runtime.Object) (runtime.Object, error) { +func (p *jsonPatcher) applyPatchToCurrentObject(requestContext context.Context, currentObject runtime.Object) (runtime.Object, error) { // Encode will convert & return a versioned object in JSON. currentObjJS, err := runtime.Encode(p.codec, currentObject) if err != nil { @@ -317,9 +326,14 @@ func (p *jsonPatcher) applyPatchToCurrentObject(currentObject runtime.Object) (r // Construct the resulting typed, unversioned object. objToUpdate := p.restPatcher.New() if err := runtime.DecodeInto(p.codec, patchedObjJS, objToUpdate); err != nil { - return nil, errors.NewInvalid(schema.GroupKind{}, "", field.ErrorList{ - field.Invalid(field.NewPath("patch"), string(patchedObjJS), err.Error()), - }) + if !runtime.IsStrictDecodingError(err) || p.validationDirective == runtime.StrictFieldValidation { + return nil, errors.NewInvalid(schema.GroupKind{}, "", field.ErrorList{ + field.Invalid(field.NewPath("patch"), string(patchedObjJS), err.Error()), + }) + } + if p.validationDirective == runtime.WarnFieldValidation { + warning.AddWarning(requestContext, "", err.Error()) + } } if p.fieldManager != nil { @@ -385,7 +399,7 @@ type smpPatcher struct { fieldManager *fieldmanager.FieldManager } -func (p *smpPatcher) applyPatchToCurrentObject(currentObject runtime.Object) (runtime.Object, error) { +func (p *smpPatcher) applyPatchToCurrentObject(requestContext context.Context, currentObject runtime.Object) (runtime.Object, error) { // Since the patch is applied on versioned objects, we need to convert the // current object to versioned representation first. currentVersionedObject, err := p.unsafeConvertor.ConvertToVersion(currentObject, p.kind.GroupVersion()) @@ -396,7 +410,7 @@ func (p *smpPatcher) applyPatchToCurrentObject(currentObject runtime.Object) (ru if err != nil { return nil, err } - if err := strategicPatchObject(p.defaulter, currentVersionedObject, p.patchBytes, versionedObjToUpdate, p.schemaReferenceObj); err != nil { + if err := strategicPatchObject(requestContext, p.defaulter, currentVersionedObject, p.patchBytes, versionedObjToUpdate, p.schemaReferenceObj, p.validationDirective); err != nil { return nil, err } // Convert the object back to the hub version @@ -424,7 +438,7 @@ type applyPatcher struct { userAgent string } -func (p *applyPatcher) applyPatchToCurrentObject(obj runtime.Object) (runtime.Object, error) { +func (p *applyPatcher) applyPatchToCurrentObject(_ context.Context, obj runtime.Object) (runtime.Object, error) { force := false if p.options.Force != nil { force = *p.options.Force @@ -446,7 +460,7 @@ func (p *applyPatcher) createNewObject() (runtime.Object, error) { if err != nil { return nil, fmt.Errorf("failed to create new object: %v", err) } - return p.applyPatchToCurrentObject(obj) + return p.applyPatchToCurrentObject(context.TODO(), obj) } // strategicPatchObject applies a strategic merge patch of to @@ -455,11 +469,13 @@ func (p *applyPatcher) createNewObject() (runtime.Object, error) { // and . // NOTE: Both and are supposed to be versioned. func strategicPatchObject( + requestContext context.Context, defaulter runtime.ObjectDefaulter, originalObject runtime.Object, patchBytes []byte, objToUpdate runtime.Object, schemaReferenceObj runtime.Object, + validationDirective runtime.FieldValidationDirective, ) error { originalObjMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(originalObject) if err != nil { @@ -471,7 +487,7 @@ func strategicPatchObject( return errors.NewBadRequest(err.Error()) } - if err := applyPatchToObject(defaulter, originalObjMap, patchMap, objToUpdate, schemaReferenceObj); err != nil { + if err := applyPatchToObject(requestContext, defaulter, originalObjMap, patchMap, objToUpdate, schemaReferenceObj, validationDirective); err != nil { return err } return nil @@ -480,7 +496,7 @@ func strategicPatchObject( // applyPatch is called every time GuaranteedUpdate asks for the updated object, // and is given the currently persisted object as input. // TODO: rename this function because the name implies it is related to applyPatcher -func (p *patcher) applyPatch(_ context.Context, _, currentObject runtime.Object) (objToUpdate runtime.Object, patchErr error) { +func (p *patcher) applyPatch(ctx context.Context, _, currentObject runtime.Object) (objToUpdate runtime.Object, patchErr error) { // Make sure we actually have a persisted currentObject p.trace.Step("About to apply patch") currentObjectHasUID, err := hasUID(currentObject) @@ -489,7 +505,7 @@ func (p *patcher) applyPatch(_ context.Context, _, currentObject runtime.Object) } else if !currentObjectHasUID { objToUpdate, patchErr = p.mechanism.createNewObject() } else { - objToUpdate, patchErr = p.mechanism.applyPatchToCurrentObject(currentObject) + objToUpdate, patchErr = p.mechanism.applyPatchToCurrentObject(ctx, currentObject) } if patchErr != nil { @@ -618,11 +634,13 @@ func (p *patcher) patchResource(ctx context.Context, scope *RequestScope) (runti // and stores the result in . // NOTE: must be a versioned object. func applyPatchToObject( + requestContext context.Context, defaulter runtime.ObjectDefaulter, originalMap map[string]interface{}, patchMap map[string]interface{}, objToUpdate runtime.Object, schemaReferenceObj runtime.Object, + validationDirective runtime.FieldValidationDirective, ) error { patchedObjMap, err := strategicpatch.StrategicMergeMapPatch(originalMap, patchMap, schemaReferenceObj) if err != nil { @@ -630,10 +648,17 @@ func applyPatchToObject( } // Rather than serialize the patched map to JSON, then decode it to an object, we go directly from a map to an object - if err := runtime.DefaultUnstructuredConverter.FromUnstructured(patchedObjMap, objToUpdate); err != nil { - return errors.NewInvalid(schema.GroupKind{}, "", field.ErrorList{ - field.Invalid(field.NewPath("patch"), fmt.Sprintf("%+v", patchMap), err.Error()), - }) + converter := runtime.DefaultUnstructuredConverter + converter.SetFieldValidationDirective(validationDirective) + if err := converter.FromUnstructured(patchedObjMap, objToUpdate); err != nil { + if !runtime.IsStrictDecodingError(err) || validationDirective == runtime.StrictFieldValidation { + return errors.NewInvalid(schema.GroupKind{}, "", field.ErrorList{ + field.Invalid(field.NewPath("patch"), fmt.Sprintf("%+v", patchMap), err.Error()), + }) + } + if validationDirective == runtime.WarnFieldValidation { + warning.AddWarning(requestContext, "", err.Error()) + } } // Decoding from JSON to a versioned object would apply defaults, so we do the same here defaulter.Default(objToUpdate) diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go index ef43939be0713..58596b8608ae6 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/rest.go @@ -22,6 +22,7 @@ import ( "fmt" "io" "io/ioutil" + "mime" "net/http" "net/url" "strings" @@ -457,6 +458,65 @@ func isDryRun(url *url.URL) bool { return len(url.Query()["dryRun"]) != 0 } +// fieldValidation checks if the fieldValidation query parameter is set on the request, +// and if so ensures that the parameter is valid and that the request has a valid +// media type, because the list of media types that support field validation are a subset of +// all supported media types (protobuf does not support field validation). +func fieldValidation(req *http.Request) (runtime.FieldValidationDirective, error) { + if !utilfeature.DefaultFeatureGate.Enabled(features.FieldValidation) { + return runtime.IgnoreFieldValidation, nil + } + + // TODO: Should we blocklist unsupportedContentTypes (just protobuf) rather than allowlisting everything that isn't protobuf? + // TODO: Is there a better way to determine if something is JSON or YAML by its media type suffix rather than adding all these + // ContentTypes to the runtime package? + supportedContentTypes := []string{runtime.ContentTypeJSON, runtime.ContentTypeJSONPatch, runtime.ContentTypeJSONMergePatch, runtime.ContentTypeJSONStrategicMergePatch, runtime.ContentTypeYAML} + contentType := req.Header.Get("Content-Type") + // TODO: Is it okay to assume empty content type is a valid one? + supported := true + if contentType != "" { + supported = false + for _, v := range strings.Split(contentType, ",") { + t, _, err := mime.ParseMediaType(v) + if err != nil { + return runtime.IgnoreFieldValidation, errors.NewBadRequest(fmt.Sprintf("could not parse media type: %v", v)) + } + for _, mt := range supportedContentTypes { + if t == mt { + supported = true + break + } + } + } + } + + validationParam := req.URL.Query()["fieldValidation"] + switch len(validationParam) { + case 0: + return runtime.IgnoreFieldValidation, nil + case 1: + switch strings.ToLower(validationParam[0]) { + case "ignore": + return runtime.IgnoreFieldValidation, nil + case "strict": + if !supported { + return runtime.IgnoreFieldValidation, errors.NewBadRequest(fmt.Sprintf("fieldValidation parameter only supports content types %v\n content type provided: %s", supportedContentTypes, contentType)) + } + return runtime.StrictFieldValidation, nil + case "warn": + if !supported { + return runtime.IgnoreFieldValidation, errors.NewBadRequest(fmt.Sprintf("fieldValidation parameter only supports content types %v\n content type provided: %s", supportedContentTypes, contentType)) + } + return runtime.WarnFieldValidation, nil + default: + return runtime.IgnoreFieldValidation, errors.NewBadRequest(fmt.Sprintf("fieldValidation parameter unsupported: %v", validationParam)) + } + default: + return runtime.IgnoreFieldValidation, errors.NewBadRequest(fmt.Sprintf("fieldValidation should only be one value: %v", validationParam)) + + } +} + type etcdError interface { Code() grpccodes.Code Error() string diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go index bfd8cb4b51473..eae5efb2e0cc4 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go +++ b/staging/src/k8s.io/apiserver/pkg/endpoints/handlers/update.go @@ -41,6 +41,7 @@ import ( "k8s.io/apiserver/pkg/registry/rest" "k8s.io/apiserver/pkg/util/dryrun" utilfeature "k8s.io/apiserver/pkg/util/feature" + "k8s.io/apiserver/pkg/warning" utiltrace "k8s.io/utils/trace" ) @@ -102,14 +103,26 @@ func UpdateResource(r rest.Updater, scope *RequestScope, admit admission.Interfa defaultGVK := scope.Kind original := r.New() - trace.Step("About to convert to expected version") - decoder := scope.Serializer.DecoderToVersion(s.Serializer, scope.HubGroupVersion) - obj, gvk, err := decoder.Decode(body, &defaultGVK, original) + validationDirective, err := fieldValidation(req) if err != nil { - err = transformDecodeError(scope.Typer, err, original, gvk, body) scope.err(err, w, req) return } + + decoder := scope.Serializer.DecoderToVersion(s.Serializer, scope.HubGroupVersion) + trace.Step("About to convert to expected version") + obj, gvk, err := decoder.Decode(body, &defaultGVK, original) + if err != nil { + if !runtime.IsStrictDecodingError(err) || validationDirective == runtime.StrictFieldValidation { + err = transformDecodeError(scope.Typer, err, original, gvk, body) + scope.err(err, w, req) + return + } + if validationDirective == runtime.WarnFieldValidation { + warning.AddWarning(req.Context(), "", err.Error()) + } + } + objGV := gvk.GroupVersion() if !scope.AcceptsGroupVersion(objGV) { err = errors.NewBadRequest(fmt.Sprintf("the API version in the data (%s) does not match the expected API version (%s)", objGV, defaultGVK.GroupVersion())) diff --git a/staging/src/k8s.io/apiserver/pkg/features/kube_features.go b/staging/src/k8s.io/apiserver/pkg/features/kube_features.go index d0da61e57b955..29768ea9015f0 100644 --- a/staging/src/k8s.io/apiserver/pkg/features/kube_features.go +++ b/staging/src/k8s.io/apiserver/pkg/features/kube_features.go @@ -185,6 +185,13 @@ const ( // // Enables expression validation for Custom Resource CustomResourceValidationExpressions featuregate.Feature = "CustomResourceValidationExpressions" + + // owner: @kevindelgado + // kep: http://kep.k8s.io/2885 + // alpha: v1.23 + // + // Enables server-side field validation. + FieldValidation featuregate.Feature = "FieldValidation" ) func init() { @@ -215,4 +222,5 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS APIServerTracing: {Default: false, PreRelease: featuregate.Alpha}, OpenAPIEnums: {Default: false, PreRelease: featuregate.Alpha}, CustomResourceValidationExpressions: {Default: false, PreRelease: featuregate.Alpha}, + FieldValidation: {Default: false, PreRelease: featuregate.Alpha}, } diff --git a/test/integration/apiserver/apiserver_test.go b/test/integration/apiserver/apiserver_test.go index f40d97683f8a6..dd4c61732ad16 100644 --- a/test/integration/apiserver/apiserver_test.go +++ b/test/integration/apiserver/apiserver_test.go @@ -71,19 +71,19 @@ import ( "k8s.io/kubernetes/test/integration/framework" ) -func setup(t *testing.T, groupVersions ...schema.GroupVersion) (*httptest.Server, clientset.Interface, framework.CloseFunc) { +func setup(t testing.TB, groupVersions ...schema.GroupVersion) (*httptest.Server, clientset.Interface, framework.CloseFunc) { return setupWithResources(t, groupVersions, nil) } -func setupWithOptions(t *testing.T, opts *framework.ControlPlaneConfigOptions, groupVersions ...schema.GroupVersion) (*httptest.Server, clientset.Interface, framework.CloseFunc) { +func setupWithOptions(t testing.TB, opts *framework.ControlPlaneConfigOptions, groupVersions ...schema.GroupVersion) (*httptest.Server, clientset.Interface, framework.CloseFunc) { return setupWithResourcesWithOptions(t, opts, groupVersions, nil) } -func setupWithResources(t *testing.T, groupVersions []schema.GroupVersion, resources []schema.GroupVersionResource) (*httptest.Server, clientset.Interface, framework.CloseFunc) { +func setupWithResources(t testing.TB, groupVersions []schema.GroupVersion, resources []schema.GroupVersionResource) (*httptest.Server, clientset.Interface, framework.CloseFunc) { return setupWithResourcesWithOptions(t, &framework.ControlPlaneConfigOptions{}, groupVersions, resources) } -func setupWithResourcesWithOptions(t *testing.T, opts *framework.ControlPlaneConfigOptions, groupVersions []schema.GroupVersion, resources []schema.GroupVersionResource) (*httptest.Server, clientset.Interface, framework.CloseFunc) { +func setupWithResourcesWithOptions(t testing.TB, opts *framework.ControlPlaneConfigOptions, groupVersions []schema.GroupVersion, resources []schema.GroupVersionResource) (*httptest.Server, clientset.Interface, framework.CloseFunc) { controlPlaneConfig := framework.NewIntegrationTestControlPlaneConfigWithOptions(opts) if len(groupVersions) > 0 || len(resources) > 0 { resourceConfig := controlplane.DefaultAPIResourceConfigSource() diff --git a/test/integration/apiserver/field_validation_test.go b/test/integration/apiserver/field_validation_test.go new file mode 100644 index 0000000000000..3e9ed9cab26d4 --- /dev/null +++ b/test/integration/apiserver/field_validation_test.go @@ -0,0 +1,687 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package apiserver + +import ( + "context" + "encoding/json" + "flag" + "fmt" + "os" + "strings" + "testing" + "time" + + apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" + apiextensionsclient "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset" + "k8s.io/apiextensions-apiserver/test/integration/fixtures" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apiserver/pkg/features" + utilfeature "k8s.io/apiserver/pkg/util/feature" + "k8s.io/client-go/dynamic" + clientset "k8s.io/client-go/kubernetes" + restclient "k8s.io/client-go/rest" + featuregatetesting "k8s.io/component-base/featuregate/testing" + "k8s.io/klog/v2" + kubeapiservertesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing" + + "k8s.io/kubernetes/test/integration/framework" +) + +// TestFieldValidationPost tests POST requests containing unknown fields with +// strict and non-strict field validation. +func TestFieldValidationPost(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.FieldValidation, true)() + + _, client, closeFn := setup(t) + defer closeFn() + + bodyBytes, err := os.ReadFile("./testdata/deploy-small-unknown-field.json") + if err != nil { + t.Fatalf("failed to read file: %v", err) + } + + var testcases = []struct { + name string + opts metav1.CreateOptions + errContains string + warnContains string + }{ + { + name: "post-strict-validation", + errContains: "unknown field", + opts: metav1.CreateOptions{ + FieldValidation: "Strict", + }, + }, + { + name: "post-warn-validation", + warnContains: "unknown field", + opts: metav1.CreateOptions{ + FieldValidation: "Warn", + }, + }, + { + name: "post-ignore-validation", + opts: metav1.CreateOptions{ + FieldValidation: "Ignore", + }, + }, + { + name: "post-default-ignore-validation", + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + body := []byte(fmt.Sprintf(string(bodyBytes), fmt.Sprintf(`"test-deployment-%s"`, tc.name))) + req := client.CoreV1().RESTClient().Post(). + AbsPath("/apis/apps/v1"). + Namespace("default"). + Resource("deployments"). + VersionedParams(&tc.opts, metav1.ParameterCodec) + result := req.Body([]byte(body)).Do(context.TODO()) + if tc.warnContains != "" { + warningMatched := false + for _, w := range result.Warnings() { + if strings.Contains(w.String(), tc.warnContains) { + warningMatched = true + } + } + if !warningMatched { + t.Fatalf("expected warning to contain: %s, got warnings: %v", tc.warnContains, result.Warnings()) + } + } + resBody, err := result.Raw() + if err == nil && tc.errContains != "" { + t.Fatalf("unexpected post succeeded") + } + //if err != nil && !strings.Contains(string(resBody), tc.errContains) { + if err != nil && (tc.errContains == "" || !strings.Contains(string(resBody), tc.errContains)) { + t.Fatalf("unexpected response: %v", string(resBody)) + } + }) + } +} + +// TestFieldValidationPut tests PUT requests containing unknown fields with +// strict and non-strict field validation. +func TestFieldValidationPut(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.FieldValidation, true)() + + _, client, closeFn := setup(t) + defer closeFn() + + deployName := `"test-deployment"` + postBytes, err := os.ReadFile("./testdata/deploy-small.json") + if err != nil { + t.Fatalf("failed to read file: %v", err) + } + postBody := []byte(fmt.Sprintf(string(postBytes), deployName)) + + if _, err := client.CoreV1().RESTClient().Post(). + AbsPath("/apis/apps/v1"). + Namespace("default"). + Resource("deployments"). + Body(postBody). + DoRaw(context.TODO()); err != nil { + t.Fatalf("failed to create initial deployment: %v", err) + } + + putBytes, err := os.ReadFile("./testdata/deploy-small-unknown-field.json") + if err != nil { + t.Fatalf("failed to read file: %v", err) + } + putBody := []byte(fmt.Sprintf(string(putBytes), deployName)) + var testcases = []struct { + name string + opts metav1.UpdateOptions + errContains string + warnContains string + }{ + { + name: "put-strict-validation", + opts: metav1.UpdateOptions{ + FieldValidation: "Strict", + }, + errContains: "unknown field", + }, + { + name: "put-warn-validation", + opts: metav1.UpdateOptions{ + FieldValidation: "Warn", + }, + warnContains: "unknown field", + }, + { + name: "put-default-ignore-validation", + opts: metav1.UpdateOptions{ + FieldValidation: "Ignore", + }, + }, + { + name: "put-ignore-validation", + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + req := client.CoreV1().RESTClient().Put(). + AbsPath("/apis/apps/v1"). + Namespace("default"). + Resource("deployments"). + Name("test-deployment"). + VersionedParams(&tc.opts, metav1.ParameterCodec) + result := req.Body([]byte(putBody)).Do(context.TODO()) + if tc.warnContains != "" { + warningMatched := false + for _, w := range result.Warnings() { + if strings.Contains(w.String(), tc.warnContains) { + warningMatched = true + } + } + if !warningMatched { + t.Fatalf("expected warning to contain: %s, got warnings: %v", tc.warnContains, result.Warnings()) + } + } + resBody, err := result.Raw() + if err == nil && tc.errContains != "" { + t.Fatalf("unexpected put succeeded") + } + //if err != nil && !strings.Contains(string(resBody), tc.errContains) { + if err != nil && (tc.errContains == "" || !strings.Contains(string(resBody), tc.errContains)) { + t.Fatalf("unexpected response: %v", string(resBody)) + } + }) + } +} + +// Benchmark field validation for strict vs non-strict +func BenchmarkFieldValidationPostPut(b *testing.B) { + defer featuregatetesting.SetFeatureGateDuringTest(b, utilfeature.DefaultFeatureGate, features.FieldValidation, true)() + + _, client, closeFn := setup(b) + defer closeFn() + + flag.Lookup("v").Value.Set("0") + corePath := "/api/v1" + appsPath := "/apis/apps/v1" + // TODO: split POST and PUT into their own test-cases. + // TODO: add test for "Warn" validation once it is implemented. + benchmarks := []struct { + name string + params map[string]string + bodyFile string + resource string + absPath string + }{ + { + name: "ignore-validation-deployment", + params: map[string]string{"fieldValidation": "Ignore"}, + bodyFile: "./testdata/deploy-small.json", + resource: "deployments", + absPath: appsPath, + }, + { + name: "strict-validation-deployment", + params: map[string]string{"fieldValidation": "Strict"}, + bodyFile: "./testdata/deploy-small.json", + resource: "deployments", + absPath: appsPath, + }, + { + name: "ignore-validation-pod", + params: map[string]string{"fieldValidation": "Ignore"}, + bodyFile: "./testdata/pod-medium.json", + resource: "pods", + absPath: corePath, + }, + { + name: "strict-validation-pod", + params: map[string]string{"fieldValidation": "Strict"}, + bodyFile: "./testdata/pod-medium.json", + resource: "pods", + absPath: corePath, + }, + { + name: "ignore-validation-big-pod", + params: map[string]string{"fieldValidation": "Ignore"}, + bodyFile: "./testdata/pod-large.json", + resource: "pods", + absPath: corePath, + }, + { + name: "strict-validation-big-pod", + params: map[string]string{"fieldValidation": "Strict"}, + bodyFile: "./testdata/pod-large.json", + resource: "pods", + absPath: corePath, + }, + } + + for _, bm := range benchmarks { + b.Run(bm.name, func(b *testing.B) { + b.ResetTimer() + b.ReportAllocs() + for n := 0; n < b.N; n++ { + // append the timestamp to the name so that we don't hit conflicts when running the test multiple times + // (i.e. without it -count=n for n>1 will fail, this might be from not tearing stuff down properly). + bodyBase, err := os.ReadFile(bm.bodyFile) + if err != nil { + b.Fatal(err) + } + + objName := fmt.Sprintf("obj-%s-%d-%d-%d", bm.name, n, b.N, time.Now().UnixNano()) + objString := fmt.Sprintf(string(bodyBase), fmt.Sprintf(`"%s"`, objName)) + body := []byte(objString) + + postReq := client.CoreV1().RESTClient().Post(). + AbsPath(bm.absPath). + Namespace("default"). + Resource(bm.resource) + for k, v := range bm.params { + postReq = postReq.Param(k, v) + } + + _, err = postReq.Body(body). + DoRaw(context.TODO()) + if err != nil { + b.Fatal(err) + } + + // TODO: put PUT in a different bench case than POST (ie. have a baseReq) be a part of the test case. + putReq := client.CoreV1().RESTClient().Put(). + AbsPath(bm.absPath). + Namespace("default"). + Resource(bm.resource). + Name(objName) + for k, v := range bm.params { + putReq = putReq.Param(k, v) + } + + _, err = putReq.Body(body). + DoRaw(context.TODO()) + if err != nil { + b.Fatal(err) + } + } + }) + + } +} + +// smpTestSetup applies an object that will later be patched +// in the actual test/benchmark. +func smpTestSetup(t testing.TB, client clientset.Interface) { + bodyBase, err := os.ReadFile("./testdata/deploy-small.json") + if err != nil { + t.Fatalf("failed to read file: %v", err) + } + _, err = client.CoreV1().RESTClient().Patch(types.ApplyPatchType). + AbsPath("/apis/apps/v1"). + Namespace("default"). + Resource("deployments"). + Name("test-deployment"). + Param("fieldManager", "apply_test"). + Body([]byte(fmt.Sprintf(string(bodyBase), `"test-deployment"`))). + Do(context.TODO()). + Get() + if err != nil { + t.Fatalf("Failed to create object using Apply patch: %v", err) + } +} + +// smpRunTest attempts to patch an object via strategic-merge-patch +// with params given from the testcase. +func smpRunTest(t testing.TB, client clientset.Interface, tc smpTestCase) { + req := client.CoreV1().RESTClient().Patch(types.StrategicMergePatchType). + AbsPath("/apis/apps/v1"). + Namespace("default"). + Resource("deployments"). + Name("test-deployment"). + VersionedParams(&tc.opts, metav1.ParameterCodec) + smpBody := `{"metadata":{"labels":{"label1": "val1"}},"spec":{"foo":"bar"}}` + result := req.Body([]byte(smpBody)).Do(context.TODO()) + if tc.warnContains != "" { + warningMatched := false + for _, w := range result.Warnings() { + if strings.Contains(w.String(), tc.warnContains) { + warningMatched = true + } + } + if !warningMatched { + t.Fatalf("expected warning to contain: %s, got warnings: %v", tc.warnContains, result.Warnings()) + } + } + resBody, err := result.Raw() + klog.Warningf("result: %v", string(resBody)) + if err == nil && tc.errContains != "" { + t.Fatalf("unexpected put succeeded") + } + //if err != nil && !strings.Contains(string(resBody), tc.errContains) { + if err != nil && (tc.errContains == "" || !strings.Contains(string(resBody), tc.errContains)) { + t.Fatalf("unexpected response: %v", string(resBody)) + } +} + +type smpTestCase struct { + name string + opts metav1.PatchOptions + errContains string + warnContains string +} + +// TestFieldValidationSMP tests that attempting a strategic-merge-patch +// with unknown fields errors out when fieldValidation is strict, +// but succeeds when fieldValidation is ignored. +func TestFieldValidationSMP(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ServerSideApply, true)() + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.FieldValidation, true)() + + _, client, closeFn := setup(t) + defer closeFn() + + smpTestSetup(t, client) + + var testcases = []smpTestCase{ + { + name: "smp-strict-validation", + opts: metav1.PatchOptions{ + FieldValidation: "Strict", + }, + errContains: "unknown field", + }, + { + name: "smp-warn-validation", + opts: metav1.PatchOptions{ + FieldValidation: "Warn", + }, + warnContains: "unknown field", + }, + { + name: "smp-ignore-validation", + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + smpRunTest(t, client, tc) + }) + } +} + +// Benchmark strategic-merge-patch field validation for strict vs non-strict +func BenchmarkFieldValidationSMP(b *testing.B) { + defer featuregatetesting.SetFeatureGateDuringTest(b, utilfeature.DefaultFeatureGate, features.ServerSideApply, true)() + defer featuregatetesting.SetFeatureGateDuringTest(b, utilfeature.DefaultFeatureGate, features.FieldValidation, true)() + + _, client, closeFn := setup(b) + defer closeFn() + + smpTestSetup(b, client) + + // TODO: add more benchmarks to test bigger objects + var benchmarks = []smpTestCase{ + { + name: "smp-strict-validation", + opts: metav1.PatchOptions{ + FieldValidation: "Strict", + }, + errContains: "unknown field", + }, + { + name: "smp-ignore-validation", + errContains: "", + }, + } + + for _, bm := range benchmarks { + b.Run(bm.name, func(b *testing.B) { + b.ResetTimer() + b.ReportAllocs() + for n := 0; n < b.N; n++ { + smpRunTest(b, client, bm) + } + + }) + } +} + +func patchCRDTestSetup(t testing.TB, server kubeapiservertesting.TestServer, name string) (restclient.Interface, *apiextensionsv1.CustomResourceDefinition) { + config := server.ClientConfig + + apiExtensionClient, err := apiextensionsclient.NewForConfig(config) + if err != nil { + t.Fatal(err) + } + dynamicClient, err := dynamic.NewForConfig(config) + if err != nil { + t.Fatal(err) + } + crdSchema, err := os.ReadFile("./testdata/crd-schema.json") + if err != nil { + t.Fatalf("failed to read file: %v", err) + } + patchYAMLBody, err := os.ReadFile("./testdata/noxu-cr-shell.yaml") + if err != nil { + t.Fatalf("failed to read file: %v", err) + } + + // create the CRD + noxuDefinition := fixtures.NewNoxuV1CustomResourceDefinition(apiextensionsv1.ClusterScoped) + var c apiextensionsv1.CustomResourceValidation + err = json.Unmarshal(crdSchema, &c) + if err != nil { + t.Fatal(err) + } + // set the CRD schema + noxuDefinition.Spec.PreserveUnknownFields = false + for i := range noxuDefinition.Spec.Versions { + noxuDefinition.Spec.Versions[i].Schema = &c + } + // install the CRD + noxuDefinition, err = fixtures.CreateNewV1CustomResourceDefinition(noxuDefinition, apiExtensionClient, dynamicClient) + if err != nil { + t.Fatal(err) + } + + kind := noxuDefinition.Spec.Names.Kind + apiVersion := noxuDefinition.Spec.Group + "/" + noxuDefinition.Spec.Versions[0].Name + + // create a CR + rest := apiExtensionClient.Discovery().RESTClient() + yamlBody := []byte(fmt.Sprintf(string(patchYAMLBody), apiVersion, kind, name)) + result, err := rest.Patch(types.ApplyPatchType). + AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Versions[0].Name, noxuDefinition.Spec.Names.Plural). + Name(name). + Param("fieldManager", "apply_test"). + Body(yamlBody). + DoRaw(context.TODO()) + if err != nil { + t.Fatalf("failed to create custom resource with apply: %v:\n%v", err, string(result)) + } + + return rest, noxuDefinition +} + +// TestFieldValidationPatchCRD tests that server-side schema validation +// works for jsonpatch and mergepatch requests. +func TestFieldValidationPatchCRD(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.FieldValidation, true)() + + var testcases = []struct { + name string + patchType types.PatchType + opts metav1.PatchOptions + body string + errContains string + warnContains string + }{ + { + name: "merge-patch-strict-validation", + patchType: types.MergePatchType, + opts: metav1.PatchOptions{ + FieldValidation: "Strict", + }, + body: `{"metadata":{"finalizers":["test-finalizer","another-one"]}, "spec":{"foo": "bar"}}`, + errContains: "unknown field", + }, + { + name: "merge-patch-warn-validation", + patchType: types.MergePatchType, + opts: metav1.PatchOptions{ + FieldValidation: "Warn", + }, + body: `{"metadata":{"finalizers":["test-finalizer","another-one"]}, "spec":{"foo": "bar"}}`, + warnContains: "unknown field", + }, + { + name: "merge-patch-no-validation", + patchType: types.MergePatchType, + body: `{"metadata":{"finalizers":["test-finalizer","another-one"]}, "spec":{"foo": "bar"}}`, + }, + { + name: "json-patch-strict-validation", + patchType: types.JSONPatchType, + opts: metav1.PatchOptions{ + FieldValidation: "Strict", + }, + body: `[{"op": "add", "path": "/spec/foo", "value": "bar"}]`, + errContains: "unknown field", + }, + { + name: "json-patch-strict-validation", + patchType: types.JSONPatchType, + body: `[{"op": "add", "path": "/spec/foo", "value": "bar"}]`, + }, + } + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + // setup the testerver and install the CRD + server, err := kubeapiservertesting.StartTestServer(t, kubeapiservertesting.NewDefaultTestServerOptions(), nil, framework.SharedEtcd()) + if err != nil { + t.Fatal(err) + } + defer server.TearDownFn() + rest, noxuDefinition := patchCRDTestSetup(t, server, tc.name) + + // patch the CR as specified by the test case + req := rest.Patch(tc.patchType). + AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Versions[0].Name, noxuDefinition.Spec.Names.Plural). + Name(tc.name). + VersionedParams(&tc.opts, metav1.ParameterCodec) + result := req.Body([]byte(tc.body)).Do(context.TODO()) + if tc.warnContains != "" { + warningMatched := false + for _, w := range result.Warnings() { + if strings.Contains(w.String(), tc.warnContains) { + warningMatched = true + } + } + if !warningMatched { + t.Fatalf("expected warning to contain: %s, got warnings: %v", tc.warnContains, result.Warnings()) + } + } + resBody, err := result.Raw() + fmt.Printf("tname %s, resBody: %s, err: %v", tc.name, string(resBody), err) + if err == nil && tc.errContains != "" { + t.Fatalf("unexpected put succeeded") + } + if err != nil && (tc.errContains == "" || !strings.Contains(string(resBody), tc.errContains)) { + t.Fatalf("unexpected response: %v", string(resBody)) + } + }) + } +} + +// Benchmark patch CRD for strict vs non-strict +func BenchmarkFieldValidationPatchCRD(b *testing.B) { + defer featuregatetesting.SetFeatureGateDuringTest(b, utilfeature.DefaultFeatureGate, features.FieldValidation, true)() + + benchmarks := []struct { + name string + patchType types.PatchType + opts metav1.PatchOptions + params map[string]string + bodyBase string + errContains string + }{ + { + name: "ignore-validation-crd-patch", + patchType: types.MergePatchType, + bodyBase: `{"metadata":{"finalizers":["test-finalizer","finalizer-ignore-%d"]}}`, + errContains: "", + }, + { + name: "strict-validation-crd-patch", + patchType: types.MergePatchType, + opts: metav1.PatchOptions{ + FieldValidation: "Strict", + }, + bodyBase: `{"metadata":{"finalizers":["test-finalizer","finalizer-strict-%d"]}}`, + errContains: "", + }, + { + name: "ignore-validation-crd-patch-unknown-field", + patchType: types.MergePatchType, + bodyBase: `{"metadata":{"finalizers":["test-finalizer","finalizer-ignore-unknown-%d"]}, "spec":{"foo": "bar"}}`, + errContains: "", + }, + { + name: "strict-validation-crd-patch-unknown-field", + patchType: types.MergePatchType, + opts: metav1.PatchOptions{ + FieldValidation: "Strict", + }, + bodyBase: `{"metadata":{"finalizers":["test-finalizer","finalizer-strict-unknown-%d"]}, "spec":{"foo": "bar"}}`, + errContains: "unknown field", + }, + } + for _, bm := range benchmarks { + b.Run(bm.name, func(b *testing.B) { + b.ResetTimer() + b.ReportAllocs() + for n := 0; n < b.N; n++ { + + // setup the testerver and install the CRD + server, err := kubeapiservertesting.StartTestServer(b, kubeapiservertesting.NewDefaultTestServerOptions(), nil, framework.SharedEtcd()) + if err != nil { + b.Fatal(err) + } + defer server.TearDownFn() + rest, noxuDefinition := patchCRDTestSetup(b, server, bm.name) + + body := fmt.Sprintf(bm.bodyBase, n) + // patch the CR as specified by the test case + req := rest.Patch(bm.patchType). + AbsPath("/apis", noxuDefinition.Spec.Group, noxuDefinition.Spec.Versions[0].Name, noxuDefinition.Spec.Names.Plural). + Name(bm.name). + VersionedParams(&bm.opts, metav1.ParameterCodec) + result, err := req. + Body([]byte(body)). + DoRaw(context.TODO()) + if err == nil && bm.errContains != "" { + b.Fatalf("unexpected patch succeeded, expected %s", bm.errContains) + } + if err != nil && !strings.Contains(string(result), bm.errContains) { + b.Fatal(err) + } + } + }) + } +} diff --git a/test/integration/apiserver/testdata/deploy-small.json b/test/integration/apiserver/testdata/deploy-small.json new file mode 100644 index 0000000000000..f19a473b52752 --- /dev/null +++ b/test/integration/apiserver/testdata/deploy-small.json @@ -0,0 +1,28 @@ +{ + "apiVersion": "apps/v1", + "kind": "Deployment", + "metadata": { + "name": %s, + "labels": {"app": "nginx"} + }, + "spec": { + "selector": { + "matchLabels": { + "app": "nginx" + } + }, + "template": { + "metadata": { + "labels": { + "app": "nginx" + } + }, + "spec": { + "containers": [{ + "name": "nginx", + "image": "nginx:latest" + }] + } + } + } +} diff --git a/test/integration/apiserver/testdata/pod-large.json b/test/integration/apiserver/testdata/pod-large.json new file mode 100644 index 0000000000000..910f9407ead62 --- /dev/null +++ b/test/integration/apiserver/testdata/pod-large.json @@ -0,0 +1,177 @@ +{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "labels": { + "app": "some-app", + "plugin1": "some-value", + "plugin2": "some-value", + "plugin3": "some-value", + "plugin4": "some-value" + }, + "name": %s, + "namespace": "default", + "ownerReferences": [ + { + "apiVersion": "apps/v1", + "blockOwnerDeletion": true, + "controller": true, + "kind": "ReplicaSet", + "name": "some-name", + "uid": "0a9d2b9e-779e-11e7-b422-42010a8001be" + } + ] + }, + "spec": { + "containers": [ + { + "args": [ + "one", + "two", + "three", + "four", + "five", + "six", + "seven", + "eight", + "nine" + ], + "env": [ + { + "name": "VAR_3", + "valueFrom": { + "secretKeyRef": { + "key": "some-other-key", + "name": "some-oher-name" + } + } + }, + { + "name": "VAR_2", + "valueFrom": { + "secretKeyRef": { + "key": "other-key", + "name": "other-name" + } + } + }, + { + "name": "VAR_1", + "valueFrom": { + "secretKeyRef": { + "key": "some-key", + "name": "some-name" + } + } + } + ], + "image": "some-image-name", + "imagePullPolicy": "IfNotPresent", + "name": "some-name", + "resources": { + "requests": { + "cpu": "0" + } + }, + "terminationMessagePath": "/dev/termination-log", + "terminationMessagePolicy": "File", + "volumeMounts": [ + { + "mountPath": "/var/run/secrets/kubernetes.io/serviceaccount", + "name": "default-token-hu5jz", + "readOnly": true + } + ] + } + ], + "dnsPolicy": "ClusterFirst", + "nodeName": "node-name", + "priority": 0, + "restartPolicy": "Always", + "schedulerName": "default-scheduler", + "securityContext": {}, + "serviceAccount": "default", + "serviceAccountName": "default", + "terminationGracePeriodSeconds": 30, + "tolerations": [ + { + "effect": "NoExecute", + "key": "node.kubernetes.io/not-ready", + "operator": "Exists", + "tolerationSeconds": 300 + }, + { + "effect": "NoExecute", + "key": "node.kubernetes.io/unreachable", + "operator": "Exists", + "tolerationSeconds": 300 + } + ], + "volumes": [ + { + "name": "default-token-hu5jz", + "secret": { + "defaultMode": 420, + "secretName": "default-token-hu5jz" + } + } + ] + }, + "status": { + "conditions": [ + { + "lastProbeTime": null, + "lastTransitionTime": "2019-07-08T09:31:18Z", + "status": "True", + "type": "Initialized" + }, + { + "lastProbeTime": null, + "lastTransitionTime": "2019-07-08T09:41:59Z", + "status": "True", + "type": "Ready" + }, + { + "lastProbeTime": null, + "lastTransitionTime": null, + "status": "True", + "type": "ContainersReady" + }, + { + "lastProbeTime": null, + "lastTransitionTime": "2019-07-08T09:31:18Z", + "status": "True", + "type": "PodScheduled" + } + ], + "containerStatuses": [ + { + "containerID": "docker://885e82a1ed0b7356541bb410a0126921ac42439607c09875cd8097dd5d7b5376", + "image": "some-image-name", + "imageID": "docker-pullable://some-image-id", + "lastState": { + "terminated": { + "containerID": "docker://d57290f9e00fad626b20d2dd87a3cf69bbc22edae07985374f86a8b2b4e39565", + "exitCode": 255, + "finishedAt": "2019-07-08T09:39:09Z", + "reason": "Error", + "startedAt": "2019-07-08T09:38:54Z" + } + }, + "name": "name", + "ready": true, + "restartCount": 6, + "state": { + "running": { + "startedAt": "2019-07-08T09:41:59Z" + } + } + } + ], + "hostIP": "10.0.0.1", + "phase": "Running", + "podIP": "10.0.0.1", + "qosClass": "BestEffort", + "startTime": "2019-07-08T09:31:18Z" + } +} diff --git a/test/integration/apiserver/testdata/pod-medium.json b/test/integration/apiserver/testdata/pod-medium.json new file mode 100644 index 0000000000000..b456a258eacd7 --- /dev/null +++ b/test/integration/apiserver/testdata/pod-medium.json @@ -0,0 +1,37 @@ +{ + "kind": "Pod", + "apiVersion": "v1", + "metadata": { + "name": %s + }, + "spec": { + "containers": [ + { + "name": "healthz", + "image": "k8s.gcr.io/exechealthz-amd64:1.2", + "args": [ + "-cmd=nslookup localhost" + ], + "ports": [ + { + "containerPort": 8080, + "protocol": "TCP" + } + ] + }, + { + "name":"test-container", + "image":"ubuntu:14.04", + "command": ["bash", "-c", "while true; do sleep 100; done"], + "livenessProbe": { + "httpGet": { + "path": "/healthz", + "port":8080 + }, + "initialDelaySeconds": 10, + "timeoutSeconds": 2 + } + } + ] + } +}