add policy_compact to rbac validation

pull/6/head
xilabao 2017-05-26 09:29:45 +08:00
parent 286bcc6f5c
commit 9fe2ef54ba
7 changed files with 393 additions and 3 deletions

View File

@ -124,6 +124,38 @@ func SubjectsStrings(subjects []Subject) ([]string, []string, []string, []string
return users, groups, sas, others
}
func (r PolicyRule) String() string {
return "PolicyRule" + r.CompactString()
}
// CompactString exposes a compact string representation for use in escalation error messages
func (r PolicyRule) CompactString() string {
formatStringParts := []string{}
formatArgs := []interface{}{}
if len(r.Resources) > 0 {
formatStringParts = append(formatStringParts, "Resources:%q")
formatArgs = append(formatArgs, r.Resources)
}
if len(r.NonResourceURLs) > 0 {
formatStringParts = append(formatStringParts, "NonResourceURLs:%q")
formatArgs = append(formatArgs, r.NonResourceURLs)
}
if len(r.ResourceNames) > 0 {
formatStringParts = append(formatStringParts, "ResourceNames:%q")
formatArgs = append(formatArgs, r.ResourceNames)
}
if len(r.APIGroups) > 0 {
formatStringParts = append(formatStringParts, "APIGroups:%q")
formatArgs = append(formatArgs, r.APIGroups)
}
if len(r.Verbs) > 0 {
formatStringParts = append(formatStringParts, "Verbs:%q")
formatArgs = append(formatArgs, r.Verbs)
}
formatString := "{" + strings.Join(formatStringParts, ", ") + "}"
return fmt.Sprintf(formatString, formatArgs...)
}
// +k8s:deepcopy-gen=false
// PolicyRuleBuilder let's us attach methods. A no-no for API types.
// We use it to construct rules in code. It's more compact than trying to write them
@ -341,3 +373,11 @@ func (r *RoleBindingBuilder) Binding() (RoleBinding, error) {
return r.RoleBinding, nil
}
type SortableRuleSlice []PolicyRule
func (s SortableRuleSlice) Len() int { return len(s) }
func (s SortableRuleSlice) Swap(i, j int) { s[i], s[j] = s[j], s[i] }
func (s SortableRuleSlice) Less(i, j int) bool {
return strings.Compare(s[i].String(), s[j].String()) < 0
}

View File

