diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/converter.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/converter.go index 59855223449..06c26b3296d 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/converter.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/converter.go @@ -245,11 +245,6 @@ func (c *delegatingCRConverter) ConvertToVersion(in runtime.Object, target runti return converted, nil } - // Deep copy the list before we invoke the converter to ensure that if the converter does mutate the - // list (which it shouldn't, but you never know), it doesn't have any impact. - convertedList := list.DeepCopy() - convertedList.SetAPIVersion(desiredAPIVersion) - convertedObjects, err := c.converter.Convert(list, toGVK.GroupVersion()) if err != nil { return nil, fmt.Errorf("conversion for %v failed: %w", in.GetObjectKind().GroupVersionKind(), err) @@ -258,8 +253,10 @@ func (c *delegatingCRConverter) ConvertToVersion(in runtime.Object, target runti return nil, fmt.Errorf("conversion for %v returned %d objects, expected %d", in.GetObjectKind().GroupVersionKind(), len(convertedObjects.Items), len(objectsToConvert)) } - // Fill in the converted objects from the response at the right spots. + // start a deepcopy of the input and fill in the converted objects from the response at the right spots. // The response list might be sparse because objects had the right version already. + convertedList := list.DeepCopy() + convertedList.SetAPIVersion(desiredAPIVersion) convertedIndex := 0 for i := range convertedList.Items { original := &convertedList.Items[i] diff --git a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/converter_test.go b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/converter_test.go index 1151e0a674d..8c60cb7f0da 100644 --- a/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/converter_test.go +++ b/staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/converter_test.go @@ -46,7 +46,6 @@ func TestConversion(t *testing.T) { SourceObject: &unstructured.Unstructured{ Object: map[string]interface{}{ "apiVersion": "example.com/v1", - "metadata": map[string]interface{}{}, "other": "data", "kind": "foo", }, @@ -54,7 +53,6 @@ func TestConversion(t *testing.T) { ExpectedObject: &unstructured.Unstructured{ Object: map[string]interface{}{ "apiVersion": "example.com/v2", - "metadata": map[string]interface{}{}, "other": "data", "kind": "foo", }, @@ -88,7 +86,6 @@ func TestConversion(t *testing.T) { { Object: map[string]interface{}{ "apiVersion": "example.com/v1", - "metadata": map[string]interface{}{}, "kind": "foo", "other": "data", }, @@ -96,7 +93,6 @@ func TestConversion(t *testing.T) { { Object: map[string]interface{}{ "apiVersion": "example.com/v1", - "metadata": map[string]interface{}{}, "kind": "foo", "other": "data2", }, @@ -112,7 +108,6 @@ func TestConversion(t *testing.T) { { Object: map[string]interface{}{ "apiVersion": "example.com/v2", - "metadata": map[string]interface{}{}, "kind": "foo", "other": "data", }, @@ -120,7 +115,6 @@ func TestConversion(t *testing.T) { { Object: map[string]interface{}{ "apiVersion": "example.com/v2", - "metadata": map[string]interface{}{}, "kind": "foo", "other": "data2", }, @@ -294,79 +288,3 @@ func TestGetObjectsToConvert(t *testing.T) { }) } } - -func TestConverterMutatesInput(t *testing.T) { - testCRD := apiextensionsv1.CustomResourceDefinition{ - Spec: apiextensionsv1.CustomResourceDefinitionSpec{ - Conversion: &apiextensionsv1.CustomResourceConversion{ - Strategy: apiextensionsv1.NoneConverter, - }, - Group: "test.k8s.io", - Versions: []apiextensionsv1.CustomResourceDefinitionVersion{ - { - Name: "v1alpha1", - Served: true, - }, - { - Name: "v1alpha2", - Served: true, - }, - }, - }, - } - - safeConverter, _, err := NewDelegatingConverter(&testCRD, &inputMutatingConverter{}) - if err != nil { - t.Fatalf("Cannot create converter: %v", err) - } - - input := &unstructured.UnstructuredList{ - Object: map[string]interface{}{ - "apiVersion": "test.k8s.io/v1alpha1", - }, - Items: []unstructured.Unstructured{ - { - Object: map[string]interface{}{ - "apiVersion": "test.k8s.io/v1alpha1", - "metadata": map[string]interface{}{ - "name": "item1", - }, - }, - }, - { - Object: map[string]interface{}{ - "apiVersion": "test.k8s.io/v1alpha1", - "metadata": map[string]interface{}{ - "name": "item2", - }, - }, - }, - }, - } - - toVersion, _ := schema.ParseGroupVersion("test.k8s.io/v1alpha2") - toVersions := schema.GroupVersions{toVersion} - converted, err := safeConverter.ConvertToVersion(input, toVersions) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - convertedList := converted.(*unstructured.UnstructuredList) - if e, a := 2, len(convertedList.Items); e != a { - t.Fatalf("length: expected %d, got %d", e, a) - } -} - -type inputMutatingConverter struct{} - -func (i *inputMutatingConverter) Convert(in *unstructured.UnstructuredList, targetGVK schema.GroupVersion) (*unstructured.UnstructuredList, error) { - out := &unstructured.UnstructuredList{} - for _, obj := range in.Items { - u := obj.DeepCopy() - u.SetAPIVersion(targetGVK.String()) - out.Items = append(out.Items, *u) - } - - in.Items = nil - - return out, nil -}