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 06c26b3296d..59855223449 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,6 +245,11 @@ 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) @@ -253,10 +258,8 @@ 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)) } - // start a deepcopy of the input and fill in the converted objects from the response at the right spots. + // 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 8c60cb7f0da..1151e0a674d 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,6 +46,7 @@ func TestConversion(t *testing.T) { SourceObject: &unstructured.Unstructured{ Object: map[string]interface{}{ "apiVersion": "example.com/v1", + "metadata": map[string]interface{}{}, "other": "data", "kind": "foo", }, @@ -53,6 +54,7 @@ func TestConversion(t *testing.T) { ExpectedObject: &unstructured.Unstructured{ Object: map[string]interface{}{ "apiVersion": "example.com/v2", + "metadata": map[string]interface{}{}, "other": "data", "kind": "foo", }, @@ -86,6 +88,7 @@ func TestConversion(t *testing.T) { { Object: map[string]interface{}{ "apiVersion": "example.com/v1", + "metadata": map[string]interface{}{}, "kind": "foo", "other": "data", }, @@ -93,6 +96,7 @@ func TestConversion(t *testing.T) { { Object: map[string]interface{}{ "apiVersion": "example.com/v1", + "metadata": map[string]interface{}{}, "kind": "foo", "other": "data2", }, @@ -108,6 +112,7 @@ func TestConversion(t *testing.T) { { Object: map[string]interface{}{ "apiVersion": "example.com/v2", + "metadata": map[string]interface{}{}, "kind": "foo", "other": "data", }, @@ -115,6 +120,7 @@ func TestConversion(t *testing.T) { { Object: map[string]interface{}{ "apiVersion": "example.com/v2", + "metadata": map[string]interface{}{}, "kind": "foo", "other": "data2", }, @@ -288,3 +294,79 @@ 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 +}