diff --git a/Makefile b/Makefile index a764db893..5d4ad0e6a 100644 --- a/Makefile +++ b/Makefile @@ -21,6 +21,7 @@ KUSTOMIZE_VERSION?=v3.5.4 KUSTOMIZE_ARCHIVE_NAME?=kustomize_$(KUSTOMIZE_VERSION)_$(GOHOSTOS)_$(GOHOSTARCH).tar.gz kustomize_dir:=$(dir $(KUSTOMIZE)) +$(call add-bindata,clusterrole,./pkg/hub/clusterrole/manifests/...,bindata,bindata,./pkg/hub/clusterrole/bindata/bindata.go) $(call add-bindata,managedcluster,./pkg/hub/managedcluster/manifests/...,bindata,bindata,./pkg/hub/managedcluster/bindata/bindata.go) $(call add-bindata,spokecluster-e2e,./deploy/spoke/...,bindata,bindata,./test/e2e/bindata/bindata.go) diff --git a/pkg/helpers/testing/testinghelpers.go b/pkg/helpers/testing/testinghelpers.go index 465bb207e..a85ccd05e 100644 --- a/pkg/helpers/testing/testinghelpers.go +++ b/pkg/helpers/testing/testinghelpers.go @@ -8,7 +8,6 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/pem" - "fmt" "io/ioutil" "math/big" "math/rand" @@ -161,10 +160,10 @@ func NewManagedClusterCondition(name, status, reason, message string, lastTransi return ret } -func NewManagedClusterLease(renewTime time.Time) *coordv1.Lease { +func NewManagedClusterLease(name string, renewTime time.Time) *coordv1.Lease { return &coordv1.Lease{ ObjectMeta: metav1.ObjectMeta{ - Name: fmt.Sprintf("cluster-lease-%s", TestManagedClusterName), + Name: name, Namespace: TestManagedClusterName, }, Spec: coordv1.LeaseSpec{ diff --git a/pkg/hub/clusterrole/bindata/bindata.go b/pkg/hub/clusterrole/bindata/bindata.go new file mode 100644 index 000000000..ccce009f5 --- /dev/null +++ b/pkg/hub/clusterrole/bindata/bindata.go @@ -0,0 +1,275 @@ +// Code generated for package bindata by go-bindata DO NOT EDIT. (@generated) +// sources: +// pkg/hub/clusterrole/manifests/managedcluster-registration-clusterrole.yaml +// pkg/hub/clusterrole/manifests/managedcluster-work-clusterrole.yaml +package bindata + +import ( + "fmt" + "io/ioutil" + "os" + "path/filepath" + "strings" + "time" +) + +type asset struct { + bytes []byte + info os.FileInfo +} + +type bindataFileInfo struct { + name string + size int64 + mode os.FileMode + modTime time.Time +} + +// Name return file name +func (fi bindataFileInfo) Name() string { + return fi.name +} + +// Size return file size +func (fi bindataFileInfo) Size() int64 { + return fi.size +} + +// Mode return file mode +func (fi bindataFileInfo) Mode() os.FileMode { + return fi.mode +} + +// Mode return file modify time +func (fi bindataFileInfo) ModTime() time.Time { + return fi.modTime +} + +// IsDir return file whether a directory +func (fi bindataFileInfo) IsDir() bool { + return fi.mode&os.ModeDir != 0 +} + +// Sys return file is sys mode +func (fi bindataFileInfo) Sys() interface{} { + return nil +} + +var _pkgHubClusterroleManifestsManagedclusterRegistrationClusterroleYaml = []byte(`apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: open-cluster-management:managedcluster:registration +rules: +# Allow spoke registration agent to get/update coordination.k8s.io/lease +- apiGroups: ["coordination.k8s.io"] + resources: ["leases"] + #TODO: for backward compatible, we do not limit the resource name in release 2.3. + #After release 2.3, we will limit the resource name. + #resourceNames: ["managed-cluster-lease"] + verbs: ["get", "update"] +`) + +func pkgHubClusterroleManifestsManagedclusterRegistrationClusterroleYamlBytes() ([]byte, error) { + return _pkgHubClusterroleManifestsManagedclusterRegistrationClusterroleYaml, nil +} + +func pkgHubClusterroleManifestsManagedclusterRegistrationClusterroleYaml() (*asset, error) { + bytes, err := pkgHubClusterroleManifestsManagedclusterRegistrationClusterroleYamlBytes() + if err != nil { + return nil, err + } + + info := bindataFileInfo{name: "pkg/hub/clusterrole/manifests/managedcluster-registration-clusterrole.yaml", size: 0, mode: os.FileMode(0), modTime: time.Unix(0, 0)} + a := &asset{bytes: bytes, info: info} + return a, nil +} + +var _pkgHubClusterroleManifestsManagedclusterWorkClusterroleYaml = []byte(`apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: open-cluster-management:managedcluster:work +rules: +# Allow work agent to send event to hub +- apiGroups: ["", "events.k8s.io"] + resources: ["events"] + verbs: ["create", "patch", "update"] +# Allow work agent to get/list/watch/update manifestworks +- apiGroups: ["work.open-cluster-management.io"] + resources: ["manifestworks"] + verbs: ["get", "list", "watch", "update"] +# Allow work agent to update the status of manifestwork +- apiGroups: ["work.open-cluster-management.io"] + resources: ["manifestworks/status"] + verbs: ["patch", "update"] +`) + +func pkgHubClusterroleManifestsManagedclusterWorkClusterroleYamlBytes() ([]byte, error) { + return _pkgHubClusterroleManifestsManagedclusterWorkClusterroleYaml, nil +} + +func pkgHubClusterroleManifestsManagedclusterWorkClusterroleYaml() (*asset, error) { + bytes, err := pkgHubClusterroleManifestsManagedclusterWorkClusterroleYamlBytes() + if err != nil { + return nil, err + } + + info := bindataFileInfo{name: "pkg/hub/clusterrole/manifests/managedcluster-work-clusterrole.yaml", size: 0, mode: os.FileMode(0), modTime: time.Unix(0, 0)} + a := &asset{bytes: bytes, info: info} + return a, nil +} + +// Asset loads and returns the asset for the given name. +// It returns an error if the asset could not be found or +// could not be loaded. +func Asset(name string) ([]byte, error) { + cannonicalName := strings.Replace(name, "\\", "/", -1) + if f, ok := _bindata[cannonicalName]; ok { + a, err := f() + if err != nil { + return nil, fmt.Errorf("Asset %s can't read by error: %v", name, err) + } + return a.bytes, nil + } + return nil, fmt.Errorf("Asset %s not found", name) +} + +// MustAsset is like Asset but panics when Asset would return an error. +// It simplifies safe initialization of global variables. +func MustAsset(name string) []byte { + a, err := Asset(name) + if err != nil { + panic("asset: Asset(" + name + "): " + err.Error()) + } + + return a +} + +// AssetInfo loads and returns the asset info for the given name. +// It returns an error if the asset could not be found or +// could not be loaded. +func AssetInfo(name string) (os.FileInfo, error) { + cannonicalName := strings.Replace(name, "\\", "/", -1) + if f, ok := _bindata[cannonicalName]; ok { + a, err := f() + if err != nil { + return nil, fmt.Errorf("AssetInfo %s can't read by error: %v", name, err) + } + return a.info, nil + } + return nil, fmt.Errorf("AssetInfo %s not found", name) +} + +// AssetNames returns the names of the assets. +func AssetNames() []string { + names := make([]string, 0, len(_bindata)) + for name := range _bindata { + names = append(names, name) + } + return names +} + +// _bindata is a table, holding each asset generator, mapped to its name. +var _bindata = map[string]func() (*asset, error){ + "pkg/hub/clusterrole/manifests/managedcluster-registration-clusterrole.yaml": pkgHubClusterroleManifestsManagedclusterRegistrationClusterroleYaml, + "pkg/hub/clusterrole/manifests/managedcluster-work-clusterrole.yaml": pkgHubClusterroleManifestsManagedclusterWorkClusterroleYaml, +} + +// AssetDir returns the file names below a certain +// directory embedded in the file by go-bindata. +// For example if you run go-bindata on data/... and data contains the +// following hierarchy: +// data/ +// foo.txt +// img/ +// a.png +// b.png +// then AssetDir("data") would return []string{"foo.txt", "img"} +// AssetDir("data/img") would return []string{"a.png", "b.png"} +// AssetDir("foo.txt") and AssetDir("notexist") would return an error +// AssetDir("") will return []string{"data"}. +func AssetDir(name string) ([]string, error) { + node := _bintree + if len(name) != 0 { + cannonicalName := strings.Replace(name, "\\", "/", -1) + pathList := strings.Split(cannonicalName, "/") + for _, p := range pathList { + node = node.Children[p] + if node == nil { + return nil, fmt.Errorf("Asset %s not found", name) + } + } + } + if node.Func != nil { + return nil, fmt.Errorf("Asset %s not found", name) + } + rv := make([]string, 0, len(node.Children)) + for childName := range node.Children { + rv = append(rv, childName) + } + return rv, nil +} + +type bintree struct { + Func func() (*asset, error) + Children map[string]*bintree +} + +var _bintree = &bintree{nil, map[string]*bintree{ + "pkg": {nil, map[string]*bintree{ + "hub": {nil, map[string]*bintree{ + "clusterrole": {nil, map[string]*bintree{ + "manifests": {nil, map[string]*bintree{ + "managedcluster-registration-clusterrole.yaml": {pkgHubClusterroleManifestsManagedclusterRegistrationClusterroleYaml, map[string]*bintree{}}, + "managedcluster-work-clusterrole.yaml": {pkgHubClusterroleManifestsManagedclusterWorkClusterroleYaml, map[string]*bintree{}}, + }}, + }}, + }}, + }}, +}} + +// RestoreAsset restores an asset under the given directory +func RestoreAsset(dir, name string) error { + data, err := Asset(name) + if err != nil { + return err + } + info, err := AssetInfo(name) + if err != nil { + return err + } + err = os.MkdirAll(_filePath(dir, filepath.Dir(name)), os.FileMode(0755)) + if err != nil { + return err + } + err = ioutil.WriteFile(_filePath(dir, name), data, info.Mode()) + if err != nil { + return err + } + err = os.Chtimes(_filePath(dir, name), info.ModTime(), info.ModTime()) + if err != nil { + return err + } + return nil +} + +// RestoreAssets restores an asset under the given directory recursively +func RestoreAssets(dir, name string) error { + children, err := AssetDir(name) + // File + if err != nil { + return RestoreAsset(dir, name) + } + // Dir + for _, child := range children { + err = RestoreAssets(dir, filepath.Join(name, child)) + if err != nil { + return err + } + } + return nil +} + +func _filePath(dir, name string) string { + cannonicalName := strings.Replace(name, "\\", "/", -1) + return filepath.Join(append([]string{dir}, strings.Split(cannonicalName, "/")...)...) +} diff --git a/pkg/hub/clusterrole/controller.go b/pkg/hub/clusterrole/controller.go new file mode 100644 index 000000000..aa4875ebf --- /dev/null +++ b/pkg/hub/clusterrole/controller.go @@ -0,0 +1,107 @@ +package clusterrole + +import ( + "context" + "fmt" + "path/filepath" + + clusterv1informer "github.com/open-cluster-management/api/client/cluster/informers/externalversions/cluster/v1" + clusterv1listers "github.com/open-cluster-management/api/client/cluster/listers/cluster/v1" + "github.com/open-cluster-management/registration/pkg/helpers" + "github.com/open-cluster-management/registration/pkg/hub/clusterrole/bindata" + + "github.com/openshift/library-go/pkg/assets" + "github.com/openshift/library-go/pkg/controller/factory" + "github.com/openshift/library-go/pkg/operator/events" + "github.com/openshift/library-go/pkg/operator/resource/resourceapply" + operatorhelpers "github.com/openshift/library-go/pkg/operator/v1helpers" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/util/sets" + rbacv1informers "k8s.io/client-go/informers/rbac/v1" + "k8s.io/client-go/kubernetes" +) + +const ( + registrationClusterRole = "open-cluster-management:managedcluster:registration" + workClusterRole = "open-cluster-management:managedcluster:work" + manifestDir = "pkg/hub/clusterrole" +) + +var clusterRoleFiles = []string{ + "manifests/managedcluster-registration-clusterrole.yaml", + "manifests/managedcluster-work-clusterrole.yaml", +} + +// clusterroleController maintains the necessary clusterroles for registraion and work agent on hub cluster. +type clusterroleController struct { + kubeClient kubernetes.Interface + clusterLister clusterv1listers.ManagedClusterLister + eventRecorder events.Recorder +} + +// NewManagedClusterClusterroleController creates a clusterrole controller on hub cluster. +func NewManagedClusterClusterroleController( + kubeClient kubernetes.Interface, + clusterInformer clusterv1informer.ManagedClusterInformer, + clusterRoleInformer rbacv1informers.ClusterRoleInformer, + recorder events.Recorder) factory.Controller { + c := &clusterroleController{ + kubeClient: kubeClient, + clusterLister: clusterInformer.Lister(), + eventRecorder: recorder.WithComponentSuffix("managed-cluster-clusterrole-controller"), + } + return factory.New(). + WithFilteredEventsInformers( + func(obj interface{}) bool { + clusterRoles := sets.NewString(registrationClusterRole, workClusterRole) + metaObj := obj.(metav1.Object) + if clusterRoles.Has(metaObj.GetName()) { + return true + } + return false + }, clusterRoleInformer.Informer()). + WithInformers(clusterInformer.Informer()). + WithSync(c.sync). + ToController("ManagedClusterClusterRoleController", recorder) +} + +func (c *clusterroleController) sync(ctx context.Context, syncCtx factory.SyncContext) error { + managedClusters, err := c.clusterLister.List(labels.Everything()) + if err != nil { + return err + } + + // Clean up managedcluser cluserroles if there are no managed clusters + if len(managedClusters) == 0 { + return helpers.CleanUpManagedClusterManifests( + ctx, + c.kubeClient, + c.eventRecorder, + func(name string) ([]byte, error) { + return assets.MustCreateAssetFromTemplate(name, bindata.MustAsset(filepath.Join(manifestDir, name)), nil).Data, nil + }, + clusterRoleFiles..., + ) + } + + // Make sure the managedcluser cluserroles are existed if there are clusters + results := resourceapply.ApplyDirectly( + resourceapply.NewKubeClientHolder(c.kubeClient), + syncCtx.Recorder(), + func(name string) ([]byte, error) { + return assets.MustCreateAssetFromTemplate(name, bindata.MustAsset(filepath.Join(manifestDir, name)), nil).Data, nil + }, + clusterRoleFiles..., + ) + + errs := []error{} + for _, result := range results { + if result.Error != nil { + errs = append(errs, fmt.Errorf("%q (%T): %v", result.File, result.Type, result.Error)) + } + } + + return operatorhelpers.NewMultiLineAggregate(errs) +} diff --git a/pkg/hub/clusterrole/controller_test.go b/pkg/hub/clusterrole/controller_test.go new file mode 100644 index 000000000..e45148f19 --- /dev/null +++ b/pkg/hub/clusterrole/controller_test.go @@ -0,0 +1,88 @@ +package clusterrole + +import ( + "context" + "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" + testinghelpers "github.com/open-cluster-management/registration/pkg/helpers/testing" + + "github.com/openshift/library-go/pkg/operator/events/eventstesting" + + rbacv1 "k8s.io/api/rbac/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" +) + +func TestSyncManagedClusterClusterRole(t *testing.T) { + cases := []struct { + name string + clusters []runtime.Object + clusterroles []runtime.Object + validateActions func(t *testing.T, actions []clienttesting.Action) + }{ + { + name: "create clusterroles", + clusters: []runtime.Object{testinghelpers.NewManagedCluster()}, + clusterroles: []runtime.Object{}, + validateActions: func(t *testing.T, actions []clienttesting.Action) { + testinghelpers.AssertActions(t, actions, "get", "create", "get", "create") + registrationClusterRole := (actions[1].(clienttesting.CreateActionImpl).Object).(*rbacv1.ClusterRole) + if registrationClusterRole.Name != "open-cluster-management:managedcluster:registration" { + t.Errorf("expected registration clusterrole, but failed") + } + workClusterRole := (actions[3].(clienttesting.CreateActionImpl).Object).(*rbacv1.ClusterRole) + if workClusterRole.Name != "open-cluster-management:managedcluster:work" { + t.Errorf("expected work clusterrole, but failed") + } + }, + }, + { + name: "delete clusterroles", + clusters: []runtime.Object{}, + clusterroles: []runtime.Object{ + &rbacv1.ClusterRole{ObjectMeta: metav1.ObjectMeta{Name: "open-cluster-management:managedcluster:registration"}}, + &rbacv1.ClusterRole{ObjectMeta: metav1.ObjectMeta{Name: "open-cluster-management:managedcluster:work"}}, + }, + validateActions: func(t *testing.T, actions []clienttesting.Action) { + testinghelpers.AssertActions(t, actions, "delete", "delete") + if actions[0].(clienttesting.DeleteActionImpl).Name != "open-cluster-management:managedcluster:registration" { + t.Errorf("expected registration clusterrole, but failed") + } + if actions[1].(clienttesting.DeleteActionImpl).Name != "open-cluster-management:managedcluster:work" { + t.Errorf("expected work clusterrole, but failed") + } + }, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + kubeClient := kubefake.NewSimpleClientset(c.clusterroles...) + + clusterClient := clusterfake.NewSimpleClientset(c.clusters...) + clusterInformerFactory := clusterinformers.NewSharedInformerFactory(clusterClient, time.Minute*10) + clusterStore := clusterInformerFactory.Cluster().V1().ManagedClusters().Informer().GetStore() + for _, cluster := range c.clusters { + clusterStore.Add(cluster) + } + + ctrl := &clusterroleController{ + kubeClient: kubeClient, + clusterLister: clusterInformerFactory.Cluster().V1().ManagedClusters().Lister(), + eventRecorder: eventstesting.NewTestingEventRecorder(t), + } + + syncErr := ctrl.sync(context.TODO(), testinghelpers.NewFakeSyncContext(t, "testmangedclsuterclusterrole")) + if syncErr != nil { + t.Errorf("unexpected err: %v", syncErr) + } + + c.validateActions(t, kubeClient.Actions()) + }) + } +} diff --git a/pkg/hub/clusterrole/doc.go b/pkg/hub/clusterrole/doc.go new file mode 100644 index 000000000..375132264 --- /dev/null +++ b/pkg/hub/clusterrole/doc.go @@ -0,0 +1,2 @@ +// package clusterrole contains the hub-side reconciler for the ManagedCluster necessary clusterrole resource. +package clusterrole diff --git a/pkg/hub/clusterrole/manifests/managedcluster-registration-clusterrole.yaml b/pkg/hub/clusterrole/manifests/managedcluster-registration-clusterrole.yaml new file mode 100644 index 000000000..da52a8767 --- /dev/null +++ b/pkg/hub/clusterrole/manifests/managedcluster-registration-clusterrole.yaml @@ -0,0 +1,12 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + name: open-cluster-management:managedcluster:registration +rules: +# Allow spoke registration agent to get/update coordination.k8s.io/lease +- apiGroups: ["coordination.k8s.io"] + resources: ["leases"] + #TODO: for backward compatible, we do not limit the resource name in release 2.3. + #After release 2.3, we will limit the resource name. + #resourceNames: ["managed-cluster-lease"] + verbs: ["get", "update"] diff --git a/pkg/hub/managedcluster/manifests/managedcluster-work-role.yaml b/pkg/hub/clusterrole/manifests/managedcluster-work-clusterrole.yaml similarity index 75% rename from pkg/hub/managedcluster/manifests/managedcluster-work-role.yaml rename to pkg/hub/clusterrole/manifests/managedcluster-work-clusterrole.yaml index be9722310..22c1021f6 100644 --- a/pkg/hub/managedcluster/manifests/managedcluster-work-role.yaml +++ b/pkg/hub/clusterrole/manifests/managedcluster-work-clusterrole.yaml @@ -1,10 +1,7 @@ apiVersion: rbac.authorization.k8s.io/v1 -kind: Role +kind: ClusterRole metadata: - name: {{ .ManagedClusterName }}:managed-cluster-work - namespace: {{ .ManagedClusterName }} - finalizers: - - cluster.open-cluster-management.io/manifest-work-cleanup + name: open-cluster-management:managedcluster:work rules: # Allow work agent to send event to hub - apiGroups: ["", "events.k8s.io"] diff --git a/pkg/hub/lease/controller.go b/pkg/hub/lease/controller.go index 2d074e577..81fe87b98 100644 --- a/pkg/hub/lease/controller.go +++ b/pkg/hub/lease/controller.go @@ -69,7 +69,7 @@ func (c *leaseController) sync(ctx context.Context, syncCtx factory.SyncContext) } // get the lease of a cluster, if the lease is not found, create it - leaseName := fmt.Sprintf("cluster-lease-%s", cluster.Name) + leaseName := "managed-cluster-lease" observedLease, err := c.leaseLister.Leases(cluster.Name).Get(leaseName) switch { case errors.IsNotFound(err): @@ -106,6 +106,36 @@ func (c *leaseController) sync(ctx context.Context, syncCtx factory.SyncContext) continue } + // for backward compatible, before release-2.3, the format of lease name is cluster-lease- + // TODO: after release-2.3, we will eliminate these + oldVersionLeaseName := fmt.Sprintf("cluster-lease-%s", cluster.Name) + oldVersionLease, err := c.leaseLister.Leases(cluster.Name).Get(oldVersionLeaseName) + switch { + case err == nil: + if time.Now().Before(oldVersionLease.Spec.RenewTime.Add(gracePeriod)) { + continue + } + case errors.IsNotFound(err): + // the old version does not exist, create a new one + oldVersionLease := &coordv1.Lease{ + ObjectMeta: metav1.ObjectMeta{ + Name: oldVersionLeaseName, + Namespace: cluster.Name, + Labels: map[string]string{"open-cluster-management.io/cluster-name": cluster.Name}, + }, + Spec: coordv1.LeaseSpec{ + HolderIdentity: pointer.StringPtr(oldVersionLeaseName), + RenewTime: &metav1.MicroTime{Time: time.Now()}, + }, + } + if _, err := c.kubeClient.CoordinationV1().Leases(cluster.Name).Create(ctx, oldVersionLease, metav1.CreateOptions{}); err != nil { + return err + } + continue + case err != nil: + return err + } + // the lease is not constantly updated, update it to unknown conditionUpdateFn := helpers.UpdateManagedClusterConditionFn(metav1.Condition{ Type: clusterv1.ManagedClusterConditionAvailable, diff --git a/pkg/hub/lease/controller_test.go b/pkg/hub/lease/controller_test.go index b26329940..d24cf7ba0 100644 --- a/pkg/hub/lease/controller_test.go +++ b/pkg/hub/lease/controller_test.go @@ -2,6 +2,7 @@ package lease import ( "context" + "fmt" "testing" "time" @@ -45,9 +46,12 @@ func TestSync(t *testing.T) { }, }, { - name: "managed cluster stop update lease", - clusters: []runtime.Object{testinghelpers.NewAvailableManagedCluster()}, - clusterLeases: []runtime.Object{testinghelpers.NewManagedClusterLease(now.Add(-5 * time.Minute))}, + name: "managed cluster stop update lease", + clusters: []runtime.Object{testinghelpers.NewAvailableManagedCluster()}, + clusterLeases: []runtime.Object{ + testinghelpers.NewManagedClusterLease("managed-cluster-lease", now.Add(-5*time.Minute)), + testinghelpers.NewManagedClusterLease(fmt.Sprintf("cluster-lease-%s", testinghelpers.TestManagedClusterName), now.Add(-5*time.Minute)), + }, validateActions: func(t *testing.T, leaseActions, clusterActions []clienttesting.Action) { expected := metav1.Condition{ Type: clusterv1.ManagedClusterConditionAvailable, @@ -63,7 +67,18 @@ func TestSync(t *testing.T) { { name: "managed cluster is available", clusters: []runtime.Object{testinghelpers.NewAvailableManagedCluster()}, - clusterLeases: []runtime.Object{testinghelpers.NewManagedClusterLease(now)}, + clusterLeases: []runtime.Object{testinghelpers.NewManagedClusterLease("managed-cluster-lease", now)}, + validateActions: func(t *testing.T, leaseActions, clusterActions []clienttesting.Action) { + testinghelpers.AssertNoActions(t, clusterActions) + }, + }, + { + name: "managed cluster is available (backward compatible)", + clusters: []runtime.Object{testinghelpers.NewAvailableManagedCluster()}, + clusterLeases: []runtime.Object{ + testinghelpers.NewManagedClusterLease("managed-cluster-lease", now.Add(-5*time.Minute)), + testinghelpers.NewManagedClusterLease(fmt.Sprintf("cluster-lease-%s", testinghelpers.TestManagedClusterName), now), + }, validateActions: func(t *testing.T, leaseActions, clusterActions []clienttesting.Action) { testinghelpers.AssertNoActions(t, clusterActions) }, diff --git a/pkg/hub/managedcluster/bindata/bindata.go b/pkg/hub/managedcluster/bindata/bindata.go index 88f7db45a..590823690 100644 --- a/pkg/hub/managedcluster/bindata/bindata.go +++ b/pkg/hub/managedcluster/bindata/bindata.go @@ -3,9 +3,7 @@ // pkg/hub/managedcluster/manifests/managedcluster-clusterrole.yaml // pkg/hub/managedcluster/manifests/managedcluster-clusterrolebinding.yaml // pkg/hub/managedcluster/manifests/managedcluster-namespace.yaml -// pkg/hub/managedcluster/manifests/managedcluster-registration-role.yaml // pkg/hub/managedcluster/manifests/managedcluster-registration-rolebinding.yaml -// pkg/hub/managedcluster/manifests/managedcluster-work-role.yaml // pkg/hub/managedcluster/manifests/managedcluster-work-rolebinding.yaml package bindata @@ -149,34 +147,6 @@ func pkgHubManagedclusterManifestsManagedclusterNamespaceYaml() (*asset, error) return a, nil } -var _pkgHubManagedclusterManifestsManagedclusterRegistrationRoleYaml = []byte(`apiVersion: rbac.authorization.k8s.io/v1 -kind: Role -metadata: - name: {{ .ManagedClusterName }}:managed-cluster-registration - namespace: {{ .ManagedClusterName }} -rules: -# Allow spoke registration agent to get/update coordination.k8s.io/lease -- apiGroups: ["coordination.k8s.io"] - resources: ["leases"] - resourceNames: ["cluster-lease-{{ .ManagedClusterName }}"] - verbs: ["get", "update"] -`) - -func pkgHubManagedclusterManifestsManagedclusterRegistrationRoleYamlBytes() ([]byte, error) { - return _pkgHubManagedclusterManifestsManagedclusterRegistrationRoleYaml, nil -} - -func pkgHubManagedclusterManifestsManagedclusterRegistrationRoleYaml() (*asset, error) { - bytes, err := pkgHubManagedclusterManifestsManagedclusterRegistrationRoleYamlBytes() - if err != nil { - return nil, err - } - - info := bindataFileInfo{name: "pkg/hub/managedcluster/manifests/managedcluster-registration-role.yaml", size: 0, mode: os.FileMode(0), modTime: time.Unix(0, 0)} - a := &asset{bytes: bytes, info: info} - return a, nil -} - var _pkgHubManagedclusterManifestsManagedclusterRegistrationRolebindingYaml = []byte(`apiVersion: rbac.authorization.k8s.io/v1 kind: RoleBinding metadata: @@ -184,8 +154,8 @@ metadata: namespace: {{ .ManagedClusterName }} roleRef: apiGroup: rbac.authorization.k8s.io - kind: Role - name: {{ .ManagedClusterName }}:managed-cluster-registration + kind: ClusterRole + name: open-cluster-management:managedcluster:registration subjects: # Bind the role with spoke agent user group, the role will be as a common role for all spoke agents # TODO: we will consider bind a specific role for each spoke agent by spoke agent name @@ -209,43 +179,6 @@ func pkgHubManagedclusterManifestsManagedclusterRegistrationRolebindingYaml() (* return a, nil } -var _pkgHubManagedclusterManifestsManagedclusterWorkRoleYaml = []byte(`apiVersion: rbac.authorization.k8s.io/v1 -kind: Role -metadata: - name: {{ .ManagedClusterName }}:managed-cluster-work - namespace: {{ .ManagedClusterName }} - finalizers: - - cluster.open-cluster-management.io/manifest-work-cleanup -rules: -# Allow work agent to send event to hub -- apiGroups: ["", "events.k8s.io"] - resources: ["events"] - verbs: ["create", "patch", "update"] -# Allow work agent to get/list/watch/update manifestworks -- apiGroups: ["work.open-cluster-management.io"] - resources: ["manifestworks"] - verbs: ["get", "list", "watch", "update"] -# Allow work agent to update the status of manifestwork -- apiGroups: ["work.open-cluster-management.io"] - resources: ["manifestworks/status"] - verbs: ["patch", "update"] -`) - -func pkgHubManagedclusterManifestsManagedclusterWorkRoleYamlBytes() ([]byte, error) { - return _pkgHubManagedclusterManifestsManagedclusterWorkRoleYaml, nil -} - -func pkgHubManagedclusterManifestsManagedclusterWorkRoleYaml() (*asset, error) { - bytes, err := pkgHubManagedclusterManifestsManagedclusterWorkRoleYamlBytes() - if err != nil { - return nil, err - } - - info := bindataFileInfo{name: "pkg/hub/managedcluster/manifests/managedcluster-work-role.yaml", size: 0, mode: os.FileMode(0), modTime: time.Unix(0, 0)} - a := &asset{bytes: bytes, info: info} - return a, nil -} - var _pkgHubManagedclusterManifestsManagedclusterWorkRolebindingYaml = []byte(`apiVersion: rbac.authorization.k8s.io/v1 kind: RoleBinding metadata: @@ -255,8 +188,8 @@ metadata: - cluster.open-cluster-management.io/manifest-work-cleanup roleRef: apiGroup: rbac.authorization.k8s.io - kind: Role - name: {{ .ManagedClusterName }}:managed-cluster-work + kind: ClusterRole + name: open-cluster-management:managedcluster:work subjects: # Bind the role with agent user group, the role will be as a common role for all agents # TODO: we will consider bind a specific role for each agent by agent name @@ -335,9 +268,7 @@ var _bindata = map[string]func() (*asset, error){ "pkg/hub/managedcluster/manifests/managedcluster-clusterrole.yaml": pkgHubManagedclusterManifestsManagedclusterClusterroleYaml, "pkg/hub/managedcluster/manifests/managedcluster-clusterrolebinding.yaml": pkgHubManagedclusterManifestsManagedclusterClusterrolebindingYaml, "pkg/hub/managedcluster/manifests/managedcluster-namespace.yaml": pkgHubManagedclusterManifestsManagedclusterNamespaceYaml, - "pkg/hub/managedcluster/manifests/managedcluster-registration-role.yaml": pkgHubManagedclusterManifestsManagedclusterRegistrationRoleYaml, "pkg/hub/managedcluster/manifests/managedcluster-registration-rolebinding.yaml": pkgHubManagedclusterManifestsManagedclusterRegistrationRolebindingYaml, - "pkg/hub/managedcluster/manifests/managedcluster-work-role.yaml": pkgHubManagedclusterManifestsManagedclusterWorkRoleYaml, "pkg/hub/managedcluster/manifests/managedcluster-work-rolebinding.yaml": pkgHubManagedclusterManifestsManagedclusterWorkRolebindingYaml, } @@ -389,9 +320,7 @@ var _bintree = &bintree{nil, map[string]*bintree{ "managedcluster-clusterrole.yaml": {pkgHubManagedclusterManifestsManagedclusterClusterroleYaml, map[string]*bintree{}}, "managedcluster-clusterrolebinding.yaml": {pkgHubManagedclusterManifestsManagedclusterClusterrolebindingYaml, map[string]*bintree{}}, "managedcluster-namespace.yaml": {pkgHubManagedclusterManifestsManagedclusterNamespaceYaml, map[string]*bintree{}}, - "managedcluster-registration-role.yaml": {pkgHubManagedclusterManifestsManagedclusterRegistrationRoleYaml, map[string]*bintree{}}, "managedcluster-registration-rolebinding.yaml": {pkgHubManagedclusterManifestsManagedclusterRegistrationRolebindingYaml, map[string]*bintree{}}, - "managedcluster-work-role.yaml": {pkgHubManagedclusterManifestsManagedclusterWorkRoleYaml, map[string]*bintree{}}, "managedcluster-work-rolebinding.yaml": {pkgHubManagedclusterManifestsManagedclusterWorkRolebindingYaml, map[string]*bintree{}}, }}, }}, diff --git a/pkg/hub/managedcluster/controller.go b/pkg/hub/managedcluster/controller.go index 060fa522e..0076eb84a 100644 --- a/pkg/hub/managedcluster/controller.go +++ b/pkg/hub/managedcluster/controller.go @@ -31,9 +31,7 @@ const ( var staticFiles = []string{ "manifests/managedcluster-clusterrole.yaml", "manifests/managedcluster-clusterrolebinding.yaml", - "manifests/managedcluster-registration-role.yaml", "manifests/managedcluster-registration-rolebinding.yaml", - "manifests/managedcluster-work-role.yaml", "manifests/managedcluster-work-rolebinding.yaml", } @@ -184,7 +182,7 @@ func (c *managedClusterController) sync(ctx context.Context, syncCtx factory.Syn func (c *managedClusterController) removeManagedClusterResources(ctx context.Context, managedClusterName string) error { errs := []error{} - // Cleap up managed cluster manifests + // Clean up managed cluster manifests assetFn := helpers.ManagedClusterAssetFn(manifestDir, managedClusterName) if err := helpers.CleanUpManagedClusterManifests(ctx, c.kubeClient, c.eventRecorder, assetFn, staticFiles...); err != nil { errs = append(errs, err) diff --git a/pkg/hub/managedcluster/manifests/managedcluster-registration-role.yaml b/pkg/hub/managedcluster/manifests/managedcluster-registration-role.yaml deleted file mode 100644 index 52f78f259..000000000 --- a/pkg/hub/managedcluster/manifests/managedcluster-registration-role.yaml +++ /dev/null @@ -1,11 +0,0 @@ -apiVersion: rbac.authorization.k8s.io/v1 -kind: Role -metadata: - name: {{ .ManagedClusterName }}:managed-cluster-registration - namespace: {{ .ManagedClusterName }} -rules: -# Allow spoke registration agent to get/update coordination.k8s.io/lease -- apiGroups: ["coordination.k8s.io"] - resources: ["leases"] - resourceNames: ["cluster-lease-{{ .ManagedClusterName }}"] - verbs: ["get", "update"] diff --git a/pkg/hub/managedcluster/manifests/managedcluster-registration-rolebinding.yaml b/pkg/hub/managedcluster/manifests/managedcluster-registration-rolebinding.yaml index 60318deb9..8a4c961e2 100644 --- a/pkg/hub/managedcluster/manifests/managedcluster-registration-rolebinding.yaml +++ b/pkg/hub/managedcluster/manifests/managedcluster-registration-rolebinding.yaml @@ -5,8 +5,8 @@ metadata: namespace: {{ .ManagedClusterName }} roleRef: apiGroup: rbac.authorization.k8s.io - kind: Role - name: {{ .ManagedClusterName }}:managed-cluster-registration + kind: ClusterRole + name: open-cluster-management:managedcluster:registration subjects: # Bind the role with spoke agent user group, the role will be as a common role for all spoke agents # TODO: we will consider bind a specific role for each spoke agent by spoke agent name diff --git a/pkg/hub/managedcluster/manifests/managedcluster-work-rolebinding.yaml b/pkg/hub/managedcluster/manifests/managedcluster-work-rolebinding.yaml index c3737d62c..c5273ccdb 100644 --- a/pkg/hub/managedcluster/manifests/managedcluster-work-rolebinding.yaml +++ b/pkg/hub/managedcluster/manifests/managedcluster-work-rolebinding.yaml @@ -7,8 +7,8 @@ metadata: - cluster.open-cluster-management.io/manifest-work-cleanup roleRef: apiGroup: rbac.authorization.k8s.io - kind: Role - name: {{ .ManagedClusterName }}:managed-cluster-work + kind: ClusterRole + name: open-cluster-management:managedcluster:work subjects: # Bind the role with agent user group, the role will be as a common role for all agents # TODO: we will consider bind a specific role for each agent by agent name diff --git a/pkg/hub/manager.go b/pkg/hub/manager.go index 9bb4c9098..49f5f0212 100644 --- a/pkg/hub/manager.go +++ b/pkg/hub/manager.go @@ -11,6 +11,7 @@ import ( clusterv1informers "github.com/open-cluster-management/api/client/cluster/informers/externalversions" workv1client "github.com/open-cluster-management/api/client/work/clientset/versioned" workv1informers "github.com/open-cluster-management/api/client/work/informers/externalversions" + "github.com/open-cluster-management/registration/pkg/hub/clusterrole" "github.com/open-cluster-management/registration/pkg/hub/csr" "github.com/open-cluster-management/registration/pkg/hub/lease" "github.com/open-cluster-management/registration/pkg/hub/managedcluster" @@ -90,6 +91,13 @@ func RunControllerManager(ctx context.Context, controllerContext *controllercmd. controllerContext.EventRecorder, ) + clusterroleController := clusterrole.NewManagedClusterClusterroleController( + kubeClient, + clusterInformers.Cluster().V1().ManagedClusters(), + kubeInfomers.Rbac().V1().ClusterRoles(), + controllerContext.EventRecorder, + ) + go clusterInformers.Start(ctx.Done()) go workInformers.Start(ctx.Done()) go kubeInfomers.Start(ctx.Done()) @@ -99,6 +107,7 @@ func RunControllerManager(ctx context.Context, controllerContext *controllercmd. go leaseController.Run(ctx, 1) go rbacFinalizerController.Run(ctx, 1) go managedClusterSetController.Run(ctx, 1) + go clusterroleController.Run(ctx, 1) <-ctx.Done() return nil diff --git a/pkg/spoke/managedcluster/lease_controller.go b/pkg/spoke/managedcluster/lease_controller.go index 908aab462..0f497c510 100644 --- a/pkg/spoke/managedcluster/lease_controller.go +++ b/pkg/spoke/managedcluster/lease_controller.go @@ -43,7 +43,7 @@ func NewManagedClusterLeaseController( leaseUpdater: &leaseUpdater{ hubClient: hubClient, clusterName: clusterName, - leaseName: fmt.Sprintf("cluster-lease-%s", clusterName), + leaseName: "managed-cluster-lease", recorder: recorder, }, } diff --git a/pkg/spoke/managedcluster/lease_controller_test.go b/pkg/spoke/managedcluster/lease_controller_test.go index 2c0b30953..609e5c78d 100644 --- a/pkg/spoke/managedcluster/lease_controller_test.go +++ b/pkg/spoke/managedcluster/lease_controller_test.go @@ -2,7 +2,6 @@ package managedcluster import ( "context" - "fmt" "testing" "time" @@ -60,12 +59,12 @@ func TestLeaseUpdate(t *testing.T) { clusterStore.Add(cluster) } - hubClient := kubefake.NewSimpleClientset(testinghelpers.NewManagedClusterLease(time.Now())) + hubClient := kubefake.NewSimpleClientset(testinghelpers.NewManagedClusterLease("managed-cluster-lease", time.Now())) leaseUpdater := &leaseUpdater{ hubClient: hubClient, clusterName: testinghelpers.TestManagedClusterName, - leaseName: fmt.Sprintf("cluster-lease-%s", testinghelpers.TestManagedClusterName), + leaseName: "managed-cluster-lease", recorder: eventstesting.NewTestingEventRecorder(t), } diff --git a/test/e2e/loopback_test.go b/test/e2e/loopback_test.go index 93e9d7ac0..ec4d6779b 100644 --- a/test/e2e/loopback_test.go +++ b/test/e2e/loopback_test.go @@ -364,7 +364,7 @@ var _ = ginkgo.Describe("Loopback registration [development]", func() { }) gomega.Expect(err).ToNot(gomega.HaveOccurred()) - leaseName := fmt.Sprintf("cluster-lease-%s", clusterName) + leaseName := "managed-cluster-lease" ginkgo.By(fmt.Sprintf("Make sure ManagedCluster lease %q exists", leaseName)) var lastRenewTime *metav1.MicroTime err = wait.Poll(1*time.Second, 30*time.Second, func() (bool, error) { diff --git a/test/integration/managedcluster_deletiong_test.go b/test/integration/managedcluster_deletiong_test.go index a5fd48a3f..49464ed4c 100644 --- a/test/integration/managedcluster_deletiong_test.go +++ b/test/integration/managedcluster_deletiong_test.go @@ -48,14 +48,15 @@ var _ = ginkgo.Describe("Cluster deleting", func() { err = clusterClient.ClusterV1().ManagedClusters().Delete(context.Background(), managedCluster.Name, metav1.DeleteOptions{}) gomega.Expect(err).NotTo(gomega.HaveOccurred()) - roleName := fmt.Sprintf("%s:managed-cluster-work", managedCluster.Name) - err = kubeClient.RbacV1().Roles(managedCluster.Name).Delete(context.Background(), roleName, metav1.DeleteOptions{}) + roleBindingName := fmt.Sprintf("%s:managed-cluster-work", managedCluster.Name) + err = kubeClient.RbacV1().RoleBindings(managedCluster.Name).Delete(context.Background(), roleBindingName, metav1.DeleteOptions{}) gomega.Expect(err).NotTo(gomega.HaveOccurred()) - role, err := kubeClient.RbacV1().Roles(managedCluster.Name).Get(context.Background(), roleName, metav1.GetOptions{}) + + roleBinding, err := kubeClient.RbacV1().RoleBindings(managedCluster.Name).Get(context.Background(), roleBindingName, metav1.GetOptions{}) gomega.Expect(err).NotTo(gomega.HaveOccurred()) - gomega.Expect(len(role.Finalizers)).Should(gomega.Equal(1)) - gomega.Expect(role.Finalizers[0]).Should(gomega.Equal("cluster.open-cluster-management.io/manifest-work-cleanup")) - gomega.Expect(role.DeletionTimestamp.IsZero()).Should(gomega.BeFalse()) + gomega.Expect(len(roleBinding.Finalizers)).Should(gomega.Equal(1)) + gomega.Expect(roleBinding.Finalizers[0]).Should(gomega.Equal("cluster.open-cluster-management.io/manifest-work-cleanup")) + gomega.Expect(roleBinding.DeletionTimestamp.IsZero()).Should(gomega.BeFalse()) // Delete work err = workClient.WorkV1().ManifestWorks(managedCluster.Name).Delete(context.Background(), "work1", metav1.DeleteOptions{}) @@ -70,7 +71,7 @@ var _ = ginkgo.Describe("Cluster deleting", func() { }, eventuallyTimeout, eventuallyInterval).Should(gomega.BeTrue()) gomega.Eventually(func() bool { - _, err := kubeClient.RbacV1().Roles(managedCluster.Name).Get(context.Background(), roleName, metav1.GetOptions{}) + _, err := kubeClient.RbacV1().RoleBindings(managedCluster.Name).Get(context.Background(), roleBindingName, metav1.GetOptions{}) if errors.IsNotFound(err) { return true } diff --git a/test/integration/managedcluster_lease_test.go b/test/integration/managedcluster_lease_test.go index 44a331487..86e818dc0 100644 --- a/test/integration/managedcluster_lease_test.go +++ b/test/integration/managedcluster_lease_test.go @@ -141,7 +141,7 @@ var _ = ginkgo.Describe("Cluster Lease Update", func() { // after one grace period, make sure the managed available condition is cluster unknown select { - case <-time.After(time.Duration(5*util.TestLeaseDurationSeconds+1) * time.Second): + case <-time.After(time.Duration(5*util.TestLeaseDurationSeconds+10) * time.Second): managedCluster, err := util.GetManagedCluster(clusterClient, managedClusterName) gomega.Expect(err).NotTo(gomega.HaveOccurred()) availableCond := meta.FindStatusCondition(managedCluster.Status.Conditions, clusterv1.ManagedClusterConditionAvailable)