diff --git a/etcdserver/cluster.go b/etcdserver/cluster.go index 4ea0049d5..058e19a0d 100644 --- a/etcdserver/cluster.go +++ b/etcdserver/cluster.go @@ -242,7 +242,6 @@ func (c *Cluster) SetStore(st store.Store) { c.store = st } func (c *Cluster) ValidateConfigurationChange(cc raftpb.ConfChange) error { appliedMembers, appliedRemoved := membersFromStore(c.store) - if appliedRemoved[types.ID(cc.NodeID)] { return ErrIDRemoved } @@ -251,6 +250,21 @@ func (c *Cluster) ValidateConfigurationChange(cc raftpb.ConfChange) error { if appliedMembers[types.ID(cc.NodeID)] != nil { return ErrIDExists } + urls := make(map[string]bool) + for _, m := range appliedMembers { + for _, u := range m.PeerURLs { + urls[u] = true + } + } + m := new(Member) + if err := json.Unmarshal(cc.Context, m); err != nil { + log.Panicf("unmarshal member should never fail: %v", err) + } + for _, u := range m.PeerURLs { + if urls[u] { + return ErrPeerURLexists + } + } case raftpb.ConfChangeRemoveNode: if appliedMembers[types.ID(cc.NodeID)] == nil { return ErrIDNotFound diff --git a/etcdserver/cluster_test.go b/etcdserver/cluster_test.go index 96ea42cbf..9fe6ca348 100644 --- a/etcdserver/cluster_test.go +++ b/etcdserver/cluster_test.go @@ -17,11 +17,14 @@ package etcdserver import ( + "encoding/json" + "fmt" "path" "reflect" "testing" "github.com/coreos/etcd/pkg/types" + "github.com/coreos/etcd/raft/raftpb" "github.com/coreos/etcd/store" ) @@ -351,6 +354,77 @@ func TestClusterValidateAndAssignIDs(t *testing.T) { } } +func TestClusterValidateConfigurationChange(t *testing.T) { + cl := newCluster("") + cl.SetStore(store.New()) + for i := 1; i <= 4; i++ { + attr := RaftAttributes{PeerURLs: []string{fmt.Sprintf("http://127.0.0.1:%d", i)}} + cl.AddMember(&Member{ID: types.ID(i), RaftAttributes: attr}) + } + cl.RemoveMember(4) + + attr := RaftAttributes{PeerURLs: []string{fmt.Sprintf("http://127.0.0.1:%d", 1)}} + cxt, err := json.Marshal(&Member{ID: types.ID(5), RaftAttributes: attr}) + if err != nil { + t.Fatal(err) + } + + tests := []struct { + cc raftpb.ConfChange + werr error + }{ + { + raftpb.ConfChange{ + Type: raftpb.ConfChangeRemoveNode, + NodeID: 3, + }, + nil, + }, + { + raftpb.ConfChange{ + Type: raftpb.ConfChangeAddNode, + NodeID: 4, + }, + ErrIDRemoved, + }, + { + raftpb.ConfChange{ + Type: raftpb.ConfChangeRemoveNode, + NodeID: 4, + }, + ErrIDRemoved, + }, + { + raftpb.ConfChange{ + Type: raftpb.ConfChangeAddNode, + NodeID: 1, + }, + ErrIDExists, + }, + { + raftpb.ConfChange{ + Type: raftpb.ConfChangeAddNode, + NodeID: 5, + Context: cxt, + }, + ErrPeerURLexists, + }, + { + raftpb.ConfChange{ + Type: raftpb.ConfChangeRemoveNode, + NodeID: 5, + }, + ErrIDNotFound, + }, + } + for i, tt := range tests { + err := cl.ValidateConfigurationChange(tt.cc) + if err != tt.werr { + t.Errorf("#%d: validateConfigurationChange error = %v, want %v", i, err, tt.werr) + } + } +} + func TestClusterGenID(t *testing.T) { cs := newTestCluster([]Member{ newTestMember(1, nil, "", nil), diff --git a/etcdserver/etcdhttp/client.go b/etcdserver/etcdhttp/client.go index 4be7f8197..5c322fc92 100644 --- a/etcdserver/etcdhttp/client.go +++ b/etcdserver/etcdhttp/client.go @@ -186,12 +186,16 @@ func (h *membersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { now := h.clock.Now() m := etcdserver.NewMember("", req.PeerURLs, "", &now) - if err := h.server.AddMember(ctx, *m); err != nil { + err = h.server.AddMember(ctx, *m) + switch { + case err == etcdserver.ErrIDExists || err == etcdserver.ErrPeerURLexists: + writeError(w, httptypes.NewHTTPError(http.StatusPreconditionFailed, err.Error())) + return + case err != nil: log.Printf("etcdhttp: error adding node %s: %v", m.ID, err) writeError(w, err) return } - res := newMember(m) w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusCreated) diff --git a/etcdserver/server.go b/etcdserver/server.go index 0dbe65a3a..aaffb745f 100644 --- a/etcdserver/server.go +++ b/etcdserver/server.go @@ -64,6 +64,7 @@ var ( ErrIDRemoved = errors.New("etcdserver: ID removed") ErrIDExists = errors.New("etcdserver: ID exists") ErrIDNotFound = errors.New("etcdserver: ID not found") + ErrPeerURLexists = errors.New("etcdserver: peerURL exists") ErrCanceled = errors.New("etcdserver: request cancelled") ErrTimeout = errors.New("etcdserver: request timed out")