Merge pull request #75264 from smarterclayton/optimize_rbac_visit

Avoid allocating when performing VisitRulesFor on service accounts

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
 	}

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 = 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,

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")
+			}
+		})
+	}
+}