From 387639e80246161d284f430b755f88462f50849a Mon Sep 17 00:00:00 2001 From: Jonathan Boulle Date: Mon, 27 Oct 2014 10:05:42 -0700 Subject: [PATCH 1/2] etcdserver/etcdhttp: treat /v2/admin/members and /v2/admin/members/ equally --- etcdserver/etcdhttp/client.go | 15 ++++++++++++--- etcdserver/etcdhttp/client_test.go | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/etcdserver/etcdhttp/client.go b/etcdserver/etcdhttp/client.go index 13f5c8813..bd70a6ffa 100644 --- a/etcdserver/etcdhttp/client.go +++ b/etcdserver/etcdhttp/client.go @@ -43,7 +43,7 @@ import ( const ( keysPrefix = "/v2/keys" deprecatedMachinesPrefix = "/v2/machines" - adminMembersPrefix = "/v2/admin/members/" + adminMembersPrefix = "/v2/admin/members" statsPrefix = "/v2/stats" versionPrefix = "/version" ) @@ -80,6 +80,7 @@ func NewClientHandler(server *etcdserver.EtcdServer) http.Handler { mux.HandleFunc(statsPrefix+"/self", sh.serveSelf) mux.HandleFunc(statsPrefix+"/leader", sh.serveLeader) mux.Handle(adminMembersPrefix, amh) + mux.Handle(adminMembersPrefix+"/", amh) mux.Handle(deprecatedMachinesPrefix, dmh) return mux } @@ -159,7 +160,7 @@ func (h *adminMembersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) switch r.Method { case "GET": - if s := strings.TrimPrefix(r.URL.Path, adminMembersPrefix); s != "" { + if trimPrefix(r.URL.Path, adminMembersPrefix) != "" { http.NotFound(w, r) return } @@ -203,7 +204,7 @@ func (h *adminMembersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) log.Printf("etcdhttp: %v", err) } case "DELETE": - idStr := strings.TrimPrefix(r.URL.Path, adminMembersPrefix) + idStr := trimPrefix(r.URL.Path, adminMembersPrefix) id, err := strconv.ParseUint(idStr, 16, 64) if err != nil { http.Error(w, err.Error(), http.StatusBadRequest) @@ -516,6 +517,14 @@ func getBool(form url.Values, key string) (b bool, err error) { return } +// trimPrefix removes a given prefix and any slash following the prefix +// e.g.: trimPrefix("foo", "foo") == trimPrefix("foo/", "foo") == "" +func trimPrefix(p, prefix string) (s string) { + s = strings.TrimPrefix(p, prefix) + s = strings.TrimPrefix(s, "/") + return +} + func newMemberCollection(ms []*etcdserver.Member) httptypes.MemberCollection { c := httptypes.MemberCollection(make([]httptypes.Member, len(ms))) diff --git a/etcdserver/etcdhttp/client_test.go b/etcdserver/etcdhttp/client_test.go index 38d9555f9..c21bf4157 100644 --- a/etcdserver/etcdhttp/client_test.go +++ b/etcdserver/etcdhttp/client_test.go @@ -574,6 +574,7 @@ func TestServeAdminMembers(t *testing.T) { wbody string }{ {adminMembersPrefix, http.StatusOK, "application/json", wmc}, + {adminMembersPrefix + "/", http.StatusOK, "application/json", wmc}, {path.Join(adminMembersPrefix, "100"), http.StatusNotFound, "text/plain; charset=utf-8", "404 page not found\n"}, {path.Join(adminMembersPrefix, "foobar"), http.StatusNotFound, "text/plain; charset=utf-8", "404 page not found\n"}, } @@ -1540,3 +1541,20 @@ func TestTrimNodeExternPrefix(t *testing.T) { } } } + +func TestTrimPrefix(t *testing.T) { + tests := []struct { + in string + prefix string + w string + }{ + {"/v2/admin/members", "/v2/admin/members", ""}, + {"/v2/admin/members/", "/v2/admin/members", ""}, + {"/v2/admin/members/foo", "/v2/admin/members", "foo"}, + } + for i, tt := range tests { + if g := trimPrefix(tt.in, tt.prefix); g != tt.w { + t.Errorf("#%d: trimPrefix = %q, want %q", i, g, tt.w) + } + } +} From e849d8e15700eb1ae9eac642f6f28e68f814408b Mon Sep 17 00:00:00 2001 From: Jonathan Boulle Date: Mon, 27 Oct 2014 10:07:19 -0700 Subject: [PATCH 2/2] etcdhttp: DELETE on members = MethodNotAllowed --- etcdserver/etcdhttp/client.go | 4 ++++ etcdserver/etcdhttp/client_test.go | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/etcdserver/etcdhttp/client.go b/etcdserver/etcdhttp/client.go index bd70a6ffa..fe3142069 100644 --- a/etcdserver/etcdhttp/client.go +++ b/etcdserver/etcdhttp/client.go @@ -205,6 +205,10 @@ func (h *adminMembersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) } case "DELETE": idStr := trimPrefix(r.URL.Path, adminMembersPrefix) + if idStr == "" { + http.Error(w, "Method Not Allowed", http.StatusMethodNotAllowed) + return + } id, err := strconv.ParseUint(idStr, 16, 64) if err != nil { http.Error(w, err.Error(), http.StatusBadRequest) diff --git a/etcdserver/etcdhttp/client_test.go b/etcdserver/etcdhttp/client_test.go index c21bf4157..0bb2621f9 100644 --- a/etcdserver/etcdhttp/client_test.go +++ b/etcdserver/etcdhttp/client_test.go @@ -779,6 +779,16 @@ func TestServeAdminMembersFail(t *testing.T) { http.StatusInternalServerError, }, + { + // etcdserver.RemoveMember error + &http.Request{ + URL: mustNewURL(t, adminMembersPrefix), + Method: "DELETE", + }, + nil, + + http.StatusMethodNotAllowed, + }, } for i, tt := range tests { h := &adminMembersHandler{