diff --git a/pkg/rbac/parser.go b/pkg/rbac/parser.go index 3bbf11fd4..92a8f9587 100644 --- a/pkg/rbac/parser.go +++ b/pkg/rbac/parser.go @@ -23,22 +23,20 @@ limitations under the License. package rbac import ( - "fmt" "sort" "strings" rbacv1 "k8s.io/api/rbac/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + sets "k8s.io/apimachinery/pkg/util/sets" "sigs.k8s.io/controller-tools/pkg/genall" "sigs.k8s.io/controller-tools/pkg/markers" ) -var ( - // RuleDefinition is a marker for defining RBAC rules. - // Call ToRule on the value to get a Kubernetes RBAC policy rule. - RuleDefinition = markers.Must(markers.MakeDefinition("kubebuilder:rbac", markers.DescribesPackage, Rule{})) -) +// RuleDefinition is a marker for defining RBAC rules. +// Call ToRule on the value to get a Kubernetes RBAC policy rule. +var RuleDefinition = markers.Must(markers.MakeDefinition("kubebuilder:rbac", markers.DescribesPackage, Rule{})) // +controllertools:marker:generateHelp:category=RBAC @@ -63,83 +61,107 @@ type Rule struct { Namespace string `marker:",optional"` } -// ruleKey represents the resources and non-resources a Rule applies. -type ruleKey struct { - Groups string - Resources string - ResourceNames string - URLs string -} +func newSet(strings []string) sets.String { + if strings == nil { // preserve nils for back-compat with existing code + return nil + } -func (key ruleKey) String() string { - return fmt.Sprintf("%s + %s + %s + %s", key.Groups, key.Resources, key.ResourceNames, key.URLs) + return sets.NewString(strings...) } -// ruleKeys implements sort.Interface -type ruleKeys []ruleKey - -func (keys ruleKeys) Len() int { return len(keys) } -func (keys ruleKeys) Swap(i, j int) { keys[i], keys[j] = keys[j], keys[i] } -func (keys ruleKeys) Less(i, j int) bool { return keys[i].String() < keys[j].String() } - -// key normalizes the Rule and returns a ruleKey object. -func (r *Rule) key() ruleKey { - r.normalize() - return ruleKey{ - Groups: strings.Join(r.Groups, "&"), - Resources: strings.Join(r.Resources, "&"), - ResourceNames: strings.Join(r.ResourceNames, "&"), - URLs: strings.Join(r.URLs, "&"), +func (r *Rule) Normalize() *NormalizedRule { + result := &NormalizedRule{ + Groups: newSet(r.Groups), + Resources: newSet(r.Resources), + ResourceNames: newSet(r.ResourceNames), + Verbs: newSet(r.Verbs), + URLs: newSet(r.URLs), + Namespace: r.Namespace, + } + + // simplify Resources and Verbs which both support special "*" to mean 'any' + // if this is specified, remove all other names + if result.Resources.Has("*") { + result.Resources = sets.NewString("*") } + + if result.Verbs.Has("*") { + result.Verbs = sets.NewString("*") + } + + // fix the group names, since letting people type "core" is nice + if result.Groups.Has("core") { + result.Groups.Delete("core") + result.Groups.Insert("") + } + + result.GenerateComparisonKey() + + return result } -// 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) { - r.Verbs = removeDupAndSort(append(r.Verbs, verbs...)) +type NormalizedRule struct { + // if two different rules have the same comparison key then they can have their verbs merged + // this key should comprise all the fields below except Verbs and Namespace (since rules + // are partitioned by namespace before merging) + ComparisonKey string + + Namespace string + Groups sets.String + Resources sets.String + ResourceNames sets.String + URLs sets.String + + Verbs sets.String } -// normalize removes duplicates from each field of a Rule, and sorts each field. -func (r *Rule) normalize() { - r.Groups = removeDupAndSort(r.Groups) - r.Resources = removeDupAndSort(r.Resources) - r.ResourceNames = removeDupAndSort(r.ResourceNames) - r.Verbs = removeDupAndSort(r.Verbs) - r.URLs = removeDupAndSort(r.URLs) +// GenerateComparisonKey generates the ComparisonKey +func (nr *NormalizedRule) GenerateComparisonKey() { + nr.ComparisonKey = strings.Join( + []string{ + strings.Join(setToSorted(nr.Groups), "&"), + strings.Join(setToSorted(nr.Resources), "&"), + strings.Join(setToSorted(nr.ResourceNames), "&"), + strings.Join(setToSorted(nr.URLs), "&"), + }, + " + ") } -// removeDupAndSort removes duplicates in strs, sorts the items, and returns a -// new slice of strings. -func removeDupAndSort(strs []string) []string { - set := make(map[string]bool) - for _, str := range strs { - if _, ok := set[str]; !ok { - set[str] = true - } +func setToSorted(set sets.String) []string { + if set == nil { // preserve nils + return nil } - var result []string - for str := range set { - result = append(result, str) - } + result := set.UnsortedList() sort.Strings(result) return result } +// Subsumes indicates if one rule entirely determines another, +// meaning that the other is unnecessary. +// Remember that Kubernetes RBAC rules are purely additive, there +// are no deny rules. +func (nr *NormalizedRule) Subsumes(other *NormalizedRule) bool { + // See the code for documentation of these fields: https://github.com/kubernetes/api/blob/v0.23.6/rbac/v1/types.go#L49 + return nr.Namespace == other.Namespace && + (nr.Groups.IsSuperset(other.Groups)) && + // Resources supports special "*" to mean "any" + (nr.Resources.Has("*") || nr.Resources.IsSuperset(other.Resources)) && + // Empty ResourceNames means "any" + (len(nr.ResourceNames) == 0 || nr.ResourceNames.IsSuperset(other.ResourceNames)) && + nr.URLs.IsSuperset(other.URLs) && // TODO: check? also URLs can have "*" at specific locations + // Verbs support special "*" to mean "any" + (nr.Verbs.Has("*") || nr.Verbs.IsSuperset(other.Verbs)) +} + // 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] = "" - } - } +func (nr *NormalizedRule) ToRule() rbacv1.PolicyRule { return rbacv1.PolicyRule{ - APIGroups: r.Groups, - Verbs: r.Verbs, - Resources: r.Resources, - ResourceNames: r.ResourceNames, - NonResourceURLs: r.URLs, + APIGroups: setToSorted(nr.Groups), + Verbs: setToSorted(nr.Verbs), + Resources: setToSorted(nr.Resources), + ResourceNames: setToSorted(nr.ResourceNames), + NonResourceURLs: setToSorted(nr.URLs), } } @@ -162,52 +184,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) { - rulesByNS := make(map[string][]*Rule) - for _, root := range ctx.Roots { - markerSet, err := markers.PackageMarkers(ctx.Collector, root) - if err != nil { - root.AddError(err) - } - - // group RBAC markers by namespace - for _, markerValue := range markerSet[RuleDefinition.Name] { - rule := markerValue.(Rule) - namespace := rule.Namespace - if _, ok := rulesByNS[namespace]; !ok { - rules := make([]*Rule, 0) - rulesByNS[namespace] = rules - } - rulesByNS[namespace] = append(rulesByNS[namespace], &rule) - } - } - - // NormalizeRules merge Rule with the same ruleKey and sort the Rules - NormalizeRules := func(rules []*Rule) []rbacv1.PolicyRule { - 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 := ruleMap[key]; !ok { - ruleMap[key] = rule - continue - } - ruleMap[key].addVerbs(rule.Verbs) - } - - // sort the Rules in rules according to their ruleKeys - keys := make([]ruleKey, 0, len(ruleMap)) - for key := range ruleMap { - keys = append(keys, key) - } - sort.Sort(ruleKeys(keys)) - - var policyRules []rbacv1.PolicyRule - for _, key := range keys { - policyRules = append(policyRules, ruleMap[key].ToRule()) - - } - return policyRules - } + rulesByNS := GroupRulesByNamespace(ctx) // collect all the namespaces and sort them var namespaces []string @@ -253,6 +230,101 @@ func GenerateRoles(ctx *genall.GenerationContext, roleName string) ([]interface{ return objs, nil } +func GroupRulesByNamespace(ctx *genall.GenerationContext) map[string][]*Rule { + rulesByNS := make(map[string][]*Rule) + for _, root := range ctx.Roots { + markerSet, err := markers.PackageMarkers(ctx.Collector, root) + if err != nil { + root.AddError(err) + } + + // group RBAC markers by namespace + for _, markerValue := range markerSet[RuleDefinition.Name] { + rule := markerValue.(Rule) + namespace := rule.Namespace + rulesByNS[namespace] = append(rulesByNS[namespace], &rule) + } + } + + return rulesByNS +} + +// insertRule inserts a rule into a destination slice, deduplicating via the +// Subsumes function or merging verbs if appropriate +func insertRule(dest []*NormalizedRule, it *NormalizedRule) []*NormalizedRule { + // this is not going to be very fast but the set of rules should always be small + mergeWith := -1 + for ix, other := range dest { + if other.Subsumes(it) { + // not needed; another rule handles this case + return dest + } + + if it.Subsumes(other) { + // rebuild whole list: + // the 'it' rule subsumes the 'other' rule; + // but it might also subsume other rules in the list, so we + // need to go through and check them as well. + // + // redoing the insertion with `it` first handles this: + result := []*NormalizedRule{it} + for _, d := range dest { + result = insertRule(result, d) + } + + return result + } + + if it.ComparisonKey == other.ComparisonKey { + // match the same things, can merge their + // verbs (if no better match) + mergeWith = ix + } + } + + if mergeWith >= 0 { + // we found one to merge with + insertAll(dest[mergeWith].Verbs, it.Verbs) + return dest + } + + // otherwise, insert it + return append(dest, it) +} + +func insertAll(set sets.String, other sets.String) { + for value := range other { + set.Insert(value) + } +} + +// Sorts the rules for deterministic output: +type ruleSorter []*NormalizedRule + +// ruleSorter implements sort.Interface +var _ sort.Interface = ruleSorter{} + +func (keys ruleSorter) Len() int { return len(keys) } +func (keys ruleSorter) Swap(i, j int) { keys[i], keys[j] = keys[j], keys[i] } +func (keys ruleSorter) Less(i, j int) bool { return keys[i].ComparisonKey < keys[j].ComparisonKey } + +// NormalizeRules merges Rules that can be merged, and sorts the Rules +func NormalizeRules(rules []*Rule) []rbacv1.PolicyRule { + var simplified []*NormalizedRule + for _, rule := range rules { + simplified = insertRule(simplified, rule.Normalize()) + } + + sort.Sort(ruleSorter(simplified)) + + result := make([]rbacv1.PolicyRule, len(simplified)) + for i := range simplified { + result[i] = simplified[i].ToRule() + } + + return result +} + func (g Generator) Generate(ctx *genall.GenerationContext) error { objs, err := GenerateRoles(ctx, g.RoleName) if err != nil { diff --git a/pkg/rbac/parser_integration_test.go b/pkg/rbac/parser_integration_test.go index a56c1ec6a..8bffb4515 100644 --- a/pkg/rbac/parser_integration_test.go +++ b/pkg/rbac/parser_integration_test.go @@ -4,7 +4,6 @@ import ( "bytes" "fmt" "io/ioutil" - "os" "github.com/google/go-cmp/cmp" . "github.com/onsi/ginkgo" @@ -21,17 +20,11 @@ import ( var _ = Describe("ClusterRole generated by the RBAC Generator", func() { // run this test multiple times to make sure the Rule order is stable. - const stableTestCount = 5 - for i := 0; i < stableTestCount; i++ { - It("should match the expected result", func() { - By("switching into testdata to appease go modules") - cwd, err := os.Getwd() - Expect(err).NotTo(HaveOccurred()) - Expect(os.Chdir("./testdata")).To(Succeed()) // go modules are directory-sensitive - defer func() { Expect(os.Chdir(cwd)).To(Succeed()) }() - + It("should match the expected result", func() { + const stableTestCount = 5 + for i := 0; i < stableTestCount; i++ { By("loading the roots") - pkgs, err := loader.LoadRoots(".") + pkgs, err := loader.LoadRoots("./testdata") Expect(err).NotTo(HaveOccurred()) By("registering RBAC rule marker") @@ -49,11 +42,11 @@ var _ = Describe("ClusterRole generated by the RBAC Generator", func() { Expect(err).NotTo(HaveOccurred()) By("loading the desired YAML") - expectedFile, err := ioutil.ReadFile("role.yaml") + expectedFile, err := ioutil.ReadFile("testdata/role.yaml") Expect(err).NotTo(HaveOccurred()) By("parsing the desired YAML") - for i, expectedRoleBytes := range bytes.Split(expectedFile, []byte("\n---\n"))[1:] { + for i, expectedRoleBytes := range bytes.Split(expectedFile, []byte("---\n"))[1:] { By(fmt.Sprintf("comparing the generated Role and expected Role (Pair %d)", i)) obj := objs[i] switch obj := obj.(type) { @@ -67,7 +60,6 @@ var _ = Describe("ClusterRole generated by the RBAC Generator", func() { Expect(obj).To(Equal(expectedRole), "type not as expected, check pkg/rbac/testdata/README.md for more details.\n\nDiff:\n\n%s", cmp.Diff(obj, expectedRole)) } } - - }) - } + } + }) }) diff --git a/pkg/rbac/parser_test.go b/pkg/rbac/parser_test.go new file mode 100644 index 000000000..50f4bcdcf --- /dev/null +++ b/pkg/rbac/parser_test.go @@ -0,0 +1,350 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package rbac + +import ( + "testing" + "testing/quick" + + "github.com/onsi/gomega" + rbacv1 "k8s.io/api/rbac/v1" + sets "k8s.io/apimachinery/pkg/util/sets" +) + +//From: https://github.com/kubernetes-sigs/controller-tools/issues/612, but altered slightly (#3 & #4 made distinct) +//- kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machinedeployments;machinedeployments/status;machinedeployments/finalizers,verbs=get;list;watch;create;update;patch;delete +//- kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machinedeployments;machinedeployments/finalizers,verbs=get;list;watch;update;patch +//- kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machinedeployments,verbs=get;list;watch +//- kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machinedeployments,verbs=update;patch +var rules = []*Rule{ + { + Groups: []string{"cluster.x-k8s.io"}, + Resources: []string{"machinedeployments", "machinedeployments/status", "machinedeployments/finalizers"}, + Verbs: []string{"get", "list", "watch", "create", "update", "patch", "delete"}, + }, + { + Groups: []string{"cluster.x-k8s.io"}, + Resources: []string{"machinedeployments", "machinedeployments/finalizers"}, + Verbs: []string{"get", "list", "watch", "update", "patch"}, + }, + { + Groups: []string{"cluster.x-k8s.io"}, + Resources: []string{"machinedeployments"}, + Verbs: []string{"get", "list", "watch"}, + }, + { + Groups: []string{"cluster.x-k8s.io"}, + Resources: []string{"machinedeployments"}, + Verbs: []string{"update", "patch"}, + }, +} + +var nrules = normalizeRules(rules) + +func normalizeRules(rules []*Rule) []*NormalizedRule { + result := make([]*NormalizedRule, len(rules)) + for ix := range rules { + result[ix] = rules[ix].Normalize() + } + + return result +} + +func Test_Subsume_Simple(t *testing.T) { + g := gomega.NewWithT(t) + // subsumes all others: + for _, nrule := range nrules { + g.Expect(nrules[0].Subsumes(nrule)).To(gomega.BeTrue()) + } + + g.Expect(nrules[1].Subsumes(nrules[2])).To(gomega.BeTrue()) + g.Expect(nrules[1].Subsumes(nrules[3])).To(gomega.BeTrue()) + + // distinct + g.Expect(nrules[3].Subsumes(nrules[2])).To(gomega.BeFalse()) + g.Expect(nrules[2].Subsumes(nrules[3])).To(gomega.BeFalse()) +} + +func Test_SubsumesIsReflexive_CannedExamples(t *testing.T) { + g := gomega.NewWithT(t) + + for _, nr := range nrules { + g.Expect(nr.Subsumes(nr)).To(gomega.BeTrue()) + } +} + +func Test_SubsumesIsReflexive_PropertyTest(t *testing.T) { + f := func(groups, verbs, resources, resourceNames []string) bool { + rule := Rule{Groups: groups, Verbs: verbs, Resources: resources, ResourceNames: resourceNames} + normed := rule.Normalize() + + return normed.Subsumes(normed) + } + + if err := quick.Check(f, nil); err != nil { + t.Error(err) + } +} + +func Test_SubsumesIsOneWay(t *testing.T) { + // note that this does not hold for any arbitrary rules + // given that Subsumes is Reflexive (as asserted by previous test) + // but it holds for all the examples we have listed above: + + g := gomega.NewWithT(t) + + for i := range nrules { + for j := range nrules { + if i == j { + continue + } + + ruleI := nrules[i] + ruleJ := nrules[j] + + if ruleI.Subsumes(ruleJ) { + g.Expect(ruleJ.Subsumes(ruleI)).To(gomega.BeFalse()) + } + } + } +} + +func Test_Simplification(t *testing.T) { + g := gomega.NewWithT(t) + + g.Expect(NormalizeRules(rules)).To(gomega.Equal([]rbacv1.PolicyRule{ + { + Verbs: []string{"create", "delete", "get", "list", "patch", "update", "watch"}, + APIGroups: []string{"cluster.x-k8s.io"}, + Resources: []string{ + "machinedeployments", + "machinedeployments/finalizers", + "machinedeployments/status", + }, + }, + })) +} + +func Test_Simplification_OrderIrrelevant(t *testing.T) { + g := gomega.NewWithT(t) + + // reversed order + rules2 := []*Rule{rules[3], rules[2], rules[1], rules[0]} + + // expect same outcome as previous test + g.Expect(NormalizeRules(rules2)).To(gomega.Equal([]rbacv1.PolicyRule{ + { + Verbs: []string{"create", "delete", "get", "list", "patch", "update", "watch"}, + APIGroups: []string{"cluster.x-k8s.io"}, + Resources: []string{ + "machinedeployments", + "machinedeployments/finalizers", + "machinedeployments/status", + }, + }, + })) +} + +func Test_Simplification_Merge(t *testing.T) { + g := gomega.NewWithT(t) + + // these two rules should merge into one + g.Expect(NormalizeRules([]*Rule{rules[2], rules[3]})).To(gomega.Equal([]rbacv1.PolicyRule{ + { + Verbs: []string{"get", "list", "patch", "update", "watch"}, + APIGroups: []string{"cluster.x-k8s.io"}, + Resources: []string{"machinedeployments"}, + }, + })) +} + +func Test_StarInGroupsDoesNotHaveMeaning(t *testing.T) { + g := gomega.NewWithT(t) + + rule1 := Rule{ + Groups: []string{"cluster.x-k8s.io"}, + Resources: []string{"machinedeployments"}, + Verbs: []string{"update", "patch"}, + } + + rule2 := rule1 + rule2.Groups = []string{"*"} + + g.Expect(rule2.Normalize().Subsumes(rule1.Normalize())).To(gomega.BeFalse()) +} + +func Test_StarInResourcesMatchesAnything(t *testing.T) { + g := gomega.NewWithT(t) + + rule1 := Rule{ + Groups: []string{"cluster.x-k8s.io"}, + Resources: []string{"machinedeployments"}, + Verbs: []string{"update", "patch"}, + } + + rule2 := rule1 + rule2.Resources = []string{"*"} + + g.Expect(rule2.Normalize().Subsumes(rule1.Normalize())).To(gomega.BeTrue()) +} + +func Test_StarInResourceNamesDoesNotHaveMeaning(t *testing.T) { + g := gomega.NewWithT(t) + + rule1 := Rule{ + Groups: []string{"cluster.x-k8s.io"}, + Resources: []string{"machinedeployments"}, + ResourceNames: []string{"resourcename"}, + Verbs: []string{"update", "patch"}, + } + + rule2 := rule1 + rule2.ResourceNames = []string{"*"} + + g.Expect(rule2.Normalize().Subsumes(rule1.Normalize())).To(gomega.BeFalse()) +} + +func Test_StarInVerbsMatchesAnything(t *testing.T) { + g := gomega.NewWithT(t) + + rule1 := Rule{ + Groups: []string{"cluster.x-k8s.io"}, + Resources: []string{"machinedeployments"}, + ResourceNames: []string{"resourcename"}, + Verbs: []string{"update", "patch"}, + } + + rule2 := rule1 + rule2.Verbs = []string{"*"} + + g.Expect(rule2.Normalize().Subsumes(rule1.Normalize())).To(gomega.BeTrue()) +} + +func Test_StarInResourcesSimplifies(t *testing.T) { + // star is special in Resources + + g := gomega.NewWithT(t) + + rule := Rule{ + Resources: []string{"some", "*", "names"}, + } + + normalized := rule.Normalize() + g.Expect(normalized.Resources).To(gomega.Equal(sets.NewString("*"))) +} + +func Test_StarInVerbsSimplifies(t *testing.T) { + // star is special in Verbs + + g := gomega.NewWithT(t) + + rule := Rule{ + Verbs: []string{"some", "*", "names"}, + } + + normalized := rule.Normalize() + g.Expect(normalized.Verbs).To(gomega.Equal(sets.NewString("*"))) +} + +func Test_StarInGroupsDoesNotSimplify(t *testing.T) { + // star is not special in Groups + + g := gomega.NewWithT(t) + + rule := Rule{ + Groups: []string{"some", "*", "things"}, + } + + normalized := rule.Normalize() + g.Expect(normalized.Groups).To(gomega.Equal(sets.NewString("some", "*", "things"))) +} + +func Test_StarInResourceNamesDoesNotSimplify(t *testing.T) { + // star is not special in Resource Names + + g := gomega.NewWithT(t) + + rule := Rule{ + ResourceNames: []string{"some", "*", "things"}, + } + + normalized := rule.Normalize() + g.Expect(normalized.ResourceNames).To(gomega.Equal(sets.NewString("some", "*", "things"))) +} + +func Test_EmptyResourceNameListMatchesAnything(t *testing.T) { + f := func(groups, verbs, resources, resourceNames []string) bool { + rule := Rule{Groups: groups, Verbs: verbs, Resources: resources, ResourceNames: resourceNames} + + biggerRule := rule + biggerRule.ResourceNames = nil + + return biggerRule.Normalize().Subsumes(rule.Normalize()) + } + + if err := quick.Check(f, nil); err != nil { + t.Error(err) + } +} + +func Test_EmptyGroupListDoesNotMatchEverything(t *testing.T) { + f := func(groups, verbs, resources, resourceNames []string) bool { + rule := Rule{Groups: groups, Verbs: verbs, Resources: resources, ResourceNames: resourceNames} + + biggerRule := rule + biggerRule.Groups = nil + + // subsumes only if the groups list was originally empty + return biggerRule.Normalize().Subsumes(rule.Normalize()) == (len(rule.Groups) == 0) + } + + if err := quick.Check(f, nil); err != nil { + t.Error(err) + } +} + +func Test_EmptyResourcesListDoesNotMatchEverything(t *testing.T) { + f := func(groups, verbs, resources, resourceNames []string) bool { + rule := Rule{Groups: groups, Verbs: verbs, Resources: resources, ResourceNames: resourceNames} + + biggerRule := rule + biggerRule.Resources = nil + + // subsumes only if the resources list was originally empty + return biggerRule.Normalize().Subsumes(rule.Normalize()) == (len(rule.Resources) == 0) + } + + if err := quick.Check(f, nil); err != nil { + t.Error(err) + } +} + +func Test_EmptyVerbsourcesListDoesNotMatchEverything(t *testing.T) { + f := func(groups, verbs, resources, resourceNames []string) bool { + rule := Rule{Groups: groups, Verbs: verbs, Resources: resources, ResourceNames: resourceNames} + + biggerRule := rule + biggerRule.Verbs = nil + + // subsumes only if the verbs list was originally empty + return biggerRule.Normalize().Subsumes(rule.Normalize()) == (len(rule.Verbs) == 0) + } + + if err := quick.Check(f, nil); err != nil { + t.Error(err) + } +} diff --git a/pkg/rbac/testdata/controller.go b/pkg/rbac/testdata/controller.go index 27800d729..602b5cd92 100644 --- a/pkg/rbac/testdata/controller.go +++ b/pkg/rbac/testdata/controller.go @@ -1,5 +1,7 @@ package controller +//go:generate ../../../.run-controller-gen.sh rbac:roleName=manager-role paths=. output:dir=. + // +kubebuilder:rbac:groups=batch.io,resources=cronjobs,verbs=get;watch;create // +kubebuilder:rbac:groups=batch.io,resources=cronjobs/status,verbs=get;update;patch // +kubebuilder:rbac:groups=art,resources=jobs,verbs=get @@ -10,4 +12,20 @@ package controller // +kubebuilder:rbac:groups=cron;batch,resources=jobs/status,verbs=get;create // +kubebuilder:rbac:groups=batch,resources=jobs/status,verbs=watch;watch // +kubebuilder:rbac:groups=art,resources=jobs,verbs=get,namespace=park -// +kubebuilder:rbac:groups=batch.io,resources=cronjobs,resourceNames=foo;bar;baz,verbs=get;watch +// +kubebuilder:rbac:groups=batch.io,resources=cronjobs2,resourceNames=foo;bar;baz,verbs=get;watch +// ensure that "core" is translated to apiGroups="": +// +kubebuilder:rbac:groups=core,resources=configmaps,resourceNames=my-configmap,verbs=update;get +// check that the following merge: +// +kubebuilder:rbac:groups=test,resources=foo,resourceNames=bar,verbs=get +// +kubebuilder:rbac:groups=test,resources=foo,resourceNames=bar,verbs=watch +// make sure that differing URLs are not merged: +// +kubebuilder:rbac:groups=test,resources=diffurl,resourceNames=same,verbs=get,urls="https://example.com/1" +// +kubebuilder:rbac:groups=test,resources=diffurl,resourceNames=same,verbs=watch,urls="https://example.com/2" +// make sure that same URLs are merged: +// +kubebuilder:rbac:groups=test,resources=sameurl,resourceNames=same,verbs=get,urls="https://example.com/x" +// +kubebuilder:rbac:groups=test,resources=sameurl,resourceNames=same,verbs=watch,urls="https://example.com/x" +// examples from: https://github.com/kubernetes-sigs/controller-tools/issues/612 +// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machinedeployments;machinedeployments/status;machinedeployments/finalizers,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machinedeployments;machinedeployments/finalizers,verbs=get;list;watch;update;patch +// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machinedeployments,verbs=get;list;watch +// +kubebuilder:rbac:groups=cluster.x-k8s.io,resources=machinedeployments,verbs=get;list;watch;create;update;patch;delete diff --git a/pkg/rbac/testdata/role.yaml b/pkg/rbac/testdata/role.yaml index 892f09d65..0471ecad2 100644 --- a/pkg/rbac/testdata/role.yaml +++ b/pkg/rbac/testdata/role.yaml @@ -1,4 +1,3 @@ - --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole @@ -6,6 +5,15 @@ metadata: creationTimestamp: null name: manager-role rules: +- apiGroups: + - "" + resourceNames: + - my-configmap + resources: + - configmaps + verbs: + - get + - update - apiGroups: - art resources: @@ -34,6 +42,14 @@ rules: - create - get - watch +- apiGroups: + - batch.io + resources: + - cronjobs/status + verbs: + - get + - patch + - update - apiGroups: - batch.io resourceNames: @@ -41,19 +57,64 @@ rules: - baz - foo resources: - - cronjobs + - cronjobs2 verbs: - get - watch - apiGroups: - - batch.io + - cluster.x-k8s.io resources: - - cronjobs/status + - machinedeployments + - machinedeployments/finalizers + - machinedeployments/status verbs: + - create + - delete - get + - list - patch - update - + - watch +- apiGroups: + - test + nonResourceURLs: + - https://example.com/1 + resourceNames: + - same + resources: + - diffurl + verbs: + - get +- apiGroups: + - test + nonResourceURLs: + - https://example.com/2 + resourceNames: + - same + resources: + - diffurl + verbs: + - watch +- apiGroups: + - test + resourceNames: + - bar + resources: + - foo + verbs: + - get + - watch +- apiGroups: + - test + nonResourceURLs: + - https://example.com/x + resourceNames: + - same + resources: + - sameurl + verbs: + - get + - watch --- apiVersion: rbac.authorization.k8s.io/v1 kind: Role @@ -68,7 +129,6 @@ rules: - jobs verbs: - get - --- apiVersion: rbac.authorization.k8s.io/v1 kind: Role