Skip to content

Commit

Permalink
Fix bug causing crb removal
Browse files Browse the repository at this point in the history
  • Loading branch information
m00g3n committed Dec 6, 2024
1 parent 0172347 commit 7629903
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,6 @@ var (
labelsManagedByKIM = map[string]string{
"reconciler.kyma-project.io/managed-by": "infrastructure-manager",
}
//nolint:gochecknoglobals
labelsManagedByReconciler = map[string]string{
"reconciler.kyma-project.io/managed-by": "reconciler",
}
)

func sFnApplyClusterRoleBindings(ctx context.Context, m *fsm, s *systemState) (stateFn, *ctrl.Result, error) {
Expand Down Expand Up @@ -104,16 +100,15 @@ func getKubeconfigSecret(ctx context.Context, cnt client.Client, runtimeID, name
return kubeconfigSecret, nil
}

func isRBACUserKindOneOf(names []string) func(rbacv1.Subject) bool {
func isRBACUserKind() func(rbacv1.Subject) bool {
return func(s rbacv1.Subject) bool {
return s.Kind == rbacv1.UserKind &&
slices.Contains(names, s.Name)
return s.Kind == rbacv1.UserKind
}
}

func isRBACServiceAccountKind() func(rbacv1.Subject) bool {
func isRBACUserKindOneOf(names []string) func(rbacv1.Subject) bool {
return func(s rbacv1.Subject) bool {
return s.Kind == rbacv1.ServiceAccountKind
return slices.Contains(names, s.Name)
}
}

Expand All @@ -126,18 +121,17 @@ func getRemoved(crbs []rbacv1.ClusterRoleBinding, admins []string) (removed []rb
}

if crb.RoleRef.Kind != "ClusterRole" && crb.RoleRef.Name != "cluster-admin" {
// cluster role binding is not admin
continue
}

index := slices.IndexFunc(crb.Subjects, isRBACUserKindOneOf(admins))
if index >= 0 {
// cluster role binding does not contain user subject
if !slices.ContainsFunc(crb.Subjects, isRBACUserKind()) {
// cluster role binding is not user kind
continue
}

index = slices.IndexFunc(crb.Subjects, isRBACServiceAccountKind())
if index >= 0 {
// cluster role binding does not contain serviceaccount subject
if slices.ContainsFunc(crb.Subjects, isRBACUserKindOneOf(admins)) {
// the administrator was not removed
continue
}

Expand All @@ -149,11 +143,9 @@ func getRemoved(crbs []rbacv1.ClusterRoleBinding, admins []string) (removed []rb
}

func managedByKIM(crb rbacv1.ClusterRoleBinding) bool {
selector := labels.Set(crb.Labels).AsSelector()
isManagedByKIM := selector.Matches(labels.Set(labelsManagedByKIM))
isManagedByReconciler := selector.Matches(labels.Set(labelsManagedByReconciler))
// Provisioner managed CRBs with label managed-by=reconciler, we have to manage them as well
return isManagedByKIM || isManagedByReconciler
selector := labels.Set(labelsManagedByKIM).AsSelector()
isManagedByKIM := selector.Matches(labels.Set(crb.Labels))
return isManagedByKIM
}

//nolint:gochecknoglobals
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ var _ = Describe(`runtime_fsm_apply_crb`, Label("applyCRB"), func() {
admins: []string{"test1", "test2", "test3"},
crbs: []rbacv1.ClusterRoleBinding{
toAdminClusterRoleBinding("test1"),
toManagedClusterRoleBinding("test2", "reconciler"),
toManagedClusterRoleBinding("test2", "infrastructure-manager"),
toManagedClusterRoleBinding("test3", "infrastructure-manager"),
},
expected: nil,
Expand Down Expand Up @@ -129,7 +129,6 @@ var _ = Describe(`runtime_fsm_apply_crb`, Label("applyCRB"), func() {
},
expected: []rbacv1.ClusterRoleBinding{
toManagedClusterRoleBinding("test1", "infrastructure-manager"),
toManagedClusterRoleBinding("test2", "reconciler"),
},
}),
)
Expand Down

0 comments on commit 7629903

Please sign in to comment.