From 871bee8991d00ed689d0e3828a330b9cdaadf541 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Tue, 5 Jul 2016 18:35:11 -0400 Subject: [PATCH] Remove reflection path in meta.Accessor Callers are required to implement their interfaces, removes the potential for mistakes. We have a reflective test pkg/api/meta_test.go#TestAccessorImplementations that verifies that all objects registered to the scheme properly implement their interfaces. --- pkg/api/meta/meta.go | 189 ++++--------- pkg/api/meta/meta_test.go | 483 +-------------------------------- pkg/api/ref.go | 35 ++- pkg/quota/generic/evaluator.go | 4 +- 4 files changed, 89 insertions(+), 622 deletions(-) diff --git a/pkg/api/meta/meta.go b/pkg/api/meta/meta.go index df1da6f472..876aa4faaf 100644 --- a/pkg/api/meta/meta.go +++ b/pkg/api/meta/meta.go @@ -29,104 +29,66 @@ import ( "github.com/golang/glog" ) +// errNotList is returned when an object implements the Object style interfaces but not the List style +// interfaces. +var errNotList = fmt.Errorf("object does not implement the List interfaces") + +// ListAccessor returns a List interface for the provided object or an error if the object does +// not provide List. +// IMPORTANT: Objects are a superset of lists, so all Objects return List metadata. Do not use this +// check to determine whether an object *is* a List. +// TODO: return bool instead of error func ListAccessor(obj interface{}) (List, error) { - if listMetaAccessor, ok := obj.(ListMetaAccessor); ok { - if om := listMetaAccessor.GetListMeta(); om != nil { - return om, nil + switch t := obj.(type) { + case List: + return t, nil + case unversioned.List: + return t, nil + case ListMetaAccessor: + if m := t.GetListMeta(); m != nil { + return m, nil } - } - if listMetaAccessor, ok := obj.(unversioned.ListMetaAccessor); ok { - if om := listMetaAccessor.GetListMeta(); om != nil { - return om, nil + return nil, errNotList + case unversioned.ListMetaAccessor: + if m := t.GetListMeta(); m != nil { + return m, nil } - } - // we may get passed an object that is directly portable to List - if list, ok := obj.(List); ok { - return list, nil - } - glog.V(4).Infof("Calling ListAccessor on non-internal object: %v", reflect.TypeOf(obj)) - // legacy path for objects that do not implement List and ListMetaAccessor via - // reflection - very slow code path. - v, err := conversion.EnforcePtr(obj) - if err != nil { - return nil, err - } - t := v.Type() - if v.Kind() != reflect.Struct { - return nil, fmt.Errorf("expected struct, but got %v: %v (%#v)", v.Kind(), t, v.Interface()) - } - a := &genericAccessor{} - listMeta := v.FieldByName("ListMeta") - if listMeta.IsValid() { - // look for the ListMeta fields - if err := extractFromListMeta(listMeta, a); err != nil { - return nil, fmt.Errorf("unable to find list fields on %#v: %v", listMeta, err) + return nil, errNotList + case Object: + return t, nil + case ObjectMetaAccessor: + if m := t.GetObjectMeta(); m != nil { + return m, nil } - } else { - return nil, fmt.Errorf("unable to find listMeta on %#v", v) + return nil, errNotList + default: + return nil, errNotList } - return a, nil } +// errNotObject is returned when an object implements the List style interfaces but not the Object style +// interfaces. +var errNotObject = fmt.Errorf("object does not implement the Object interfaces") + // Accessor takes an arbitrary object pointer and returns meta.Interface. // obj must be a pointer to an API type. An error is returned if the minimum // required fields are missing. Fields that are not required return the default // value and are a no-op if set. +// TODO: return bool instead of error func Accessor(obj interface{}) (Object, error) { - if objectMetaAccessor, ok := obj.(ObjectMetaAccessor); ok { - if om := objectMetaAccessor.GetObjectMeta(); om != nil { - return om, nil + switch t := obj.(type) { + case Object: + return t, nil + case ObjectMetaAccessor: + if m := t.GetObjectMeta(); m != nil { + return m, nil } + return nil, errNotObject + case List, unversioned.List, ListMetaAccessor, unversioned.ListMetaAccessor: + return nil, errNotObject + default: + return nil, errNotObject } - // we may get passed an object that is directly portable to Object - if object, ok := obj.(Object); ok { - return object, nil - } - - glog.V(4).Infof("Calling Accessor on non-internal object: %v", reflect.TypeOf(obj)) - // legacy path for objects that do not implement Object and ObjectMetaAccessor via - // reflection - very slow code path. - v, err := conversion.EnforcePtr(obj) - if err != nil { - return nil, err - } - t := v.Type() - if v.Kind() != reflect.Struct { - return nil, fmt.Errorf("expected struct, but got %v: %v (%#v)", v.Kind(), t, v.Interface()) - } - - typeMeta := v.FieldByName("TypeMeta") - if !typeMeta.IsValid() { - return nil, fmt.Errorf("struct %v lacks embedded TypeMeta type", t) - } - - a := &genericAccessor{} - if err := extractFromTypeMeta(typeMeta, a); err != nil { - return nil, fmt.Errorf("unable to find type fields on %#v: %v", typeMeta, err) - } - - objectMeta := v.FieldByName("ObjectMeta") - if objectMeta.IsValid() { - // look for the ObjectMeta fields - if err := extractFromObjectMeta(objectMeta, a); err != nil { - return nil, fmt.Errorf("unable to find object fields on %#v: %v", objectMeta, err) - } - } else { - listMeta := v.FieldByName("ListMeta") - if listMeta.IsValid() { - // look for the ListMeta fields - if err := extractFromListMeta(listMeta, a); err != nil { - return nil, fmt.Errorf("unable to find list fields on %#v: %v", listMeta, err) - } - } else { - // look for the older TypeMeta with all metadata - if err := extractFromObjectMeta(typeMeta, a); err != nil { - return nil, fmt.Errorf("unable to find object fields on %#v: %v", typeMeta, err) - } - } - } - - return a, nil } // TypeAccessor returns an interface that allows retrieving and modifying the APIVersion @@ -283,7 +245,7 @@ func (resourceAccessor) SetUID(obj runtime.Object, uid types.UID) error { } func (resourceAccessor) SelfLink(obj runtime.Object) (string, error) { - accessor, err := Accessor(obj) + accessor, err := ListAccessor(obj) if err != nil { return "", err } @@ -291,7 +253,7 @@ func (resourceAccessor) SelfLink(obj runtime.Object) (string, error) { } func (resourceAccessor) SetSelfLink(obj runtime.Object, selfLink string) error { - accessor, err := Accessor(obj) + accessor, err := ListAccessor(obj) if err != nil { return err } @@ -334,7 +296,7 @@ func (resourceAccessor) SetAnnotations(obj runtime.Object, annotations map[strin } func (resourceAccessor) ResourceVersion(obj runtime.Object) (string, error) { - accessor, err := Accessor(obj) + accessor, err := ListAccessor(obj) if err != nil { return "", err } @@ -342,7 +304,7 @@ func (resourceAccessor) ResourceVersion(obj runtime.Object) (string, error) { } func (resourceAccessor) SetResourceVersion(obj runtime.Object, version string) error { - accessor, err := Accessor(obj) + accessor, err := ListAccessor(obj) if err != nil { return err } @@ -603,54 +565,3 @@ func extractFromTypeMeta(v reflect.Value, a *genericAccessor) error { } return nil } - -// extractFromObjectMeta extracts pointers to metadata fields from an object -func extractFromObjectMeta(v reflect.Value, a *genericAccessor) error { - if err := runtime.FieldPtr(v, "Namespace", &a.namespace); err != nil { - return err - } - if err := runtime.FieldPtr(v, "Name", &a.name); err != nil { - return err - } - if err := runtime.FieldPtr(v, "GenerateName", &a.generateName); err != nil { - return err - } - if err := runtime.FieldPtr(v, "UID", &a.uid); err != nil { - return err - } - if err := runtime.FieldPtr(v, "ResourceVersion", &a.resourceVersion); err != nil { - return err - } - if err := runtime.FieldPtr(v, "SelfLink", &a.selfLink); err != nil { - return err - } - if err := runtime.FieldPtr(v, "Labels", &a.labels); err != nil { - return err - } - if err := runtime.FieldPtr(v, "Annotations", &a.annotations); err != nil { - return err - } - if err := runtime.FieldPtr(v, "Finalizers", &a.finalizers); err != nil { - return err - } - ownerReferences := v.FieldByName("OwnerReferences") - if !ownerReferences.IsValid() { - return fmt.Errorf("struct %#v lacks OwnerReferences type", v) - } - if ownerReferences.Kind() != reflect.Slice { - return fmt.Errorf("expect %v to be a slice", ownerReferences.Kind()) - } - a.ownerReferences = ownerReferences.Addr() - return nil -} - -// extractFromObjectMeta extracts pointers to metadata fields from a list object -func extractFromListMeta(v reflect.Value, a *genericAccessor) error { - if err := runtime.FieldPtr(v, "ResourceVersion", &a.resourceVersion); err != nil { - return err - } - if err := runtime.FieldPtr(v, "SelfLink", &a.selfLink); err != nil { - return err - } - return nil -} diff --git a/pkg/api/meta/meta_test.go b/pkg/api/meta/meta_test.go index e1437d2922..ecd2a00779 100644 --- a/pkg/api/meta/meta_test.go +++ b/pkg/api/meta/meta_test.go @@ -55,8 +55,8 @@ func TestAPIObjectMeta(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if accessor != meta.Object(&j.ObjectMeta) { - t.Fatalf("should have returned the same pointer: %#v %#v", accessor, j) + if accessor != meta.Object(j) { + t.Fatalf("should have returned the same pointer: %#v\n\n%#v", accessor, j) } if e, a := "bar", accessor.GetNamespace(); e != a { t.Errorf("expected %v, got %v", e, a) @@ -156,49 +156,8 @@ func TestGenericTypeMeta(t *testing.T) { OwnerReferences []api.OwnerReference `json:"ownerReferences,omitempty"` Finalizers []string `json:"finalizers,omitempty"` } - type Object struct { - TypeMeta `json:",inline"` - } - j := Object{ - TypeMeta{ - Namespace: "bar", - Name: "foo", - GenerateName: "prefix", - UID: "uid", - APIVersion: "a", - Kind: "b", - ResourceVersion: "1", - SelfLink: "some/place/only/we/know", - Labels: map[string]string{"foo": "bar"}, - Annotations: map[string]string{"x": "y"}, - Finalizers: []string{"finalizer.1", "finalizer.2"}, - }, - } - accessor, err := meta.Accessor(&j) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if e, a := "bar", accessor.GetNamespace(); e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "foo", accessor.GetName(); e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "prefix", accessor.GetGenerateName(); e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "uid", string(accessor.GetUID()); e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "1", accessor.GetResourceVersion(); e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "some/place/only/we/know", accessor.GetSelfLink(); e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := []string{"finalizer.1", "finalizer.2"}, accessor.GetFinalizers(); !reflect.DeepEqual(e, a) { - t.Errorf("expected %v, got %v", e, a) - } + + j := struct{ TypeMeta }{TypeMeta{APIVersion: "a", Kind: "b"}} typeAccessor, err := meta.TypeAccessor(&j) if err != nil { @@ -211,47 +170,19 @@ func TestGenericTypeMeta(t *testing.T) { t.Errorf("expected %v, got %v", e, a) } - accessor.SetNamespace("baz") - accessor.SetName("bar") - accessor.SetGenerateName("generate") - accessor.SetUID("other") typeAccessor.SetAPIVersion("c") typeAccessor.SetKind("d") - accessor.SetResourceVersion("2") - accessor.SetSelfLink("google.com") - accessor.SetFinalizers([]string{"finalizer.3"}) - // Prove that accessor changes the original object. - if e, a := "baz", j.Namespace; e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "bar", j.Name; e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "generate", j.GenerateName; e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "other", j.UID; e != a { - t.Errorf("expected %v, got %v", e, a) - } if e, a := "c", j.APIVersion; e != a { t.Errorf("expected %v, got %v", e, a) } if e, a := "d", j.Kind; e != a { t.Errorf("expected %v, got %v", e, a) } - if e, a := "2", j.ResourceVersion; e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "google.com", j.SelfLink; e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := []string{"finalizer.3"}, j.Finalizers; !reflect.DeepEqual(e, a) { - t.Errorf("expected %v, got %v", e, a) - } typeAccessor.SetAPIVersion("d") typeAccessor.SetKind("e") + if e, a := "d", j.APIVersion; e != a { t.Errorf("expected %v, got %v", e, a) } @@ -276,381 +207,17 @@ type InternalTypeMeta struct { OwnerReferences []api.OwnerReference `json:"ownerReferences,omitempty"` } -type InternalObject struct { - TypeMeta InternalTypeMeta `json:",inline"` -} - -func (obj *InternalObject) GetObjectKind() unversioned.ObjectKind { return obj } -func (obj *InternalObject) SetGroupVersionKind(gvk unversioned.GroupVersionKind) { - obj.TypeMeta.APIVersion, obj.TypeMeta.Kind = gvk.ToAPIVersionAndKind() -} -func (obj *InternalObject) GroupVersionKind() unversioned.GroupVersionKind { - return unversioned.FromAPIVersionAndKind(obj.TypeMeta.APIVersion, obj.TypeMeta.Kind) -} - -func TestGenericTypeMetaAccessor(t *testing.T) { - j := &InternalObject{ - InternalTypeMeta{ - Namespace: "bar", - Name: "foo", - GenerateName: "prefix", - UID: "uid", - APIVersion: "/a", - Kind: "b", - ResourceVersion: "1", - SelfLink: "some/place/only/we/know", - Labels: map[string]string{"foo": "bar"}, - Annotations: map[string]string{"x": "y"}, - // OwnerReferences are tested separately - }, - } - accessor := meta.NewAccessor() - namespace, err := accessor.Namespace(j) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - if e, a := "bar", namespace; e != a { - t.Errorf("expected %v, got %v", e, a) - } - name, err := accessor.Name(j) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - if e, a := "foo", name; e != a { - t.Errorf("expected %v, got %v", e, a) - } - generateName, err := accessor.GenerateName(j) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - if e, a := "prefix", generateName; e != a { - t.Errorf("expected %v, got %v", e, a) - } - uid, err := accessor.UID(j) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - if e, a := "uid", string(uid); e != a { - t.Errorf("expected %v, got %v", e, a) - } - apiVersion, err := accessor.APIVersion(j) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - if e, a := "a", apiVersion; e != a { - t.Errorf("expected %v, got %v", e, a) - } - kind, err := accessor.Kind(j) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - if e, a := "b", kind; e != a { - t.Errorf("expected %v, got %v", e, a) - } - rv, err := accessor.ResourceVersion(j) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - if e, a := "1", rv; e != a { - t.Errorf("expected %v, got %v", e, a) - } - selfLink, err := accessor.SelfLink(j) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - if e, a := "some/place/only/we/know", selfLink; e != a { - t.Errorf("expected %v, got %v", e, a) - } - labels, err := accessor.Labels(j) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - if e, a := 1, len(labels); e != a { - t.Errorf("expected %v, got %v", e, a) - } - annotations, err := accessor.Annotations(j) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - if e, a := 1, len(annotations); e != a { - t.Errorf("expected %v, got %v", e, a) - } - - if err := accessor.SetNamespace(j, "baz"); err != nil { - t.Errorf("unexpected error: %v", err) - } - if err := accessor.SetName(j, "bar"); err != nil { - t.Errorf("unexpected error: %v", err) - } - if err := accessor.SetGenerateName(j, "generate"); err != nil { - t.Errorf("unexpected error: %v", err) - } - if err := accessor.SetUID(j, "other"); err != nil { - t.Errorf("unexpected error: %v", err) - } - if err := accessor.SetAPIVersion(j, "c"); err != nil { - t.Errorf("unexpected error: %v", err) - } - if err := accessor.SetKind(j, "d"); err != nil { - t.Errorf("unexpected error: %v", err) - } - if err := accessor.SetResourceVersion(j, "2"); err != nil { - t.Errorf("unexpected error: %v", err) - } - if err := accessor.SetSelfLink(j, "google.com"); err != nil { - t.Errorf("unexpected error: %v", err) - } - if err := accessor.SetLabels(j, map[string]string{}); err != nil { - t.Errorf("unexpected error: %v", err) - } - var nilMap map[string]string - if err := accessor.SetAnnotations(j, nilMap); err != nil { - t.Errorf("unexpected error: %v", err) - } - - // Prove that accessor changes the original object. - if e, a := "baz", j.TypeMeta.Namespace; e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "bar", j.TypeMeta.Name; e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "generate", j.TypeMeta.GenerateName; e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "other", j.TypeMeta.UID; e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "c", j.TypeMeta.APIVersion; e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "d", j.TypeMeta.Kind; e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "2", j.TypeMeta.ResourceVersion; e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "google.com", j.TypeMeta.SelfLink; e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := map[string]string{}, j.TypeMeta.Labels; !reflect.DeepEqual(e, a) { - t.Errorf("expected %#v, got %#v", e, a) - } - if e, a := nilMap, j.TypeMeta.Annotations; !reflect.DeepEqual(e, a) { - t.Errorf("expected %#v, got %#v", e, a) - } -} - -func TestGenericObjectMeta(t *testing.T) { - type TypeMeta struct { - Kind string `json:"kind,omitempty"` - APIVersion string `json:"apiVersion,omitempty"` - } - type ObjectMeta struct { - Namespace string `json:"namespace,omitempty"` - Name string `json:"name,omitempty"` - GenerateName string `json:"generateName,omitempty"` - UID string `json:"uid,omitempty"` - CreationTimestamp unversioned.Time `json:"creationTimestamp,omitempty"` - SelfLink string `json:"selfLink,omitempty"` - ResourceVersion string `json:"resourceVersion,omitempty"` - Labels map[string]string `json:"labels,omitempty"` - Annotations map[string]string `json:"annotations,omitempty"` - Finalizers []string `json:"finalizers,omitempty"` - OwnerReferences []api.OwnerReference `json:"ownerReferences,omitempty"` - } - type Object struct { - TypeMeta `json:",inline"` - ObjectMeta `json:"metadata"` - } - j := Object{ - TypeMeta{ - APIVersion: "a", - Kind: "b", - }, - ObjectMeta{ - Namespace: "bar", - Name: "foo", - GenerateName: "prefix", - UID: "uid", - ResourceVersion: "1", - SelfLink: "some/place/only/we/know", - Labels: map[string]string{"foo": "bar"}, - Annotations: map[string]string{"a": "b"}, - Finalizers: []string{ - "finalizer.1", - "finalizer.2", - }, - }, - } - accessor, err := meta.Accessor(&j) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if e, a := "bar", accessor.GetNamespace(); e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "foo", accessor.GetName(); e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "prefix", accessor.GetGenerateName(); e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "uid", string(accessor.GetUID()); e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "1", accessor.GetResourceVersion(); e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "some/place/only/we/know", accessor.GetSelfLink(); e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := 1, len(accessor.GetLabels()); e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := 1, len(accessor.GetAnnotations()); e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := []string{"finalizer.1", "finalizer.2"}, accessor.GetFinalizers(); !reflect.DeepEqual(e, a) { - t.Errorf("expected %v, got %v", e, a) - } - - typeAccessor, err := meta.TypeAccessor(&j) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if e, a := "a", typeAccessor.GetAPIVersion(); e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "b", typeAccessor.GetKind(); e != a { - t.Errorf("expected %v, got %v", e, a) - } - - accessor.SetNamespace("baz") - accessor.SetName("bar") - accessor.SetGenerateName("generate") - accessor.SetUID("other") - typeAccessor.SetAPIVersion("c") - typeAccessor.SetKind("d") - accessor.SetResourceVersion("2") - accessor.SetSelfLink("google.com") - accessor.SetLabels(map[string]string{"other": "label"}) - accessor.SetAnnotations(map[string]string{"c": "d"}) - accessor.SetFinalizers([]string{"finalizer.3"}) - - // Prove that accessor changes the original object. - if e, a := "baz", j.Namespace; e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "bar", j.Name; e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "generate", j.GenerateName; e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "other", j.UID; e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "c", j.APIVersion; e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "d", j.Kind; e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "2", j.ResourceVersion; e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "google.com", j.SelfLink; e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := map[string]string{"other": "label"}, j.Labels; !reflect.DeepEqual(e, a) { - t.Errorf("expected %#v, got %#v", e, a) - } - if e, a := map[string]string{"c": "d"}, j.Annotations; !reflect.DeepEqual(e, a) { - t.Errorf("expected %#v, got %#v", e, a) - } - if e, a := []string{"finalizer.3"}, j.Finalizers; !reflect.DeepEqual(e, a) { - t.Errorf("expected %v, got %v", e, a) - } -} - -func TestGenericListMeta(t *testing.T) { - type TypeMeta struct { - Kind string `json:"kind,omitempty"` - APIVersion string `json:"apiVersion,omitempty"` - } - type ListMeta struct { - SelfLink string `json:"selfLink,omitempty"` - ResourceVersion string `json:"resourceVersion,omitempty"` - } - type Object struct { - TypeMeta `json:",inline"` - ListMeta `json:"metadata"` - } - j := Object{ - TypeMeta{ - APIVersion: "a", - Kind: "b", - }, - ListMeta{ - ResourceVersion: "1", - SelfLink: "some/place/only/we/know", - }, - } - accessor, err := meta.Accessor(&j) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if e, a := "", accessor.GetName(); e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "", string(accessor.GetUID()); e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "1", accessor.GetResourceVersion(); e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "some/place/only/we/know", accessor.GetSelfLink(); e != a { - t.Errorf("expected %v, got %v", e, a) - } - - typeAccessor, err := meta.TypeAccessor(&j) - if err != nil { - t.Fatalf("unexpected error: %v", err) - } - if e, a := "a", typeAccessor.GetAPIVersion(); e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "b", typeAccessor.GetKind(); e != a { - t.Errorf("expected %v, got %v", e, a) - } - - accessor.SetName("bar") - accessor.SetUID("other") - typeAccessor.SetAPIVersion("c") - typeAccessor.SetKind("d") - accessor.SetResourceVersion("2") - accessor.SetSelfLink("google.com") - - // Prove that accessor changes the original object. - if e, a := "c", j.APIVersion; e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "d", j.Kind; e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "2", j.ResourceVersion; e != a { - t.Errorf("expected %v, got %v", e, a) - } - if e, a := "google.com", j.SelfLink; e != a { - t.Errorf("expected %v, got %v", e, a) - } -} +func (m *InternalTypeMeta) GetResourceVersion() string { return m.ResourceVersion } +func (m *InternalTypeMeta) SetResourceVersion(rv string) { m.ResourceVersion = rv } +func (m *InternalTypeMeta) GetSelfLink() string { return m.SelfLink } +func (m *InternalTypeMeta) SetSelfLink(link string) { m.SelfLink = link } type MyAPIObject struct { TypeMeta InternalTypeMeta `json:",inline"` } +func (obj *MyAPIObject) GetListMeta() unversioned.List { return &obj.TypeMeta } + func (obj *MyAPIObject) GetObjectKind() unversioned.ObjectKind { return obj } func (obj *MyAPIObject) SetGroupVersionKind(gvk unversioned.GroupVersionKind) { obj.TypeMeta.APIVersion, obj.TypeMeta.Kind = gvk.ToAPIVersionAndKind() @@ -851,31 +418,3 @@ func BenchmarkAccessorSetFastPath(b *testing.B) { } b.StopTimer() } - -// BenchmarkAccessorSetReflection provides a baseline for accessor performance -func BenchmarkAccessorSetReflection(b *testing.B) { - obj := &InternalObject{ - InternalTypeMeta{ - Namespace: "bar", - Name: "foo", - GenerateName: "prefix", - UID: "uid", - APIVersion: "a", - Kind: "b", - ResourceVersion: "1", - SelfLink: "some/place/only/we/know", - Labels: map[string]string{"foo": "bar"}, - Annotations: map[string]string{"x": "y"}, - }, - } - - b.ResetTimer() - for i := 0; i < b.N; i++ { - acc, err := meta.Accessor(obj) - if err != nil { - b.Fatal(err) - } - acc.SetNamespace("something") - } - b.StopTimer() -} diff --git a/pkg/api/ref.go b/pkg/api/ref.go index 4aed9d2950..b864593a0a 100644 --- a/pkg/api/ref.go +++ b/pkg/api/ref.go @@ -45,10 +45,6 @@ func GetReference(obj runtime.Object) (*ObjectReference, error) { // Don't make a reference to a reference. return ref, nil } - meta, err := meta.Accessor(obj) - if err != nil { - return nil, err - } gvk := obj.GetObjectKind().GroupVersionKind() @@ -64,10 +60,22 @@ func GetReference(obj runtime.Object) (*ObjectReference, error) { kind = gvks[0].Kind } + // An object that implements only List has enough metadata to build a reference + var listMeta meta.List + objectMeta, err := meta.Accessor(obj) + if err != nil { + listMeta, err = meta.ListAccessor(obj) + if err != nil { + return nil, err + } + } else { + listMeta = objectMeta + } + // if the object referenced is actually persisted, we can also get version from meta version := gvk.GroupVersion().String() if len(version) == 0 { - selfLink := meta.GetSelfLink() + selfLink := listMeta.GetSelfLink() if len(selfLink) == 0 { return nil, ErrNoSelfLink } @@ -83,13 +91,22 @@ func GetReference(obj runtime.Object) (*ObjectReference, error) { version = parts[2] } + // only has list metadata + if objectMeta == nil { + return &ObjectReference{ + Kind: kind, + APIVersion: version, + ResourceVersion: listMeta.GetResourceVersion(), + }, nil + } + return &ObjectReference{ Kind: kind, APIVersion: version, - Name: meta.GetName(), - Namespace: meta.GetNamespace(), - UID: meta.GetUID(), - ResourceVersion: meta.GetResourceVersion(), + Name: objectMeta.GetName(), + Namespace: objectMeta.GetNamespace(), + UID: objectMeta.GetUID(), + ResourceVersion: objectMeta.GetResourceVersion(), }, nil } diff --git a/pkg/quota/generic/evaluator.go b/pkg/quota/generic/evaluator.go index cb0869389b..96dc4878f3 100644 --- a/pkg/quota/generic/evaluator.go +++ b/pkg/quota/generic/evaluator.go @@ -175,9 +175,9 @@ func (g *GenericEvaluator) UsageStats(options quota.UsageStatsOptions) (quota.Us if err != nil { return result, fmt.Errorf("%s: Failed to list %v: %v", g.Name, g.GroupKind(), err) } - _, err = meta.Accessor(list) + _, err = meta.ListAccessor(list) if err != nil { - return result, fmt.Errorf("%s: Unable to understand list result %#v", g.Name, list) + return result, fmt.Errorf("%s: Unable to understand list result, does not appear to be a list %#v", g.Name, list) } items, err := meta.ExtractList(list) if err != nil {