From 09c604de63251a4e0cfbba48666a9bc3521281d0 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Mon, 11 Mar 2019 11:55:44 -0400 Subject: [PATCH] UPSTREAM: 75264: Optimize authorization service account check Reduces total allocations about 5% in profiles, see https://github.com/kubernetes/kubernetes/pull/75264 Origin-commit: 8ca46a1eddf10f366bd0f10b66a531f44d55ad79 --- pkg/registry/rbac/validation/rule.go | 3 +- .../pkg/authentication/serviceaccount/util.go | 21 ++++++++++ .../serviceaccount/util_test.go | 40 ++++++++++++++++++- 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/pkg/registry/rbac/validation/rule.go b/pkg/registry/rbac/validation/rule.go index 833ffc1e6c1e..9a295e50fda4 100644 --- a/pkg/registry/rbac/validation/rule.go +++ b/pkg/registry/rbac/validation/rule.go @@ -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 } diff --git a/staging/src/k8s.io/apiserver/pkg/authentication/serviceaccount/util.go b/staging/src/k8s.io/apiserver/pkg/authentication/serviceaccount/util.go index 1b7bbc1390d0..f773b384afbc 100644 --- a/staging/src/k8s.io/apiserver/pkg/authentication/serviceaccount/util.go +++ b/staging/src/k8s.io/apiserver/pkg/authentication/serviceaccount/util.go @@ -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, diff --git a/staging/src/k8s.io/apiserver/pkg/authentication/serviceaccount/util_test.go b/staging/src/k8s.io/apiserver/pkg/authentication/serviceaccount/util_test.go index 14784b16cdd3..91c8cb0ddd88 100644 --- a/staging/src/k8s.io/apiserver/pkg/authentication/serviceaccount/util_test.go +++ b/staging/src/k8s.io/apiserver/pkg/authentication/serviceaccount/util_test.go @@ -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") + } + }) + } +}