reorganize ut common functions and improve test coverage

go.mod

@@ -3,7 +3,6 @@ module github.com/open-cluster-management/registration
 go 1.13
 
 require (
-	github.com/davecgh/go-spew v1.1.1
 	github.com/go-bindata/go-bindata v3.1.2+incompatible
 	github.com/onsi/ginkgo v1.11.0
 	github.com/onsi/gomega v1.8.1

pkg/helpers/helpers.go

@@ -203,7 +203,7 @@ func CleanUpManagedClusterManifests(
 	return errorhelpers.NewMultiLineAggregate(errs)
 }
 
-// CleanUpGroupFromClusterRoleBindings search all clusterrolebings for managed cluster group and remove the subject entry
+// CleanUpGroupFromClusterRoleBindings search all clusterrolebindings for managed cluster group and remove the subject entry
 // or delete the clusterrolebinding if it's the only subject.
 func CleanUpGroupFromClusterRoleBindings(
 	ctx context.Context,
@@ -248,7 +248,7 @@ func CleanUpGroupFromClusterRoleBindings(
 	return nil
 }
 
-// CleanUpGroupFromRoleBindings search all rolebings for managed cluster group and remove the subject entry
+// CleanUpGroupFromRoleBindings search all rolebindings for managed cluster group and remove the subject entry
 // or delete the rolebinding if it's the only subject.
 func CleanUpGroupFromRoleBindings(
 	ctx context.Context,

pkg/helpers/helpers_test.go

@@ -10,13 +10,14 @@ import (
 
 	clusterfake "github.com/open-cluster-management/api/client/cluster/clientset/versioned/fake"
 	clusterv1 "github.com/open-cluster-management/api/cluster/v1"
+	testinghelpers "github.com/open-cluster-management/registration/pkg/helpers/testing"
+
 	"github.com/openshift/library-go/pkg/operator/events/eventstesting"
 
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/diff"
 	fakekube "k8s.io/client-go/kubernetes/fake"
@@ -40,46 +41,46 @@ func TestUpdateStatusCondition(t *testing.T) {
 		{
 			name:               "add to empty",
 			startingConditions: []clusterv1.StatusCondition{},
-			newCondition:       newCondition("test", "True", "my-reason", "my-message", nil),
+			newCondition:       testinghelpers.NewManagedClusterCondition("test", "True", "my-reason", "my-message", nil),
 			expextedUpdated:    true,
-			expectedConditions: []clusterv1.StatusCondition{newCondition("test", "True", "my-reason", "my-message", nil)},
+			expectedConditions: []clusterv1.StatusCondition{testinghelpers.NewManagedClusterCondition("test", "True", "my-reason", "my-message", nil)},
 		},
 		{
 			name: "add to non-conflicting",
 			startingConditions: []clusterv1.StatusCondition{
-				newCondition("two", "True", "my-reason", "my-message", nil),
+				testinghelpers.NewManagedClusterCondition("two", "True", "my-reason", "my-message", nil),
 			},
-			newCondition:    newCondition("one", "True", "my-reason", "my-message", nil),
+			newCondition:    testinghelpers.NewManagedClusterCondition("one", "True", "my-reason", "my-message", nil),
 			expextedUpdated: true,
 			expectedConditions: []clusterv1.StatusCondition{
-				newCondition("two", "True", "my-reason", "my-message", nil),
-				newCondition("one", "True", "my-reason", "my-message", nil),
+				testinghelpers.NewManagedClusterCondition("two", "True", "my-reason", "my-message", nil),
+				testinghelpers.NewManagedClusterCondition("one", "True", "my-reason", "my-message", nil),
 			},
 		},
 		{
 			name: "change existing status",
 			startingConditions: []clusterv1.StatusCondition{
-				newCondition("two", "True", "my-reason", "my-message", nil),
-				newCondition("one", "True", "my-reason", "my-message", nil),
+				testinghelpers.NewManagedClusterCondition("two", "True", "my-reason", "my-message", nil),
+				testinghelpers.NewManagedClusterCondition("one", "True", "my-reason", "my-message", nil),
 			},
-			newCondition:    newCondition("one", "False", "my-different-reason", "my-othermessage", nil),
+			newCondition:    testinghelpers.NewManagedClusterCondition("one", "False", "my-different-reason", "my-othermessage", nil),
 			expextedUpdated: true,
 			expectedConditions: []clusterv1.StatusCondition{
-				newCondition("two", "True", "my-reason", "my-message", nil),
-				newCondition("one", "False", "my-different-reason", "my-othermessage", nil),
+				testinghelpers.NewManagedClusterCondition("two", "True", "my-reason", "my-message", nil),
+				testinghelpers.NewManagedClusterCondition("one", "False", "my-different-reason", "my-othermessage", nil),
 			},
 		},
 		{
 			name: "leave existing transition time",
 			startingConditions: []clusterv1.StatusCondition{
-				newCondition("two", "True", "my-reason", "my-message", nil),
-				newCondition("one", "True", "my-reason", "my-message", &beforeish),
+				testinghelpers.NewManagedClusterCondition("two", "True", "my-reason", "my-message", nil),
+				testinghelpers.NewManagedClusterCondition("one", "True", "my-reason", "my-message", &beforeish),
 			},
-			newCondition:    newCondition("one", "True", "my-reason", "my-message", &afterish),
+			newCondition:    testinghelpers.NewManagedClusterCondition("one", "True", "my-reason", "my-message", &afterish),
 			expextedUpdated: false,
 			expectedConditions: []clusterv1.StatusCondition{
-				newCondition("two", "True", "my-reason", "my-message", nil),
-				newCondition("one", "True", "my-reason", "my-message", &beforeish),
+				testinghelpers.NewManagedClusterCondition("two", "True", "my-reason", "my-message", nil),
+				testinghelpers.NewManagedClusterCondition("one", "True", "my-reason", "my-message", &beforeish),
 			},
 		},
 	}
@@ -159,11 +160,15 @@ func TestIsValidHTTPSURL(t *testing.T) {
 
 func TestCleanUpManagedClusterManifests(t *testing.T) {
 	applyFiles := map[string]runtime.Object{
-		"namespace":          newUnstructured("v1", "Namespace", "", "n1"),
-		"clusterrole":        newUnstructured("rbac.authorization.k8s.io/v1", "ClusterRole", "", "cr1"),
-		"clusterrolebinding": newUnstructured("rbac.authorization.k8s.io/v1", "ClusterRoleBinding", "", "crb1"),
-		"role":               newUnstructured("rbac.authorization.k8s.io/v1", "Role", "n1", "r1"),
-		"rolebinding":        newUnstructured("rbac.authorization.k8s.io/v1", "RoleBinding", "n1", "rb1"),
+		"namespace":          testinghelpers.NewUnstructuredObj("v1", "Namespace", "", "n1"),
+		"clusterrole":        testinghelpers.NewUnstructuredObj("rbac.authorization.k8s.io/v1", "ClusterRole", "", "cr1"),
+		"clusterrolebinding": testinghelpers.NewUnstructuredObj("rbac.authorization.k8s.io/v1", "ClusterRoleBinding", "", "crb1"),
+		"role":               testinghelpers.NewUnstructuredObj("rbac.authorization.k8s.io/v1", "Role", "n1", "r1"),
+		"rolebinding":        testinghelpers.NewUnstructuredObj("rbac.authorization.k8s.io/v1", "RoleBinding", "n1", "rb1"),
+	}
+	expectedActions := []string{}
+	for i := 0; i < len(applyFiles); i++ {
+		expectedActions = append(expectedActions, "delete")
 	}
 	cases := []struct {
 		name            string
@@ -183,7 +188,7 @@ func TestCleanUpManagedClusterManifests(t *testing.T) {
 			},
 			applyFiles: applyFiles,
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				assertDeleteActions(t, len(applyFiles), actions)
+				testinghelpers.AssertActions(t, actions, expectedActions...)
 			},
 		},
 		{
@@ -191,19 +196,15 @@ func TestCleanUpManagedClusterManifests(t *testing.T) {
 			applyObject: []runtime.Object{},
 			applyFiles:  applyFiles,
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				assertDeleteActions(t, len(applyFiles), actions)
+				testinghelpers.AssertActions(t, actions, expectedActions...)
 			},
 		},
 		{
-			name:        "unhandled types",
-			applyObject: []runtime.Object{},
-			applyFiles:  map[string]runtime.Object{"secret": newUnstructured("v1", "Secret", "n1", "s1")},
-			expectedErr: "unhandled type *v1.Secret",
-			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				if len(actions) != 0 {
-					t.Errorf("expected no actions, but %v", actions)
-				}
-			},
+			name:            "unhandled types",
+			applyObject:     []runtime.Object{},
+			applyFiles:      map[string]runtime.Object{"secret": testinghelpers.NewUnstructuredObj("v1", "Secret", "n1", "s1")},
+			expectedErr:     "unhandled type *v1.Secret",
+			validateActions: testinghelpers.AssertNoActions,
 		},
 	}
 	for _, c := range cases {
@@ -221,19 +222,7 @@ func TestCleanUpManagedClusterManifests(t *testing.T) {
 				},
 				getApplyFileNames(c.applyFiles)...,
 			)
-			if len(c.expectedErr) > 0 && cleanUpErr == nil {
-				t.Errorf("expected %q error", c.expectedErr)
-				return
-			}
-			if len(c.expectedErr) > 0 && cleanUpErr != nil && cleanUpErr.Error() != c.expectedErr {
-				t.Errorf("expected %q error, got %q", c.expectedErr, cleanUpErr.Error())
-				return
-			}
-			if len(c.expectedErr) == 0 && cleanUpErr != nil {
-				t.Errorf("unexpected err: %v", cleanUpErr)
-				return
-			}
-
+			testinghelpers.AssertError(t, cleanUpErr, c.expectedErr)
 			c.validateActions(t, kubeClient.Actions())
 		})
 	}
@@ -366,44 +355,6 @@ func TestCleanUpGroupFromRoleBindings(t *testing.T) {
 	}
 }
 
