From 4c27c1090b1e0e0b7bb4a9304248e1304ad0bb15 Mon Sep 17 00:00:00 2001
From: Christian Schlotter <christian.schlotter@broadcom.com>
Date: Thu, 29 Aug 2024 15:54:12 +0200
Subject: [PATCH] rbac: fix deduplication of core group and add test coverage

---
 pkg/rbac/parser.go              | 13 +++++++------
 pkg/rbac/testdata/controller.go |  3 +++
 pkg/rbac/testdata/role.yaml     | 15 +++++++++++++++
 3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/pkg/rbac/parser.go b/pkg/rbac/parser.go
index d9f377b2d..89729d436 100644
--- a/pkg/rbac/parser.go
+++ b/pkg/rbac/parser.go
@@ -146,12 +146,6 @@ func removeDupAndSort(strs []string) []string {
 
 // ToRule converts this rule to its Kubernetes API form.
 func (r *Rule) ToRule() rbacv1.PolicyRule {
-	// fix the group names first, since letting people type "core" is nice
-	for i, group := range r.Groups {
-		if group == "core" {
-			r.Groups[i] = ""
-		}
-	}
 	return rbacv1.PolicyRule{
 		APIGroups:       r.Groups,
 		Verbs:           r.Verbs,
@@ -230,6 +224,13 @@ func GenerateRoles(ctx *genall.GenerationContext, roleName string) ([]interface{
 		ruleMap := make(map[ruleKey]*Rule)
 		// all the Rules having the same ruleKey will be merged into the first Rule
 		for _, rule := range rules {
+			// fix the group name first, since letting people type "core" is nice
+			for i, name := range rule.Groups {
+				if name == "core" {
+					rule.Groups[i] = ""
+				}
+			}
+
 			key := rule.key()
 			if _, ok := ruleMap[key]; !ok {
 				ruleMap[key] = rule
diff --git a/pkg/rbac/testdata/controller.go b/pkg/rbac/testdata/controller.go
index 9a8f5d256..84831a71e 100644
--- a/pkg/rbac/testdata/controller.go
+++ b/pkg/rbac/testdata/controller.go
@@ -30,3 +30,6 @@ package controller
 // +kubebuilder:rbac:groups=not-deduplicate-groups2,resources=some,verbs=list
 // +kubebuilder:rbac:urls=/url-to-duplicate,verbs=get
 // +kubebuilder:rbac:urls=/another/url-to-duplicate,verbs=get
+// +kubebuilder:rbac:groups=core,resources=deduplicate,verbs=list
+// +kubebuilder:rbac:groups="",resources=me,verbs=list
+// +kubebuilder:rbac:groups=core;"";some-other-to-deduplicate-with-core,resources=me,verbs=list;get
diff --git a/pkg/rbac/testdata/role.yaml b/pkg/rbac/testdata/role.yaml
index 44ac99ce0..00e914050 100644
--- a/pkg/rbac/testdata/role.yaml
+++ b/pkg/rbac/testdata/role.yaml
@@ -9,6 +9,21 @@ rules:
   - /url-to-duplicate
   verbs:
   - get
+- apiGroups:
+  - ""
+  resources:
+  - deduplicate
+  - me
+  verbs:
+  - list
+- apiGroups:
+  - ""
+  - some-other-to-deduplicate-with-core
+  resources:
+  - me
+  verbs:
+  - get
+  - list
 - apiGroups:
   - art
   resources: