Merge pull request kubernetes#17796 from deads2k/gv-object-typer

Auto commit by PR queue bot
nikhiljindal · Dec 7, 2015 · ec1ba74 · ec1ba74
2 parents dc4bc2a + f764e00
commit ec1ba74
Show file tree

Hide file tree

Showing 31 changed files with 338 additions and 273 deletions.
diff --git a/pkg/api/ref.go b/pkg/api/ref.go
@@ -53,10 +53,11 @@ func GetReference(obj runtime.Object) (*ObjectReference, error) {
 	// if we are building an object reference to something not yet persisted, we should fallback to scheme
 	kind := meta.Kind()
 	if kind == "" {
-		_, kind, err = Scheme.ObjectVersionAndKind(obj)
+		gvk, err := Scheme.ObjectKind(obj)
 		if err != nil {
 			return nil, err
 		}
+		kind = gvk.Kind
 	}
 
 	// if the object referenced is actually persisted, we can also get version from meta

diff --git a/pkg/api/rest/create.go b/pkg/api/rest/create.go
@@ -111,9 +111,9 @@ func objectMetaAndKind(typer runtime.ObjectTyper, obj runtime.Object) (*api.Obje
 	if err != nil {
 		return nil, "", errors.NewInternalError(err)
 	}
-	_, kind, err := typer.ObjectVersionAndKind(obj)
+	gvk, err := typer.ObjectKind(obj)
 	if err != nil {
 		return nil, "", errors.NewInternalError(err)
 	}
-	return objectMeta, kind, nil
+	return objectMeta, gvk.Kind, nil
 }
diff --git a/pkg/api/serialization_test.go b/pkg/api/serialization_test.go
@@ -63,6 +63,9 @@ func fuzzInternalObject(t *testing.T, forVersion string, item runtime.Object, se
 func roundTrip(t *testing.T, codec runtime.Codec, item runtime.Object) {
 	printer := spew.ConfigState{DisableMethods: true}
 
+	gvk, err := api.Scheme.ObjectKind(item)
+	t.Logf("fully qualified kind for %v is %v with codec %v", reflect.TypeOf(item), gvk, codec)
+
 	name := reflect.TypeOf(item).Elem().Name()
 	data, err := codec.Encode(item)
 	if err != nil {
@@ -96,7 +99,7 @@ func roundTrip(t *testing.T, codec runtime.Codec, item runtime.Object) {
 func roundTripSame(t *testing.T, item runtime.Object, except ...string) {
 	set := sets.NewString(except...)
 	seed := rand.Int63()
-	fuzzInternalObject(t, "", item, seed)
+	fuzzInternalObject(t, testapi.Default.InternalGroupVersion().String(), item, seed)
 
 	version := testapi.Default.VersionUnderTest
 	codecs := []runtime.Codec{}
@@ -154,6 +157,7 @@ func TestRoundTripTypes(t *testing.T) {
 	// defer api.Scheme.Log(nil)
 
 	for kind := range api.Scheme.KnownTypes(testapi.Default.InternalGroupVersion()) {
+		t.Logf("working on %v in %v", kind, testapi.Default.InternalGroupVersion())
 		if nonRoundTrippableTypes.Has(kind) {
 			continue
 		}
@@ -168,18 +172,18 @@ func TestRoundTripTypes(t *testing.T) {
 }
 
 func doRoundTripTest(kind string, t *testing.T) {
-	item, err := api.Scheme.New("", kind)
+	item, err := api.Scheme.New(testapi.Default.InternalGroupVersion().String(), kind)
 	if err != nil {
 		t.Fatalf("Couldn't make a %v? %v", kind, err)
 	}
 	if _, err := meta.TypeAccessor(item); err != nil {
 		t.Fatalf("%q is not a TypeMeta and cannot be tested - add it to nonRoundTrippableTypes: %v", kind, err)
 	}
-	if api.Scheme.Recognizes(testapi.Default.VersionUnderTest, kind) {
+	if api.Scheme.Recognizes(testapi.Default.GroupVersion().WithKind(kind)) {
 		roundTripSame(t, item, nonRoundTrippableTypesByVersion[kind]...)
 	}
 	if !nonInternalRoundTrippableTypes.Has(kind) {
-		roundTrip(t, api.Codec, fuzzInternalObject(t, "", item, rand.Int63()))
+		roundTrip(t, api.Codec, fuzzInternalObject(t, testapi.Default.InternalGroupVersion().String(), item, rand.Int63()))
 	}
 }
 

diff --git a/pkg/api/testapi/testapi.go b/pkg/api/testapi/testapi.go
@@ -214,22 +214,23 @@ func (g TestGroup) RESTMapper() meta.RESTMapper {
 
 // Get codec based on runtime.Object
 func GetCodecForObject(obj runtime.Object) (runtime.Codec, error) {
-	_, kind, err := api.Scheme.ObjectVersionAndKind(obj)
+	gvk, err := api.Scheme.ObjectKind(obj)
 	if err != nil {
 		return nil, fmt.Errorf("unexpected encoding error: %v", err)
 	}
-	// TODO: caesarxuchao: we should detect which group an object belongs to
-	// by using the version returned by Schem.ObjectVersionAndKind() once we
-	// split the schemes for internal objects.
-	// TODO: caesarxuchao: we should add a map from kind to group in Scheme.
+
 	for _, group := range Groups {
-		if api.Scheme.Recognizes(group.GroupAndVersion(), kind) {
+		if group.GroupVersion().Group != gvk.Group {
+			continue
+		}
+
+		if api.Scheme.Recognizes(gvk) {
 			return group.Codec(), nil
 		}
 	}
 	// Codec used for unversioned types
-	if api.Scheme.Recognizes("", kind) {
+	if api.Scheme.Recognizes(gvk) {
 		return api.Codec, nil
 	}
-	return nil, fmt.Errorf("unexpected kind: %v", kind)
+	return nil, fmt.Errorf("unexpected kind: %v", gvk)
 }
diff --git a/pkg/api/testing/fuzzer.go b/pkg/api/testing/fuzzer.go
@@ -175,8 +175,10 @@ func FuzzerFor(t *testing.T, version string, src rand.Source) *fuzz.Fuzzer {
 			// TODO: uncomment when round trip starts from a versioned object
 			if true { //c.RandBool() {
 				*j = &runtime.Unknown{
-					TypeMeta: runtime.TypeMeta{Kind: "Something", APIVersion: "unknown"},
-					RawJSON:  []byte(`{"apiVersion":"unknown","kind":"Something","someKey":"someValue"}`),
+					// apiVersion has rules now.  Since it includes <group>/<version> and only `v1` can be bare,
+					// then this must choose a valid format to deserialize
+					TypeMeta: runtime.TypeMeta{Kind: "Something", APIVersion: "unknown.group/unknown"},
+					RawJSON:  []byte(`{"apiVersion":"unknown.group/unknown","kind":"Something","someKey":"someValue"}`),
 				}
 			} else {
 				types := []runtime.Object{&api.Pod{}, &api.ReplicationController{}}

diff --git a/pkg/api/unversioned/group_version.go b/pkg/api/unversioned/group_version.go
@@ -93,7 +93,7 @@ func (gvk GroupVersionKind) GroupVersion() GroupVersion {
 	return GroupVersion{Group: gvk.Group, Version: gvk.Version}
 }
 
-func (gvk *GroupVersionKind) String() string {
+func (gvk GroupVersionKind) String() string {
 	return gvk.Group + "/" + gvk.Version + ", Kind=" + gvk.Kind
 }
 

diff --git a/pkg/apiserver/api_installer.go b/pkg/apiserver/api_installer.go
@@ -122,19 +122,39 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 	hasSubresource := len(subresource) > 0
 
 	object := storage.New()
-	_, kind, err := a.group.Typer.ObjectVersionAndKind(object)
+	fqKinds, err := a.group.Typer.ObjectKinds(object)
 	if err != nil {
 		return nil, err
 	}
-	gvk := a.group.GroupVersion.WithKind(kind)
+	// a given go type can have multiple potential fully qualified kinds.  Find the one that corresponds with the group
+	// we're trying to register here
+	fqKindToRegister := unversioned.GroupVersionKind{}
+	for _, fqKind := range fqKinds {
+		if fqKind.Group == a.group.GroupVersion.Group {
+			fqKindToRegister = fqKind
+			break
+		}
+
+		// TODO This keeps it doing what it was doing before, but it doesn't feel right.
+		if fqKind.Group == "extensions" && fqKind.Kind == "ThirdPartyResourceData" {
+			fqKindToRegister = fqKind
+			fqKindToRegister.Group = a.group.GroupVersion.Group
+			fqKindToRegister.Version = a.group.GroupVersion.Version
+		}
+	}
+
+	if fqKindToRegister.IsEmpty() {
+		return nil, fmt.Errorf("unable to locate fully qualified kind for %v: found %v when registering for %v", reflect.TypeOf(object), fqKinds, a.group.GroupVersion)
+	}
+	kind := fqKindToRegister.Kind
 
 	versionedPtr, err := a.group.Creater.New(a.group.GroupVersion.String(), kind)
 	if err != nil {
 		return nil, err
 	}
 	versionedObject := indirectArbitraryPointer(versionedPtr)
 
-	mapping, err := a.group.Mapper.RESTMapping(gvk.GroupKind(), gvk.Version)
+	mapping, err := a.group.Mapper.RESTMapping(fqKindToRegister.GroupKind(), a.group.GroupVersion.Version)
 	if err != nil {
 		return nil, err
 	}
@@ -146,13 +166,25 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 			return nil, fmt.Errorf("subresources can only be declared when the parent is also registered: %s needs %s", path, resource)
 		}
 		parentObject := parentStorage.New()
-		_, parentKind, err := a.group.Typer.ObjectVersionAndKind(parentObject)
+
+		parentFQKinds, err := a.group.Typer.ObjectKinds(parentObject)
 		if err != nil {
 			return nil, err
 		}
-		parentGVK := a.group.GroupVersion.WithKind(parentKind)
+		// a given go type can have multiple potential fully qualified kinds.  Find the one that corresponds with the group
+		// we're trying to register here
+		parentFQKindToRegister := unversioned.GroupVersionKind{}
+		for _, fqKind := range parentFQKinds {
+			if fqKind.Group == a.group.GroupVersion.Group {
+				parentFQKindToRegister = fqKind
+				break
+			}
+		}
+		if parentFQKindToRegister.IsEmpty() {
+			return nil, fmt.Errorf("unable to locate fully qualified kind for %v: found %v when registering for %v", reflect.TypeOf(object), fqKinds, a.group.GroupVersion)
+		}
 
-		parentMapping, err := a.group.Mapper.RESTMapping(parentGVK.GroupKind(), parentGVK.Version)
+		parentMapping, err := a.group.Mapper.RESTMapping(parentFQKindToRegister.GroupKind(), a.group.GroupVersion.Version)
 		if err != nil {
 			return nil, err
 		}
@@ -184,8 +216,8 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 	var versionedList interface{}
 	if isLister {
 		list := lister.NewList()
-		_, listKind, err := a.group.Typer.ObjectVersionAndKind(list)
-		versionedListPtr, err := a.group.Creater.New(a.group.GroupVersion.String(), listKind)
+		listGVK, err := a.group.Typer.ObjectKind(list)
+		versionedListPtr, err := a.group.Creater.New(a.group.GroupVersion.String(), listGVK.Kind)
 		if err != nil {
 			return nil, err
 		}
@@ -225,19 +257,14 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 	)
 	if isGetterWithOptions {
 		getOptions, getSubpath, getSubpathKey = getterWithOptions.NewGetOptions()
-		getOptionsGVString, getOptionsKind, err := a.group.Typer.ObjectVersionAndKind(getOptions)
-		if err != nil {
-			return nil, err
-		}
-		gv, err := unversioned.ParseGroupVersion(getOptionsGVString)
+		getOptionsInternalKind, err = a.group.Typer.ObjectKind(getOptions)
 		if err != nil {
 			return nil, err
 		}
-		getOptionsInternalKind = gv.WithKind(getOptionsKind)
 		// TODO this should be a list of all the different external versions we can coerce into the internalKind
-		getOptionsExternalKind = serverGroupVersion.WithKind(getOptionsKind)
+		getOptionsExternalKind = serverGroupVersion.WithKind(getOptionsInternalKind.Kind)
 
-		versionedGetOptions, err = a.group.Creater.New(serverGroupVersion.String(), getOptionsKind)
+		versionedGetOptions, err = a.group.Creater.New(serverGroupVersion.String(), getOptionsInternalKind.Kind)
 		if err != nil {
 			return nil, err
 		}
@@ -255,19 +282,14 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
 	if isConnecter {
 		connectOptions, connectSubpath, connectSubpathKey = connecter.NewConnectOptions()
 		if connectOptions != nil {
-			connectOptionsGVString, connectOptionsKind, err := a.group.Typer.ObjectVersionAndKind(connectOptions)
-			if err != nil {
-				return nil, err
-			}
-			gv, err := unversioned.ParseGroupVersion(connectOptionsGVString)
+			connectOptionsInternalKind, err = a.group.Typer.ObjectKind(connectOptions)
 			if err != nil {
 				return nil, err
 			}
-			connectOptionsInternalKind = gv.WithKind(connectOptionsKind)
 			// TODO this should be a list of all the different external versions we can coerce into the internalKind
-			connectOptionsExternalKind = serverGroupVersion.WithKind(connectOptionsKind)
+			connectOptionsExternalKind = serverGroupVersion.WithKind(connectOptionsInternalKind.Kind)
 
-			versionedConnectOptions, err = a.group.Creater.New(serverGroupVersion.String(), connectOptionsKind)
+			versionedConnectOptions, err = a.group.Creater.New(serverGroupVersion.String(), connectOptionsInternalKind.Kind)
 		}
 	}
 

diff --git a/pkg/apiserver/resthandler.go b/pkg/apiserver/resthandler.go
@@ -745,14 +745,14 @@ func finishRequest(timeout time.Duration, fn resultFunc) (result runtime.Object,
 
 // transformDecodeError adds additional information when a decode fails.
 func transformDecodeError(typer runtime.ObjectTyper, baseErr error, into runtime.Object, body []byte) error {
-	_, kind, err := typer.ObjectVersionAndKind(into)
+	objectGroupVersionKind, err := typer.ObjectKind(into)
 	if err != nil {
 		return err
 	}
-	if version, dataKind, err := typer.DataVersionAndKind(body); err == nil && len(dataKind) > 0 {
-		return errors.NewBadRequest(fmt.Sprintf("%s in version %s cannot be handled as a %s: %v", dataKind, version, kind, baseErr))
+	if dataGroupVersionKind, err := typer.DataKind(body); err == nil && len(dataGroupVersionKind.Kind) > 0 {
+		return errors.NewBadRequest(fmt.Sprintf("%s in version %v cannot be handled as a %s: %v", dataGroupVersionKind.Kind, dataGroupVersionKind.GroupVersion(), objectGroupVersionKind.Kind, baseErr))
 	}
-	return errors.NewBadRequest(fmt.Sprintf("the object provided is unrecognized (must be of type %s): %v", kind, baseErr))
+	return errors.NewBadRequest(fmt.Sprintf("the object provided is unrecognized (must be of type %s): %v", objectGroupVersionKind.Kind, baseErr))
 }
 
 // setSelfLink sets the self link of an object (or the child items in a list) to the base URL of the request

diff --git a/pkg/auth/authorizer/abac/abac.go b/pkg/auth/authorizer/abac/abac.go
@@ -77,12 +77,12 @@ func NewFromFile(path string) (policyList, error) {
 			continue
 		}
 
-		version, kind, err := api.Scheme.DataVersionAndKind(b)
+		dataKind, err := api.Scheme.DataKind(b)
 		if err != nil {
 			return nil, policyLoadError{path, i, b, err}
 		}
 
-		if version == "" && kind == "" {
+		if dataKind.IsEmpty() {
 			unversionedLines++
 			// Migrate unversioned policy object
 			oldPolicy := &v0.Policy{}

diff --git a/pkg/client/unversioned/client_test.go b/pkg/client/unversioned/client_test.go
@@ -208,7 +208,7 @@ func validateFields(a, b string) bool {
 
 func body(t *testing.T, obj runtime.Object, raw *string) *string {
 	if obj != nil {
-		_, kind, err := api.Scheme.ObjectVersionAndKind(obj)
+		fqKind, err := api.Scheme.ObjectKind(obj)
 		if err != nil {
 			t.Errorf("unexpected encoding error: %v", err)
 		}
@@ -217,18 +217,18 @@ func body(t *testing.T, obj runtime.Object, raw *string) *string {
 		// split the schemes for internal objects.
 		// TODO: caesarxuchao: we should add a map from kind to group in Scheme.
 		var bs []byte
-		if api.Scheme.Recognizes(testapi.Default.GroupAndVersion(), kind) {
+		if api.Scheme.Recognizes(testapi.Default.GroupVersion().WithKind(fqKind.Kind)) {
 			bs, err = testapi.Default.Codec().Encode(obj)
 			if err != nil {
 				t.Errorf("unexpected encoding error: %v", err)
 			}
-		} else if api.Scheme.Recognizes(testapi.Extensions.GroupAndVersion(), kind) {
+		} else if api.Scheme.Recognizes(testapi.Extensions.GroupVersion().WithKind(fqKind.Kind)) {
 			bs, err = testapi.Extensions.Codec().Encode(obj)
 			if err != nil {
 				t.Errorf("unexpected encoding error: %v", err)
 			}
 		} else {
-			t.Errorf("unexpected kind: %v", kind)
+			t.Errorf("unexpected kind: %v", fqKind.Kind)
 		}
 		body := string(bs)
 		return &body

diff --git a/pkg/client/unversioned/testclient/fixture.go b/pkg/client/unversioned/testclient/fixture.go
@@ -210,10 +210,11 @@ func (o objects) Kind(gvk unversioned.GroupVersionKind, name string) (runtime.Ob
 }
 
 func (o objects) Add(obj runtime.Object) error {
-	_, kind, err := o.scheme.ObjectVersionAndKind(obj)
+	gvk, err := o.scheme.ObjectKind(obj)
 	if err != nil {
 		return err
 	}
+	kind := gvk.Kind
 
 	switch {
 	case meta.IsListType(obj):