Compact rules that differ only by resourceName

liggitt · Dec 10, 2017 · af7a939 · af7a939
1 parent e217575
commit af7a939
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 20 deletions.
diff --git a/pkg/process_test.go b/pkg/process_test.go
@@ -148,12 +148,8 @@ func TestProcessOptions(t *testing.T) {
 				ClusterRoles: []*rbacinternal.ClusterRole{&rbacinternal.ClusterRole{
 					ObjectMeta: metav1.ObjectMeta{Name: "audit2rbac"},
 					Rules: []rbacinternal.PolicyRule{
-						// TODO: improve compaction to make this a single rule referencing two names
-						rbacinternal.NewRule("get").Groups("").Resources("nodes").Names("node1").RuleOrDie(),
-						rbacinternal.NewRule("get").Groups("").Resources("nodes").Names("node2").RuleOrDie(),
-						// TODO: improve compaction to make this a single rule referencing two names
-						rbacinternal.NewRule("get").Groups("storage.k8s.io").Resources("storageclasses").Names("sc1").RuleOrDie(),
-						rbacinternal.NewRule("get").Groups("storage.k8s.io").Resources("storageclasses").Names("sc2").RuleOrDie(),
+						rbacinternal.NewRule("get").Groups("").Resources("nodes").Names("node1", "node2").RuleOrDie(),
+						rbacinternal.NewRule("get").Groups("storage.k8s.io").Resources("storageclasses").Names("sc1", "sc2").RuleOrDie(),
 					},
 				}},
 				ClusterRoleBindings: []*rbacinternal.ClusterRoleBinding{&rbacinternal.ClusterRoleBinding{
@@ -439,23 +435,15 @@ func TestProcessOptions(t *testing.T) {
 					&rbacinternal.Role{
 						ObjectMeta: metav1.ObjectMeta{Name: "audit2rbac", Namespace: "ns2"},
 						Rules: []rbacinternal.PolicyRule{
-							// TODO: improve compaction to make this a single rule referencing two names
-							rbacinternal.NewRule("get").Groups("").Resources("pods").Names("pod1").RuleOrDie(),
-							rbacinternal.NewRule("get").Groups("").Resources("pods").Names("pod2").RuleOrDie(),
-							// TODO: improve compaction to make this a single rule referencing two names
-							rbacinternal.NewRule("get").Groups("apps").Resources("deployments").Names("dep1").RuleOrDie(),
-							rbacinternal.NewRule("get").Groups("apps").Resources("deployments").Names("dep2").RuleOrDie(),
+							rbacinternal.NewRule("get").Groups("").Resources("pods").Names("pod1", "pod2").RuleOrDie(),
+							rbacinternal.NewRule("get").Groups("apps").Resources("deployments").Names("dep1", "dep2").RuleOrDie(),
 						},
 					},
 					&rbacinternal.Role{
 						ObjectMeta: metav1.ObjectMeta{Name: "audit2rbac", Namespace: "ns3"},
 						Rules: []rbacinternal.PolicyRule{
-							// TODO: improve compaction to make this a single rule referencing two names
-							rbacinternal.NewRule("get").Groups("").Resources("pods").Names("pod1").RuleOrDie(),
-							rbacinternal.NewRule("get").Groups("").Resources("pods").Names("pod3").RuleOrDie(),
-							// TODO: improve compaction to make this a single rule referencing two names
-							rbacinternal.NewRule("get").Groups("apps").Resources("deployments").Names("dep1").RuleOrDie(),
-							rbacinternal.NewRule("get").Groups("apps").Resources("deployments").Names("dep3").RuleOrDie(),
+							rbacinternal.NewRule("get").Groups("").Resources("pods").Names("pod1", "pod3").RuleOrDie(),
+							rbacinternal.NewRule("get").Groups("apps").Resources("deployments").Names("dep1", "dep3").RuleOrDie(),
 						},
 					},
 				},

diff --git a/pkg/util.go b/pkg/util.go
@@ -67,12 +67,25 @@ func compactRules(rules []rbac.PolicyRule) []rbac.PolicyRule {
 		// strip resource
 		resourcelessRule := rule
 		resourcelessRule.Resources = nil
+		// strip name
+		namelessRule := rule
+		namelessRule.ResourceNames = nil
 		for j, accumulatingRule := range accumulatingRules {
+			// strip name
+			namelessAccumulatingRule := accumulatingRule
+			namelessAccumulatingRule.ResourceNames = nil
+			if reflect.DeepEqual(namelessRule, namelessAccumulatingRule) {
+				combinedNames := sets.NewString(accumulatingRule.ResourceNames...)
+				combinedNames.Insert(rule.ResourceNames...)
+				accumulatingRule.ResourceNames = combinedNames.List()
+				accumulatingRules[j] = accumulatingRule
+				accumulated = true
+				break
+			}
+
 			// strip resource
 			resourcelessAccumulatingRule := accumulatingRule
 			resourcelessAccumulatingRule.Resources = nil
-
-			// if all other fields are identical (api group, verbs, names, etc, accumulate resources)
 			if reflect.DeepEqual(resourcelessRule, resourcelessAccumulatingRule) {
 				combinedResources := sets.NewString(accumulatingRule.Resources...)
 				combinedResources.Insert(rule.Resources...)