From ad0664da9cc5279bae5f89d49b9c86e4b8089585 Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Tue, 28 Oct 2014 09:44:59 -0700 Subject: [PATCH 1/4] doc: fix documentation of POST /v2/admin/members --- Documentation/0.5/admin_api.md | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/Documentation/0.5/admin_api.md b/Documentation/0.5/admin_api.md index f8e4b582b..80a86247b 100644 --- a/Documentation/0.5/admin_api.md +++ b/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", - "peerURLs":[ - "http://10.0.0.10:2379" - ], - }, - ] + { + "id":"3777296169", + "peerURLs":[ + "http://10.0.0.10:2379" + ], + } ``` ### DELETE /v2/admin/members/:id From 8b0eaa9e15f7eea95f269a48033e02c7c1ec04f4 Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Tue, 28 Oct 2014 10:09:05 -0700 Subject: [PATCH 2/4] etcdhttp: separate member create deserialization --- etcdserver/etcdhttp/client.go | 16 +++---- etcdserver/etcdhttp/httptypes/member.go | 25 +++++++++++ etcdserver/etcdhttp/httptypes/member_test.go | 44 ++++++++++++++++++++ 3 files changed, 75 insertions(+), 10 deletions(-) diff --git a/etcdserver/etcdhttp/client.go b/etcdserver/etcdhttp/client.go index ce67ac929..7cf255bdd 100644 --- a/etcdserver/etcdhttp/client.go +++ b/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" ) @@ -181,24 +180,21 @@ func (h *adminMembersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) writeError(w, httptypes.NewHTTPError(http.StatusBadRequest, err.Error())) return } - raftAttr := etcdserver.RaftAttributes{} - if err := json.Unmarshal(b, &raftAttr); err != nil { + req := httptypes.MemberCreateRequest{} + 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) + w.Header().Set("Content-Type", "application/json") w.WriteHeader(http.StatusCreated) if err := json.NewEncoder(w).Encode(m); err != nil { diff --git a/etcdserver/etcdhttp/httptypes/member.go b/etcdserver/etcdhttp/httptypes/member.go index 7d972daf0..06fd0ccb8 100644 --- a/etcdserver/etcdhttp/httptypes/member.go +++ b/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) { diff --git a/etcdserver/etcdhttp/httptypes/member_test.go b/etcdserver/etcdhttp/httptypes/member_test.go index 6c0b3cdf5..2b07c28b0 100644 --- a/etcdserver/etcdhttp/httptypes/member_test.go +++ b/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) + } + } +} From d1fb732e639be121227e69868923b8534c116f0e Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Tue, 28 Oct 2014 10:21:11 -0700 Subject: [PATCH 3/4] etcdhttp: properly serialize Member on POST --- etcdserver/etcdhttp/client.go | 29 +++++++++------- etcdserver/etcdhttp/client_test.go | 53 +++++++++++++++++++----------- 2 files changed, 51 insertions(+), 31 deletions(-) diff --git a/etcdserver/etcdhttp/client.go b/etcdserver/etcdhttp/client.go index 7cf255bdd..485b72367 100644 --- a/etcdserver/etcdhttp/client.go +++ b/etcdserver/etcdhttp/client.go @@ -195,9 +195,10 @@ func (h *adminMembersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) } 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": @@ -530,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{ - 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 + c[i] = newMember(m) } 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 +} diff --git a/etcdserver/etcdhttp/client_test.go b/etcdserver/etcdhttp/client_test.go index 4a5100019..49b253574 100644 --- a/etcdserver/etcdhttp/client_test.go +++ b/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, err := json.Marshal(raftAttr) - if err != nil { - t.Fatal(err) - } - body := bytes.NewReader(b) - req, err := http.NewRequest("POST", u.String(), body) + b := []byte(`{"peerURLs":["http://127.0.0.1:1"]}`) + req, err := http.NewRequest("POST", u.String(), bytes.NewReader(b)) 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 != w { - t.Errorf("got body=%q, want %q", g, w) + if g != wb { + t.Errorf("got body=%q, want %q", g, wb) } + + 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) @@ -1598,3 +1594,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) + } +} From c07b9ae32e71434a08f5eab66d2dd92e416bf070 Mon Sep 17 00:00:00 2001 From: Brian Waldon Date: Tue, 28 Oct 2014 10:35:15 -0700 Subject: [PATCH 4/4] etcdhttp: 415 when content-type not JSON --- etcdserver/etcdhttp/client.go | 2 +- etcdserver/etcdhttp/client_test.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/etcdserver/etcdhttp/client.go b/etcdserver/etcdhttp/client.go index 485b72367..f35e5fdd5 100644 --- a/etcdserver/etcdhttp/client.go +++ b/etcdserver/etcdhttp/client.go @@ -172,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) diff --git a/etcdserver/etcdhttp/client_test.go b/etcdserver/etcdhttp/client_test.go index 49b253574..1abcbe7da 100644 --- a/etcdserver/etcdhttp/client_test.go +++ b/etcdserver/etcdhttp/client_test.go @@ -717,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{}, @@ -732,7 +733,7 @@ func TestServeAdminMembersFail(t *testing.T) { }, &errServer{}, - http.StatusBadRequest, + http.StatusUnsupportedMediaType, }, { // bad url