From 41ebb22011cb79aed2241417249e824cdfcda5fc Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Thu, 31 Jan 2019 17:42:04 +0000 Subject: [PATCH] Use a single deep copied object between all reactors in fake client --- staging/src/k8s.io/client-go/testing/BUILD | 5 +- staging/src/k8s.io/client-go/testing/fake.go | 15 +- .../src/k8s.io/client-go/testing/fake_test.go | 166 ++++++++++++++++++ 3 files changed, 179 insertions(+), 7 deletions(-) create mode 100644 staging/src/k8s.io/client-go/testing/fake_test.go diff --git a/staging/src/k8s.io/client-go/testing/BUILD b/staging/src/k8s.io/client-go/testing/BUILD index d0849fcb1f4..42b1b615033 100644 --- a/staging/src/k8s.io/client-go/testing/BUILD +++ b/staging/src/k8s.io/client-go/testing/BUILD @@ -34,7 +34,10 @@ go_library( go_test( name = "go_default_test", - srcs = ["fixture_test.go"], + srcs = [ + "fake_test.go", + "fixture_test.go", + ], embed = [":go_default_library"], deps = [ "//staging/src/k8s.io/apimachinery/pkg/api/meta:go_default_library", diff --git a/staging/src/k8s.io/client-go/testing/fake.go b/staging/src/k8s.io/client-go/testing/fake.go index 8b3f31eaf5c..8b9ee149c83 100644 --- a/staging/src/k8s.io/client-go/testing/fake.go +++ b/staging/src/k8s.io/client-go/testing/fake.go @@ -131,13 +131,14 @@ func (c *Fake) Invokes(action Action, defaultReturnObj runtime.Object) (runtime. c.Lock() defer c.Unlock() + actionCopy := action.DeepCopy() c.actions = append(c.actions, action.DeepCopy()) for _, reactor := range c.ReactionChain { - if !reactor.Handles(action) { + if !reactor.Handles(actionCopy) { continue } - handled, ret, err := reactor.React(action.DeepCopy()) + handled, ret, err := reactor.React(actionCopy) if !handled { continue } @@ -154,13 +155,14 @@ func (c *Fake) InvokesWatch(action Action) (watch.Interface, error) { c.Lock() defer c.Unlock() + actionCopy := action.DeepCopy() c.actions = append(c.actions, action.DeepCopy()) for _, reactor := range c.WatchReactionChain { - if !reactor.Handles(action) { + if !reactor.Handles(actionCopy) { continue } - handled, ret, err := reactor.React(action.DeepCopy()) + handled, ret, err := reactor.React(actionCopy) if !handled { continue } @@ -177,13 +179,14 @@ func (c *Fake) InvokesProxy(action Action) restclient.ResponseWrapper { c.Lock() defer c.Unlock() + actionCopy := action.DeepCopy() c.actions = append(c.actions, action.DeepCopy()) for _, reactor := range c.ProxyReactionChain { - if !reactor.Handles(action) { + if !reactor.Handles(actionCopy) { continue } - handled, ret, err := reactor.React(action.DeepCopy()) + handled, ret, err := reactor.React(actionCopy) if !handled || err != nil { continue } diff --git a/staging/src/k8s.io/client-go/testing/fake_test.go b/staging/src/k8s.io/client-go/testing/fake_test.go new file mode 100644 index 00000000000..b335e700a20 --- /dev/null +++ b/staging/src/k8s.io/client-go/testing/fake_test.go @@ -0,0 +1,166 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package testing + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" +) + +func TestOriginalObjectCaptured(t *testing.T) { + // this ReactionFunc sets the resources ClusterName + const testClusterName = "some-value" + reactors := []ReactionFunc{ + func(action Action) (bool, runtime.Object, error) { + createAction := action.(CreateActionImpl) + accessor, err := meta.Accessor(createAction.Object) + if err != nil { + return false, nil, err + } + + // set any field on the resource + accessor.SetClusterName(testClusterName) + + return true, createAction.Object, nil + }, + } + + // create a new Fake with the test reactors + f := &Fake{} + for _, r := range reactors { + f.AddReactor("", "", r) + } + + // construct a test resource + testResource := schema.GroupVersionResource{Group: "", Version: "test_version", Resource: "test_kind"} + testObj := getArbitraryResource(testResource, "test_name", "test_namespace") + + // create a fake CreateAction + action := CreateActionImpl{ + Object: testObj, + } + + // execute the reaction chain + ret, err := f.Invokes(action, nil) + assert.NoError(t, err, "running Invokes failed") + + // obtain a metadata accessor for the returned resource + accessor, err := meta.Accessor(ret) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // validate that the returned resource was modified by the ReactionFunc + if accessor.GetClusterName() != testClusterName { + t.Errorf("expected resource returned by Invokes to be modified by the ReactionFunc") + } + // verify one action was performed + if len(f.actions) != 1 { + t.Errorf("expected 1 action to be executed") + t.FailNow() + } + // check to ensure the recorded action has not been modified by the chain + createAction := f.actions[0].(CreateActionImpl) + accessor, err = meta.Accessor(createAction.Object) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if accessor.GetClusterName() != "" { + t.Errorf("expected Action recorded to not be modified by ReactionFunc but it was") + } +} + +func TestReactorChangesPersisted(t *testing.T) { + // this ReactionFunc sets the resources ClusterName + const testClusterName = "some-value" + reactors := []ReactionFunc{ + func(action Action) (bool, runtime.Object, error) { + createAction := action.(CreateActionImpl) + accessor, err := meta.Accessor(createAction.Object) + if err != nil { + return false, nil, err + } + + // set any field on the resource + accessor.SetClusterName(testClusterName) + + return false, createAction.Object, nil + }, + func(action Action) (bool, runtime.Object, error) { + createAction := action.(CreateActionImpl) + accessor, err := meta.Accessor(createAction.Object) + if err != nil { + return false, nil, err + } + + // ensure the clusterName is set to testClusterName already + if accessor.GetClusterName() != testClusterName { + t.Errorf("expected resource passed to second reactor to be modified by first reactor") + } + + return true, createAction.Object, nil + }, + } + + // create a new Fake with the test reactors + f := &Fake{} + for _, r := range reactors { + f.AddReactor("", "", r) + } + + // construct a test resource + testResource := schema.GroupVersionResource{Group: "", Version: "test_version", Resource: "test_kind"} + testObj := getArbitraryResource(testResource, "test_name", "test_namespace") + + // create a fake CreateAction + action := CreateActionImpl{ + Object: testObj, + } + + // execute the reaction chain + ret, err := f.Invokes(action, nil) + assert.NoError(t, err, "running Invokes failed") + + // obtain a metadata accessor for the returned resource + accessor, err := meta.Accessor(ret) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // validate that the returned resource was modified by the ReactionFunc + if accessor.GetClusterName() != testClusterName { + t.Errorf("expected resource returned by Invokes to be modified by the ReactionFunc") + } + // verify one action was performed + if len(f.actions) != 1 { + t.Errorf("expected 1 action to be executed") + t.FailNow() + } + // check to ensure the recorded action has not been modified by the chain + createAction := f.actions[0].(CreateActionImpl) + accessor, err = meta.Accessor(createAction.Object) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if accessor.GetClusterName() != "" { + t.Errorf("expected Action recorded to not be modified by ReactionFunc but it was") + } +}