@ -85,6 +85,7 @@ go_library(
"//pkg/controller/deployment/util:go_default_library",
"//pkg/fieldpath:go_default_library",
"//pkg/printers:go_default_library",
"//pkg/registry/rbac/validation:go_default_library",
"//pkg/util/node:go_default_library",
"//pkg/util/slice:go_default_library",
"//vendor/github.com/fatih/camelcase:go_default_library",

View File

@ -11,14 +11,17 @@ load(
go_test(
name = "go_default_test",
srcs = [
"policy_compact_test.go",
"policy_comparator_test.go",
"rule_test.go",
],
library = ":go_default_library",
tags = ["automanaged"],
deps = [
"//pkg/api:go_default_library",
"//pkg/apis/rbac:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/diff:go_default_library",
"//vendor/k8s.io/apiserver/pkg/authentication/user:go_default_library",
],
@ -27,14 +30,17 @@ go_test(
go_library(
name = "go_default_library",
srcs = [
"policy_compact.go",
"policy_comparator.go",
"rule.go",
],
tags = ["automanaged"],
deps = [
"//pkg/api:go_default_library",
"//pkg/apis/rbac:go_default_library",
"//vendor/github.com/golang/glog:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/errors:go_default_library",
"//vendor/k8s.io/apiserver/pkg/authentication/serviceaccount:go_default_library",
"//vendor/k8s.io/apiserver/pkg/authentication/user:go_default_library",

View File

@ -0,0 +1,89 @@
/*
Copyright 2017 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 validation
import (
"fmt"
"reflect"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/apis/rbac"
)
// CompactRules combines rules that contain a single APIGroup/Resource, differ only by verb, and contain no other attributes.
// this is a fast check, and works well with the decomposed "missing rules" list from a Covers check.
func CompactRules(rules []rbac.PolicyRule) ([]rbac.PolicyRule, error) {
compacted := make([]rbac.PolicyRule, 0, len(rules))
simpleRules := map[schema.GroupResource]*rbac.PolicyRule{}
for _, rule := range rules {
if resource, isSimple := isSimpleResourceRule(&rule); isSimple {
if existingRule, ok := simpleRules[resource]; ok {
// Add the new verbs to the existing simple resource rule
if existingRule.Verbs == nil {
existingRule.Verbs = []string{}
}
existingRule.Verbs = append(existingRule.Verbs, rule.Verbs...)
} else {
// Copy the rule to accumulate matching simple resource rules into
objCopy, err := api.Scheme.DeepCopy(rule)
if err != nil {
// Unit tests ensure this should not ever happen
return nil, err
}
ruleCopy, ok := objCopy.(rbac.PolicyRule)
if !ok {
// Unit tests ensure this should not ever happen
return nil, fmt.Errorf("expected rbac.PolicyRule, got %#v", objCopy)
}
simpleRules[resource] = &ruleCopy
}
} else {
compacted = append(compacted, rule)
}
}
// Once we've consolidated the simple resource rules, add them to the compacted list
for _, simpleRule := range simpleRules {
compacted = append(compacted, *simpleRule)
}
return compacted, nil
}
// isSimpleResourceRule returns true if the given rule contains verbs, a single resource, a single API group, and no other values
func isSimpleResourceRule(rule *rbac.PolicyRule) (schema.GroupResource, bool) {
resource := schema.GroupResource{}
// If we have "complex" rule attributes, return early without allocations or expensive comparisons
if len(rule.ResourceNames) > 0 || len(rule.NonResourceURLs) > 0 {
return resource, false
}
// If we have multiple api groups or resources, return early
if len(rule.APIGroups) != 1 || len(rule.Resources) != 1 {
return resource, false
}
// Test if this rule only contains APIGroups/Resources/Verbs
simpleRule := &rbac.PolicyRule{APIGroups: rule.APIGroups, Resources: rule.Resources, Verbs: rule.Verbs}
if !reflect.DeepEqual(simpleRule, rule) {
return resource, false
}
resource = schema.GroupResource{Group: rule.APIGroups[0], Resource: rule.Resources[0]}
return resource, true
}

View File

@ -0,0 +1,214 @@
/*
Copyright 2017 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 validation
import (
"reflect"
"sort"
"testing"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/kubernetes/pkg/api"
"k8s.io/kubernetes/pkg/apis/rbac"
)
func TestCompactRules(t *testing.T) {
testcases := map[string]struct {
Rules []rbac.PolicyRule
Expected []rbac.PolicyRule
}{
"empty": {
Rules: []rbac.PolicyRule{},
Expected: []rbac.PolicyRule{},
},
"simple": {
Rules: []rbac.PolicyRule{
{Verbs: []string{"get"}, APIGroups: []string{""}, Resources: []string{"builds"}},
{Verbs: []string{"list"}, APIGroups: []string{""}, Resources: []string{"builds"}},
{Verbs: []string{"update", "patch"}, APIGroups: []string{""}, Resources: []string{"builds"}},
{Verbs: []string{"create"}, APIGroups: []string{"extensions"}, Resources: []string{"daemonsets"}},
{Verbs: []string{"delete"}, APIGroups: []string{"extensions"}, Resources: []string{"daemonsets"}},
{Verbs: []string{"educate"}, APIGroups: []string{""}, Resources: []string{"dolphins"}},
// nil verbs are preserved in non-merge cases.
// these are the pirates who don't do anything.
{Verbs: nil, APIGroups: []string{""}, Resources: []string{"pirates"}},
// Test merging into a nil Verbs string set
{Verbs: nil, APIGroups: []string{""}, Resources: []string{"pods"}},
{Verbs: []string{"create"}, APIGroups: []string{""}, Resources: []string{"pods"}},
},
Expected: []rbac.PolicyRule{
{Verbs: []string{"create", "delete"}, APIGroups: []string{"extensions"}, Resources: []string{"daemonsets"}},
{Verbs: []string{"get", "list", "update", "patch"}, APIGroups: []string{""}, Resources: []string{"builds"}},
{Verbs: []string{"educate"}, APIGroups: []string{""}, Resources: []string{"dolphins"}},
{Verbs: nil, APIGroups: []string{""}, Resources: []string{"pirates"}},
{Verbs: []string{"create"}, APIGroups: []string{""}, Resources: []string{"pods"}},
},
},
"complex multi-group": {
Rules: []rbac.PolicyRule{
{Verbs: []string{"get"}, APIGroups: []string{"", "builds.openshift.io"}, Resources: []string{"builds"}},
{Verbs: []string{"list"}, APIGroups: []string{"", "builds.openshift.io"}, Resources: []string{"builds"}},
},
Expected: []rbac.PolicyRule{
{Verbs: []string{"get"}, APIGroups: []string{"", "builds.openshift.io"}, Resources: []string{"builds"}},
{Verbs: []string{"list"}, APIGroups: []string{"", "builds.openshift.io"}, Resources: []string{"builds"}},
},
},
"complex multi-resource": {
Rules: []rbac.PolicyRule{
{Verbs: []string{"get"}, APIGroups: []string{""}, Resources: []string{"builds", "images"}},
{Verbs: []string{"list"}, APIGroups: []string{""}, Resources: []string{"builds", "images"}},
},
Expected: []rbac.PolicyRule{
{Verbs: []string{"get"}, APIGroups: []string{""}, Resources: []string{"builds", "images"}},
{Verbs: []string{"list"}, APIGroups: []string{""}, Resources: []string{"builds", "images"}},
},
},
"complex named-resource": {
Rules: []rbac.PolicyRule{
{Verbs: []string{"get"}, APIGroups: []string{""}, Resources: []string{"builds"}, ResourceNames: []string{"mybuild"}},
{Verbs: []string{"list"}, APIGroups: []string{""}, Resources: []string{"builds"}, ResourceNames: []string{"mybuild2"}},
},
Expected: []rbac.PolicyRule{
{Verbs: []string{"get"}, APIGroups: []string{""}, Resources: []string{"builds"}, ResourceNames: []string{"mybuild"}},
{Verbs: []string{"list"}, APIGroups: []string{""}, Resources: []string{"builds"}, ResourceNames: []string{"mybuild2"}},
},
},
"complex non-resource": {
Rules: []rbac.PolicyRule{
{Verbs: []string{"get"}, APIGroups: []string{""}, Resources: []string{"builds"}, NonResourceURLs: []string{"/"}},
{Verbs: []string{"get"}, APIGroups: []string{""}, Resources: []string{"builds"}, NonResourceURLs: []string{"/foo"}},
},
Expected: []rbac.PolicyRule{
{Verbs: []string{"get"}, APIGroups: []string{""}, Resources: []string{"builds"}, NonResourceURLs: []string{"/"}},
{Verbs: []string{"get"}, APIGroups: []string{""}, Resources: []string{"builds"}, NonResourceURLs: []string{"/foo"}},
},
},
}
for k, tc := range testcases {
rules := tc.Rules
originalRules, err := api.Scheme.DeepCopy(tc.Rules)
if err != nil {
t.Errorf("%s: couldn't copy rules: %v", k, err)
continue
}
compacted, err := CompactRules(tc.Rules)
if err != nil {
t.Errorf("%s: unexpected error: %v", k, err)
continue
}
if !reflect.DeepEqual(rules, originalRules) {
t.Errorf("%s: CompactRules mutated rules. Expected\n%#v\ngot\n%#v", k, originalRules, rules)
continue
}
if covers, missing := Covers(compacted, rules); !covers {
t.Errorf("%s: compacted rules did not cover original rules. missing: %#v", k, missing)
continue
}
if covers, missing := Covers(rules, compacted); !covers {
t.Errorf("%s: original rules did not cover compacted rules. missing: %#v", k, missing)
continue
}
sort.Stable(rbac.SortableRuleSlice(compacted))
sort.Stable(rbac.SortableRuleSlice(tc.Expected))
if !reflect.DeepEqual(compacted, tc.Expected) {
t.Errorf("%s: Expected\n%#v\ngot\n%#v", k, tc.Expected, compacted)
continue
}
}
}
func TestIsSimpleResourceRule(t *testing.T) {
testcases := map[string]struct {
Rule rbac.PolicyRule
Simple bool
Resource schema.GroupResource
}{
"simple, no verbs": {
Rule: rbac.PolicyRule{Verbs: []string{}, APIGroups: []string{""}, Resources: []string{"builds"}},
Simple: true,
Resource: schema.GroupResource{Group: "", Resource: "builds"},
},
"simple, one verb": {
Rule: rbac.PolicyRule{Verbs: []string{"get"}, APIGroups: []string{""}, Resources: []string{"builds"}},
Simple: true,
Resource: schema.GroupResource{Group: "", Resource: "builds"},
},
"simple, multi verb": {
Rule: rbac.PolicyRule{Verbs: []string{"get", "list"}, APIGroups: []string{""}, Resources: []string{"builds"}},
Simple: true,
Resource: schema.GroupResource{Group: "", Resource: "builds"},
},
"complex, empty": {
Rule: rbac.PolicyRule{},
Simple: false,
Resource: schema.GroupResource{},
},
"complex, no group": {
Rule: rbac.PolicyRule{Verbs: []string{"get"}, APIGroups: []string{}, Resources: []string{"builds"}},
Simple: false,
Resource: schema.GroupResource{},
},
"complex, multi group": {
Rule: rbac.PolicyRule{Verbs: []string{"get"}, APIGroups: []string{"a", "b"}, Resources: []string{"builds"}},
Simple: false,
Resource: schema.GroupResource{},
},
"complex, no resource": {
Rule: rbac.PolicyRule{Verbs: []string{"get"}, APIGroups: []string{""}, Resources: []string{}},
Simple: false,
Resource: schema.GroupResource{},
},
"complex, multi resource": {
Rule: rbac.PolicyRule{Verbs: []string{"get"}, APIGroups: []string{""}, Resources: []string{"builds", "images"}},
Simple: false,
Resource: schema.GroupResource{},
},
"complex, resource names": {
Rule: rbac.PolicyRule{Verbs: []string{"get"}, APIGroups: []string{""}, Resources: []string{"builds"}, ResourceNames: []string{"foo"}},
Simple: false,
Resource: schema.GroupResource{},
},
"complex, non-resource urls": {
Rule: rbac.PolicyRule{Verbs: []string{"get"}, APIGroups: []string{""}, Resources: []string{"builds"}, NonResourceURLs: []string{"/"}},
Simple: false,
Resource: schema.GroupResource{},
},
}
for k, tc := range testcases {
resource, simple := isSimpleResourceRule(&tc.Rule)
if simple != tc.Simple {
t.Errorf("%s: expected simple=%v, got simple=%v", k, tc.Simple, simple)
continue
}
if resource != tc.Resource {
t.Errorf("%s: expected resource=%v, got resource=%v", k, tc.Resource, resource)
continue
}
}
}

View File

@ -32,7 +32,7 @@ func Covers(ownerRules, servantRules []rbac.PolicyRule) (bool, []rbac.PolicyRule
subrules := []rbac.PolicyRule{}
for _, servantRule := range servantRules {
subrules = append(subrules, breakdownRule(servantRule)...)
subrules = append(subrules, BreakdownRule(servantRule)...)
}
uncoveredRules := []rbac.PolicyRule{}
@ -53,9 +53,9 @@ func Covers(ownerRules, servantRules []rbac.PolicyRule) (bool, []rbac.PolicyRule
return (len(uncoveredRules) == 0), uncoveredRules
}
// breadownRule takes a rule and builds an equivalent list of rules that each have at most one verb, one
// BreadownRule takes a rule and builds an equivalent list of rules that each have at most one verb, one
// resource, and one resource name
func breakdownRule(rule rbac.PolicyRule) []rbac.PolicyRule {
func BreakdownRule(rule rbac.PolicyRule) []rbac.PolicyRule {
subrules := []rbac.PolicyRule{}
for _, group := range rule.APIGroups {
for _, resource := range rule.Resources {

View File

@ -124,6 +124,38 @@ func SubjectsStrings(subjects []Subject) ([]string, []string, []string, []string
return users, groups, sas, others
}
func (r PolicyRule) String() string {
return "PolicyRule" + r.CompactString()
}
// CompactString exposes a compact string representation for use in escalation error messages
func (r PolicyRule) CompactString() string {
formatStringParts := []string{}
formatArgs := []interface{}{}
if len(r.Resources) > 0 {
formatStringParts = append(formatStringParts, "Resources:%q")
formatArgs = append(formatArgs, r.Resources)
}
if len(r.NonResourceURLs) > 0 {
formatStringParts = append(formatStringParts, "NonResourceURLs:%q")
formatArgs = append(formatArgs, r.NonResourceURLs)
}
if len(r.ResourceNames) > 0 {
formatStringParts = append(formatStringParts, "ResourceNames:%q")
formatArgs = append(formatArgs, r.ResourceNames)
}
if len(r.APIGroups) > 0 {
formatStringParts = append(formatStringParts, "APIGroups:%q")
formatArgs = append(formatArgs, r.APIGroups)
}
if len(r.Verbs) > 0 {
formatStringParts = append(formatStringParts, "Verbs:%q")
formatArgs = append(formatArgs, r.Verbs)
}
formatString := "{" + strings.Join(formatStringParts, ", ") + "}"
return fmt.Sprintf(formatString, formatArgs...)
}
// PolicyRuleBuilder let's us attach methods. A no-no for API types.
// We use it to construct rules in code. It's more compact than trying to write them
// out in a literal and allows us to perform some basic checking during construction
@ -338,3 +370,11 @@ func (r *RoleBindingBuilder) Binding() (RoleBinding, error) {
return r.RoleBinding, nil
}
type SortableRuleSlice []PolicyRule
func (s SortableRuleSlice) Len() int { return len(s) }
func (s SortableRuleSlice) Swap(i, j int) { s[i], s[j] = s[j], s[i] }
func (s SortableRuleSlice) Less(i, j int) bool {
return strings.Compare(s[i].String(), s[j].String()) < 0
}