-func assertDeleteActions(t *testing.T, actionCounts int, actions []clienttesting.Action) {
-	if len(actions) != actionCounts {
-		t.Errorf("expected %d actions, but %v", actionCounts, actions)
-	}
-	for _, action := range actions {
-		if action.GetVerb() != "delete" {
-			t.Errorf("expected delete actions, but %v", action)
-		}
-	}
-}
-
-func newCondition(name, status, reason, message string, lastTransition *metav1.Time) clusterv1.StatusCondition {
-	ret := clusterv1.StatusCondition{
-		Type:    name,
-		Status:  metav1.ConditionStatus(status),
-		Reason:  reason,
-		Message: message,
-	}
-	if lastTransition != nil {
-		ret.LastTransitionTime = *lastTransition
-	}
-	return ret
-}
-
-func newUnstructured(apiVersion, kind, namespace, name string) *unstructured.Unstructured {
-	object := &unstructured.Unstructured{
-		Object: map[string]interface{}{
-			"apiVersion": apiVersion,
-			"kind":       kind,
-			"metadata": map[string]interface{}{
-				"namespace": namespace,
-				"name":      name,
-			},
-		},
-	}
-	return object
-}
-
 func getApplyFileNames(applyFiles map[string]runtime.Object) []string {
 	keys := []string{}
 	for key := range applyFiles {

pkg/helpers/testing/assertion.go

@@ -0,0 +1,193 @@
+package testing
+
+import (
+	"reflect"
+	"testing"
+
+	clusterv1 "github.com/open-cluster-management/api/cluster/v1"
+
+	authorizationv1 "k8s.io/api/authorization/v1"
+	certv1beta1 "k8s.io/api/certificates/v1beta1"
+	coordinationv1 "k8s.io/api/coordination/v1"
+	"k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	clienttesting "k8s.io/client-go/testing"
+	"k8s.io/utils/diff"
+)
+
+// AssertError asserts the actual error representation is the same with the expected,
+// if the expected error representation is empty, the actual should be nil
+func AssertError(t *testing.T, actual error, expectedErr string) {
+	if len(expectedErr) > 0 && actual == nil {
+		t.Errorf("expected %q error", expectedErr)
+		return
+	}
+	if len(expectedErr) > 0 && actual != nil && actual.Error() != expectedErr {
+		t.Errorf("expected %q error, but got %q", expectedErr, actual.Error())
+		return
+	}
+	if len(expectedErr) == 0 && actual != nil {
+		t.Errorf("unexpected err: %v", actual)
+		return
+	}
+}
+
+// AssertActions asserts the actual actions have the expected action verb
+func AssertActions(t *testing.T, actualActions []clienttesting.Action, expectedVerbs ...string) {
+	if len(actualActions) != len(expectedVerbs) {
+		t.Errorf("expected %d call but got: %#v", len(expectedVerbs), actualActions)
+	}
+	for i, expected := range expectedVerbs {
+		if actualActions[i].GetVerb() != expected {
+			t.Errorf("expected %s action but got: %#v", expected, actualActions[i])
+		}
+	}
+}
+
+// AssertNoActions asserts no actions are happened
+func AssertNoActions(t *testing.T, actualActions []clienttesting.Action) {
+	AssertActions(t, actualActions)
+}
+
+// AssertUpdateActions asserts the actions are get-then-update action
+func AssertUpdateActions(t *testing.T, actions []clienttesting.Action) {
+	for i := 0; i < len(actions); i = i + 2 {
+		if actions[i].GetVerb() != "get" {
+			t.Errorf("expected action %d is get, but %v", i, actions[i])
+		}
+		if actions[i+1].GetVerb() != "update" {
+			t.Errorf("expected action %d is update, but %v", i, actions[i+1])
+		}
+	}
+}
+
+// AssertNoMoreUpdates asserts only one update action in given actions
+func AssertNoMoreUpdates(t *testing.T, actions []clienttesting.Action) {
+	updateActions := 0
+	for _, action := range actions {
+		if action.GetVerb() == "update" {
+			updateActions++
+		}
+	}
+	if updateActions != 1 {
+		t.Errorf("expected there is only one update action, but failed")
+	}
+}
+
+// AssertFinalizers asserts the given runtime object has the expected finalizers
+func AssertFinalizers(t *testing.T, obj runtime.Object, finalizers []string) {
+	accessor, _ := meta.Accessor(obj)
+	actual := accessor.GetFinalizers()
+	if len(actual) == 0 && len(finalizers) == 0 {
+		return
+	}
+	if !reflect.DeepEqual(actual, finalizers) {
+		t.Fatal(diff.ObjectDiff(actual, finalizers))
+	}
+}
+
+// AssertManagedClusterCondition asserts the actual managed cluster conditions has
+// the expected condition
+func AssertManagedClusterCondition(
+	t *testing.T,
+	actualConditions []clusterv1.StatusCondition,
+	expectedCondition clusterv1.StatusCondition) {
+	var cond *clusterv1.StatusCondition
+	for i := range actualConditions {
+		condition := actualConditions[i]
+		if condition.Type == expectedCondition.Type {
+			cond = &condition
+			break
+		}
+	}
+	if cond == nil {
+		t.Errorf("expected condition %s but got: %s", expectedCondition.Type, cond.Type)
+	}
+	if cond.Status != expectedCondition.Status {
+		t.Errorf("expected status %s but got: %s", expectedCondition.Status, cond.Status)
+	}
+	if cond.Reason != expectedCondition.Reason {
+		t.Errorf("expected reason %s but got: %s", expectedCondition.Reason, cond.Reason)
+	}
+	if cond.Message != expectedCondition.Message {
+		t.Errorf("expected message %s but got: %s", expectedCondition.Message, cond.Message)
+	}
+}
+
+// AssertManagedClusterClientConfigs asserts the actual managed cluster client configs are the
+// same wiht the expected
+func AssertManagedClusterClientConfigs(t *testing.T, actual, expected []clusterv1.ClientConfig) {
+	if len(actual) == 0 && len(expected) == 0 {
+		return
+	}
+	if !reflect.DeepEqual(actual, expected) {
+		t.Errorf("expected client configs %#v but got: %#v", expected, actual)
+	}
+}
+
+// AssertManagedClusterStatus sserts the actual managed cluster status is the same
+// wiht the expected
+func AssertManagedClusterStatus(t *testing.T, actual, expected clusterv1.ManagedClusterStatus) {
+	if !reflect.DeepEqual(actual.Version, expected.Version) {
+		t.Errorf("expected version %#v but got: %#v", expected.Version, actual.Version)
+	}
+	if !reflect.DeepEqual(actual.Capacity["cpu"], expected.Capacity["cpu"]) {
+		t.Errorf("expected cpu capacity %#v but got: %#v", expected.Capacity["cpu"], actual.Capacity["cpu"])
+	}
+	if !reflect.DeepEqual(actual.Capacity["memory"], expected.Capacity["memory"]) {
+		t.Errorf("expected memory capacity %#v but got: %#v", expected.Capacity["memory"], actual.Capacity["memory"])
+	}
+	if !reflect.DeepEqual(actual.Allocatable["cpu"], expected.Allocatable["cpu"]) {
+		t.Errorf("expected cpu allocatable %#v but got: %#v", expected.Allocatable["cpu"], actual.Allocatable["cpu"])
+	}
+	if !reflect.DeepEqual(actual.Allocatable["memory"], expected.Allocatable["memory"]) {
+		t.Errorf("expected memory alocatabel %#v but got: %#v", expected.Allocatable["memory"], actual.Allocatable["memory"])
+	}
+}
+
+// AssertSubjectAccessReviewObj asserts the given runtime object is the
+// authorization SubjectAccessReview object
+func AssertSubjectAccessReviewObj(t *testing.T, actual runtime.Object) {
+	_, ok := actual.(*authorizationv1.SubjectAccessReview)
+	if !ok {
+		t.Errorf("expected subjectaccessreview object, but got: %#v", actual)
+	}
+}
+
+// AssertCSRCondition asserts the actual csr conditions has the expected condition
+func AssertCSRCondition(
+	t *testing.T,
+	actualConditions []certv1beta1.CertificateSigningRequestCondition,
+	expectedCondition certv1beta1.CertificateSigningRequestCondition) {
+	var cond *certv1beta1.CertificateSigningRequestCondition
+	for i := range actualConditions {
+		condition := actualConditions[i]
+		if condition.Type == expectedCondition.Type {
+			cond = &condition
+			break
+		}
+	}
+	if cond == nil {
+		t.Errorf("expected condition %s but got: %s", expectedCondition.Type, cond.Type)
+	}
+	if cond.Reason != expectedCondition.Reason {
+		t.Errorf("expected reason %s but got: %s", expectedCondition.Reason, cond.Reason)
+	}
+	if cond.Message != expectedCondition.Message {
+		t.Errorf("expected message %s but got: %s", expectedCondition.Message, cond.Message)
+	}
+}
+
+// AssertLeaseUpdated asserts the lease obj is updated
+func AssertLeaseUpdated(t *testing.T, lease, lastLease *coordinationv1.Lease) {
+	if lease == nil || lastLease == nil {
+		t.Errorf("expected lease objects are not nil, but failed")
+	}
+	if (lease.Namespace != lastLease.Namespace) || (lease.Name != lastLease.Name) {
+		t.Errorf("expected two lease objects are same lease, but failed")
+	}
+	if !lease.Spec.RenewTime.BeforeTime(&metav1.Time{Time: lastLease.Spec.RenewTime.Time}) {
+		t.Errorf("expected lease updated, but failed")
+	}
+}

pkg/helpers/testing/testinghelpers.go

@@ -1,16 +1,45 @@
 package testing
 
 import (
+	"crypto/ecdsa"
+	"crypto/elliptic"
+	cryptorand "crypto/rand"
+	"crypto/rsa"
+	"crypto/x509"
+	"crypto/x509/pkix"
+	"encoding/pem"
+	"fmt"
+	"io/ioutil"
+	"math/big"
+	"math/rand"
+	"net"
 	"testing"
 	"time"
 
-	"k8s.io/client-go/util/workqueue"
-
 	clusterv1 "github.com/open-cluster-management/api/cluster/v1"
 	workapiv1 "github.com/open-cluster-management/api/work/v1"
+
 	"github.com/openshift/library-go/pkg/operator/events"
 	"github.com/openshift/library-go/pkg/operator/events/eventstesting"
+
+	certv1beta1 "k8s.io/api/certificates/v1beta1"
+	coordv1 "k8s.io/api/coordination/v1"
+	corev1 "k8s.io/api/core/v1"
+	rbacv1 "k8s.io/api/rbac/v1"
+	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	kubeversion "k8s.io/client-go/pkg/version"
+	"k8s.io/client-go/tools/clientcmd"
+	clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
+	certutil "k8s.io/client-go/util/cert"
+	"k8s.io/client-go/util/keyutil"
+	"k8s.io/client-go/util/workqueue"
+)
+
+const (
+	TestLeaseDurationSeconds int32 = 1
+	TestManagedClusterName         = "testmanagedcluster"
 )
 
 type FakeSyncContext struct {
@@ -19,6 +48,10 @@ type FakeSyncContext struct {
 	queue     workqueue.RateLimitingInterface
 }
 
+func (f FakeSyncContext) Queue() workqueue.RateLimitingInterface { return f.queue }
+func (f FakeSyncContext) QueueKey() string                       { return f.spokeName }
+func (f FakeSyncContext) Recorder() events.Recorder              { return f.recorder }
+
 func NewFakeSyncContext(t *testing.T, clusterName string) *FakeSyncContext {
 	return &FakeSyncContext{
 		spokeName: clusterName,
@@ -27,9 +60,128 @@ func NewFakeSyncContext(t *testing.T, clusterName string) *FakeSyncContext {
 	}
 }
 
-func (f FakeSyncContext) Queue() workqueue.RateLimitingInterface { return f.queue }
-func (f FakeSyncContext) QueueKey() string                       { return f.spokeName }
-func (f FakeSyncContext) Recorder() events.Recorder              { return f.recorder }
+func NewManagedCluster() *clusterv1.ManagedCluster {
+	return &clusterv1.ManagedCluster{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: TestManagedClusterName,
+		},
+	}
+}
+
+func NewAcceptingManagedCluster() *clusterv1.ManagedCluster {
+	managedCluster := NewManagedCluster()
+	managedCluster.Finalizers = []string{"cluster.open-cluster-management.io/api-resource-cleanup"}
+	managedCluster.Spec.HubAcceptsClient = true
+	return managedCluster
+}
+
+func NewAcceptedManagedCluster() *clusterv1.ManagedCluster {
+	managedCluster := NewAcceptingManagedCluster()
+	acceptedCondtion := NewManagedClusterCondition(
+		clusterv1.ManagedClusterConditionHubAccepted,
+		"True",
+		"HubClusterAdminAccepted",
+		"Accepted by hub cluster admin",
+		nil,
+	)
+	managedCluster.Status.Conditions = append(managedCluster.Status.Conditions, acceptedCondtion)
+	managedCluster.Spec.LeaseDurationSeconds = TestLeaseDurationSeconds
+	return managedCluster
+}
+
+func NewAvailableManagedCluster() *clusterv1.ManagedCluster {
+	managedCluster := NewAcceptedManagedCluster()
+	availableCondtion := NewManagedClusterCondition(
+		clusterv1.ManagedClusterConditionAvailable,
+		"True",
+		"ManagedClusterAvailable",
+		"Managed cluster is available",
+		nil,
+	)
+	managedCluster.Status.Conditions = append(managedCluster.Status.Conditions, availableCondtion)
+	return managedCluster
+}
+
+func NewJoinedManagedCluster() *clusterv1.ManagedCluster {
+	managedCluster := NewAcceptedManagedCluster()
+	joinedCondtion := NewManagedClusterCondition(
+		clusterv1.ManagedClusterConditionJoined,
+		"True",
+		"ManagedClusterJoined",
+		"Managed cluster joined",
+		nil,
+	)
+	managedCluster.Status.Conditions = append(managedCluster.Status.Conditions, joinedCondtion)
+	return managedCluster
+}
+
+func NewManagedClusterWithStatus(capacity, allocatable corev1.ResourceList) *clusterv1.ManagedCluster {
+	managedCluster := NewJoinedManagedCluster()
+	managedCluster.Status.Capacity = clusterv1.ResourceList{
+		"cpu":    capacity.Cpu().DeepCopy(),
+		"memory": capacity.Memory().DeepCopy(),
+	}
+	managedCluster.Status.Allocatable = clusterv1.ResourceList{
+		"cpu":    allocatable.Cpu().DeepCopy(),
+		"memory": allocatable.Memory().DeepCopy(),
+	}
+	managedCluster.Status.Version = clusterv1.ManagedClusterVersion{
+		Kubernetes: kubeversion.Get().GitVersion,
+	}
+	return managedCluster
+}
+
+func NewDeniedManagedCluster() *clusterv1.ManagedCluster {
+	managedCluster := NewAcceptedManagedCluster()
+	managedCluster.Spec.HubAcceptsClient = false
+	return managedCluster
+}
+
+func NewDeletingManagedCluster() *clusterv1.ManagedCluster {
+	now := metav1.Now()
+	return &clusterv1.ManagedCluster{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:              TestManagedClusterName,
+			DeletionTimestamp: &now,
+			Finalizers:        []string{"cluster.open-cluster-management.io/api-resource-cleanup"},
+		},
+	}
+}
+
+func NewManagedClusterCondition(name, status, reason, message string, lastTransition *metav1.Time) clusterv1.StatusCondition {
+	ret := clusterv1.StatusCondition{
+		Type:    name,
+		Status:  metav1.ConditionStatus(status),
+		Reason:  reason,
+		Message: message,
+	}
+	if lastTransition != nil {
+		ret.LastTransitionTime = *lastTransition
+	}
+	return ret
+}
+
+func NewManagedClusterLease(renewTime time.Time) *coordv1.Lease {
+	return &coordv1.Lease{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      fmt.Sprintf("cluster-lease-%s", TestManagedClusterName),
+			Namespace: TestManagedClusterName,
+		},
+		Spec: coordv1.LeaseSpec{
+			RenewTime: &metav1.MicroTime{Time: renewTime},
+		},
+	}
+}
+
+func NewNamespace(name string, terminated bool) *corev1.Namespace {
+	namespace := &corev1.Namespace{}
+	namespace.Name = name
+	if terminated {
+		now := metav1.Now()
+		namespace.DeletionTimestamp = &now
+	}
+	return namespace
+}
 
 func NewManifestWork(namespace, name string, finalizers []string, deletionTimestamp *metav1.Time) *workapiv1.ManifestWork {
 	work := &workapiv1.ManifestWork{
@@ -40,21 +192,242 @@ func NewManifestWork(namespace, name string, finalizers []string, deletionTimest
 			DeletionTimestamp: deletionTimestamp,
 		},
 	}
-
 	return work
 }
 
-func NewDeletionTimestamp(offset time.Duration) *metav1.Time {
-	return &metav1.Time{
-		Time: metav1.Now().Add(offset),
+func NewRole(namespace, name string, finalizers []string, terminated bool) *rbacv1.Role {
+	role := &rbacv1.Role{}
+	role.Namespace = namespace
+	role.Name = name
+	role.Finalizers = finalizers
+	if terminated {
+		now := metav1.Now()
+		role.DeletionTimestamp = &now
+	}
+	return role
+}
+
+func NewRoleBinding(namespace, name string, finalizers []string, terminated bool) *rbacv1.RoleBinding {
+	rolebinding := &rbacv1.RoleBinding{}
+	rolebinding.Namespace = namespace
+	rolebinding.Name = name
+	rolebinding.Finalizers = finalizers
+	if terminated {
+		now := metav1.Now()
+		rolebinding.DeletionTimestamp = &now
+	}
+	return rolebinding
+}
+
+func NewResourceList(cpu, mem int) corev1.ResourceList {
+	return corev1.ResourceList{
+		corev1.ResourceCPU:    *resource.NewQuantity(int64(cpu), resource.DecimalExponent),
+		corev1.ResourceMemory: *resource.NewQuantity(int64(1024*1024*mem), resource.BinarySI),
 	}
 }
 
-func NewManagedCluster(name string, deletionTime *metav1.Time) *clusterv1.ManagedCluster {
-	return &clusterv1.ManagedCluster{
+func NewNode(name string, capacity, allocatable corev1.ResourceList) *corev1.Node {
+	return &corev1.Node{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:              name,
-			DeletionTimestamp: deletionTime,
+			Name: name,
+		},
+		Status: corev1.NodeStatus{
+			Capacity:    capacity,
+			Allocatable: allocatable,
 		},
 	}
 }
+
+func NewUnstructuredObj(apiVersion, kind, namespace, name string) *unstructured.Unstructured {
+	return &unstructured.Unstructured{
+		Object: map[string]interface{}{
+			"apiVersion": apiVersion,
+			"kind":       kind,
+			"metadata": map[string]interface{}{
+				"namespace": namespace,
+				"name":      name,
+			},
+		},
+	}
+}
+
+type CSRHolder struct {
+	Name         string
+	Labels       map[string]string
+	SignerName   *string
+	CN           string
+	Orgs         []string
+	Username     string
+	ReqBlockType string
+}
+
+func NewCSR(holder CSRHolder) *certv1beta1.CertificateSigningRequest {
+	insecureRand := rand.New(rand.NewSource(0))
+	pk, err := ecdsa.GenerateKey(elliptic.P256(), insecureRand)
+	if err != nil {
+		panic(err)
+	}
+	csrb, err := x509.CreateCertificateRequest(insecureRand, &x509.CertificateRequest{
+		Subject: pkix.Name{
+			CommonName:   holder.CN,
+			Organization: holder.Orgs,
+		},
+		DNSNames:       []string{},
+		EmailAddresses: []string{},
+		IPAddresses:    []net.IP{},
+	}, pk)
+	if err != nil {
+		panic(err)
+	}
+	return &certv1beta1.CertificateSigningRequest{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:         holder.Name,
+			GenerateName: "csr-",
+			Labels:       holder.Labels,
+		},
+		Spec: certv1beta1.CertificateSigningRequestSpec{
+			Username:   holder.Username,
+			Usages:     []certv1beta1.KeyUsage{},
+			SignerName: holder.SignerName,
+			Request:    pem.EncodeToMemory(&pem.Block{Type: holder.ReqBlockType, Bytes: csrb}),
+		},
+	}
+}
+
+func NewDeniedCSR(holder CSRHolder) *certv1beta1.CertificateSigningRequest {
+	csr := NewCSR(holder)
+	csr.Status.Conditions = append(csr.Status.Conditions, certv1beta1.CertificateSigningRequestCondition{
+		Type: certv1beta1.CertificateDenied,
+	})
+	return csr
+}
+
+func NewApprovedCSR(holder CSRHolder) *certv1beta1.CertificateSigningRequest {
+	csr := NewCSR(holder)
+	csr.Status.Conditions = append(csr.Status.Conditions, certv1beta1.CertificateSigningRequestCondition{
+		Type: certv1beta1.CertificateApproved,
+	})
+	return csr
+}
+
+func NewKubeconfig(key, cert []byte) []byte {
+	var clientKey, clientCertificate string
+	var clientKeyData, clientCertificateData []byte
+	if key != nil {
+		clientKeyData = key
+	} else {
+		clientKey = "tls.key"
+	}
+	if cert != nil {
+		clientCertificateData = cert
+	} else {
+		clientCertificate = "tls.crt"
+	}
+
+	kubeconfig := clientcmdapi.Config{
+		Clusters: map[string]*clientcmdapi.Cluster{"default-cluster": {
+			Server:                "https://127.0.0.1:6001",
+			InsecureSkipTLSVerify: true,
+		}},
+		AuthInfos: map[string]*clientcmdapi.AuthInfo{"default-auth": {
+			ClientCertificate:     clientCertificate,
+			ClientCertificateData: clientCertificateData,
+			ClientKey:             clientKey,
+			ClientKeyData:         clientKeyData,
+		}},
+		Contexts: map[string]*clientcmdapi.Context{"default-context": {
+			Cluster:   "default-cluster",
+			AuthInfo:  "default-auth",
+			Namespace: "default",
+		}},
+		CurrentContext: "default-context",
+	}
+
+	kubeconfigData, err := clientcmd.Write(kubeconfig)
+	if err != nil {
+		panic(err)
+	}
+	return kubeconfigData
+}
+
+type TestCert struct {
+	Cert []byte
+	Key  []byte
+}
+
+func NewHubKubeconfigSecret(namespace, name, resourceVersion string, cert *TestCert, data map[string][]byte) *corev1.Secret {
+	secret := &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace:       namespace,
+			Name:            name,
+			ResourceVersion: resourceVersion,
+		},
+		Data: data,
+	}
+	if cert != nil && cert.Cert != nil {
+		secret.Data["tls.crt"] = cert.Cert
+	}
+	if cert != nil && cert.Key != nil {
+		secret.Data["tls.key"] = cert.Key
+	}
+	return secret
+}
+
+func NewTestCert(commonName string, duration time.Duration) *TestCert {
+	caKey, err := rsa.GenerateKey(cryptorand.Reader, 2048)
+	if err != nil {
+		panic(err)
+	}
+
+	caCert, err := certutil.NewSelfSignedCACert(certutil.Config{CommonName: "open-cluster-management.io"}, caKey)
+	if err != nil {
+		panic(err)
+	}
+
+	key, err := rsa.GenerateKey(cryptorand.Reader, 2048)
+	if err != nil {
+		panic(err)
+	}
+
+	certDERBytes, err := x509.CreateCertificate(
+		cryptorand.Reader,
+		&x509.Certificate{
+			Subject: pkix.Name{
+				CommonName: commonName,
+			},
+			SerialNumber: big.NewInt(1),
+			NotBefore:    caCert.NotBefore,
+			NotAfter:     time.Now().Add(duration).UTC(),
+			KeyUsage:     x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
+			ExtKeyUsage:  []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth},
+		},
+		caCert,
+		key.Public(),
+		caKey,
+	)
+	if err != nil {
+		panic(err)
+	}
+
+	cert, err := x509.ParseCertificate(certDERBytes)
+	if err != nil {
+		panic(err)
+	}
+
+	return &TestCert{
+		Cert: pem.EncodeToMemory(&pem.Block{
+			Type:  certutil.CertificateBlockType,
+			Bytes: cert.Raw,
+		}),
+		Key: pem.EncodeToMemory(&pem.Block{
+			Type:  keyutil.RSAPrivateKeyBlockType,
+			Bytes: x509.MarshalPKCS1PrivateKey(key),
+		}),
+	}
+}
+
+func WriteFile(filename string, data []byte) {
+	if err := ioutil.WriteFile(filename, data, 0644); err != nil {
+		panic(err)
+	}
+}

pkg/hub/csr/controller_test.go

@@ -2,30 +2,30 @@ package csr
 
 import (
 	"context"
-	"crypto/ecdsa"
-	"crypto/elliptic"
-	"crypto/x509"
-	"crypto/x509/pkix"
-	"encoding/pem"
-	"math/rand"
-	"net"
 	"testing"
 
 	testinghelpers "github.com/open-cluster-management/registration/pkg/helpers/testing"
+
 	"github.com/openshift/library-go/pkg/operator/events/eventstesting"
+
 	authorizationv1 "k8s.io/api/authorization/v1"
 	certificatesv1beta1 "k8s.io/api/certificates/v1beta1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	kubefake "k8s.io/client-go/kubernetes/fake"
 	clienttesting "k8s.io/client-go/testing"
 )
 
-const testCSRName = "test_csr"
-
 var (
-	labels     = map[string]string{"open-cluster-management.io/cluster-name": "managedcluster1"}
 	signerName = certificatesv1beta1.KubeAPIServerClientSignerName
+	validCSR   = testinghelpers.CSRHolder{
+		Name:         "testcsr",
+		Labels:       map[string]string{"open-cluster-management.io/cluster-name": "managedcluster1"},
+		SignerName:   &signerName,
+		CN:           "system:open-cluster-management:managedcluster1:spokeagent1",
+		Orgs:         []string{"system:open-cluster-management:managedcluster1"},
+		Username:     "system:open-cluster-management:managedcluster1:spokeagent1",
+		ReqBlockType: "CERTIFICATE REQUEST",
+	}
 )
 
 func TestSync(t *testing.T) {
@@ -34,51 +34,64 @@ func TestSync(t *testing.T) {
 		startingCSRs         []runtime.Object
 		autoApprovingAllowed bool
 		validateActions      func(t *testing.T, actions []clienttesting.Action)
-		expectedErr          string
 	}{
 		{
 			name:         "sync a deleted csr",
 			startingCSRs: []runtime.Object{},
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				assertActions(t, actions, "get")
+				testinghelpers.AssertActions(t, actions, "get")
 			},
 		},
 		{
 			name:         "sync a denied csr",
-			startingCSRs: []runtime.Object{newDeniedCSR()},
+			startingCSRs: []runtime.Object{testinghelpers.NewDeniedCSR(validCSR)},
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				assertActions(t, actions, "get")
+				testinghelpers.AssertActions(t, actions, "get")
 			},
 		},
 		{
 			name:         "sync an approved csr",
-			startingCSRs: []runtime.Object{newApprovedCSR()},
+			startingCSRs: []runtime.Object{testinghelpers.NewApprovedCSR(validCSR)},
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				assertActions(t, actions, "get")
+				testinghelpers.AssertActions(t, actions, "get")
 			},
 		},
 		{
-			name:         "sync an invalid csr",
-			startingCSRs: []runtime.Object{newInvalidCSR()},
+			name: "sync an invalid csr",
+			startingCSRs: []runtime.Object{testinghelpers.NewCSR(testinghelpers.CSRHolder{
+				Name:         validCSR.Name,
+				Labels:       validCSR.Labels,
+				SignerName:   validCSR.SignerName,
+				CN:           "system:open-cluster-management:managedcluster1:invalidagent",
+				Orgs:         validCSR.Orgs,
+				Username:     validCSR.Username,
+				ReqBlockType: validCSR.ReqBlockType,
+			})},
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				assertActions(t, actions, "get")
+				testinghelpers.AssertActions(t, actions, "get")
 			},
 		},
 		{
 			name:         "deny an auto approving csr",
-			startingCSRs: []runtime.Object{newRenewalCSR()},
+			startingCSRs: []runtime.Object{testinghelpers.NewCSR(validCSR)},
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				assertActions(t, actions, "get", "create")
-				assertSubjectAccessReviewCreated(t, actions[1].(clienttesting.CreateActionImpl).Object)
+				testinghelpers.AssertActions(t, actions, "get", "create")
+				testinghelpers.AssertSubjectAccessReviewObj(t, actions[1].(clienttesting.CreateActionImpl).Object)
 			},
 		},
 		{
 			name:                 "allow an auto approving csr",
-			startingCSRs:         []runtime.Object{newRenewalCSR()},
+			startingCSRs:         []runtime.Object{testinghelpers.NewCSR(validCSR)},
 			autoApprovingAllowed: true,
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				assertActions(t, actions, "get", "create", "update")
-				assertCondition(t, actions[2].(clienttesting.UpdateActionImpl).Object, certificatesv1beta1.CertificateApproved, "AutoApprovedByHubCSRApprovingController")
+				expectedCondition := certificatesv1beta1.CertificateSigningRequestCondition{
+					Type:    certificatesv1beta1.CertificateApproved,
+					Reason:  "AutoApprovedByHubCSRApprovingController",
+					Message: "Auto approving Managed cluster agent certificate after SubjectAccessReview.",
+				}
+				testinghelpers.AssertActions(t, actions, "get", "create", "update")
+				actual := actions[2].(clienttesting.UpdateActionImpl).Object
+				testinghelpers.AssertCSRCondition(t, actual.(*certificatesv1beta1.CertificateSigningRequest).Status.Conditions, expectedCondition)
 			},
 		},
 	}
@@ -99,15 +112,7 @@ func TestSync(t *testing.T) {
 			)
 
 			ctrl := &csrApprovingController{kubeClient, eventstesting.NewTestingEventRecorder(t)}
-			syncErr := ctrl.sync(context.TODO(), testinghelpers.NewFakeSyncContext(t, testCSRName))
-			if len(c.expectedErr) > 0 && syncErr == nil {
-				t.Errorf("expected %q error", c.expectedErr)
-				return
-			}
-			if len(c.expectedErr) > 0 && syncErr != nil && syncErr.Error() != c.expectedErr {
-				t.Errorf("expected %q error, got %q", c.expectedErr, syncErr.Error())
-				return
-			}
+			syncErr := ctrl.sync(context.TODO(), testinghelpers.NewFakeSyncContext(t, validCSR.Name))
 			if syncErr != nil {
 				t.Errorf("unexpected err: %v", syncErr)
 			}
