Merge pull request #4879 from mitake/auth-user-error

etcdserver: return internal error in a case of not auth specific errors

auth/store.go

@@ -15,8 +15,9 @@
 package auth
 
 import (
+	"errors"
+
 	"github.com/coreos/etcd/auth/authpb"
-	"github.com/coreos/etcd/etcdserver/api/v3rpc/rpctypes"
 	pb "github.com/coreos/etcd/etcdserver/etcdserverpb"
 	"github.com/coreos/etcd/storage/backend"
 	"github.com/coreos/pkg/capnslog"
@@ -29,6 +30,8 @@ var (
 	authUsersBucketName = []byte("authUsers")
 
 	plog = capnslog.NewPackageLogger("github.com/coreos/etcd", "auth")
+
+	ErrUserAlreadyExist = errors.New("auth: user already exists")
 )
 
 type AuthStore interface {
@@ -79,7 +82,7 @@ func (as *authStore) UserAdd(r *pb.AuthUserAddRequest) (*pb.AuthUserAddResponse,
 
 	_, vs := tx.UnsafeRange(authUsersBucketName, []byte(r.Name), nil, 0)
 	if len(vs) != 0 {
-		return &pb.AuthUserAddResponse{}, rpctypes.ErrUserAlreadyExist
+		return &pb.AuthUserAddResponse{}, ErrUserAlreadyExist
 	}
 
 	newUser := authpb.User{

etcdserver/api/v3rpc/auth.go

@@ -29,7 +29,11 @@ func NewAuthServer(s *etcdserver.EtcdServer) *AuthServer {
 }
 
 func (as *AuthServer) AuthEnable(ctx context.Context, r *pb.AuthEnableRequest) (*pb.AuthEnableResponse, error) {
-	return as.authenticator.AuthEnable(ctx, r)
+	resp, err := as.authenticator.AuthEnable(ctx, r)
+	if err != nil {
+		return nil, togRPCError(err)
+	}
+	return resp, nil
 }
 
 func (as *AuthServer) AuthDisable(ctx context.Context, r *pb.AuthDisableRequest) (*pb.AuthDisableResponse, error) {
@@ -68,7 +72,11 @@ func (as *AuthServer) RoleGrant(ctx context.Context, r *pb.AuthRoleGrantRequest)
 }
 
 func (as *AuthServer) UserAdd(ctx context.Context, r *pb.AuthUserAddRequest) (*pb.AuthUserAddResponse, error) {
-	return as.authenticator.UserAdd(ctx, r)
+	resp, err := as.authenticator.UserAdd(ctx, r)
+	if err != nil {
+		return nil, togRPCError(err)
+	}
+	return resp, nil
 }
 
 func (as *AuthServer) UserDelete(ctx context.Context, r *pb.AuthUserDeleteRequest) (*pb.AuthUserDeleteResponse, error) {

etcdserver/api/v3rpc/key.go

@@ -21,12 +21,8 @@ import (
 	"github.com/coreos/etcd/etcdserver"
 	"github.com/coreos/etcd/etcdserver/api/v3rpc/rpctypes"
 	pb "github.com/coreos/etcd/etcdserver/etcdserverpb"
-	"github.com/coreos/etcd/lease"
-	"github.com/coreos/etcd/storage"
 	"github.com/coreos/pkg/capnslog"
 	"golang.org/x/net/context"
-	"google.golang.org/grpc"
-	"google.golang.org/grpc/codes"
 )
 
 var (
@@ -261,21 +257,3 @@ func checkRequestUnion(u *pb.RequestUnion) error {
 	}
 	return nil
 }
-
-func togRPCError(err error) error {
-	switch err {
-	case storage.ErrCompacted:
-		return rpctypes.ErrCompacted
-	case storage.ErrFutureRev:
-		return rpctypes.ErrFutureRev
-	case lease.ErrLeaseNotFound:
-		return rpctypes.ErrLeaseNotFound
-	// TODO: handle error from raft and timeout
-	case etcdserver.ErrRequestTooLarge:
-		return rpctypes.ErrRequestTooLarge
-	case etcdserver.ErrNoSpace:
-		return rpctypes.ErrNoSpace
-	default:
-		return grpc.Errorf(codes.Internal, err.Error())
-	}
-}

etcdserver/api/v3rpc/util.go

@@ -0,0 +1,45 @@
+// Copyright 2016 Nippon Telegraph and Telephone Corporation.
+//
+// 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 v3rpc
+
+import (
+	"github.com/coreos/etcd/auth"
+	"github.com/coreos/etcd/etcdserver"
+	"github.com/coreos/etcd/etcdserver/api/v3rpc/rpctypes"
+	"github.com/coreos/etcd/lease"
+	"github.com/coreos/etcd/storage"
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/codes"
+)
+
+func togRPCError(err error) error {
+	switch err {
+	case storage.ErrCompacted:
+		return rpctypes.ErrCompacted
+	case storage.ErrFutureRev:
+		return rpctypes.ErrFutureRev
+	case lease.ErrLeaseNotFound:
+		return rpctypes.ErrLeaseNotFound
+	// TODO: handle error from raft and timeout
+	case etcdserver.ErrRequestTooLarge:
+		return rpctypes.ErrRequestTooLarge
+	case etcdserver.ErrNoSpace:
+		return rpctypes.ErrNoSpace
+	case auth.ErrUserAlreadyExist:
+		return rpctypes.ErrUserAlreadyExist
+	default:
+		return grpc.Errorf(codes.Internal, err.Error())
+	}
+}