Merge pull request #1465 from bcwaldon/member-post

Clean up POST /v2/admin/members

Documentation/0.5/admin_api.md

@@ -42,21 +42,19 @@ Returns an HTTP 201 response code and the representation of added member with a
 If the POST body is malformed an HTTP 400 will be returned. If the member exists in the cluster or existed in the cluster at some point in the past an HTTP 500(TODO: fix this) will be returned. If the cluster fails to process the request within timeout an HTTP 500 will be returned, though the request may be processed later.
 ```
     Example Request: POST
-                     http://localhost:2379/v2/admin/members/
+                     http://localhost:2379/v2/admin/members
                      Body:
-                     [{"PeerURLs":["http://10.0.0.10:2379"]}]
+                     {"peerURLs":["http://10.0.0.10:2379"]}
     Respose formats: JSON
     Example Response:
 ```
 ```json
-    [
+    {
-        {
+        "id":"3777296169",
-            "id":"3777296169",
+        "peerURLs":[
-            "peerURLs":[
+            "http://10.0.0.10:2379"
-                "http://10.0.0.10:2379"
+        ],
-            ],
+    }
-        },
-    ]
 ```
 
 ### DELETE /v2/admin/members/:id

etcdserver/etcdhttp/client.go

@@ -36,7 +36,6 @@ import (
 	"github.com/coreos/etcd/etcdserver/etcdhttp/httptypes"
 	"github.com/coreos/etcd/etcdserver/etcdserverpb"
 	"github.com/coreos/etcd/pkg/strutil"
-	"github.com/coreos/etcd/pkg/types"
 	"github.com/coreos/etcd/store"
 	"github.com/coreos/etcd/version"
 )
