rbac: deduplicate apiGroups

pkg/rbac/parser.go

@@ -99,6 +99,12 @@ func (r *Rule) KeyWithoutResources() string {
 	return fmt.Sprintf("%s + %s + %s + %s", key.Groups, key.ResourceNames, key.URLs, verbs)
 }
 
+func (r *Rule) KeyWithoutGroups() string {
+	key := r.key()
+	verbs := strings.Join(r.Verbs, "&")
+	return fmt.Sprintf("%s + %s + %s + %s", key.Resources, key.ResourceNames, key.URLs, verbs)
+}
+
 // addVerbs adds new verbs into a Rule.
 // The duplicates in `r.Verbs` will be removed, and then `r.Verbs` will be sorted.
 func (r *Rule) addVerbs(verbs []string) {
@@ -174,7 +180,7 @@ func (Generator) RegisterMarkers(into *markers.Registry) error {
 // GenerateRoles generate a slice of objs representing either a ClusterRole or a Role object
 // The order of the objs in the returned slice is stable and determined by their namespaces.
 func GenerateRoles(ctx *genall.GenerationContext, roleName string) ([]interface{}, error) {
-	rulesByNSAndResource := make(map[string][]*Rule)
+	rulesByNSResource := make(map[string][]*Rule)
 	for _, root := range ctx.Roots {
 		markerSet, err := markers.PackageMarkers(ctx.Collector, root)
 		if err != nil {
@@ -194,43 +200,42 @@ func GenerateRoles(ctx *genall.GenerationContext, roleName string) ([]interface{
 					Verbs:         rule.Verbs,
 				}
 				namespace := r.Namespace
-				if _, ok := rulesByNSAndResource[namespace]; !ok {
+				if _, ok := rulesByNSResource[namespace]; !ok {
 					rules := make([]*Rule, 0)
-					rulesByNSAndResource[namespace] = rules
+					rulesByNSResource[namespace] = rules
 				}
-				rulesByNSAndResource[namespace] = append(rulesByNSAndResource[namespace], &r)
+				rulesByNSResource[namespace] = append(rulesByNSResource[namespace], &r)
 			}
 		}
 	}
 
 	// NormalizeRules merge Rule with the same ruleKey and sort the Rules
 	NormalizeRules := func(rules []*Rule) []rbacv1.PolicyRule {
-		ruleMapByResource := make(map[ruleKey]*Rule)
+		ruleMap := make(map[ruleKey]*Rule)
 		// all the Rules having the same ruleKey will be merged into the first Rule
 		for _, rule := range rules {
 			key := rule.key()
-			if _, ok := ruleMapByResource[key]; !ok {
-				ruleMapByResource[key] = rule
+			if _, ok := ruleMap[key]; !ok {
+				ruleMap[key] = rule
 				continue
 			}
-			ruleMapByResource[key].addVerbs(rule.Verbs)
+			ruleMap[key].addVerbs(rule.Verbs)
 		}
 
-		// all rules having the same Groups, ResourceNames, URLs and Verbs will be
-		// merged together.
+		// deduplicate resources
+		// 1. create map based on key without resources
 		ruleMapWithoutResources := make(map[string][]*Rule)
-		for _, rule := range ruleMapByResource {
+		for _, rule := range ruleMap {
 			// unset Resources on the key
-			groupKey := rule.KeyWithoutResources()
-			if _, ok := ruleMapWithoutResources[groupKey]; !ok {
+			key := rule.KeyWithoutResources()
+			if _, ok := ruleMapWithoutResources[key]; !ok {
 				rules := make([]*Rule, 0)
-				ruleMapWithoutResources[groupKey] = rules
+				ruleMapWithoutResources[key] = rules
 			}
-			ruleMapWithoutResources[groupKey] = append(ruleMapWithoutResources[groupKey], rule)
+			ruleMapWithoutResources[key] = append(ruleMapWithoutResources[key], rule)
 		}
-
-		// all rules which are equal except for resources get merged together.
-		ruleMap := make(map[ruleKey]*Rule)
+		// 2. merge to ruleMap
+		ruleMap = make(map[ruleKey]*Rule)
 		for _, rules := range ruleMapWithoutResources {
 			rule := rules[0]
 			for _, mergeRule := range rules[1:] {
@@ -241,6 +246,29 @@ func GenerateRoles(ctx *genall.GenerationContext, roleName string) ([]interface{
 			ruleMap[key] = rule
 		}
 
+		// deduplicate groups
+		// 1. create map based on key without group
+		ruleMapWithoutGroup := make(map[string][]*Rule)
+		for _, rule := range ruleMap {
+			// unset Resources on the key
+			key := rule.KeyWithoutGroups()
+			if _, ok := ruleMapWithoutGroup[key]; !ok {
+				rules := make([]*Rule, 0)
+				ruleMapWithoutGroup[key] = rules
+			}
+			ruleMapWithoutGroup[key] = append(ruleMapWithoutGroup[key], rule)
+		}
+		// 2. merge to ruleMap
+		ruleMap = make(map[ruleKey]*Rule)
+		for _, rules := range ruleMapWithoutGroup {
+			rule := rules[0]
+			for _, mergeRule := range rules[1:] {
+				rule.Groups = append(rule.Groups, mergeRule.Groups...)
+			}
+			key := rule.key()
+			ruleMap[key] = rule
+		}
+
 		// sort the Rules in rules according to their ruleKeys
 		keys := make([]ruleKey, 0, len(ruleMap))
 		for key := range ruleMap {
@@ -257,15 +285,15 @@ func GenerateRoles(ctx *genall.GenerationContext, roleName string) ([]interface{
 
 	// collect all the namespaces and sort them
 	var namespaces []string
-	for ns := range rulesByNSAndResource {
+	for ns := range rulesByNSResource {
 		namespaces = append(namespaces, ns)
 	}
 	sort.Strings(namespaces)
 
 	// process the items in rulesByNS by the order specified in `namespaces` to make sure that the Role order is stable
 	var objs []interface{}
 	for _, ns := range namespaces {
-		rules := rulesByNSAndResource[ns]
+		rules := rulesByNSResource[ns]
 		policyRules := NormalizeRules(rules)
 		if len(policyRules) == 0 {
 			continue

pkg/rbac/testdata/role.yaml

@@ -81,11 +81,6 @@ metadata:
 rules:
 - apiGroups:
   - art
-  resources:
-  - jobs
-  verbs:
-  - get
-- apiGroups:
   - wave
   resources:
   - jobs