Merge pull request #107697 from kevindelgado/nested-decoding

Nested decoders handle strict decoding errors

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",

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
 }

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) {
+		if strictErr, ok := runtime.AsStrictDecodingError(err); obj != nil && ok {
-			// save the strictDecodingError and the caller decide what to do with it
+			// save the strictDecodingError and let the caller decide what to do with it
-			strictDecodingErr = err
+			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

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")}

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 {

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 {