@@ -122,172 +127,99 @@ func TestIsSpokeClusterClientCertRenewal(t *testing.T) {
 
 	cases := []struct {
 		name      string
-		csr       *certificatesv1beta1.CertificateSigningRequest
+		csr       testinghelpers.CSRHolder
 		isRenewal bool
 	}{
 		{
 			name:      "a spoke cluster csr without labels",
-			csr:       newCSR(map[string]string{}, nil, "", []string{}, "", ""),
+			csr:       testinghelpers.CSRHolder{},
 			isRenewal: false,
 		},
 		{
-			name:      "an invalid signer name",
-			csr:       newCSR(labels, &invalidSignerName, "", []string{}, "", ""),
+			name: "an invalid signer name",
+			csr: testinghelpers.CSRHolder{
+				Labels:     validCSR.Labels,
+				SignerName: &invalidSignerName,
+			},
 			isRenewal: false,
 		},
 		{
-			name:      "a wrong block type",
-			csr:       newCSR(labels, &signerName, "", []string{}, "", "RSA PRIVATE KEY"),
+			name: "a wrong block type",
+			csr: testinghelpers.CSRHolder{
+				Labels:       validCSR.Labels,
+				SignerName:   validCSR.SignerName,
+				ReqBlockType: "RSA PRIVATE KEY",
+			},
 			isRenewal: false,
 		},
 		{
-			name:      "an empty organization",
-			csr:       newCSR(labels, &signerName, "", []string{}, "", "CERTIFICATE REQUEST"),
+			name: "an empty organization",
+			csr: testinghelpers.CSRHolder{
+				Labels:       validCSR.Labels,
+				SignerName:   validCSR.SignerName,
+				ReqBlockType: validCSR.ReqBlockType,
+			},
 			isRenewal: false,
 		},
 		{
-			name:      "an invalid organization",
-			csr:       newCSR(labels, &signerName, "", []string{"test"}, "", "CERTIFICATE REQUEST"),
+			name: "an invalid organization",
+			csr: testinghelpers.CSRHolder{
+				Labels:       validCSR.Labels,
+				SignerName:   &signerName,
+				Orgs:         []string{"test"},
+				ReqBlockType: validCSR.ReqBlockType,
+			},
 			isRenewal: false,
 		},
 		{
-			name:      "an invalid common name",
-			csr:       newCSR(labels, &signerName, "", []string{"system:open-cluster-management:managedcluster1"}, "", "CERTIFICATE REQUEST"),
+			name: "an invalid common name",
+			csr: testinghelpers.CSRHolder{
+				Labels:       validCSR.Labels,
+				SignerName:   validCSR.SignerName,
+				Orgs:         validCSR.Orgs,
+				ReqBlockType: validCSR.ReqBlockType,
+			},
 			isRenewal: false,
 		},
 		{
-			name:      "an common name does not equal user name",
-			csr:       newInvalidCSR(),
+			name: "an common name does not equal user name",
+			csr: testinghelpers.CSRHolder{
+				Name:         validCSR.Name,
+				Labels:       validCSR.Labels,
+				SignerName:   validCSR.SignerName,
+				CN:           "system:open-cluster-management:managedcluster1:invalidagent",
+				Orgs:         validCSR.Orgs,
+				Username:     validCSR.Username,
+				ReqBlockType: validCSR.ReqBlockType,
+			},
 			isRenewal: false,
 		},
 		{
-			name:      "a renewal csr without signer name",
-			csr:       newCSRWithSignerName(nil),
+			name: "a renewal csr without signer name",
+			csr: testinghelpers.CSRHolder{
+				Name:         validCSR.Name,
+				Labels:       validCSR.Labels,
+				SignerName:   nil,
+				CN:           validCSR.CN,
+				Orgs:         validCSR.Orgs,
+				Username:     validCSR.Username,
+				ReqBlockType: validCSR.ReqBlockType,
+			},
 			isRenewal: true,
 		},
 		{
 			name:      "a renewal csr",
-			csr:       newRenewalCSR(),
+			csr:       validCSR,
 			isRenewal: true,
 		},
 	}
 
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
-			isRenewal := isSpokeClusterClientCertRenewal(c.csr)
+			isRenewal := isSpokeClusterClientCertRenewal(testinghelpers.NewCSR(c.csr))
 			if isRenewal != c.isRenewal {
 				t.Errorf("expected %t, but failed", c.isRenewal)
 			}
 		})
 	}
 }
-
-func newCSR(labels map[string]string, signerName *string, cn string, orgs []string, username string, reqBlockType string) *certificatesv1beta1.CertificateSigningRequest {
-	insecureRand := rand.New(rand.NewSource(0))
-	pk, err := ecdsa.GenerateKey(elliptic.P256(), insecureRand)
-	if err != nil {
-		panic(err)
-	}
-	csrb, err := x509.CreateCertificateRequest(insecureRand, &x509.CertificateRequest{
-		Subject: pkix.Name{
-			CommonName:   cn,
-			Organization: orgs,
-		},
-		DNSNames:       []string{},
-		EmailAddresses: []string{},
-		IPAddresses:    []net.IP{},
-	}, pk)
-	if err != nil {
-		panic(err)
-	}
-	return &certificatesv1beta1.CertificateSigningRequest{
-		ObjectMeta: metav1.ObjectMeta{
-			Labels: labels,
-		},
-		Spec: certificatesv1beta1.CertificateSigningRequestSpec{
-			Username:   username,
-			Usages:     []certificatesv1beta1.KeyUsage{},
-			SignerName: signerName,
-			Request:    pem.EncodeToMemory(&pem.Block{Type: reqBlockType, Bytes: csrb}),
-		},
-	}
-}
-
-func newCSRWithSignerName(signer *string) *certificatesv1beta1.CertificateSigningRequest {
-	csr := newCSR(
-		labels,
-		signer,
-		"system:open-cluster-management:managedcluster1:spokeagent1",
-		[]string{"system:open-cluster-management:managedcluster1"},
-		"system:open-cluster-management:managedcluster1:spokeagent1",
-		"CERTIFICATE REQUEST",
-	)
-	csr.Name = testCSRName
-	return csr
-}
-
-func newRenewalCSR() *certificatesv1beta1.CertificateSigningRequest {
-	return newCSRWithSignerName(&signerName)
-}
-
-func newInvalidCSR() *certificatesv1beta1.CertificateSigningRequest {
-	csr := newCSR(
-		labels,
-		&signerName,
-		"system:open-cluster-management:managedcluster1:spokeagent2",
-		[]string{"system:open-cluster-management:managedcluster1"},
-		"system:open-cluster-management:managedcluster1:spokeagent1",
-		"CERTIFICATE REQUEST",
-	)
-	csr.Name = testCSRName
-	return csr
-}
-
-func newDeniedCSR() *certificatesv1beta1.CertificateSigningRequest {
-	csr := newRenewalCSR()
-	csr.Status.Conditions = append(csr.Status.Conditions, certificatesv1beta1.CertificateSigningRequestCondition{
-		Type: certificatesv1beta1.CertificateDenied,
-	})
-	return csr
-}
-
-func newApprovedCSR() *certificatesv1beta1.CertificateSigningRequest {
-	csr := newRenewalCSR()
-	csr.Status.Conditions = append(csr.Status.Conditions, certificatesv1beta1.CertificateSigningRequestCondition{
-		Type: certificatesv1beta1.CertificateApproved,
-	})
-	return csr
-}
-
-func assertActions(t *testing.T, actualActions []clienttesting.Action, expectedActions ...string) {
-	if len(actualActions) != len(expectedActions) {
-		t.Errorf("expected %d call but got: %#v", len(expectedActions), actualActions)
-	}
-	for i, expected := range expectedActions {
-		if actualActions[i].GetVerb() != expected {
-			t.Errorf("expected %s action but got: %#v", expected, actualActions[i])
-		}
-	}
-}
-
-func assertSubjectAccessReviewCreated(t *testing.T, actual runtime.Object) {
-	_, ok := actual.(*authorizationv1.SubjectAccessReview)
-	if !ok {
-		t.Errorf("expected subjectaccessreview created, but got: %#v", actual)
-	}
-}
-
-func assertCondition(t *testing.T, actual runtime.Object, expectedCondition certificatesv1beta1.RequestConditionType, expectedReason string) {
-	csr := actual.(*certificatesv1beta1.CertificateSigningRequest)
-	conditions := csr.Status.Conditions
-	if len(conditions) != 1 {
-		t.Errorf("expected 1 condition but got: %#v", conditions)
-	}
-	condition := conditions[0]
-	if condition.Type != expectedCondition {
-		t.Errorf("expected %s but got: %s", expectedCondition, condition.Type)
-	}
-	if condition.Reason != expectedReason {
-		t.Errorf("expected %s but got: %s", expectedReason, condition.Reason)
-	}
-}

pkg/hub/lease/controller_test.go

@@ -8,10 +8,8 @@ import (
 	clusterfake "github.com/open-cluster-management/api/client/cluster/clientset/versioned/fake"
 	clusterinformers "github.com/open-cluster-management/api/client/cluster/informers/externalversions"
 	clusterv1 "github.com/open-cluster-management/api/cluster/v1"
-	"github.com/open-cluster-management/registration/pkg/helpers"
 	testinghelpers "github.com/open-cluster-management/registration/pkg/helpers/testing"
 
-	coordv1 "k8s.io/api/coordination/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	kubeinformers "k8s.io/client-go/informers"
@@ -27,48 +25,47 @@ func TestSync(t *testing.T) {
 		clusters        []runtime.Object
 		clusterLeases   []runtime.Object
 		validateActions func(t *testing.T, leaseActions, clusterActions []clienttesting.Action)
-		expectedErr     string
 	}{
 		{
 			name:          "sync unaccepted managed cluster",
-			clusters:      []runtime.Object{newManagedCluster()},
+			clusters:      []runtime.Object{testinghelpers.NewManagedCluster()},
 			clusterLeases: []runtime.Object{},
 			validateActions: func(t *testing.T, leaseActions, clusterActions []clienttesting.Action) {
-				assertActions(t, leaseActions)
-				assertActions(t, clusterActions)
+				testinghelpers.AssertNoActions(t, leaseActions)
+				testinghelpers.AssertNoActions(t, clusterActions)
 			},
 		},
 		{
 			name:          "there is no lease for a managed cluster",
-			clusters:      []runtime.Object{newManagedCluster(newAcceptedCondtion())},
+			clusters:      []runtime.Object{testinghelpers.NewAcceptedManagedCluster()},
 			clusterLeases: []runtime.Object{},
 			validateActions: func(t *testing.T, leaseActions, clusterActions []clienttesting.Action) {
-				assertActions(t, leaseActions, "create")
-				assertActions(t, clusterActions)
+				testinghelpers.AssertActions(t, leaseActions, "create")
+				testinghelpers.AssertNoActions(t, clusterActions)
 			},
 		},
 		{
 			name:          "managed cluster stop update lease",
-			clusters:      []runtime.Object{newManagedCluster(newAcceptedCondtion(), newAvailableCondtion())},
-			clusterLeases: []runtime.Object{newClusterLease(now.Add(-5 * time.Minute))},
+			clusters:      []runtime.Object{testinghelpers.NewAvailableManagedCluster()},
+			clusterLeases: []runtime.Object{testinghelpers.NewManagedClusterLease(now.Add(-5 * time.Minute))},
 			validateActions: func(t *testing.T, leaseActions, clusterActions []clienttesting.Action) {
-				assertActions(t, clusterActions, "get", "update")
-				actual := clusterActions[1].(clienttesting.UpdateActionImpl).Object
 				expected := clusterv1.StatusCondition{
 					Type:    clusterv1.ManagedClusterConditionAvailable,
 					Status:  metav1.ConditionUnknown,
 					Reason:  "ManagedClusterLeaseUpdateStopped",
 					Message: "Registration agent stopped updating its lease within 5 minutes.",
 				}
-				assertCondition(t, actual, expected)
+				testinghelpers.AssertActions(t, clusterActions, "get", "update")
+				actual := clusterActions[1].(clienttesting.UpdateActionImpl).Object
+				testinghelpers.AssertManagedClusterCondition(t, actual.(*clusterv1.ManagedCluster).Status.Conditions, expected)
 			},
 		},
 		{
 			name:          "managed cluster is available",
-			clusters:      []runtime.Object{newManagedCluster(newAcceptedCondtion(), newAvailableCondtion())},
-			clusterLeases: []runtime.Object{newClusterLease(now)},
+			clusters:      []runtime.Object{testinghelpers.NewAvailableManagedCluster()},
+			clusterLeases: []runtime.Object{testinghelpers.NewManagedClusterLease(now)},
 			validateActions: func(t *testing.T, leaseActions, clusterActions []clienttesting.Action) {
-				assertActions(t, clusterActions)
+				testinghelpers.AssertNoActions(t, clusterActions)
 			},
 		},
 	}
@@ -96,87 +93,10 @@ func TestSync(t *testing.T) {
 				leaseLister:   leaseInformerFactory.Coordination().V1().Leases().Lister(),
 			}
 			syncErr := ctrl.sync(context.TODO(), testinghelpers.NewFakeSyncContext(t, ""))
-			if len(c.expectedErr) > 0 && syncErr == nil {
-				t.Errorf("expected %q error", c.expectedErr)
-				return
-			}
-			if len(c.expectedErr) > 0 && syncErr != nil && syncErr.Error() != c.expectedErr {
-				t.Errorf("expected %q error, got %q", c.expectedErr, syncErr.Error())
-				return
-			}
-			if len(c.expectedErr) == 0 && syncErr != nil {
+			if syncErr != nil {
 				t.Errorf("unexpected err: %v", syncErr)
 			}
 			c.validateActions(t, leaseClient.Actions(), clusterClient.Actions())
 		})
 	}
 }
-
-func assertActions(t *testing.T, actualActions []clienttesting.Action, expectedActions ...string) {
-	if len(actualActions) != len(expectedActions) {
-		t.Errorf("expected %d call but got: %#v", len(expectedActions), actualActions)
-	}
-	for i, expected := range expectedActions {
-		if actualActions[i].GetVerb() != expected {
-			t.Errorf("expected %s action but got: %#v", expected, actualActions[i])
-		}
-	}
-}
-
-func assertCondition(t *testing.T, actual runtime.Object, expectedCondition clusterv1.StatusCondition) {
-	managedCluster := actual.(*clusterv1.ManagedCluster)
-	cond := helpers.FindManagedClusterCondition(managedCluster.Status.Conditions, expectedCondition.Type)
-	if cond == nil {
-		t.Errorf("expected condition %s but got: %s", expectedCondition.Type, cond.Type)
-	}
-	if cond.Status != expectedCondition.Status {
-		t.Errorf("expected status %s but got: %s", expectedCondition.Status, cond.Status)
-	}
-	if cond.Reason != expectedCondition.Reason {
-		t.Errorf("expected reason %s but got: %s", expectedCondition.Reason, cond.Reason)
-	}
-	if cond.Message != expectedCondition.Message {
-		t.Errorf("expected message %s but got: %s", expectedCondition.Message, cond.Message)
-	}
-}
-
-func newManagedCluster(conditions ...clusterv1.StatusCondition) *clusterv1.ManagedCluster {
-	return &clusterv1.ManagedCluster{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: "testmanagedcluster",
-		},
-		Status: clusterv1.ManagedClusterStatus{
-			Conditions: conditions,
-		},
-	}
-}
-
-func newAcceptedCondtion() clusterv1.StatusCondition {
-	return clusterv1.StatusCondition{
-		Type:    clusterv1.ManagedClusterConditionHubAccepted,
-		Status:  metav1.ConditionTrue,
-		Reason:  "HubClusterAdminAccepted",
-		Message: "Accepted by hub cluster admin",
-	}
-}
-
-func newAvailableCondtion() clusterv1.StatusCondition {
-	return clusterv1.StatusCondition{
-		Type:    clusterv1.ManagedClusterConditionAvailable,
-		Status:  metav1.ConditionTrue,
-		Reason:  "ManagedClusterAvailable",
-		Message: "Managed cluster is available",
-	}
-}
-
-func newClusterLease(renewTime time.Time) *coordv1.Lease {
-	return &coordv1.Lease{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "cluster-lease-testmanagedcluster",
-			Namespace: "testmanagedcluster",
-		},
-		Spec: coordv1.LeaseSpec{
-			RenewTime: &metav1.MicroTime{Time: renewTime},
-		},
-	}
-}

pkg/hub/managedcluster/controller_test.go

@@ -2,92 +2,88 @@ package managedcluster
 
 import (
 	"context"
-	"reflect"
 	"testing"
-	"time"
 
 	clusterfake "github.com/open-cluster-management/api/client/cluster/clientset/versioned/fake"
+	v1 "github.com/open-cluster-management/api/cluster/v1"
+	testinghelpers "github.com/open-cluster-management/registration/pkg/helpers/testing"
+
+	"github.com/openshift/library-go/pkg/operator/events/eventstesting"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	kubefake "k8s.io/client-go/kubernetes/fake"
 	clienttesting "k8s.io/client-go/testing"
-
-	v1 "github.com/open-cluster-management/api/cluster/v1"
-	testinghelpers "github.com/open-cluster-management/registration/pkg/helpers/testing"
-	"github.com/openshift/library-go/pkg/operator/events/eventstesting"
 )
 
-const testManagedClusterName = "test_managed_cluster"
-
 func TestSyncManagedCluster(t *testing.T) {
 	cases := []struct {
 		name            string
 		startingObjects []runtime.Object
 		validateActions func(t *testing.T, actions []clienttesting.Action)
-		expectedErr     string
 	}{
 		{
 			name:            "sync a deleted spoke cluster",
 			startingObjects: []runtime.Object{},
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				if len(actions) != 1 {
-					t.Errorf("expected 1 call but got: %#v", actions)
-				}
-				assertAction(t, actions[0], "get")
+				testinghelpers.AssertActions(t, actions, "get")
 			},
 		},
 		{
 			name:            "create a new spoke cluster",
-			startingObjects: []runtime.Object{newManagedCluster()},
+			startingObjects: []runtime.Object{testinghelpers.NewManagedCluster()},
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				if len(actions) != 2 {
-					t.Errorf("expected 1 call but got: %#v", actions)
-				}
-				assertAction(t, actions[1], "update")
-				assertFinalizers(t, actions[1].(clienttesting.UpdateActionImpl).Object, []string{managedClusterFinalizer})
+				testinghelpers.AssertActions(t, actions, "get", "update")
+				managedCluster := (actions[1].(clienttesting.UpdateActionImpl).Object).(*v1.ManagedCluster)
+				testinghelpers.AssertFinalizers(t, managedCluster, []string{managedClusterFinalizer})
 			},
 		},
 		{
 			name:            "accept a spoke cluster",
-			startingObjects: []runtime.Object{newAcceptedManagedCluster([]string{managedClusterFinalizer})},
+			startingObjects: []runtime.Object{testinghelpers.NewAcceptingManagedCluster()},
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				if len(actions) != 3 {
-					t.Errorf("expected 3 call but got: %#v", actions)
+				expectedCondition := v1.StatusCondition{
+					Type:    v1.ManagedClusterConditionHubAccepted,
+					Status:  metav1.ConditionTrue,
+					Reason:  "HubClusterAdminAccepted",
+					Message: "Accepted by hub cluster admin",
 				}
-				assertAction(t, actions[2], "update")
-				assertCondition(t, actions[2].(clienttesting.UpdateActionImpl).Object, v1.ManagedClusterConditionHubAccepted, metav1.ConditionTrue)
+				testinghelpers.AssertActions(t, actions, "get", "get", "update")
+				actual := actions[2].(clienttesting.UpdateActionImpl).Object
+				managedCluster := actual.(*v1.ManagedCluster)
+				testinghelpers.AssertManagedClusterCondition(t, managedCluster.Status.Conditions, expectedCondition)
 			},
 		},
 		{
 			name:            "sync an accepted spoke cluster",
-			startingObjects: []runtime.Object{newAcceptedManagedClusterWithCondition()},
+			startingObjects: []runtime.Object{testinghelpers.NewAcceptedManagedCluster()},
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				if len(actions) != 2 {
-					t.Errorf("expected 2 call but got: %#v", actions)
-				}
-				assertAction(t, actions[1], "get")
+				testinghelpers.AssertActions(t, actions, "get", "get")
 			},
 		},
 		{
 			name:            "deny an accepted spoke cluster",
-			startingObjects: []runtime.Object{newDeniedManagedCluster()},
+			startingObjects: []runtime.Object{testinghelpers.NewDeniedManagedCluster()},
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				if len(actions) != 3 {
-					t.Errorf("expected 3 call but got: %#v", actions)
+				expectedCondition := v1.StatusCondition{
+					Type:    v1.ManagedClusterConditionHubAccepted,
+					Status:  metav1.ConditionFalse,
+					Reason:  "HubClusterAdminDenied",
+					Message: "Denied by hub cluster admin",
 				}
-				assertAction(t, actions[2], "update")
-				assertCondition(t, actions[2].(clienttesting.UpdateActionImpl).Object, v1.ManagedClusterConditionHubAccepted, metav1.ConditionFalse)
+				testinghelpers.AssertActions(t, actions, "get", "get", "update")
+				actual := actions[2].(clienttesting.UpdateActionImpl).Object
+				managedCluster := actual.(*v1.ManagedCluster)
+				testinghelpers.AssertManagedClusterCondition(t, managedCluster.Status.Conditions, expectedCondition)
 			},
 		},
 		{
 			name:            "delete a spoke cluster",
-			startingObjects: []runtime.Object{newDeletingManagedCluster()},
+			startingObjects: []runtime.Object{testinghelpers.NewDeletingManagedCluster()},
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				if len(actions) != 2 {
-					t.Errorf("expected 2 call but got: %#v", actions)
-				}
-				assertAction(t, actions[1], "update")
-				assertFinalizers(t, actions[1].(clienttesting.UpdateActionImpl).Object, []string{})
+				testinghelpers.AssertActions(t, actions, "get", "update")
+				managedCluster := (actions[1].(clienttesting.UpdateActionImpl).Object).(*v1.ManagedCluster)
+				testinghelpers.AssertFinalizers(t, managedCluster, []string{})
 			},
 		},
 	}
@@ -98,15 +94,7 @@ func TestSyncManagedCluster(t *testing.T) {
 			kubeClient := kubefake.NewSimpleClientset()
 
 			ctrl := managedClusterController{kubeClient, clusterClient, eventstesting.NewTestingEventRecorder(t)}
-			syncErr := ctrl.sync(context.TODO(), testinghelpers.NewFakeSyncContext(t, testManagedClusterName))
-			if len(c.expectedErr) > 0 && syncErr == nil {
-				t.Errorf("expected %q error", c.expectedErr)
-				return
-			}
-			if len(c.expectedErr) > 0 && syncErr != nil && syncErr.Error() != c.expectedErr {
-				t.Errorf("expected %q error, got %q", c.expectedErr, syncErr.Error())
-				return
-			}
+			syncErr := ctrl.sync(context.TODO(), testinghelpers.NewFakeSyncContext(t, testinghelpers.TestManagedClusterName))
 			if syncErr != nil {
 				t.Errorf("unexpected err: %v", syncErr)
 			}
@@ -115,76 +103,3 @@ func TestSyncManagedCluster(t *testing.T) {
 		})
 	}
 }
