Improve reconciling crb finalizers (#756)

Signed-off-by: Anatolii Bazko <abazko@redhat.com>
pull/758/head
Anatolii Bazko 2021-04-07 09:05:17 +03:00 committed by GitHub
parent 4b003c79ba
commit 127918ab6d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 62 additions and 13 deletions

View File

@ -19,7 +19,7 @@ metadata:
subjects:
- kind: ServiceAccount
name: che-operator
namespace: che
namespace: eclipse-che
roleRef:
kind: ClusterRole
name: che-operator

View File

@ -19,7 +19,7 @@ metadata:
subjects:
- kind: ServiceAccount
name: che-operator
namespace: che
namespace: eclipse-che
roleRef:
kind: ClusterRole
name: che-namespace-editor

View File

@ -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)
}
}
}
}

View File

@ -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,

View File

@ -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")
}
}

View File

@ -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
}

View File

@ -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)
}
}