Remove explicit authStore->ConsistencyIndex updates, as they are taken care by hook.

This commit is contained in:
Piotr Tabor 2021-04-12 19:10:13 +02:00
parent 50051675f9
commit fe3254aee3
4 changed files with 20 additions and 41 deletions

View File

@ -29,7 +29,6 @@ import (
"go.etcd.io/etcd/api/v3/authpb" "go.etcd.io/etcd/api/v3/authpb"
pb "go.etcd.io/etcd/api/v3/etcdserverpb" pb "go.etcd.io/etcd/api/v3/etcdserverpb"
"go.etcd.io/etcd/api/v3/v3rpc/rpctypes" "go.etcd.io/etcd/api/v3/v3rpc/rpctypes"
"go.etcd.io/etcd/server/v3/etcdserver/cindex"
"go.etcd.io/etcd/server/v3/mvcc/backend" "go.etcd.io/etcd/server/v3/mvcc/backend"
"go.uber.org/zap" "go.uber.org/zap"
@ -215,7 +214,6 @@ type authStore struct {
tokenProvider TokenProvider tokenProvider TokenProvider
bcryptCost int // the algorithm cost / strength for hashing auth passwords bcryptCost int // the algorithm cost / strength for hashing auth passwords
ci cindex.ConsistentIndexer
} }
func (as *authStore) AuthEnable() error { func (as *authStore) AuthEnable() error {
@ -266,7 +264,6 @@ func (as *authStore) AuthDisable() {
tx.Lock() tx.Lock()
tx.UnsafePut(authBucketName, enableFlagKey, authDisabled) tx.UnsafePut(authBucketName, enableFlagKey, authDisabled)
as.commitRevision(tx) as.commitRevision(tx)
as.saveConsistentIndex(tx)
tx.Unlock() tx.Unlock()
b.ForceCommit() b.ForceCommit()
@ -424,7 +421,6 @@ func (as *authStore) UserAdd(r *pb.AuthUserAddRequest) (*pb.AuthUserAddResponse,
putUser(as.lg, tx, newUser) putUser(as.lg, tx, newUser)
as.commitRevision(tx) as.commitRevision(tx)
as.saveConsistentIndex(tx)
as.lg.Info("added a user", zap.String("user-name", r.Name)) as.lg.Info("added a user", zap.String("user-name", r.Name))
return &pb.AuthUserAddResponse{}, nil return &pb.AuthUserAddResponse{}, nil
@ -448,7 +444,6 @@ func (as *authStore) UserDelete(r *pb.AuthUserDeleteRequest) (*pb.AuthUserDelete
delUser(tx, r.Name) delUser(tx, r.Name)
as.commitRevision(tx) as.commitRevision(tx)
as.saveConsistentIndex(tx)
as.invalidateCachedPerm(r.Name) as.invalidateCachedPerm(r.Name)
as.tokenProvider.invalidateUser(r.Name) as.tokenProvider.invalidateUser(r.Name)
@ -491,7 +486,6 @@ func (as *authStore) UserChangePassword(r *pb.AuthUserChangePasswordRequest) (*p
putUser(as.lg, tx, updatedUser) putUser(as.lg, tx, updatedUser)
as.commitRevision(tx) as.commitRevision(tx)
as.saveConsistentIndex(tx)
as.invalidateCachedPerm(r.Name) as.invalidateCachedPerm(r.Name)
as.tokenProvider.invalidateUser(r.Name) as.tokenProvider.invalidateUser(r.Name)
@ -540,7 +534,6 @@ func (as *authStore) UserGrantRole(r *pb.AuthUserGrantRoleRequest) (*pb.AuthUser
as.invalidateCachedPerm(r.User) as.invalidateCachedPerm(r.User)
as.commitRevision(tx) as.commitRevision(tx)
as.saveConsistentIndex(tx)
as.lg.Info( as.lg.Info(
"granted a role to a user", "granted a role to a user",
@ -619,7 +612,6 @@ func (as *authStore) UserRevokeRole(r *pb.AuthUserRevokeRoleRequest) (*pb.AuthUs
as.invalidateCachedPerm(r.Name) as.invalidateCachedPerm(r.Name)
as.commitRevision(tx) as.commitRevision(tx)
as.saveConsistentIndex(tx)
as.lg.Info( as.lg.Info(
"revoked a role from a user", "revoked a role from a user",
@ -690,7 +682,6 @@ func (as *authStore) RoleRevokePermission(r *pb.AuthRoleRevokePermissionRequest)
as.clearCachedPerm() as.clearCachedPerm()
as.commitRevision(tx) as.commitRevision(tx)
as.saveConsistentIndex(tx)
as.lg.Info( as.lg.Info(
"revoked a permission on range", "revoked a permission on range",
@ -742,7 +733,6 @@ func (as *authStore) RoleDelete(r *pb.AuthRoleDeleteRequest) (*pb.AuthRoleDelete
} }
as.commitRevision(tx) as.commitRevision(tx)
as.saveConsistentIndex(tx)
as.lg.Info("deleted a role", zap.String("role-name", r.Role)) as.lg.Info("deleted a role", zap.String("role-name", r.Role))
return &pb.AuthRoleDeleteResponse{}, nil return &pb.AuthRoleDeleteResponse{}, nil
@ -769,7 +759,6 @@ func (as *authStore) RoleAdd(r *pb.AuthRoleAddRequest) (*pb.AuthRoleAddResponse,
putRole(as.lg, tx, newRole) putRole(as.lg, tx, newRole)
as.commitRevision(tx) as.commitRevision(tx)
as.saveConsistentIndex(tx)
as.lg.Info("created a role", zap.String("role-name", r.Name)) as.lg.Info("created a role", zap.String("role-name", r.Name))
return &pb.AuthRoleAddResponse{}, nil return &pb.AuthRoleAddResponse{}, nil
@ -829,7 +818,6 @@ func (as *authStore) RoleGrantPermission(r *pb.AuthRoleGrantPermissionRequest) (
as.clearCachedPerm() as.clearCachedPerm()
as.commitRevision(tx) as.commitRevision(tx)
as.saveConsistentIndex(tx)
as.lg.Info( as.lg.Info(
"granted/updated a permission to a user", "granted/updated a permission to a user",
@ -1021,7 +1009,7 @@ func (as *authStore) IsAuthEnabled() bool {
} }
// NewAuthStore creates a new AuthStore. // NewAuthStore creates a new AuthStore.
func NewAuthStore(lg *zap.Logger, be backend.Backend, ci cindex.ConsistentIndexer, tp TokenProvider, bcryptCost int) *authStore { func NewAuthStore(lg *zap.Logger, be backend.Backend, tp TokenProvider, bcryptCost int) *authStore {
if lg == nil { if lg == nil {
lg = zap.NewNop() lg = zap.NewNop()
} }
@ -1056,7 +1044,6 @@ func NewAuthStore(lg *zap.Logger, be backend.Backend, ci cindex.ConsistentIndexe
revision: getRevision(tx), revision: getRevision(tx),
lg: lg, lg: lg,
be: be, be: be,
ci: ci,
enabled: enabled, enabled: enabled,
rangePermCache: make(map[string]*unifiedRangePermissions), rangePermCache: make(map[string]*unifiedRangePermissions),
tokenProvider: tp, tokenProvider: tp,
@ -1317,14 +1304,6 @@ func (as *authStore) BcryptCost() int {
return as.bcryptCost return as.bcryptCost
} }
func (as *authStore) saveConsistentIndex(tx backend.BatchTx) {
if as.ci != nil {
as.ci.UnsafeSave(tx)
} else {
as.lg.Error("failed to save consistentIndex,consistentIndexer is nil")
}
}
func (as *authStore) setupMetricsReporter() { func (as *authStore) setupMetricsReporter() {
reportCurrentAuthRevMu.Lock() reportCurrentAuthRevMu.Lock()
reportCurrentAuthRev = func() float64 { reportCurrentAuthRev = func() float64 {

View File

@ -52,7 +52,7 @@ func TestNewAuthStoreRevision(t *testing.T) {
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
as := NewAuthStore(zap.NewExample(), b, nil, tp, bcrypt.MinCost) as := NewAuthStore(zap.NewExample(), b, tp, bcrypt.MinCost)
err = enableAuthAndCreateRoot(as) err = enableAuthAndCreateRoot(as)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
@ -64,7 +64,7 @@ func TestNewAuthStoreRevision(t *testing.T) {
// no changes to commit // no changes to commit
b2 := backend.NewDefaultBackend(tPath) b2 := backend.NewDefaultBackend(tPath)
defer b2.Close() defer b2.Close()
as = NewAuthStore(zap.NewExample(), b2, nil, tp, bcrypt.MinCost) as = NewAuthStore(zap.NewExample(), b2, tp, bcrypt.MinCost)
defer as.Close() defer as.Close()
new := as.Revision() new := as.Revision()
@ -85,7 +85,7 @@ func TestNewAuthStoreBcryptCost(t *testing.T) {
invalidCosts := [2]int{bcrypt.MinCost - 1, bcrypt.MaxCost + 1} invalidCosts := [2]int{bcrypt.MinCost - 1, bcrypt.MaxCost + 1}
for _, invalidCost := range invalidCosts { for _, invalidCost := range invalidCosts {
as := NewAuthStore(zap.NewExample(), b, nil, tp, invalidCost) as := NewAuthStore(zap.NewExample(), b, tp, invalidCost)
defer as.Close() defer as.Close()
if as.BcryptCost() != bcrypt.DefaultCost { if as.BcryptCost() != bcrypt.DefaultCost {
t.Fatalf("expected DefaultCost when bcryptcost is invalid") t.Fatalf("expected DefaultCost when bcryptcost is invalid")
@ -105,7 +105,7 @@ func setupAuthStore(t *testing.T) (store *authStore, teardownfunc func(t *testin
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
as := NewAuthStore(zap.NewExample(), b, nil, tp, bcrypt.MinCost) as := NewAuthStore(zap.NewExample(), b, tp, bcrypt.MinCost)
err = enableAuthAndCreateRoot(as) err = enableAuthAndCreateRoot(as)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
@ -657,7 +657,7 @@ func TestAuthInfoFromCtxRace(t *testing.T) {
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
as := NewAuthStore(zap.NewExample(), b, nil, tp, bcrypt.MinCost) as := NewAuthStore(zap.NewExample(), b, tp, bcrypt.MinCost)
defer as.Close() defer as.Close()
donec := make(chan struct{}) donec := make(chan struct{})
@ -730,7 +730,7 @@ func TestRecoverFromSnapshot(t *testing.T) {
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
as2 := NewAuthStore(zap.NewExample(), as.be, nil, tp, bcrypt.MinCost) as2 := NewAuthStore(zap.NewExample(), as.be, tp, bcrypt.MinCost)
defer as2.Close() defer as2.Close()
if !as2.IsAuthEnabled() { if !as2.IsAuthEnabled() {
@ -811,7 +811,7 @@ func TestRolesOrder(t *testing.T) {
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
as := NewAuthStore(zap.NewExample(), b, nil, tp, bcrypt.MinCost) as := NewAuthStore(zap.NewExample(), b, tp, bcrypt.MinCost)
defer as.Close() defer as.Close()
err = enableAuthAndCreateRoot(as) err = enableAuthAndCreateRoot(as)
if err != nil { if err != nil {
@ -867,7 +867,7 @@ func testAuthInfoFromCtxWithRoot(t *testing.T, opts string) {
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
as := NewAuthStore(zap.NewExample(), b, nil, tp, bcrypt.MinCost) as := NewAuthStore(zap.NewExample(), b, tp, bcrypt.MinCost)
defer as.Close() defer as.Close()
if err = enableAuthAndCreateRoot(as); err != nil { if err = enableAuthAndCreateRoot(as); err != nil {

View File

@ -598,7 +598,7 @@ func NewServer(cfg config.ServerConfig) (srv *EtcdServer, err error) {
} }
} }
srv.authStore = auth.NewAuthStore(srv.Logger(), srv.be, srv.consistIndex, tp, int(cfg.BcryptCost)) srv.authStore = auth.NewAuthStore(srv.Logger(), srv.be, tp, int(cfg.BcryptCost))
newSrv := srv // since srv == nil in defer if srv is returned as nil newSrv := srv // since srv == nil in defer if srv is returned as nil
defer func() { defer func() {

View File

@ -989,9 +989,9 @@ func TestSnapshot(t *testing.T) {
lg: zap.NewExample(), lg: zap.NewExample(),
r: *r, r: *r,
v2store: st, v2store: st,
consistIndex: cindex.NewConsistentIndex(be.BatchTx()), consistIndex: cindex.NewConsistentIndex(be),
} }
srv.kv = mvcc.New(zap.NewExample(), be, &lease.FakeLessor{}, srv.consistIndex, mvcc.StoreConfig{}) srv.kv = mvcc.New(zap.NewExample(), be, &lease.FakeLessor{}, mvcc.StoreConfig{})
srv.be = be srv.be = be
ch := make(chan struct{}, 2) ch := make(chan struct{}, 2)
@ -1074,11 +1074,11 @@ func TestSnapshotOrdering(t *testing.T) {
snapshotter: snap.New(zap.NewExample(), snapdir), snapshotter: snap.New(zap.NewExample(), snapdir),
cluster: cl, cluster: cl,
SyncTicker: &time.Ticker{}, SyncTicker: &time.Ticker{},
consistIndex: cindex.NewConsistentIndex(be.BatchTx()), consistIndex: cindex.NewConsistentIndex(be),
} }
s.applyV2 = &applierV2store{store: s.v2store, cluster: s.cluster} s.applyV2 = &applierV2store{store: s.v2store, cluster: s.cluster}
s.kv = mvcc.New(zap.NewExample(), be, &lease.FakeLessor{}, s.consistIndex, mvcc.StoreConfig{}) s.kv = mvcc.New(zap.NewExample(), be, &lease.FakeLessor{}, mvcc.StoreConfig{})
s.be = be s.be = be
s.start() s.start()
@ -1148,11 +1148,11 @@ func TestTriggerSnap(t *testing.T) {
v2store: st, v2store: st,
reqIDGen: idutil.NewGenerator(0, time.Time{}), reqIDGen: idutil.NewGenerator(0, time.Time{}),
SyncTicker: &time.Ticker{}, SyncTicker: &time.Ticker{},
consistIndex: cindex.NewConsistentIndex(be.BatchTx()), consistIndex: cindex.NewConsistentIndex(be),
} }
srv.applyV2 = &applierV2store{store: srv.v2store, cluster: srv.cluster} srv.applyV2 = &applierV2store{store: srv.v2store, cluster: srv.cluster}
srv.kv = mvcc.New(zap.NewExample(), be, &lease.FakeLessor{}, srv.consistIndex, mvcc.StoreConfig{}) srv.kv = mvcc.New(zap.NewExample(), be, &lease.FakeLessor{}, mvcc.StoreConfig{})
srv.be = be srv.be = be
srv.start() srv.start()
@ -1227,11 +1227,11 @@ func TestConcurrentApplyAndSnapshotV3(t *testing.T) {
snapshotter: snap.New(zap.NewExample(), testdir), snapshotter: snap.New(zap.NewExample(), testdir),
cluster: cl, cluster: cl,
SyncTicker: &time.Ticker{}, SyncTicker: &time.Ticker{},
consistIndex: cindex.NewConsistentIndex(be.BatchTx()), consistIndex: cindex.NewConsistentIndex(be),
} }
s.applyV2 = &applierV2store{store: s.v2store, cluster: s.cluster} s.applyV2 = &applierV2store{store: s.v2store, cluster: s.cluster}
s.kv = mvcc.New(zap.NewExample(), be, &lease.FakeLessor{}, s.consistIndex, mvcc.StoreConfig{}) s.kv = mvcc.New(zap.NewExample(), be, &lease.FakeLessor{}, mvcc.StoreConfig{})
s.be = be s.be = be
s.start() s.start()
@ -1562,7 +1562,7 @@ func TestPublishV3(t *testing.T) {
w: w, w: w,
reqIDGen: idutil.NewGenerator(0, time.Time{}), reqIDGen: idutil.NewGenerator(0, time.Time{}),
SyncTicker: &time.Ticker{}, SyncTicker: &time.Ticker{},
authStore: auth.NewAuthStore(lg, be, nil, nil, 0), authStore: auth.NewAuthStore(lg, be, nil, 0),
be: be, be: be,
ctx: ctx, ctx: ctx,
cancel: cancel, cancel: cancel,
@ -1633,7 +1633,7 @@ func TestPublishV3Retry(t *testing.T) {
cluster: &membership.RaftCluster{}, cluster: &membership.RaftCluster{},
reqIDGen: idutil.NewGenerator(0, time.Time{}), reqIDGen: idutil.NewGenerator(0, time.Time{}),
SyncTicker: &time.Ticker{}, SyncTicker: &time.Ticker{},
authStore: auth.NewAuthStore(lg, be, nil, nil, 0), authStore: auth.NewAuthStore(lg, be, nil, 0),
be: be, be: be,
ctx: ctx, ctx: ctx,
cancel: cancel, cancel: cancel,