From 13bcbc7396cfd62927ed892c9ed8067fdbcaed0f Mon Sep 17 00:00:00 2001 From: liuwei Date: Wed, 8 Jul 2020 20:15:54 +0800 Subject: [PATCH] reorganize ut common functions and improve test coverage --- go.mod | 1 - pkg/helpers/helpers.go | 4 +- pkg/helpers/helpers_test.go | 119 ++-- pkg/helpers/testing/assertion.go | 193 +++++++ pkg/helpers/testing/testinghelpers.go | 399 ++++++++++++- pkg/hub/csr/controller_test.go | 258 ++++----- pkg/hub/lease/controller_test.go | 110 +--- pkg/hub/managedcluster/controller_test.go | 159 ++---- pkg/hub/rbacfinalizerdeletion/controller.go | 2 + .../rbacfinalizerdeletion/controller_test.go | 208 ++++--- pkg/spoke/hubclientcert/certificate_test.go | 473 ++++------------ pkg/spoke/hubclientcert/controller.go | 1 + pkg/spoke/hubclientcert/controller_test.go | 531 +++++------------- .../creating_controller_test.go | 51 +- .../healthcheck_controller_test.go | 56 +- .../managedcluster/joining_controller_test.go | 236 ++------ .../managedcluster/lease_controller_test.go | 87 +-- pkg/spoke/spokeagent_test.go | 366 +++++++----- pkg/webhook/webhook.go | 11 +- pkg/webhook/webhook_test.go | 36 +- 20 files changed, 1483 insertions(+), 1818 deletions(-) create mode 100644 pkg/helpers/testing/assertion.go diff --git a/go.mod b/go.mod index 1e37afc28..fdf0d6a17 100644 --- a/go.mod +++ b/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 diff --git a/pkg/helpers/helpers.go b/pkg/helpers/helpers.go index 55b53a74a..a0df29fe5 100644 --- a/pkg/helpers/helpers.go +++ b/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, diff --git a/pkg/helpers/helpers_test.go b/pkg/helpers/helpers_test.go index a0d066cee..07fdd6aee 100644 --- a/pkg/helpers/helpers_test.go +++ b/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 { diff --git a/pkg/helpers/testing/assertion.go b/pkg/helpers/testing/assertion.go new file mode 100644 index 000000000..38737b30e --- /dev/null +++ b/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") + } +} diff --git a/pkg/helpers/testing/testinghelpers.go b/pkg/helpers/testing/testinghelpers.go index f688baa89..b11929298 100644 --- a/pkg/helpers/testing/testinghelpers.go +++ b/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) + } +} diff --git a/pkg/hub/csr/controller_test.go b/pkg/hub/csr/controller_test.go index a683171df..b76272eec 100644 --- a/pkg/hub/csr/controller_test.go +++ b/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) - } -} diff --git a/pkg/hub/lease/controller_test.go b/pkg/hub/lease/controller_test.go index 573a4f4d8..c6452fe2c 100644 --- a/pkg/hub/lease/controller_test.go +++ b/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}, - }, - } -} diff --git a/pkg/hub/managedcluster/controller_test.go b/pkg/hub/managedcluster/controller_test.go index 957e981a5..2d08ac3f4 100644 --- a/pkg/hub/managedcluster/controller_test.go +++ b/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}, - } -} diff --git a/pkg/hub/rbacfinalizerdeletion/controller.go b/pkg/hub/rbacfinalizerdeletion/controller.go index c9b5db1ca..1bf975b99 100644 --- a/pkg/hub/rbacfinalizerdeletion/controller.go +++ b/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" diff --git a/pkg/hub/rbacfinalizerdeletion/controller_test.go b/pkg/hub/rbacfinalizerdeletion/controller_test.go index 8b336b152..57c9c09e6 100644 --- a/pkg/hub/rbacfinalizerdeletion/controller_test.go +++ b/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)) - } -} diff --git a/pkg/spoke/hubclientcert/certificate_test.go b/pkg/spoke/hubclientcert/certificate_test.go index 4ea27b835..5e8943892 100644 --- a/pkg/spoke/hubclientcert/certificate_test.go +++ b/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) + } + }) } } diff --git a/pkg/spoke/hubclientcert/controller.go b/pkg/spoke/hubclientcert/controller.go index b5c57efcc..8afd5ab59 100644 --- a/pkg/spoke/hubclientcert/controller.go +++ b/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" diff --git a/pkg/spoke/hubclientcert/controller_test.go b/pkg/spoke/hubclientcert/controller_test.go index ab6caba6d..69778c31f 100644 --- a/pkg/spoke/hubclientcert/controller_test.go +++ b/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()) + }) } } diff --git a/pkg/spoke/managedcluster/creating_controller_test.go b/pkg/spoke/managedcluster/creating_controller_test.go index 5cbffc80f..d9901ad9c 100644 --- a/pkg/spoke/managedcluster/creating_controller_test.go +++ b/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) - } -} diff --git a/pkg/spoke/managedcluster/healthcheck_controller_test.go b/pkg/spoke/managedcluster/healthcheck_controller_test.go index 02beb0a21..0cefde9dc 100644 --- a/pkg/spoke/managedcluster/healthcheck_controller_test.go +++ b/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()) }) diff --git a/pkg/spoke/managedcluster/joining_controller_test.go b/pkg/spoke/managedcluster/joining_controller_test.go index 4c0d6af3f..91703bd61 100644 --- a/pkg/spoke/managedcluster/joining_controller_test.go +++ b/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, - }, - } -} diff --git a/pkg/spoke/managedcluster/lease_controller_test.go b/pkg/spoke/managedcluster/lease_controller_test.go index 3e17651eb..2c0b30953 100644 --- a/pkg/spoke/managedcluster/lease_controller_test.go +++ b/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") - } -} diff --git a/pkg/spoke/spokeagent_test.go b/pkg/spoke/spokeagent_test.go index bcc07c8f3..1550be3b5 100644 --- a/pkg/spoke/spokeagent_test.go +++ b/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) + } + }) } } diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go index 63a19922d..ebe5c20e8 100644 --- a/pkg/webhook/webhook.go +++ b/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" ) diff --git a/pkg/webhook/webhook_test.go b/pkg/webhook/webhook_test.go index 893bfc4ab..5f73715fe 100644 --- a/pkg/webhook/webhook_test.go +++ b/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, }