-
-func assertAction(t *testing.T, actual clienttesting.Action, expected string) {
-	if actual.GetVerb() != expected {
-		t.Errorf("expected %s action but got: %#v", expected, actual)
-	}
-}
-
-func assertFinalizers(t *testing.T, actual runtime.Object, expected []string) {
-	managedCluster := actual.(*v1.ManagedCluster)
-	if !reflect.DeepEqual(managedCluster.Finalizers, expected) {
-		t.Errorf("expected %#v but got %#v", expected, managedCluster.Finalizers)
-	}
-}
-
-func assertCondition(t *testing.T, actual runtime.Object, expectedCondition string, expectedStatus metav1.ConditionStatus) {
-	managedCluster := actual.(*v1.ManagedCluster)
-	conditions := managedCluster.Status.Conditions
-	if len(conditions) != 1 {
-		t.Errorf("expected 1 condition but got: %#v", conditions)
-	}
-	condition := conditions[0]
-	if condition.Type != expectedCondition {
-		t.Errorf("expected %s but got: %s", expectedCondition, condition.Type)
-	}
-	if condition.Status != expectedStatus {
-		t.Errorf("expected %s but got: %s", expectedStatus, condition.Status)
-	}
-}
-
-func newManagedCluster() *v1.ManagedCluster {
-	return &v1.ManagedCluster{ObjectMeta: metav1.ObjectMeta{Name: testManagedClusterName}}
-}
-
-func newAcceptedManagedCluster(finalizers []string) *v1.ManagedCluster {
-	return &v1.ManagedCluster{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:       testManagedClusterName,
-			Finalizers: finalizers,
-		},
-		Spec: v1.ManagedClusterSpec{HubAcceptsClient: true},
-	}
-}
-
-func newAcceptedManagedClusterWithCondition() *v1.ManagedCluster {
-	spokeCluster := newAcceptedManagedCluster([]string{managedClusterFinalizer})
-	spokeCluster.Finalizers = append(spokeCluster.Finalizers, managedClusterFinalizer)
-	spokeCluster.Status.Conditions = append(spokeCluster.Status.Conditions, v1.StatusCondition{
-		Type:               v1.ManagedClusterConditionHubAccepted,
-		Status:             metav1.ConditionTrue,
-		Reason:             "HubClusterAdminAccepted",
-		Message:            "Accepted by hub cluster admin",
-		LastTransitionTime: metav1.Time{Time: metav1.Now().Add(-10 * time.Second)},
-	})
-	return spokeCluster
-}
-
-func newDeniedManagedCluster() *v1.ManagedCluster {
-	spokeCluster := newAcceptedManagedClusterWithCondition()
-	spokeCluster.Spec.HubAcceptsClient = false
-	return spokeCluster
-}
-
-func newDeletingManagedCluster() *v1.ManagedCluster {
-	now := metav1.Now()
-	return &v1.ManagedCluster{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:              testManagedClusterName,
-			DeletionTimestamp: &now,
-			Finalizers:        []string{managedClusterFinalizer},
-		},
-		Spec: v1.ManagedClusterSpec{HubAcceptsClient: true},
-	}
-}

pkg/hub/rbacfinalizerdeletion/controller.go

@@ -8,8 +8,10 @@ import (
 	clusterv1listers "github.com/open-cluster-management/api/client/cluster/listers/cluster/v1"
 	worklister "github.com/open-cluster-management/api/client/work/listers/work/v1"
 	clusterv1 "github.com/open-cluster-management/api/cluster/v1"
+
 	"github.com/openshift/library-go/pkg/controller/factory"
 	"github.com/openshift/library-go/pkg/operator/events"
+
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
 	"k8s.io/apimachinery/pkg/api/errors"

pkg/hub/rbacfinalizerdeletion/controller_test.go

@@ -2,87 +2,105 @@ package rbacfinalizerdeletion
 
 import (
 	"context"
-	"reflect"
+	"fmt"
 	"testing"
 	"time"
 
-	"github.com/davecgh/go-spew/spew"
+	fakeclusterclient "github.com/open-cluster-management/api/client/cluster/clientset/versioned/fake"
+	clusterinformers "github.com/open-cluster-management/api/client/cluster/informers/externalversions"
 	fakeworkclient "github.com/open-cluster-management/api/client/work/clientset/versioned/fake"
 	workinformers "github.com/open-cluster-management/api/client/work/informers/externalversions"
 	clusterv1 "github.com/open-cluster-management/api/cluster/v1"
 	workapiv1 "github.com/open-cluster-management/api/work/v1"
 	testinghelpers "github.com/open-cluster-management/registration/pkg/helpers/testing"
+
 	"github.com/openshift/library-go/pkg/operator/events"
+
 	corev1 "k8s.io/api/core/v1"
 	rbacv1 "k8s.io/api/rbac/v1"
-	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	kubeinformers "k8s.io/client-go/informers"
 	fakeclient "k8s.io/client-go/kubernetes/fake"
 	clienttesting "k8s.io/client-go/testing"
-	"k8s.io/utils/diff"
 )
 
-func newRole(namespace, name string, finalizers []string, terminated bool) *rbacv1.Role {
-	role := &rbacv1.Role{}
+var roleName = fmt.Sprintf("%s:spoke-work", testinghelpers.TestManagedClusterName)
 
-	role.Namespace = namespace
-	role.Name = name
-	role.Finalizers = finalizers
-	if terminated {
-		now := metav1.Now()
-		role.DeletionTimestamp = &now
-	}
-
-	return role
-}
-
-func newRoleBinding(namespace, name string, finalizers []string, terminated bool) *rbacv1.RoleBinding {
-	rolebinding := &rbacv1.RoleBinding{}
-
-	rolebinding.Namespace = namespace
-	rolebinding.Name = name
-	rolebinding.Finalizers = finalizers
-	if terminated {
-		now := metav1.Now()
-		rolebinding.DeletionTimestamp = &now
-	}
-
-	return rolebinding
-}
-
-func newNamespace(name string, terminated bool) *corev1.Namespace {
-	namespace := &corev1.Namespace{}
-	namespace.Name = name
-	if terminated {
-		now := metav1.Now()
-		namespace.DeletionTimestamp = &now
-	}
-
-	return namespace
-}
-
-func newManifestWork(namespace, name string, finalizers []string, deletionTimestamp *metav1.Time) *workapiv1.ManifestWork {
-	work := &workapiv1.ManifestWork{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace:         namespace,
-			Name:              name,
-			Finalizers:        finalizers,
-			DeletionTimestamp: deletionTimestamp,
+func TestSync(t *testing.T) {
+	cases := []struct {
+		name         string
+		key          string
+		clusters     []runtime.Object
+		namespaces   []runtime.Object
+		roles        []runtime.Object
+		roleBindings []runtime.Object
+		works        []runtime.Object
+		expectedErr  string
+	}{
+		{
+			name:        "managed cluster namespace is not found",
+			key:         fmt.Sprintf("%s/%s", testinghelpers.TestManagedClusterName, roleName),
+			expectedErr: "namespace \"testmanagedcluster\" not found",
+		},
+		{
+			name:       "there are no resources in managed cluster namespace",
+			key:        fmt.Sprintf("%s/%s", testinghelpers.TestManagedClusterName, roleName),
+			namespaces: []runtime.Object{testinghelpers.NewNamespace(testinghelpers.TestManagedClusterName, true)},
+		},
+		{
+			name:         "still have works in deleting managed cluster namespace",
+			key:          fmt.Sprintf("%s/%s", testinghelpers.TestManagedClusterName, roleName),
+			namespaces:   []runtime.Object{testinghelpers.NewNamespace(testinghelpers.TestManagedClusterName, true)},
+			roles:        []runtime.Object{testinghelpers.NewRole(testinghelpers.TestManagedClusterName, roleName, []string{manifestWorkFinalizer}, true)},
+			roleBindings: []runtime.Object{testinghelpers.NewRoleBinding(testinghelpers.TestManagedClusterName, roleName, []string{manifestWorkFinalizer}, true)},
+			works:        []runtime.Object{testinghelpers.NewManifestWork(testinghelpers.TestManagedClusterName, "work1", []string{manifestWorkFinalizer}, nil)},
+			expectedErr:  "Still having 1 works in the cluster namespace testmanagedcluster",
 		},
 	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			kubeClient := fakeclient.NewSimpleClientset()
+			kubeInformerFactory := kubeinformers.NewSharedInformerFactory(kubeClient, time.Minute*10)
+			nsStore := kubeInformerFactory.Core().V1().Namespaces().Informer().GetStore()
+			for _, ns := range c.namespaces {
+				nsStore.Add(ns)
+			}
+			roleStore := kubeInformerFactory.Rbac().V1().Roles().Informer().GetStore()
+			for _, role := range c.roles {
+				roleStore.Add(role)
+			}
+			roleBindingStore := kubeInformerFactory.Rbac().V1().RoleBindings().Informer().GetStore()
+			for _, roleBinding := range c.roleBindings {
+				roleBindingStore.Add(roleBinding)
+			}
 
-	return work
-}
+			clusterClient := fakeclusterclient.NewSimpleClientset()
+			clusterInformerFactory := clusterinformers.NewSharedInformerFactory(clusterClient, time.Minute*10)
 
-func newDeletionTimestamp(offset time.Duration) *metav1.Time {
-	return &metav1.Time{
-		Time: metav1.Now().Add(offset),
+			workClient := fakeworkclient.NewSimpleClientset()
+			workInformerFactory := workinformers.NewSharedInformerFactory(workClient, 5*time.Minute)
+			workStore := workInformerFactory.Work().V1().ManifestWorks().Informer().GetStore()
+			for _, work := range c.works {
+				workStore.Add(work)
+			}
+
+			ctrl := &finalizeController{
+				roleLister:         kubeInformerFactory.Rbac().V1().Roles().Lister(),
+				roleBindingLister:  kubeInformerFactory.Rbac().V1().RoleBindings().Lister(),
+				namespaceLister:    kubeInformerFactory.Core().V1().Namespaces().Lister(),
+				clusterLister:      clusterInformerFactory.Cluster().V1().ManagedClusters().Lister(),
+				manifestWorkLister: workInformerFactory.Work().V1().ManifestWorks().Lister(),
+				rbacClient:         kubeClient.RbacV1(),
+				eventRecorder:      events.NewInMemoryRecorder(""),
+			}
+			err := ctrl.sync(context.TODO(), testinghelpers.NewFakeSyncContext(t, c.key))
+			testinghelpers.AssertError(t, err, c.expectedErr)
+		})
 	}
 }
 
 func TestSyncRoleAndRoleBinding(t *testing.T) {
-	now := metav1.Now()
 	cases := []struct {
 		name                          string
 		role                          *rbacv1.Role
@@ -98,57 +116,51 @@ func TestSyncRoleAndRoleBinding(t *testing.T) {
 	}{
 		{
 			name:                "skip if neither role nor rolebinding exists",
-			cluster:             testinghelpers.NewManagedCluster("cluster1", nil),
-			namespace:           newNamespace("cluster1", false),
-			work:                testinghelpers.NewManifestWork("cluster1", "work1", nil, nil),
-			validateRbacActions: noAction,
+			cluster:             testinghelpers.NewManagedCluster(),
+			namespace:           testinghelpers.NewNamespace(testinghelpers.TestManagedClusterName, false),
+			work:                testinghelpers.NewManifestWork(testinghelpers.TestManagedClusterName, "work1", nil, nil),
+			validateRbacActions: testinghelpers.AssertNoActions,
 		},
 		{
 			name:                   "skip if neither role nor rolebinding has finalizer",
-			role:                   newRole("cluster1", "cluster1:spoke-work", nil, false),
-			roleBinding:            newRoleBinding("cluster1", "cluster1:spoke-work", nil, false),
-			cluster:                testinghelpers.NewManagedCluster("cluster1", nil),
-			namespace:              newNamespace("cluster1", false),
-			work:                   testinghelpers.NewManifestWork("cluster1", "work1", []string{manifestWorkFinalizer}, nil),
+			role:                   testinghelpers.NewRole(testinghelpers.TestManagedClusterName, roleName, nil, false),
+			roleBinding:            testinghelpers.NewRoleBinding(testinghelpers.TestManagedClusterName, roleName, nil, false),
+			cluster:                testinghelpers.NewManagedCluster(),
+			namespace:              testinghelpers.NewNamespace(testinghelpers.TestManagedClusterName, false),
+			work:                   testinghelpers.NewManifestWork(testinghelpers.TestManagedClusterName, "work1", []string{manifestWorkFinalizer}, nil),
 			expectedWorkFinalizers: []string{manifestWorkFinalizer},
-			validateRbacActions:    noAction,
+			validateRbacActions:    testinghelpers.AssertNoActions,
 		},
 		{
 			name:                          "remove finalizer from deleting role within non-terminating namespace",
-			role:                          newRole("cluster1", "cluster1:spoke-work", []string{manifestWorkFinalizer}, true),
-			roleBinding:                   newRoleBinding("cluster1", "cluster1:spoke-work", []string{manifestWorkFinalizer}, false),
-			cluster:                       testinghelpers.NewManagedCluster("cluster1", nil),
-			namespace:                     newNamespace("cluster1", false),
-			work:                          testinghelpers.NewManifestWork("cluster1", "work1", []string{manifestWorkFinalizer}, nil),
+			role:                          testinghelpers.NewRole(testinghelpers.TestManagedClusterName, roleName, []string{manifestWorkFinalizer}, true),
+			roleBinding:                   testinghelpers.NewRoleBinding(testinghelpers.TestManagedClusterName, roleName, []string{manifestWorkFinalizer}, false),
+			cluster:                       testinghelpers.NewManagedCluster(),
+			namespace:                     testinghelpers.NewNamespace(testinghelpers.TestManagedClusterName, false),
+			work:                          testinghelpers.NewManifestWork(testinghelpers.TestManagedClusterName, "work1", []string{manifestWorkFinalizer}, nil),
 			expectedRoleBindingFinalizers: []string{manifestWorkFinalizer},
 			expectedWorkFinalizers:        []string{manifestWorkFinalizer},
 			validateRbacActions: func(t *testing.T, actions []clienttesting.Action) {
-				if len(actions) != 1 {
-					t.Fatal(spew.Sdump(actions))
-				}
+				testinghelpers.AssertActions(t, actions, "update")
 			},
 		},
 		{
 			name:        "remove finalizer from role/rolebinding within terminating cluster",
-			role:        newRole("cluster1", "cluster1:spoke-work", []string{manifestWorkFinalizer}, true),
-			roleBinding: newRoleBinding("cluster1", "cluster1:spoke-work", []string{manifestWorkFinalizer}, true),
-			cluster:     testinghelpers.NewManagedCluster("cluster1", &now),
-			namespace:   newNamespace("cluster1", false),
+			role:        testinghelpers.NewRole(testinghelpers.TestManagedClusterName, roleName, []string{manifestWorkFinalizer}, true),
+			roleBinding: testinghelpers.NewRoleBinding(testinghelpers.TestManagedClusterName, roleName, []string{manifestWorkFinalizer}, true),
+			cluster:     testinghelpers.NewDeletingManagedCluster(),
+			namespace:   testinghelpers.NewNamespace(testinghelpers.TestManagedClusterName, false),
 			validateRbacActions: func(t *testing.T, actions []clienttesting.Action) {
-				if len(actions) != 2 {
-					t.Fatal(spew.Sdump(actions))
-				}
+				testinghelpers.AssertActions(t, actions, "update", "update")
 			},
 		},
 		{
 			name:        "remove finalizer from role/rolebinding within terminating ns",
-			role:        newRole("cluster1", "cluster1:spoke-work", []string{manifestWorkFinalizer}, true),
-			roleBinding: newRoleBinding("cluster1", "cluster1:spoke-work", []string{manifestWorkFinalizer}, true),
-			namespace:   newNamespace("cluster1", true),
+			role:        testinghelpers.NewRole(testinghelpers.TestManagedClusterName, roleName, []string{manifestWorkFinalizer}, true),
+			roleBinding: testinghelpers.NewRoleBinding(testinghelpers.TestManagedClusterName, roleName, []string{manifestWorkFinalizer}, true),
+			namespace:   testinghelpers.NewNamespace(testinghelpers.TestManagedClusterName, true),
 			validateRbacActions: func(t *testing.T, actions []clienttesting.Action) {
-				if len(actions) != 2 {
-					t.Fatal(spew.Sdump(actions))
-				}
+				testinghelpers.AssertActions(t, actions, "update", "update")
 			},
 		},
 	}
@@ -198,7 +210,7 @@ func TestSyncRoleAndRoleBinding(t *testing.T) {
 					if err != nil {
 						t.Fatal(err)
 					}
-					assertFinalizers(t, role, c.expectedRoleFinalizers)
+					testinghelpers.AssertFinalizers(t, role, c.expectedRoleFinalizers)
 				}
 
 				if c.roleBinding != nil {
@@ -206,7 +218,7 @@ func TestSyncRoleAndRoleBinding(t *testing.T) {
 					if err != nil {
 						t.Fatal(err)
 					}
-					assertFinalizers(t, rolebinding, c.expectedRoleBindingFinalizers)
+					testinghelpers.AssertFinalizers(t, rolebinding, c.expectedRoleBindingFinalizers)
 				}
 
 				if c.work != nil {
@@ -214,7 +226,7 @@ func TestSyncRoleAndRoleBinding(t *testing.T) {
 					if err != nil {
 						t.Fatal(err)
 					}
-					assertFinalizers(t, work, c.expectedWorkFinalizers)
+					testinghelpers.AssertFinalizers(t, work, c.expectedWorkFinalizers)
 				}
 
 				actual := controllerContext.Queue().Len()
@@ -225,21 +237,3 @@ func TestSyncRoleAndRoleBinding(t *testing.T) {
 		})
 	}
 }
-
-func assertFinalizers(t *testing.T, obj runtime.Object, finalizers []string) {
-	accessor, _ := meta.Accessor(obj)
-	actual := accessor.GetFinalizers()
-	if len(actual) == 0 && len(finalizers) == 0 {
-		return
-	}
-
-	if !reflect.DeepEqual(actual, finalizers) {
-		t.Fatal(diff.ObjectDiff(actual, finalizers))
-	}
-}
-
-func noAction(t *testing.T, actions []clienttesting.Action) {
-	if len(actions) > 0 {
-		t.Fatal(spew.Sdump(actions))
-	}
-}

pkg/spoke/hubclientcert/certificate_test.go

@@ -1,392 +1,143 @@
 package hubclientcert
 
 import (
-	"crypto"
-	"crypto/rand"
-	cryptorand "crypto/rand"
-	"crypto/rsa"
 	"crypto/x509"
-	"crypto/x509/pkix"
-	"encoding/pem"
-	"errors"
-	"math"
-	"math/big"
 	"testing"
 	"time"
 
+	testinghelpers "github.com/open-cluster-management/registration/pkg/helpers/testing"
+
 	certificates "k8s.io/api/certificates/v1beta1"
 	corev1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/client-go/tools/clientcmd"
-	clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
 	certutil "k8s.io/client-go/util/cert"
-	"k8s.io/client-go/util/keyutil"
 )
 
-func newCertKey(commonName string, duration time.Duration) ([]byte, []byte, error) {
-	signingKey, err := rsa.GenerateKey(cryptorand.Reader, 2048)
-	if err != nil {
-		return nil, nil, err
-	}
-
-	signingCert, err := certutil.NewSelfSignedCACert(certutil.Config{CommonName: "open-cluster-management.io"}, signingKey)
-	if err != nil {
-		return nil, nil, err
-	}
-
-	key, err := rsa.GenerateKey(cryptorand.Reader, 2048)
-	if err != nil {
-		return nil, nil, err
-	}
-
-	cert, err := newSignedCert(
-		certutil.Config{
-			CommonName: commonName,
-			Usages:     []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth},
+func TestCSRApproved(t *testing.T) {
+	cases := []struct {
+		name        string
+		csr         *certificates.CertificateSigningRequest
+		csrApproved bool
+	}{
+		{
+			name: "pending csr",
+			csr:  testinghelpers.NewCSR(testinghelpers.CSRHolder{}),
 		},
-		key, signingCert, signingKey, duration,
-	)
-	if err != nil {
-		return nil, nil, err
-	}
-
-	return encodePrivateKeyPEM(key), encodeCertPEM(cert), nil
-}
-
-// encodePrivateKeyPEM returns PEM-encoded private key data
-func encodePrivateKeyPEM(key *rsa.PrivateKey) []byte {
-	block := pem.Block{
-		Type:  keyutil.RSAPrivateKeyBlockType,
-		Bytes: x509.MarshalPKCS1PrivateKey(key),
-	}
-	return pem.EncodeToMemory(&block)
-}
-
-// encodeCertPEM returns PEM-encoded certificate data
-func encodeCertPEM(cert *x509.Certificate) []byte {
-	block := pem.Block{
-		Type:  certutil.CertificateBlockType,
-		Bytes: cert.Raw,
-	}
-	return pem.EncodeToMemory(&block)
-}
-
-// newSignedCert creates a signed certificate using the given CA certificate and key
-func newSignedCert(cfg certutil.Config, key crypto.Signer, caCert *x509.Certificate, caKey crypto.Signer, duration time.Duration) (*x509.Certificate, error) {
-	serial, err := rand.Int(rand.Reader, new(big.Int).SetInt64(math.MaxInt64))
-	if err != nil {
-		return nil, err
-	}
-	if len(cfg.CommonName) == 0 {
-		return nil, errors.New("must specify a CommonName")
-	}
-	if len(cfg.Usages) == 0 {
-		return nil, errors.New("must specify at least one ExtKeyUsage")
-	}
-
-	certTmpl := x509.Certificate{
-		Subject: pkix.Name{
-			CommonName:   cfg.CommonName,
-			Organization: cfg.Organization,
+		{
+			name: "denied csr",
+			csr:  testinghelpers.NewDeniedCSR(testinghelpers.CSRHolder{}),
 		},
-		DNSNames:     cfg.AltNames.DNSNames,
-		IPAddresses:  cfg.AltNames.IPs,
-		SerialNumber: serial,
-		NotBefore:    caCert.NotBefore,
-		NotAfter:     time.Now().Add(duration).UTC(),
-		KeyUsage:     x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
-		ExtKeyUsage:  cfg.Usages,
-	}
-	certDERBytes, err := x509.CreateCertificate(cryptorand.Reader, &certTmpl, caCert, key.Public(), caKey)
-	if err != nil {
-		return nil, err
-	}
-	return x509.ParseCertificate(certDERBytes)
-}
-
-func newKubeconfig(key, cert []byte) clientcmdapi.Config {
-	var clientKey, clientCertificate string
-	var clientKeyData, clientCertificateData []byte
-	if key != nil {
-		clientKeyData = key
-	} else {
-		clientKey = "tls.key"
-	}
-	if cert != nil {
-		clientCertificateData = cert
-	} else {
-		clientCertificate = "tls.crt"
-	}
-
-	kubeconfig := clientcmdapi.Config{
-		// Define a cluster stanza based on the bootstrap kubeconfig.
-		Clusters: map[string]*clientcmdapi.Cluster{"default-cluster": {
-			Server:                "https://127.0.0.1:6001",
-			InsecureSkipTLSVerify: true,
-		}},
-		// Define auth based on the obtained client cert.
-		AuthInfos: map[string]*clientcmdapi.AuthInfo{"default-auth": {
-			ClientCertificate:     clientCertificate,
-			ClientCertificateData: clientCertificateData,
-			ClientKey:             clientKey,
-			ClientKeyData:         clientKeyData,
-		}},
-		// Define a context that connects the auth info and cluster, and set it as the default
-		Contexts: map[string]*clientcmdapi.Context{"default-context": {
-			Cluster:   "default-cluster",
-			AuthInfo:  "default-auth",
-			Namespace: "default",
-		}},
-		CurrentContext: "default-context",
-	}
-
-	return kubeconfig
-}
-
-func TestIsCSRApprovedWithPendingCSR(t *testing.T) {
-	csr := &certificates.CertificateSigningRequest{
-		ObjectMeta: metav1.ObjectMeta{
-			GenerateName: "csr-",
+		{
+			name:        "approved csr",
+			csr:         testinghelpers.NewApprovedCSR(testinghelpers.CSRHolder{}),
+			csrApproved: true,
 		},
-		Spec: certificates.CertificateSigningRequestSpec{},
 	}
-
-	if isCSRApproved(csr) {
-		t.Error("csr is not approved")
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			csrApproved := isCSRApproved(c.csr)
+			if csrApproved != c.csrApproved {
+				t.Errorf("expected %t, but got %t", c.csrApproved, csrApproved)
+			}
+		})
 	}
 }
 
-func TestIsCSRApprovedWithApprovedCSR(t *testing.T) {
-	csr := &certificates.CertificateSigningRequest{
-		ObjectMeta: metav1.ObjectMeta{
-			GenerateName: "csr-",
+func TestValidKubeconfig(t *testing.T) {
+	cases := []struct {
+		name    string
+		secret  *corev1.Secret
+		isValid bool
+	}{
+		{
+			name:   "no data",
+			secret: testinghelpers.NewHubKubeconfigSecret(testNamespace, testSecretName, "", nil, nil),
 		},
-		Spec: certificates.CertificateSigningRequestSpec{},
-		Status: certificates.CertificateSigningRequestStatus{
-			Conditions: []certificates.CertificateSigningRequestCondition{
-				{
-					Type: certificates.CertificateApproved,
-				},
-			},
+		{
+			name:   "no kubeconfig",
+			secret: testinghelpers.NewHubKubeconfigSecret(testNamespace, testSecretName, "", nil, map[string][]byte{}),
+		},
+		{
+			name: "no key",
+			secret: testinghelpers.NewHubKubeconfigSecret(testNamespace, testSecretName, "", nil, map[string][]byte{
+				KubeconfigFile: testinghelpers.NewKubeconfig(nil, nil),
+			}),
+		},
+		{
+			name: "no cert",
+			secret: testinghelpers.NewHubKubeconfigSecret(testNamespace, testSecretName, "", &testinghelpers.TestCert{Key: []byte("key")}, map[string][]byte{
+				KubeconfigFile: testinghelpers.NewKubeconfig(nil, nil),
+			}),
+		},
+		{
+			name: "bad cert",
+			secret: testinghelpers.NewHubKubeconfigSecret(testNamespace, testSecretName, "", &testinghelpers.TestCert{Key: []byte("key"), Cert: []byte("bad cert")}, map[string][]byte{
+				KubeconfigFile: testinghelpers.NewKubeconfig(nil, nil),
+			}),
+		},
+		{
+			name: "valid hub config",
+			secret: testinghelpers.NewHubKubeconfigSecret(testNamespace, testSecretName, "", testinghelpers.NewTestCert("test", 60*time.Second), map[string][]byte{
+				KubeconfigFile: testinghelpers.NewKubeconfig(nil, nil),
+			}),
+			isValid: true,
 		},
 	}
-
-	if !isCSRApproved(csr) {
-		t.Error("csr is approved")
-	}
-}
-
-func TestIsCSRApprovedWithDeniedCSR(t *testing.T) {
-	csr := &certificates.CertificateSigningRequest{
-		ObjectMeta: metav1.ObjectMeta{
-			GenerateName: "csr-",
-		},
-		Spec: certificates.CertificateSigningRequestSpec{},
-		Status: certificates.CertificateSigningRequestStatus{
-			Conditions: []certificates.CertificateSigningRequestCondition{
-				{
-					Type: certificates.CertificateApproved,
-				},
-				{
-					Type: certificates.CertificateDenied,
-				},
-			},
-		},
-	}
-
-	if isCSRApproved(csr) {
-		t.Error("csr is denied")
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			isValid := hasValidKubeconfig(c.secret)
+			if isValid != c.isValid {
+				t.Errorf("expected %t, but got %t", c.isValid, isValid)
+			}
+		})
 	}
 }
 
 func TestGetCertValidityPeriod(t *testing.T) {
-	_, cert, err := newCertKey("cluster0", 10*time.Second)
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	secret := &corev1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace: "default",
-			Name:      "secret",
+	certs := []byte{}
+	certs = append(certs, testinghelpers.NewTestCert("cluster0", 10*time.Second).Cert...)
+	secondCert := testinghelpers.NewTestCert("cluster0", 5*time.Second).Cert
+	certs = append(certs, secondCert...)
+	expectedCerts, _ := certutil.ParseCertsPEM(secondCert)
+	cases := []struct {
+		name         string
+		secret       *corev1.Secret
+		expectedCert *x509.Certificate
+		expectedErr  string
+	}{
+		{
+			name:        "no data",
+			secret:      testinghelpers.NewHubKubeconfigSecret(testNamespace, testSecretName, "", nil, nil),
+			expectedErr: "no client certificate found in secret \"testns/testsecret\"",
 		},
-		Data: map[string][]byte{
-			TLSCertFile: cert,
+		{
+			name:        "no cert",
+			secret:      testinghelpers.NewHubKubeconfigSecret(testNamespace, testSecretName, "", nil, map[string][]byte{}),
+			expectedErr: "no client certificate found in secret \"testns/testsecret\"",
+		},
+		{
+			name:        "bad cert",
+			secret:      testinghelpers.NewHubKubeconfigSecret(testNamespace, testSecretName, "", &testinghelpers.TestCert{Cert: []byte("bad cert")}, map[string][]byte{}),
+			expectedErr: "unable to parse TLS certificates: data does not contain any valid RSA or ECDSA certificates",
+		},
+		{
+			name:         "valid cert",
+			secret:       testinghelpers.NewHubKubeconfigSecret(testNamespace, testSecretName, "", &testinghelpers.TestCert{Cert: certs}, map[string][]byte{}),
+			expectedCert: expectedCerts[0],
 		},
 	}
-
-	notBefore, notAfter, err := getCertValidityPeriod(secret)
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	if notBefore == nil {
-		t.Error("notBefore should not be nil")
-	}
-
-	if notAfter == nil {
-		t.Error("notAfter should not be nil")
-	}
-}
-
-func TestIsCertificateValid(t *testing.T) {
-	_, cert, err := newCertKey("cluster0", 100*time.Second)
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	valid, err := IsCertificateValid(cert)
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-	if !valid {
-		t.Error("cert is valid")
-	}
-}
-
-func TestIsCertificateValidWithExpiredCert(t *testing.T) {
-	_, cert, err := newCertKey("cluster0", -3*time.Second)
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	valid, err := IsCertificateValid(cert)
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-	if valid {
-		t.Error("cert is expired")
-	}
-}
-
-func TestHasValidKubeconfig(t *testing.T) {
-	kubeconfigData, err := clientcmd.Write(newKubeconfig(nil, nil))
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	key, cert, err := newCertKey("cluster0", 100*time.Second)
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	secret := &corev1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace: "default",
-			Name:      "secret",
-		},
-		Data: map[string][]byte{
-			KubeconfigFile: kubeconfigData,
-			TLSCertFile:    cert,
-			TLSKeyFile:     key,
-		},
-	}
-
-	if !hasValidKubeconfig(secret) {
-		t.Error("kubeconfig is valid")
-	}
-}
-
-func TestHasValidKubeconfigWithExpiredCert(t *testing.T) {
-	kubeconfigData, err := clientcmd.Write(newKubeconfig(nil, nil))
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	key, cert, err := newCertKey("cluster0", -3*time.Second)
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	secret := &corev1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace: "default",
-			Name:      "secret",
-		},
-		Data: map[string][]byte{
-			KubeconfigFile: kubeconfigData,
-			TLSCertFile:    cert,
-			TLSKeyFile:     key,
-		},
-	}
-
-	if hasValidKubeconfig(secret) {
-		t.Error("kubeconfig is invalid")
-	}
-}
-
-func TestHasValidKubeconfigWithoutKey(t *testing.T) {
-	kubeconfigData, err := clientcmd.Write(newKubeconfig(nil, nil))
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	_, cert, err := newCertKey("cluster0", 100*time.Second)
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	secret := &corev1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace: "default",
-			Name:      "secret",
-		},
-		Data: map[string][]byte{
-			KubeconfigFile: kubeconfigData,
-			TLSCertFile:    cert,
-		},
-	}
-
-	if hasValidKubeconfig(secret) {
-		t.Error("kubeconfig is invalid")
-	}
-}
-
-func TestHasValidKubeconfigWithoutCert(t *testing.T) {
-	kubeconfigData, err := clientcmd.Write(newKubeconfig(nil, nil))
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	key, _, err := newCertKey("cluster0", 100*time.Second)
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	secret := &corev1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace: "default",
-			Name:      "secret",
-		},
-		Data: map[string][]byte{
-			KubeconfigFile: kubeconfigData,
-			TLSKeyFile:     key,
-		},
-	}
-
-	if hasValidKubeconfig(secret) {
-		t.Error("kubeconfig is invalid")
-	}
-}
-
-func TestHasValidKubeconfigWithoutKubeconfig(t *testing.T) {
-	key, cert, err := newCertKey("cluster0", 100*time.Second)
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	secret := &corev1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace: "default",
-			Name:      "secret",
-		},
-		Data: map[string][]byte{
-			TLSCertFile: cert,
-			TLSKeyFile:  key,
-		},
-	}
-
-	if hasValidKubeconfig(secret) {
-		t.Error("kubeconfig is invalid")
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			notBefore, notAfter, err := getCertValidityPeriod(c.secret)
+			testinghelpers.AssertError(t, err, c.expectedErr)
+			if c.expectedCert == nil {
+				return
+			}
+			if !c.expectedCert.NotBefore.Equal(*notBefore) {
+				t.Errorf("expect %v, but got %v", expectedCerts[0].NotBefore, *notBefore)
+			}
+			if !c.expectedCert.NotAfter.Equal(*notAfter) {
+				t.Errorf("expect %v, but got %v", expectedCerts[0].NotAfter, *notAfter)
+			}
+		})
 	}
 }

