Avoid allocating when performing VisitRulesFor on service accounts

Service account authorization checks are done frequently and were
observed to perform 7% of allocations on a system running e2e tests.
The allocation comes from when we walk the authorization rules to
find matching service accounts.

Optimize the check for service account names to avoid allocating.
This commit is contained in:
Clayton Coleman
2019-03-11 11:50:44 -04:00
parent 6ec5a7d337
commit 4c87a14e6b
3 changed files with 62 additions and 2 deletions

View File

@@ -286,7 +286,8 @@ func appliesToUser(user user.Info, subject rbacv1.Subject, namespace string) boo
if len(saNamespace) == 0 {
return false
}
return serviceaccount.MakeUsername(saNamespace, subject.Name) == user.GetName()
// use a more efficient comparison for RBAC checking
return serviceaccount.MatchesUsername(saNamespace, subject.Name, user.GetName())
default:
return false
}

View File

@@ -36,6 +36,27 @@ func MakeUsername(namespace, name string) string {
return ServiceAccountUsernamePrefix + namespace + ServiceAccountUsernameSeparator + name
}
// MatchesUsername checks whether the provided username matches the namespace and name without
// allocating. Use this when checking a service account namespace and name against a known string.
func MatchesUsername(namespace, name string, username string) bool {
if !strings.HasPrefix(username, ServiceAccountUsernamePrefix) {
return false
}
username = username[len(ServiceAccountUsernamePrefix):]
if !strings.HasPrefix(username, namespace) {
return false
}
username = username[len(namespace):]
if !strings.HasPrefix(username, ServiceAccountUsernameSeparator) {
return false
}
username = username[len(ServiceAccountUsernameSeparator):]
return username == name
}
var invalidUsernameErr = fmt.Errorf("Username must be in the form %s", MakeUsername("namespace", "name"))
// SplitUsername returns the namespace and ServiceAccount name embedded in the given username,

View File

@@ -62,7 +62,9 @@ func TestMakeUsername(t *testing.T) {
for k, tc := range testCases {
username := MakeUsername(tc.Namespace, tc.Name)
if !MatchesUsername(tc.Namespace, tc.Name, username) {
t.Errorf("%s: Expected to match username", k)
}
namespace, name, err := SplitUsername(username)
if (err != nil) != tc.ExpectedErr {
t.Errorf("%s: Expected error=%v, got %v", k, tc.ExpectedErr, err)
@@ -80,3 +82,39 @@ func TestMakeUsername(t *testing.T) {
}
}
}
func TestMatchUsername(t *testing.T) {
testCases := []struct {
TestName string
Namespace string
Name string
Username string
Expect bool
}{
{Namespace: "foo", Name: "bar", Username: "foo", Expect: false},
{Namespace: "foo", Name: "bar", Username: "system:serviceaccount:", Expect: false},
{Namespace: "foo", Name: "bar", Username: "system:serviceaccount:foo", Expect: false},
{Namespace: "foo", Name: "bar", Username: "system:serviceaccount:foo:", Expect: false},
{Namespace: "foo", Name: "bar", Username: "system:serviceaccount:foo:bar", Expect: true},
{Namespace: "foo", Name: "bar", Username: "system:serviceaccount::bar", Expect: false},
{Namespace: "foo", Name: "bar", Username: "system:serviceaccount:bar", Expect: false},
{Namespace: "foo", Name: "bar", Username: ":bar", Expect: false},
{Namespace: "foo", Name: "bar", Username: "foo:bar", Expect: false},
{Namespace: "foo", Name: "bar", Username: "", Expect: false},
{Namespace: "foo2", Name: "bar", Username: "system:serviceaccount:foo:bar", Expect: false},
{Namespace: "foo", Name: "bar2", Username: "system:serviceaccount:foo:bar", Expect: false},
{Namespace: "foo:", Name: "bar", Username: "system:serviceaccount:foo:bar", Expect: false},
{Namespace: "foo", Name: ":bar", Username: "system:serviceaccount:foo:bar", Expect: false},
}
for _, tc := range testCases {
t.Run(tc.TestName, func(t *testing.T) {
actual := MatchesUsername(tc.Namespace, tc.Name, tc.Username)
if actual != tc.Expect {
t.Fatalf("unexpected match")
}
})
}
}