diff --git a/internal/controller/runtime/fsm/runtime_fsm_apply_clusterrolebindings.go b/internal/controller/runtime/fsm/runtime_fsm_apply_clusterrolebindings.go index 2b63b63f..a73bffc2 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_apply_clusterrolebindings.go +++ b/internal/controller/runtime/fsm/runtime_fsm_apply_clusterrolebindings.go @@ -17,10 +17,13 @@ import ( var ( //nolint:gochecknoglobals - labelsClusterRoleBindings = map[string]string{ - "app": "kyma", + 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) { @@ -94,7 +97,7 @@ func isRBACUserKindOneOf(names []string) func(rbacv1.Subject) bool { func getRemoved(crbs []rbacv1.ClusterRoleBinding, admins []string) (removed []rbacv1.ClusterRoleBinding) { // iterate over cluster role bindings to find out removed administrators for _, crb := range crbs { - if !labels.Set(crb.Labels).AsSelector().Matches(labels.Set(labelsClusterRoleBindings)) { + if !managedByKIM(crb) { // cluster role binding is not controlled by KIM continue } @@ -116,10 +119,18 @@ func getRemoved(crbs []rbacv1.ClusterRoleBinding, admins []string) (removed []rb return removed } +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 +} + //nolint:gochecknoglobals var newContainsAdmin = func(admin string) func(rbacv1.ClusterRoleBinding) bool { return func(crb rbacv1.ClusterRoleBinding) bool { - if !labels.Set(crb.Labels).AsSelector().Matches(labels.Set(labelsClusterRoleBindings)) { + if !managedByKIM(crb) { return false } isAdmin := isRBACUserKindOneOf([]string{admin}) @@ -144,7 +155,7 @@ func toAdminClusterRoleBinding(name string) rbacv1.ClusterRoleBinding { return rbacv1.ClusterRoleBinding{ ObjectMeta: metav1.ObjectMeta{ GenerateName: "admin-", - Labels: labelsClusterRoleBindings, + Labels: labelsManagedByKIM, }, Subjects: []rbacv1.Subject{{ Kind: rbacv1.UserKind, diff --git a/internal/controller/runtime/fsm/runtime_fsm_apply_crb_test.go b/internal/controller/runtime/fsm/runtime_fsm_apply_crb_test.go index c8b7eb38..5fbea952 100644 --- a/internal/controller/runtime/fsm/runtime_fsm_apply_crb_test.go +++ b/internal/controller/runtime/fsm/runtime_fsm_apply_crb_test.go @@ -31,11 +31,11 @@ var _ = Describe(`runtime_fsm_apply_crb`, Label("applyCRB"), func() { } DescribeTable("getMissing", - func(tc tcGetCRB) { + func(tc tcCRBData) { actual := getMissing(tc.crbs, tc.admins) Expect(actual).To(BeComparableTo(tc.expected)) }, - Entry("should return a list with CRBs to be created", tcGetCRB{ + Entry("should return a list with CRBs to be created", tcCRBData{ admins: []string{"test1", "test2"}, crbs: nil, expected: []rbacv1.ClusterRoleBinding{ @@ -43,7 +43,16 @@ var _ = Describe(`runtime_fsm_apply_crb`, Label("applyCRB"), func() { toAdminClusterRoleBinding("test2"), }, }), - Entry("should return nil list if no admins missing", tcGetCRB{ + Entry("should return nil list if no admins missing", tcCRBData{ + admins: []string{"test1", "test2", "test3"}, + crbs: []rbacv1.ClusterRoleBinding{ + toAdminClusterRoleBinding("test1"), + toManagedClusterRoleBinding("test2", "reconciler"), + toManagedClusterRoleBinding("test3", "infrastructure-manager"), + }, + expected: nil, + }), + Entry("should return nil list if no admins missing", tcCRBData{ admins: []string{"test1"}, crbs: []rbacv1.ClusterRoleBinding{ toAdminClusterRoleBinding("test1"), @@ -53,26 +62,26 @@ var _ = Describe(`runtime_fsm_apply_crb`, Label("applyCRB"), func() { ) DescribeTable("getRemoved", - func(tc tcGetCRB) { + func(tc tcCRBData) { actual := getRemoved(tc.crbs, tc.admins) Expect(actual).To(BeComparableTo(tc.expected)) }, - Entry("should return nil list if CRB list is nil", tcGetCRB{ + Entry("should return nil list if CRB list is nil", tcCRBData{ admins: []string{"test1"}, crbs: nil, expected: nil, }), - Entry("should return nil list if CRB list is empty", tcGetCRB{ + Entry("should return nil list if CRB list is empty", tcCRBData{ admins: []string{"test1"}, crbs: []rbacv1.ClusterRoleBinding{}, expected: nil, }), - Entry("should return nil list if no admins to remove", tcGetCRB{ + Entry("should return nil list if no admins to remove", tcCRBData{ admins: []string{"test1"}, crbs: []rbacv1.ClusterRoleBinding{toAdminClusterRoleBinding("test1")}, expected: nil, }), - Entry("should return list if with CRBs to remove", tcGetCRB{ + Entry("should return list if with CRBs to remove", tcCRBData{ admins: []string{"test2"}, crbs: []rbacv1.ClusterRoleBinding{ toAdminClusterRoleBinding("test1"), @@ -84,6 +93,38 @@ var _ = Describe(`runtime_fsm_apply_crb`, Label("applyCRB"), func() { toAdminClusterRoleBinding("test3"), }, }), + Entry("should not remove CRB managed by reconciler", tcCRBData{ + admins: []string{"test1", "test2"}, + crbs: []rbacv1.ClusterRoleBinding{ + toManagedClusterRoleBinding("test1", "reconciler"), + toAdminClusterRoleBinding("test3"), + }, + expected: []rbacv1.ClusterRoleBinding{ + toAdminClusterRoleBinding("test3"), + }, + }), + Entry("should not remove CRB not managed by reconciler or KIM", tcCRBData{ + admins: []string{"test1", "test2"}, + crbs: []rbacv1.ClusterRoleBinding{ + toManagedClusterRoleBinding("test3", "should-stay"), + toManagedClusterRoleBinding("test4", ""), + }, + expected: nil, + }), + Entry("should remove CRB managed by reconciler or KIM, that are not in the admin list", tcCRBData{ + admins: []string{"test4", "test5"}, + crbs: []rbacv1.ClusterRoleBinding{ + toManagedClusterRoleBinding("test1", "infrastructure-manager"), + toManagedClusterRoleBinding("test2", "reconciler"), + toManagedClusterRoleBinding("test3", "should-stay"), + toManagedClusterRoleBinding("test4", "infrastructure-manager"), + toManagedClusterRoleBinding("test5", "reconciler"), + }, + expected: []rbacv1.ClusterRoleBinding{ + toManagedClusterRoleBinding("test1", "infrastructure-manager"), + toManagedClusterRoleBinding("test2", "reconciler"), + }, + }), ) testRuntime := imv1.Runtime{ @@ -199,7 +240,7 @@ var _ = Describe(`runtime_fsm_apply_crb`, Label("applyCRB"), func() { ) }) -type tcGetCRB struct { +type tcCRBData struct { crbs []rbacv1.ClusterRoleBinding admins []string expected []rbacv1.ClusterRoleBinding @@ -237,3 +278,11 @@ func newTestScheme() (*runtime.Scheme, error) { } return schema, nil } + +func toManagedClusterRoleBinding(name, managedBy string) rbacv1.ClusterRoleBinding { + clusterRoleBinding := toAdminClusterRoleBinding(name) + clusterRoleBinding.Labels = map[string]string{ + "reconciler.kyma-project.io/managed-by": managedBy, + } + return clusterRoleBinding +}