From 84fe23d530ba006aa22b3703ef1737aa46d94094 Mon Sep 17 00:00:00 2001 From: Jingyi Hu Date: Thu, 6 Feb 2020 12:28:14 -0800 Subject: [PATCH] auth: remove capnslog (#11596) --- auth/jwt.go | 67 +++---- auth/range_perm_cache.go | 26 +-- auth/simple_token.go | 31 ++- auth/store.go | 394 +++++++++++++-------------------------- 4 files changed, 170 insertions(+), 348 deletions(-) diff --git a/auth/jwt.go b/auth/jwt.go index 52cafe4aa..778bb9a2d 100644 --- a/auth/jwt.go +++ b/auth/jwt.go @@ -60,25 +60,17 @@ func (t *tokenJWT) info(ctx context.Context, token string, rev uint64) (*AuthInf }) if err != nil { - if t.lg != nil { - t.lg.Warn( - "failed to parse a JWT token", - zap.String("token", token), - zap.Error(err), - ) - } else { - plog.Warningf("failed to parse jwt token: %s", err) - } + t.lg.Warn( + "failed to parse a JWT token", + zap.String("token", token), + zap.Error(err), + ) return nil, false } claims, ok := parsed.Claims.(jwt.MapClaims) if !parsed.Valid || !ok { - if t.lg != nil { - t.lg.Warn("invalid JWT token", zap.String("token", token)) - } else { - plog.Warningf("invalid jwt token: %s", token) - } + t.lg.Warn("invalid JWT token", zap.String("token", token)) return nil, false } @@ -104,42 +96,33 @@ func (t *tokenJWT) assign(ctx context.Context, username string, revision uint64) token, err := tk.SignedString(t.key) if err != nil { - if t.lg != nil { - t.lg.Debug( - "failed to sign a JWT token", - zap.String("user-name", username), - zap.Uint64("revision", revision), - zap.Error(err), - ) - } else { - plog.Debugf("failed to sign jwt token: %s", err) - } + t.lg.Debug( + "failed to sign a JWT token", + zap.String("user-name", username), + zap.Uint64("revision", revision), + zap.Error(err), + ) return "", err } - if t.lg != nil { - t.lg.Debug( - "created/assigned a new JWT token", - zap.String("user-name", username), - zap.Uint64("revision", revision), - zap.String("token", token), - ) - } else { - plog.Debugf("jwt token: %s", token) - } + t.lg.Debug( + "created/assigned a new JWT token", + zap.String("user-name", username), + zap.Uint64("revision", revision), + zap.String("token", token), + ) return token, err } func newTokenProviderJWT(lg *zap.Logger, optMap map[string]string) (*tokenJWT, error) { + if lg == nil { + lg = zap.NewNop() + } var err error var opts jwtOptions err = opts.ParseWithDefaults(optMap) if err != nil { - if lg != nil { - lg.Error("problem loading JWT options", zap.Error(err)) - } else { - plog.Errorf("problem loading JWT options: %s", err) - } + lg.Error("problem loading JWT options", zap.Error(err)) return nil, ErrInvalidAuthOpts } @@ -150,11 +133,7 @@ func newTokenProviderJWT(lg *zap.Logger, optMap map[string]string) (*tokenJWT, e } } if len(keys) > 0 { - if lg != nil { - lg.Warn("unknown JWT options", zap.Strings("keys", keys)) - } else { - plog.Warningf("unknown JWT options: %v", keys) - } + lg.Warn("unknown JWT options", zap.Strings("keys", keys)) } key, err := opts.Key() diff --git a/auth/range_perm_cache.go b/auth/range_perm_cache.go index 7b6c18240..5c6a5e607 100644 --- a/auth/range_perm_cache.go +++ b/auth/range_perm_cache.go @@ -32,7 +32,7 @@ func getMergedPerms(lg *zap.Logger, tx backend.BatchTx, userName string) *unifie writePerms := adt.NewIntervalTree() for _, roleName := range user.Roles { - role := getRole(tx, roleName) + role := getRole(lg, tx, roleName) if role == nil { continue } @@ -87,11 +87,7 @@ func checkKeyInterval( case authpb.WRITE: return cachedPerms.writePerms.Contains(ivl) default: - if lg != nil { - lg.Panic("unknown auth type", zap.String("auth-type", permtyp.String())) - } else { - plog.Panicf("unknown auth type: %v", permtyp) - } + lg.Panic("unknown auth type", zap.String("auth-type", permtyp.String())) } return false } @@ -104,11 +100,7 @@ func checkKeyPoint(lg *zap.Logger, cachedPerms *unifiedRangePermissions, key []b case authpb.WRITE: return cachedPerms.writePerms.Intersects(pt) default: - if lg != nil { - lg.Panic("unknown auth type", zap.String("auth-type", permtyp.String())) - } else { - plog.Panicf("unknown auth type: %v", permtyp) - } + lg.Panic("unknown auth type", zap.String("auth-type", permtyp.String())) } return false } @@ -119,14 +111,10 @@ func (as *authStore) isRangeOpPermitted(tx backend.BatchTx, userName string, key if !ok { perms := getMergedPerms(as.lg, tx, userName) if perms == nil { - if as.lg != nil { - as.lg.Warn( - "failed to create a merged permission", - zap.String("user-name", userName), - ) - } else { - plog.Errorf("failed to create a unified permission of user %s", userName) - } + as.lg.Error( + "failed to create a merged permission", + zap.String("user-name", userName), + ) return false } as.rangePermCache[userName] = perms diff --git a/auth/simple_token.go b/auth/simple_token.go index 934978c98..d96cc48db 100644 --- a/auth/simple_token.go +++ b/auth/simple_token.go @@ -127,15 +127,11 @@ func (t *tokenSimple) assignSimpleTokenToUser(username, token string) { _, ok := t.simpleTokens[token] if ok { - if t.lg != nil { - t.lg.Panic( - "failed to assign already-used simple token to a user", - zap.String("user-name", username), - zap.String("token", token), - ) - } else { - plog.Panicf("token %s is already used", token) - } + t.lg.Panic( + "failed to assign already-used simple token to a user", + zap.String("user-name", username), + zap.String("token", token), + ) } t.simpleTokens[token] = username @@ -159,15 +155,11 @@ func (t *tokenSimple) invalidateUser(username string) { func (t *tokenSimple) enable() { delf := func(tk string) { if username, ok := t.simpleTokens[tk]; ok { - if t.lg != nil { - t.lg.Info( - "deleted a simple token", - zap.String("user-name", username), - zap.String("token", tk), - ) - } else { - plog.Infof("deleting token %s for user %s", tk, username) - } + t.lg.Info( + "deleted a simple token", + zap.String("user-name", username), + zap.String("token", tk), + ) delete(t.simpleTokens, tk) } } @@ -235,6 +227,9 @@ func (t *tokenSimple) isValidSimpleToken(ctx context.Context, token string) bool } func newTokenProviderSimple(lg *zap.Logger, indexWaiter func(uint64) <-chan struct{}) *tokenSimple { + if lg == nil { + lg = zap.NewNop() + } return &tokenSimple{ lg: lg, simpleTokens: make(map[string]string), diff --git a/auth/store.go b/auth/store.go index 3c1536229..e1ba64a96 100644 --- a/auth/store.go +++ b/auth/store.go @@ -29,7 +29,6 @@ import ( pb "go.etcd.io/etcd/etcdserver/etcdserverpb" "go.etcd.io/etcd/mvcc/backend" - "github.com/coreos/pkg/capnslog" "go.uber.org/zap" "golang.org/x/crypto/bcrypt" "google.golang.org/grpc/credentials" @@ -48,8 +47,6 @@ var ( authUsersBucketName = []byte("authUsers") authRolesBucketName = []byte("authRoles") - plog = capnslog.NewPackageLogger("go.etcd.io/etcd", "auth") - ErrRootUserNotExist = errors.New("auth: root user does not exist") ErrRootRoleNotExist = errors.New("auth: root user does not have root role") ErrUserAlreadyExist = errors.New("auth: user already exists") @@ -217,11 +214,7 @@ func (as *authStore) AuthEnable() error { as.enabledMu.Lock() defer as.enabledMu.Unlock() if as.enabled { - if as.lg != nil { - as.lg.Info("authentication is already enabled; ignored auth enable request") - } else { - plog.Noticef("Authentication already enabled") - } + as.lg.Info("authentication is already enabled; ignored auth enable request") return nil } b := as.be @@ -250,11 +243,7 @@ func (as *authStore) AuthEnable() error { as.setRevision(getRevision(tx)) - if as.lg != nil { - as.lg.Info("enabled authentication") - } else { - plog.Noticef("Authentication enabled") - } + as.lg.Info("enabled authentication") return nil } @@ -275,11 +264,7 @@ func (as *authStore) AuthDisable() { as.enabled = false as.tokenProvider.disable() - if as.lg != nil { - as.lg.Info("disabled authentication") - } else { - plog.Noticef("Authentication disabled") - } + as.lg.Info("disabled authentication") } func (as *authStore) Close() error { @@ -318,15 +303,11 @@ func (as *authStore) Authenticate(ctx context.Context, username, password string return nil, err } - if as.lg != nil { - as.lg.Debug( - "authenticated a user", - zap.String("user-name", username), - zap.String("token", token), - ) - } else { - plog.Debugf("authorized %s, token is %s", username, token) - } + as.lg.Debug( + "authenticated a user", + zap.String("user-name", username), + zap.String("token", token), + ) return &pb.AuthenticateResponse{Token: token}, nil } @@ -349,11 +330,7 @@ func (as *authStore) CheckPassword(username, password string) (uint64, error) { } if bcrypt.CompareHashAndPassword(user.Password, []byte(password)) != nil { - if as.lg != nil { - as.lg.Info("invalid password", zap.String("user-name", username)) - } else { - plog.Noticef("authentication failed, invalid password for user %s", username) - } + as.lg.Info("invalid password", zap.String("user-name", username)) return 0, ErrAuthFailed } return getRevision(tx), nil @@ -392,15 +369,11 @@ func (as *authStore) UserAdd(r *pb.AuthUserAddRequest) (*pb.AuthUserAddResponse, if !noPassword { hashed, err = bcrypt.GenerateFromPassword([]byte(r.Password), as.bcryptCost) if err != nil { - if as.lg != nil { - as.lg.Warn( - "failed to bcrypt hash password", - zap.String("user-name", r.Name), - zap.Error(err), - ) - } else { - plog.Errorf("failed to hash password: %s", err) - } + as.lg.Error( + "failed to bcrypt hash password", + zap.String("user-name", r.Name), + zap.Error(err), + ) return nil, err } } @@ -431,21 +404,13 @@ func (as *authStore) UserAdd(r *pb.AuthUserAddRequest) (*pb.AuthUserAddResponse, as.commitRevision(tx) - if as.lg != nil { - as.lg.Info("added a user", zap.String("user-name", r.Name)) - } else { - plog.Noticef("added a new user: %s", r.Name) - } + as.lg.Info("added a user", zap.String("user-name", r.Name)) return &pb.AuthUserAddResponse{}, nil } func (as *authStore) UserDelete(r *pb.AuthUserDeleteRequest) (*pb.AuthUserDeleteResponse, error) { if as.enabled && r.Name == rootUser { - if as.lg != nil { - as.lg.Warn("cannot delete 'root' user", zap.String("user-name", r.Name)) - } else { - plog.Errorf("the user root must not be deleted") - } + as.lg.Error("cannot delete 'root' user", zap.String("user-name", r.Name)) return nil, ErrInvalidAuthMgmt } @@ -465,15 +430,11 @@ func (as *authStore) UserDelete(r *pb.AuthUserDeleteRequest) (*pb.AuthUserDelete as.invalidateCachedPerm(r.Name) as.tokenProvider.invalidateUser(r.Name) - if as.lg != nil { - as.lg.Info( - "deleted a user", - zap.String("user-name", r.Name), - zap.Strings("user-roles", user.Roles), - ) - } else { - plog.Noticef("deleted a user: %s", r.Name) - } + as.lg.Info( + "deleted a user", + zap.String("user-name", r.Name), + zap.Strings("user-roles", user.Roles), + ) return &pb.AuthUserDeleteResponse{}, nil } @@ -482,15 +443,11 @@ func (as *authStore) UserChangePassword(r *pb.AuthUserChangePasswordRequest) (*p // If the cost is too high, we should move the encryption to outside of the raft hashed, err := bcrypt.GenerateFromPassword([]byte(r.Password), as.bcryptCost) if err != nil { - if as.lg != nil { - as.lg.Warn( - "failed to bcrypt hash password", - zap.String("user-name", r.Name), - zap.Error(err), - ) - } else { - plog.Errorf("failed to hash password: %s", err) - } + as.lg.Error( + "failed to bcrypt hash password", + zap.String("user-name", r.Name), + zap.Error(err), + ) return nil, err } @@ -517,15 +474,11 @@ func (as *authStore) UserChangePassword(r *pb.AuthUserChangePasswordRequest) (*p as.invalidateCachedPerm(r.Name) as.tokenProvider.invalidateUser(r.Name) - if as.lg != nil { - as.lg.Info( - "changed a password of a user", - zap.String("user-name", r.Name), - zap.Strings("user-roles", user.Roles), - ) - } else { - plog.Noticef("changed a password of a user: %s", r.Name) - } + as.lg.Info( + "changed a password of a user", + zap.String("user-name", r.Name), + zap.Strings("user-roles", user.Roles), + ) return &pb.AuthUserChangePasswordResponse{}, nil } @@ -540,7 +493,7 @@ func (as *authStore) UserGrantRole(r *pb.AuthUserGrantRoleRequest) (*pb.AuthUser } if r.Role != rootRole { - role := getRole(tx, r.Role) + role := getRole(as.lg, tx, r.Role) if role == nil { return nil, ErrRoleNotFound } @@ -548,16 +501,12 @@ func (as *authStore) UserGrantRole(r *pb.AuthUserGrantRoleRequest) (*pb.AuthUser idx := sort.SearchStrings(user.Roles, r.Role) if idx < len(user.Roles) && user.Roles[idx] == r.Role { - if as.lg != nil { - as.lg.Warn( - "ignored grant role request to a user", - zap.String("user-name", r.User), - zap.Strings("user-roles", user.Roles), - zap.String("duplicate-role-name", r.Role), - ) - } else { - plog.Warningf("user %s is already granted role %s", r.User, r.Role) - } + as.lg.Warn( + "ignored grant role request to a user", + zap.String("user-name", r.User), + zap.Strings("user-roles", user.Roles), + zap.String("duplicate-role-name", r.Role), + ) return &pb.AuthUserGrantRoleResponse{}, nil } @@ -570,16 +519,12 @@ func (as *authStore) UserGrantRole(r *pb.AuthUserGrantRoleRequest) (*pb.AuthUser as.commitRevision(tx) - if as.lg != nil { - as.lg.Info( - "granted a role to a user", - zap.String("user-name", r.User), - zap.Strings("user-roles", user.Roles), - zap.String("added-role-name", r.Role), - ) - } else { - plog.Noticef("granted role %s to user %s", r.Role, r.User) - } + as.lg.Info( + "granted a role to a user", + zap.String("user-name", r.User), + zap.Strings("user-roles", user.Roles), + zap.String("added-role-name", r.Role), + ) return &pb.AuthUserGrantRoleResponse{}, nil } @@ -613,15 +558,11 @@ func (as *authStore) UserList(r *pb.AuthUserListRequest) (*pb.AuthUserListRespon func (as *authStore) UserRevokeRole(r *pb.AuthUserRevokeRoleRequest) (*pb.AuthUserRevokeRoleResponse, error) { if as.enabled && r.Name == rootUser && r.Role == rootRole { - if as.lg != nil { - as.lg.Warn( - "'root' user cannot revoke 'root' role", - zap.String("user-name", r.Name), - zap.String("role-name", r.Role), - ) - } else { - plog.Errorf("the role root must not be revoked from the user root") - } + as.lg.Error( + "'root' user cannot revoke 'root' role", + zap.String("user-name", r.Name), + zap.String("role-name", r.Role), + ) return nil, ErrInvalidAuthMgmt } @@ -656,17 +597,13 @@ func (as *authStore) UserRevokeRole(r *pb.AuthUserRevokeRoleRequest) (*pb.AuthUs as.commitRevision(tx) - if as.lg != nil { - as.lg.Info( - "revoked a role from a user", - zap.String("user-name", r.Name), - zap.Strings("old-user-roles", user.Roles), - zap.Strings("new-user-roles", updatedUser.Roles), - zap.String("revoked-role-name", r.Role), - ) - } else { - plog.Noticef("revoked role %s from user %s", r.Role, r.Name) - } + as.lg.Info( + "revoked a role from a user", + zap.String("user-name", r.Name), + zap.Strings("old-user-roles", user.Roles), + zap.Strings("new-user-roles", updatedUser.Roles), + zap.String("revoked-role-name", r.Role), + ) return &pb.AuthUserRevokeRoleResponse{}, nil } @@ -677,7 +614,7 @@ func (as *authStore) RoleGet(r *pb.AuthRoleGetRequest) (*pb.AuthRoleGetResponse, var resp pb.AuthRoleGetResponse - role := getRole(tx, r.Role) + role := getRole(as.lg, tx, r.Role) if role == nil { return nil, ErrRoleNotFound } @@ -703,7 +640,7 @@ func (as *authStore) RoleRevokePermission(r *pb.AuthRoleRevokePermissionRequest) tx.Lock() defer tx.Unlock() - role := getRole(tx, r.Role) + role := getRole(as.lg, tx, r.Role) if role == nil { return nil, ErrRoleNotFound } @@ -730,26 +667,18 @@ func (as *authStore) RoleRevokePermission(r *pb.AuthRoleRevokePermissionRequest) as.commitRevision(tx) - if as.lg != nil { - as.lg.Info( - "revoked a permission on range", - zap.String("role-name", r.Role), - zap.String("key", string(r.Key)), - zap.String("range-end", string(r.RangeEnd)), - ) - } else { - plog.Noticef("revoked key %s from role %s", r.Key, r.Role) - } + as.lg.Info( + "revoked a permission on range", + zap.String("role-name", r.Role), + zap.String("key", string(r.Key)), + zap.String("range-end", string(r.RangeEnd)), + ) return &pb.AuthRoleRevokePermissionResponse{}, nil } func (as *authStore) RoleDelete(r *pb.AuthRoleDeleteRequest) (*pb.AuthRoleDeleteResponse, error) { if as.enabled && r.Role == rootRole { - if as.lg != nil { - as.lg.Warn("cannot delete 'root' role", zap.String("role-name", r.Role)) - } else { - plog.Errorf("the role root must not be deleted") - } + as.lg.Error("cannot delete 'root' role", zap.String("role-name", r.Role)) return nil, ErrInvalidAuthMgmt } @@ -757,7 +686,7 @@ func (as *authStore) RoleDelete(r *pb.AuthRoleDeleteRequest) (*pb.AuthRoleDelete tx.Lock() defer tx.Unlock() - role := getRole(tx, r.Role) + role := getRole(as.lg, tx, r.Role) if role == nil { return nil, ErrRoleNotFound } @@ -789,11 +718,7 @@ func (as *authStore) RoleDelete(r *pb.AuthRoleDeleteRequest) (*pb.AuthRoleDelete as.commitRevision(tx) - if as.lg != nil { - as.lg.Info("deleted a role", zap.String("role-name", r.Role)) - } else { - plog.Noticef("deleted role %s", r.Role) - } + as.lg.Info("deleted a role", zap.String("role-name", r.Role)) return &pb.AuthRoleDeleteResponse{}, nil } @@ -806,7 +731,7 @@ func (as *authStore) RoleAdd(r *pb.AuthRoleAddRequest) (*pb.AuthRoleAddResponse, tx.Lock() defer tx.Unlock() - role := getRole(tx, r.Name) + role := getRole(as.lg, tx, r.Name) if role != nil { return nil, ErrRoleAlreadyExist } @@ -819,11 +744,7 @@ func (as *authStore) RoleAdd(r *pb.AuthRoleAddRequest) (*pb.AuthRoleAddResponse, as.commitRevision(tx) - if as.lg != nil { - as.lg.Info("created a role", zap.String("role-name", r.Name)) - } else { - plog.Noticef("Role %s is created", r.Name) - } + as.lg.Info("created a role", zap.String("role-name", r.Name)) return &pb.AuthRoleAddResponse{}, nil } @@ -850,7 +771,7 @@ func (as *authStore) RoleGrantPermission(r *pb.AuthRoleGrantPermissionRequest) ( tx.Lock() defer tx.Unlock() - role := getRole(tx, r.Name) + role := getRole(as.lg, tx, r.Name) if role == nil { return nil, ErrRoleNotFound } @@ -882,15 +803,11 @@ func (as *authStore) RoleGrantPermission(r *pb.AuthRoleGrantPermissionRequest) ( as.commitRevision(tx) - if as.lg != nil { - as.lg.Info( - "granted/updated a permission to a user", - zap.String("user-name", r.Name), - zap.String("permission-name", authpb.Permission_Type_name[int32(r.Perm.PermType)]), - ) - } else { - plog.Noticef("role %s's permission of key %s is updated as %s", r.Name, r.Perm.Key, authpb.Permission_Type_name[int32(r.Perm.PermType)]) - } + as.lg.Info( + "granted/updated a permission to a user", + zap.String("user-name", r.Name), + zap.String("permission-name", authpb.Permission_Type_name[int32(r.Perm.PermType)]), + ) return &pb.AuthRoleGrantPermissionResponse{}, nil } @@ -915,11 +832,7 @@ func (as *authStore) isOpPermitted(userName string, revision uint64, key, rangeE user := getUser(as.lg, tx, userName) if user == nil { - if as.lg != nil { - as.lg.Warn("cannot find a user for permission check", zap.String("user-name", userName)) - } else { - plog.Errorf("invalid user name %s for permission checking", userName) - } + as.lg.Error("cannot find a user for permission check", zap.String("user-name", userName)) return ErrPermissionDenied } @@ -980,15 +893,11 @@ func getUser(lg *zap.Logger, tx backend.BatchTx, username string) *authpb.User { user := &authpb.User{} err := user.Unmarshal(vs[0]) if err != nil { - if lg != nil { - lg.Panic( - "failed to unmarshal 'authpb.User'", - zap.String("user-name", username), - zap.Error(err), - ) - } else { - plog.Panicf("failed to unmarshal user struct (name: %s): %s", username, err) - } + lg.Panic( + "failed to unmarshal 'authpb.User'", + zap.String("user-name", username), + zap.Error(err), + ) } return user } @@ -1004,11 +913,7 @@ func getAllUsers(lg *zap.Logger, tx backend.BatchTx) []*authpb.User { user := &authpb.User{} err := user.Unmarshal(vs[i]) if err != nil { - if lg != nil { - lg.Panic("failed to unmarshal 'authpb.User'", zap.Error(err)) - } else { - plog.Panicf("failed to unmarshal user struct: %s", err) - } + lg.Panic("failed to unmarshal 'authpb.User'", zap.Error(err)) } users[i] = user } @@ -1018,11 +923,7 @@ func getAllUsers(lg *zap.Logger, tx backend.BatchTx) []*authpb.User { func putUser(lg *zap.Logger, tx backend.BatchTx, user *authpb.User) { b, err := user.Marshal() if err != nil { - if lg != nil { - lg.Panic("failed to unmarshal 'authpb.User'", zap.Error(err)) - } else { - plog.Panicf("failed to marshal user struct (name: %s): %s", user.Name, err) - } + lg.Panic("failed to unmarshal 'authpb.User'", zap.Error(err)) } tx.UnsafePut(authUsersBucketName, user.Name, b) } @@ -1031,7 +932,7 @@ func delUser(tx backend.BatchTx, username string) { tx.UnsafeDelete(authUsersBucketName, []byte(username)) } -func getRole(tx backend.BatchTx, rolename string) *authpb.Role { +func getRole(lg *zap.Logger, tx backend.BatchTx, rolename string) *authpb.Role { _, vs := tx.UnsafeRange(authRolesBucketName, []byte(rolename), nil, 0) if len(vs) == 0 { return nil @@ -1040,7 +941,7 @@ func getRole(tx backend.BatchTx, rolename string) *authpb.Role { role := &authpb.Role{} err := role.Unmarshal(vs[0]) if err != nil { - plog.Panicf("failed to unmarshal role struct (name: %s): %s", rolename, err) + lg.Panic("failed to unmarshal 'authpb.Role'", zap.Error(err)) } return role } @@ -1056,11 +957,7 @@ func getAllRoles(lg *zap.Logger, tx backend.BatchTx) []*authpb.Role { role := &authpb.Role{} err := role.Unmarshal(vs[i]) if err != nil { - if lg != nil { - lg.Panic("failed to unmarshal 'authpb.Role'", zap.Error(err)) - } else { - plog.Panicf("failed to unmarshal role struct: %s", err) - } + lg.Panic("failed to unmarshal 'authpb.Role'", zap.Error(err)) } roles[i] = role } @@ -1070,15 +967,11 @@ func getAllRoles(lg *zap.Logger, tx backend.BatchTx) []*authpb.Role { func putRole(lg *zap.Logger, tx backend.BatchTx, role *authpb.Role) { b, err := role.Marshal() if err != nil { - if lg != nil { - lg.Panic( - "failed to marshal 'authpb.Role'", - zap.String("role-name", string(role.Name)), - zap.Error(err), - ) - } else { - plog.Panicf("failed to marshal role struct (name: %s): %s", role.Name, err) - } + lg.Panic( + "failed to marshal 'authpb.Role'", + zap.String("role-name", string(role.Name)), + zap.Error(err), + ) } tx.UnsafePut(authRolesBucketName, role.Name, b) @@ -1096,19 +989,18 @@ func (as *authStore) IsAuthEnabled() bool { // NewAuthStore creates a new AuthStore. func NewAuthStore(lg *zap.Logger, be backend.Backend, tp TokenProvider, bcryptCost int) *authStore { - if bcryptCost < bcrypt.MinCost || bcryptCost > bcrypt.MaxCost { - if lg != nil { - lg.Warn( - "use default bcrypt cost instead of the invalid given cost", - zap.Int("min-cost", bcrypt.MinCost), - zap.Int("max-cost", bcrypt.MaxCost), - zap.Int("default-cost", bcrypt.DefaultCost), - zap.Int("given-cost", bcryptCost)) - } else { - plog.Warningf("Use default bcrypt-cost %d instead of the invalid value %d", - bcrypt.DefaultCost, bcryptCost) - } + if lg == nil { + lg = zap.NewNop() + } + if bcryptCost < bcrypt.MinCost || bcryptCost > bcrypt.MaxCost { + lg.Warn( + "use default bcrypt cost instead of the invalid given cost", + zap.Int("min-cost", bcrypt.MinCost), + zap.Int("max-cost", bcrypt.MaxCost), + zap.Int("default-cost", bcrypt.DefaultCost), + zap.Int("given-cost", bcryptCost), + ) bcryptCost = bcrypt.DefaultCost } @@ -1205,28 +1097,20 @@ func (as *authStore) AuthInfoFromTLS(ctx context.Context) (ai *AuthInfo) { // header. The proxy uses etcd client server certificate. If the certificate // has a CommonName we should never use this for authentication. if gw := md["grpcgateway-accept"]; len(gw) > 0 { - if as.lg != nil { - as.lg.Warn( - "ignoring common name in gRPC-gateway proxy request", - zap.String("common-name", ai.Username), - zap.String("user-name", ai.Username), - zap.Uint64("revision", ai.Revision), - ) - } else { - plog.Warningf("ignoring common name in gRPC-gateway proxy request %s", ai.Username) - } - return nil - } - if as.lg != nil { - as.lg.Debug( - "found command name", + as.lg.Warn( + "ignoring common name in gRPC-gateway proxy request", zap.String("common-name", ai.Username), zap.String("user-name", ai.Username), zap.Uint64("revision", ai.Revision), ) - } else { - plog.Debugf("found common name %s", ai.Username) + return nil } + as.lg.Debug( + "found command name", + zap.String("common-name", ai.Username), + zap.String("user-name", ai.Username), + zap.Uint64("revision", ai.Revision), + ) break } return ai @@ -1250,11 +1134,7 @@ func (as *authStore) AuthInfoFromCtx(ctx context.Context) (*AuthInfo, error) { token := ts[0] authInfo, uok := as.authInfoFromToken(ctx, token) if !uok { - if as.lg != nil { - as.lg.Warn("invalid auth token", zap.String("token", token)) - } else { - plog.Warningf("invalid auth token: %s", token) - } + as.lg.Warn("invalid auth token", zap.String("token", token)) return nil, ErrInvalidAuthToken } @@ -1275,22 +1155,18 @@ func decomposeOpts(lg *zap.Logger, optstr string) (string, map[string]string, er if len(pair) != 2 { if lg != nil { - lg.Warn("invalid token option", zap.String("option", optstr)) - } else { - plog.Errorf("invalid token specific option: %s", optstr) + lg.Error("invalid token option", zap.String("option", optstr)) } return "", nil, ErrInvalidAuthOpts } if _, ok := typeSpecificOpts[pair[0]]; ok { if lg != nil { - lg.Warn( + lg.Error( "invalid token option", zap.String("option", optstr), zap.String("duplicate-parameter", pair[0]), ) - } else { - plog.Errorf("invalid token specific option, duplicated parameters (%s): %s", pair[0], optstr) } return "", nil, ErrInvalidAuthOpts } @@ -1316,8 +1192,6 @@ func NewTokenProvider( case tokenTypeSimple: if lg != nil { lg.Warn("simple token is not cryptographically signed") - } else { - plog.Warningf("simple token is not cryptographically signed") } return newTokenProviderSimple(lg, indexWaiter), nil @@ -1334,8 +1208,6 @@ func NewTokenProvider( zap.String("type", tokenType), zap.Error(ErrInvalidAuthOpts), ) - } else { - plog.Errorf("unknown token type: %s", tokenType) } return nil, ErrInvalidAuthOpts } @@ -1351,14 +1223,10 @@ func (as *authStore) WithRoot(ctx context.Context) context.Context { ctx1 := context.WithValue(ctx, AuthenticateParamIndex{}, uint64(0)) prefix, err := ts.genTokenPrefix() if err != nil { - if as.lg != nil { - as.lg.Warn( - "failed to generate prefix of internally used token", - zap.Error(err), - ) - } else { - plog.Errorf("failed to generate prefix of internally used token") - } + as.lg.Error( + "failed to generate prefix of internally used token", + zap.Error(err), + ) return ctx } ctxForAssign = context.WithValue(ctx1, AuthenticateParamSimpleTokenPrefix{}, prefix) @@ -1369,14 +1237,10 @@ func (as *authStore) WithRoot(ctx context.Context) context.Context { token, err := as.tokenProvider.assign(ctxForAssign, "root", as.Revision()) if err != nil { // this must not happen - if as.lg != nil { - as.lg.Warn( - "failed to assign token for lease revoking", - zap.Error(err), - ) - } else { - plog.Errorf("failed to assign token for lease revoking: %s", err) - } + as.lg.Error( + "failed to assign token for lease revoking", + zap.Error(err), + ) return ctx } @@ -1396,15 +1260,11 @@ func (as *authStore) HasRole(user, role string) bool { tx.Unlock() if u == nil { - if as.lg != nil { - as.lg.Warn( - "'has-role' requested for non-existing user", - zap.String("user-name", user), - zap.String("role-name", role), - ) - } else { - plog.Warningf("tried to check user %s has role %s, but user %s doesn't exist", user, role, user) - } + as.lg.Warn( + "'has-role' requested for non-existing user", + zap.String("user-name", user), + zap.String("role-name", role), + ) return false }