Merge pull request #60709 from deads2k/client-02-fake-copy

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

deep copy fake client actions to avoid accidental mutation

I just got bit by this downstream.  Without a deep copy it is possible accidentally mutate the thing you created, thus invalidating your testing.  It's particularly nasty inside of a controller doing a loop on objects, making refs to them, and creating.  This works running in an actual process since we serialize and write, but fails unit tests since there is no serialization step.

@kubernetes/sig-api-machinery-bugs 

```release-note
NONE
```

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,
+	}
+}

staging/src/k8s.io/client-go/testing/fake.go

@@ -131,13 +131,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
 		}
@@ -154,13 +154,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
 		}
@@ -177,13 +177,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
 		}

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("")