From 280b65fe4d8edfe2130ea056c8511c15558d17b3 Mon Sep 17 00:00:00 2001 From: Hitoshi Mitake Date: Thu, 2 Feb 2017 13:34:59 +0900 Subject: [PATCH 1/2] auth: add a test case for recoverying from snapshot --- auth/store_test.go | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/auth/store_test.go b/auth/store_test.go index 72ea66b07..fc9e41ee2 100644 --- a/auth/store_test.go +++ b/auth/store_test.go @@ -496,6 +496,41 @@ func TestIsAdminPermitted(t *testing.T) { } } +func TestRecoverFromSnapshot(t *testing.T) { + as, _ := setupAuthStore(t) + + ua := &pb.AuthUserAddRequest{Name: "foo"} + _, err := as.UserAdd(ua) // add an existing user + if err == nil { + t.Fatalf("expected %v, got %v", ErrUserAlreadyExist, err) + } + if err != ErrUserAlreadyExist { + t.Fatalf("expected %v, got %v", ErrUserAlreadyExist, err) + } + + ua = &pb.AuthUserAddRequest{Name: ""} + _, err = as.UserAdd(ua) // add a user with empty name + if err != ErrUserEmpty { + t.Fatal(err) + } + + as.Close() + + as2 := NewAuthStore(as.be, dummyIndexWaiter) + + if !as2.isAuthEnabled() { + t.Fatal("recovering authStore from existing backend failed") + } + + ul, err := as.UserList(&pb.AuthUserListRequest{}) + if err != nil { + t.Fatal(err) + } + if !contains(ul.Users, "root") { + t.Errorf("expected %v in %v", "root", ul.Users) + } +} + func contains(array []string, str string) bool { for _, s := range array { if s == str { From 9976d869c132c60fb3641d4a11d1621e5b5fdb9a Mon Sep 17 00:00:00 2001 From: Hitoshi Mitake Date: Wed, 1 Feb 2017 17:47:09 +0900 Subject: [PATCH 2/2] auth: correct initialization in NewAuthStore() Because of my own silly mistake, current NewAuthStore() doesn't initialize authStore in a correct manner. For example, after recovery from snapshot, it cannot revive the flag of enabled/disabled. This commit fixes the problem. Fix https://github.com/coreos/etcd/issues/7165 --- auth/store.go | 46 ++++++++++++++++++++++++++++++++-------------- auth/store_test.go | 3 +++ 2 files changed, 35 insertions(+), 14 deletions(-) diff --git a/auth/store.go b/auth/store.go index 6b2c6c923..6d218d313 100644 --- a/auth/store.go +++ b/auth/store.go @@ -182,6 +182,17 @@ type authStore struct { indexWaiter func(uint64) <-chan struct{} } +func newDeleterFunc(as *authStore) func(string) { + return func(t string) { + as.simpleTokensMu.Lock() + defer as.simpleTokensMu.Unlock() + if username, ok := as.simpleTokens[t]; ok { + plog.Infof("deleting token %s for user %s", t, username) + delete(as.simpleTokens, t) + } + } +} + func (as *authStore) AuthEnable() error { as.enabledMu.Lock() defer as.enabledMu.Unlock() @@ -210,15 +221,7 @@ func (as *authStore) AuthEnable() error { as.enabled = true - tokenDeleteFunc := func(t string) { - as.simpleTokensMu.Lock() - defer as.simpleTokensMu.Unlock() - if username, ok := as.simpleTokens[t]; ok { - plog.Infof("deleting token %s for user %s", t, username) - delete(as.simpleTokens, t) - } - } - as.simpleTokenKeeper = NewSimpleTokenTTLKeeper(tokenDeleteFunc) + as.simpleTokenKeeper = NewSimpleTokenTTLKeeper(newDeleterFunc(as)) as.rangePermCache = make(map[string]*unifiedRangePermissions) @@ -892,11 +895,25 @@ func NewAuthStore(be backend.Backend, indexWaiter func(uint64) <-chan struct{}) tx.UnsafeCreateBucket(authUsersBucketName) tx.UnsafeCreateBucket(authRolesBucketName) + enabled := false + _, vs := tx.UnsafeRange(authBucketName, enableFlagKey, nil, 0) + if len(vs) == 1 { + if bytes.Equal(vs[0], authEnabled) { + enabled = true + } + } + as := &authStore{ - be: be, - simpleTokens: make(map[string]string), - revision: 0, - indexWaiter: indexWaiter, + be: be, + simpleTokens: make(map[string]string), + revision: getRevision(tx), + indexWaiter: indexWaiter, + enabled: enabled, + rangePermCache: make(map[string]*unifiedRangePermissions), + } + + if enabled { + as.simpleTokenKeeper = NewSimpleTokenTTLKeeper(newDeleterFunc(as)) } as.commitRevision(tx) @@ -926,7 +943,8 @@ func (as *authStore) commitRevision(tx backend.BatchTx) { func getRevision(tx backend.BatchTx) uint64 { _, vs := tx.UnsafeRange(authBucketName, []byte(revisionKey), nil, 0) if len(vs) != 1 { - plog.Panicf("failed to get the key of auth store revision") + // this can happen in the initialization phase + return 0 } return binary.BigEndian.Uint64(vs[0]) diff --git a/auth/store_test.go b/auth/store_test.go index fc9e41ee2..3bc3a0014 100644 --- a/auth/store_test.go +++ b/auth/store_test.go @@ -517,6 +517,9 @@ func TestRecoverFromSnapshot(t *testing.T) { as.Close() as2 := NewAuthStore(as.be, dummyIndexWaiter) + defer func(a *authStore) { + a.Close() + }(as2) if !as2.isAuthEnabled() { t.Fatal("recovering authStore from existing backend failed")