Merge pull request #64208 from sttts/sttts-split-up-nop-converter

Automatic merge from submit-queue (batch tested with PRs 64060, 63904, 64218, 64208, 64247). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

apiextensions: extract orthortogonal behaviour from nopConve

This is preparation for adding more CR converters. Not every new converter should implement the generic conversion behaviour of CRs.

Fixes parts of https://github.com/kubernetes/kubernetes/issues/64136.

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/converter.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 )
@@ -33,13 +34,62 @@ func NewCRDConverter(crd *apiextensions.CustomResourceDefinition) (safe, unsafe
 
 	// The only converter right now is nopConverter. More converters will be returned based on the
 	// CRD object when they introduced.
-	unsafe = &nopConverter{
+	unsafe = &crdConverter{
 		clusterScoped: crd.Spec.Scope == apiextensions.ClusterScoped,
+		delegate: &nopConverter{
 			validVersions: validVersions,
+		},
 	}
 	return &safeConverterWrapper{unsafe}, unsafe
 }
 
+var _ runtime.ObjectConvertor = &crdConverter{}
+
+// crdConverter extends the delegate with generic CRD conversion behaviour. The delegate will implement the
+// user defined conversion strategy given in the CustomResourceDefinition.
+type crdConverter struct {
+	delegate      runtime.ObjectConvertor
+	clusterScoped bool
+}
+
+func (c *crdConverter) ConvertFieldLabel(version, kind, label, value string) (string, string, error) {
+	// We currently only support metadata.namespace and metadata.name.
+	switch {
+	case label == "metadata.name":
+		return label, value, nil
+	case !c.clusterScoped && label == "metadata.namespace":
+		return label, value, nil
+	default:
+		return "", "", fmt.Errorf("field label not supported: %s", label)
+	}
+}
+
+func (c *crdConverter) Convert(in, out, context interface{}) error {
+	return c.delegate.Convert(in, out, context)
+}
+
+// ConvertToVersion converts in object to the given gvk in place and returns the same `in` object.
+func (c *crdConverter) ConvertToVersion(in runtime.Object, target runtime.GroupVersioner) (runtime.Object, error) {
+	// Run the converter on the list items instead of list itself
+	if list, ok := in.(*unstructured.UnstructuredList); ok {
+		for i := range list.Items {
+			obj, err := c.delegate.ConvertToVersion(&list.Items[i], target)
+			if err != nil {
+				return nil, err
+			}
+
+			u, ok := obj.(*unstructured.Unstructured)
+			if !ok {
+				return nil, fmt.Errorf("output type %T in not valid for unstructured conversion", obj)
+			}
+			list.Items[i] = *u
+		}
+		return list, nil
+	}
+
+	return c.delegate.ConvertToVersion(in, target)
+}
+
 // safeConverterWrapper is a wrapper over an unsafe object converter that makes copy of the input and then delegate to the unsafe converter.
 type safeConverterWrapper struct {
 	unsafe runtime.ObjectConvertor

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/conversion/nop_converter.go

@@ -17,6 +17,7 @@ limitations under the License.
 package conversion
 
 import (
+	"errors"
 	"fmt"
 
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -24,24 +25,15 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 )
 
-// nopConverter is a converter that only sets the apiVersion fields, but does not real conversion. It supports fields selectors.
+// nopConverter is a converter that only sets the apiVersion fields, but does not real conversion.
 type nopConverter struct {
-	clusterScoped bool
 	validVersions map[schema.GroupVersion]bool
 }
 
 var _ runtime.ObjectConvertor = &nopConverter{}
 
-func (c *nopConverter) ConvertFieldLabel(version, kind, label, value string) (string, string, error) {
+func (nopConverter) ConvertFieldLabel(version, kind, label, value string) (string, string, error) {
-	// We currently only support metadata.namespace and metadata.name.
+	return "", "", errors.New("unstructured cannot convert field labels")
-	switch {
-	case label == "metadata.name":
-		return label, value, nil
-	case !c.clusterScoped && label == "metadata.namespace":
-		return label, value, nil
-	default:
-		return "", "", fmt.Errorf("field label not supported: %s", label)
-	}
 }
 
 func (c *nopConverter) Convert(in, out, context interface{}) error {
@@ -72,29 +64,16 @@ func (c *nopConverter) Convert(in, out, context interface{}) error {
 	return nil
 }
 
-func (c *nopConverter) convertToVersion(in runtime.Object, target runtime.GroupVersioner) error {
+func (c *nopConverter) ConvertToVersion(in runtime.Object, target runtime.GroupVersioner) (runtime.Object, error) {
 	kind := in.GetObjectKind().GroupVersionKind()
 	gvk, ok := target.KindForGroupVersionKinds([]schema.GroupVersionKind{kind})
 	if !ok {
 		// TODO: should this be a typed error?
-		return fmt.Errorf("%v is unstructured and is not suitable for converting to %q", kind, target)
+		return nil, fmt.Errorf("%v is unstructured and is not suitable for converting to %q", kind, target)
 	}
 	if !c.validVersions[gvk.GroupVersion()] {
-		return fmt.Errorf("request to convert CRD to an invalid group/version: %s", gvk.String())
+		return nil, fmt.Errorf("request to convert CRD to an invalid group/version: %s", gvk.String())
 	}
 	in.GetObjectKind().SetGroupVersionKind(gvk)
-	return nil
+	return in, nil
-}
-
-// ConvertToVersion converts in object to the given gvk in place and returns the same `in` object.
-func (c *nopConverter) ConvertToVersion(in runtime.Object, target runtime.GroupVersioner) (runtime.Object, error) {
-	var err error
-	// Run the converter on the list items instead of list itself
-	if list, ok := in.(*unstructured.UnstructuredList); ok {
-		err = list.EachListItem(func(item runtime.Object) error {
-			return c.convertToVersion(item, target)
-		})
-	}
-	err = c.convertToVersion(in, target)
-	return in, err
 }