diff --git a/error/error.go b/error/error.go index 3eb6c4561..dd06067ff 100644 --- a/error/error.go +++ b/error/error.go @@ -111,10 +111,19 @@ func (e Error) toJsonString() string { func (e Error) Write(w http.ResponseWriter) { w.Header().Add("X-Etcd-Index", fmt.Sprint(e.Index)) - // 3xx is reft internal error - if e.ErrorCode/100 == 3 { - http.Error(w, e.toJsonString(), http.StatusInternalServerError) - } else { - http.Error(w, e.toJsonString(), http.StatusBadRequest) + // 3xx is raft internal error + status := http.StatusBadRequest + switch e.ErrorCode { + case EcodeKeyNotFound: + status = http.StatusNotFound + case EcodeNotFile, EcodeDirNotEmpty: + status = http.StatusForbidden + case EcodeTestFailed, EcodeNodeExist: + status = http.StatusPreconditionFailed + default: + if e.ErrorCode/100 == 3 { + status = http.StatusInternalServerError + } } + http.Error(w, e.toJsonString(), status) } diff --git a/server/v2/tests/delete_handler_test.go b/server/v2/tests/delete_handler_test.go index b6f7fa042..335564f0a 100644 --- a/server/v2/tests/delete_handler_test.go +++ b/server/v2/tests/delete_handler_test.go @@ -2,6 +2,7 @@ package v2 import ( "fmt" + "net/http" "net/url" "testing" @@ -22,6 +23,7 @@ func TestV2DeleteKey(t *testing.T) { resp, err := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) tests.ReadBody(resp) resp, err = tests.DeleteForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), url.Values{}) + assert.Equal(t, resp.StatusCode, http.StatusOK) body := tests.ReadBody(resp) assert.Nil(t, err, "") assert.Equal(t, string(body), `{"action":"delete","node":{"key":"/foo/bar","modifiedIndex":3,"createdIndex":2}}`, "") @@ -31,7 +33,7 @@ func TestV2DeleteKey(t *testing.T) { // Ensures that an empty directory is deleted when dir is set. // // $ curl -X PUT localhost:4001/v2/keys/foo?dir=true -// $ curl -X PUT localhost:4001/v2/keys/foo ->fail +// $ curl -X DELETE localhost:4001/v2/keys/foo ->fail // $ curl -X DELETE localhost:4001/v2/keys/foo?dir=true // func TestV2DeleteEmptyDirectory(t *testing.T) { @@ -39,9 +41,11 @@ func TestV2DeleteEmptyDirectory(t *testing.T) { resp, err := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo?dir=true"), url.Values{}) tests.ReadBody(resp) resp, err = tests.DeleteForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo"), url.Values{}) + assert.Equal(t, resp.StatusCode, http.StatusForbidden) bodyJson := tests.ReadBodyJSON(resp) assert.Equal(t, bodyJson["errorCode"], 102, "") resp, err = tests.DeleteForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo?dir=true"), url.Values{}) + assert.Equal(t, resp.StatusCode, http.StatusOK) body := tests.ReadBody(resp) assert.Nil(t, err, "") assert.Equal(t, string(body), `{"action":"delete","node":{"key":"/foo","dir":true,"modifiedIndex":3,"createdIndex":2}}`, "") @@ -59,9 +63,11 @@ func TestV2DeleteNonEmptyDirectory(t *testing.T) { resp, err := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar?dir=true"), url.Values{}) tests.ReadBody(resp) resp, err = tests.DeleteForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo?dir=true"), url.Values{}) + assert.Equal(t, resp.StatusCode, http.StatusForbidden) bodyJson := tests.ReadBodyJSON(resp) assert.Equal(t, bodyJson["errorCode"], 108, "") resp, err = tests.DeleteForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo?dir=true&recursive=true"), url.Values{}) + assert.Equal(t, resp.StatusCode, http.StatusOK) body := tests.ReadBody(resp) assert.Nil(t, err, "") assert.Equal(t, string(body), `{"action":"delete","node":{"key":"/foo","dir":true,"modifiedIndex":3,"createdIndex":2}}`, "") @@ -78,6 +84,7 @@ func TestV2DeleteDirectoryRecursiveImpliesDir(t *testing.T) { resp, err := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo?dir=true"), url.Values{}) tests.ReadBody(resp) resp, err = tests.DeleteForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo?recursive=true"), url.Values{}) + assert.Equal(t, resp.StatusCode, http.StatusOK) body := tests.ReadBody(resp) assert.Nil(t, err, "") assert.Equal(t, string(body), `{"action":"delete","node":{"key":"/foo","dir":true,"modifiedIndex":3,"createdIndex":2}}`, "") diff --git a/server/v2/tests/get_handler_test.go b/server/v2/tests/get_handler_test.go index a8dd0e1d3..97feaa314 100644 --- a/server/v2/tests/get_handler_test.go +++ b/server/v2/tests/get_handler_test.go @@ -2,6 +2,7 @@ package v2 import ( "fmt" + "net/http" "net/url" "testing" "time" @@ -13,6 +14,7 @@ import ( // Ensures that a value can be retrieve for a given key. // +// $ curl localhost:4001/v2/keys/foo/bar -> fail // $ curl -X PUT localhost:4001/v2/keys/foo/bar -d value=XXX // $ curl localhost:4001/v2/keys/foo/bar // @@ -20,9 +22,15 @@ func TestV2GetKey(t *testing.T) { tests.RunServer(func(s *server.Server) { v := url.Values{} v.Set("value", "XXX") - resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) + fullURL := fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar") + resp, _ := tests.Get(fullURL) + assert.Equal(t, resp.StatusCode, http.StatusNotFound) + + resp, _ = tests.PutForm(fullURL, v) tests.ReadBody(resp) - resp, _ = tests.Get(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar")) + + resp, _ = tests.Get(fullURL) + assert.Equal(t, resp.StatusCode, http.StatusOK) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["action"], "get", "") node := body["node"].(map[string]interface{}) @@ -51,6 +59,7 @@ func TestV2GetKeyRecursively(t *testing.T) { tests.ReadBody(resp) resp, _ = tests.Get(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo?recursive=true")) + assert.Equal(t, resp.StatusCode, http.StatusOK) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["action"], "get", "") node := body["node"].(map[string]interface{}) diff --git a/server/v2/tests/post_handler_test.go b/server/v2/tests/post_handler_test.go index a048d5cb5..92acfc90a 100644 --- a/server/v2/tests/post_handler_test.go +++ b/server/v2/tests/post_handler_test.go @@ -3,6 +3,7 @@ package v2 import ( "fmt" "testing" + "net/http" "github.com/coreos/etcd/server" "github.com/coreos/etcd/tests" @@ -18,7 +19,9 @@ import ( func TestV2CreateUnique(t *testing.T) { tests.RunServer(func(s *server.Server) { // POST should add index to list. - resp, _ := tests.PostForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), nil) + fullURL := fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar") + resp, _ := tests.PostForm(fullURL, nil) + assert.Equal(t, resp.StatusCode, http.StatusCreated) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["action"], "create", "") @@ -28,7 +31,8 @@ func TestV2CreateUnique(t *testing.T) { assert.Equal(t, node["modifiedIndex"], 2, "") // Second POST should add next index to list. - resp, _ = tests.PostForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), nil) + resp, _ = tests.PostForm(fullURL, nil) + assert.Equal(t, resp.StatusCode, http.StatusCreated) body = tests.ReadBodyJSON(resp) node = body["node"].(map[string]interface{}) @@ -36,6 +40,7 @@ func TestV2CreateUnique(t *testing.T) { // POST to a different key should add index to that list. resp, _ = tests.PostForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/baz"), nil) + assert.Equal(t, resp.StatusCode, http.StatusCreated) body = tests.ReadBodyJSON(resp) node = body["node"].(map[string]interface{}) diff --git a/server/v2/tests/put_handler_test.go b/server/v2/tests/put_handler_test.go index 7b51da4c2..f507c0d3d 100644 --- a/server/v2/tests/put_handler_test.go +++ b/server/v2/tests/put_handler_test.go @@ -2,6 +2,7 @@ package v2 import ( "fmt" + "net/http" "net/url" "testing" "time" @@ -20,6 +21,7 @@ func TestV2SetKey(t *testing.T) { v := url.Values{} v.Set("value", "XXX") resp, err := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) + assert.Equal(t, resp.StatusCode, http.StatusCreated) body := tests.ReadBody(resp) assert.Nil(t, err, "") assert.Equal(t, string(body), `{"action":"set","node":{"key":"/foo/bar","value":"XXX","modifiedIndex":2,"createdIndex":2}}`, "") @@ -33,6 +35,7 @@ func TestV2SetKey(t *testing.T) { func TestV2SetDirectory(t *testing.T) { tests.RunServer(func(s *server.Server) { resp, err := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo?dir=true"), url.Values{}) + assert.Equal(t, resp.StatusCode, http.StatusCreated) body := tests.ReadBody(resp) assert.Nil(t, err, "") assert.Equal(t, string(body), `{"action":"set","node":{"key":"/foo","dir":true,"modifiedIndex":2,"createdIndex":2}}`, "") @@ -50,6 +53,7 @@ func TestV2SetKeyWithTTL(t *testing.T) { v.Set("value", "XXX") v.Set("ttl", "20") resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) + assert.Equal(t, resp.StatusCode, http.StatusCreated) body := tests.ReadBodyJSON(resp) node := body["node"].(map[string]interface{}) assert.Equal(t, node["ttl"], 20, "") @@ -70,6 +74,7 @@ func TestV2SetKeyWithBadTTL(t *testing.T) { v.Set("value", "XXX") v.Set("ttl", "bad_ttl") resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) + assert.Equal(t, resp.StatusCode, http.StatusBadRequest) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["errorCode"], 202, "") assert.Equal(t, body["message"], "The given TTL in POST form is not a number", "") @@ -77,7 +82,7 @@ func TestV2SetKeyWithBadTTL(t *testing.T) { }) } -// Ensures that a key is conditionally set only if it previously did not exist. +// Ensures that a key is conditionally set if it previously did not exist. // // $ curl -X PUT localhost:4001/v2/keys/foo/bar -d value=XXX -d prevExist=false // @@ -87,25 +92,29 @@ func TestV2CreateKeySuccess(t *testing.T) { v.Set("value", "XXX") v.Set("prevExist", "false") resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) + assert.Equal(t, resp.StatusCode, http.StatusCreated) body := tests.ReadBodyJSON(resp) node := body["node"].(map[string]interface{}) assert.Equal(t, node["value"], "XXX", "") }) } -// Ensures that a key is not conditionally because it previously existed. +// Ensures that a key is not conditionally set because it previously existed. // -// $ curl -X PUT localhost:4001/v2/keys/foo/bar -d value=XXX // $ curl -X PUT localhost:4001/v2/keys/foo/bar -d value=XXX -d prevExist=false +// $ curl -X PUT localhost:4001/v2/keys/foo/bar -d value=XXX -d prevExist=false -> fail // func TestV2CreateKeyFail(t *testing.T) { tests.RunServer(func(s *server.Server) { v := url.Values{} v.Set("value", "XXX") v.Set("prevExist", "false") - resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) + fullURL := fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar") + resp, _ := tests.PutForm(fullURL, v) + assert.Equal(t, resp.StatusCode, http.StatusCreated) tests.ReadBody(resp) - resp, _ = tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ = tests.PutForm(fullURL, v) + assert.Equal(t, resp.StatusCode, http.StatusPreconditionFailed) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["errorCode"], 105, "") assert.Equal(t, body["message"], "Key already exists", "") @@ -123,12 +132,15 @@ func TestV2UpdateKeySuccess(t *testing.T) { v := url.Values{} v.Set("value", "XXX") - resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) + fullURL := fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar") + resp, _ := tests.PutForm(fullURL, v) + assert.Equal(t, resp.StatusCode, http.StatusCreated) tests.ReadBody(resp) v.Set("value", "YYY") v.Set("prevExist", "true") - resp, _ = tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ = tests.PutForm(fullURL, v) + assert.Equal(t, resp.StatusCode, http.StatusOK) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["action"], "update", "") }) @@ -144,9 +156,11 @@ func TestV2UpdateKeyFailOnValue(t *testing.T) { v := url.Values{} resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo?dir=true"), v) + assert.Equal(t, resp.StatusCode, http.StatusCreated) v.Set("value", "YYY") v.Set("prevExist", "true") resp, _ = tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) + assert.Equal(t, resp.StatusCode, http.StatusNotFound) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["errorCode"], 100, "") assert.Equal(t, body["message"], "Key not found", "") @@ -156,19 +170,27 @@ func TestV2UpdateKeyFailOnValue(t *testing.T) { // Ensures that a key is not conditionally set if it previously did not exist. // -// $ curl -X PUT localhost:4001/v2/keys/foo -d value=XXX -d prevExist=true -// $ curl -X PUT localhost:4001/v2/keys/foo/bar -d value=XXX -d prevExist=true +// $ curl -X PUT localhost:4001/v2/keys/foo -d value=YYY -d prevExist=true -> fail +// $ curl -X PUT localhost:4001/v2/keys/foo/bar -d value=YYY -d prevExist=true -> fail // func TestV2UpdateKeyFailOnMissingDirectory(t *testing.T) { tests.RunServer(func(s *server.Server) { v := url.Values{} v.Set("value", "YYY") v.Set("prevExist", "true") - resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo"), v) + assert.Equal(t, resp.StatusCode, http.StatusNotFound) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["errorCode"], 100, "") assert.Equal(t, body["message"], "Key not found", "") assert.Equal(t, body["cause"], "/foo", "") + + resp, _ = tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) + assert.Equal(t, resp.StatusCode, http.StatusNotFound) + body = tests.ReadBodyJSON(resp) + assert.Equal(t, body["errorCode"], 100, "") + assert.Equal(t, body["message"], "Key not found", "") + assert.Equal(t, body["cause"], "/foo", "") }) } @@ -181,11 +203,14 @@ func TestV2SetKeyCASOnIndexSuccess(t *testing.T) { tests.RunServer(func(s *server.Server) { v := url.Values{} v.Set("value", "XXX") - resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) + fullURL := fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar") + resp, _ := tests.PutForm(fullURL, v) + assert.Equal(t, resp.StatusCode, http.StatusCreated) tests.ReadBody(resp) v.Set("value", "YYY") v.Set("prevIndex", "2") - resp, _ = tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ = tests.PutForm(fullURL, v) + assert.Equal(t, resp.StatusCode, http.StatusOK) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["action"], "compareAndSwap", "") node := body["node"].(map[string]interface{}) @@ -203,11 +228,14 @@ func TestV2SetKeyCASOnIndexFail(t *testing.T) { tests.RunServer(func(s *server.Server) { v := url.Values{} v.Set("value", "XXX") - resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) + fullURL := fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar") + resp, _ := tests.PutForm(fullURL, v) + assert.Equal(t, resp.StatusCode, http.StatusCreated) tests.ReadBody(resp) v.Set("value", "YYY") v.Set("prevIndex", "10") - resp, _ = tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ = tests.PutForm(fullURL, v) + assert.Equal(t, resp.StatusCode, http.StatusPreconditionFailed) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["errorCode"], 101, "") assert.Equal(t, body["message"], "Compare failed", "") @@ -226,6 +254,7 @@ func TestV2SetKeyCASWithInvalidIndex(t *testing.T) { v.Set("value", "YYY") v.Set("prevIndex", "bad_index") resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) + assert.Equal(t, resp.StatusCode, http.StatusBadRequest) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["errorCode"], 203, "") assert.Equal(t, body["message"], "The given index in POST form is not a number", "") @@ -242,11 +271,14 @@ func TestV2SetKeyCASOnValueSuccess(t *testing.T) { tests.RunServer(func(s *server.Server) { v := url.Values{} v.Set("value", "XXX") - resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) + fullURL := fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar") + resp, _ := tests.PutForm(fullURL, v) + assert.Equal(t, resp.StatusCode, http.StatusCreated) tests.ReadBody(resp) v.Set("value", "YYY") v.Set("prevValue", "XXX") - resp, _ = tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ = tests.PutForm(fullURL, v) + assert.Equal(t, resp.StatusCode, http.StatusOK) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["action"], "compareAndSwap", "") node := body["node"].(map[string]interface{}) @@ -264,11 +296,14 @@ func TestV2SetKeyCASOnValueFail(t *testing.T) { tests.RunServer(func(s *server.Server) { v := url.Values{} v.Set("value", "XXX") - resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) + fullURL := fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar") + resp, _ := tests.PutForm(fullURL, v) + assert.Equal(t, resp.StatusCode, http.StatusCreated) tests.ReadBody(resp) v.Set("value", "YYY") v.Set("prevValue", "AAA") - resp, _ = tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) + resp, _ = tests.PutForm(fullURL, v) + assert.Equal(t, resp.StatusCode, http.StatusPreconditionFailed) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["errorCode"], 101, "") assert.Equal(t, body["message"], "Compare failed", "") @@ -287,6 +322,7 @@ func TestV2SetKeyCASWithMissingValueFails(t *testing.T) { v.Set("value", "XXX") v.Set("prevValue", "") resp, _ := tests.PutForm(fmt.Sprintf("%s%s", s.URL(), "/v2/keys/foo/bar"), v) + assert.Equal(t, resp.StatusCode, http.StatusBadRequest) body := tests.ReadBodyJSON(resp) assert.Equal(t, body["errorCode"], 201, "") assert.Equal(t, body["message"], "PrevValue is Required in POST form", "") diff --git a/tests/functional/v1_migration_test.go b/tests/functional/v1_migration_test.go index c8906a915..3a0512101 100644 --- a/tests/functional/v1_migration_test.go +++ b/tests/functional/v1_migration_test.go @@ -3,6 +3,7 @@ package test import ( "fmt" "io/ioutil" + "net/http" "os" "os/exec" "path/filepath" @@ -91,7 +92,7 @@ func TestV1ClusterMigration(t *testing.T) { resp, err := tests.Get("http://localhost:4001/v2/keys/message") body := tests.ReadBody(resp) assert.Nil(t, err, "") - assert.Equal(t, resp.StatusCode, 400) + assert.Equal(t, resp.StatusCode, http.StatusNotFound) assert.Equal(t, string(body), `{"errorCode":100,"message":"Key not found","cause":"/message","index":11}`+"\n") // Ensure TTL'd message is removed.