version: bump to v2.3.5

Fixes #5374

Documentation/admin_guide.md

@@ -135,7 +135,7 @@ The data directory contains all the data to recover a member to its point-in-tim
 
 * Stop the member process.
 * Copy the data directory of the now-idle member to the new machine.
-* Update the peer URLs for the replaced member to reflect the new machine according to the [runtime reconfiguration instructions][update-member].
+* Update the peer URLs for the replaced member to reflect the new machine according to the [runtime reconfiguration instructions][update-a-member].
 * Start etcd on the new machine, using the same configuration and the copy of the data directory.
 
 This example will walk you through the process of migrating the infra1 member to a new machine:
@@ -217,12 +217,14 @@ To recover from such scenarios, etcd provides functionality to backup and restor
 
 **NB:** Windows users must stop etcd before running the backup command.
 
-The first step of the recovery is to backup the data directory on a functioning etcd node. To do this, use the `etcdctl backup` command, passing in the original data directory used by etcd. For example:
+The first step of the recovery is to backup the data directory and wal directory, if stored separately, on a functioning etcd node. To do this, use the `etcdctl backup` command, passing in the original data (and wal) directory used by etcd. For example:
 
 ```sh
     etcdctl backup \
       --data-dir %data_dir% \
+      [--wal-dir %wal_dir%] \
       --backup-dir %backup_data_dir%
+      [--backup-wal-dir %backup_wal_dir%]
 ```
 
 This command will rewrite some of the metadata contained in the backup (specifically, the node ID and cluster ID), which means that the node will lose its former identity. In order to recreate a cluster from the backup, you will need to start a new, single-node cluster. The metadata is rewritten to prevent the new node from inadvertently being joined onto an existing cluster.
@@ -234,26 +236,30 @@ To restore a backup using the procedure created above, start etcd with the `-for
 ```sh
     etcd \
       -data-dir=%backup_data_dir% \
+      [-wal-dir=%backup_wal_dir%] \
       -force-new-cluster \
       ...
 ```
 
 Now etcd should be available on this node and serving the original datastore.
 
-Once you have verified that etcd has started successfully, shut it down and move the data back to the previous location (you may wish to make another copy as well to be safe):
+Once you have verified that etcd has started successfully, shut it down and move the data and wal, if stored separately, back to the previous location (you may wish to make another copy as well to be safe):
 
 ```sh
     pkill etcd
     rm -fr %data_dir%
+    rm -fr %wal_dir%
     mv %backup_data_dir% %data_dir%
+    mv %backup_wal_dir% %wal_dir%
     etcd \
       -data-dir=%data_dir% \