pkg/spoke/hubclientcert/controller.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/openshift/library-go/pkg/controller/factory"
 	"github.com/openshift/library-go/pkg/operator/events"
+
 	certificates "k8s.io/api/certificates/v1beta1"
 	corev1 "k8s.io/api/core/v1"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"

pkg/spoke/hubclientcert/controller_test.go

@@ -3,405 +3,186 @@ package hubclientcert
 import (
 	"context"
 	"fmt"
+	"reflect"
 	"testing"
 	"time"
 
-	"github.com/openshift/library-go/pkg/controller/factory"
-	"github.com/openshift/library-go/pkg/operator/events"
+	testinghelpers "github.com/open-cluster-management/registration/pkg/helpers/testing"
+
 	certificates "k8s.io/api/certificates/v1beta1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/informers"
 	kubefake "k8s.io/client-go/kubernetes/fake"
-	"k8s.io/client-go/tools/cache"
+	clienttesting "k8s.io/client-go/testing"
 	"k8s.io/client-go/tools/clientcmd"
-	"k8s.io/client-go/util/keyutil"
 )
 
-func newSecret(namespace, name, resourceVersion string, data map[string][]byte) *corev1.Secret {
-	secret := &corev1.Secret{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace:       namespace,
-			Name:            name,
-			ResourceVersion: resourceVersion,
-		},
-		Data: data,
-	}
+const (
+	testNamespace  = "testns"
+	testAgentName  = "testagent"
+	testSecretName = "testsecret"
+	testCSRName    = "testcsr"
+)
 
-	return secret
-}
+var commonName = fmt.Sprintf("%s%s:%s", subjectPrefix, testinghelpers.TestManagedClusterName, testAgentName)
 
