From 85c50f4ccb449d919f1de64a7795571fe418d6b7 Mon Sep 17 00:00:00 2001 From: Chao Xu Date: Sat, 29 Apr 2017 15:43:28 -0700 Subject: [PATCH] remove registry from testing/fixture.go; update client-gen to not use registry in the generated clients Kubernetes-commit: bbb94e42c108840c540c5667f8da97c33b81d84a --- testing/actions.go | 11 +- testing/fixture.go | 272 ++++++++++++++++++--------------------------- 2 files changed, 118 insertions(+), 165 deletions(-) diff --git a/testing/actions.go b/testing/actions.go index fdcebc9b..12a2ecf9 100644 --- a/testing/actions.go +++ b/testing/actions.go @@ -47,20 +47,22 @@ func NewGetAction(resource schema.GroupVersionResource, namespace, name string) return action } -func NewRootListAction(resource schema.GroupVersionResource, opts interface{}) ListActionImpl { +func NewRootListAction(resource schema.GroupVersionResource, kind schema.GroupVersionKind, opts interface{}) ListActionImpl { action := ListActionImpl{} action.Verb = "list" action.Resource = resource + action.Kind = kind labelSelector, fieldSelector, _ := ExtractFromListOptions(opts) action.ListRestrictions = ListRestrictions{labelSelector, fieldSelector} return action } -func NewListAction(resource schema.GroupVersionResource, namespace string, opts interface{}) ListActionImpl { +func NewListAction(resource schema.GroupVersionResource, kind schema.GroupVersionKind, namespace string, opts interface{}) ListActionImpl { action := ListActionImpl{} action.Verb = "list" action.Resource = resource + action.Kind = kind action.Namespace = namespace labelSelector, fieldSelector, _ := ExtractFromListOptions(opts) action.ListRestrictions = ListRestrictions{labelSelector, fieldSelector} @@ -375,9 +377,14 @@ func (a GetActionImpl) GetName() string { type ListActionImpl struct { ActionImpl + Kind schema.GroupVersionKind ListRestrictions ListRestrictions } +func (a ListActionImpl) GetKind() schema.GroupVersionKind { + return a.Kind +} + func (a ListActionImpl) GetListRestrictions() ListRestrictions { return a.ListRestrictions } diff --git a/testing/fixture.go b/testing/fixture.go index f6858535..a53c960c 100644 --- a/testing/fixture.go +++ b/testing/fixture.go @@ -22,7 +22,6 @@ import ( "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" - "k8s.io/apimachinery/pkg/apimachinery/registered" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -39,22 +38,22 @@ type ObjectTracker interface { Add(obj runtime.Object) error // Get retrieves the object by its kind, namespace and name. - Get(gvk schema.GroupVersionKind, ns, name string) (runtime.Object, error) + Get(gvr schema.GroupVersionResource, ns, name string) (runtime.Object, error) // Create adds an object to the tracker in the specified namespace. - Create(obj runtime.Object, ns string) error + Create(gvr schema.GroupVersionResource, obj runtime.Object, ns string) error // Update updates an existing object in the tracker in the specified namespace. - Update(obj runtime.Object, ns string) error + Update(gvr schema.GroupVersionResource, obj runtime.Object, ns string) error // List retrieves all objects of a given kind in the given // namespace. Only non-List kinds are accepted. - List(gvk schema.GroupVersionKind, ns string) (runtime.Object, error) + List(gvr schema.GroupVersionResource, gvk schema.GroupVersionKind, ns string) (runtime.Object, error) // Delete deletes an existing object from the tracker. If object // didn't exist in the tracker prior to deletion, Delete returns // no error. - Delete(gvk schema.GroupVersionKind, ns, name string) error + Delete(gvr schema.GroupVersionResource, ns, name string) error } // ObjectScheme abstracts the implementation of common operations on objects. @@ -66,25 +65,11 @@ type ObjectScheme interface { // ObjectReaction returns a ReactionFunc that applies core.Action to // the given tracker. -func ObjectReaction(tracker ObjectTracker, mapper meta.RESTMapper) ReactionFunc { +func ObjectReaction(tracker ObjectTracker) ReactionFunc { return func(action Action) (bool, runtime.Object, error) { ns := action.GetNamespace() gvr := action.GetResource() - gvk, err := mapper.KindFor(gvr) - if err != nil { - return false, nil, fmt.Errorf("error getting kind for resource %q: %s", gvr, err) - } - - // This is a temporary fix. Because there is no internal resource, so - // the caller has no way to express that it expects to get an internal - // kind back. A more proper fix will be directly specify the Kind when - // build the action. - gvk.Version = gvr.Version - if len(gvk.Version) == 0 { - gvk.Version = runtime.APIVersionInternal - } - // Here and below we need to switch on implementation types, // not on interfaces, as some interfaces are identical // (e.g. UpdateAction and CreateAction), so if we use them, @@ -92,11 +77,11 @@ func ObjectReaction(tracker ObjectTracker, mapper meta.RESTMapper) ReactionFunc switch action := action.(type) { case ListActionImpl: - obj, err := tracker.List(gvk, ns) + obj, err := tracker.List(gvr, action.GetKind(), ns) return true, obj, err case GetActionImpl: - obj, err := tracker.Get(gvk, ns, action.GetName()) + obj, err := tracker.Get(gvr, ns, action.GetName()) return true, obj, err case CreateActionImpl: @@ -105,17 +90,17 @@ func ObjectReaction(tracker ObjectTracker, mapper meta.RESTMapper) ReactionFunc return true, nil, err } if action.GetSubresource() == "" { - err = tracker.Create(action.GetObject(), ns) + err = tracker.Create(gvr, action.GetObject(), ns) } else { // TODO: Currently we're handling subresource creation as an update // on the enclosing resource. This works for some subresources but // might not be generic enough. - err = tracker.Update(action.GetObject(), ns) + err = tracker.Update(gvr, action.GetObject(), ns) } if err != nil { return true, nil, err } - obj, err := tracker.Get(gvk, ns, objMeta.GetName()) + obj, err := tracker.Get(gvr, ns, objMeta.GetName()) return true, obj, err case UpdateActionImpl: @@ -123,15 +108,15 @@ func ObjectReaction(tracker ObjectTracker, mapper meta.RESTMapper) ReactionFunc if err != nil { return true, nil, err } - err = tracker.Update(action.GetObject(), ns) + err = tracker.Update(gvr, action.GetObject(), ns) if err != nil { return true, nil, err } - obj, err := tracker.Get(gvk, ns, objMeta.GetName()) + obj, err := tracker.Get(gvr, ns, objMeta.GetName()) return true, obj, err case DeleteActionImpl: - err := tracker.Delete(gvk, ns, action.GetName()) + err := tracker.Delete(gvr, ns, action.GetName()) if err != nil { return true, nil, err } @@ -144,32 +129,35 @@ func ObjectReaction(tracker ObjectTracker, mapper meta.RESTMapper) ReactionFunc } type tracker struct { - registry *registered.APIRegistrationManager - scheme ObjectScheme - decoder runtime.Decoder - lock sync.RWMutex - objects map[schema.GroupVersionKind][]runtime.Object + scheme ObjectScheme + decoder runtime.Decoder + lock sync.RWMutex + objects map[schema.GroupVersionResource][]runtime.Object } var _ ObjectTracker = &tracker{} // NewObjectTracker returns an ObjectTracker that can be used to keep track // of objects for the fake clientset. Mostly useful for unit tests. -func NewObjectTracker(registry *registered.APIRegistrationManager, scheme ObjectScheme, decoder runtime.Decoder) ObjectTracker { +func NewObjectTracker(scheme ObjectScheme, decoder runtime.Decoder) ObjectTracker { return &tracker{ - registry: registry, - scheme: scheme, - decoder: decoder, - objects: make(map[schema.GroupVersionKind][]runtime.Object), + scheme: scheme, + decoder: decoder, + objects: make(map[schema.GroupVersionResource][]runtime.Object), } } -func (t *tracker) List(gvk schema.GroupVersionKind, ns string) (runtime.Object, error) { +func (t *tracker) List(gvr schema.GroupVersionResource, gvk schema.GroupVersionKind, ns string) (runtime.Object, error) { // Heuristic for list kind: original kind + List suffix. Might // not always be true but this tracker has a pretty limited // understanding of the actual API model. listGVK := gvk listGVK.Kind = listGVK.Kind + "List" + // GVK does have the concept of "internal version". The scheme recognizes + // the runtime.APIVersionInternal, but not the empty string. + if listGVK.Version == "" { + listGVK.Version = runtime.APIVersionInternal + } list, err := t.scheme.New(listGVK) if err != nil { @@ -183,7 +171,7 @@ func (t *tracker) List(gvk schema.GroupVersionKind, ns string) (runtime.Object, t.lock.RLock() defer t.lock.RUnlock() - objs, ok := t.objects[gvk] + objs, ok := t.objects[gvr] if !ok { return list, nil } @@ -201,17 +189,13 @@ func (t *tracker) List(gvk schema.GroupVersionKind, ns string) (runtime.Object, return list, nil } -func (t *tracker) Get(gvk schema.GroupVersionKind, ns, name string) (runtime.Object, error) { - if err := checkNamespace(t.registry, gvk, ns); err != nil { - return nil, err - } - - errNotFound := errors.NewNotFound(schema.GroupResource{Group: gvk.Group, Resource: gvk.Kind}, name) +func (t *tracker) Get(gvr schema.GroupVersionResource, ns, name string) (runtime.Object, error) { + errNotFound := errors.NewNotFound(gvr.GroupResource(), name) t.lock.RLock() defer t.lock.RUnlock() - objs, ok := t.objects[gvk] + objs, ok := t.objects[gvr] if !ok { return nil, errNotFound } @@ -224,7 +208,7 @@ func (t *tracker) Get(gvk schema.GroupVersionKind, ns, name string) (runtime.Obj return nil, errNotFound } if len(matchingObjs) > 1 { - return nil, fmt.Errorf("more than one object matched gvk %s, ns: %q name: %q", gvk, ns, name) + return nil, fmt.Errorf("more than one object matched gvr %s, ns: %q name: %q", gvr, ns, name) } // Only one object should match in the tracker if it works @@ -236,9 +220,6 @@ func (t *tracker) Get(gvk schema.GroupVersionKind, ns, name string) (runtime.Obj } if status, ok := obj.(*metav1.Status); ok { - if status.Details != nil { - status.Details.Kind = gvk.Kind - } if status.Status != metav1.StatusSuccess { return nil, &errors.StatusError{ErrStatus: *status} } @@ -251,23 +232,10 @@ func (t *tracker) Add(obj runtime.Object) error { if meta.IsListType(obj) { return t.addList(obj, false) } - objMeta, err := meta.Accessor(obj) if err != nil { return err } - return t.add(obj, objMeta.GetNamespace(), false) -} - -func (t *tracker) Create(obj runtime.Object, ns string) error { - return t.add(obj, ns, false) -} - -func (t *tracker) Update(obj runtime.Object, ns string) error { - return t.add(obj, ns, true) -} - -func (t *tracker) add(obj runtime.Object, ns string, replaceExisting bool) error { gvks, _, err := t.scheme.ObjectKinds(obj) if err != nil { return err @@ -275,66 +243,84 @@ func (t *tracker) add(obj runtime.Object, ns string, replaceExisting bool) error if len(gvks) == 0 { return fmt.Errorf("no registered kinds for %v", obj) } + for _, gvk := range gvks { + // NOTE: UnsafeGuessKindToResource is a heuristic and default match. The + // actual registration in apiserver can specify arbitrary route for a + // gvk. If a test uses such objects, it cannot preset the tracker with + // objects via Add(). Instead, it should trigger the Create() function + // of the tracker, where an arbitrary gvr can be specified. + gvr, _ := meta.UnsafeGuessKindToResource(gvk) + // Resource doesn't have the concept of "__internal" version, just set it to "". + if gvr.Version == runtime.APIVersionInternal { + gvr.Version = "" + } + err := t.add(gvr, obj, objMeta.GetNamespace(), false) + if err != nil { + return err + } + } + return nil +} + +func (t *tracker) Create(gvr schema.GroupVersionResource, obj runtime.Object, ns string) error { + return t.add(gvr, obj, ns, false) +} + +func (t *tracker) Update(gvr schema.GroupVersionResource, obj runtime.Object, ns string) error { + return t.add(gvr, obj, ns, true) +} + +func (t *tracker) add(gvr schema.GroupVersionResource, obj runtime.Object, ns string, replaceExisting bool) error { t.lock.Lock() defer t.lock.Unlock() - for _, gvk := range gvks { - gr := schema.GroupResource{Group: gvk.Group, Resource: gvk.Kind} + gr := gvr.GroupResource() - // To avoid the object from being accidentally modified by caller - // after it's been added to the tracker, we always store the deep - // copy. - obj, err = t.scheme.Copy(obj) - if err != nil { - return err - } - - if status, ok := obj.(*metav1.Status); ok && status.Details != nil { - gvk.Kind = status.Details.Kind - } - - newMeta, err := meta.Accessor(obj) - if err != nil { - return err - } - - // Propagate namespace to the new object if hasn't already been set. - if len(newMeta.GetNamespace()) == 0 { - newMeta.SetNamespace(ns) - } - - if ns != newMeta.GetNamespace() { - msg := fmt.Sprintf("request namespace does not match object namespace, request: %q object: %q", ns, newMeta.GetNamespace()) - return errors.NewBadRequest(msg) - } - - if err := checkNamespace(t.registry, gvk, newMeta.GetNamespace()); err != nil { - return err - } - - for i, existingObj := range t.objects[gvk] { - oldMeta, err := meta.Accessor(existingObj) - if err != nil { - return err - } - if oldMeta.GetNamespace() == newMeta.GetNamespace() && oldMeta.GetName() == newMeta.GetName() { - if replaceExisting { - t.objects[gvk][i] = obj - return nil - } - return errors.NewAlreadyExists(gr, newMeta.GetName()) - } - } - - if replaceExisting { - // Tried to update but no matching object was found. - return errors.NewNotFound(gr, newMeta.GetName()) - } - - t.objects[gvk] = append(t.objects[gvk], obj) + // To avoid the object from being accidentally modified by caller + // after it's been added to the tracker, we always store the deep + // copy. + obj, err := t.scheme.Copy(obj) + if err != nil { + return err } + newMeta, err := meta.Accessor(obj) + if err != nil { + return err + } + + // Propagate namespace to the new object if hasn't already been set. + if len(newMeta.GetNamespace()) == 0 { + newMeta.SetNamespace(ns) + } + + if ns != newMeta.GetNamespace() { + msg := fmt.Sprintf("request namespace does not match object namespace, request: %q object: %q", ns, newMeta.GetNamespace()) + return errors.NewBadRequest(msg) + } + + for i, existingObj := range t.objects[gvr] { + oldMeta, err := meta.Accessor(existingObj) + if err != nil { + return err + } + if oldMeta.GetNamespace() == newMeta.GetNamespace() && oldMeta.GetName() == newMeta.GetName() { + if replaceExisting { + t.objects[gvr][i] = obj + return nil + } + return errors.NewAlreadyExists(gr, newMeta.GetName()) + } + } + + if replaceExisting { + // Tried to update but no matching object was found. + return errors.NewNotFound(gr, newMeta.GetName()) + } + + t.objects[gvr] = append(t.objects[gvr], obj) + return nil } @@ -348,35 +334,26 @@ func (t *tracker) addList(obj runtime.Object, replaceExisting bool) error { return errs[0] } for _, obj := range list { - objMeta, err := meta.Accessor(obj) - if err != nil { - return err - } - err = t.add(obj, objMeta.GetNamespace(), replaceExisting) - if err != nil { + if err := t.Add(obj); err != nil { return err } } return nil } -func (t *tracker) Delete(gvk schema.GroupVersionKind, ns, name string) error { - if err := checkNamespace(t.registry, gvk, ns); err != nil { - return err - } - +func (t *tracker) Delete(gvr schema.GroupVersionResource, ns, name string) error { t.lock.Lock() defer t.lock.Unlock() found := false - for i, existingObj := range t.objects[gvk] { + for i, existingObj := range t.objects[gvr] { objMeta, err := meta.Accessor(existingObj) if err != nil { return err } if objMeta.GetNamespace() == ns && objMeta.GetName() == name { - t.objects[gvk] = append(t.objects[gvk][:i], t.objects[gvk][i+1:]...) + t.objects[gvr] = append(t.objects[gvr][:i], t.objects[gvr][i+1:]...) found = true break } @@ -386,7 +363,7 @@ func (t *tracker) Delete(gvk schema.GroupVersionKind, ns, name string) error { return nil } - return errors.NewNotFound(schema.GroupResource{Group: gvk.Group, Resource: gvk.Kind}, name) + return errors.NewNotFound(gvr.GroupResource(), name) } // filterByNamespaceAndName returns all objects in the collection that @@ -412,37 +389,6 @@ func filterByNamespaceAndName(objs []runtime.Object, ns, name string) ([]runtime return res, nil } -// checkNamespace makes sure that the scope of gvk matches ns. It -// returns an error if namespace is empty but gvk is a namespaced -// kind, or if ns is non-empty and gvk is a namespaced kind. -func checkNamespace(registry *registered.APIRegistrationManager, gvk schema.GroupVersionKind, ns string) error { - group, err := registry.Group(gvk.Group) - if err != nil { - return err - } - mapping, err := group.RESTMapper.RESTMapping(gvk.GroupKind(), gvk.Version) - if err != nil { - return err - } - switch mapping.Scope.Name() { - case meta.RESTScopeNameRoot: - if ns != "" { - return fmt.Errorf("namespace specified for a non-namespaced kind %s", gvk) - } - case meta.RESTScopeNameNamespace: - if ns == "" { - // Skipping this check for Events, since - // controllers emit events that have no namespace, - // even though Event is a namespaced resource. - if gvk.Kind != "Event" { - return fmt.Errorf("no namespace specified for a namespaced kind %s", gvk) - } - } - } - - return nil -} - func DefaultWatchReactor(watchInterface watch.Interface, err error) WatchReactionFunc { return func(action Action) (bool, watch.Interface, error) { return true, watchInterface, err