diff --git a/pkg/scheduler/apis/config/scheme/scheme_test.go b/pkg/scheduler/apis/config/scheme/scheme_test.go index a2b8400ac4b..8970352f8aa 100644 --- a/pkg/scheduler/apis/config/scheme/scheme_test.go +++ b/pkg/scheduler/apis/config/scheme/scheme_test.go @@ -263,7 +263,7 @@ profiles: - Score: 2 Utilization: 1 `), - wantErr: `decoding .profiles[0].pluginConfig[0]: decoding args for plugin NodeResourcesFit: strict decoding error: unknown field "scoringStrategy.requestedToCapacityRatio.shape[0].Score", unknown field "scoringStrategy.requestedToCapacityRatio.shape[0].Utilization"`, + wantErr: `strict decoding error: decoding .profiles[0].pluginConfig[0]: strict decoding error: decoding args for plugin NodeResourcesFit: strict decoding error: unknown field "scoringStrategy.requestedToCapacityRatio.shape[0].Score", unknown field "scoringStrategy.requestedToCapacityRatio.shape[0].Utilization"`, }, { name: "v1beta2 NodeResourcesFitArgs resources encoding is strict", @@ -279,7 +279,7 @@ profiles: - Name: cpu Weight: 1 `), - wantErr: `decoding .profiles[0].pluginConfig[0]: decoding args for plugin NodeResourcesFit: strict decoding error: unknown field "scoringStrategy.resources[0].Name", unknown field "scoringStrategy.resources[0].Weight"`, + wantErr: `strict decoding error: decoding .profiles[0].pluginConfig[0]: strict decoding error: decoding args for plugin NodeResourcesFit: strict decoding error: unknown field "scoringStrategy.resources[0].Name", unknown field "scoringStrategy.resources[0].Weight"`, }, { name: "out-of-tree plugin args", @@ -604,7 +604,7 @@ profiles: - Score: 2 Utilization: 1 `), - wantErr: `decoding .profiles[0].pluginConfig[0]: decoding args for plugin NodeResourcesFit: strict decoding error: unknown field "scoringStrategy.requestedToCapacityRatio.shape[0].Score", unknown field "scoringStrategy.requestedToCapacityRatio.shape[0].Utilization"`, + wantErr: `strict decoding error: decoding .profiles[0].pluginConfig[0]: strict decoding error: decoding args for plugin NodeResourcesFit: strict decoding error: unknown field "scoringStrategy.requestedToCapacityRatio.shape[0].Score", unknown field "scoringStrategy.requestedToCapacityRatio.shape[0].Utilization"`, }, { name: "v1beta3 NodeResourcesFitArgs resources encoding is strict", @@ -620,7 +620,7 @@ profiles: - Name: cpu Weight: 1 `), - wantErr: `decoding .profiles[0].pluginConfig[0]: decoding args for plugin NodeResourcesFit: strict decoding error: unknown field "scoringStrategy.resources[0].Name", unknown field "scoringStrategy.resources[0].Weight"`, + wantErr: `strict decoding error: decoding .profiles[0].pluginConfig[0]: strict decoding error: decoding args for plugin NodeResourcesFit: strict decoding error: unknown field "scoringStrategy.resources[0].Name", unknown field "scoringStrategy.resources[0].Weight"`, }, { name: "out-of-tree plugin args", diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/interfaces.go b/staging/src/k8s.io/apimachinery/pkg/runtime/interfaces.go index 3ed5bf7bce6..bd4f22b1bc8 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/interfaces.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/interfaces.go @@ -207,6 +207,12 @@ type NestedObjectEncoder interface { // NestedObjectDecoder is an optional interface that objects may implement to be given // an opportunity to decode any nested Objects / RawExtensions during serialization. +// It is possible for DecodeNestedObjects to return a non-nil error but for the decoding +// to have succeeded in the case of strict decoding errors (e.g. unknown/duplicate fields). +// As such it is important for callers of DecodeNestedObjects to check to confirm whether +// an error is a runtime.StrictDecodingError before short circuiting. +// Similarly, implementations of DecodeNestedObjects should ensure that a runtime.StrictDecodingError +// is only returned when the rest of decoding has succeeded. type NestedObjectDecoder interface { DecodeNestedObjects(d Decoder) error } 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 ea7c580bd6b..844730e6ba3 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 @@ -133,24 +133,34 @@ func (c *codec) Decode(data []byte, defaultGVK *schema.GroupVersionKind, into ru } } - var strictDecodingErr error + var strictDecodingErrs []error obj, gvk, err := c.decoder.Decode(data, defaultGVK, decodeInto) if err != nil { - if obj != nil && runtime.IsStrictDecodingError(err) { - // save the strictDecodingError and the caller decide what to do with it - strictDecodingErr = err + if strictErr, ok := runtime.AsStrictDecodingError(err); obj != nil && ok { + // save the strictDecodingError and let the caller decide what to do with it + strictDecodingErrs = append(strictDecodingErrs, strictErr.Errors()...) } else { return nil, gvk, err } } - // TODO: look into strict handling of nested object decoding if d, ok := obj.(runtime.NestedObjectDecoder); ok { if err := d.DecodeNestedObjects(runtime.WithoutVersionDecoder{c.decoder}); err != nil { - return nil, gvk, err + if strictErr, ok := runtime.AsStrictDecodingError(err); ok { + // save the strictDecodingError let and the caller decide what to do with it + strictDecodingErrs = append(strictDecodingErrs, strictErr.Errors()...) + } else { + return nil, gvk, err + + } } } + // aggregate the strict decoding errors into one + var strictDecodingErr error + if len(strictDecodingErrs) > 0 { + strictDecodingErr = runtime.NewStrictDecodingError(strictDecodingErrs) + } // if we specify a target, use generic conversion. if into != nil { // perform defaulting if requested diff --git a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/versioning/versioning_test.go b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/versioning/versioning_test.go index ee3058c3c79..82372cd25cf 100644 --- a/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/versioning/versioning_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/runtime/serializer/versioning/versioning_test.go @@ -82,6 +82,23 @@ func TestNestedDecode(t *testing.T) { } } +func TestNestedDecodeStrictDecodingError(t *testing.T) { + strictErr := runtime.NewStrictDecodingError([]error{fmt.Errorf("duplicate field")}) + n := &testNestedDecodable{nestedErr: strictErr} + decoder := &mockSerializer{obj: n} + codec := NewCodec(nil, decoder, nil, nil, nil, nil, nil, nil, "TestNestedDecode") + o, _, err := codec.Decode([]byte(`{}`), nil, n) + if strictErr, ok := runtime.AsStrictDecodingError(err); !ok || err != strictErr { + t.Errorf("unexpected error: %v", err) + } + if o != n { + t.Errorf("did not successfully decode with strict decoding error: %v", o) + } + if !n.nestedCalled { + t.Errorf("did not invoke nested decoder") + } +} + func TestNestedEncode(t *testing.T) { n := &testNestedDecodable{nestedErr: fmt.Errorf("unable to decode")} n2 := &testNestedDecodable{nestedErr: fmt.Errorf("unable to decode 2")} diff --git a/staging/src/k8s.io/kube-scheduler/config/v1beta2/types.go b/staging/src/k8s.io/kube-scheduler/config/v1beta2/types.go index d1d1d1a595e..f561260e974 100644 --- a/staging/src/k8s.io/kube-scheduler/config/v1beta2/types.go +++ b/staging/src/k8s.io/kube-scheduler/config/v1beta2/types.go @@ -100,15 +100,24 @@ type KubeSchedulerConfiguration struct { // DecodeNestedObjects decodes plugin args for known types. func (c *KubeSchedulerConfiguration) DecodeNestedObjects(d runtime.Decoder) error { + var strictDecodingErrs []error for i := range c.Profiles { prof := &c.Profiles[i] for j := range prof.PluginConfig { err := prof.PluginConfig[j].decodeNestedObjects(d) if err != nil { - return fmt.Errorf("decoding .profiles[%d].pluginConfig[%d]: %w", i, j, err) + decodingErr := fmt.Errorf("decoding .profiles[%d].pluginConfig[%d]: %w", i, j, err) + if runtime.IsStrictDecodingError(err) { + strictDecodingErrs = append(strictDecodingErrs, decodingErr) + } else { + return decodingErr + } } } } + if len(strictDecodingErrs) > 0 { + return runtime.NewStrictDecodingError(strictDecodingErrs) + } return nil } @@ -237,15 +246,21 @@ func (c *PluginConfig) decodeNestedObjects(d runtime.Decoder) error { return nil } + var strictDecodingErr error obj, parsedGvk, err := d.Decode(c.Args.Raw, &gvk, nil) if err != nil { - return fmt.Errorf("decoding args for plugin %s: %w", c.Name, err) + decodingArgsErr := fmt.Errorf("decoding args for plugin %s: %w", c.Name, err) + if obj != nil && runtime.IsStrictDecodingError(err) { + strictDecodingErr = runtime.NewStrictDecodingError([]error{decodingArgsErr}) + } else { + return decodingArgsErr + } } if parsedGvk.GroupKind() != gvk.GroupKind() { return fmt.Errorf("args for plugin %s were not of type %s, got %s", c.Name, gvk.GroupKind(), parsedGvk.GroupKind()) } c.Args.Object = obj - return nil + return strictDecodingErr } func (c *PluginConfig) encodeNestedObjects(e runtime.Encoder) error { diff --git a/staging/src/k8s.io/kube-scheduler/config/v1beta3/types.go b/staging/src/k8s.io/kube-scheduler/config/v1beta3/types.go index 4ab11007c23..385109f16e8 100644 --- a/staging/src/k8s.io/kube-scheduler/config/v1beta3/types.go +++ b/staging/src/k8s.io/kube-scheduler/config/v1beta3/types.go @@ -93,15 +93,24 @@ type KubeSchedulerConfiguration struct { // DecodeNestedObjects decodes plugin args for known types. func (c *KubeSchedulerConfiguration) DecodeNestedObjects(d runtime.Decoder) error { + var strictDecodingErrs []error for i := range c.Profiles { prof := &c.Profiles[i] for j := range prof.PluginConfig { err := prof.PluginConfig[j].decodeNestedObjects(d) if err != nil { - return fmt.Errorf("decoding .profiles[%d].pluginConfig[%d]: %w", i, j, err) + decodingErr := fmt.Errorf("decoding .profiles[%d].pluginConfig[%d]: %w", i, j, err) + if runtime.IsStrictDecodingError(err) { + strictDecodingErrs = append(strictDecodingErrs, decodingErr) + } else { + return decodingErr + } } } } + if len(strictDecodingErrs) > 0 { + return runtime.NewStrictDecodingError(strictDecodingErrs) + } return nil } @@ -245,15 +254,21 @@ func (c *PluginConfig) decodeNestedObjects(d runtime.Decoder) error { return nil } + var strictDecodingErr error obj, parsedGvk, err := d.Decode(c.Args.Raw, &gvk, nil) if err != nil { - return fmt.Errorf("decoding args for plugin %s: %w", c.Name, err) + decodingArgsErr := fmt.Errorf("decoding args for plugin %s: %w", c.Name, err) + if obj != nil && runtime.IsStrictDecodingError(err) { + strictDecodingErr = runtime.NewStrictDecodingError([]error{decodingArgsErr}) + } else { + return decodingArgsErr + } } if parsedGvk.GroupKind() != gvk.GroupKind() { return fmt.Errorf("args for plugin %s were not of type %s, got %s", c.Name, gvk.GroupKind(), parsedGvk.GroupKind()) } c.Args.Object = obj - return nil + return strictDecodingErr } func (c *PluginConfig) encodeNestedObjects(e runtime.Encoder) error {