-func TestSyncCSR(t *testing.T) {
-	secretNamespace := "default"
-	secretName := "secret"
-	secret := newSecret(secretNamespace, secretName, "", nil)
-
-	key, cert, err := newCertKey("cluster0", 10*time.Second)
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	csrName := "csr1"
-	csr := &certificates.CertificateSigningRequest{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: csrName,
+func TestSync(t *testing.T) {
+	cases := []struct {
+		name            string
+		secrets         []runtime.Object
+		approvedCSRCert *testinghelpers.TestCert
+		keyDataExpected bool
+		csrNameExpected bool
+		validateActions func(t *testing.T, hubActions, agentActions []clienttesting.Action)
+	}{
+		{
+			name:            "agent bootstrap",
+			secrets:         []runtime.Object{},
+			keyDataExpected: true,
+			csrNameExpected: true,
+			validateActions: func(t *testing.T, hubActions, agentActions []clienttesting.Action) {
+				testinghelpers.AssertActions(t, hubActions, "create")
+				actual := hubActions[0].(clienttesting.CreateActionImpl).Object
+				if _, ok := actual.(*certificates.CertificateSigningRequest); !ok {
+					t.Errorf("expected csr was created, but failed")
+				}
+				expectedSecret := &corev1.Secret{
+					ObjectMeta: metav1.ObjectMeta{
+						Namespace: testNamespace,
+						Name:      testSecretName,
+					},
+					Data: map[string][]byte{
+						ClusterNameFile: []byte(testinghelpers.TestManagedClusterName),
+						AgentNameFile:   []byte(testAgentName),
+					},
+				}
+				testinghelpers.AssertActions(t, agentActions, "create")
+				actualSecret := agentActions[0].(clienttesting.CreateActionImpl).Object
+				if !reflect.DeepEqual(expectedSecret, actualSecret) {
+					t.Errorf("expected secret %v, but got %v", expectedSecret, actualSecret)
+				}
+			},
 		},
-		Spec: certificates.CertificateSigningRequestSpec{},
-		Status: certificates.CertificateSigningRequestStatus{
-			Conditions: []certificates.CertificateSigningRequestCondition{
-				{
-					Type: certificates.CertificateApproved,
+		{
+			name: "syc csr after bootstrap",
+			secrets: []runtime.Object{
+				testinghelpers.NewHubKubeconfigSecret(testNamespace, testSecretName, "1", nil, map[string][]byte{
+					ClusterNameFile: []byte(testinghelpers.TestManagedClusterName),
+					AgentNameFile:   []byte(testAgentName),
 				},
+				),
+			},
+			approvedCSRCert: testinghelpers.NewTestCert(testinghelpers.TestManagedClusterName, 10*time.Second),
+			validateActions: func(t *testing.T, hubActions, agentActions []clienttesting.Action) {
+				testinghelpers.AssertActions(t, hubActions, "get")
+				testinghelpers.AssertActions(t, agentActions, "update")
+				actual := agentActions[0].(clienttesting.UpdateActionImpl).Object
+				if !hasValidKubeconfig(actual.(*corev1.Secret)) {
+					t.Error("kubeconfig secret is invalid")
+				}
+			},
+		},
+		{
+			name: "sync a valid hub kubeconfig secret",
+			secrets: []runtime.Object{
+				testinghelpers.NewHubKubeconfigSecret(testNamespace, testSecretName, "1", testinghelpers.NewTestCert(commonName, 100*time.Second), map[string][]byte{
+					ClusterNameFile: []byte(testinghelpers.TestManagedClusterName),
+					AgentNameFile:   []byte(testAgentName),
+					KubeconfigFile:  testinghelpers.NewKubeconfig(nil, nil),
+				}),
+			},
+			validateActions: func(t *testing.T, hubActions, agentActions []clienttesting.Action) {
+				testinghelpers.AssertNoActions(t, hubActions)
+				testinghelpers.AssertNoActions(t, agentActions)
+			},
+		},
+		{
+			name: "sync an expiring hub kubeconfig secret",
+			secrets: []runtime.Object{
+				testinghelpers.NewHubKubeconfigSecret(testNamespace, testSecretName, "1", testinghelpers.NewTestCert(commonName, -3*time.Second), map[string][]byte{
+					ClusterNameFile: []byte(testinghelpers.TestManagedClusterName),
+					AgentNameFile:   []byte(testAgentName),
+					KubeconfigFile:  testinghelpers.NewKubeconfig(nil, nil),
+				}),
+			},
+			keyDataExpected: true,
+			csrNameExpected: true,
+			validateActions: func(t *testing.T, hubActions, agentActions []clienttesting.Action) {
+				testinghelpers.AssertActions(t, hubActions, "create")
+				actual := hubActions[0].(clienttesting.CreateActionImpl).Object
+				if _, ok := actual.(*certificates.CertificateSigningRequest); !ok {
+					t.Errorf("expected csr was created, but failed")
+				}
+				testinghelpers.AssertNoActions(t, agentActions)
 			},
-			Certificate: cert,
 		},
 	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			csrs := []runtime.Object{}
+			if c.approvedCSRCert != nil {
+				csr := testinghelpers.NewApprovedCSR(testinghelpers.CSRHolder{Name: testCSRName})
+				csr.Status.Certificate = c.approvedCSRCert.Cert
+				csrs = append(csrs, csr)
+			}
+			hubKubeClient := kubefake.NewSimpleClientset(csrs...)
+			// GenerateName is not working for fake clent, we set the name with prepend reactor
+			hubKubeClient.PrependReactor(
+				"create",
+				"certificatesigningrequests",
+				func(action clienttesting.Action) (handled bool, ret runtime.Object, err error) {
+					return true, testinghelpers.NewCSR(testinghelpers.CSRHolder{Name: testCSRName}), nil
+				},
+			)
+			hubInformerFactory := informers.NewSharedInformerFactory(hubKubeClient, 3*time.Minute)
 
-	fakeKubeClient := kubefake.NewSimpleClientset(csr, secret)
-	csrInformer := informers.NewSharedInformerFactory(fakeKubeClient,
-		3*time.Minute).Certificates().V1beta1().CertificateSigningRequests()
+			agentKubeClient := kubefake.NewSimpleClientset(c.secrets...)
+			agentInformerFactory := informers.NewSharedInformerFactory(agentKubeClient, 3*time.Minute)
+			secretStore := agentInformerFactory.Core().V1().Secrets().Informer().GetStore()
+			for _, secret := range c.secrets {
+				secretStore.Add(secret)
+			}
 
-	// create csr informer/lister
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-	go csrInformer.Informer().Run(ctx.Done())
-	if ok := cache.WaitForCacheSync(ctx.Done(), csrInformer.Informer().HasSynced); !ok {
-		t.Error("failed to wait for kubernetes caches to sync")
-	}
+			controller := &ClientCertForHubController{
+				clusterName:                  testinghelpers.TestManagedClusterName,
+				agentName:                    testAgentName,
+				hubKubeconfigSecretNamespace: testNamespace,
+				hubKubeconfigSecretName:      testSecretName,
+				hubCSRLister:                 hubInformerFactory.Certificates().V1beta1().CertificateSigningRequests().Lister(),
+				hubCSRClient:                 hubKubeClient.CertificatesV1beta1().CertificateSigningRequests(),
+				spokeSecretLister:            agentInformerFactory.Core().V1().Secrets().Lister(),
+				spokeCoreClient:              agentKubeClient.CoreV1(),
+			}
 
-	// create a fake client config as template
-	kubeconfigData, err := clientcmd.Write(newKubeconfig(key, cert))
-	clientConfig, err := clientcmd.RESTConfigFromKubeConfig(kubeconfigData)
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
+			if c.approvedCSRCert != nil {
+				controller.csrName = testCSRName
+				controller.keyData = c.approvedCSRCert.Key
+				kubeconfig := testinghelpers.NewKubeconfig(c.approvedCSRCert.Key, c.approvedCSRCert.Cert)
+				clientConfig, err := clientcmd.RESTConfigFromKubeConfig(kubeconfig)
+				if err != nil {
+					t.Errorf("unexpected error: %v", err)
+				}
+				controller.hubClientConfig = clientConfig
+			}
 
-	controller := &ClientCertForHubController{
-		csrName:         csrName,
-		keyData:         key,
-		hubCSRLister:    csrInformer.Lister(),
-		hubClientConfig: clientConfig,
-		hubCSRClient:    fakeKubeClient.CertificatesV1beta1().CertificateSigningRequests(),
-	}
-	newSecretConfig, err := controller.syncCSR(secret)
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-	if newSecretConfig == nil {
-		t.Error("secret should be changed")
-	}
+			err := controller.sync(context.TODO(), testinghelpers.NewFakeSyncContext(t, ""))
+			if err != nil {
+				t.Errorf("unexpected error %v", err)
+			}
 
-	// check if csrName/keyData are cleared
-	if controller.csrName != "" {
-		t.Error("controller.csrName should be empty")
-	}
-	if controller.keyData != nil {
-		t.Error("controller.keyData should be nil")
-	}
+			hasKeyData := controller.keyData != nil
+			if c.keyDataExpected != hasKeyData {
+				t.Error("controller.keyData should be set")
+			}
 
-	// validate the kubeconfig in secret
-	secret.Data = newSecretConfig
-	if !hasValidKubeconfig(secret) {
-		t.Error("kubeconfig should be valid")
-	}
-}
+			hasCSRName := controller.csrName != ""
+			if c.csrNameExpected != hasCSRName {
+				t.Error("controller.csrName should be set")
+			}
 
-// test bootstrap
-func TestSyncWithoutHubKubeconfigSecret(t *testing.T) {
-	secretNamespace := "default"
-	secretName := "secret"
-
-	clusterName := "cluster0"
-	agentName := "agent0"
-	fakeKubeClient := kubefake.NewSimpleClientset()
-
-	// create secret informer/lister
-	secretInformer := informers.NewSharedInformerFactory(fakeKubeClient,
-		3*time.Minute).Core().V1().Secrets()
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-	go secretInformer.Informer().Run(ctx.Done())
-	if ok := cache.WaitForCacheSync(ctx.Done(), secretInformer.Informer().HasSynced); !ok {
-		t.Error("failed to wait for kubernetes caches to sync")
-	}
-
-	controller := &ClientCertForHubController{
-		clusterName:                  clusterName,
-		agentName:                    agentName,
-		hubKubeconfigSecretNamespace: secretNamespace,
-		hubKubeconfigSecretName:      secretName,
-		spokeCoreClient:              fakeKubeClient.CoreV1(),
-		hubCSRClient:                 fakeKubeClient.CertificatesV1beta1().CertificateSigningRequests(),
-		spokeSecretLister:            secretInformer.Lister(),
-	}
-
-	eventRecorder := events.NewInMemoryRecorder("")
-	syncContext := factory.NewSyncContext("", eventRecorder)
-	err := controller.sync(nil, syncContext)
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	secret, err := fakeKubeClient.CoreV1().Secrets(secretNamespace).Get(context.Background(), secretName, metav1.GetOptions{})
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	// check if cluster/agent name are stored into secret
-	if len(secret.Data) == 0 {
-		t.Errorf("secret should have data stored")
-	}
-	if string(secret.Data[ClusterNameFile]) != clusterName {
-		t.Errorf("expected cluster name %q but got: %s", clusterName, string(secret.Data[ClusterNameFile]))
-	}
-	if string(secret.Data[AgentNameFile]) != agentName {
-		t.Errorf("expected agent name %q but got: %s", agentName, string(secret.Data[AgentNameFile]))
-	}
-
-	// check if there is new csr created
-	csrs, err := fakeKubeClient.CertificatesV1beta1().CertificateSigningRequests().List(context.Background(), metav1.ListOptions{})
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	if len(csrs.Items) != 1 {
-		t.Errorf("expect 1 csr created, but got: %d", len(csrs.Items))
-	}
-
-	// check if csrName/keyData are set. Since GenerateName is not working for fake clent, check keyData only
-	if controller.keyData == nil {
-		t.Error("controller.keyData should be set")
-	}
-}
-
-func TestSyncWithValidKubeconfig(t *testing.T) {
-	kubeconfigData, err := clientcmd.Write(newKubeconfig(nil, nil))
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	clusterName := "cluster0"
-	agentName := "agent0"
-	key, cert, err := newCertKey(fmt.Sprintf("%s%s:%s", subjectPrefix, clusterName, agentName), 100*time.Second)
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	secretNamespace := "default"
-	secretName := "secret"
-	secret := newSecret(secretNamespace, secretName, "1", map[string][]byte{
-		KubeconfigFile: kubeconfigData,
-		TLSCertFile:    cert,
-		TLSKeyFile:     key,
-	})
-
-	fakeKubeClient := kubefake.NewSimpleClientset(secret)
-
-	// create secret informer/lister
-	secretInformer := informers.NewSharedInformerFactory(fakeKubeClient,
-		3*time.Minute).Core().V1().Secrets()
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-	go secretInformer.Informer().Run(ctx.Done())
-	if ok := cache.WaitForCacheSync(ctx.Done(), secretInformer.Informer().HasSynced); !ok {
-		t.Error("failed to wait for kubernetes caches to sync")
-	}
-
-	controller := &ClientCertForHubController{
-		clusterName:                  clusterName,
-		agentName:                    agentName,
-		hubKubeconfigSecretNamespace: secretNamespace,
-		hubKubeconfigSecretName:      secretName,
-		spokeCoreClient:              fakeKubeClient.CoreV1(),
-		hubCSRClient:                 fakeKubeClient.CertificatesV1beta1().CertificateSigningRequests(),
-		spokeSecretLister:            secretInformer.Lister(),
-	}
-
-	eventRecorder := events.NewInMemoryRecorder("")
-	syncContext := factory.NewSyncContext("", eventRecorder)
-	err = controller.sync(nil, syncContext)
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	// check if there is any csr created
-	csrs, err := fakeKubeClient.CertificatesV1beta1().CertificateSigningRequests().List(context.Background(), metav1.ListOptions{})
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	if len(csrs.Items) != 0 {
-		t.Errorf("expect 0 csr created, but got: %d", len(csrs.Items))
-	}
-
-	// check if csrName/keyData are unset
-	if controller.csrName != "" {
-		t.Error("controller.csrName should be empty")
-	}
-	if controller.keyData != nil {
-		t.Error("controller.keyData should be nil")
-	}
-}
-
-// test cert rotation
-func TestSyncWithExpiringCert(t *testing.T) {
-	kubeconfigData, err := clientcmd.Write(newKubeconfig(nil, nil))
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	clusterName := "cluster0"
-	agentName := "agent0"
-
-	key, cert, err := newCertKey(fmt.Sprintf("%s%s:%s", subjectPrefix, clusterName, agentName), -3*time.Second)
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	secretNamespace := "default"
-	secretName := "secret"
-	secret := newSecret(secretNamespace, secretName, "1", map[string][]byte{
-		KubeconfigFile: kubeconfigData,
-		TLSCertFile:    cert,
-		TLSKeyFile:     key,
-	})
-
-	fakeKubeClient := kubefake.NewSimpleClientset(secret)
-
-	// create secret informer/lister
-	secretInformer := informers.NewSharedInformerFactory(fakeKubeClient,
-		3*time.Minute).Core().V1().Secrets()
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
-	go secretInformer.Informer().Run(ctx.Done())
-	if ok := cache.WaitForCacheSync(ctx.Done(), secretInformer.Informer().HasSynced); !ok {
-		t.Error("failed to wait for kubernetes caches to sync")
-	}
-
-	controller := &ClientCertForHubController{
-		clusterName:                  clusterName,
-		agentName:                    agentName,
-		hubKubeconfigSecretNamespace: secretNamespace,
-		hubKubeconfigSecretName:      secretName,
-		spokeCoreClient:              fakeKubeClient.CoreV1(),
-		hubCSRClient:                 fakeKubeClient.CertificatesV1beta1().CertificateSigningRequests(),
-		spokeSecretLister:            secretInformer.Lister(),
-	}
-
-	eventRecorder := events.NewInMemoryRecorder("")
-	syncContext := factory.NewSyncContext("", eventRecorder)
-	err = controller.sync(nil, syncContext)
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	// check if there is any csr created
-	csrs, err := fakeKubeClient.CertificatesV1beta1().CertificateSigningRequests().List(context.Background(), metav1.ListOptions{})
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	if len(csrs.Items) != 1 {
-		t.Errorf("expect 1 csr created, but got: %d", len(csrs.Items))
-	}
-
-	// check if csrName/keyData are set. Since GenerateName is not working for fake clent, check keyData only
-	if controller.keyData == nil {
-		t.Error("controller.keyData should be set")
-	}
-}
-
-func TestCreateCSR(t *testing.T) {
-	fakeKubeClient := kubefake.NewSimpleClientset()
-
-	keyData, err := keyutil.MakeEllipticPrivateKeyPEM()
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-	controller := &ClientCertForHubController{
-		clusterName:  "cluster0",
-		agentName:    "agent0",
-		keyData:      keyData,
-		hubCSRClient: fakeKubeClient.CertificatesV1beta1().CertificateSigningRequests(),
-	}
-
-	csrName, err := controller.createCSR()
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	_, err = fakeKubeClient.CertificatesV1beta1().CertificateSigningRequests().Get(context.Background(), csrName, metav1.GetOptions{})
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-}
-
-func TestSaveHubKubeconfigSecret(t *testing.T) {
-	secretNamespace := "default"
-	secretName := "secret"
-
-	fakeKubeClient := kubefake.NewSimpleClientset()
-
-	controller := &ClientCertForHubController{
-		hubKubeconfigSecretNamespace: secretNamespace,
-		hubKubeconfigSecretName:      secretName,
-		spokeCoreClient:              fakeKubeClient.CoreV1(),
-	}
-
-	key, value := "key", "value"
-	secret := newSecret(secretNamespace, secretName, "", map[string][]byte{
-		key: []byte(value),
-	})
-	err := controller.saveHubKubeconfigSecret(secret)
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	secret, err = fakeKubeClient.CoreV1().Secrets(secretNamespace).Get(context.Background(), secretName, metav1.GetOptions{})
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-	if len(secret.Data) == 0 {
-		t.Error("secret should have data stored")
-	}
-	if string(secret.Data[key]) != value {
-		t.Errorf("expected %q but get: %s", value, string(secret.Data[key]))
-	}
-}
-
-func TestSaveHubKubeconfigSecretWithExistingSecret(t *testing.T) {
-	secretNamespace := "default"
-	secretName := "secret"
-	secret := newSecret(secretNamespace, secretName, "1", nil)
-
-	fakeKubeClient := kubefake.NewSimpleClientset(secret)
-	controller := &ClientCertForHubController{
-		hubKubeconfigSecretNamespace: secretNamespace,
-		hubKubeconfigSecretName:      secretName,
-		spokeCoreClient:              fakeKubeClient.CoreV1(),
-	}
-
-	key, value := "key", "value"
-	secret.Data = map[string][]byte{
-		key: []byte(value),
-	}
-
-	err := controller.saveHubKubeconfigSecret(secret)
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	secret, err = fakeKubeClient.CoreV1().Secrets(secretNamespace).Get(context.Background(), secretName, metav1.GetOptions{})
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-	if len(secret.Data) == 0 {
-		t.Error("secret should have data stored")
-	}
-	if string(secret.Data[key]) != value {
-		t.Errorf("expected %q but get: %s", value, string(secret.Data[key]))
+			c.validateActions(t, hubKubeClient.Actions(), agentKubeClient.Actions())
+		})
 	}
 }

pkg/spoke/managedcluster/creating_controller_test.go

@@ -1,13 +1,13 @@
 package managedcluster
 
 import (
-	"bytes"
 	"context"
 	"testing"
 
 	clusterfake "github.com/open-cluster-management/api/client/cluster/clientset/versioned/fake"
 	clusterv1 "github.com/open-cluster-management/api/cluster/v1"
 	testinghelpers "github.com/open-cluster-management/registration/pkg/helpers/testing"
+
 	"k8s.io/apimachinery/pkg/runtime"
 	clienttesting "k8s.io/client-go/testing"
 )
@@ -19,23 +19,28 @@ func TestCreateSpokeCluster(t *testing.T) {
 		name            string
 		startingObjects []runtime.Object
 		validateActions func(t *testing.T, actions []clienttesting.Action)
-		expectedErr     string
 	}{
 		{
 			name:            "create a new cluster",
 			startingObjects: []runtime.Object{},
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				assertActions(t, actions, "get", "create")
+				expectedClientConfigs := []clusterv1.ClientConfig{
+					{
+						URL:      testSpokeExternalServerUrl,
+						CABundle: []byte("testcabundle"),
+					},
+				}
+				testinghelpers.AssertActions(t, actions, "get", "create")
 				actual := actions[1].(clienttesting.CreateActionImpl).Object
-				assertSpokeExternalServerUrl(t, actual, testSpokeExternalServerUrl)
-				assertSpokeCABundle(t, actual, []byte("testcabundle"))
+				actualClientConfigs := actual.(*clusterv1.ManagedCluster).Spec.ManagedClusterClientConfigs
+				testinghelpers.AssertManagedClusterClientConfigs(t, actualClientConfigs, expectedClientConfigs)
 			},
 		},
 		{
 			name:            "create an existed cluster",
-			startingObjects: []runtime.Object{newManagedCluster()},
+			startingObjects: []runtime.Object{testinghelpers.NewManagedCluster()},
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				assertActions(t, actions, "get")
+				testinghelpers.AssertActions(t, actions, "get")
 			},
 		},
 	}
@@ -44,22 +49,14 @@ func TestCreateSpokeCluster(t *testing.T) {
 		t.Run(c.name, func(t *testing.T) {
 			clusterClient := clusterfake.NewSimpleClientset(c.startingObjects...)
 			ctrl := managedClusterCreatingController{
-				clusterName:             testManagedClusterName,
+				clusterName:             testinghelpers.TestManagedClusterName,
 				spokeExternalServerURLs: []string{testSpokeExternalServerUrl},
 				spokeCABundle:           []byte("testcabundle"),
 				hubClusterClient:        clusterClient,
 			}
 
 			syncErr := ctrl.sync(context.TODO(), testinghelpers.NewFakeSyncContext(t, ""))
-			if len(c.expectedErr) > 0 && syncErr == nil {
-				t.Errorf("expected %q error", c.expectedErr)
-				return
-			}
-			if len(c.expectedErr) > 0 && syncErr != nil && syncErr.Error() != c.expectedErr {
-				t.Errorf("expected %q error, got %q", c.expectedErr, syncErr.Error())
-				return
-			}
-			if len(c.expectedErr) == 0 && syncErr != nil {
+			if syncErr != nil {
 				t.Errorf("unexpected err: %v", syncErr)
 			}
 
@@ -67,23 +64,3 @@ func TestCreateSpokeCluster(t *testing.T) {
 		})
 	}
 }
-
-func assertSpokeExternalServerUrl(t *testing.T, actual runtime.Object, expected string) {
-	spokeCluster := actual.(*clusterv1.ManagedCluster)
-	if len(spokeCluster.Spec.ManagedClusterClientConfigs) != 1 {
-		t.Errorf("expected one spoke client config, but got %v", spokeCluster.Spec.ManagedClusterClientConfigs)
-	}
-	if spokeCluster.Spec.ManagedClusterClientConfigs[0].URL != expected {
-		t.Errorf("expected %q error, but got %q", expected, spokeCluster.Spec.ManagedClusterClientConfigs[0].URL)
-	}
-}
-
-func assertSpokeCABundle(t *testing.T, actual runtime.Object, expected []byte) {
-	spokeCluster := actual.(*clusterv1.ManagedCluster)
-	if len(spokeCluster.Spec.ManagedClusterClientConfigs) != 1 {
-		t.Errorf("expected one spoke client config, but got %v", spokeCluster.Spec.ManagedClusterClientConfigs)
-	}
-	if !bytes.Equal(spokeCluster.Spec.ManagedClusterClientConfigs[0].CABundle, expected) {
-		t.Errorf("expected %q error, but got %q", expected, spokeCluster.Spec.ManagedClusterClientConfigs[0].CABundle)
-	}
-}

pkg/spoke/managedcluster/healthcheck_controller_test.go

@@ -47,76 +47,74 @@ func TestHealthCheck(t *testing.T) {
 		expectedErr     string
 	}{
 		{
-			name:     "there are no managed clusters",
-			clusters: []runtime.Object{},
-			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				assertActions(t, actions)
-			},
-			expectedErr: "unable to get managed cluster \"testmanagedcluster\" from hub: managedcluster.cluster.open-cluster-management.io \"testmanagedcluster\" not found",
+			name:            "there are no managed clusters",
+			clusters:        []runtime.Object{},
+			validateActions: testinghelpers.AssertNoActions,
+			expectedErr:     "unable to get managed cluster \"testmanagedcluster\" from hub: managedcluster.cluster.open-cluster-management.io \"testmanagedcluster\" not found",
 		},
 		{
 			name:        "kube-apiserver is not health",
-			clusters:    []runtime.Object{newAcceptedManagedCluster()},
+			clusters:    []runtime.Object{testinghelpers.NewAcceptedManagedCluster()},
 			httpStatus:  http.StatusInternalServerError,
 			responseMsg: "internal server error",
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				assertActions(t, actions, "get", "update")
-				actual := actions[1].(clienttesting.UpdateActionImpl).Object
 				expectedCondition := clusterv1.StatusCondition{
 					Type:    clusterv1.ManagedClusterConditionAvailable,
 					Status:  metav1.ConditionFalse,
 					Reason:  "ManagedClusterKubeAPIServerUnavailable",
 					Message: "The kube-apiserver is not ok, status code: 500, an error on the server (\"internal server error\") has prevented the request from succeeding",
 				}
-				assertCondition(t, actual, expectedCondition)
+				testinghelpers.AssertActions(t, actions, "get", "update")
+				actual := actions[1].(clienttesting.UpdateActionImpl).Object
+				testinghelpers.AssertManagedClusterCondition(t, actual.(*clusterv1.ManagedCluster).Status.Conditions, expectedCondition)
 			},
 		},
 		{
 			name:       "kube-apiserver is ok",
-			clusters:   []runtime.Object{newAcceptedManagedCluster()},
+			clusters:   []runtime.Object{testinghelpers.NewAcceptedManagedCluster()},
 			httpStatus: http.StatusOK,
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				assertActions(t, actions, "get", "update")
-				actual := actions[1].(clienttesting.UpdateActionImpl).Object
 				expectedCondition := clusterv1.StatusCondition{
 					Type:    clusterv1.ManagedClusterConditionAvailable,
 					Status:  metav1.ConditionTrue,
 					Reason:  "ManagedClusterAvailable",
 					Message: "Managed cluster is available",
 				}
-				assertCondition(t, actual, expectedCondition)
+				testinghelpers.AssertActions(t, actions, "get", "update")
+				actual := actions[1].(clienttesting.UpdateActionImpl).Object
+				testinghelpers.AssertManagedClusterCondition(t, actual.(*clusterv1.ManagedCluster).Status.Conditions, expectedCondition)
 			},
 		},
 		{
 			name:       "there is no readyz endpoint",
-			clusters:   []runtime.Object{newAcceptedManagedCluster()},
+			clusters:   []runtime.Object{testinghelpers.NewAcceptedManagedCluster()},
 			httpStatus: http.StatusNotFound,
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				assertActions(t, actions, "get", "update")
-				actual := actions[1].(clienttesting.UpdateActionImpl).Object
 				expectedCondition := clusterv1.StatusCondition{
 					Type:    clusterv1.ManagedClusterConditionAvailable,
 					Status:  metav1.ConditionTrue,
 					Reason:  "ManagedClusterAvailable",
 					Message: "Managed cluster is available",
 				}
-				assertCondition(t, actual, expectedCondition)
+				testinghelpers.AssertActions(t, actions, "get", "update")
+				actual := actions[1].(clienttesting.UpdateActionImpl).Object
+				testinghelpers.AssertManagedClusterCondition(t, actual.(*clusterv1.ManagedCluster).Status.Conditions, expectedCondition)
 			},
 		},
 		{
 			name:       "readyz is forbidden",
-			clusters:   []runtime.Object{newAcceptedManagedCluster()},
+			clusters:   []runtime.Object{testinghelpers.NewAcceptedManagedCluster()},
 			httpStatus: http.StatusForbidden,
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				assertActions(t, actions, "get", "update")
-				actual := actions[1].(clienttesting.UpdateActionImpl).Object
 				expectedCondition := clusterv1.StatusCondition{
 					Type:    clusterv1.ManagedClusterConditionAvailable,
 					Status:  metav1.ConditionTrue,
 					Reason:  "ManagedClusterAvailable",
 					Message: "Managed cluster is available",
 				}
-				assertCondition(t, actual, expectedCondition)
+				testinghelpers.AssertActions(t, actions, "get", "update")
+				actual := actions[1].(clienttesting.UpdateActionImpl).Object
+				testinghelpers.AssertManagedClusterCondition(t, actual.(*clusterv1.ManagedCluster).Status.Conditions, expectedCondition)
 			},
 		},
 	}
