From ac69e63fa89c90ab0cab5e6d5480c9f345e2f9ea Mon Sep 17 00:00:00 2001 From: Hitoshi Mitake Date: Wed, 12 Apr 2017 13:36:06 +0900 Subject: [PATCH 1/2] etcdserver: fill-in Auth API Header in apply layer Replacing "etcdserver: fill a response header in auth RPCs" The revision should be set at the time of "apply", not in later RPC layer. Fix https://github.com/coreos/etcd/issues/7691 Signed-off-by: Gyu-Ho Lee --- etcdserver/apply.go | 102 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 83 insertions(+), 19 deletions(-) diff --git a/etcdserver/apply.go b/etcdserver/apply.go index cfb7dfcb5..426f80195 100644 --- a/etcdserver/apply.go +++ b/etcdserver/apply.go @@ -486,15 +486,14 @@ func (a *applierV3backend) LeaseGrant(lc *pb.LeaseGrantRequest) (*pb.LeaseGrantR if err == nil { resp.ID = int64(l.ID) resp.TTL = l.TTL() - resp.Header = &pb.ResponseHeader{Revision: a.s.KV().Rev()} + resp.Header = newHeader(a.s) } - return resp, err } func (a *applierV3backend) LeaseRevoke(lc *pb.LeaseRevokeRequest) (*pb.LeaseRevokeResponse, error) { err := a.s.lessor.Revoke(lease.LeaseID(lc.ID)) - return &pb.LeaseRevokeResponse{Header: &pb.ResponseHeader{Revision: a.s.KV().Rev()}}, err + return &pb.LeaseRevokeResponse{Header: newHeader(a.s)}, err } func (a *applierV3backend) Alarm(ar *pb.AlarmRequest) (*pb.AlarmResponse, error) { @@ -575,69 +574,125 @@ func (a *applierV3backend) AuthEnable() (*pb.AuthEnableResponse, error) { if err != nil { return nil, err } - return &pb.AuthEnableResponse{}, nil + return &pb.AuthEnableResponse{Header: newHeader(a.s)}, nil } func (a *applierV3backend) AuthDisable() (*pb.AuthDisableResponse, error) { a.s.AuthStore().AuthDisable() - return &pb.AuthDisableResponse{}, nil + return &pb.AuthDisableResponse{Header: newHeader(a.s)}, nil } func (a *applierV3backend) Authenticate(r *pb.InternalAuthenticateRequest) (*pb.AuthenticateResponse, error) { ctx := context.WithValue(context.WithValue(a.s.ctx, "index", a.s.consistIndex.ConsistentIndex()), "simpleToken", r.SimpleToken) - return a.s.AuthStore().Authenticate(ctx, r.Name, r.Password) + resp, err := a.s.AuthStore().Authenticate(ctx, r.Name, r.Password) + if resp != nil { + resp.Header = newHeader(a.s) + } + return resp, err } func (a *applierV3backend) UserAdd(r *pb.AuthUserAddRequest) (*pb.AuthUserAddResponse, error) { - return a.s.AuthStore().UserAdd(r) + resp, err := a.s.AuthStore().UserAdd(r) + if resp != nil { + resp.Header = newHeader(a.s) + } + return resp, err } func (a *applierV3backend) UserDelete(r *pb.AuthUserDeleteRequest) (*pb.AuthUserDeleteResponse, error) { - return a.s.AuthStore().UserDelete(r) + resp, err := a.s.AuthStore().UserDelete(r) + if resp != nil { + resp.Header = newHeader(a.s) + } + return resp, err } func (a *applierV3backend) UserChangePassword(r *pb.AuthUserChangePasswordRequest) (*pb.AuthUserChangePasswordResponse, error) { - return a.s.AuthStore().UserChangePassword(r) + resp, err := a.s.AuthStore().UserChangePassword(r) + if resp != nil { + resp.Header = newHeader(a.s) + } + return resp, err } func (a *applierV3backend) UserGrantRole(r *pb.AuthUserGrantRoleRequest) (*pb.AuthUserGrantRoleResponse, error) { - return a.s.AuthStore().UserGrantRole(r) + resp, err := a.s.AuthStore().UserGrantRole(r) + if resp != nil { + resp.Header = newHeader(a.s) + } + return resp, err } func (a *applierV3backend) UserGet(r *pb.AuthUserGetRequest) (*pb.AuthUserGetResponse, error) { - return a.s.AuthStore().UserGet(r) + resp, err := a.s.AuthStore().UserGet(r) + if resp != nil { + resp.Header = newHeader(a.s) + } + return resp, err } func (a *applierV3backend) UserRevokeRole(r *pb.AuthUserRevokeRoleRequest) (*pb.AuthUserRevokeRoleResponse, error) { - return a.s.AuthStore().UserRevokeRole(r) + resp, err := a.s.AuthStore().UserRevokeRole(r) + if resp != nil { + resp.Header = newHeader(a.s) + } + return resp, err } func (a *applierV3backend) RoleAdd(r *pb.AuthRoleAddRequest) (*pb.AuthRoleAddResponse, error) { - return a.s.AuthStore().RoleAdd(r) + resp, err := a.s.AuthStore().RoleAdd(r) + if resp != nil { + resp.Header = newHeader(a.s) + } + return resp, err } func (a *applierV3backend) RoleGrantPermission(r *pb.AuthRoleGrantPermissionRequest) (*pb.AuthRoleGrantPermissionResponse, error) { - return a.s.AuthStore().RoleGrantPermission(r) + resp, err := a.s.AuthStore().RoleGrantPermission(r) + if resp != nil { + resp.Header = newHeader(a.s) + } + return resp, err } func (a *applierV3backend) RoleGet(r *pb.AuthRoleGetRequest) (*pb.AuthRoleGetResponse, error) { - return a.s.AuthStore().RoleGet(r) + resp, err := a.s.AuthStore().RoleGet(r) + if resp != nil { + resp.Header = newHeader(a.s) + } + return resp, err } func (a *applierV3backend) RoleRevokePermission(r *pb.AuthRoleRevokePermissionRequest) (*pb.AuthRoleRevokePermissionResponse, error) { - return a.s.AuthStore().RoleRevokePermission(r) + resp, err := a.s.AuthStore().RoleRevokePermission(r) + if resp != nil { + resp.Header = newHeader(a.s) + } + return resp, err } func (a *applierV3backend) RoleDelete(r *pb.AuthRoleDeleteRequest) (*pb.AuthRoleDeleteResponse, error) { - return a.s.AuthStore().RoleDelete(r) + resp, err := a.s.AuthStore().RoleDelete(r) + if resp != nil { + resp.Header = newHeader(a.s) + } + return resp, err } func (a *applierV3backend) UserList(r *pb.AuthUserListRequest) (*pb.AuthUserListResponse, error) { - return a.s.AuthStore().UserList(r) + resp, err := a.s.AuthStore().UserList(r) + if resp != nil { + resp.Header = newHeader(a.s) + } + return resp, err } func (a *applierV3backend) RoleList(r *pb.AuthRoleListRequest) (*pb.AuthRoleListResponse, error) { - return a.s.AuthStore().RoleList(r) + resp, err := a.s.AuthStore().RoleList(r) + if resp != nil { + resp.Header = newHeader(a.s) + } + return resp, err } type quotaApplierV3 struct { @@ -815,3 +870,12 @@ func pruneKVs(rr *mvcc.RangeResult, isPrunable func(*mvccpb.KeyValue) bool) { } rr.KVs = rr.KVs[:j] } + +func newHeader(s *EtcdServer) *pb.ResponseHeader { + return &pb.ResponseHeader{ + ClusterId: uint64(s.Cluster().ID()), + MemberId: uint64(s.ID()), + Revision: s.KV().Rev(), + RaftTerm: s.Term(), + } +} From 253e5a90bb8ebe43a1b0be9b57b0e46bb281d8c3 Mon Sep 17 00:00:00 2001 From: Gyu-Ho Lee Date: Mon, 17 Apr 2017 11:26:39 -0700 Subject: [PATCH 2/2] integration: test auth API response header revision Signed-off-by: Gyu-Ho Lee --- integration/v3_auth_test.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/integration/v3_auth_test.go b/integration/v3_auth_test.go index 0ade82db3..06ee68ec9 100644 --- a/integration/v3_auth_test.go +++ b/integration/v3_auth_test.go @@ -78,6 +78,32 @@ func TestV3AuthTokenWithDisable(t *testing.T) { <-donec } +func TestV3AuthRevision(t *testing.T) { + defer testutil.AfterTest(t) + clus := NewClusterV3(t, &ClusterConfig{Size: 1}) + defer clus.Terminate(t) + + api := toGRPC(clus.Client(0)) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + presp, perr := api.KV.Put(ctx, &pb.PutRequest{Key: []byte("foo"), Value: []byte("bar")}) + cancel() + if perr != nil { + t.Fatal(perr) + } + rev := presp.Header.Revision + + ctx, cancel = context.WithTimeout(context.Background(), 5*time.Second) + aresp, aerr := api.Auth.UserAdd(ctx, &pb.AuthUserAddRequest{Name: "root", Password: "123"}) + cancel() + if aerr != nil { + t.Fatal(aerr) + } + if aresp.Header.Revision != rev { + t.Fatalf("revision expected %d, got %d", rev, aresp.Header.Revision) + } +} + func authSetupRoot(t *testing.T, auth pb.AuthClient) { if _, err := auth.UserAdd(context.TODO(), &pb.AuthUserAddRequest{Name: "root", Password: "123"}); err != nil { t.Fatal(err)