From 083927137dc097db7428e1db096e912fb6ba9a6f Mon Sep 17 00:00:00 2001 From: hongming Date: Thu, 26 Sep 2024 16:13:18 +0800 Subject: [PATCH] fix: remove the incorrect RBAC rule merging logic (#6209) Signed-off-by: hongming --- pkg/componenthelper/auth/rbac/helper.go | 3 +-- pkg/componenthelper/auth/rbac/policy_comparator.go | 3 ++- pkg/controller/namespace/namespace_controller.go | 2 +- pkg/controller/namespace/namespace_controller_test.go | 7 +++++-- pkg/controller/role/role_controller.go | 4 ++-- 5 files changed, 11 insertions(+), 8 deletions(-) diff --git a/pkg/componenthelper/auth/rbac/helper.go b/pkg/componenthelper/auth/rbac/helper.go index 3ffab0dcb..3027ec00e 100644 --- a/pkg/componenthelper/auth/rbac/helper.go +++ b/pkg/componenthelper/auth/rbac/helper.go @@ -157,8 +157,7 @@ func (h *Helper) AggregationRole(ctx context.Context, ruleOwner RuleOwner, recor if !cover { needUpdate = true newRule := append(ruleOwner.GetRules(), uncovered...) - squashedRules := SquashRules(len(newRule), newRule) - ruleOwner.SetRules(squashedRules) + ruleOwner.SetRules(newRule) } if !templateNamesEqual { diff --git a/pkg/componenthelper/auth/rbac/policy_comparator.go b/pkg/componenthelper/auth/rbac/policy_comparator.go index f9d75cf8a..7d8a21d1b 100644 --- a/pkg/componenthelper/auth/rbac/policy_comparator.go +++ b/pkg/componenthelper/auth/rbac/policy_comparator.go @@ -149,7 +149,8 @@ func ruleCovers(ownerRule, subRule rbacv1.PolicyRule) bool { verbMatches := has(ownerRule.Verbs, rbacv1.VerbAll) || hasAll(ownerRule.Verbs, subRule.Verbs) groupMatches := has(ownerRule.APIGroups, rbacv1.APIGroupAll) || hasAll(ownerRule.APIGroups, subRule.APIGroups) resourceMatches := resourceCoversAll(ownerRule.Resources, subRule.Resources) - nonResourceURLMatches := nonResourceURLsCoversAll(ownerRule.NonResourceURLs, subRule.NonResourceURLs) + nonResourceURLMatches := (len(ownerRule.NonResourceURLs) == 0 && len(subRule.NonResourceURLs) == 0) || (len(ownerRule.Resources) == 0 && + len(subRule.Resources) == 0 && nonResourceURLsCoversAll(ownerRule.NonResourceURLs, subRule.NonResourceURLs)) resourceNameMatches := false diff --git a/pkg/controller/namespace/namespace_controller.go b/pkg/controller/namespace/namespace_controller.go index f375b3e15..2896b9508 100644 --- a/pkg/controller/namespace/namespace_controller.go +++ b/pkg/controller/namespace/namespace_controller.go @@ -140,7 +140,7 @@ func (r *Reconciler) reconcileWorkspaceOwnerReference(ctx context.Context, names return nil } - if !metav1.IsControlledBy(namespace, workspace) { + if !metav1.IsControlledBy(namespace, workspace) && namespace.Labels[constants.KubeSphereManagedLabel] == "true" { namespace = namespace.DeepCopy() if err := controllerutil.SetControllerReference(workspace, namespace, scheme.Scheme); err != nil { return err diff --git a/pkg/controller/namespace/namespace_controller_test.go b/pkg/controller/namespace/namespace_controller_test.go index 34fd89669..8f6c6ef79 100644 --- a/pkg/controller/namespace/namespace_controller_test.go +++ b/pkg/controller/namespace/namespace_controller_test.go @@ -42,8 +42,11 @@ var _ = Describe("Namespace", func() { It("Should create successfully", func() { namespace := &corev1.Namespace{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-namespace", - Labels: map[string]string{tenantv1beta1.WorkspaceLabel: workspace.Name}, + Name: "test-namespace", + Labels: map[string]string{ + tenantv1beta1.WorkspaceLabel: workspace.Name, + constants.KubeSphereManagedLabel: "true", + }, }, } diff --git a/pkg/controller/role/role_controller.go b/pkg/controller/role/role_controller.go index 56145e7f0..4e60573fd 100644 --- a/pkg/controller/role/role_controller.go +++ b/pkg/controller/role/role_controller.go @@ -96,9 +96,9 @@ func (r *Reconciler) syncToKubernetes(ctx context.Context, role *iamv1beta1.Role }) if err != nil { - r.logger.Error(err, "sync role failed", "role", role.Name) + r.logger.Error(err, "sync role failed", "namespace", role.Namespace, "role", role.Name) } - r.logger.V(4).Info("sync role to K8s", "role", role.Name, "op", op) + r.logger.V(4).Info("sync role to K8s", "namespace", role.Namespace, "role", role.Name, "op", op) return nil }