+      [-wal-dir=%wal_dir%] \
       ...
 ```
 
 #### Restoring the cluster
 
-Now that the node is running successfully, [change its advertised peer URLs][update-member], as the `--force-new-cluster` option has set the peer URL to the default listening on localhost.
+Now that the node is running successfully, [change its advertised peer URLs][update-a-member], as the `--force-new-cluster` option has set the peer URL to the default listening on localhost.
 
 You can then add more nodes to the cluster and restore resiliency. See the [add a new member][add-a-member] guide for more details. **NB:** If you are trying to restore your cluster using old failed etcd nodes, please make sure you have stopped old etcd instances and removed their old data directories specified by the data-dir configuration parameter.
 

Documentation/auth_api.md

@@ -145,8 +145,8 @@ GET/HEAD  /v2/auth/users
                   "role": "root",
                   "permissions": {
                     "kv": {
-                      "read": ["*"],
-                      "write": ["*"]
+                      "read": ["/*"],
+                      "write": ["/*"]
                     }
                   }
                 }
@@ -159,8 +159,8 @@ GET/HEAD  /v2/auth/users
                   "role": "guest",
                   "permissions": {
                     "kv": {
-                      "read": ["*"],
-                      "write": ["*"]
+                      "read": ["/*"],
+                      "write": ["/*"]
                     }
                   }
                 }
@@ -198,8 +198,8 @@ GET/HEAD  /v2/auth/users/alice
               "role": "etcd",
               "permissions" : {
                 "kv" : {
-                  "read": [ "*" ],
-                  "write": [ "*" ]
+                  "read": [ "/*" ],
+                  "write": [ "/*" ]
                 }
               }
             }
@@ -311,8 +311,8 @@ GET/HEAD  /v2/auth/roles
               "role": "etcd",
               "permissions": {
                 "kv": {
-                  "read": ["*"],
-                  "write": ["*"]
+                  "read": ["/*"],
+                  "write": ["/*"]
                 }
               }
             },
@@ -320,8 +320,8 @@ GET/HEAD  /v2/auth/roles
               "role": "quay",
               "permissions": {
                 "kv": {
-                  "read": ["*"],
-                  "write": ["*"]
+                  "read": ["/*"],
+                  "write": ["/*"]
                 }
               }
             }
@@ -393,7 +393,7 @@ PUT  /v2/auth/roles/guest
           "revoke" : {
             "kv" : {
               "write": [
-                "*"
+                "/*"
               ]
             }
           }

README.md

@@ -118,6 +118,17 @@ See [CONTRIBUTING](CONTRIBUTING.md) for details on submitting patches and the co
 
 See [reporting bugs](Documentation/reporting_bugs.md) for details about reporting any issue you may encounter.
 
+## Known bugs
+
+[GH518](https://github.com/coreos/etcd/issues/518) is a known bug. Issue is that:
+
+```
+curl http://127.0.0.1:2379/v2/keys/foo -XPUT -d value=bar
+curl http://127.0.0.1:2379/v2/keys/foo -XPUT -d dir=true -d prevExist=true
+```
+
+If the previous node is a key and client tries to overwrite it with `dir=true`, it does not give warnings such as `Not a directory`. Instead, the key is set to empty value.
+
 ## Project Details
 
 ### Versioning
@@ -137,12 +148,15 @@ curl -L http://127.0.0.1:2379/version
 
 The `v2` API responses should not change after the 2.0.0 release but new features will be added over time.
 
-#### 32-bit systems
+#### 32-bit and other unsupported systems
 
 etcd has known issues on 32-bit systems due to a bug in the Go runtime. See #[358][358] for more information.
 
-To avoid inadvertantly producing an unstable etcd server, 32-bit builds emit an `etcd` that prints
-a warning message and immediately exits.
+To avoid inadvertantly running a possibly unstable etcd server, `etcd` on unsupported architectures will print
+a warning message and immediately exit if the environment variable `ETCD_UNSUPPORTED_ARCH` is not set to
+the target architecture.
+
+Currently only the amd64 architecture is officially supported by `etcd`.
 
 [358]: https://github.com/coreos/etcd/issues/358
 

client/auth_user.go

@@ -36,6 +36,12 @@ type User struct {
 	Revoke   []string `json:"revoke,omitempty"`
 }
 
+// userListEntry is the user representation given by the server for ListUsers
+type userListEntry struct {
+	User  string `json:"user"`
+	Roles []Role `json:"roles"`
+}
+
 type UserRoles struct {
 	User  string `json:"user"`
 	Roles []Role `json:"roles"`
@@ -198,7 +204,7 @@ func (u *httpAuthUserAPI) ListUsers(ctx context.Context) ([]string, error) {
 	}
 
 	var userList struct {
-		Users []User `json:"users"`
+		Users []userListEntry `json:"users"`
 	}
 
 	if err = json.Unmarshal(body, &userList); err != nil {

client/client.go

@@ -342,7 +342,9 @@ func (c *httpClusterClient) Do(ctx context.Context, act httpAction) (*http.Respo
 		resp, body, err = hc.Do(ctx, action)
 		if err != nil {
 			cerr.Errors = append(cerr.Errors, err)
-			// mask previous errors with context error, which is controlled by user
+			if err == ctx.Err() {
+				return nil, nil, ctx.Err()
+			}
 			if err == context.Canceled || err == context.DeadlineExceeded {
 				return nil, nil, err
 			}

client/client_test.go

@@ -295,7 +295,7 @@ func TestSimpleHTTPClientDoHeaderTimeout(t *testing.T) {
 			t.Fatalf("expected non-nil error, got nil")
 		}
 	case <-time.After(time.Second):
-		t.Fatalf("unexpected timeout when waitting for the test to finish")
+		t.Fatalf("unexpected timeout when waiting for the test to finish")
 	}
 }
 
@@ -444,7 +444,51 @@ func TestHTTPClusterClientDoDeadlineExceedContext(t *testing.T) {
 			t.Errorf("err = %+v, want %+v", err, context.DeadlineExceeded)
 		}
 	case <-time.After(time.Second):
-		t.Fatalf("unexpected timeout when waitting for request to deadline exceed")
+		t.Fatalf("unexpected timeout when waiting for request to deadline exceed")
+	}
+}
+
+type fakeCancelContext struct{}
+
+var fakeCancelContextError = errors.New("fake context canceled")
+
+func (f fakeCancelContext) Deadline() (time.Time, bool) { return time.Time{}, false }
+func (f fakeCancelContext) Done() <-chan struct{} {
+	d := make(chan struct{}, 1)
+	d <- struct{}{}
+	return d
+}
+func (f fakeCancelContext) Err() error                        { return fakeCancelContextError }
+func (f fakeCancelContext) Value(key interface{}) interface{} { return 1 }
+
+func withTimeout(parent context.Context, timeout time.Duration) (context.Context, context.CancelFunc) {
+	return parent, func() { parent = nil }
+}
+
+func TestHTTPClusterClientDoCanceledContext(t *testing.T) {
+	fakeURL := url.URL{}
+	tr := newFakeTransport()
+	tr.finishCancel <- struct{}{}
+	c := &httpClusterClient{
+		clientFactory: newHTTPClientFactory(tr, DefaultCheckRedirect, 0),
+		endpoints:     []url.URL{fakeURL},
+	}
+
+	errc := make(chan error)
+	go func() {
+		ctx, cancel := withTimeout(fakeCancelContext{}, time.Millisecond)
+		cancel()
+		_, _, err := c.Do(ctx, &fakeAction{})
+		errc <- err
+	}()
+
+	select {
+	case err := <-errc:
+		if err != fakeCancelContextError {
+			t.Errorf("err = %+v, want %+v", err, fakeCancelContextError)
+		}
+	case <-time.After(time.Second):
+		t.Fatalf("unexpected timeout when waiting for request to fake context canceled")
 	}
 }
 

e2e/etcdctl_test.go

@@ -185,9 +185,18 @@ func testCtlV2GetRoleUser(t *testing.T, cfg *etcdProcessClusterConfig) {
 	if err := etcdctlUserGet(epc, "username"); err != nil {
 		t.Fatalf("failed to get user (%v)", err)
 	}
+	// ensure double grant gives an error; was crashing in 2.3.1
+	regrantArgs := etcdctlPrefixArgs(epc)
+	regrantArgs = append(regrantArgs, "user", "grant", "--roles", "foo", "username")
+	if err := spawnWithExpect(regrantArgs, "duplicate"); err != nil {
+		t.Fatalf("missing duplicate error on double grant role (%v)", err)
+	}
 }
 
-func TestCtlV2UserList(t *testing.T) {
+func TestCtlV2UserListUsername(t *testing.T) { testCtlV2UserList(t, "username") }
+func TestCtlV2UserListRoot(t *testing.T)     { testCtlV2UserList(t, "root") }
+
+func testCtlV2UserList(t *testing.T, username string) {
 	defer testutil.AfterTest(t)
 
 	epc := setupEtcdctlTest(t, &configWithProxy, false)
@@ -197,10 +206,10 @@ func TestCtlV2UserList(t *testing.T) {
 		}
 	}()
 
-	if err := etcdctlUserAdd(epc, "username", "password"); err != nil {
+	if err := etcdctlUserAdd(epc, username, "password"); err != nil {
 		t.Fatalf("failed to add user (%v)", err)
 	}
-	if err := etcdctlUserList(epc, "username"); err != nil {
+	if err := etcdctlUserList(epc, username); err != nil {
 		t.Fatalf("failed to list users (%v)", err)
 	}
 }

etcdctl/command/backup_command.go

@@ -37,7 +37,9 @@ func NewBackupCommand() cli.Command {
 		ArgsUsage: " ",
 		Flags: []cli.Flag{
 			cli.StringFlag{Name: "data-dir", Value: "", Usage: "Path to the etcd data dir"},
+			cli.StringFlag{Name: "wal-dir", Value: "", Usage: "Path to the etcd wal dir"},
 			cli.StringFlag{Name: "backup-dir", Value: "", Usage: "Path to the backup dir"},
+			cli.StringFlag{Name: "backup-wal-dir", Value: "", Usage: "Path to the backup wal dir"},
 		},
 		Action: handleBackup,
 	}
@@ -45,10 +47,23 @@ func NewBackupCommand() cli.Command {
 
 // handleBackup handles a request that intends to do a backup.
 func handleBackup(c *cli.Context) {
+	var srcWAL string
+	var destWAL string
+
 	srcSnap := path.Join(c.String("data-dir"), "member", "snap")
 	destSnap := path.Join(c.String("backup-dir"), "member", "snap")
-	srcWAL := path.Join(c.String("data-dir"), "member", "wal")
-	destWAL := path.Join(c.String("backup-dir"), "member", "wal")
+
+	if c.String("wal-dir") != "" {
+		srcWAL = c.String("wal-dir")
+	} else {
+		srcWAL = path.Join(c.String("data-dir"), "member", "wal")
+	}
+
+	if c.String("backup-wal-dir") != "" {
+		destWAL = c.String("backup-wal-dir")
+	} else {
+		destWAL = path.Join(c.String("backup-dir"), "member", "wal")
+	}
 
 	if err := os.MkdirAll(destSnap, 0700); err != nil {
 		log.Fatalf("failed creating backup snapshot dir %v: %v", destSnap, err)

etcdctl/command/user_commands.go

@@ -17,8 +17,6 @@ package command
 import (
 	"fmt"
 	"os"
-	"reflect"
-	"sort"
 	"strings"
 
 	"github.com/coreos/etcd/Godeps/_workspace/src/github.com/bgentry/speakeasy"
@@ -195,21 +193,12 @@ func userGrantRevoke(c *cli.Context, grant bool) {
 		os.Exit(1)
 	}
 
-	var newUser *client.User
 	if grant {
-		newUser, err = api.GrantUser(ctx, user, roles)
+		_, err = api.GrantUser(ctx, user, roles)
 	} else {
-		newUser, err = api.RevokeUser(ctx, user, roles)
-	}
-	sort.Strings(newUser.Roles)
-	sort.Strings(currentUser.Roles)
-	if reflect.DeepEqual(newUser.Roles, currentUser.Roles) {
-		if grant {
-			fmt.Printf("User unchanged; roles already granted")
-		} else {
-			fmt.Printf("User unchanged; roles already revoked")
-		}
+		_, err = api.RevokeUser(ctx, user, roles)
 	}
+
 	if err != nil {
 		fmt.Fprintln(os.Stderr, err.Error())
 		os.Exit(1)

etcdctl/command/util.go

@@ -168,7 +168,13 @@ func getTransport(c *cli.Context) (*http.Transport, error) {
 		CertFile: certfile,
 		KeyFile:  keyfile,
 	}
-	return transport.NewTransport(tls, defaultDialTimeout)
+
+	dialTimeout := defaultDialTimeout
+	totalTimeout := c.GlobalDuration("total-timeout")
+	if totalTimeout != 0 && totalTimeout < dialTimeout {
+		dialTimeout = totalTimeout
+	}
+	return transport.NewTransport(tls, dialTimeout)
 }
 
 func getUsernamePasswordFromFlag(usernameFlag string) (username string, password string, err error) {
@@ -215,7 +221,7 @@ func mustNewClient(c *cli.Context) client.Client {
 		if debug {
 			fmt.Fprintf(os.Stderr, "start to sync cluster using endpoints(%s)\n", strings.Join(hc.Endpoints(), ","))
 		}
-		ctx, cancel := context.WithTimeout(context.Background(), client.DefaultRequestTimeout)
+		ctx, cancel := contextWithTotalTimeout(c)
 		err := hc.Sync(ctx)
 		cancel()
 		if err != nil {

etcdmain/etcd.go

@@ -12,9 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// TODO: support arm64
-// +build amd64
-
 package etcdmain
 
 import (
@@ -79,6 +76,8 @@ var (
 )
 
 func Main() {
+	checkSupportArch()
+
 	cfg := NewConfig()
 	err := cfg.Parse(os.Args[1:])
 	if err != nil {
@@ -554,3 +553,16 @@ func setupLogging(cfg *config) {
 		repoLog.SetLogLevel(settings)
 	}
 }
+
+func checkSupportArch() {
+	// TODO qualify arm64
+	if runtime.GOARCH == "amd64" {
+		return
+	}
+	if env, ok := os.LookupEnv("ETCD_UNSUPPORTED_ARCH"); ok && env == runtime.GOARCH {
+		plog.Warningf("running etcd on unsupported architecture %q since ETCD_UNSUPPORTED_ARCH is set", env)
+		return
+	}
+	plog.Errorf("etcd on unsupported platform without ETCD_UNSUPPORTED_ARCH=%s set.", runtime.GOARCH)
+	os.Exit(1)
+}

etcdmain/etcd_phony.go

@@ -1,31 +0,0 @@
-// Copyright 2016 CoreOS, Inc.
-//
-// 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.
-
-// +build !amd64
-
-package etcdmain
-
-import (
-	"fmt"
-	"os"
-
-	"github.com/coreos/etcd/Godeps/_workspace/src/github.com/coreos/pkg/capnslog"
-)
-
-var plog = capnslog.NewPackageLogger("github.com/coreos/etcd", "etcdmain")
-
-func Main() {
-	fmt.Println("unsupported architecture; unreliable, unstable")
-	os.Exit(-1)
-}

etcdserver/auth/auth.go

@@ -281,13 +281,7 @@ func (s *store) UpdateUser(user User) (User, error) {
 		return old, err
 	}
 
-	hash, err := s.HashPassword(user.Password)
-	if err != nil {
-		return old, err
-	}
-	user.Password = hash
-
-	newUser, err := old.merge(user)
+	newUser, err := old.merge(user, s.PasswordStore)
 	if err != nil {
 		return old, err
 	}
@@ -452,29 +446,33 @@ func (s *store) DisableAuth() error {
 // is called and returns a new User with these modifications applied. Think of
 // all Users as immutable sets of data. Merge allows you to perform the set
 // operations (desired grants and revokes) atomically
-func (u User) merge(n User) (User, error) {
+func (ou User) merge(nu User, s PasswordStore) (User, error) {
 	var out User
-	if u.User != n.User {
-		return out, authErr(http.StatusConflict, "Merging user data with conflicting usernames: %s %s", u.User, n.User)
+	if ou.User != nu.User {
+		return out, authErr(http.StatusConflict, "Merging user data with conflicting usernames: %s %s", ou.User, nu.User)
 	}
-	out.User = u.User
-	if n.Password != "" {
-		out.Password = n.Password
+	out.User = ou.User
+	if nu.Password != "" {
+		hash, err := s.HashPassword(nu.Password)
+		if err != nil {
+			return ou, err
+		}
+		out.Password = hash
 	} else {
-		out.Password = u.Password
+		out.Password = ou.Password
 	}
-	currentRoles := types.NewUnsafeSet(u.Roles...)
-	for _, g := range n.Grant {
+	currentRoles := types.NewUnsafeSet(ou.Roles...)
+	for _, g := range nu.Grant {
 		if currentRoles.Contains(g) {
-			plog.Noticef("granting duplicate role %s for user %s", g, n.User)
-			return User{}, authErr(http.StatusConflict, fmt.Sprintf("Granting duplicate role %s for user %s", g, n.User))
+			plog.Noticef("granting duplicate role %s for user %s", g, nu.User)
+			return User{}, authErr(http.StatusConflict, fmt.Sprintf("Granting duplicate role %s for user %s", g, nu.User))
 		}
 		currentRoles.Add(g)
 	}
-	for _, r := range n.Revoke {
+	for _, r := range nu.Revoke {
 		if !currentRoles.Contains(r) {
-			plog.Noticef("revoking ungranted role %s for user %s", r, n.User)
-			return User{}, authErr(http.StatusConflict, fmt.Sprintf("Revoking ungranted role %s for user %s", r, n.User))
+			plog.Noticef("revoking ungranted role %s for user %s", r, nu.User)
+			return User{}, authErr(http.StatusConflict, fmt.Sprintf("Revoking ungranted role %s for user %s", r, nu.User))
 		}
 		currentRoles.Remove(r)
 	}

etcdserver/auth/auth_test.go

@@ -26,6 +26,21 @@ import (
 	etcdstore "github.com/coreos/etcd/store"
 )
 
+type fakeDoer struct{}
+
+func (_ fakeDoer) Do(context.Context, etcdserverpb.Request) (etcdserver.Response, error) {
+	return etcdserver.Response{}, nil
+}
+
+func TestCheckPassword(t *testing.T) {
+	st := NewStore(fakeDoer{}, 5*time.Second)
+	u := User{Password: "$2a$10$I3iddh1D..EIOXXQtsra4u8AjOtgEa2ERxVvYGfXFBJDo1omXwP.q"}
+	matched := st.CheckPassword(u, "foo")
+	if matched {
+		t.Fatalf("expected false, got %v", matched)
+	}
+}
+
 const testTimeout = time.Millisecond
 
 func TestMergeUser(t *testing.T) {
@@ -71,16 +86,16 @@ func TestMergeUser(t *testing.T) {
 			User{User: "foo", Roles: []string{"role1", "role2"}},
 			false,
 		},
-		{
-			User{User: "foo"},
-			User{User: "foo", Password: "$2a$10$aUPOdbOGNawaVSusg3g2wuC3AH6XxIr9/Ms4VgDvzrAVOJPYzZILa"},
-			User{User: "foo", Roles: []string{}, Password: "$2a$10$aUPOdbOGNawaVSusg3g2wuC3AH6XxIr9/Ms4VgDvzrAVOJPYzZILa"},
+		{ // empty password will not overwrite the previous password
+			User{User: "foo", Password: "foo", Roles: []string{}},
+			User{User: "foo", Password: ""},
+			User{User: "foo", Password: "foo", Roles: []string{}},
 			false,
 		},
 	}
 
 	for i, tt := range tbl {
-		out, err := tt.input.merge(tt.merge)
+		out, err := tt.input.merge(tt.merge, passwordStore{})
 		if err != nil && !tt.iserr {
 			t.Fatalf("Got unexpected error on item %d", i)
 		}

etcdserver/cluster_util.go

@@ -71,6 +71,7 @@ func getClusterFromRemotePeers(urls []string, timeout time.Duration, logerr bool
 			continue
 		}
 		b, err := ioutil.ReadAll(resp.Body)
+		resp.Body.Close()
 		if err != nil {
 			if logerr {
 				plog.Warningf("could not read the body of cluster response: %v", err)

etcdserver/etcdhttp/client_auth.go

@@ -285,6 +285,10 @@ type userWithRoles struct {
 	Roles []auth.Role `json:"roles,omitempty"`
 }
 
+type usersCollections struct {
+	Users []userWithRoles `json:"users"`
+}
+
 func (sh *authHandler) baseUsers(w http.ResponseWriter, r *http.Request) {
 	if !allowMethod(w, r.Method, "GET") {
 		return
@@ -311,9 +315,7 @@ func (sh *authHandler) baseUsers(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	var usersCollections struct {
-		Users []userWithRoles `json:"users"`
-	}
+	ucs := usersCollections{}
 	for _, userName := range users {
 		var user auth.User
 		user, err = sh.sec.GetUser(userName)
@@ -327,15 +329,14 @@ func (sh *authHandler) baseUsers(w http.ResponseWriter, r *http.Request) {
 			var role auth.Role
 			role, err = sh.sec.GetRole(roleName)
 			if err != nil {
-				writeError(w, r, err)
-				return
+				continue
 			}
 			uwr.Roles = append(uwr.Roles, role)
 		}
 
-		usersCollections.Users = append(usersCollections.Users, uwr)
+		ucs.Users = append(ucs.Users, uwr)
 	}
-	err = json.NewEncoder(w).Encode(usersCollections)
+	err = json.NewEncoder(w).Encode(ucs)
 
 	if err != nil {
 		plog.Warningf("baseUsers error encoding on %s", r.URL)

etcdserver/etcdhttp/client_auth_test.go

@@ -15,10 +15,14 @@
 package etcdhttp
 
 import (
+	"encoding/json"
 	"errors"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
 	"path"
+	"sort"
 	"strings"
 	"testing"
 
@@ -43,7 +47,14 @@ type mockAuthStore struct {
 	enabled bool
 }
 
-func (s *mockAuthStore) AllUsers() ([]string, error) { return []string{"alice", "bob", "root"}, s.err }
+func (s *mockAuthStore) AllUsers() ([]string, error) {
+	var us []string
+	for u := range s.users {
+		us = append(us, u)
+	}
+	sort.Strings(us)
+	return us, s.err
+}
 func (s *mockAuthStore) GetUser(name string) (auth.User, error) {
 	u, ok := s.users[name]
 	if !ok {
@@ -67,9 +78,15 @@ func (s *mockAuthStore) UpdateUser(user auth.User) (auth.User, error) {
 func (s *mockAuthStore) AllRoles() ([]string, error) {
 	return []string{"awesome", "guest", "root"}, s.err
 }
-func (s *mockAuthStore) GetRole(name string) (auth.Role, error) { return *s.roles[name], s.err }
-func (s *mockAuthStore) CreateRole(role auth.Role) error        { return s.err }
-func (s *mockAuthStore) DeleteRole(name string) error           { return s.err }
+func (s *mockAuthStore) GetRole(name string) (auth.Role, error) {
+	r, ok := s.roles[name]
+	if ok {
+		return *r, s.err
+	}
+	return auth.Role{}, fmt.Errorf("%q does not exist (%v)", name, s.err)
+}
+func (s *mockAuthStore) CreateRole(role auth.Role) error { return s.err }
+func (s *mockAuthStore) DeleteRole(name string) error    { return s.err }
 func (s *mockAuthStore) UpdateRole(role auth.Role) (auth.Role, error) {
 	return *s.roles[role.Role], s.err
 }
@@ -361,6 +378,61 @@ func TestAuthFlow(t *testing.T) {
 	}
 }
 
+func TestGetUserGrantedWithNonexistingRole(t *testing.T) {
+	sh := &authHandler{
+		sec: &mockAuthStore{
+			users: map[string]*auth.User{
+				"root": {
+					User:  "root",
+					Roles: []string{"root", "foo"},
+				},
+			},
+			roles: map[string]*auth.Role{
+				"root": {
+					Role: "root",
+				},
+			},
+		},
+		cluster: &fakeCluster{id: 1},
+	}
+	srv := httptest.NewServer(http.HandlerFunc(sh.baseUsers))
+	defer srv.Close()
+
+	req, err := http.NewRequest("GET", "", nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	req.URL, err = url.Parse(srv.URL)
+	if err != nil {
+		t.Fatal(err)
+	}
+	req.Header.Set("Content-Type", "application/json")
+
+	cli := http.DefaultClient
+	resp, err := cli.Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer resp.Body.Close()
+
+	var uc usersCollections
+	if err := json.NewDecoder(resp.Body).Decode(&uc); err != nil {
+		t.Fatal(err)
+	}
+	if len(uc.Users) != 1 {
+		t.Fatalf("expected 1 user, got %+v", uc.Users)
+	}
+	if uc.Users[0].User != "root" {
+		t.Fatalf("expected 'root', got %q", uc.Users[0].User)
+	}
+	if len(uc.Users[0].Roles) != 1 {
+		t.Fatalf("expected 1 role, got %+v", uc.Users[0].Roles)
+	}
+	if uc.Users[0].Roles[0].Role != "root" {
+		t.Fatalf("expected 'root', got %q", uc.Users[0].Roles[0].Role)
+	}
+}
+
 func mustAuthRequest(method, username, password string) *http.Request {
 	req, err := http.NewRequest(method, "path", strings.NewReader(""))
 	if err != nil {

etcdserver/server.go

@@ -203,6 +203,10 @@ type EtcdServer struct {
 	// count the number of inflight snapshots.
 	// MUST use atomic operation to access this field.
 	inflightSnapshots int64
+
+	// wg is used to wait for the go routines that depends on the server state
+	// to exit when stopping the server.
+	wg sync.WaitGroup
 }
 
 // NewServer creates a new EtcdServer from the supplied configuration. The
@@ -515,9 +519,15 @@ func (s *EtcdServer) run() {
 	}
 
 	defer func() {
-		s.r.stop()
 		sched.Stop()
 
+		// wait for snapshots before closing raft so wal stays open
+		s.wg.Wait()
+
+		// must stop raft after scheduler-- etcdserver can leak rafthttp pipelines
+		// by adding a peer after raft stops the transport
+		s.r.stop()
+
 		// kv, lessor and backend can be nil if running without v3 enabled
 		// or running unit tests.
 		if s.lessor != nil {
@@ -1162,7 +1172,10 @@ func (s *EtcdServer) applyConfChange(cc raftpb.ConfChange, confState *raftpb.Con
 func (s *EtcdServer) snapshot(snapi uint64, confState raftpb.ConfState) {
 	clone := s.store.Clone()
 
+	s.wg.Add(1)
 	go func() {
+		defer s.wg.Done()
+
 		d, err := clone.SaveNoCopy()
 		// TODO: current store will never fail to do a snapshot
 		// what should we do if the store might fail?

integration/cluster_test.go

@@ -231,9 +231,14 @@ func TestIssue2681(t *testing.T) {
 }
 
 // Ensure we can remove a member after a snapshot then add a new one back.
-func TestIssue2746(t *testing.T) {
+func TestIssue2746(t *testing.T) { testIssue2746(t, 5) }
+
+// With 3 nodes TestIssue2476 sometimes had a shutdown with an inflight snapshot.
+func TestIssue2746WithThree(t *testing.T) { testIssue2746(t, 3) }
+
+func testIssue2746(t *testing.T, members int) {
 	defer testutil.AfterTest(t)
-	c := NewCluster(t, 5)
+	c := NewCluster(t, members)
 
 	for _, m := range c.Members {
 		m.SnapCount = 10
@@ -247,7 +252,7 @@ func TestIssue2746(t *testing.T) {
 		clusterMustProgress(t, c.Members)
 	}
 
-	c.RemoveMember(t, uint64(c.Members[4].s.ID()))
+	c.RemoveMember(t, uint64(c.Members[members-1].s.ID()))
 	c.waitLeader(t, c.Members)
 
 	c.AddMember(t)

proxy/reverse.go

@@ -111,9 +111,10 @@ func (p *reverseProxy) ServeHTTP(rw http.ResponseWriter, clientreq *http.Request
 	closeNotifier, ok := rw.(http.CloseNotifier)
 	cancel := httputil.RequestCanceler(p.transport, proxyreq)
 	if ok {
+		closeCh := closeNotifier.CloseNotify()
 		go func() {
 			select {
-			case <-closeNotifier.CloseNotify():
+			case <-closeCh:
 				atomic.StoreInt32(&requestClosed, 1)
 				log.Printf("proxy: client %v closed request prematurely", clientreq.RemoteAddr)
 				cancel()

raft/raft.go

@@ -837,6 +837,12 @@ func (r *raft) addNode(id uint64) {
 func (r *raft) removeNode(id uint64) {
 	r.delProgress(id)
 	r.pendingConf = false
+
+	// do not try to commit or abort transferring if there is no nodes in the cluster.
+	if len(r.prs) == 0 {
+		return
+	}
+
 	// The quorum size is now smaller, so see if any pending entries can
 	// be committed.
 	if r.maybeCommit() {

raft/raft_test.go

@@ -1780,6 +1780,13 @@ func TestRemoveNode(t *testing.T) {
 	if g := r.nodes(); !reflect.DeepEqual(g, w) {
 		t.Errorf("nodes = %v, want %v", g, w)
 	}
+
+	// remove all nodes from cluster
+	r.removeNode(1)
+	w = []uint64{}
+	if g := r.nodes(); !reflect.DeepEqual(g, w) {
+		t.Errorf("nodes = %v, want %v", g, w)
+	}
 }
 
 func TestPromotable(t *testing.T) {

rafthttp/peer.go

@@ -142,11 +142,23 @@ func startPeer(transport *Transport, urls types.URLs, local, to, cid types.ID, r
 	go func() {
 		for {
 			select {
-			case mm := <-p.propc:
+			case mm := <-p.recvc:
 				if err := r.Process(ctx, mm); err != nil {
 					plog.Warningf("failed to process raft message (%v)", err)
 				}
-			case mm := <-p.recvc:
+			case <-p.stopc:
+				return
+			}
+		}
+	}()
+
+	// r.Process might block for processing proposal when there is no leader.
+	// Thus propc must be put into a separate routine with recvc to avoid blocking
+	// processing other raft messages.
+	go func() {
+		for {
+			select {
+			case mm := <-p.propc:
 				if err := r.Process(ctx, mm); err != nil {
 					plog.Warningf("failed to process raft message (%v)", err)
 				}

version/version.go

@@ -29,7 +29,7 @@ import (
 var (
 	// MinClusterVersion is the min cluster version this etcd binary is compatible with.
 	MinClusterVersion = "2.2.0"
-	Version           = "2.3.0"
+	Version           = "2.3.5"
 
 	// Git SHA Value will be set during build
 	GitSHA = "Not provided (use ./build instead of go build)"