From af7a9391d55303d58f839a64df00531b25a52e61 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Sun, 10 Dec 2017 12:32:21 -0500 Subject: [PATCH] Compact rules that differ only by resourceName --- pkg/process_test.go | 24 ++++++------------------ pkg/util.go | 17 +++++++++++++++-- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/pkg/process_test.go b/pkg/process_test.go index 2598602..b17ac7e 100644 --- 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 index 0a2165a..79bbdc9 100644 --- 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...)