test(clientfakes): add k8s client decorator and shared options

- Add k8s fake client decorator
- Add shared options for k8s and olm decorators
- Add simple name generator option

Author: njhale

pkg/api/client/clientset/versioned/fake/decorator.go

@@ -18,6 +18,8 @@ package fake
 import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/testing"
+
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/clientfake"
 )
 
 // ClientsetDecorator defines decorator methods for a Clientset.
@@ -38,14 +40,19 @@ type ReactionForwardingClientsetDecorator struct {
 
 // NewReactionForwardingClientsetDecorator returns the ReactionForwardingClientsetDecorator wrapped Clientset result
 // of calling NewSimpleClientset with the given objects.
-func NewReactionForwardingClientsetDecorator(objects ...runtime.Object) *ReactionForwardingClientsetDecorator {
+func NewReactionForwardingClientsetDecorator(objects []runtime.Object, options ...clientfake.Option) *ReactionForwardingClientsetDecorator {
 	decorator := &ReactionForwardingClientsetDecorator{
 		Clientset: *NewSimpleClientset(objects...),
 	}
 
 	// Swap out the embedded ReactionChain with a Reactor that reduces over the decorator's ReactionChain.
 	decorator.ReactionChain = decorator.Clientset.ReactionChain
 	decorator.Clientset.ReactionChain = []testing.Reactor{&testing.SimpleReactor{"*", "*", decorator.reduceReactions}}
+	
+	// Apply options
+	for _, option := range options {
+		option(decorator)
+	}
 
 	return decorator
 }

pkg/controller/operators/catalog/operator_test.go

@@ -16,13 +16,11 @@ import (
 	rbacv1 "k8s.io/api/rbac/v1"
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 	apiextensionsfake "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/fake"
-	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/informers"
 	k8sfake "k8s.io/client-go/kubernetes/fake"
-	clitesting "k8s.io/client-go/testing"
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/util/workqueue"
 	apiregistrationfake "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/fake"
@@ -34,6 +32,7 @@ import (
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/reconciler"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/fakes"
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/clientfake"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorclient"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/operatorlister"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/ownerutil"
@@ -379,8 +378,6 @@ func TestSyncCatalogSources(t *testing.T) {
 	}
 }
 
-// TODO: CatalogSource tests for RegistryServiceStatus
-
 func TestCompetingCRDOwnersExist(t *testing.T) {
 
 	testNamespace := "default"
@@ -465,43 +462,13 @@ func fakeConfigMapData() map[string]string {
 	return data
 }
 
-// fakeClientOption configures a fake option
-type fakeClientOption func(fake.ClientsetDecorator)
-
-// withSelfLinks returns a fakeClientOption that configures a ClientsetDecorator to write selfLinks to all OLM types on create.
-func withSelfLinks(t *testing.T) fakeClientOption {
-	return func(c fake.ClientsetDecorator) {
-		c.PrependReactor("create", "*", func(a clitesting.Action) (bool, runtime.Object, error) {
-			ca, ok := a.(clitesting.CreateAction)
-			if !ok {
-				t.Fatalf("expected CreateAction")
-			}
-
-			obj := ca.GetObject()
-			accessor, err := meta.Accessor(obj)
-			if err != nil {
-				return false, nil, err
-			}
-			if accessor.GetSelfLink() != "" {
-				// SelfLink is already set
-				return false, nil, nil
-			}
-
-			gvr := ca.GetResource()
-			accessor.SetSelfLink(buildSelfLink(gvr.GroupVersion().String(), gvr.Resource, accessor.GetNamespace(), accessor.GetName()))
-
-			return false, obj, nil
-		})
-	}
-}
-
 // fakeOperatorConfig is the configuration for a fake operator.
 type fakeOperatorConfig struct {
-	clientObjs        []runtime.Object
-	k8sObjs           []runtime.Object
-	extObjs           []runtime.Object
-	regObjs           []runtime.Object
-	fakeClientOptions []fakeClientOption
+	clientObjs    []runtime.Object
+	k8sObjs       []runtime.Object
+	extObjs       []runtime.Object
+	regObjs       []runtime.Object
+	clientOptions []clientfake.Option
 }
 
 // fakeOperatorOption applies an option to the given fake operator configuration.
@@ -525,9 +492,9 @@ func extObjs(extObjs ...runtime.Object) fakeOperatorOption {
 	}
 }
 
-func withFakeClientOptions(options ...fakeClientOption) fakeOperatorOption {
+func withFakeClientOptions(options ...clientfake.Option) fakeOperatorOption {
 	return func(config *fakeOperatorConfig) {
-		config.fakeClientOptions = options
+		config.clientOptions = options
 	}
 }
 
@@ -540,11 +507,7 @@ func NewFakeOperator(namespace string, watchedNamespaces []string, stopCh <-chan
 	}
 
 	// Create client fakes
-	clientFake := fake.NewReactionForwardingClientsetDecorator(config.clientObjs...)
-	for _, option := range config.fakeClientOptions {
-		option(clientFake)
-	}
-
+	clientFake := fake.NewReactionForwardingClientsetDecorator(config.clientObjs, config.clientOptions...)
 	opClientFake := operatorclient.NewClient(k8sfake.NewSimpleClientset(config.k8sObjs...), apiextensionsfake.NewSimpleClientset(config.extObjs...), apiregistrationfake.NewSimpleClientset(config.regObjs...))
 
 	// Create operator namespace
@@ -707,11 +670,3 @@ func toManifest(obj runtime.Object) string {
 	raw, _ := json.Marshal(obj)
 	return string(raw)
 }
-
-// selfLink returns a selfLink.
-func buildSelfLink(groupVersion, plural, namespace, name string) string {
-	if namespace == metav1.NamespaceAll {
-		return fmt.Sprintf("/apis/%s/%s/%s", groupVersion, plural, name)
-	}
-	return fmt.Sprintf("/apis/%s/namespaces/%s/%s/%s", groupVersion, namespace, plural, name)
-}

pkg/controller/operators/catalog/subscriptions_test.go

@@ -14,15 +14,16 @@ import (
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/reconciler"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/fakes"
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/clientfake"
 )
 
 func TestSyncSubscriptions(t *testing.T) {
 	now := metav1.Date(2018, time.January, 26, 20, 40, 0, 0, time.UTC)
 	timeNow = func() metav1.Time { return now }
-
 	testNamespace := "testNamespace"
+
 	type fields struct {
-		fakeClientOptions []fakeClientOption
+		clientOptions     []clientfake.Option
 		sourcesLastUpdate metav1.Time
 		resolveSteps      []*v1alpha1.Step
 		resolveSubs       []*v1alpha1.Subscription
@@ -51,7 +52,7 @@ func TestSyncSubscriptions(t *testing.T) {
 		{
 			name: "NoStatus/NoCurrentCSV/FoundInCatalog",
 			fields: fields{
-				fakeClientOptions: []fakeClientOption{withSelfLinks(t)},
+				clientOptions: []clientfake.Option{clientfake.WithSelfLinks(t)},
 				existingOLMObjs: []runtime.Object{
 					&v1alpha1.Subscription{
 						ObjectMeta: metav1.ObjectMeta{
@@ -182,7 +183,7 @@ func TestSyncSubscriptions(t *testing.T) {
 		{
 			name: "Status/HaveCurrentCSV/UpdateFoundInCatalog",
 			fields: fields{
-				fakeClientOptions: []fakeClientOption{withSelfLinks(t)},
+				clientOptions: []clientfake.Option{clientfake.WithSelfLinks(t)},
 				existingOLMObjs: []runtime.Object{
 					&v1alpha1.ClusterServiceVersion{
 						ObjectMeta: metav1.ObjectMeta{
@@ -322,7 +323,7 @@ func TestSyncSubscriptions(t *testing.T) {
 		{
 			name: "Status/HaveCurrentCSV/UpdateFoundInCatalog/UpdateRequiresDependency",
 			fields: fields{
-				fakeClientOptions: []fakeClientOption{withSelfLinks(t)},
+				clientOptions: []clientfake.Option{clientfake.WithSelfLinks(t)},
 				existingOLMObjs: []runtime.Object{
 					&v1alpha1.ClusterServiceVersion{
 						ObjectMeta: metav1.ObjectMeta{
@@ -514,7 +515,7 @@ func TestSyncSubscriptions(t *testing.T) {
 			// Create test operator
 			stopCh := make(chan struct{})
 			defer func() { stopCh <- struct{}{} }()
-			o, err := NewFakeOperator(testNamespace, []string{testNamespace}, stopCh, withClientObjs(tt.fields.existingOLMObjs...), withK8sObjs(tt.fields.existingObjects...), withFakeClientOptions(tt.fields.fakeClientOptions...))
+			o, err := NewFakeOperator(testNamespace, []string{testNamespace}, stopCh, withClientObjs(tt.fields.existingOLMObjs...), withK8sObjs(tt.fields.existingObjects...), withFakeClientOptions(tt.fields.clientOptions...))
 			require.NoError(t, err)
 
 			o.reconciler = &fakes.FakeRegistryReconcilerFactory{

pkg/lib/clientfake/client_options.go

@@ -0,0 +1,53 @@
+package clientfake
+
+import (
+	"testing"
+
+	"k8s.io/apimachinery/pkg/api/meta"
+	"k8s.io/apimachinery/pkg/runtime"
+	clitesting "k8s.io/client-go/testing"
+)
+
+// Option configures a ClientsetDecorator
+type Option func(ClientsetDecorator)
+
+// WithSelfLinks returns a fakeClientOption that configures a ClientsetDecorator to write selfLinks to all OLM types on create.
+func WithSelfLinks(t *testing.T) Option {
+	return func(c ClientsetDecorator) {
+		c.PrependReactor("create", "*", func(a clitesting.Action) (bool, runtime.Object, error) {
+			ca, ok := a.(clitesting.CreateAction)
+			if !ok {
+				t.Fatalf("expected CreateAction")
+			}
+
+			obj := ca.GetObject()
+			accessor, err := meta.Accessor(obj)
+			if err != nil {
+				return false, nil, err
+			}
+			if accessor.GetSelfLink() != "" {
+				// SelfLink is already set
+				return false, nil, nil
+			}
+
+			gvr := ca.GetResource()
+			accessor.SetSelfLink(BuildSelfLink(gvr.GroupVersion().String(), gvr.Resource, accessor.GetNamespace(), accessor.GetName()))
+
+			return false, obj, nil
+		})
+	}
+}
+
+// WithNameGeneration returns a fakeK8sClientOption that configures a Clientset to write generated names to all types on create.
+func WithNameGeneration(t *testing.T) Option {
+	return func(c ClientsetDecorator) {
+		c.PrependReactor("create", "*", func(a clitesting.Action) (bool, runtime.Object, error) {
+			ca, ok := a.(clitesting.CreateAction)
+			if !ok {
+				t.Fatalf("expected CreateAction")
+			}
+
+			return false, AddSimpleGeneratedName(ca.GetObject()), nil
+		})
+	}
+}

pkg/lib/clientfake/decorator.go

@@ -0,0 +1,72 @@
+package clientfake
+
+import (
+	"k8s.io/apimachinery/pkg/runtime"
+	fake "k8s.io/client-go/kubernetes/fake"
+	"k8s.io/client-go/testing"
+)
+
+// This is used to prepend reactors to the k8s fake client. should be removed when client go is updated.
+// TODO: see if we can merge the OLM ClientsetDecorator and this one.
+
+// ClientsetDecorator defines decorator methods for a Clientset.
+type ClientsetDecorator interface {
+	// PrependReactor adds a reactor to the beginning of the chain.
+	PrependReactor(verb, resource string, reaction testing.ReactionFunc)
+}
+
+// ReactionForwardingClientsetDecorator wraps a Clientset and "forwards" Action object mutations
+// from all successful non-handling Reactors along the chain to the first handling Reactor. This is
+// is a stopgap until we can upgrade to client-go v11.0, where the behavior is the default
+// (see https://github.com/kubernetes/client-go/blob/6ee68ca5fd8355d024d02f9db0b3b667e8357a0f/testing/fake.go#L130).
+type ReactionForwardingClientsetDecorator struct {
+	fake.Clientset
+	ReactionChain []testing.Reactor // shadow embedded ReactionChain
+	actions       []testing.Action  // these may be castable to other types, but "Action" is the minimum
+}
+
+// NewReactionForwardingClientsetDecorator returns the ReactionForwardingClientsetDecorator wrapped Clientset result
+// of calling NewSimpleClientset with the given objects.
+func NewReactionForwardingClientsetDecorator(objects []runtime.Object, options ...Option) *ReactionForwardingClientsetDecorator {
+	decorator := &ReactionForwardingClientsetDecorator{
+		Clientset: *fake.NewSimpleClientset(objects...),
+	}
+
+	// Swap out the embedded ReactionChain with a Reactor that reduces over the decorator's ReactionChain.
+	decorator.ReactionChain = decorator.Clientset.ReactionChain
+	decorator.Clientset.ReactionChain = []testing.Reactor{&testing.SimpleReactor{"*", "*", decorator.reduceReactions}}
+
+	// Apply options
+	for _, option := range options {
+		option(decorator)
+	}
+
+	return decorator
+}
+
+// reduceReactions reduces over all reactions in the chain while "forwarding" Action object mutations
+// from all successful non-handling Reactors along the chain to the first handling Reactor.
+func (c *ReactionForwardingClientsetDecorator) reduceReactions(action testing.Action) (handled bool, ret runtime.Object, err error) {
+	// The embedded Client set is already locked, so there's no need to lock again
+	actionCopy := action.DeepCopy()
+	c.actions = append(c.actions, action.DeepCopy())
+	for _, reactor := range c.ReactionChain {
+		if !reactor.Handles(actionCopy) {
+			continue
+		}
+
+		handled, ret, err = reactor.React(actionCopy)
+		if !handled {
+			continue
+		}
+
+		return
+	}
+
+	return
+}
+
+// PrependReactor adds a reactor to the beginning of the chain.
+func (c *ReactionForwardingClientsetDecorator) PrependReactor(verb, resource string, reaction testing.ReactionFunc) {
+	c.ReactionChain = append([]testing.Reactor{&testing.SimpleReactor{verb, resource, reaction}}, c.ReactionChain...)
+}

pkg/lib/clientfake/meta.go

@@ -0,0 +1,45 @@
+package clientfake
+
+import (
+	"fmt"
+
+	"k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apiserver/pkg/storage/names"
+)
+
+// BuildSelfLink returns a selflink for the given group version, plural, namespace, and name.
+func BuildSelfLink(groupVersion, plural, namespace, name string) string {
+	if namespace == metav1.NamespaceAll {
+		return fmt.Sprintf("/apis/%s/%s/%s", groupVersion, plural, name)
+	}
+	return fmt.Sprintf("/apis/%s/namespaces/%s/%s/%s", groupVersion, namespace, plural, name)
+}
+
+// AddSimpleGeneratedName returns the given object with a simple generated name added to its metadata.
+// If a name already exists, there is no GenerateName field set, or there is an issue accessing the object's metadata
+// the object is returned unmodified.
+func AddSimpleGeneratedName(obj runtime.Object) runtime.Object {
+	accessor, err := meta.Accessor(obj)
+	if err != nil {
+		return obj
+	}
+	if accessor.GetName() == "" && accessor.GetGenerateName() != "" {
+		// TODO: for tests, it would be nice to be able to retrieve this name later
+		accessor.SetName(names.SimpleNameGenerator.GenerateName(accessor.GetGenerateName()))
+	}
+
+	return obj
+}
+
+// AddSimpleGeneratedNames returns the list objects with simple generated names added to their metadata.
+// If a name already exists, there is no GenerateName field set, or there is an issue accessing the object's metadata
+// the object is returned unmodified.
+func AddSimpleGeneratedNames(objs ...runtime.Object) []runtime.Object {
+	for i, obj := range objs {
+		objs[i] = AddSimpleGeneratedName(obj)
+	}
+
+	return objs
+}