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 ba1d0b65949..91d23a8ec23 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 @@ -1331,10 +1331,14 @@ func (v *unstructuredSchemaCoercer) apply(u *unstructured.Unstructured) (unknown if err != nil { return nil, err } - objectMeta, foundObjectMeta, err := schemaobjectmeta.GetObjectMeta(u.Object, v.dropInvalidMetadata) + objectMeta, foundObjectMeta, metaUnknownFields, err := schemaobjectmeta.GetObjectMetaWithOptions(u.Object, schemaobjectmeta.ObjectMetaOptions{ + DropMalformedFields: v.dropInvalidMetadata, + ReturnUnknownFieldPaths: v.returnUnknownFieldPaths, + }) if err != nil { return nil, err } + unknownFieldPaths = append(unknownFieldPaths, metaUnknownFields...) // compare group and kind because also other object like DeleteCollection options pass through here gv, err := schema.ParseGroupVersion(apiVersion) @@ -1345,17 +1349,23 @@ func (v *unstructuredSchemaCoercer) apply(u *unstructured.Unstructured) (unknown 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 - pruneOpts := structuralpruning.PruneOptions{} + pruneOpts := structuralschema.UnknownFieldPathOptions{} if v.returnUnknownFieldPaths { - pruneOpts.ReturnPruned = true + pruneOpts.TrackUnknownFieldPaths = true } - unknownFieldPaths = structuralpruning.PruneWithOptions(u.Object, v.structuralSchemas[gv.Version], true, pruneOpts) + unknownFieldPaths = append(unknownFieldPaths, structuralpruning.PruneWithOptions(u.Object, v.structuralSchemas[gv.Version], true, pruneOpts)...) structuraldefaulting.PruneNonNullableNullsWithoutDefaults(u.Object, v.structuralSchemas[gv.Version]) } - if err := schemaobjectmeta.Coerce(nil, u.Object, v.structuralSchemas[gv.Version], false, v.dropInvalidMetadata); err != nil { + err, paths := schemaobjectmeta.CoerceWithOptions(nil, u.Object, v.structuralSchemas[gv.Version], false, schemaobjectmeta.CoerceOptions{ + DropInvalidFields: v.dropInvalidMetadata, + ReturnUnknownFieldPaths: v.returnUnknownFieldPaths, + }) + if err != nil { return nil, err } + unknownFieldPaths = append(unknownFieldPaths, paths...) + // fixup missing generation in very old CRs if v.repairGeneration && objectMeta.Generation == 0 { objectMeta.Generation = 1 diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/algorithm.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/algorithm.go index 4cd32c1293b..811c38377b8 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/algorithm.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/algorithm.go @@ -17,15 +17,34 @@ limitations under the License. package objectmeta import ( + "sort" + structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/validation/field" ) +// CoerceOptions gives the ability to ReturnUnknownFieldPaths for fields +// unrecognized by the schema or DropInvalidFields for fields that are a part +// of the schema, but are malformed. +type CoerceOptions struct { + // DropInvalidFields discards malformed serialized metadata fields that + // cannot be successfully decoded to the corresponding ObjectMeta field. + // This only applies to fields that are recognized as part of the schema, + // but of an invalid type (i.e. cause an error when unmarshaling, rather + // than being dropped or causing a strictErr). + DropInvalidFields bool + // ReturnUnknownFieldPaths will return the paths to fields that are not + // recognized as part of the schema. + ReturnUnknownFieldPaths bool +} + // Coerce checks types of embedded ObjectMeta and TypeMeta and prunes unknown fields inside the former. // It does coerce ObjectMeta and TypeMeta at the root if isResourceRoot is true. -// If dropInvalidFields is true, fields of wrong type will be dropped. -func Coerce(pth *field.Path, obj interface{}, s *structuralschema.Structural, isResourceRoot, dropInvalidFields bool) *field.Error { +// If opts.ReturnUnknownFieldPaths is true, it will return the paths of any fields that are not a part of the +// schema that are dropped when unmarshaling. +// If opts.DropInvalidFields is true, fields of wrong type will be dropped. +func CoerceWithOptions(pth *field.Path, obj interface{}, s *structuralschema.Structural, isResourceRoot bool, opts CoerceOptions) (*field.Error, []string) { if isResourceRoot { if s == nil { s = &structuralschema.Structural{} @@ -36,36 +55,57 @@ func Coerce(pth *field.Path, obj interface{}, s *structuralschema.Structural, is s = &clone } } - c := coercer{dropInvalidFields: dropInvalidFields} - return c.coerce(pth, obj, s) + c := coercer{DropInvalidFields: opts.DropInvalidFields, ReturnUnknownFieldPaths: opts.ReturnUnknownFieldPaths} + schemaOpts := &structuralschema.UnknownFieldPathOptions{ + TrackUnknownFieldPaths: opts.ReturnUnknownFieldPaths, + } + fieldErr := c.coerce(pth, obj, s, schemaOpts) + sort.Strings(schemaOpts.UnknownFieldPaths) + return fieldErr, schemaOpts.UnknownFieldPaths +} + +// Coerce calls CoerceWithOptions without returning unknown field paths. +func Coerce(pth *field.Path, obj interface{}, s *structuralschema.Structural, isResourceRoot, dropInvalidFields bool) *field.Error { + fieldErr, _ := CoerceWithOptions(pth, obj, s, isResourceRoot, CoerceOptions{DropInvalidFields: dropInvalidFields}) + return fieldErr } type coercer struct { - dropInvalidFields bool + DropInvalidFields bool + ReturnUnknownFieldPaths bool } -func (c *coercer) coerce(pth *field.Path, x interface{}, s *structuralschema.Structural) *field.Error { +func (c *coercer) coerce(pth *field.Path, x interface{}, s *structuralschema.Structural, opts *structuralschema.UnknownFieldPathOptions) *field.Error { if s == nil { return nil } + origPathLen := len(opts.ParentPath) + defer func() { + opts.ParentPath = opts.ParentPath[:origPathLen] + }() switch x := x.(type) { case map[string]interface{}: for k, v := range x { if s.XEmbeddedResource { switch k { case "apiVersion", "kind": - if _, ok := v.(string); !ok && c.dropInvalidFields { + if _, ok := v.(string); !ok && c.DropInvalidFields { delete(x, k) } else if !ok { return field.Invalid(pth.Child(k), v, "must be a string") } case "metadata": - meta, found, err := GetObjectMeta(x, c.dropInvalidFields) + meta, found, unknownFields, err := GetObjectMetaWithOptions(x, ObjectMetaOptions{ + DropMalformedFields: c.DropInvalidFields, + ReturnUnknownFieldPaths: c.ReturnUnknownFieldPaths, + ParentPath: pth, + }) + opts.UnknownFieldPaths = append(opts.UnknownFieldPaths, unknownFields...) if err != nil { - if !c.dropInvalidFields { + if !c.DropInvalidFields { return field.Invalid(pth.Child("metadata"), v, err.Error()) } - // pass through on error if dropInvalidFields is true + // pass through on error if DropInvalidFields is true } else if found { if err := SetObjectMeta(x, meta); err != nil { return field.Invalid(pth.Child("metadata"), v, err.Error()) @@ -78,20 +118,26 @@ func (c *coercer) coerce(pth *field.Path, x interface{}, s *structuralschema.Str } prop, ok := s.Properties[k] if ok { - if err := c.coerce(pth.Child(k), v, &prop); err != nil { + opts.AppendKey(k) + if err := c.coerce(pth.Child(k), v, &prop, opts); err != nil { return err } + opts.ParentPath = opts.ParentPath[:origPathLen] } else if s.AdditionalProperties != nil { - if err := c.coerce(pth.Key(k), v, s.AdditionalProperties.Structural); err != nil { + opts.AppendKey(k) + if err := c.coerce(pth.Key(k), v, s.AdditionalProperties.Structural, opts); err != nil { return err } + opts.ParentPath = opts.ParentPath[:origPathLen] } } case []interface{}: for i, v := range x { - if err := c.coerce(pth.Index(i), v, s.Items); err != nil { + opts.AppendIndex(i) + if err := c.coerce(pth.Index(i), v, s.Items, opts); err != nil { return err } + opts.ParentPath = opts.ParentPath[:origPathLen] } default: // scalars, do nothing diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/algorithm_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/algorithm_test.go index 326a97d396d..01b516c42e7 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/algorithm_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/algorithm_test.go @@ -19,6 +19,7 @@ package objectmeta import ( "bytes" "reflect" + "strings" "testing" structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema" @@ -28,13 +29,14 @@ import ( func TestCoerce(t *testing.T) { tests := []struct { - name string - json string - includeRoot bool - dropInvalidFields bool - schema *structuralschema.Structural - expected string - expectedError bool + name string + json string + includeRoot bool + dropInvalidFields bool + schema *structuralschema.Structural + expected string + expectedError bool + expectedUnknownFields []string }{ {name: "empty", json: "null", schema: nil, expected: "null"}, {name: "scalar", json: "4", schema: &structuralschema.Structural{}, expected: "4"}, @@ -199,7 +201,12 @@ func TestCoerce(t *testing.T) { } } } -`}, +`, expectedUnknownFields: []string{ + "nested.metadata.unspecified", + "nested.spec.embedded.metadata.unspecified", + "preserving.metadata.unspecified", + "pruned.metadata.unspecified", + }}, {name: "x-kubernetes-embedded-resource, with includeRoot=true", json: ` { "apiVersion": "foo/v1", @@ -356,8 +363,13 @@ func TestCoerce(t *testing.T) { } } } -} -`}, +}`, expectedUnknownFields: []string{ + "metadata.unspecified", + "nested.metadata.unspecified", + "nested.spec.embedded.metadata.unspecified", + "preserving.metadata.unspecified", + "pruned.metadata.unspecified", + }}, {name: "without name", json: ` { "apiVersion": "foo/v1", @@ -495,7 +507,10 @@ func TestCoerce(t *testing.T) { t.Fatal(err) } - err := Coerce(nil, in, tt.schema, tt.includeRoot, tt.dropInvalidFields) + err, unknownFields := CoerceWithOptions(nil, in, tt.schema, tt.includeRoot, CoerceOptions{ + DropInvalidFields: tt.dropInvalidFields, + ReturnUnknownFieldPaths: true, + }) if tt.expectedError && err == nil { t.Error("expected error, but did not get any") } else if !tt.expectedError && err != nil { @@ -510,6 +525,9 @@ func TestCoerce(t *testing.T) { } t.Errorf("expected: %s\ngot: %s\ndiff: %s", tt.expected, buf.String(), diff.ObjectDiff(expected, in)) } + if !reflect.DeepEqual(unknownFields, tt.expectedUnknownFields) { + t.Errorf("expected unknown fields:\n\t%v\ngot:\n\t%v\n", strings.Join(tt.expectedUnknownFields, "\n\t"), strings.Join(unknownFields, "\n\t")) + } }) } } diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/coerce.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/coerce.go index d9c486c37ef..42b968b8ac4 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/coerce.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/coerce.go @@ -23,35 +23,83 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" utiljson "k8s.io/apimachinery/pkg/util/json" + "k8s.io/apimachinery/pkg/util/validation/field" + kjson "sigs.k8s.io/json" ) -// GetObjectMeta does conversion of JSON to ObjectMeta. It first tries json.Unmarshal into a metav1.ObjectMeta -// type. If that does not work and dropMalformedFields is true, it does field-by-field best-effort conversion -// throwing away fields which lead to errors. +// GetObjectMeta calls GetObjectMetaWithOptions without returning unknown field paths. func GetObjectMeta(obj map[string]interface{}, dropMalformedFields bool) (*metav1.ObjectMeta, bool, error) { + meta, found, _, err := GetObjectMetaWithOptions(obj, ObjectMetaOptions{ + DropMalformedFields: dropMalformedFields, + }) + return meta, found, err +} + +// ObjectMetaOptions provides the options for how GetObjectMeta should retrieve the object meta. +type ObjectMetaOptions struct { + // DropMalformedFields discards malformed serialized metadata fields that + // cannot be successfully decoded to the corresponding ObjectMeta field. + // This only applies to fields that are recognized as part of the schema, + // but of an invalid type (i.e. cause an error when unmarshaling, rather + // than being dropped or causing a strictErr). + DropMalformedFields bool + // ReturnUnknownFieldPaths will return the paths to fields that are not + // recognized as part of the schema. + ReturnUnknownFieldPaths bool + // ParentPath provides the current path up to the given ObjectMeta. + // If nil, the metadata is assumed to be at the root of the object. + ParentPath *field.Path +} + +// GetObjectMetaWithOptions does conversion of JSON to ObjectMeta. +// It first tries json.Unmarshal into a metav1.ObjectMeta +// type. If that does not work and opts.DropMalformedFields is true, it does field-by-field best-effort conversion +// throwing away fields which lead to errors. +// If opts.ReturnedUnknownFields is true, it will UnmarshalStrict instead, returning the paths of any unknown fields +// it encounters (i.e. paths returned as strict errs from UnmarshalStrict) +func GetObjectMetaWithOptions(obj map[string]interface{}, opts ObjectMetaOptions) (*metav1.ObjectMeta, bool, []string, error) { metadata, found := obj["metadata"] if !found { - return nil, false, nil + return nil, false, nil, nil } // round-trip through JSON first, hoping that unmarshalling just works objectMeta := &metav1.ObjectMeta{} metadataBytes, err := utiljson.Marshal(metadata) if err != nil { - return nil, false, err + return nil, false, nil, err } - if err = utiljson.Unmarshal(metadataBytes, objectMeta); err == nil { - // if successful, return - return objectMeta, true, nil + var unmarshalErr error + if opts.ReturnUnknownFieldPaths { + var strictErrs []error + strictErrs, unmarshalErr = kjson.UnmarshalStrict(metadataBytes, objectMeta) + if unmarshalErr == nil { + if len(strictErrs) > 0 { + unknownPaths := []string{} + prefix := opts.ParentPath.Child("metadata").String() + for _, err := range strictErrs { + if fieldPathErr, ok := err.(kjson.FieldError); ok { + unknownPaths = append(unknownPaths, prefix+"."+fieldPathErr.FieldPath()) + } + } + return objectMeta, true, unknownPaths, nil + } + return objectMeta, true, nil, nil + } + } else { + if unmarshalErr = utiljson.Unmarshal(metadataBytes, objectMeta); unmarshalErr == nil { + // if successful, return + return objectMeta, true, nil, nil + } } - if !dropMalformedFields { + if !opts.DropMalformedFields { // if we're not trying to drop malformed fields, return the error - return nil, true, err + return nil, true, nil, unmarshalErr } metadataMap, ok := metadata.(map[string]interface{}) if !ok { - return nil, false, fmt.Errorf("invalid metadata: expected object, got %T", metadata) + return nil, false, nil, fmt.Errorf("invalid metadata: expected object, got %T", metadata) } // Go field by field accumulating into the metadata object. @@ -59,18 +107,31 @@ func GetObjectMeta(obj map[string]interface{}, dropMalformedFields bool) (*metav // each iteration preserving the old key-values. accumulatedObjectMeta := &metav1.ObjectMeta{} testObjectMeta := &metav1.ObjectMeta{} + var unknownFields []string for k, v := range metadataMap { // serialize a single field if singleFieldBytes, err := utiljson.Marshal(map[string]interface{}{k: v}); err == nil { // do a test unmarshal if utiljson.Unmarshal(singleFieldBytes, testObjectMeta) == nil { // if that succeeds, unmarshal for real - utiljson.Unmarshal(singleFieldBytes, accumulatedObjectMeta) + if opts.ReturnUnknownFieldPaths { + strictErrs, _ := kjson.UnmarshalStrict(singleFieldBytes, accumulatedObjectMeta) + if len(strictErrs) > 0 { + prefix := opts.ParentPath.Child("metadata").String() + for _, err := range strictErrs { + if fieldPathErr, ok := err.(kjson.FieldError); ok { + unknownFields = append(unknownFields, prefix+"."+fieldPathErr.FieldPath()) + } + } + } + } else { + utiljson.Unmarshal(singleFieldBytes, accumulatedObjectMeta) + } } } } - return accumulatedObjectMeta, true, nil + return accumulatedObjectMeta, true, unknownFields, nil } // SetObjectMeta writes back ObjectMeta into a JSON data structure. diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/coerce_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/coerce_test.go index c72240146fe..526156c2b28 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/coerce_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/objectmeta/coerce_test.go @@ -161,6 +161,194 @@ func TestMalformedObjectMetaFields(t *testing.T) { } } +func TestGetObjectMetaWithOptions(t *testing.T) { + unknownAndMalformed := map[string]interface{}{ + "kind": "Pod", + "apiVersion": "v1", + "metadata": map[string]interface{}{ + "name": "my-meta", + "unknownField": "foo", + "generateName": nil, + "generation": nil, + "labels": map[string]string{ + "foo": "bar", + }, + "annotations": 11, + }, + } + + unknownOnly := map[string]interface{}{ + "kind": "Pod", + "apiVersion": "v1", + "metadata": map[string]interface{}{ + "name": "my-meta", + "unknownField": "foo", + "generateName": nil, + "generation": nil, + "labels": map[string]string{ + "foo": "bar", + }, + }, + } + + malformedOnly := map[string]interface{}{ + "kind": "Pod", + "apiVersion": "v1", + "metadata": map[string]interface{}{ + "name": "my-meta", + "generateName": nil, + "generation": nil, + "labels": map[string]string{ + "foo": "bar", + }, + "annotations": 11, + }, + } + + var testcases = []struct { + obj map[string]interface{} + dropMalformedFields bool + returnUnknownFieldPaths bool + expectedObject *metav1.ObjectMeta + expectedUnknownPaths []string + expectedErr string + }{ + { + obj: unknownAndMalformed, + dropMalformedFields: false, + returnUnknownFieldPaths: false, + expectedErr: "json: cannot unmarshal number into Go struct field ObjectMeta.annotations of type map[string]string", + }, + { + obj: unknownAndMalformed, + dropMalformedFields: true, + returnUnknownFieldPaths: false, + expectedObject: &metav1.ObjectMeta{ + Name: "my-meta", + Labels: map[string]string{ + "foo": "bar", + }, + }, + }, + { + obj: unknownAndMalformed, + dropMalformedFields: false, + returnUnknownFieldPaths: true, + expectedErr: "json: cannot unmarshal number into Go struct field ObjectMeta.annotations of type map[string]string", + }, + { + obj: unknownAndMalformed, + dropMalformedFields: true, + returnUnknownFieldPaths: true, + expectedObject: &metav1.ObjectMeta{ + Name: "my-meta", + Labels: map[string]string{ + "foo": "bar", + }, + }, + expectedUnknownPaths: []string{"metadata.unknownField"}, + }, + + { + obj: unknownOnly, + dropMalformedFields: false, + returnUnknownFieldPaths: false, + expectedObject: &metav1.ObjectMeta{ + Name: "my-meta", + Labels: map[string]string{ + "foo": "bar", + }, + }, + }, + { + obj: unknownOnly, + dropMalformedFields: true, + returnUnknownFieldPaths: false, + expectedObject: &metav1.ObjectMeta{ + Name: "my-meta", + Labels: map[string]string{ + "foo": "bar", + }, + }, + }, + { + obj: unknownOnly, + dropMalformedFields: false, + returnUnknownFieldPaths: true, + expectedObject: &metav1.ObjectMeta{ + Name: "my-meta", + Labels: map[string]string{ + "foo": "bar", + }, + }, + expectedUnknownPaths: []string{"metadata.unknownField"}, + }, + { + obj: unknownOnly, + dropMalformedFields: true, + returnUnknownFieldPaths: true, + expectedObject: &metav1.ObjectMeta{ + Name: "my-meta", + Labels: map[string]string{ + "foo": "bar", + }, + }, + expectedUnknownPaths: []string{"metadata.unknownField"}, + }, + + { + obj: malformedOnly, + dropMalformedFields: false, + returnUnknownFieldPaths: false, + expectedErr: "json: cannot unmarshal number into Go struct field ObjectMeta.annotations of type map[string]string", + }, + { + obj: malformedOnly, + dropMalformedFields: true, + returnUnknownFieldPaths: false, + expectedObject: &metav1.ObjectMeta{ + Name: "my-meta", + Labels: map[string]string{ + "foo": "bar", + }, + }, + }, + { + obj: malformedOnly, + dropMalformedFields: false, + returnUnknownFieldPaths: true, + expectedErr: "json: cannot unmarshal number into Go struct field ObjectMeta.annotations of type map[string]string", + }, + { + obj: malformedOnly, + dropMalformedFields: true, + returnUnknownFieldPaths: true, + expectedObject: &metav1.ObjectMeta{ + Name: "my-meta", + Labels: map[string]string{ + "foo": "bar", + }, + }, + }, + } + for _, tc := range testcases { + opts := ObjectMetaOptions{ + ReturnUnknownFieldPaths: tc.returnUnknownFieldPaths, + DropMalformedFields: tc.dropMalformedFields, + } + obj, _, unknownPaths, err := GetObjectMetaWithOptions(tc.obj, opts) + if !reflect.DeepEqual(tc.expectedObject, obj) { + t.Errorf("expected: %v, got: %v", tc.expectedObject, obj) + } + if (err == nil && tc.expectedErr != "") || err != nil && (err.Error() != tc.expectedErr) { + t.Errorf("expected: %v, got: %v", tc.expectedErr, err) + } + if !reflect.DeepEqual(tc.expectedUnknownPaths, unknownPaths) { + t.Errorf("expected: %v, got: %v", tc.expectedUnknownPaths, unknownPaths) + } + } +} + func TestGetObjectMetaNils(t *testing.T) { u := &unstructured.Unstructured{ Object: map[string]interface{}{ diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/options.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/options.go new file mode 100644 index 00000000000..547e5917519 --- /dev/null +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/options.go @@ -0,0 +1,67 @@ +/* +Copyright 2022 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 schema + +import ( + "strconv" + "strings" +) + +// UnknownFieldPathOptions allow for tracking paths to unknown fields. +type UnknownFieldPathOptions struct { + // TrackUnknownFieldPaths determines whether or not unknown field + // paths should be stored or not. + TrackUnknownFieldPaths bool + // ParentPath builds the path to unknown fields as the object + // is recursively traversed. + ParentPath []string + // UnknownFieldPaths is the list of all unknown fields identified. + UnknownFieldPaths []string +} + +// RecordUnknownFields adds a path to an unknown field to the +// record of UnknownFieldPaths, if TrackUnknownFieldPaths is true +func (o *UnknownFieldPathOptions) RecordUnknownField(field string) { + if !o.TrackUnknownFieldPaths { + return + } + l := len(o.ParentPath) + o.AppendKey(field) + o.UnknownFieldPaths = append(o.UnknownFieldPaths, strings.Join(o.ParentPath, "")) + o.ParentPath = o.ParentPath[:l] +} + +// AppendKey adds a key (i.e. field) to the current parent +// path, if TrackUnknownFieldPaths is true. +func (o *UnknownFieldPathOptions) AppendKey(key string) { + if !o.TrackUnknownFieldPaths { + return + } + if len(o.ParentPath) > 0 { + o.ParentPath = append(o.ParentPath, ".") + } + o.ParentPath = append(o.ParentPath, key) +} + +// AppendIndex adds an index to the most recent field of +// the current parent path, if TrackUnknownFieldPaths is true. +func (o *UnknownFieldPathOptions) AppendIndex(index int) { + if !o.TrackUnknownFieldPaths { + return + } + o.ParentPath = append(o.ParentPath, "[", strconv.Itoa(index), "]") +} 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 b2b1bf81fbd..a929e52522c 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 @@ -18,37 +18,15 @@ package pruning import ( "sort" - "strconv" - "strings" structuralschema "k8s.io/apiextensions-apiserver/pkg/apiserver/schema" ) -// PruneOptions sets options for pruning -// unknown fields -type PruneOptions struct { - // parentPath collects the path that the pruning - // takes as it traverses the object. - // It is used to report the full path to any unknown - // fields that the pruning encounters. - // It is only populated if ReturnPruned is true. - parentPath []string - - // prunedPaths collects pruned field paths resulting from - // calls to recordPrunedKey. - // It is only populated if ReturnPruned is true. - prunedPaths []string - - // ReturnPruned defines whether we want to track the - // fields that are pruned - ReturnPruned bool -} - // PruneWithOptions 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. -// It returns the set of fields that it prunes if opts.ReturnPruned is true -func PruneWithOptions(obj interface{}, s *structuralschema.Structural, isResourceRoot bool, opts PruneOptions) []string { +// It returns the set of fields that it prunes if opts.TrackUnknownFieldPaths is true +func PruneWithOptions(obj interface{}, s *structuralschema.Structural, isResourceRoot bool, opts structuralschema.UnknownFieldPathOptions) []string { if isResourceRoot { if s == nil { s = &structuralschema.Structural{} @@ -60,14 +38,14 @@ func PruneWithOptions(obj interface{}, s *structuralschema.Structural, isResourc } } prune(obj, s, &opts) - sort.Strings(opts.prunedPaths) - return opts.prunedPaths + sort.Strings(opts.UnknownFieldPaths) + return opts.UnknownFieldPaths } // Prune is equivalent to -// PruneWithOptions(obj, s, isResourceRoot, PruneOptions{}) +// PruneWithOptions(obj, s, isResourceRoot, structuralschema.UnknownFieldPathOptions{}) func Prune(obj interface{}, s *structuralschema.Structural, isResourceRoot bool) { - PruneWithOptions(obj, s, isResourceRoot, PruneOptions{}) + PruneWithOptions(obj, s, isResourceRoot, structuralschema.UnknownFieldPathOptions{}) } var metaFields = map[string]bool{ @@ -76,48 +54,21 @@ var metaFields = map[string]bool{ "metadata": true, } -func (p *PruneOptions) recordPrunedKey(key string) { - if !p.ReturnPruned { - return - } - l := len(p.parentPath) - p.appendKey(key) - p.prunedPaths = append(p.prunedPaths, strings.Join(p.parentPath, "")) - p.parentPath = p.parentPath[:l] -} - -func (p *PruneOptions) appendKey(key string) { - if !p.ReturnPruned { - return - } - if len(p.parentPath) > 0 { - p.parentPath = append(p.parentPath, ".") - } - p.parentPath = append(p.parentPath, key) -} - -func (p *PruneOptions) appendIndex(index int) { - if !p.ReturnPruned { - return - } - p.parentPath = append(p.parentPath, "[", strconv.Itoa(index), "]") -} - -func prune(x interface{}, s *structuralschema.Structural, opts *PruneOptions) { +func prune(x interface{}, s *structuralschema.Structural, opts *structuralschema.UnknownFieldPathOptions) { if s != nil && s.XPreserveUnknownFields { skipPrune(x, s, opts) return } - origPathLen := len(opts.parentPath) + origPathLen := len(opts.ParentPath) defer func() { - opts.parentPath = opts.parentPath[:origPathLen] + opts.ParentPath = opts.ParentPath[:origPathLen] }() switch x := x.(type) { case map[string]interface{}: if s == nil { for k := range x { - opts.recordPrunedKey(k) + opts.RecordUnknownField(k) delete(x, k) } return @@ -128,16 +79,16 @@ func prune(x interface{}, s *structuralschema.Structural, opts *PruneOptions) { } prop, ok := s.Properties[k] if ok { - opts.appendKey(k) + opts.AppendKey(k) prune(v, &prop, opts) - opts.parentPath = opts.parentPath[:origPathLen] + opts.ParentPath = opts.ParentPath[:origPathLen] } else if s.AdditionalProperties != nil { - opts.appendKey(k) + opts.AppendKey(k) prune(v, s.AdditionalProperties.Structural, opts) - opts.parentPath = opts.parentPath[:origPathLen] + opts.ParentPath = opts.ParentPath[:origPathLen] } else { - if !metaFields[k] || len(opts.parentPath) > 0 { - opts.recordPrunedKey(k) + if !metaFields[k] || len(opts.ParentPath) > 0 { + opts.RecordUnknownField(k) } delete(x, k) } @@ -145,29 +96,29 @@ func prune(x interface{}, s *structuralschema.Structural, opts *PruneOptions) { case []interface{}: if s == nil { for i, v := range x { - opts.appendIndex(i) + opts.AppendIndex(i) prune(v, nil, opts) - opts.parentPath = opts.parentPath[:origPathLen] + opts.ParentPath = opts.ParentPath[:origPathLen] } return } for i, v := range x { - opts.appendIndex(i) + opts.AppendIndex(i) prune(v, s.Items, opts) - opts.parentPath = opts.parentPath[:origPathLen] + opts.ParentPath = opts.ParentPath[:origPathLen] } default: // scalars, do nothing } } -func skipPrune(x interface{}, s *structuralschema.Structural, opts *PruneOptions) { +func skipPrune(x interface{}, s *structuralschema.Structural, opts *structuralschema.UnknownFieldPathOptions) { if s == nil { return } - origPathLen := len(opts.parentPath) + origPathLen := len(opts.ParentPath) defer func() { - opts.parentPath = opts.parentPath[:origPathLen] + opts.ParentPath = opts.ParentPath[:origPathLen] }() switch x := x.(type) { @@ -177,20 +128,20 @@ func skipPrune(x interface{}, s *structuralschema.Structural, opts *PruneOptions continue } if prop, ok := s.Properties[k]; ok { - opts.appendKey(k) + opts.AppendKey(k) prune(v, &prop, opts) - opts.parentPath = opts.parentPath[:origPathLen] + opts.ParentPath = opts.ParentPath[:origPathLen] } else if s.AdditionalProperties != nil { - opts.appendKey(k) + opts.AppendKey(k) prune(v, s.AdditionalProperties.Structural, opts) - opts.parentPath = opts.parentPath[:origPathLen] + opts.ParentPath = opts.ParentPath[:origPathLen] } } case []interface{}: for i, v := range x { - opts.appendIndex(i) + opts.AppendIndex(i) skipPrune(v, s.Items, opts) - opts.parentPath = opts.parentPath[:origPathLen] + opts.ParentPath = opts.ParentPath[:origPathLen] } default: // scalars, do nothing diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/pruning/algorithm_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/pruning/algorithm_test.go index 9f529f6924a..176b3a2c92d 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/pruning/algorithm_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/pruning/algorithm_test.go @@ -575,8 +575,8 @@ func TestPrune(t *testing.T) { t.Fatal(err) } - pruned := PruneWithOptions(in, tt.schema, tt.isResourceRoot, PruneOptions{ - ReturnPruned: true, + pruned := PruneWithOptions(in, tt.schema, tt.isResourceRoot, structuralschema.UnknownFieldPathOptions{ + TrackUnknownFieldPaths: true, }) if !reflect.DeepEqual(in, expectedObject) { var buf bytes.Buffer @@ -592,8 +592,8 @@ func TestPrune(t *testing.T) { t.Errorf("expected pruned:\n\t%v\ngot:\n\t%v\n", strings.Join(tt.expectedPruned, "\n\t"), strings.Join(pruned, "\n\t")) } - // now check that pruned is empty when ReturnPruned is false - emptyPruned := PruneWithOptions(in, tt.schema, tt.isResourceRoot, PruneOptions{}) + // now check that pruned is empty when TrackUnknownFieldPaths is false + emptyPruned := PruneWithOptions(in, tt.schema, tt.isResourceRoot, structuralschema.UnknownFieldPathOptions{}) if !reflect.DeepEqual(in, expectedObject) { var buf bytes.Buffer enc := json.NewEncoder(&buf) diff --git a/test/integration/apiserver/field_validation_test.go b/test/integration/apiserver/field_validation_test.go index f9736f8799f..bdf2bd352ec 100644 --- a/test/integration/apiserver/field_validation_test.go +++ b/test/integration/apiserver/field_validation_test.go @@ -38,6 +38,7 @@ import ( clientset "k8s.io/client-go/kubernetes" "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" @@ -49,8 +50,10 @@ var ( "apiVersion": "apps/v1", "kind": "Deployment", "metadata": { + "name": "dupename", "name": "%s", - "labels": {"app": "nginx"} + "labels": {"app": "nginx"}, + "unknownMeta": "metaVal" }, "spec": { "unknown1": "val1", @@ -118,7 +121,9 @@ var ( invalidBodyYAML = `apiVersion: apps/v1 kind: Deployment metadata: + name: dupename name: %s + unknownMeta: metaVal labels: app: nginx spec: @@ -236,7 +241,9 @@ spec: "apiVersion": "%s", "kind": "%s", "metadata": { + "name": "dupename", "name": "%s", + "unknownMeta": "metaVal", "resourceVersion": "%s" }, "spec": { @@ -252,7 +259,16 @@ spec: "hostPort": 8081, "hostPort": 8082, "unknownNested": "val" - }] + }], + "embeddedObj": { + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "my-cm", + "namespace": "my-ns", + "unknownEmbeddedMeta": "foo" + } + } } }` @@ -271,7 +287,14 @@ spec: "containerPort": 8080, "protocol": "TCP", "hostPort": 8081 - }] + }], + "embeddedObj": { + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "my-cm" + } + } } } ` @@ -280,8 +303,10 @@ spec: apiVersion: "%s" kind: "%s" metadata: + name: dupename name: "%s" resourceVersion: "%s" + unknownMeta: metaVal spec: unknown1: val1 unknownDupe: valDupe @@ -294,7 +319,14 @@ spec: protocol: TCP hostPort: 8081 hostPort: 8082 - unknownNested: val` + unknownNested: val + embeddedObj: + apiVersion: v1 + kind: ConfigMap + metadata: + name: my-cm + namespace: my-ns + unknownEmbeddedMeta: foo` crdValidBodyYAML = ` apiVersion: "%s" @@ -308,7 +340,13 @@ spec: - name: portName containerPort: 8080 protocol: TCP - hostPort: 8081` + hostPort: 8081 + embeddedObj: + apiVersion: v1 + kind: ConfigMap + metadata: + name: my-cm + namespace: my-ns` crdApplyInvalidBody = ` { @@ -326,7 +364,15 @@ spec: "protocol": "TCP", "hostPort": 8081, "hostPort": 8082 - }] + }], + "embeddedObj": { + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "my-cm", + "namespace": "my-ns" + } + } } }` @@ -344,7 +390,15 @@ spec: "containerPort": 8080, "protocol": "TCP", "hostPort": 8082 - }] + }], + "embeddedObj": { + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "my-cm", + "namespace": "my-ns" + } + } } }` @@ -362,7 +416,15 @@ spec: "containerPort": 8080, "protocol": "TCP", "hostPort": 8083 - }] + }], + "embeddedObj": { + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "my-cm", + "namespace": "my-ns" + } + } } }` @@ -397,6 +459,21 @@ spec: "knownField1": { "type": "string" }, + "embeddedObj": { + "x-kubernetes-embedded-resource": true, + "type": "object", + "properties": { + "apiversion": { + "type": "string" + }, + "kind": { + "type": "string" + }, + "metadata": { + "type": "object" + } + } + }, "ports": { "type": "array", "x-kubernetes-list-map-keys": [ @@ -516,7 +593,7 @@ func testFieldValidationPost(t *testing.T, client clientset.Interface) { FieldValidation: "Strict", }, bodyBase: invalidBodyJSON, - strictDecodingError: `strict decoding error: unknown field "spec.unknown1", unknown field "spec.unknownDupe", duplicate field "spec.paused", unknown field "spec.template.spec.containers[0].unknownNested", duplicate field "spec.template.spec.containers[0].imagePullPolicy"`, + strictDecodingError: `strict decoding error: duplicate field "metadata.name", unknown field "metadata.unknownMeta", unknown field "spec.unknown1", unknown field "spec.unknownDupe", duplicate field "spec.paused", unknown field "spec.template.spec.containers[0].unknownNested", duplicate field "spec.template.spec.containers[0].imagePullPolicy"`, }, { name: "post-warn-validation", @@ -525,6 +602,8 @@ func testFieldValidationPost(t *testing.T, client clientset.Interface) { }, bodyBase: invalidBodyJSON, strictDecodingWarnings: []string{ + `duplicate field "metadata.name"`, + `unknown field "metadata.unknownMeta"`, `unknown field "spec.unknown1"`, `unknown field "spec.unknownDupe"`, `duplicate field "spec.paused"`, @@ -546,6 +625,8 @@ func testFieldValidationPost(t *testing.T, client clientset.Interface) { name: "post-no-validation", bodyBase: invalidBodyJSON, strictDecodingWarnings: []string{ + `duplicate field "metadata.name"`, + `unknown field "metadata.unknownMeta"`, `unknown field "spec.unknown1"`, `unknown field "spec.unknownDupe"`, `duplicate field "spec.paused"`, @@ -564,9 +645,10 @@ func testFieldValidationPost(t *testing.T, client clientset.Interface) { bodyBase: invalidBodyYAML, contentType: "application/yaml", strictDecodingError: `strict decoding error: yaml: unmarshal errors: - line 10: key "unknownDupe" already set in map - line 12: key "paused" already set in map - line 26: key "imagePullPolicy" already set in map, unknown field "spec.template.spec.containers[0].unknownNested", unknown field "spec.unknown1", unknown field "spec.unknownDupe"`, + line 5: key "name" already set in map + line 12: key "unknownDupe" already set in map + line 14: key "paused" already set in map + line 28: key "imagePullPolicy" already set in map, unknown field "metadata.unknownMeta", unknown field "spec.template.spec.containers[0].unknownNested", unknown field "spec.unknown1", unknown field "spec.unknownDupe"`, }, { name: "post-warn-validation-yaml", @@ -576,9 +658,11 @@ func testFieldValidationPost(t *testing.T, client clientset.Interface) { bodyBase: invalidBodyYAML, contentType: "application/yaml", strictDecodingWarnings: []string{ - `line 10: key "unknownDupe" already set in map`, - `line 12: key "paused" already set in map`, - `line 26: key "imagePullPolicy" already set in map`, + `line 5: key "name" already set in map`, + `line 12: key "unknownDupe" already set in map`, + `line 14: key "paused" already set in map`, + `line 28: key "imagePullPolicy" already set in map`, + `unknown field "metadata.unknownMeta"`, `unknown field "spec.template.spec.containers[0].unknownNested"`, `unknown field "spec.unknown1"`, `unknown field "spec.unknownDupe"`, @@ -597,9 +681,11 @@ func testFieldValidationPost(t *testing.T, client clientset.Interface) { bodyBase: invalidBodyYAML, contentType: "application/yaml", strictDecodingWarnings: []string{ - `line 10: key "unknownDupe" already set in map`, - `line 12: key "paused" already set in map`, - `line 26: key "imagePullPolicy" already set in map`, + `line 5: key "name" already set in map`, + `line 12: key "unknownDupe" already set in map`, + `line 14: key "paused" already set in map`, + `line 28: key "imagePullPolicy" already set in map`, + `unknown field "metadata.unknownMeta"`, `unknown field "spec.template.spec.containers[0].unknownNested"`, `unknown field "spec.unknown1"`, `unknown field "spec.unknownDupe"`, @@ -609,6 +695,7 @@ func testFieldValidationPost(t *testing.T, client clientset.Interface) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { + klog.Warningf("running tc named: %s", tc.name) body := []byte(fmt.Sprintf(tc.bodyBase, fmt.Sprintf("test-deployment-%s", tc.name))) req := client.CoreV1().RESTClient().Post(). AbsPath("/apis/apps/v1"). @@ -667,7 +754,7 @@ func testFieldValidationPut(t *testing.T, client clientset.Interface) { FieldValidation: "Strict", }, putBodyBase: invalidBodyJSON, - strictDecodingError: `strict decoding error: unknown field "spec.unknown1", unknown field "spec.unknownDupe", duplicate field "spec.paused", unknown field "spec.template.spec.containers[0].unknownNested", duplicate field "spec.template.spec.containers[0].imagePullPolicy"`, + strictDecodingError: `strict decoding error: duplicate field "metadata.name", unknown field "metadata.unknownMeta", unknown field "spec.unknown1", unknown field "spec.unknownDupe", duplicate field "spec.paused", unknown field "spec.template.spec.containers[0].unknownNested", duplicate field "spec.template.spec.containers[0].imagePullPolicy"`, }, { name: "put-warn-validation", @@ -676,6 +763,8 @@ func testFieldValidationPut(t *testing.T, client clientset.Interface) { }, putBodyBase: invalidBodyJSON, strictDecodingWarnings: []string{ + `duplicate field "metadata.name"`, + `unknown field "metadata.unknownMeta"`, `unknown field "spec.unknown1"`, `unknown field "spec.unknownDupe"`, `duplicate field "spec.paused"`, @@ -687,16 +776,18 @@ func testFieldValidationPut(t *testing.T, client clientset.Interface) { }, }, { - name: "put-default-ignore-validation", + name: "put-ignore-validation", opts: metav1.UpdateOptions{ FieldValidation: "Ignore", }, putBodyBase: invalidBodyJSON, }, { - name: "put-ignore-validation", + name: "put-no-validation", putBodyBase: invalidBodyJSON, strictDecodingWarnings: []string{ + `duplicate field "metadata.name"`, + `unknown field "metadata.unknownMeta"`, `unknown field "spec.unknown1"`, `unknown field "spec.unknownDupe"`, `duplicate field "spec.paused"`, @@ -715,9 +806,10 @@ func testFieldValidationPut(t *testing.T, client clientset.Interface) { putBodyBase: invalidBodyYAML, contentType: "application/yaml", strictDecodingError: `strict decoding error: yaml: unmarshal errors: - line 10: key "unknownDupe" already set in map - line 12: key "paused" already set in map - line 26: key "imagePullPolicy" already set in map, unknown field "spec.template.spec.containers[0].unknownNested", unknown field "spec.unknown1", unknown field "spec.unknownDupe"`, + line 5: key "name" already set in map + line 12: key "unknownDupe" already set in map + line 14: key "paused" already set in map + line 28: key "imagePullPolicy" already set in map, unknown field "metadata.unknownMeta", unknown field "spec.template.spec.containers[0].unknownNested", unknown field "spec.unknown1", unknown field "spec.unknownDupe"`, }, { name: "put-warn-validation-yaml", @@ -727,9 +819,11 @@ func testFieldValidationPut(t *testing.T, client clientset.Interface) { putBodyBase: invalidBodyYAML, contentType: "application/yaml", strictDecodingWarnings: []string{ - `line 10: key "unknownDupe" already set in map`, - `line 12: key "paused" already set in map`, - `line 26: key "imagePullPolicy" already set in map`, + `line 5: key "name" already set in map`, + `line 12: key "unknownDupe" already set in map`, + `line 14: key "paused" already set in map`, + `line 28: key "imagePullPolicy" already set in map`, + `unknown field "metadata.unknownMeta"`, `unknown field "spec.template.spec.containers[0].unknownNested"`, `unknown field "spec.unknown1"`, `unknown field "spec.unknownDupe"`, @@ -748,9 +842,11 @@ func testFieldValidationPut(t *testing.T, client clientset.Interface) { putBodyBase: invalidBodyYAML, contentType: "application/yaml", strictDecodingWarnings: []string{ - `line 10: key "unknownDupe" already set in map`, - `line 12: key "paused" already set in map`, - `line 26: key "imagePullPolicy" already set in map`, + `line 5: key "name" already set in map`, + `line 12: key "unknownDupe" already set in map`, + `line 14: key "paused" already set in map`, + `line 28: key "imagePullPolicy" already set in map`, + `unknown field "metadata.unknownMeta"`, `unknown field "spec.template.spec.containers[0].unknownNested"`, `unknown field "spec.unknown1"`, `unknown field "spec.unknownDupe"`, @@ -1421,7 +1517,7 @@ func testFieldValidationPostCRD(t *testing.T, rest rest.Interface, gvk schema.Gr FieldValidation: "Strict", }, body: crdInvalidBody, - strictDecodingError: `strict decoding error: duplicate field "spec.unknownDupe", duplicate field "spec.knownField1", duplicate field "spec.ports[0].hostPort", unknown field "spec.ports[0].unknownNested", unknown field "spec.unknown1", unknown field "spec.unknownDupe"`, + strictDecodingError: `strict decoding error: duplicate field "metadata.name", duplicate field "spec.unknownDupe", duplicate field "spec.knownField1", duplicate field "spec.ports[0].hostPort", unknown field "metadata.unknownMeta", unknown field "spec.ports[0].unknownNested", unknown field "spec.unknown1", unknown field "spec.unknownDupe", unknown field "spec.embeddedObj.metadata.unknownEmbeddedMeta"`, }, { name: "crd-post-warn-validation", @@ -1430,12 +1526,15 @@ func testFieldValidationPostCRD(t *testing.T, rest rest.Interface, gvk schema.Gr }, body: crdInvalidBody, strictDecodingWarnings: []string{ + `duplicate field "metadata.name"`, `duplicate field "spec.unknownDupe"`, `duplicate field "spec.knownField1"`, `duplicate field "spec.ports[0].hostPort"`, + `unknown field "metadata.unknownMeta"`, `unknown field "spec.ports[0].unknownNested"`, `unknown field "spec.unknown1"`, `unknown field "spec.unknownDupe"`, + `unknown field "spec.embeddedObj.metadata.unknownEmbeddedMeta"`, }, }, { @@ -1449,12 +1548,15 @@ func testFieldValidationPostCRD(t *testing.T, rest rest.Interface, gvk schema.Gr name: "crd-post-no-validation", body: crdInvalidBody, strictDecodingWarnings: []string{ + `duplicate field "metadata.name"`, `duplicate field "spec.unknownDupe"`, `duplicate field "spec.knownField1"`, `duplicate field "spec.ports[0].hostPort"`, + `unknown field "metadata.unknownMeta"`, `unknown field "spec.ports[0].unknownNested"`, `unknown field "spec.unknown1"`, `unknown field "spec.unknownDupe"`, + `unknown field "spec.embeddedObj.metadata.unknownEmbeddedMeta"`, }, }, { @@ -1465,9 +1567,10 @@ func testFieldValidationPostCRD(t *testing.T, rest rest.Interface, gvk schema.Gr body: crdInvalidBodyYAML, contentType: "application/yaml", strictDecodingError: `strict decoding error: yaml: unmarshal errors: - line 10: key "unknownDupe" already set in map - line 12: key "knownField1" already set in map - line 18: key "hostPort" already set in map, unknown field "spec.ports[0].unknownNested", unknown field "spec.unknown1", unknown field "spec.unknownDupe"`, + line 6: key "name" already set in map + line 12: key "unknownDupe" already set in map + line 14: key "knownField1" already set in map + line 20: key "hostPort" already set in map, unknown field "metadata.unknownMeta", unknown field "spec.ports[0].unknownNested", unknown field "spec.unknown1", unknown field "spec.unknownDupe", unknown field "spec.embeddedObj.metadata.unknownEmbeddedMeta"`, }, { name: "crd-post-warn-validation-yaml", @@ -1477,12 +1580,15 @@ func testFieldValidationPostCRD(t *testing.T, rest rest.Interface, gvk schema.Gr body: crdInvalidBodyYAML, contentType: "application/yaml", strictDecodingWarnings: []string{ - `line 10: key "unknownDupe" already set in map`, - `line 12: key "knownField1" already set in map`, - `line 18: key "hostPort" already set in map`, + `line 6: key "name" already set in map`, + `line 12: key "unknownDupe" already set in map`, + `line 14: key "knownField1" already set in map`, + `line 20: key "hostPort" already set in map`, + `unknown field "metadata.unknownMeta"`, `unknown field "spec.ports[0].unknownNested"`, `unknown field "spec.unknown1"`, `unknown field "spec.unknownDupe"`, + `unknown field "spec.embeddedObj.metadata.unknownEmbeddedMeta"`, }, }, { @@ -1498,17 +1604,21 @@ func testFieldValidationPostCRD(t *testing.T, rest rest.Interface, gvk schema.Gr body: crdInvalidBodyYAML, contentType: "application/yaml", strictDecodingWarnings: []string{ - `line 10: key "unknownDupe" already set in map`, - `line 12: key "knownField1" already set in map`, - `line 18: key "hostPort" already set in map`, + `line 6: key "name" already set in map`, + `line 12: key "unknownDupe" already set in map`, + `line 14: key "knownField1" already set in map`, + `line 20: key "hostPort" already set in map`, + `unknown field "metadata.unknownMeta"`, `unknown field "spec.ports[0].unknownNested"`, `unknown field "spec.unknown1"`, `unknown field "spec.unknownDupe"`, + `unknown field "spec.embeddedObj.metadata.unknownEmbeddedMeta"`, }, }, } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { + klog.Warningf("running tc named: %s", tc.name) kind := gvk.Kind apiVersion := gvk.Group + "/" + gvk.Version @@ -1558,7 +1668,7 @@ func testFieldValidationPostCRDSchemaless(t *testing.T, rest rest.Interface, gvk FieldValidation: "Strict", }, body: crdInvalidBody, - strictDecodingError: `strict decoding error: duplicate field "spec.unknownDupe", duplicate field "spec.knownField1", duplicate field "spec.ports[0].hostPort", unknown field "spec.ports[0].unknownNested"`, + strictDecodingError: `strict decoding error: duplicate field "metadata.name", duplicate field "spec.unknownDupe", duplicate field "spec.knownField1", duplicate field "spec.ports[0].hostPort", unknown field "metadata.unknownMeta", unknown field "spec.ports[0].unknownNested", unknown field "spec.embeddedObj.metadata.unknownEmbeddedMeta"`, }, { name: "schemaless-crd-post-warn-validation", @@ -1567,10 +1677,13 @@ func testFieldValidationPostCRDSchemaless(t *testing.T, rest rest.Interface, gvk }, body: crdInvalidBody, strictDecodingWarnings: []string{ + `duplicate field "metadata.name"`, `duplicate field "spec.unknownDupe"`, `duplicate field "spec.knownField1"`, `duplicate field "spec.ports[0].hostPort"`, + `unknown field "metadata.unknownMeta"`, `unknown field "spec.ports[0].unknownNested"`, + `unknown field "spec.embeddedObj.metadata.unknownEmbeddedMeta"`, }, }, { @@ -1584,10 +1697,13 @@ func testFieldValidationPostCRDSchemaless(t *testing.T, rest rest.Interface, gvk name: "schemaless-crd-post-no-validation", body: crdInvalidBody, strictDecodingWarnings: []string{ + `duplicate field "metadata.name"`, `duplicate field "spec.unknownDupe"`, `duplicate field "spec.knownField1"`, `duplicate field "spec.ports[0].hostPort"`, + `unknown field "metadata.unknownMeta"`, `unknown field "spec.ports[0].unknownNested"`, + `unknown field "spec.embeddedObj.metadata.unknownEmbeddedMeta"`, }, }, { @@ -1598,9 +1714,10 @@ func testFieldValidationPostCRDSchemaless(t *testing.T, rest rest.Interface, gvk body: crdInvalidBodyYAML, contentType: "application/yaml", strictDecodingError: `strict decoding error: yaml: unmarshal errors: - line 10: key "unknownDupe" already set in map - line 12: key "knownField1" already set in map - line 18: key "hostPort" already set in map, unknown field "spec.ports[0].unknownNested"`, + line 6: key "name" already set in map + line 12: key "unknownDupe" already set in map + line 14: key "knownField1" already set in map + line 20: key "hostPort" already set in map, unknown field "metadata.unknownMeta", unknown field "spec.ports[0].unknownNested", unknown field "spec.embeddedObj.metadata.unknownEmbeddedMeta"`, }, { name: "schemaless-crd-post-warn-validation-yaml", @@ -1610,10 +1727,13 @@ func testFieldValidationPostCRDSchemaless(t *testing.T, rest rest.Interface, gvk body: crdInvalidBodyYAML, contentType: "application/yaml", strictDecodingWarnings: []string{ - `line 10: key "unknownDupe" already set in map`, - `line 12: key "knownField1" already set in map`, - `line 18: key "hostPort" already set in map`, + `line 6: key "name" already set in map`, + `line 12: key "unknownDupe" already set in map`, + `line 14: key "knownField1" already set in map`, + `line 20: key "hostPort" already set in map`, + `unknown field "metadata.unknownMeta"`, `unknown field "spec.ports[0].unknownNested"`, + `unknown field "spec.embeddedObj.metadata.unknownEmbeddedMeta"`, }, }, { @@ -1629,10 +1749,13 @@ func testFieldValidationPostCRDSchemaless(t *testing.T, rest rest.Interface, gvk body: crdInvalidBodyYAML, contentType: "application/yaml", strictDecodingWarnings: []string{ - `line 10: key "unknownDupe" already set in map`, - `line 12: key "knownField1" already set in map`, - `line 18: key "hostPort" already set in map`, + `line 6: key "name" already set in map`, + `line 12: key "unknownDupe" already set in map`, + `line 14: key "knownField1" already set in map`, + `line 20: key "hostPort" already set in map`, + `unknown field "metadata.unknownMeta"`, `unknown field "spec.ports[0].unknownNested"`, + `unknown field "spec.embeddedObj.metadata.unknownEmbeddedMeta"`, }, }, } @@ -1695,7 +1818,7 @@ func testFieldValidationPutCRD(t *testing.T, rest rest.Interface, gvk schema.Gro FieldValidation: "Strict", }, putBody: crdInvalidBody, - strictDecodingError: `strict decoding error: duplicate field "spec.unknownDupe", duplicate field "spec.knownField1", duplicate field "spec.ports[0].hostPort", unknown field "spec.ports[0].unknownNested", unknown field "spec.unknown1", unknown field "spec.unknownDupe"`, + strictDecodingError: `strict decoding error: duplicate field "metadata.name", duplicate field "spec.unknownDupe", duplicate field "spec.knownField1", duplicate field "spec.ports[0].hostPort", unknown field "metadata.unknownMeta", unknown field "spec.ports[0].unknownNested", unknown field "spec.unknown1", unknown field "spec.unknownDupe", unknown field "spec.embeddedObj.metadata.unknownEmbeddedMeta"`, }, { name: "crd-put-warn-validation", @@ -1704,12 +1827,15 @@ func testFieldValidationPutCRD(t *testing.T, rest rest.Interface, gvk schema.Gro }, putBody: crdInvalidBody, strictDecodingWarnings: []string{ + `duplicate field "metadata.name"`, `duplicate field "spec.unknownDupe"`, `duplicate field "spec.knownField1"`, `duplicate field "spec.ports[0].hostPort"`, + `unknown field "metadata.unknownMeta"`, `unknown field "spec.ports[0].unknownNested"`, `unknown field "spec.unknown1"`, `unknown field "spec.unknownDupe"`, + `unknown field "spec.embeddedObj.metadata.unknownEmbeddedMeta"`, }, }, { @@ -1723,12 +1849,15 @@ func testFieldValidationPutCRD(t *testing.T, rest rest.Interface, gvk schema.Gro name: "crd-put-no-validation", putBody: crdInvalidBody, strictDecodingWarnings: []string{ + `duplicate field "metadata.name"`, `duplicate field "spec.unknownDupe"`, `duplicate field "spec.knownField1"`, `duplicate field "spec.ports[0].hostPort"`, + `unknown field "metadata.unknownMeta"`, `unknown field "spec.ports[0].unknownNested"`, `unknown field "spec.unknown1"`, `unknown field "spec.unknownDupe"`, + `unknown field "spec.embeddedObj.metadata.unknownEmbeddedMeta"`, }, }, { @@ -1739,9 +1868,10 @@ func testFieldValidationPutCRD(t *testing.T, rest rest.Interface, gvk schema.Gro putBody: crdInvalidBodyYAML, contentType: "application/yaml", strictDecodingError: `strict decoding error: yaml: unmarshal errors: - line 10: key "unknownDupe" already set in map - line 12: key "knownField1" already set in map - line 18: key "hostPort" already set in map, unknown field "spec.ports[0].unknownNested", unknown field "spec.unknown1", unknown field "spec.unknownDupe"`, + line 6: key "name" already set in map + line 12: key "unknownDupe" already set in map + line 14: key "knownField1" already set in map + line 20: key "hostPort" already set in map, unknown field "metadata.unknownMeta", unknown field "spec.ports[0].unknownNested", unknown field "spec.unknown1", unknown field "spec.unknownDupe", unknown field "spec.embeddedObj.metadata.unknownEmbeddedMeta"`, }, { name: "crd-put-warn-validation-yaml", @@ -1751,12 +1881,15 @@ func testFieldValidationPutCRD(t *testing.T, rest rest.Interface, gvk schema.Gro putBody: crdInvalidBodyYAML, contentType: "application/yaml", strictDecodingWarnings: []string{ - `line 10: key "unknownDupe" already set in map`, - `line 12: key "knownField1" already set in map`, - `line 18: key "hostPort" already set in map`, + `line 6: key "name" already set in map`, + `line 12: key "unknownDupe" already set in map`, + `line 14: key "knownField1" already set in map`, + `line 20: key "hostPort" already set in map`, + `unknown field "metadata.unknownMeta"`, `unknown field "spec.ports[0].unknownNested"`, `unknown field "spec.unknown1"`, `unknown field "spec.unknownDupe"`, + `unknown field "spec.embeddedObj.metadata.unknownEmbeddedMeta"`, }, }, { @@ -1772,12 +1905,15 @@ func testFieldValidationPutCRD(t *testing.T, rest rest.Interface, gvk schema.Gro putBody: crdInvalidBodyYAML, contentType: "application/yaml", strictDecodingWarnings: []string{ - `line 10: key "unknownDupe" already set in map`, - `line 12: key "knownField1" already set in map`, - `line 18: key "hostPort" already set in map`, + `line 6: key "name" already set in map`, + `line 12: key "unknownDupe" already set in map`, + `line 14: key "knownField1" already set in map`, + `line 20: key "hostPort" already set in map`, + `unknown field "metadata.unknownMeta"`, `unknown field "spec.ports[0].unknownNested"`, `unknown field "spec.unknown1"`, `unknown field "spec.unknownDupe"`, + `unknown field "spec.embeddedObj.metadata.unknownEmbeddedMeta"`, }, }, } @@ -1847,7 +1983,7 @@ func testFieldValidationPutCRDSchemaless(t *testing.T, rest rest.Interface, gvk FieldValidation: "Strict", }, putBody: crdInvalidBody, - strictDecodingError: `strict decoding error: duplicate field "spec.unknownDupe", duplicate field "spec.knownField1", duplicate field "spec.ports[0].hostPort", unknown field "spec.ports[0].unknownNested"`, + strictDecodingError: `strict decoding error: duplicate field "metadata.name", duplicate field "spec.unknownDupe", duplicate field "spec.knownField1", duplicate field "spec.ports[0].hostPort", unknown field "metadata.unknownMeta", unknown field "spec.ports[0].unknownNested", unknown field "spec.embeddedObj.metadata.unknownEmbeddedMeta"`, }, { name: "schemaless-crd-put-warn-validation", @@ -1856,10 +1992,13 @@ func testFieldValidationPutCRDSchemaless(t *testing.T, rest rest.Interface, gvk }, putBody: crdInvalidBody, strictDecodingWarnings: []string{ + `duplicate field "metadata.name"`, `duplicate field "spec.unknownDupe"`, `duplicate field "spec.knownField1"`, `duplicate field "spec.ports[0].hostPort"`, + `unknown field "metadata.unknownMeta"`, `unknown field "spec.ports[0].unknownNested"`, + `unknown field "spec.embeddedObj.metadata.unknownEmbeddedMeta"`, }, }, { @@ -1873,10 +2012,13 @@ func testFieldValidationPutCRDSchemaless(t *testing.T, rest rest.Interface, gvk name: "schemaless-crd-put-no-validation", putBody: crdInvalidBody, strictDecodingWarnings: []string{ + `duplicate field "metadata.name"`, `duplicate field "spec.unknownDupe"`, `duplicate field "spec.knownField1"`, `duplicate field "spec.ports[0].hostPort"`, + `unknown field "metadata.unknownMeta"`, `unknown field "spec.ports[0].unknownNested"`, + `unknown field "spec.embeddedObj.metadata.unknownEmbeddedMeta"`, }, }, { @@ -1887,9 +2029,10 @@ func testFieldValidationPutCRDSchemaless(t *testing.T, rest rest.Interface, gvk putBody: crdInvalidBodyYAML, contentType: "application/yaml", strictDecodingError: `strict decoding error: yaml: unmarshal errors: - line 10: key "unknownDupe" already set in map - line 12: key "knownField1" already set in map - line 18: key "hostPort" already set in map, unknown field "spec.ports[0].unknownNested"`, + line 6: key "name" already set in map + line 12: key "unknownDupe" already set in map + line 14: key "knownField1" already set in map + line 20: key "hostPort" already set in map, unknown field "metadata.unknownMeta", unknown field "spec.ports[0].unknownNested", unknown field "spec.embeddedObj.metadata.unknownEmbeddedMeta"`, }, { name: "schemaless-crd-put-warn-validation-yaml", @@ -1899,10 +2042,13 @@ func testFieldValidationPutCRDSchemaless(t *testing.T, rest rest.Interface, gvk putBody: crdInvalidBodyYAML, contentType: "application/yaml", strictDecodingWarnings: []string{ - `line 10: key "unknownDupe" already set in map`, - `line 12: key "knownField1" already set in map`, - `line 18: key "hostPort" already set in map`, + `line 6: key "name" already set in map`, + `line 12: key "unknownDupe" already set in map`, + `line 14: key "knownField1" already set in map`, + `line 20: key "hostPort" already set in map`, + `unknown field "metadata.unknownMeta"`, `unknown field "spec.ports[0].unknownNested"`, + `unknown field "spec.embeddedObj.metadata.unknownEmbeddedMeta"`, }, }, { @@ -1918,10 +2064,13 @@ func testFieldValidationPutCRDSchemaless(t *testing.T, rest rest.Interface, gvk putBody: crdInvalidBodyYAML, contentType: "application/yaml", strictDecodingWarnings: []string{ - `line 10: key "unknownDupe" already set in map`, - `line 12: key "knownField1" already set in map`, - `line 18: key "hostPort" already set in map`, + `line 6: key "name" already set in map`, + `line 12: key "unknownDupe" already set in map`, + `line 14: key "knownField1" already set in map`, + `line 20: key "hostPort" already set in map`, + `unknown field "metadata.unknownMeta"`, `unknown field "spec.ports[0].unknownNested"`, + `unknown field "spec.embeddedObj.metadata.unknownEmbeddedMeta"`, }, }, } diff --git a/vendor/sigs.k8s.io/json/internal/golang/encoding/json/decode.go b/vendor/sigs.k8s.io/json/internal/golang/encoding/json/decode.go index 3a8b64547da..dd551dbd087 100644 --- a/vendor/sigs.k8s.io/json/internal/golang/encoding/json/decode.go +++ b/vendor/sigs.k8s.io/json/internal/golang/encoding/json/decode.go @@ -695,7 +695,7 @@ func (d *decodeState) object(v reflect.Value) error { seenKeys = map[string]struct{}{} } if _, seen := seenKeys[fieldName]; seen { - d.saveStrictError(d.newFieldError("duplicate field", fieldName)) + d.saveStrictError(d.newFieldError(duplicateStrictErrType, fieldName)) } else { seenKeys[fieldName] = struct{}{} } @@ -711,7 +711,7 @@ func (d *decodeState) object(v reflect.Value) error { var seenKeys uint64 checkDuplicateField = func(fieldNameIndex int, fieldName string) { if seenKeys&(1< 0 { - return fmt.Errorf("%s %q", msg, strings.Join(d.strictFieldStack, "")+"."+field) + return &strictError{ + ErrType: errType, + Path: strings.Join(d.strictFieldStack, "") + "." + field, + } } else { - return fmt.Errorf("%s %q", msg, field) + return &strictError{ + ErrType: errType, + Path: field, + } } } // saveStrictError saves a strict decoding error, // for reporting at the end of the unmarshal if no other errors occurred. -func (d *decodeState) saveStrictError(err error) { +func (d *decodeState) saveStrictError(err *strictError) { // prevent excessive numbers of accumulated errors if len(d.savedStrictErrors) >= 100 { return @@ -118,6 +123,33 @@ func (d *decodeState) appendStrictFieldStackIndex(i int) { d.strictFieldStack = append(d.strictFieldStack, "[", strconv.Itoa(i), "]") } +type strictErrType string + +const ( + unknownStrictErrType strictErrType = "unknown field" + duplicateStrictErrType strictErrType = "duplicate field" +) + +// strictError is a strict decoding error +// It has an ErrType (either unknown or duplicate) +// and a path to the erroneous field +type strictError struct { + ErrType strictErrType + Path string +} + +func (e *strictError) Error() string { + return string(e.ErrType) + " " + strconv.Quote(e.Path) +} + +func (e *strictError) FieldPath() string { + return e.Path +} + +func (e *strictError) SetFieldPath(path string) { + e.Path = path +} + // UnmarshalStrictError holds errors resulting from use of strict disallow___ decoder directives. // If this is returned from Unmarshal(), it means the decoding was successful in all other respects. type UnmarshalStrictError struct { diff --git a/vendor/sigs.k8s.io/json/json.go b/vendor/sigs.k8s.io/json/json.go index 764e2a84c72..0a1da3ea115 100644 --- a/vendor/sigs.k8s.io/json/json.go +++ b/vendor/sigs.k8s.io/json/json.go @@ -137,3 +137,11 @@ func SyntaxErrorOffset(err error) (isSyntaxError bool, offset int64) { return false, 0 } } + +// FieldError are errors that provide access to +// the path of the erroneous field +type FieldError interface { + Error() string + FieldPath() string + SetFieldPath(path string) +}