diff --git a/deploy/cluster_role_binding.yaml b/deploy/cluster_role_binding.yaml index 442bf8c7e..7bd92e347 100644 --- a/deploy/cluster_role_binding.yaml +++ b/deploy/cluster_role_binding.yaml @@ -19,7 +19,7 @@ metadata: subjects: - kind: ServiceAccount name: che-operator - namespace: che + namespace: eclipse-che roleRef: kind: ClusterRole name: che-operator diff --git a/deploy/namespaces_cluster_role_binding.yaml b/deploy/namespaces_cluster_role_binding.yaml index 3607fee77..99cca0df2 100644 --- a/deploy/namespaces_cluster_role_binding.yaml +++ b/deploy/namespaces_cluster_role_binding.yaml @@ -19,7 +19,7 @@ metadata: subjects: - kind: ServiceAccount name: che-operator - namespace: che + namespace: eclipse-che roleRef: kind: ClusterRole name: che-namespace-editor diff --git a/pkg/controller/che/che_controller.go b/pkg/controller/che/che_controller.go index ea3d10ca9..39db9dcdb 100644 --- a/pkg/controller/che/che_controller.go +++ b/pkg/controller/che/che_controller.go @@ -667,7 +667,7 @@ func (r *ReconcileChe) Reconcile(request reconcile.Request) (reconcile.Result, e cheClusterRoles := strings.Split(instance.Spec.Server.CheClusterRoles, ",") for _, cheClusterRole := range cheClusterRoles { cheClusterRole := strings.TrimSpace(cheClusterRole) - cheClusterRoleBindingName := deploy.GetUniqueClusterRoleBindingName(deployContext, CheServiceAccountName, cheClusterRole) + cheClusterRoleBindingName := cheClusterRole done, err := deploy.SyncClusterRoleBindingAndAddFinalizerToCluster(deployContext, cheClusterRoleBindingName, CheServiceAccountName, cheClusterRole) if !tests { if !done { @@ -1235,10 +1235,16 @@ func (r *ReconcileChe) reconcileFinalizers(deployContext *deploy.DeployContext) cheClusterRoles := strings.Split(deployContext.CheCluster.Spec.Server.CheClusterRoles, ",") for _, cheClusterRole := range cheClusterRoles { cheClusterRole := strings.TrimSpace(cheClusterRole) - cheClusterRoleBindingName := deploy.GetUniqueClusterRoleBindingName(deployContext, CheServiceAccountName, cheClusterRole) + cheClusterRoleBindingName := cheClusterRole if err := deploy.ReconcileClusterRoleBindingFinalizer(deployContext, cheClusterRoleBindingName); err != nil { logrus.Error(err) } + + // Removes any legacy CRB https://github.com/eclipse/che/issues/19506 + cheClusterRoleBindingName = deploy.GetLegacyUniqueClusterRoleBindingName(deployContext, CheServiceAccountName, cheClusterRole) + if err := deploy.ReconcileLegacyClusterRoleBindingFinalizer(deployContext, cheClusterRoleBindingName); err != nil { + logrus.Error(err) + } } } } diff --git a/pkg/deploy/clusterrolebinding.go b/pkg/deploy/clusterrolebinding.go index 302f96657..b08fc0716 100644 --- a/pkg/deploy/clusterrolebinding.go +++ b/pkg/deploy/clusterrolebinding.go @@ -41,20 +41,33 @@ func SyncClusterRoleBindingAndAddFinalizerToCluster( serviceAccountName string, clusterRoleName string) (bool, error) { - finalizer := GetFinalizerName(strings.ToLower(name) + ".clusterrolebinding") + finalizer := GetFinalizerName(strings.ToLower(name) + ".crb") crbSpec := getClusterRoleBindingSpec(deployContext, name, serviceAccountName, clusterRoleName) return SyncAndAddFinalizer(deployContext, crbSpec, crbDiffOpts, finalizer) } func ReconcileClusterRoleBindingFinalizer(deployContext *DeployContext, name string) error { - finalizer := GetFinalizerName(strings.ToLower(name) + ".clusterrolebinding") + if deployContext.CheCluster.DeletionTimestamp.IsZero() { + return nil + } + + finalizer := GetFinalizerName(strings.ToLower(name) + ".crb") return DeleteObjectWithFinalizer(deployContext, types.NamespacedName{Name: name}, &rbac.ClusterRoleBinding{}, finalizer) } -func GetUniqueClusterRoleBindingName(deployContext *DeployContext, serviceAccount string, clusterRole string) string { +func GetLegacyUniqueClusterRoleBindingName(deployContext *DeployContext, serviceAccount string, clusterRole string) string { return deployContext.CheCluster.Namespace + "-" + serviceAccount + "-" + clusterRole } +func ReconcileLegacyClusterRoleBindingFinalizer(deployContext *DeployContext, name string) error { + if deployContext.CheCluster.DeletionTimestamp.IsZero() { + return nil + } + + finalizer := strings.ToLower(name) + ".clusterrolebinding.finalizers.che.eclipse.org" + return DeleteObjectWithFinalizer(deployContext, types.NamespacedName{Name: name}, &rbac.ClusterRoleBinding{}, finalizer) +} + func getClusterRoleBindingSpec( deployContext *DeployContext, name string, diff --git a/pkg/deploy/clusterrolebinding_test.go b/pkg/deploy/clusterrolebinding_test.go index 704dfd711..eaa569533 100644 --- a/pkg/deploy/clusterrolebinding_test.go +++ b/pkg/deploy/clusterrolebinding_test.go @@ -95,23 +95,38 @@ func TestSyncClusterRoleBindingAndAddFinalizerToCluster(t *testing.T) { t.Fatalf("Failed to sync crb: %v", err) } - if !util.ContainsString(deployContext.CheCluster.Finalizers, "test.clusterrolebinding.finalizers.che.eclipse.org") { + if !util.ContainsString(deployContext.CheCluster.Finalizers, "test.crb.finalizers.che.eclipse.org") { t.Fatalf("Failed to add finalizer") } - deployContext.CheCluster.ObjectMeta.DeletionTimestamp = &metav1.Time{Time: time.Now()} + // don't expect any deletion err = ReconcileClusterRoleBindingFinalizer(deployContext, "test") if err != nil { - t.Fatalf("Failed to remove finalizer: %v", err) + t.Fatalf("Failed to reconcile finalizer: %v", err) } actual := &rbacv1.ClusterRoleBinding{} err = cli.Get(context.TODO(), types.NamespacedName{Name: "test"}, actual) + if err != nil { + t.Fatalf("CRD shouldn't be deleted: %v", err) + } + if !util.ContainsString(deployContext.CheCluster.Finalizers, "test.crb.finalizers.che.eclipse.org") { + t.Fatalf("Finalizer shouldn't be deleted") + } + + // crb must be deleted as well as finalizer + deployContext.CheCluster.ObjectMeta.DeletionTimestamp = &metav1.Time{Time: time.Now()} + err = ReconcileClusterRoleBindingFinalizer(deployContext, "test") + if err != nil { + t.Fatalf("Failed to reconcile finalizer: %v", err) + } + + actual = &rbacv1.ClusterRoleBinding{} + err = cli.Get(context.TODO(), types.NamespacedName{Name: "test"}, actual) if err == nil { t.Fatalf("Failed to remove crb: %v", err) } - - if util.ContainsString(deployContext.CheCluster.Finalizers, "test.clusterrolebinding.finalizers.che.eclipse.org") { + if util.ContainsString(deployContext.CheCluster.Finalizers, "test.crb.finalizers.che.eclipse.org") { t.Fatalf("Failed to remove finalizer") } } diff --git a/pkg/deploy/finalizer.go b/pkg/deploy/finalizer.go index fe48f303b..cad30e7b0 100644 --- a/pkg/deploy/finalizer.go +++ b/pkg/deploy/finalizer.go @@ -76,5 +76,10 @@ func DeleteObjectWithFinalizer(deployContext *DeployContext, key client.ObjectKe } func GetFinalizerName(prefix string) string { - return prefix + ".finalizers.che.eclipse.org" + finalizer := prefix + ".finalizers.che.eclipse.org" + diff := len(finalizer) - 63 + if diff > 0 { + return finalizer[:len(finalizer)-diff] + } + return finalizer } diff --git a/pkg/deploy/finalizer_test.go b/pkg/deploy/finalizer_test.go index f6bc25d49..374d66f46 100644 --- a/pkg/deploy/finalizer_test.go +++ b/pkg/deploy/finalizer_test.go @@ -97,3 +97,13 @@ func TestDeleteFinalizer(t *testing.T) { t.Fatalf("Failed to delete finalizer: %v", err) } } + +func TestGetFinalizerNameShouldReturnStringLess64Chars(t *testing.T) { + expected := "7890123456789012345678901234567891234567.finalizers.che.eclipse" + prefix := "7890123456789012345678901234567891234567" + + actual := GetFinalizerName(prefix) + if expected != actual { + t.Fatalf("Incorrect finalizer name: %s", actual) + } +}