Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid allocating when performing VisitRulesFor on service accounts #75264

Merged
merged 1 commit into from Mar 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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) {
smarterclayton marked this conversation as resolved.
Show resolved Hide resolved
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,
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")
}
})
}
}