Merge pull request #1044 from chrischdi/pr-rbac-fix-urls

🐛 rbac: fix adding nonResourceURLs including normalisation

pkg/rbac/parser.go

@@ -105,6 +105,12 @@ func (r *Rule) keyWithResourcesResourceNamesURLsVerbs() string {
 	return fmt.Sprintf("%s + %s + %s + %s", key.Resources, key.ResourceNames, key.URLs, verbs)
 }
 
+func (r *Rule) keyWitGroupResourcesResourceNamesVerbs() string {
+	key := r.key()
+	verbs := strings.Join(r.Verbs, "&")
+	return fmt.Sprintf("%s + %s + %s + %s", key.Groups, key.Resources, key.ResourceNames, 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) {
@@ -190,6 +196,20 @@ func GenerateRoles(ctx *genall.GenerationContext, roleName string) ([]interface{
 		// group RBAC markers by namespace and separate by resource
 		for _, markerValue := range markerSet[RuleDefinition.Name] {
 			rule := markerValue.(Rule)
+			if len(rule.Resources) == 0 {
+				// Add a rule without any resource if Resources is empty.
+				r := Rule{
+					Groups:        rule.Groups,
+					Resources:     []string{},
+					ResourceNames: rule.ResourceNames,
+					URLs:          rule.URLs,
+					Namespace:     rule.Namespace,
+					Verbs:         rule.Verbs,
+				}
+				namespace := r.Namespace
+				rulesByNSResource[namespace] = append(rulesByNSResource[namespace], &r)
+				continue
+			}
 			for _, resource := range rule.Resources {
 				r := Rule{
 					Groups:        rule.Groups,
@@ -257,6 +277,25 @@ func GenerateRoles(ctx *genall.GenerationContext, roleName string) ([]interface{
 			ruleMap[key] = rule
 		}
 
+		// deduplicate URLs
+		// 1. create map based on key without URLs
+		ruleMapWithoutURLs := make(map[string][]*Rule)
+		for _, rule := range ruleMap {
+			// get key without Group
+			key := rule.keyWitGroupResourcesResourceNamesVerbs()
+			ruleMapWithoutURLs[key] = append(ruleMapWithoutURLs[key], rule)
+		}
+		// 2. merge to ruleMap
+		ruleMap = make(map[ruleKey]*Rule)
+		for _, rules := range ruleMapWithoutURLs {
+			rule := rules[0]
+			for _, mergeRule := range rules[1:] {
+				rule.URLs = append(rule.URLs, mergeRule.URLs...)
+			}
+			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 {

pkg/rbac/testdata/controller.go

@@ -28,3 +28,5 @@ package controller
 // +kubebuilder:rbac:groups=not-deduplicate-resources,resources=another,verbs=list
 // +kubebuilder:rbac:groups=not-deduplicate-groups1,resources=some,verbs=get
 // +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

pkg/rbac/testdata/role.yaml

@@ -4,6 +4,11 @@ kind: ClusterRole
 metadata:
   name: manager-role
 rules:
+- nonResourceURLs:
+  - /another/url-to-duplicate
+  - /url-to-duplicate
+  verbs:
+  - get
 - apiGroups:
   - art
   resources: