Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update ObjectTyper for GroupVersion #17796

Merged
merged 1 commit into from
Dec 7, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion pkg/api/ref.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions pkg/api/rest/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
12 changes: 8 additions & 4 deletions pkg/api/serialization_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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{}
Expand Down Expand Up @@ -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
}
Expand All @@ -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()))
}
}

Expand Down
17 changes: 9 additions & 8 deletions pkg/api/testapi/testapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
6 changes: 4 additions & 2 deletions pkg/api/testing/fuzzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}}
Expand Down
2 changes: 1 addition & 1 deletion pkg/api/unversioned/group_version.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,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
}

Expand Down
70 changes: 46 additions & 24 deletions pkg/apiserver/api_installer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

break or no?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

break or no?

I think I want an actual match to override this "match". That way you can specify exactly what you mean.

}
}

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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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)
}
}

Expand Down
8 changes: 4 additions & 4 deletions pkg/apiserver/resthandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions pkg/auth/authorizer/abac/abac.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down
8 changes: 4 additions & 4 deletions pkg/client/unversioned/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,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)
}
Expand All @@ -216,18 +216,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
Expand Down
3 changes: 2 additions & 1 deletion pkg/client/unversioned/testclient/fixture.go
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
Loading