Merge pull request #65780 from liggitt/AddFieldLabelConversionFuncGVK

Automatic merge from submit-queue (batch tested with PRs 65830, 65780, 65961). 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>.

change field selector conversion registration to be strongly typed

the signature of these methods is misleading... they require a group-version-kind

```release-note
NONE
```

pkg/api/testing/BUILD

@@ -37,6 +37,7 @@ go_library(
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/fuzzer:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/fields:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library",
         "//vendor/github.com/google/gofuzz:go_default_library",
     ],

pkg/api/testing/conversion.go

@@ -20,6 +20,7 @@ import (
 	"testing"
 
 	"k8s.io/apimachinery/pkg/fields"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/kubernetes/pkg/api/legacyscheme"
 )
 
@@ -38,6 +39,13 @@ func TestSelectableFieldLabelConversionsOfKind(t *testing.T, apiVersion string,
 
 	value := "value"
 
+	gv, err := schema.ParseGroupVersion(apiVersion)
+	if err != nil {
+		t.Errorf("kind=%s: got unexpected error: %v", kind, err)
+		return
+	}
+	gvk := gv.WithKind(kind)
+
 	if len(fields) == 0 {
 		t.Logf("no selectable fields for kind %q, skipping", kind)
 	}
@@ -46,7 +54,7 @@ func TestSelectableFieldLabelConversionsOfKind(t *testing.T, apiVersion string,
 			t.Logf("FIXME: \"name\" is deprecated by \"metadata.name\", it should be removed from selectable fields of kind=%s", kind)
 			continue
 		}
-		newLabel, newValue, err := legacyscheme.Scheme.ConvertFieldLabel(apiVersion, kind, label, value)
+		newLabel, newValue, err := legacyscheme.Scheme.ConvertFieldLabel(gvk, label, value)
 		if err != nil {
 			t.Errorf("kind=%s label=%s: got unexpected error: %v", kind, label, err)
 		} else {
@@ -64,7 +72,7 @@ func TestSelectableFieldLabelConversionsOfKind(t *testing.T, apiVersion string,
 	}
 
 	for _, label := range badFieldLabels {
-		_, _, err := legacyscheme.Scheme.ConvertFieldLabel(apiVersion, kind, label, "value")
+		_, _, err := legacyscheme.Scheme.ConvertFieldLabel(gvk, label, "value")
 		if err == nil {
 			t.Errorf("kind=%s label=%s: got unexpected non-error", kind, label)
 		}

pkg/apis/apps/v1beta1/conversion.go

@@ -60,7 +60,7 @@ func addConversionFuncs(scheme *runtime.Scheme) error {
 	}
 
 	// Add field label conversions for kinds having selectable nothing but ObjectMeta fields.
-	err = scheme.AddFieldLabelConversionFunc("apps/v1beta1", "StatefulSet",
+	err = scheme.AddFieldLabelConversionFunc(SchemeGroupVersion.WithKind("StatefulSet"),
 		func(label, value string) (string, string, error) {
 			switch label {
 			case "metadata.name", "metadata.namespace", "status.successful":

pkg/apis/apps/v1beta2/conversion.go

@@ -75,7 +75,7 @@ func addConversionFuncs(scheme *runtime.Scheme) error {
 	}
 
 	// Add field label conversions for kinds having selectable nothing but ObjectMeta fields.
-	err = scheme.AddFieldLabelConversionFunc("apps/v1beta2", "StatefulSet",
+	err = scheme.AddFieldLabelConversionFunc(SchemeGroupVersion.WithKind("StatefulSet"),
 		func(label, value string) (string, string, error) {
 			switch label {
 			case "metadata.name", "metadata.namespace", "status.successful":

pkg/apis/batch/v1/conversion.go

@@ -37,7 +37,7 @@ func addConversionFuncs(scheme *runtime.Scheme) error {
 		return err
 	}
 
-	return scheme.AddFieldLabelConversionFunc("batch/v1", "Job",
+	return scheme.AddFieldLabelConversionFunc(SchemeGroupVersion.WithKind("Job"),
 		func(label, value string) (string, string, error) {
 			switch label {
 			case "metadata.name", "metadata.namespace", "status.successful":

pkg/apis/batch/v1beta1/conversion.go

@@ -27,7 +27,7 @@ func addConversionFuncs(scheme *runtime.Scheme) error {
 	// Add field label conversions for kinds having selectable nothing but ObjectMeta fields.
 	for _, k := range []string{"Job", "JobTemplate", "CronJob"} {
 		kind := k // don't close over range variables
-		err = scheme.AddFieldLabelConversionFunc("batch/v1beta1", kind,
+		err = scheme.AddFieldLabelConversionFunc(SchemeGroupVersion.WithKind(kind),
 			func(label, value string) (string, string, error) {
 				switch label {
 				case "metadata.name", "metadata.namespace", "status.successful":

pkg/apis/batch/v2alpha1/conversion.go

@@ -27,7 +27,7 @@ func addConversionFuncs(scheme *runtime.Scheme) error {
 	// Add field label conversions for kinds having selectable nothing but ObjectMeta fields.
 	for _, k := range []string{"Job", "JobTemplate", "CronJob"} {
 		kind := k // don't close over range variables
-		err = scheme.AddFieldLabelConversionFunc("batch/v2alpha1", kind,
+		err = scheme.AddFieldLabelConversionFunc(SchemeGroupVersion.WithKind(kind),
 			func(label, value string) (string, string, error) {
 				switch label {
 				case "metadata.name", "metadata.namespace", "status.successful":

pkg/apis/core/v1/conversion.go

@@ -154,7 +154,7 @@ func addConversionFuncs(scheme *runtime.Scheme) error {
 	}
 
 	// Add field conversion funcs.
-	err = scheme.AddFieldLabelConversionFunc("v1", "Pod",
+	err = scheme.AddFieldLabelConversionFunc(SchemeGroupVersion.WithKind("Pod"),
 		func(label, value string) (string, string, error) {
 			switch label {
 			case "metadata.name",
@@ -177,7 +177,7 @@ func addConversionFuncs(scheme *runtime.Scheme) error {
 	if err != nil {
 		return err
 	}
-	err = scheme.AddFieldLabelConversionFunc("v1", "Node",
+	err = scheme.AddFieldLabelConversionFunc(SchemeGroupVersion.WithKind("Node"),
 		func(label, value string) (string, string, error) {
 			switch label {
 			case "metadata.name":
@@ -192,7 +192,7 @@ func addConversionFuncs(scheme *runtime.Scheme) error {
 	if err != nil {
 		return err
 	}
-	err = scheme.AddFieldLabelConversionFunc("v1", "ReplicationController",
+	err = scheme.AddFieldLabelConversionFunc(SchemeGroupVersion.WithKind("ReplicationController"),
 		func(label, value string) (string, string, error) {
 			switch label {
 			case "metadata.name",
@@ -553,7 +553,7 @@ func Convert_v1_ResourceList_To_core_ResourceList(in *v1.ResourceList, out *core
 }
 
 func AddFieldLabelConversionsForEvent(scheme *runtime.Scheme) error {
-	return scheme.AddFieldLabelConversionFunc("v1", "Event",
+	return scheme.AddFieldLabelConversionFunc(SchemeGroupVersion.WithKind("Event"),
 		func(label, value string) (string, string, error) {
 			switch label {
 			case "involvedObject.kind",
@@ -576,7 +576,7 @@ func AddFieldLabelConversionsForEvent(scheme *runtime.Scheme) error {
 }
 
 func AddFieldLabelConversionsForNamespace(scheme *runtime.Scheme) error {
-	return scheme.AddFieldLabelConversionFunc("v1", "Namespace",
+	return scheme.AddFieldLabelConversionFunc(SchemeGroupVersion.WithKind("Namespace"),
 		func(label, value string) (string, string, error) {
 			switch label {
 			case "status.phase",
@@ -589,7 +589,7 @@ func AddFieldLabelConversionsForNamespace(scheme *runtime.Scheme) error {
 }
 
 func AddFieldLabelConversionsForSecret(scheme *runtime.Scheme) error {
-	return scheme.AddFieldLabelConversionFunc("v1", "Secret",
+	return scheme.AddFieldLabelConversionFunc(SchemeGroupVersion.WithKind("Secret"),
 		func(label, value string) (string, string, error) {
 			switch label {
 			case "type",

pkg/printers/internalversion/describe.go

@@ -1644,7 +1644,12 @@ type EnvVarResolverFunc func(e api.EnvVar) string
 // EnvValueFrom is exported for use by describers in other packages
 func EnvValueRetriever(pod *api.Pod) EnvVarResolverFunc {
 	return func(e api.EnvVar) string {
-		internalFieldPath, _, err := legacyscheme.Scheme.ConvertFieldLabel(e.ValueFrom.FieldRef.APIVersion, "Pod", e.ValueFrom.FieldRef.FieldPath, "")
+		gv, err := schema.ParseGroupVersion(e.ValueFrom.FieldRef.APIVersion)
+		if err != nil {
+			return ""
+		}
+		gvk := gv.WithKind("Pod")
+		internalFieldPath, _, err := legacyscheme.Scheme.ConvertFieldLabel(gvk, e.ValueFrom.FieldRef.FieldPath, "")
 		if err != nil {
 			return "" // pod validation should catch this on create
 		}

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/BUILD

@@ -94,6 +94,7 @@ go_test(
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1/unstructured:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
+        "//staging/src/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime/serializer/json:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library",

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

@@ -52,7 +52,7 @@ type crdConverter struct {
 	clusterScoped bool
 }
 
-func (c *crdConverter) ConvertFieldLabel(version, kind, label, value string) (string, string, error) {
+func (c *crdConverter) ConvertFieldLabel(gvk schema.GroupVersionKind, label, value string) (string, string, error) {
 	// We currently only support metadata.namespace and metadata.name.
 	switch {
 	case label == "metadata.name":
@@ -98,8 +98,8 @@ type safeConverterWrapper struct {
 var _ runtime.ObjectConvertor = &nopConverter{}
 
 // ConvertFieldLabel delegate the call to the unsafe converter.
-func (c *safeConverterWrapper) ConvertFieldLabel(version, kind, label, value string) (string, string, error) {
+func (c *safeConverterWrapper) ConvertFieldLabel(gvk schema.GroupVersionKind, label, value string) (string, string, error) {
-	return c.unsafe.ConvertFieldLabel(version, kind, label, value)
+	return c.unsafe.ConvertFieldLabel(gvk, label, value)
 }
 
 // Convert makes a copy of in object and then delegate the call to the unsafe converter.

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

@@ -32,7 +32,7 @@ type nopConverter struct {
 
 var _ runtime.ObjectConvertor = &nopConverter{}
 
-func (nopConverter) ConvertFieldLabel(version, kind, label, value string) (string, string, error) {
+func (nopConverter) ConvertFieldLabel(gvk schema.GroupVersionKind, label, value string) (string, string, error) {
 	return "", "", errors.New("unstructured cannot convert field labels")
 }
 

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler.go

@@ -786,8 +786,8 @@ func (v schemaCoercingConverter) ConvertToVersion(in runtime.Object, gv runtime.
 	return out, nil
 }
 
-func (v schemaCoercingConverter) ConvertFieldLabel(version, kind, label, value string) (string, string, error) {
+func (v schemaCoercingConverter) ConvertFieldLabel(gvk schema.GroupVersionKind, label, value string) (string, string, error) {
-	return v.ConvertFieldLabel(version, kind, label, value)
+	return v.delegate.ConvertFieldLabel(gvk, label, value)
 }
 
 // unstructuredSchemaCoercer does the validation for Unstructured that json.Unmarshal

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/customresource_handler_test.go

@@ -30,6 +30,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
 	"k8s.io/apimachinery/pkg/runtime/serializer/json"
 	"k8s.io/apimachinery/pkg/util/diff"
@@ -87,7 +88,7 @@ func TestConvertFieldLabel(t *testing.T) {
 			}
 			_, c := conversion.NewCRDConverter(&crd)
 
-			label, value, err := c.ConvertFieldLabel("", "", test.label, "value")
+			label, value, err := c.ConvertFieldLabel(schema.GroupVersionKind{}, test.label, "value")
 			if e, a := test.expectError, err != nil; e != a {
 				t.Fatalf("err: expected %t, got %t", e, a)
 			}

staging/src/k8s.io/apimachinery/pkg/runtime/interfaces.go

@@ -185,7 +185,7 @@ type ObjectConvertor interface {
 	// This method is similar to Convert() but handles specific details of choosing the correct
 	// output version.
 	ConvertToVersion(in Object, gv GroupVersioner) (out Object, err error)
-	ConvertFieldLabel(version, kind, label, value string) (string, string, error)
+	ConvertFieldLabel(gvk schema.GroupVersionKind, label, value string) (string, string, error)
 }
 
 // ObjectTyper contains methods for extracting the APIVersion and Kind

staging/src/k8s.io/apimachinery/pkg/runtime/scheme.go

@@ -63,7 +63,7 @@ type Scheme struct {
 
 	// Map from version and resource to the corresponding func to convert
 	// resource field labels in that version to internal version.
-	fieldLabelConversionFuncs map[string]map[string]FieldLabelConversionFunc
+	fieldLabelConversionFuncs map[schema.GroupVersionKind]FieldLabelConversionFunc
 
 	// defaulterFuncs is an array of interfaces to be called with an object to provide defaulting
 	// the provided object must be a pointer.
@@ -95,7 +95,7 @@ func NewScheme() *Scheme {
 		typeToGVK:                 map[reflect.Type][]schema.GroupVersionKind{},
 		unversionedTypes:          map[reflect.Type]schema.GroupVersionKind{},
 		unversionedKinds:          map[string]reflect.Type{},
-		fieldLabelConversionFuncs: map[string]map[string]FieldLabelConversionFunc{},
+		fieldLabelConversionFuncs: map[schema.GroupVersionKind]FieldLabelConversionFunc{},
 		defaulterFuncs:            map[reflect.Type]func(interface{}){},
 		versionPriority:           map[string][]string{},
 		schemeName:                naming.GetNameFromCallsite(internalPackages...),
@@ -368,12 +368,8 @@ func (s *Scheme) AddGeneratedConversionFuncs(conversionFuncs ...interface{}) err
 
 // AddFieldLabelConversionFunc adds a conversion function to convert field selectors
 // of the given kind from the given version to internal version representation.
-func (s *Scheme) AddFieldLabelConversionFunc(version, kind string, conversionFunc FieldLabelConversionFunc) error {
+func (s *Scheme) AddFieldLabelConversionFunc(gvk schema.GroupVersionKind, conversionFunc FieldLabelConversionFunc) error {
-	if s.fieldLabelConversionFuncs[version] == nil {
+	s.fieldLabelConversionFuncs[gvk] = conversionFunc
-		s.fieldLabelConversionFuncs[version] = map[string]FieldLabelConversionFunc{}
-	}
-
-	s.fieldLabelConversionFuncs[version][kind] = conversionFunc
 	return nil
 }
 
@@ -486,11 +482,8 @@ func (s *Scheme) Convert(in, out interface{}, context interface{}) error {
 
 // ConvertFieldLabel alters the given field label and value for an kind field selector from
 // versioned representation to an unversioned one or returns an error.
-func (s *Scheme) ConvertFieldLabel(version, kind, label, value string) (string, string, error) {
+func (s *Scheme) ConvertFieldLabel(gvk schema.GroupVersionKind, label, value string) (string, string, error) {
-	if s.fieldLabelConversionFuncs[version] == nil {
+	conversionFunc, ok := s.fieldLabelConversionFuncs[gvk]
-		return DefaultMetaV1FieldSelectorConversion(label, value)
-	}
-	conversionFunc, ok := s.fieldLabelConversionFuncs[version][kind]
 	if !ok {
 		return DefaultMetaV1FieldSelectorConversion(label, value)
 	}

staging/src/k8s.io/apimachinery/pkg/runtime/serializer/versioning/versioning_test.go

@@ -307,7 +307,7 @@ func (c *checkConvertor) ConvertToVersion(in runtime.Object, outVersion runtime.
 	}
 	return c.obj, c.err
 }
-func (c *checkConvertor) ConvertFieldLabel(version, kind, label, value string) (string, string, error) {
+func (c *checkConvertor) ConvertFieldLabel(gvk schema.GroupVersionKind, label, value string) (string, string, error) {
 	return "", "", fmt.Errorf("unexpected call to ConvertFieldLabel")
 }
 

staging/src/k8s.io/apiserver/pkg/endpoints/apiserver_test.go

@@ -176,17 +176,17 @@ func init() {
 	addTestTypes()
 	addNewTestTypes()
 
-	scheme.AddFieldLabelConversionFunc(grouplessGroupVersion.String(), "Simple",
+	scheme.AddFieldLabelConversionFunc(grouplessGroupVersion.WithKind("Simple"),
 		func(label, value string) (string, string, error) {
 			return label, value, nil
 		},
 	)
-	scheme.AddFieldLabelConversionFunc(testGroupVersion.String(), "Simple",
+	scheme.AddFieldLabelConversionFunc(testGroupVersion.WithKind("Simple"),
 		func(label, value string) (string, string, error) {
 			return label, value, nil
 		},
 	)
-	scheme.AddFieldLabelConversionFunc(newGroupVersion.String(), "Simple",
+	scheme.AddFieldLabelConversionFunc(newGroupVersion.WithKind("Simple"),
 		func(label, value string) (string, string, error) {
 			return label, value, nil
 		},

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/delete.go

@@ -224,7 +224,7 @@ func DeleteCollection(r rest.CollectionDeleter, checkBody bool, scope RequestSco
 		// TODO: DecodeParametersInto should do this.
 		if listOptions.FieldSelector != nil {
 			fn := func(label, value string) (newLabel, newValue string, err error) {
-				return scope.Convertor.ConvertFieldLabel(scope.Kind.GroupVersion().String(), scope.Kind.Kind, label, value)
+				return scope.Convertor.ConvertFieldLabel(scope.Kind, label, value)
 			}
 			if listOptions.FieldSelector, err = listOptions.FieldSelector.Transform(fn); err != nil {
 				// TODO: allow bad request to set field causes based on query parameters

staging/src/k8s.io/apiserver/pkg/endpoints/handlers/get.go

@@ -196,7 +196,7 @@ func ListResource(r rest.Lister, rw rest.Watcher, scope RequestScope, forceWatch
 		// TODO: DecodeParametersInto should do this.
 		if opts.FieldSelector != nil {
 			fn := func(label, value string) (newLabel, newValue string, err error) {
-				return scope.Convertor.ConvertFieldLabel(scope.Kind.GroupVersion().String(), scope.Kind.Kind, label, value)
+				return scope.Convertor.ConvertFieldLabel(scope.Kind, label, value)
 			}
 			if opts.FieldSelector, err = opts.FieldSelector.Transform(fn); err != nil {
 				// TODO: allow bad request to set field causes based on query parameters