From 443c6773577b08d9fb693116a22bf13d01bd2f7a Mon Sep 17 00:00:00 2001 From: Hitoshi Mitake Date: Wed, 30 Mar 2016 22:40:42 +0900 Subject: [PATCH 1/2] etcdserver: extract togRPCError() to a separated file It is used from multiple files in v3rpc package. --- etcdserver/api/v3rpc/key.go | 22 ------------------- etcdserver/api/v3rpc/util.go | 42 ++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 22 deletions(-) create mode 100644 etcdserver/api/v3rpc/util.go diff --git a/etcdserver/api/v3rpc/key.go b/etcdserver/api/v3rpc/key.go index 33039f15b..c7f2f9d56 100644 --- a/etcdserver/api/v3rpc/key.go +++ b/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()) - } -} diff --git a/etcdserver/api/v3rpc/util.go b/etcdserver/api/v3rpc/util.go new file mode 100644 index 000000000..8780d71d6 --- /dev/null +++ b/etcdserver/api/v3rpc/util.go @@ -0,0 +1,42 @@ +// 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/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 + default: + return grpc.Errorf(codes.Internal, err.Error()) + } +} From 8ee8d755bb81599cf57bda936230dd733e58b6ce Mon Sep 17 00:00:00 2001 From: Hitoshi Mitake Date: Mon, 28 Mar 2016 13:00:10 +0900 Subject: [PATCH 2/2] etcdserver: return internal error in a case of not auth specific errors --- auth/store.go | 7 +++++-- etcdserver/api/v3rpc/auth.go | 12 ++++++++++-- etcdserver/api/v3rpc/util.go | 3 +++ 3 files changed, 18 insertions(+), 4 deletions(-) diff --git a/auth/store.go b/auth/store.go index cdd1f9973..d7a239477 100644 --- a/auth/store.go +++ b/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{ diff --git a/etcdserver/api/v3rpc/auth.go b/etcdserver/api/v3rpc/auth.go index 170740e5d..f53f6cd21 100644 --- a/etcdserver/api/v3rpc/auth.go +++ b/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) { diff --git a/etcdserver/api/v3rpc/util.go b/etcdserver/api/v3rpc/util.go index 8780d71d6..2055e46f1 100644 --- a/etcdserver/api/v3rpc/util.go +++ b/etcdserver/api/v3rpc/util.go @@ -15,6 +15,7 @@ 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" @@ -36,6 +37,8 @@ func togRPCError(err error) error { return rpctypes.ErrRequestTooLarge case etcdserver.ErrNoSpace: return rpctypes.ErrNoSpace + case auth.ErrUserAlreadyExist: + return rpctypes.ErrUserAlreadyExist default: return grpc.Errorf(codes.Internal, err.Error()) }