@@ -133,23 +131,13 @@ func TestHealthCheck(t *testing.T) {
 			serverResponse.responseMsg = c.responseMsg
 
 			ctrl := &managedClusterHealthCheckController{
-				clusterName:                   testManagedClusterName,
+				clusterName:                   testinghelpers.TestManagedClusterName,
 				hubClusterClient:              clusterClient,
 				hubClusterLister:              clusterInformerFactory.Cluster().V1().ManagedClusters().Lister(),
 				managedClusterDiscoveryClient: discoveryClient,
 			}
 			syncErr := ctrl.sync(context.TODO(), testinghelpers.NewFakeSyncContext(t, ""))
-			if len(c.expectedErr) > 0 && syncErr == nil {
-				t.Errorf("expected %q error", c.expectedErr)
-				return
-			}
-			if len(c.expectedErr) > 0 && syncErr != nil && syncErr.Error() != c.expectedErr {
-				t.Errorf("expected %q error, got %q", c.expectedErr, syncErr.Error())
-				return
-			}
-			if len(c.expectedErr) == 0 && syncErr != nil {
-				t.Errorf("unexpected err: %v", syncErr)
-			}
+			testinghelpers.AssertError(t, syncErr, c.expectedErr)
 
 			c.validateActions(t, clusterClient.Actions())
 		})

pkg/spoke/managedcluster/joining_controller_test.go

@@ -2,30 +2,23 @@ package managedcluster
 
 import (
 	"context"
-	"reflect"
 	"testing"
 	"time"
 
 	clusterfake "github.com/open-cluster-management/api/client/cluster/clientset/versioned/fake"
 	clusterinformers "github.com/open-cluster-management/api/client/cluster/informers/externalversions"
 	clusterv1 "github.com/open-cluster-management/api/cluster/v1"
-	"github.com/open-cluster-management/registration/pkg/helpers"
-
 	testinghelpers "github.com/open-cluster-management/registration/pkg/helpers/testing"
 
-	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/version"
 	kubeinformers "k8s.io/client-go/informers"
 	kubefake "k8s.io/client-go/kubernetes/fake"
 	kubeversion "k8s.io/client-go/pkg/version"
 	clienttesting "k8s.io/client-go/testing"
 )
 
-const testManagedClusterName = "testmanagedcluster"
-
 func TestSyncManagedCluster(t *testing.T) {
 	cases := []struct {
 		name            string
@@ -37,67 +30,89 @@ func TestSyncManagedCluster(t *testing.T) {
 		{
 			name:            "sync no managed cluster",
 			startingObjects: []runtime.Object{},
-			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				if len(actions) != 0 {
-					t.Errorf("expected 0 call but got: %#v", actions)
-				}
-			},
-			expectedErr: "unable to get managed cluster with name \"testmanagedcluster\" from hub: managedcluster.cluster.open-cluster-management.io \"testmanagedcluster\" not found",
+			validateActions: testinghelpers.AssertNoActions,
+			expectedErr:     "unable to get managed cluster with name \"testmanagedcluster\" from hub: managedcluster.cluster.open-cluster-management.io \"testmanagedcluster\" not found",
 		},
 		{
 			name:            "sync an unaccepted managed cluster",
-			startingObjects: []runtime.Object{newManagedCluster()},
-			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				if len(actions) != 0 {
-					t.Errorf("expected 0 call but got: %#v", actions)
-				}
-			},
+			startingObjects: []runtime.Object{testinghelpers.NewManagedCluster()},
+			validateActions: testinghelpers.AssertNoActions,
 		},
 		{
 			name:            "sync an accepted managed cluster",
-			startingObjects: []runtime.Object{newAcceptedManagedCluster()},
-			nodes:           []runtime.Object{newNode("testnode1", newResourceList(32, 64), newResourceList(16, 32))},
+			startingObjects: []runtime.Object{testinghelpers.NewAcceptedManagedCluster()},
+			nodes: []runtime.Object{
+				testinghelpers.NewNode("testnode1", testinghelpers.NewResourceList(32, 64), testinghelpers.NewResourceList(16, 32)),
+			},
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				assertActions(t, actions, "get", "update")
-				actual := actions[1].(clienttesting.UpdateActionImpl).Object
 				expectedCondition := clusterv1.StatusCondition{
 					Type:    clusterv1.ManagedClusterConditionJoined,
 					Status:  metav1.ConditionTrue,
 					Reason:  "ManagedClusterJoined",
 					Message: "Managed cluster joined",
 				}
-				assertCondition(t, actual, expectedCondition)
-				assertStatusVersion(t, actual, kubeversion.Get())
-				assertStatusResource(t, actual, newResourceList(32, 64), newResourceList(16, 32))
+				expectedStatus := clusterv1.ManagedClusterStatus{
+					Version: clusterv1.ManagedClusterVersion{
+						Kubernetes: kubeversion.Get().GitVersion,
+					},
+					Capacity: clusterv1.ResourceList{
+						clusterv1.ResourceCPU:    *resource.NewQuantity(int64(32), resource.DecimalExponent),
+						clusterv1.ResourceMemory: *resource.NewQuantity(int64(1024*1024*64), resource.BinarySI),
+					},
+					Allocatable: clusterv1.ResourceList{
+						clusterv1.ResourceCPU:    *resource.NewQuantity(int64(16), resource.DecimalExponent),
+						clusterv1.ResourceMemory: *resource.NewQuantity(int64(1024*1024*32), resource.BinarySI),
+					},
+				}
+				testinghelpers.AssertActions(t, actions, "get", "update")
+				actual := actions[1].(clienttesting.UpdateActionImpl).Object
+				testinghelpers.AssertManagedClusterCondition(t, actual.(*clusterv1.ManagedCluster).Status.Conditions, expectedCondition)
+				testinghelpers.AssertManagedClusterStatus(t, actual.(*clusterv1.ManagedCluster).Status, expectedStatus)
 			},
 		},
 		{
-			name:            "sync a joined managed cluster without status change",
-			startingObjects: []runtime.Object{newJoinedManagedCluster(newResourceList(32, 64), newResourceList(16, 32))},
-			nodes:           []runtime.Object{newNode("testnode1", newResourceList(32, 64), newResourceList(16, 32))},
+			name: "sync a joined managed cluster without status change",
+			startingObjects: []runtime.Object{
+				testinghelpers.NewManagedClusterWithStatus(testinghelpers.NewResourceList(32, 64), testinghelpers.NewResourceList(16, 32)),
+			},
+			nodes: []runtime.Object{
+				testinghelpers.NewNode("testnode1", testinghelpers.NewResourceList(32, 64), testinghelpers.NewResourceList(16, 32)),
+			},
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				assertActions(t, actions, "get")
+				testinghelpers.AssertActions(t, actions, "get")
 			},
 		},
 		{
 			name:            "sync a joined managed cluster with status change",
-			startingObjects: []runtime.Object{newJoinedManagedCluster(newResourceList(32, 64), newResourceList(16, 32))},
+			startingObjects: []runtime.Object{testinghelpers.NewJoinedManagedCluster()},
 			nodes: []runtime.Object{
-				newNode("testnode1", newResourceList(32, 64), newResourceList(16, 32)),
-				newNode("testnode2", newResourceList(32, 64), newResourceList(16, 32)),
+				testinghelpers.NewNode("testnode1", testinghelpers.NewResourceList(32, 64), testinghelpers.NewResourceList(16, 32)),
+				testinghelpers.NewNode("testnode2", testinghelpers.NewResourceList(32, 64), testinghelpers.NewResourceList(16, 32)),
 			},
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				assertActions(t, actions, "get", "update")
-				actual := actions[1].(clienttesting.UpdateActionImpl).Object
 				expectedCondition := clusterv1.StatusCondition{
 					Type:    clusterv1.ManagedClusterConditionJoined,
 					Status:  metav1.ConditionTrue,
 					Reason:  "ManagedClusterJoined",
 					Message: "Managed cluster joined",
 				}
-				assertCondition(t, actual, expectedCondition)
-				assertStatusVersion(t, actual, kubeversion.Get())
-				assertStatusResource(t, actual, newResourceList(64, 128), newResourceList(32, 64))
+				expectedStatus := clusterv1.ManagedClusterStatus{
+					Version: clusterv1.ManagedClusterVersion{
+						Kubernetes: kubeversion.Get().GitVersion,
+					},
+					Capacity: clusterv1.ResourceList{
+						clusterv1.ResourceCPU:    *resource.NewQuantity(int64(64), resource.DecimalExponent),
+						clusterv1.ResourceMemory: *resource.NewQuantity(int64(1024*1024*128), resource.BinarySI),
+					},
+					Allocatable: clusterv1.ResourceList{
+						clusterv1.ResourceCPU:    *resource.NewQuantity(int64(32), resource.DecimalExponent),
+						clusterv1.ResourceMemory: *resource.NewQuantity(int64(1024*1024*64), resource.BinarySI),
+					},
+				}
+				testinghelpers.AssertActions(t, actions, "get", "update")
+				actual := actions[1].(clienttesting.UpdateActionImpl).Object
+				testinghelpers.AssertManagedClusterCondition(t, actual.(*clusterv1.ManagedCluster).Status.Conditions, expectedCondition)
+				testinghelpers.AssertManagedClusterStatus(t, actual.(*clusterv1.ManagedCluster).Status, expectedStatus)
 			},
 		},
 	}
@@ -119,7 +134,7 @@ func TestSyncManagedCluster(t *testing.T) {
 			}
 
 			ctrl := managedClusterJoiningController{
-				clusterName:      testManagedClusterName,
+				clusterName:      testinghelpers.TestManagedClusterName,
 				hubClusterClient: clusterClient,
 				hubClusterLister: clusterInformerFactory.Cluster().V1().ManagedClusters().Lister(),
 				discoveryClient:  kubeClient.Discovery(),
@@ -127,150 +142,9 @@ func TestSyncManagedCluster(t *testing.T) {
 			}
 
 			syncErr := ctrl.sync(context.TODO(), testinghelpers.NewFakeSyncContext(t, ""))
-			if len(c.expectedErr) > 0 && syncErr == nil {
-				t.Errorf("expected %q error", c.expectedErr)
-				return
-			}
-			if len(c.expectedErr) > 0 && syncErr != nil && syncErr.Error() != c.expectedErr {
-				t.Errorf("expected %q error, got %q", c.expectedErr, syncErr.Error())
-				return
-			}
-			if len(c.expectedErr) == 0 && syncErr != nil {
-				t.Errorf("unexpected err: %v", syncErr)
-			}
+			testinghelpers.AssertError(t, syncErr, c.expectedErr)
 
 			c.validateActions(t, clusterClient.Actions())
 		})
 	}
 }
-
-func assertActions(t *testing.T, actualActions []clienttesting.Action, expectedActions ...string) {
-	if len(actualActions) != len(expectedActions) {
-		t.Errorf("expected %d call but got: %#v", len(expectedActions), actualActions)
-	}
-	for i, expected := range expectedActions {
-		if actualActions[i].GetVerb() != expected {
-			t.Errorf("expected %s action but got: %#v", expected, actualActions[i])
-		}
-	}
-}
-
-func assertManagedCluster(t *testing.T, actual runtime.Object, expectedName string) {
-	managedCluster, ok := actual.(*clusterv1.ManagedCluster)
-	if !ok {
-		t.Errorf("expected managed cluster but got: %#v", actual)
-	}
-	if managedCluster.Name != expectedName {
-		t.Errorf("expected %s but got: %#v", expectedName, managedCluster.Name)
-	}
-}
-
-func assertCondition(t *testing.T, actual runtime.Object, expectedCondition clusterv1.StatusCondition) {
-	managedCluster := actual.(*clusterv1.ManagedCluster)
-	cond := helpers.FindManagedClusterCondition(managedCluster.Status.Conditions, expectedCondition.Type)
-	if cond == nil {
-		t.Errorf("expected condition %s but got: %s", expectedCondition.Type, cond.Type)
-	}
-	if cond.Status != expectedCondition.Status {
-		t.Errorf("expected status %s but got: %s", expectedCondition.Status, cond.Status)
-	}
-	if cond.Reason != expectedCondition.Reason {
-		t.Errorf("expected reason %s but got: %s", expectedCondition.Reason, cond.Reason)
-	}
-	if cond.Message != expectedCondition.Message {
-		t.Errorf("expected message %s but got: %s", expectedCondition.Message, cond.Message)
-	}
-}
-
-func assertStatusVersion(t *testing.T, actual runtime.Object, expected version.Info) {
-	managedCluster := actual.(*clusterv1.ManagedCluster)
-	if !reflect.DeepEqual(managedCluster.Status.Version, clusterv1.ManagedClusterVersion{
-		Kubernetes: expected.GitVersion,
-	}) {
-		t.Errorf("expected %s but got: %#v", expected, managedCluster.Status.Version)
-	}
-}
-
-func assertStatusResource(t *testing.T, actual runtime.Object, expectedCapacity, expectedAllocatable corev1.ResourceList) {
-	managedCluster := actual.(*clusterv1.ManagedCluster)
-	if !reflect.DeepEqual(managedCluster.Status.Capacity["cpu"], expectedCapacity["cpu"]) {
-		t.Errorf("expected %#v but got: %#v", expectedCapacity, managedCluster.Status.Capacity)
-	}
-	if !reflect.DeepEqual(managedCluster.Status.Capacity["memory"], expectedCapacity["memory"]) {
-		t.Errorf("expected %#v but got: %#v", expectedCapacity, managedCluster.Status.Capacity)
-	}
-	if !reflect.DeepEqual(managedCluster.Status.Allocatable["cpu"], expectedAllocatable["cpu"]) {
-		t.Errorf("expected %#v but got: %#v", expectedAllocatable, managedCluster.Status.Allocatable)
-	}
-	if !reflect.DeepEqual(managedCluster.Status.Allocatable["memory"], expectedAllocatable["memory"]) {
-		t.Errorf("expected %#v but got: %#v", expectedAllocatable, managedCluster.Status.Allocatable)
-	}
-}
-
-func newManagedCluster(conditions ...clusterv1.StatusCondition) *clusterv1.ManagedCluster {
-	return &clusterv1.ManagedCluster{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: testManagedClusterName,
-		},
-		Status: clusterv1.ManagedClusterStatus{
-			Conditions: conditions,
-		},
-	}
-}
-
-func newAcceptedManagedCluster() *clusterv1.ManagedCluster {
-	return newManagedCluster(clusterv1.StatusCondition{
-		Type:    clusterv1.ManagedClusterConditionHubAccepted,
-		Status:  metav1.ConditionTrue,
-		Reason:  "HubClusterAdminAccepted",
-		Message: "Accepted by hub cluster admin",
-	})
-}
-
-func newJoinedManagedCluster(capacity, allocatable corev1.ResourceList) *clusterv1.ManagedCluster {
-	managedCluster := newManagedCluster(
-		clusterv1.StatusCondition{
-			Type:    clusterv1.ManagedClusterConditionHubAccepted,
-			Status:  metav1.ConditionTrue,
-			Reason:  "HubClusterAdminAccepted",
-			Message: "Accepted by hub cluster admin",
-		},
-		clusterv1.StatusCondition{
-			Type:    clusterv1.ManagedClusterConditionJoined,
-			Status:  metav1.ConditionTrue,
-			Reason:  "ManagedClusterJoined",
-			Message: "Managed cluster joined",
-		},
-	)
-	managedCluster.Status.Capacity = clusterv1.ResourceList{
-		"cpu":    capacity.Cpu().DeepCopy(),
-		"memory": capacity.Memory().DeepCopy(),
-	}
-	managedCluster.Status.Allocatable = clusterv1.ResourceList{
-		"cpu":    allocatable.Cpu().DeepCopy(),
-		"memory": allocatable.Memory().DeepCopy(),
-	}
-	managedCluster.Status.Version = clusterv1.ManagedClusterVersion{
-		Kubernetes: kubeversion.Get().GitVersion,
-	}
-	return managedCluster
-}
-
-func newResourceList(cpu, mem int) corev1.ResourceList {
-	return corev1.ResourceList{
-		corev1.ResourceCPU:    *resource.NewQuantity(int64(cpu), resource.DecimalExponent),
-		corev1.ResourceMemory: *resource.NewQuantity(int64(1024*1024*mem), resource.BinarySI),
-	}
-}
-
-func newNode(name string, capacity, allocatable corev1.ResourceList) *corev1.Node {
-	return &corev1.Node{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: name,
-		},
-		Status: corev1.NodeStatus{
-			Capacity:    capacity,
-			Allocatable: allocatable,
-		},
-	}
-}

pkg/spoke/managedcluster/lease_controller_test.go

@@ -8,20 +8,16 @@ import (
 
 	clusterfake "github.com/open-cluster-management/api/client/cluster/clientset/versioned/fake"
 	clusterinformers "github.com/open-cluster-management/api/client/cluster/informers/externalversions"
-	clusterv1 "github.com/open-cluster-management/api/cluster/v1"
 	testinghelpers "github.com/open-cluster-management/registration/pkg/helpers/testing"
 
 	"github.com/openshift/library-go/pkg/operator/events/eventstesting"
 
 	coordinationv1 "k8s.io/api/coordination/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	kubefake "k8s.io/client-go/kubernetes/fake"
 	clienttesting "k8s.io/client-go/testing"
 )
 
-const testLeaseDurationSeconds int32 = 1
-
 func TestLeaseUpdate(t *testing.T) {
 	cases := []struct {
 		name                    string
@@ -32,30 +28,26 @@ func TestLeaseUpdate(t *testing.T) {
 	}{
 		{
 			name:     "start lease update routine",
-			clusters: []runtime.Object{newAcceptedManagedClusterWithLeaseDuration()},
+			clusters: []runtime.Object{testinghelpers.NewAcceptedManagedCluster()},
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				assertLeaseUpdateActions(t, actions)
+				testinghelpers.AssertUpdateActions(t, actions)
 				leaseObj := actions[1].(clienttesting.UpdateActionImpl).Object
 				lastLeaseObj := actions[len(actions)-1].(clienttesting.UpdateActionImpl).Object
-				assertLeaseUpdated(t, leaseObj, lastLeaseObj)
+				testinghelpers.AssertLeaseUpdated(t, leaseObj.(*coordinationv1.Lease), lastLeaseObj.(*coordinationv1.Lease))
 			},
 		},
 		{
 			name:                    "delete a managed cluster after lease update routine is started",
 			clusters:                []runtime.Object{},
 			needToStartUpdateBefore: true,
-			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				assertNoMoreUpdates(t, actions)
-			},
-			expectedErr: "unable to get managed cluster \"testmanagedcluster\" from hub: managedcluster.cluster.open-cluster-management.io \"testmanagedcluster\" not found",
+			validateActions:         testinghelpers.AssertNoMoreUpdates,
+			expectedErr:             "unable to get managed cluster \"testmanagedcluster\" from hub: managedcluster.cluster.open-cluster-management.io \"testmanagedcluster\" not found",
 		},
 		{
 			name:                    "unaccept a managed cluster after lease update routine is started",
-			clusters:                []runtime.Object{newManagedCluster()},
+			clusters:                []runtime.Object{testinghelpers.NewManagedCluster()},
 			needToStartUpdateBefore: true,
-			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				assertNoMoreUpdates(t, actions)
-			},
+			validateActions:         testinghelpers.AssertNoMoreUpdates,
 		},
 	}
 
@@ -68,43 +60,28 @@ func TestLeaseUpdate(t *testing.T) {
 				clusterStore.Add(cluster)
 			}
 
-			hubClient := kubefake.NewSimpleClientset(&coordinationv1.Lease{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "cluster-lease-testmanagedcluster",
-					Namespace: "testmanagedcluster",
-				},
-			})
+			hubClient := kubefake.NewSimpleClientset(testinghelpers.NewManagedClusterLease(time.Now()))
 
 			leaseUpdater := &leaseUpdater{
 				hubClient:   hubClient,
-				clusterName: testManagedClusterName,
-				leaseName:   fmt.Sprintf("cluster-lease-%s", testManagedClusterName),
+				clusterName: testinghelpers.TestManagedClusterName,
+				leaseName:   fmt.Sprintf("cluster-lease-%s", testinghelpers.TestManagedClusterName),
 				recorder:    eventstesting.NewTestingEventRecorder(t),
 			}
 
 			if c.needToStartUpdateBefore {
-				leaseUpdater.start(context.TODO(), time.Duration(testLeaseDurationSeconds)*time.Second)
+				leaseUpdater.start(context.TODO(), time.Duration(testinghelpers.TestLeaseDurationSeconds)*time.Second)
 				// wait a few milliseconds to start the lease update routine
 				time.Sleep(200 * time.Millisecond)
 			}
 
 			ctrl := &managedClusterLeaseController{
-				clusterName:      testManagedClusterName,
+				clusterName:      testinghelpers.TestManagedClusterName,
 				hubClusterLister: clusterInformerFactory.Cluster().V1().ManagedClusters().Lister(),
 				leaseUpdater:     leaseUpdater,
 			}
 			syncErr := ctrl.sync(context.TODO(), testinghelpers.NewFakeSyncContext(t, ""))
