Skip to content

Commit

Permalink
UPSTREAM: 75264: Optimize authorization service account check
Browse files Browse the repository at this point in the history
Reduces total allocations about 5% in profiles, see kubernetes#75264

Origin-commit: 8ca46a1eddf10f366bd0f10b66a531f44d55ad79
  • Loading branch information
smarterclayton authored and k8s-publishing-bot committed Mar 11, 2019
1 parent f3ec90e commit 09c604d
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 2 deletions.
3 changes: 2 additions & 1 deletion pkg/registry/rbac/validation/rule.go
Expand Up @@ -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
}
Expand Down
Expand Up @@ -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 = strings.TrimPrefix(username, ServiceAccountUsernamePrefix)

if !strings.HasSuffix(username, name) {
return false
}
username = strings.TrimSuffix(username, name)

if !strings.HasPrefix(username, namespace) {
return false
}
username = strings.TrimPrefix(username, namespace)

return username == ServiceAccountUsernameSeparator
}

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,
Expand Down
Expand Up @@ -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)
Expand All @@ -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")
}
})
}
}

0 comments on commit 09c604d

Please sign in to comment.