From 9ef99d189f2e454a293b8c343fee173c876ed21f Mon Sep 17 00:00:00 2001 From: David Eads Date: Fri, 2 Mar 2018 09:19:52 -0500 Subject: [PATCH] deep copy fake client actions to avoid accidental mutation --- .../src/k8s.io/client-go/testing/actions.go | 103 ++++++++++++++++++ staging/src/k8s.io/client-go/testing/fake.go | 12 +- .../client/custom_metrics/fake/fake_client.go | 12 ++ 3 files changed, 121 insertions(+), 6 deletions(-) diff --git a/staging/src/k8s.io/client-go/testing/actions.go b/staging/src/k8s.io/client-go/testing/actions.go index 6f1c3a896fd..0210b67845c 100644 --- a/staging/src/k8s.io/client-go/testing/actions.go +++ b/staging/src/k8s.io/client-go/testing/actions.go @@ -324,6 +324,10 @@ type Action interface { GetResource() schema.GroupVersionResource GetSubresource() string Matches(verb, resource string) bool + + // DeepCopy is used to copy an action to avoid any risk of accidental mutation. Most people never need to call this + // because the invocation logic deep copies before calls to storage and reactors. + DeepCopy() Action } type GenericAction interface { @@ -404,6 +408,10 @@ func (a ActionImpl) Matches(verb, resource string) bool { return strings.ToLower(verb) == strings.ToLower(a.Verb) && strings.ToLower(resource) == strings.ToLower(a.Resource.Resource) } +func (a ActionImpl) DeepCopy() Action { + ret := a + return ret +} type GenericActionImpl struct { ActionImpl @@ -414,6 +422,14 @@ func (a GenericActionImpl) GetValue() interface{} { return a.Value } +func (a GenericActionImpl) DeepCopy() Action { + return GenericActionImpl{ + ActionImpl: a.ActionImpl.DeepCopy().(ActionImpl), + // TODO this is wrong, but no worse than before + Value: a.Value, + } +} + type GetActionImpl struct { ActionImpl Name string @@ -423,6 +439,13 @@ func (a GetActionImpl) GetName() string { return a.Name } +func (a GetActionImpl) DeepCopy() Action { + return GetActionImpl{ + ActionImpl: a.ActionImpl.DeepCopy().(ActionImpl), + Name: a.Name, + } +} + type ListActionImpl struct { ActionImpl Kind schema.GroupVersionKind @@ -438,6 +461,18 @@ func (a ListActionImpl) GetListRestrictions() ListRestrictions { return a.ListRestrictions } +func (a ListActionImpl) DeepCopy() Action { + return ListActionImpl{ + ActionImpl: a.ActionImpl.DeepCopy().(ActionImpl), + Kind: a.Kind, + Name: a.Name, + ListRestrictions: ListRestrictions{ + Labels: a.ListRestrictions.Labels.DeepCopySelector(), + Fields: a.ListRestrictions.Fields.DeepCopySelector(), + }, + } +} + type CreateActionImpl struct { ActionImpl Name string @@ -448,6 +483,14 @@ func (a CreateActionImpl) GetObject() runtime.Object { return a.Object } +func (a CreateActionImpl) DeepCopy() Action { + return CreateActionImpl{ + ActionImpl: a.ActionImpl.DeepCopy().(ActionImpl), + Name: a.Name, + Object: a.Object.DeepCopyObject(), + } +} + type UpdateActionImpl struct { ActionImpl Object runtime.Object @@ -457,6 +500,13 @@ func (a UpdateActionImpl) GetObject() runtime.Object { return a.Object } +func (a UpdateActionImpl) DeepCopy() Action { + return UpdateActionImpl{ + ActionImpl: a.ActionImpl.DeepCopy().(ActionImpl), + Object: a.Object.DeepCopyObject(), + } +} + type PatchActionImpl struct { ActionImpl Name string @@ -471,6 +521,16 @@ func (a PatchActionImpl) GetPatch() []byte { return a.Patch } +func (a PatchActionImpl) DeepCopy() Action { + patch := make([]byte, len(a.Patch)) + copy(patch, a.Patch) + return PatchActionImpl{ + ActionImpl: a.ActionImpl.DeepCopy().(ActionImpl), + Name: a.Name, + Patch: patch, + } +} + type DeleteActionImpl struct { ActionImpl Name string @@ -480,6 +540,13 @@ func (a DeleteActionImpl) GetName() string { return a.Name } +func (a DeleteActionImpl) DeepCopy() Action { + return DeleteActionImpl{ + ActionImpl: a.ActionImpl.DeepCopy().(ActionImpl), + Name: a.Name, + } +} + type DeleteCollectionActionImpl struct { ActionImpl ListRestrictions ListRestrictions @@ -489,6 +556,16 @@ func (a DeleteCollectionActionImpl) GetListRestrictions() ListRestrictions { return a.ListRestrictions } +func (a DeleteCollectionActionImpl) DeepCopy() Action { + return DeleteCollectionActionImpl{ + ActionImpl: a.ActionImpl.DeepCopy().(ActionImpl), + ListRestrictions: ListRestrictions{ + Labels: a.ListRestrictions.Labels.DeepCopySelector(), + Fields: a.ListRestrictions.Fields.DeepCopySelector(), + }, + } +} + type WatchActionImpl struct { ActionImpl WatchRestrictions WatchRestrictions @@ -498,6 +575,17 @@ func (a WatchActionImpl) GetWatchRestrictions() WatchRestrictions { return a.WatchRestrictions } +func (a WatchActionImpl) DeepCopy() Action { + return WatchActionImpl{ + ActionImpl: a.ActionImpl.DeepCopy().(ActionImpl), + WatchRestrictions: WatchRestrictions{ + Labels: a.WatchRestrictions.Labels.DeepCopySelector(), + Fields: a.WatchRestrictions.Fields.DeepCopySelector(), + ResourceVersion: a.WatchRestrictions.ResourceVersion, + }, + } +} + type ProxyGetActionImpl struct { ActionImpl Scheme string @@ -526,3 +614,18 @@ func (a ProxyGetActionImpl) GetPath() string { func (a ProxyGetActionImpl) GetParams() map[string]string { return a.Params } + +func (a ProxyGetActionImpl) DeepCopy() Action { + params := map[string]string{} + for k, v := range a.Params { + params[k] = v + } + return ProxyGetActionImpl{ + ActionImpl: a.ActionImpl.DeepCopy().(ActionImpl), + Scheme: a.Scheme, + Name: a.Name, + Port: a.Port, + Path: a.Path, + Params: params, + } +} diff --git a/staging/src/k8s.io/client-go/testing/fake.go b/staging/src/k8s.io/client-go/testing/fake.go index da47b23b952..cf625c3c0b5 100644 --- a/staging/src/k8s.io/client-go/testing/fake.go +++ b/staging/src/k8s.io/client-go/testing/fake.go @@ -134,13 +134,13 @@ func (c *Fake) Invokes(action Action, defaultReturnObj runtime.Object) (runtime. c.Lock() defer c.Unlock() - c.actions = append(c.actions, action) + c.actions = append(c.actions, action.DeepCopy()) for _, reactor := range c.ReactionChain { if !reactor.Handles(action) { continue } - handled, ret, err := reactor.React(action) + handled, ret, err := reactor.React(action.DeepCopy()) if !handled { continue } @@ -157,13 +157,13 @@ func (c *Fake) InvokesWatch(action Action) (watch.Interface, error) { c.Lock() defer c.Unlock() - c.actions = append(c.actions, action) + c.actions = append(c.actions, action.DeepCopy()) for _, reactor := range c.WatchReactionChain { if !reactor.Handles(action) { continue } - handled, ret, err := reactor.React(action) + handled, ret, err := reactor.React(action.DeepCopy()) if !handled { continue } @@ -180,13 +180,13 @@ func (c *Fake) InvokesProxy(action Action) restclient.ResponseWrapper { c.Lock() defer c.Unlock() - c.actions = append(c.actions, action) + c.actions = append(c.actions, action.DeepCopy()) for _, reactor := range c.ProxyReactionChain { if !reactor.Handles(action) { continue } - handled, ret, err := reactor.React(action) + handled, ret, err := reactor.React(action.DeepCopy()) if !handled || err != nil { continue } diff --git a/staging/src/k8s.io/metrics/pkg/client/custom_metrics/fake/fake_client.go b/staging/src/k8s.io/metrics/pkg/client/custom_metrics/fake/fake_client.go index 08da222844a..c44aa931b01 100644 --- a/staging/src/k8s.io/metrics/pkg/client/custom_metrics/fake/fake_client.go +++ b/staging/src/k8s.io/metrics/pkg/client/custom_metrics/fake/fake_client.go @@ -51,6 +51,18 @@ func (i GetForActionImpl) GetSubresource() string { return i.MetricName } +func (i GetForActionImpl) DeepCopy() testing.Action { + var labelSelector labels.Selector + if i.LabelSelector != nil { + labelSelector = i.LabelSelector.DeepCopySelector() + } + return GetForActionImpl{ + GetAction: i.GetAction.DeepCopy().(testing.GetAction), + MetricName: i.MetricName, + LabelSelector: labelSelector, + } +} + func NewGetForAction(groupKind schema.GroupKind, namespace, name string, metricName string, labelSelector labels.Selector) GetForActionImpl { // the version doesn't matter gvk := groupKind.WithVersion("")