Merge pull request #13163 from serathius/authusers

etcdserver: Move all get/put/delete on AuthUsers and AuthRoles to buckets module

server/auth/range_perm_cache.go

@@ -18,12 +18,13 @@ import (
 	"go.etcd.io/etcd/api/v3/authpb"
 	"go.etcd.io/etcd/pkg/v3/adt"
 	"go.etcd.io/etcd/server/v3/mvcc/backend"
+	"go.etcd.io/etcd/server/v3/mvcc/buckets"
 
 	"go.uber.org/zap"
 )
 
 func getMergedPerms(lg *zap.Logger, tx backend.BatchTx, userName string) *unifiedRangePermissions {
-	user := getUser(lg, tx, userName)
+	user := buckets.UnsafeGetUser(lg, tx, userName)
 	if user == nil {
 		return nil
 	}
@@ -32,7 +33,7 @@ func getMergedPerms(lg *zap.Logger, tx backend.BatchTx, userName string) *unifie
 	writePerms := adt.NewIntervalTree()
 
 	for _, roleName := range user.Roles {
-		role := getRole(lg, tx, roleName)
+		role := buckets.UnsafeGetRole(lg, tx, roleName)
 		if role == nil {
 			continue
 		}

server/auth/store.go

@@ -228,7 +228,7 @@ func (as *authStore) AuthEnable() error {
 		b.ForceCommit()
 	}()
 
-	u := getUser(as.lg, tx, rootUser)
+	u := buckets.UnsafeGetUser(as.lg, tx, rootUser)
 	if u == nil {
 		return ErrRootUserNotExist
 	}
@@ -289,7 +289,7 @@ func (as *authStore) Authenticate(ctx context.Context, username, password string
 	tx.Lock()
 	defer tx.Unlock()
 
-	user := getUser(as.lg, tx, username)
+	user := buckets.UnsafeGetUser(as.lg, tx, username)
 	if user == nil {
 		return nil, ErrAuthFailed
 	}
@@ -327,7 +327,7 @@ func (as *authStore) CheckPassword(username, password string) (uint64, error) {
 		tx.Lock()
 		defer tx.Unlock()
 
-		user = getUser(as.lg, tx, username)
+		user = buckets.UnsafeGetUser(as.lg, tx, username)
 		if user == nil {
 			return 0, ErrAuthFailed
 		}
@@ -387,7 +387,7 @@ func (as *authStore) UserAdd(r *pb.AuthUserAddRequest) (*pb.AuthUserAddResponse,
 	tx.Lock()
 	defer tx.Unlock()
 
-	user := getUser(as.lg, tx, r.Name)
+	user := buckets.UnsafeGetUser(as.lg, tx, r.Name)
 	if user != nil {
 		return nil, ErrUserAlreadyExist
 	}
@@ -415,7 +415,7 @@ func (as *authStore) UserAdd(r *pb.AuthUserAddRequest) (*pb.AuthUserAddResponse,
 		Options:  options,
 	}
 
-	putUser(as.lg, tx, newUser)
+	buckets.UnsafePutUser(as.lg, tx, newUser)
 
 	as.commitRevision(tx)
 
@@ -433,12 +433,12 @@ func (as *authStore) UserDelete(r *pb.AuthUserDeleteRequest) (*pb.AuthUserDelete
 	tx.Lock()
 	defer tx.Unlock()
 
-	user := getUser(as.lg, tx, r.Name)
+	user := buckets.UnsafeGetUser(as.lg, tx, r.Name)
 	if user == nil {
 		return nil, ErrUserNotFound
 	}
 
-	delUser(tx, r.Name)
+	buckets.UnsafeDeleteUser(tx, r.Name)
 
 	as.commitRevision(tx)
 
@@ -458,7 +458,7 @@ func (as *authStore) UserChangePassword(r *pb.AuthUserChangePasswordRequest) (*p
 	tx.Lock()
 	defer tx.Unlock()
 
-	user := getUser(as.lg, tx, r.Name)
+	user := buckets.UnsafeGetUser(as.lg, tx, r.Name)
 	if user == nil {
 		return nil, ErrUserNotFound
 	}
@@ -480,7 +480,7 @@ func (as *authStore) UserChangePassword(r *pb.AuthUserChangePasswordRequest) (*p
 		Options:  user.Options,
 	}
 
-	putUser(as.lg, tx, updatedUser)
+	buckets.UnsafePutUser(as.lg, tx, updatedUser)
 
 	as.commitRevision(tx)
 
@@ -500,13 +500,13 @@ func (as *authStore) UserGrantRole(r *pb.AuthUserGrantRoleRequest) (*pb.AuthUser
 	tx.Lock()
 	defer tx.Unlock()
 
-	user := getUser(as.lg, tx, r.User)
+	user := buckets.UnsafeGetUser(as.lg, tx, r.User)
 	if user == nil {
 		return nil, ErrUserNotFound
 	}
 
 	if r.Role != rootRole {
-		role := getRole(as.lg, tx, r.Role)
+		role := buckets.UnsafeGetRole(as.lg, tx, r.Role)
 		if role == nil {
 			return nil, ErrRoleNotFound
 		}
@@ -526,7 +526,7 @@ func (as *authStore) UserGrantRole(r *pb.AuthUserGrantRoleRequest) (*pb.AuthUser
 	user.Roles = append(user.Roles, r.Role)
 	sort.Strings(user.Roles)
 
-	putUser(as.lg, tx, user)
+	buckets.UnsafePutUser(as.lg, tx, user)
 
 	as.invalidateCachedPerm(r.User)
 
@@ -544,7 +544,7 @@ func (as *authStore) UserGrantRole(r *pb.AuthUserGrantRoleRequest) (*pb.AuthUser
 func (as *authStore) UserGet(r *pb.AuthUserGetRequest) (*pb.AuthUserGetResponse, error) {
 	tx := as.be.BatchTx()
 	tx.Lock()
-	user := getUser(as.lg, tx, r.Name)
+	user := buckets.UnsafeGetUser(as.lg, tx, r.Name)
 	tx.Unlock()
 
 	if user == nil {
@@ -559,7 +559,7 @@ func (as *authStore) UserGet(r *pb.AuthUserGetRequest) (*pb.AuthUserGetResponse,
 func (as *authStore) UserList(r *pb.AuthUserListRequest) (*pb.AuthUserListResponse, error) {
 	tx := as.be.BatchTx()
 	tx.Lock()
-	users := getAllUsers(as.lg, tx)
+	users := buckets.UnsafeGetAllUsers(as.lg, tx)
 	tx.Unlock()
 
 	resp := &pb.AuthUserListResponse{Users: make([]string, len(users))}
@@ -583,7 +583,7 @@ func (as *authStore) UserRevokeRole(r *pb.AuthUserRevokeRoleRequest) (*pb.AuthUs
 	tx.Lock()
 	defer tx.Unlock()
 
-	user := getUser(as.lg, tx, r.Name)
+	user := buckets.UnsafeGetUser(as.lg, tx, r.Name)
 	if user == nil {
 		return nil, ErrUserNotFound
 	}
@@ -604,7 +604,7 @@ func (as *authStore) UserRevokeRole(r *pb.AuthUserRevokeRoleRequest) (*pb.AuthUs
 		return nil, ErrRoleNotGranted
 	}
 
-	putUser(as.lg, tx, updatedUser)
+	buckets.UnsafePutUser(as.lg, tx, updatedUser)
 
 	as.invalidateCachedPerm(r.Name)
 
@@ -627,7 +627,7 @@ func (as *authStore) RoleGet(r *pb.AuthRoleGetRequest) (*pb.AuthRoleGetResponse,
 
 	var resp pb.AuthRoleGetResponse
 
-	role := getRole(as.lg, tx, r.Role)
+	role := buckets.UnsafeGetRole(as.lg, tx, r.Role)
 	if role == nil {
 		return nil, ErrRoleNotFound
 	}
@@ -642,7 +642,7 @@ func (as *authStore) RoleGet(r *pb.AuthRoleGetRequest) (*pb.AuthRoleGetResponse,
 func (as *authStore) RoleList(r *pb.AuthRoleListRequest) (*pb.AuthRoleListResponse, error) {
 	tx := as.be.BatchTx()
 	tx.Lock()
-	roles := getAllRoles(as.lg, tx)
+	roles := buckets.UnsafeGetAllRoles(as.lg, tx)
 	tx.Unlock()
 
 	resp := &pb.AuthRoleListResponse{Roles: make([]string, len(roles))}
@@ -657,7 +657,7 @@ func (as *authStore) RoleRevokePermission(r *pb.AuthRoleRevokePermissionRequest)
 	tx.Lock()
 	defer tx.Unlock()
 
-	role := getRole(as.lg, tx, r.Role)
+	role := buckets.UnsafeGetRole(as.lg, tx, r.Role)
 	if role == nil {
 		return nil, ErrRoleNotFound
 	}
@@ -676,7 +676,7 @@ func (as *authStore) RoleRevokePermission(r *pb.AuthRoleRevokePermissionRequest)
 		return nil, ErrPermissionNotGranted
 	}
 
-	putRole(as.lg, tx, updatedRole)
+	buckets.UnsafePutRole(as.lg, tx, updatedRole)
 
 	// TODO(mitake): currently single role update invalidates every cache
 	// It should be optimized.
@@ -703,14 +703,14 @@ func (as *authStore) RoleDelete(r *pb.AuthRoleDeleteRequest) (*pb.AuthRoleDelete
 	tx.Lock()
 	defer tx.Unlock()
 
-	role := getRole(as.lg, tx, r.Role)
+	role := buckets.UnsafeGetRole(as.lg, tx, r.Role)
 	if role == nil {
 		return nil, ErrRoleNotFound
 	}
 
-	delRole(tx, r.Role)
+	buckets.UnsafeDeleteRole(tx, r.Role)
 
-	users := getAllUsers(as.lg, tx)
+	users := buckets.UnsafeGetAllUsers(as.lg, tx)
 	for _, user := range users {
 		updatedUser := &authpb.User{
 			Name:     user.Name,
@@ -728,7 +728,7 @@ func (as *authStore) RoleDelete(r *pb.AuthRoleDeleteRequest) (*pb.AuthRoleDelete
 			continue
 		}
 
-		putUser(as.lg, tx, updatedUser)
+		buckets.UnsafePutUser(as.lg, tx, updatedUser)
 
 		as.invalidateCachedPerm(string(user.Name))
 	}
@@ -748,7 +748,7 @@ func (as *authStore) RoleAdd(r *pb.AuthRoleAddRequest) (*pb.AuthRoleAddResponse,
 	tx.Lock()
 	defer tx.Unlock()
 
-	role := getRole(as.lg, tx, r.Name)
+	role := buckets.UnsafeGetRole(as.lg, tx, r.Name)
 	if role != nil {
 		return nil, ErrRoleAlreadyExist
 	}
@@ -757,7 +757,7 @@ func (as *authStore) RoleAdd(r *pb.AuthRoleAddRequest) (*pb.AuthRoleAddResponse,
 		Name: []byte(r.Name),
 	}
 
-	putRole(as.lg, tx, newRole)
+	buckets.UnsafePutRole(as.lg, tx, newRole)
 
 	as.commitRevision(tx)
 
@@ -792,7 +792,7 @@ func (as *authStore) RoleGrantPermission(r *pb.AuthRoleGrantPermissionRequest) (
 	tx.Lock()
 	defer tx.Unlock()
 
-	role := getRole(as.lg, tx, r.Name)
+	role := buckets.UnsafeGetRole(as.lg, tx, r.Name)
 	if role == nil {
 		return nil, ErrRoleNotFound
 	}
@@ -816,7 +816,7 @@ func (as *authStore) RoleGrantPermission(r *pb.AuthRoleGrantPermissionRequest) (
 		sort.Sort(permSlice(role.KeyPermission))
 	}
 
-	putRole(as.lg, tx, role)
+	buckets.UnsafePutRole(as.lg, tx, role)
 
 	// TODO(mitake): currently single role update invalidates every cache
 	// It should be optimized.
@@ -856,7 +856,7 @@ func (as *authStore) isOpPermitted(userName string, revision uint64, key, rangeE
 	tx.Lock()
 	defer tx.Unlock()
 
-	user := getUser(as.lg, tx, userName)
+	user := buckets.UnsafeGetUser(as.lg, tx, userName)
 	if user == nil {
 		as.lg.Error("cannot find a user for permission check", zap.String("user-name", userName))
 		return ErrPermissionDenied
@@ -896,7 +896,7 @@ func (as *authStore) IsAdminPermitted(authInfo *AuthInfo) error {
 
 	tx := as.be.BatchTx()
 	tx.Lock()
-	u := getUser(as.lg, tx, authInfo.Username)
+	u := buckets.UnsafeGetUser(as.lg, tx, authInfo.Username)
 	tx.Unlock()
 
 	if u == nil {
@@ -910,103 +910,6 @@ func (as *authStore) IsAdminPermitted(authInfo *AuthInfo) error {
 	return nil
 }
 
-func getUser(lg *zap.Logger, tx backend.BatchTx, username string) *authpb.User {
-	_, vs := tx.UnsafeRange(buckets.AuthUsers, []byte(username), nil, 0)
-	if len(vs) == 0 {
-		return nil
-	}
-
-	user := &authpb.User{}
-	err := user.Unmarshal(vs[0])
-	if err != nil {
-		lg.Panic(
-			"failed to unmarshal 'authpb.User'",
-			zap.String("user-name", username),
-			zap.Error(err),
-		)
-	}
-	return user
-}
-
-func getAllUsers(lg *zap.Logger, tx backend.BatchTx) []*authpb.User {
-	_, vs := tx.UnsafeRange(buckets.AuthUsers, []byte{0}, []byte{0xff}, -1)
-	if len(vs) == 0 {
-		return nil
-	}
-
-	users := make([]*authpb.User, len(vs))
-	for i := range vs {
-		user := &authpb.User{}
-		err := user.Unmarshal(vs[i])
-		if err != nil {
-			lg.Panic("failed to unmarshal 'authpb.User'", zap.Error(err))
-		}
-		users[i] = user
-	}
-	return users
-}
-
-func putUser(lg *zap.Logger, tx backend.BatchTx, user *authpb.User) {
-	b, err := user.Marshal()
-	if err != nil {
-		lg.Panic("failed to unmarshal 'authpb.User'", zap.Error(err))
-	}
-	tx.UnsafePut(buckets.AuthUsers, user.Name, b)
-}
-
-func delUser(tx backend.BatchTx, username string) {
-	tx.UnsafeDelete(buckets.AuthUsers, []byte(username))
-}
-
-func getRole(lg *zap.Logger, tx backend.BatchTx, roleName string) *authpb.Role {
-	_, vs := tx.UnsafeRange(buckets.AuthRoles, []byte(roleName), nil, 0)
-	if len(vs) == 0 {
-		return nil
-	}
-
-	role := &authpb.Role{}
-	err := role.Unmarshal(vs[0])
-	if err != nil {
-		lg.Panic("failed to unmarshal 'authpb.Role'", zap.Error(err))
-	}
-	return role
-}
-
-func getAllRoles(lg *zap.Logger, tx backend.BatchTx) []*authpb.Role {
-	_, vs := tx.UnsafeRange(buckets.AuthRoles, []byte{0}, []byte{0xff}, -1)
-	if len(vs) == 0 {
-		return nil
-	}
-
-	roles := make([]*authpb.Role, len(vs))
-	for i := range vs {
-		role := &authpb.Role{}
-		err := role.Unmarshal(vs[i])
-		if err != nil {
-			lg.Panic("failed to unmarshal 'authpb.Role'", zap.Error(err))
-		}
-		roles[i] = role
-	}
-	return roles
-}
-
-func putRole(lg *zap.Logger, tx backend.BatchTx, role *authpb.Role) {
-	b, err := role.Marshal()
-	if err != nil {
-		lg.Panic(
-			"failed to marshal 'authpb.Role'",
-			zap.String("role-name", string(role.Name)),
-			zap.Error(err),
-		)
-	}
-
-	tx.UnsafePut(buckets.AuthRoles, role.Name, b)
-}
-
-func delRole(tx backend.BatchTx, rolename string) {
-	tx.UnsafeDelete(buckets.AuthRoles, []byte(rolename))
-}
-
 func (as *authStore) IsAuthEnabled() bool {
 	as.enabledMu.RLock()
 	defer as.enabledMu.RUnlock()
@@ -1285,7 +1188,7 @@ func (as *authStore) WithRoot(ctx context.Context) context.Context {
 func (as *authStore) HasRole(user, role string) bool {
 	tx := as.be.BatchTx()
 	tx.Lock()
-	u := getUser(as.lg, tx, user)
+	u := buckets.UnsafeGetUser(as.lg, tx, user)
 	tx.Unlock()
 
 	if u == nil {

server/mvcc/buckets/auth_roles.go

@@ -0,0 +1,70 @@
+// Copyright 2021 The etcd Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package buckets
+
+import (
+	"go.etcd.io/etcd/api/v3/authpb"
+	"go.etcd.io/etcd/server/v3/mvcc/backend"
+	"go.uber.org/zap"
+)
+
+func UnsafeGetRole(lg *zap.Logger, tx backend.BatchTx, roleName string) *authpb.Role {
+	_, vs := tx.UnsafeRange(AuthRoles, []byte(roleName), nil, 0)
+	if len(vs) == 0 {
+		return nil
+	}
+
+	role := &authpb.Role{}
+	err := role.Unmarshal(vs[0])
+	if err != nil {
+		lg.Panic("failed to unmarshal 'authpb.Role'", zap.Error(err))
+	}
+	return role
+}
+
+func UnsafeGetAllRoles(lg *zap.Logger, tx backend.BatchTx) []*authpb.Role {
+	_, vs := tx.UnsafeRange(AuthRoles, []byte{0}, []byte{0xff}, -1)
+	if len(vs) == 0 {
+		return nil
+	}
+
+	roles := make([]*authpb.Role, len(vs))
+	for i := range vs {
+		role := &authpb.Role{}
+		err := role.Unmarshal(vs[i])
+		if err != nil {
+			lg.Panic("failed to unmarshal 'authpb.Role'", zap.Error(err))
+		}
+		roles[i] = role
+	}
+	return roles
+}
+
+func UnsafePutRole(lg *zap.Logger, tx backend.BatchTx, role *authpb.Role) {
+	b, err := role.Marshal()
+	if err != nil {
+		lg.Panic(
+			"failed to marshal 'authpb.Role'",
+			zap.String("role-name", string(role.Name)),
+			zap.Error(err),
+		)
+	}
+
+	tx.UnsafePut(AuthRoles, role.Name, b)
+}
+
+func UnsafeDeleteRole(tx backend.BatchTx, rolename string) {
+	tx.UnsafeDelete(AuthRoles, []byte(rolename))
+}

server/mvcc/buckets/auth_users.go

@@ -0,0 +1,69 @@
+// Copyright 2021 The etcd Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package buckets
+
+import (
+	"go.etcd.io/etcd/api/v3/authpb"
+	"go.etcd.io/etcd/server/v3/mvcc/backend"
+	"go.uber.org/zap"
+)
+
+func UnsafeGetUser(lg *zap.Logger, tx backend.BatchTx, username string) *authpb.User {
+	_, vs := tx.UnsafeRange(AuthUsers, []byte(username), nil, 0)
+	if len(vs) == 0 {
+		return nil
+	}
+
+	user := &authpb.User{}
+	err := user.Unmarshal(vs[0])
+	if err != nil {
+		lg.Panic(
+			"failed to unmarshal 'authpb.User'",
+			zap.String("user-name", username),
+			zap.Error(err),
+		)
+	}
+	return user
+}
+
+func UnsafeGetAllUsers(lg *zap.Logger, tx backend.BatchTx) []*authpb.User {
+	_, vs := tx.UnsafeRange(AuthUsers, []byte{0}, []byte{0xff}, -1)
+	if len(vs) == 0 {
+		return nil
+	}
+
+	users := make([]*authpb.User, len(vs))
+	for i := range vs {
+		user := &authpb.User{}
+		err := user.Unmarshal(vs[i])
+		if err != nil {
+			lg.Panic("failed to unmarshal 'authpb.User'", zap.Error(err))
+		}
+		users[i] = user
+	}
+	return users
+}
+
+func UnsafePutUser(lg *zap.Logger, tx backend.BatchTx, user *authpb.User) {
+	b, err := user.Marshal()
+	if err != nil {
+		lg.Panic("failed to unmarshal 'authpb.User'", zap.Error(err))
+	}
+	tx.UnsafePut(AuthUsers, user.Name, b)
+}
+
+func UnsafeDeleteUser(tx backend.BatchTx, username string) {
+	tx.UnsafeDelete(AuthUsers, []byte(username))
+}