Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

no longer deletes crbs managed by reconciler #499

Merged
merged 4 commits into from
Nov 21, 2024
Merged

no longer deletes crbs managed by reconciler #499

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d5b49b0
no longer deletes crbs managed by reconciler
Disper Nov 21, 2024
07de66e
Merge branch 'main' into crb_proper_recreation
Disper Nov 21, 2024
7be4336
linter fix
Disper Nov 21, 2024
6c1870a
Merge branch 'main' into crb_proper_recreation
Disper Nov 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions internal/controller/runtime/fsm/runtime_fsm_apply_clusterrolebindings.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
}
Expand All @@ -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})
Expand All @@ -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,
Expand Down
67 changes: 58 additions & 9 deletions internal/controller/runtime/fsm/runtime_fsm_apply_crb_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,28 @@ 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{
toAdminClusterRoleBinding("test1"),
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"),
Expand All @@ -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"),
Expand All @@ -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{
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Loading