-			if len(c.expectedErr) > 0 && syncErr == nil {
-				t.Errorf("expected %q error", c.expectedErr)
-				return
-			}
-			if len(c.expectedErr) > 0 && syncErr != nil && syncErr.Error() != c.expectedErr {
-				t.Errorf("expected %q error, got %q", c.expectedErr, syncErr.Error())
-				return
-			}
-			if len(c.expectedErr) == 0 && syncErr != nil {
-				t.Errorf("unexpected err: %v", syncErr)
-			}
+			testinghelpers.AssertError(t, syncErr, c.expectedErr)
 
 			// wait one cycle
 			time.Sleep(1200 * time.Millisecond)
@@ -112,41 +89,3 @@ func TestLeaseUpdate(t *testing.T) {
 		})
 	}
 }
-
-func newAcceptedManagedClusterWithLeaseDuration() *clusterv1.ManagedCluster {
-	cluster := newAcceptedManagedCluster()
-	cluster.Spec.LeaseDurationSeconds = testLeaseDurationSeconds
-	return cluster
-}
-
-func assertLeaseUpdateActions(t *testing.T, actions []clienttesting.Action) {
-	for i := 0; i < len(actions); i = i + 2 {
-		if actions[i].GetVerb() != "get" {
-			t.Errorf("expected action %d is get, but %v", i, actions[i])
-		}
-		if actions[i+1].GetVerb() != "update" {
-			t.Errorf("expected action %d is update, but %v", i, actions[i+1])
-		}
-	}
-}
-
-func assertLeaseUpdated(t *testing.T, lease, lastLeaseObj runtime.Object) {
-	firstRenewTime := lease.(*coordinationv1.Lease).Spec.RenewTime
-	lastRenewTime := lastLeaseObj.(*coordinationv1.Lease).Spec.RenewTime
-	if !firstRenewTime.BeforeTime(&metav1.Time{Time: lastRenewTime.Time}) {
-		t.Errorf("expected lease updated, but failed")
-	}
-}
-
-func assertNoMoreUpdates(t *testing.T, actions []clienttesting.Action) {
-	updateActions := 0
-	for _, action := range actions {
-		if action.GetVerb() == "update" {
-			updateActions++
-		}
-	}
-	// make sure the lease update routine is started and no more update actions
-	if updateActions != 1 {
-		t.Errorf("expected there is only one update action, but failed")
-	}
-}

pkg/spoke/spokeagent_test.go

@@ -8,171 +8,259 @@ import (
 	"testing"
 	"time"
 
+	testinghelpers "github.com/open-cluster-management/registration/pkg/helpers/testing"
 	"github.com/open-cluster-management/registration/pkg/spoke/hubclientcert"
+
 	"k8s.io/client-go/rest"
 )
 
-func TestGetOrGenerateClusterAgentNames(t *testing.T) {
-	o := &SpokeAgentOptions{
-		HubKubeconfigDir: "/path/not/existing",
+func TestComplete(t *testing.T) {
+	options := NewSpokeAgentOptions()
+	if err := options.Complete(); err != nil {
+		t.Errorf("unexpected error: %v", err)
 	}
-
-	clusterName, agentName := o.getOrGenerateClusterAgentNames()
-	if clusterName == "" {
+	if options.ComponentNamespace == "" {
+		t.Error("component namespace should not be empty")
+	}
+	if options.ClusterName == "" {
 		t.Error("cluster name should not be empty")
 	}
-
-	if agentName == "" {
+	if options.AgentName == "" {
 		t.Error("agent name should not be empty")
 	}
 }
 
-func TestGetOrGenerateClusterAgentNamesWithClusterNameOverride(t *testing.T) {
-	dir, err := ioutil.TempDir("", "prefix")
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-	defer os.RemoveAll(dir)
-
-	o := &SpokeAgentOptions{
-		HubKubeconfigDir: "/path/not/existing",
-		ClusterName:      "cluster0",
-	}
-
-	clusterName, agentName := o.getOrGenerateClusterAgentNames()
-
-	if clusterName != o.ClusterName {
-		t.Errorf("expect cluster name %q but got %q", o.ClusterName, clusterName)
-	}
-
-	if agentName == "" {
-		t.Error("agent name should not be empty")
-	}
-}
-
-func TestGetOrGenerateClusterAgentNamesWithExistingNames(t *testing.T) {
-	dir, err := ioutil.TempDir("", "prefix")
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-	defer os.RemoveAll(dir)
-
-	cn, an := "cluster0", "agent0"
-
-	clusterNameFilePath := path.Join(dir, hubclientcert.ClusterNameFile)
-	err = ioutil.WriteFile(clusterNameFilePath, []byte(cn), 0644)
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	agentNameFilePath := path.Join(dir, hubclientcert.AgentNameFile)
-	err = ioutil.WriteFile(agentNameFilePath, []byte(an), 0644)
-	if err != nil {
-		t.Errorf("unexpected error: %v", err)
-	}
-
-	o := &SpokeAgentOptions{
-		HubKubeconfigDir: dir,
-	}
-
-	clusterName, agentName := o.getOrGenerateClusterAgentNames()
-
-	if clusterName != cn {
-		t.Errorf("expect cluster name %q but got %q", cn, clusterName)
-	}
-
-	if agentName != an {
-		t.Errorf("expect agent name %q but got %q", an, agentName)
-	}
-}
-
 func TestValidate(t *testing.T) {
-	var err error
+	defaultCompletedOptions := NewSpokeAgentOptions()
+	defaultCompletedOptions.BootstrapKubeconfig = "/spoke/bootstrap/kubeconfig"
+	defaultCompletedOptions.ClusterName = "testcluster"
+	defaultCompletedOptions.AgentName = "testagent"
 
-	withoutBootstrapKubeconfig := &SpokeAgentOptions{}
-	err = withoutBootstrapKubeconfig.Validate()
-	if err == nil || err.Error() != "bootstrap-kubeconfig is required" {
-		t.Errorf("expect 'bootstrap-kubeconfig is required' error but got %v", err)
+	cases := []struct {
+		name        string
+		options     *SpokeAgentOptions
+		expectedErr string
+	}{
+		{
+			name:        "no bootstrap kubeconfig",
+			options:     &SpokeAgentOptions{},
+			expectedErr: "bootstrap-kubeconfig is required",
+		},
+		{
+			name:        "no cluster name",
+			options:     &SpokeAgentOptions{BootstrapKubeconfig: "/spoke/bootstrap/kubeconfig"},
+			expectedErr: "cluster name is empty",
+		},
+		{
+			name:        "no agent name",
+			options:     &SpokeAgentOptions{BootstrapKubeconfig: "/spoke/bootstrap/kubeconfig", ClusterName: "testcluster"},
+			expectedErr: "agent name is empty",
+		},
+		{
+			name: "invalid external server URLs",
+			options: &SpokeAgentOptions{
+				BootstrapKubeconfig:     "/spoke/bootstrap/kubeconfig",
+				ClusterName:             "testcluster",
+				AgentName:               "testagent",
+				SpokeExternalServerURLs: []string{"https://127.0.0.1:64433", "http://127.0.0.1:8080"},
+			},
+			expectedErr: "\"http://127.0.0.1:8080\" is invalid",
+		},
+		{
+			name: "invalid cluster healthcheck period",
+			options: &SpokeAgentOptions{
+				BootstrapKubeconfig:      "/spoke/bootstrap/kubeconfig",
+				ClusterName:              "testcluster",
+				AgentName:                "testagent",
+				ClusterHealthCheckPeriod: 0,
+			},
+			expectedErr: "cluster healthcheck period must greater than zero",
+		},
+		{
+			name:        "default completed options",
+			options:     defaultCompletedOptions,
+			expectedErr: "",
+		},
 	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			err := c.options.Validate()
+			testinghelpers.AssertError(t, err, c.expectedErr)
+		})
+	}
+}
 
-	withoutClusterName := &SpokeAgentOptions{
-		BootstrapKubeconfig: "/spoke/bootstrap/kubeconfig",
-	}
-	err = withoutClusterName.Validate()
-	if err == nil || err.Error() != "cluster name is empty" {
-		t.Errorf("expect \"cluster name is empty\" error but got %v", err)
-	}
-
-	withoutAgentName := &SpokeAgentOptions{
-		BootstrapKubeconfig: "/spoke/bootstrap/kubeconfig",
-		ClusterName:         "testcluster",
-	}
-	err = withoutAgentName.Validate()
-	if err == nil || err.Error() != "agent name is empty" {
-		t.Errorf("expect \"agent name is empty\" error but got %v", err)
-	}
-
-	withoutSpokeExternalServerURLs := &SpokeAgentOptions{
-		BootstrapKubeconfig:      "/spoke/bootstrap/kubeconfig",
-		ClusterName:              "testcluster",
-		AgentName:                "testagent",
-		ClusterHealthCheckPeriod: 1 * time.Minute,
-	}
-	err = withoutSpokeExternalServerURLs.Validate()
+func TestHasValidHubClientConfig(t *testing.T) {
+	tempDir, err := ioutil.TempDir("", "testvalidhubclientconfig")
 	if err != nil {
-		t.Errorf("expect no error but got %v", err)
+		t.Errorf("unexpected error: %v", err)
 	}
+	defer os.RemoveAll(tempDir)
 
-	withInvalidSpokeExternalServerURL := &SpokeAgentOptions{
-		BootstrapKubeconfig:     "/spoke/bootstrap/kubeconfig",
-		ClusterName:             "testcluster",
-		AgentName:               "testagent",
-		SpokeExternalServerURLs: []string{"https://127.0.0.1:64433", "http://127.0.0.1:8080"},
-	}
-	err = withInvalidSpokeExternalServerURL.Validate()
-	if err == nil || err.Error() != "\"http://127.0.0.1:8080\" is invalid" {
-		t.Errorf("expect \"http://127.0.0.1:8080 is invalid\" error but got %v", err)
-	}
+	cert := testinghelpers.NewTestCert("test", 60*time.Second)
+	kubeconfig := testinghelpers.NewKubeconfig(cert.Key, cert.Cert)
 
-	withInvalidClusterHealthCheckPeriod := &SpokeAgentOptions{
-		BootstrapKubeconfig:      "/spoke/bootstrap/kubeconfig",
-		ClusterName:              "testcluster",
-		AgentName:                "testagent",
-		ClusterHealthCheckPeriod: 0,
+	cases := []struct {
+		name       string
+		kubeconfig []byte
+		tlsCert    []byte
+		tlsKey     []byte
+		isValid    bool
+	}{
+		{
+			name:    "no kubeconfig",
+			isValid: false,
+		},
+		{
+			name:       "no tls key",
+			kubeconfig: kubeconfig,
+			isValid:    false,
+		},
+		{
+			name:       "no tls cert",
+			kubeconfig: kubeconfig,
+			tlsKey:     cert.Key,
+			isValid:    false,
+		},
+		{
+			name:       "valid hub client config",
+			kubeconfig: kubeconfig,
+			tlsKey:     cert.Key,
+			tlsCert:    cert.Cert,
+			isValid:    true,
+		},
 	}
-	err = withInvalidClusterHealthCheckPeriod.Validate()
-	if err == nil || err.Error() != "cluster healthcheck period must greater than zero" {
-		t.Errorf("expect no error but got %v", err)
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			if c.kubeconfig != nil {
+				testinghelpers.WriteFile(path.Join(tempDir, "kubeconfig"), c.kubeconfig)
+			}
+			if c.tlsKey != nil {
+				testinghelpers.WriteFile(path.Join(tempDir, "tls.key"), c.tlsKey)
+			}
+			if c.tlsCert != nil {
+				testinghelpers.WriteFile(path.Join(tempDir, "tls.crt"), c.tlsCert)
+			}
+
+			options := &SpokeAgentOptions{HubKubeconfigDir: tempDir}
+			valid, err := options.hasValidHubClientConfig()
+			if err != nil {
+				t.Errorf("unexpected error: %v", err)
+			}
+			if c.isValid != valid {
+				t.Errorf("expect %t, but %t", c.isValid, valid)
+			}
+		})
+	}
+}
+
+func TestGetOrGenerateClusterAgentNames(t *testing.T) {
+	tempDir, err := ioutil.TempDir("", "testgetorgenerateclusteragentnames")
+	if err != nil {
+		t.Errorf("unexpected error: %v", err)
+	}
+	defer os.RemoveAll(tempDir)
+
+	cases := []struct {
+		name                string
+		options             *SpokeAgentOptions
+		expectedClusterName string
+		expectedAgentName   string
+	}{
+		{
+			name:                "cluster name is specified",
+			options:             &SpokeAgentOptions{ClusterName: "cluster0"},
+			expectedClusterName: "cluster0",
+		},
+		{
+			name:                "cluster name and agent name are in file",
+			options:             &SpokeAgentOptions{HubKubeconfigDir: tempDir},
+			expectedClusterName: "cluster1",
+			expectedAgentName:   "agent1",
+		},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			if c.options.HubKubeconfigDir != "" {
+				testinghelpers.WriteFile(path.Join(tempDir, hubclientcert.ClusterNameFile), []byte(c.expectedClusterName))
+				testinghelpers.WriteFile(path.Join(tempDir, hubclientcert.AgentNameFile), []byte(c.expectedAgentName))
+			}
+			clusterName, agentName := c.options.getOrGenerateClusterAgentNames()
+			if clusterName != c.expectedClusterName {
+				t.Errorf("expect cluster name %q but got %q", c.expectedClusterName, clusterName)
+			}
+
+			// agent name cannot be empty, it is either generated or from file
+			if agentName == "" {
+				t.Error("agent name should not be empty")
+			}
+
+			if c.expectedAgentName != "" && c.expectedAgentName != agentName {
+				t.Errorf("expect agent name %q but got %q", c.expectedAgentName, agentName)
+			}
+		})
 	}
 }
 
 func TestGetSpokeClusterCABundle(t *testing.T) {
-	withoutSpokeExternalServerURLs := &SpokeAgentOptions{}
-	caData, err := withoutSpokeExternalServerURLs.getSpokeClusterCABundle(&rest.Config{})
-	if err != nil {
-		t.Errorf("expect no error but got %v", err)
-	}
-	if caData != nil {
-		t.Errorf("expect no ca data but got %v", caData)
-	}
-
-	withSpokeExternalServerURLs := &SpokeAgentOptions{SpokeExternalServerURLs: []string{"https://127.0.0.1:6443"}}
-	caData, err = withSpokeExternalServerURLs.getSpokeClusterCABundle(&rest.Config{})
-	if err == nil {
-		t.Errorf("expect error happened but no error")
-	}
-	if caData != nil {
-		t.Errorf("expect no ca data but got %v", caData)
-	}
-
-	expectedCAData := []byte("cadata")
-	caData, err = withSpokeExternalServerURLs.getSpokeClusterCABundle(&rest.Config{
-		TLSClientConfig: rest.TLSClientConfig{CAData: expectedCAData},
-	})
+	tempDir, err := ioutil.TempDir("", "testgetspokeclustercabundle")
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}
-	if !bytes.Equal(caData, expectedCAData) {
-		t.Errorf("expected %v but got %v", expectedCAData, caData)
+	defer os.RemoveAll(tempDir)
+
+	cases := []struct {
+		name           string
+		caFile         string
+		options        *SpokeAgentOptions
+		expectedErr    string
+		expectedCAData []byte
+	}{
+		{
+			name:           "no external server URLs",
+			options:        &SpokeAgentOptions{},
+			expectedErr:    "",
+			expectedCAData: nil,
+		},
+		{
+			name:           "no ca data",
+			options:        &SpokeAgentOptions{SpokeExternalServerURLs: []string{"https://127.0.0.1:6443"}},
+			expectedErr:    "open : no such file or directory",
+			expectedCAData: nil,
+		},
+		{
+			name:           "has ca data",
+			options:        &SpokeAgentOptions{SpokeExternalServerURLs: []string{"https://127.0.0.1:6443"}},
+			expectedErr:    "",
+			expectedCAData: []byte("cadata"),
+		},
+		{
+			name:           "has ca file",
+			caFile:         "ca.data",
+			options:        &SpokeAgentOptions{SpokeExternalServerURLs: []string{"https://127.0.0.1:6443"}},
+			expectedErr:    "",
+			expectedCAData: []byte("cadata"),
+		},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			restConig := &rest.Config{}
+			if c.expectedCAData != nil {
+				restConig.CAData = c.expectedCAData
+			}
+			if c.caFile != "" {
+				testinghelpers.WriteFile(path.Join(tempDir, c.caFile), c.expectedCAData)
+				restConig.CAData = nil
+				restConig.CAFile = path.Join(tempDir, c.caFile)
+			}
+			caData, err := c.options.getSpokeClusterCABundle(restConig)
+			testinghelpers.AssertError(t, err, c.expectedErr)
+			if c.expectedCAData == nil && caData == nil {
+				return
+			}
+			if !bytes.Equal(caData, c.expectedCAData) {
+				t.Errorf("expect %v but got %v", c.expectedCAData, caData)
+			}
+		})
 	}
 }

pkg/webhook/webhook.go

@@ -6,18 +6,19 @@ import (
 	"fmt"
 	"net/http"
 
+	clusterv1 "github.com/open-cluster-management/api/cluster/v1"
+	"github.com/open-cluster-management/registration/pkg/helpers"
+
+	operatorhelpers "github.com/openshift/library-go/pkg/operator/v1helpers"
+
 	admissionv1beta1 "k8s.io/api/admission/v1beta1"
 	authenticationv1 "k8s.io/api/authentication/v1"
 	authorizationv1 "k8s.io/api/authorization/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
-
-	clusterv1 "github.com/open-cluster-management/api/cluster/v1"
-	"github.com/open-cluster-management/registration/pkg/helpers"
-	operatorhelpers "github.com/openshift/library-go/pkg/operator/v1helpers"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/klog"
 )
 

pkg/webhook/webhook_test.go

@@ -7,6 +7,8 @@ import (
 	"testing"
 
 	clusterv1 "github.com/open-cluster-management/api/cluster/v1"
+	testinghelpers "github.com/open-cluster-management/registration/pkg/helpers/testing"
+
 	admissionv1beta1 "k8s.io/api/admission/v1beta1"
 	authenticationv1 "k8s.io/api/authentication/v1"
 	authorizationv1 "k8s.io/api/authorization/v1"
@@ -22,7 +24,7 @@ var managedclustersSchema = metav1.GroupVersionResource{
 	Resource: "managedclusters",
 }
 
-func TestSpokeClusterValidate(t *testing.T) {
+func TestManagedClusterValidate(t *testing.T) {
 	cases := []struct {
 		name                   string
 		request                *admissionv1beta1.AdmissionRequest
@@ -177,42 +179,26 @@ func TestSpokeClusterValidate(t *testing.T) {
 }
 
 func newManagedClusterObj() runtime.RawExtension {
-	spokeCluster := &clusterv1.ManagedCluster{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: "testspokecluster",
-		},
-	}
-	clusterObj, _ := json.Marshal(spokeCluster)
+	managedCluster := testinghelpers.NewManagedCluster()
+	clusterObj, _ := json.Marshal(managedCluster)
 	return runtime.RawExtension{
 		Raw: clusterObj,
 	}
 }
 
 func newManagedClusterObjWithHubAcceptsClient(hubAcceptsClient bool) runtime.RawExtension {
-	spokeCluster := &clusterv1.ManagedCluster{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: "testspokecluster",
-		},
-		Spec: clusterv1.ManagedClusterSpec{
-			HubAcceptsClient: hubAcceptsClient,
-		},
-	}
-	clusterObj, _ := json.Marshal(spokeCluster)
+	managedCluster := testinghelpers.NewManagedCluster()
+	managedCluster.Spec.HubAcceptsClient = hubAcceptsClient
+	clusterObj, _ := json.Marshal(managedCluster)
 	return runtime.RawExtension{
 		Raw: clusterObj,
 	}
 }
 
 func newManagedClusterObjWithClientConfigs(clientConfig clusterv1.ClientConfig) runtime.RawExtension {
-	spokeCluster := &clusterv1.ManagedCluster{
-		ObjectMeta: metav1.ObjectMeta{
-			Name: "testspokecluster",
-		},
-		Spec: clusterv1.ManagedClusterSpec{
-			ManagedClusterClientConfigs: []clusterv1.ClientConfig{clientConfig},
-		},
-	}
-	clusterObj, _ := json.Marshal(spokeCluster)
+	managedCluster := testinghelpers.NewManagedCluster()
+	managedCluster.Spec.ManagedClusterClientConfigs = []clusterv1.ClientConfig{clientConfig}
+	clusterObj, _ := json.Marshal(managedCluster)
 	return runtime.RawExtension{
 		Raw: clusterObj,
 	}