@@ -173,7 +172,7 @@ func (h *adminMembersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 	case "POST":
 		ctype := r.Header.Get("Content-Type")
 		if ctype != "application/json" {
-			writeError(w, httptypes.NewHTTPError(http.StatusBadRequest, fmt.Sprintf("Bad Content-Type %s, accept application/json", ctype)))
+			writeError(w, httptypes.NewHTTPError(http.StatusUnsupportedMediaType, fmt.Sprintf("Bad Content-Type %s, accept application/json", ctype)))
 			return
 		}
 		b, err := ioutil.ReadAll(r.Body)
@@ -181,27 +180,25 @@ func (h *adminMembersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 			writeError(w, httptypes.NewHTTPError(http.StatusBadRequest, err.Error()))
 			return
 		}
-		raftAttr := etcdserver.RaftAttributes{}
+		req := httptypes.MemberCreateRequest{}
-		if err := json.Unmarshal(b, &raftAttr); err != nil {
+		if err := json.Unmarshal(b, &req); err != nil {
 			writeError(w, httptypes.NewHTTPError(http.StatusBadRequest, err.Error()))
 			return
 		}
-		validURLs, err := types.NewURLs(raftAttr.PeerURLs)
+
-		if err != nil {
-			writeError(w, httptypes.NewHTTPError(http.StatusBadRequest, "Bad peer urls"))
-			return
-		}
 		now := h.clock.Now()
-		m := etcdserver.NewMember("", validURLs, "", &now)
+		m := etcdserver.NewMember("", req.PeerURLs, "", &now)
 		if err := h.server.AddMember(ctx, *m); err != nil {
 			log.Printf("etcdhttp: error adding node %x: %v", m.ID, err)
 			writeError(w, err)
 			return
 		}
-		log.Printf("etcdhttp: added node %x with peer urls %v", m.ID, raftAttr.PeerURLs)
+		log.Printf("etcdhttp: added node %x with peer urls %v", m.ID, req.PeerURLs)
+
+		res := newMember(m)
 		w.Header().Set("Content-Type", "application/json")
 		w.WriteHeader(http.StatusCreated)
-		if err := json.NewEncoder(w).Encode(m); err != nil {
+		if err := json.NewEncoder(w).Encode(res); err != nil {
 			log.Printf("etcdhttp: %v", err)
 		}
 	case "DELETE":
@@ -534,18 +531,22 @@ func newMemberCollection(ms []*etcdserver.Member) httptypes.MemberCollection {
 	c := httptypes.MemberCollection(make([]httptypes.Member, len(ms)))
 
 	for i, m := range ms {
-		tm := httptypes.Member{
+		c[i] = newMember(m)
-			ID:         strutil.IDAsHex(m.ID),
-			Name:       m.Name,
-			PeerURLs:   make([]string, len(m.PeerURLs)),
-			ClientURLs: make([]string, len(m.ClientURLs)),
-		}
-
-		copy(tm.PeerURLs, m.PeerURLs)
-		copy(tm.ClientURLs, m.ClientURLs)
-
-		c[i] = tm
 	}
 
 	return c
 }
+
+func newMember(m *etcdserver.Member) httptypes.Member {
+	tm := httptypes.Member{
+		ID:         strutil.IDAsHex(m.ID),
+		Name:       m.Name,
+		PeerURLs:   make([]string, len(m.PeerURLs)),
+		ClientURLs: make([]string, len(m.ClientURLs)),
+	}
+
+	copy(tm.PeerURLs, m.PeerURLs)
+	copy(tm.ClientURLs, m.ClientURLs)
+
+	return tm
+}

etcdserver/etcdhttp/client_test.go

@@ -601,15 +601,10 @@ func TestServeAdminMembers(t *testing.T) {
 	}
 }
 
-func TestServeAdminMembersPut(t *testing.T) {
+func TestServeAdminMembersCreate(t *testing.T) {
 	u := mustNewURL(t, adminMembersPrefix)
-	raftAttr := etcdserver.RaftAttributes{PeerURLs: []string{"http://127.0.0.1:1"}}
+	b := []byte(`{"peerURLs":["http://127.0.0.1:1"]}`)
-	b, err := json.Marshal(raftAttr)
+	req, err := http.NewRequest("POST", u.String(), bytes.NewReader(b))
-	if err != nil {
-		t.Fatal(err)
-	}
-	body := bytes.NewReader(b)
-	req, err := http.NewRequest("POST", u.String(), body)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -628,15 +623,7 @@ func TestServeAdminMembersPut(t *testing.T) {
 	if rw.Code != wcode {
 		t.Errorf("code=%d, want %d", rw.Code, wcode)
 	}
-	wm := etcdserver.Member{
-		ID:             3064321551348478165,
-		RaftAttributes: raftAttr,
-	}
 
-	wb, err := json.Marshal(wm)
-	if err != nil {
-		t.Fatal(err)
-	}
 	wct := "application/json"
 	if gct := rw.Header().Get("Content-Type"); gct != wct {
 		t.Errorf("content-type = %s, want %s", gct, wct)
@@ -646,11 +633,20 @@ func TestServeAdminMembersPut(t *testing.T) {
 	if gcid != wcid {
 		t.Errorf("cid = %s, want %s", gcid, wcid)
 	}
+
+	wb := `{"id":"2a86a83729b330d5","name":"","peerURLs":["http://127.0.0.1:1"],"clientURLs":[]}` + "\n"
 	g := rw.Body.String()
-	w := string(wb) + "\n"
+	if g != wb {
-	if g != w {
+		t.Errorf("got body=%q, want %q", g, wb)
-		t.Errorf("got body=%q, want %q", g, w)
 	}
+
+	wm := etcdserver.Member{
+		ID: 3064321551348478165,
+		RaftAttributes: etcdserver.RaftAttributes{
+			PeerURLs: []string{"http://127.0.0.1:1"},
+		},
+	}
+
 	wactions := []action{{name: "AddMember", params: []interface{}{wm}}}
 	if !reflect.DeepEqual(s.actions, wactions) {
 		t.Errorf("actions = %+v, want %+v", s.actions, wactions)
@@ -721,6 +717,7 @@ func TestServeAdminMembersFail(t *testing.T) {
 				URL:    mustNewURL(t, adminMembersPrefix),
 				Method: "POST",
 				Body:   ioutil.NopCloser(strings.NewReader("bad json")),
+				Header: map[string][]string{"Content-Type": []string{"application/json"}},
 			},
 			&resServer{},
 
@@ -736,7 +733,7 @@ func TestServeAdminMembersFail(t *testing.T) {
 			},
 			&errServer{},
 
-			http.StatusBadRequest,
+			http.StatusUnsupportedMediaType,
 		},
 		{
 			// bad url
@@ -1598,3 +1595,22 @@ func TestNewMemberCollection(t *testing.T) {
 		t.Fatalf("newMemberCollection failure: want=%#v, got=%#v", want, got)
 	}
 }
+
+func TestNewMember(t *testing.T) {
+	fixture := &etcdserver.Member{
+		ID:             12,
+		Attributes:     etcdserver.Attributes{ClientURLs: []string{"http://localhost:8080", "http://localhost:8081"}},
+		RaftAttributes: etcdserver.RaftAttributes{PeerURLs: []string{"http://localhost:8082", "http://localhost:8083"}},
+	}
+	got := newMember(fixture)
+
+	want := httptypes.Member{
+		ID:         "c",
+		ClientURLs: []string{"http://localhost:8080", "http://localhost:8081"},
+		PeerURLs:   []string{"http://localhost:8082", "http://localhost:8083"},
+	}
+
+	if !reflect.DeepEqual(want, got) {
+		t.Fatalf("newMember failure: want=%#v, got=%#v", want, got)
+	}
+}

etcdserver/etcdhttp/httptypes/member.go

@@ -18,6 +18,8 @@ package httptypes
 
 import (
 	"encoding/json"
+
+	"github.com/coreos/etcd/pkg/types"
 )
 
 type Member struct {
@@ -27,6 +29,29 @@ type Member struct {
 	ClientURLs []string `json:"clientURLs"`
 }
 
+type MemberCreateRequest struct {
+	PeerURLs types.URLs
+}
+
+func (m *MemberCreateRequest) UnmarshalJSON(data []byte) error {
+	s := struct {
+		PeerURLs []string `json:"peerURLs"`
+	}{}
+
+	err := json.Unmarshal(data, &s)
+	if err != nil {
+		return err
+	}
+
+	urls, err := types.NewURLs(s.PeerURLs)
+	if err != nil {
+		return err
+	}
+
+	m.PeerURLs = urls
+	return nil
+}
+
 type MemberCollection []Member
 
 func (c *MemberCollection) MarshalJSON() ([]byte, error) {

etcdserver/etcdhttp/httptypes/member_test.go

@@ -18,8 +18,11 @@ package httptypes
 
 import (
 	"encoding/json"
+	"net/url"
 	"reflect"
 	"testing"
+
+	"github.com/coreos/etcd/pkg/types"
 )
 
 func TestMemberUnmarshal(t *testing.T) {
@@ -155,3 +158,44 @@ func TestMemberCollectionUnmarshal(t *testing.T) {
 		}
 	}
 }
+
+func TestMemberCreateRequestUnmarshal(t *testing.T) {
+	body := []byte(`{"peerURLs": ["http://127.0.0.1:8081", "https://127.0.0.1:8080"]}`)
+	want := MemberCreateRequest{
+		PeerURLs: types.URLs([]url.URL{
+			url.URL{Scheme: "http", Host: "127.0.0.1:8081"},
+			url.URL{Scheme: "https", Host: "127.0.0.1:8080"},
+		}),
+	}
+
+	var req MemberCreateRequest
+	if err := json.Unmarshal(body, &req); err != nil {
+		t.Fatalf("Unmarshal returned unexpected err=%v", err)
+	}
+
+	if !reflect.DeepEqual(want, req) {
+		t.Fatalf("Failed to unmarshal MemberCreateRequest: want=%#v, got=%#v", want, req)
+	}
+}
+
+func TestMemberCreateRequestUnmarshalFail(t *testing.T) {
+	tests := [][]byte{
+		// invalid JSON
+		[]byte(``),
+		[]byte(`{`),
+
+		// spot-check validation done in types.NewURLs
+		[]byte(`{"peerURLs": "foo"}`),
+		[]byte(`{"peerURLs": ["."]}`),
+		[]byte(`{"peerURLs": []}`),
+		[]byte(`{"peerURLs": ["http://127.0.0.1:4001/foo"]}`),
+		[]byte(`{"peerURLs": ["http://127.0.0.1"]}`),
+	}
+
+	for i, tt := range tests {
+		var req MemberCreateRequest
+		if err := json.Unmarshal(tt, &req); err == nil {
+			t.Errorf("#%d: expected err, got nil", i)
+		}
+	}
+}