From 238ae5d44c2f84a93820bba5dcb29ab62b352feb Mon Sep 17 00:00:00 2001 From: nic-chen Date: Wed, 23 Dec 2020 22:28:12 +0800 Subject: [PATCH 1/5] fix: not allowed to specify `create_time` and `update_time` when create/edit route, service, upstream and consumer --- api/filter/schema.go | 12 ++++ .../e2e/route_with_management_fileds_test.go | 72 +++++++++++++++++++ 2 files changed, 84 insertions(+) diff --git a/api/filter/schema.go b/api/filter/schema.go index 64ba821b9b..5153c3a455 100644 --- a/api/filter/schema.go +++ b/api/filter/schema.go @@ -106,6 +106,18 @@ func parseCert(crt, key string) ([]string, error) { } func handleSpecialField(resource string, reqBody []byte) ([]byte, error) { + var bodyMap map[string]interface{} + err := json.Unmarshal(reqBody, &bodyMap) + if err != nil { + return reqBody, fmt.Errorf("read request body failed: %s", err) + } + if _, ok := bodyMap["create_time"]; ok { + return reqBody, fmt.Errorf("not support specifying create_time") + } + if _, ok := bodyMap["update_time"]; ok { + return reqBody, fmt.Errorf("not support specifying update_time") + } + // remove script, because it's a map, and need to be parsed into lua code if resource == "routes" { var route map[string]interface{} diff --git a/api/test/e2e/route_with_management_fileds_test.go b/api/test/e2e/route_with_management_fileds_test.go index 59c40fd735..e65279a617 100644 --- a/api/test/e2e/route_with_management_fileds_test.go +++ b/api/test/e2e/route_with_management_fileds_test.go @@ -363,3 +363,75 @@ func TestRoute_search_by_label(t *testing.T) { testCaseCheck(tc) } } + +func TestRoute_With_Create_Time(t *testing.T) { + tests := []HttpTestCase{ + { + caseDesc: "create route with create_time", + Object: ManagerApiExpect(t), + Path: "/apisix/admin/routes/r1", + Method: http.MethodPut, + Body: `{ + "uri": "/hello", + "create_time": 1, + "upstream": { + "nodes": { + "172.16.238.20:1980": 1 + }, + "type": "roundrobin" + } + }`, + Headers: map[string]string{"Authorization": token}, + ExpectStatus: http.StatusBadRequest, + }, + { + caseDesc: "create route with update_time", + Object: ManagerApiExpect(t), + Path: "/apisix/admin/routes/r1", + Method: http.MethodPut, + Body: `{ + "uri": "/hello", + "update_time": 1, + "upstream": { + "nodes": { + "172.16.238.20:1980": 1 + }, + "type": "roundrobin" + } + }`, + Headers: map[string]string{"Authorization": token}, + ExpectStatus: http.StatusBadRequest, + }, + { + caseDesc: "create route with create_time and update_time", + Object: ManagerApiExpect(t), + Path: "/apisix/admin/routes/r1", + Method: http.MethodPut, + Body: `{ + "uri": "/hello", + "create_time": 1, + "update_time": 1, + "upstream": { + "nodes": { + "172.16.238.20:1980": 1 + }, + "type": "roundrobin" + } + }`, + Headers: map[string]string{"Authorization": token}, + ExpectStatus: http.StatusBadRequest, + }, + { + caseDesc: "make sure the route not created", + Object: APISIXExpect(t), + Method: http.MethodGet, + Path: "/hello", + ExpectStatus: http.StatusNotFound, + ExpectBody: `{"error_msg":"404 Route Not Found"}`, + }, + } + + for _, tc := range tests { + testCaseCheck(tc) + } +} From a4b492690e2141fcf4449304ad0274d4a33e4623 Mon Sep 17 00:00:00 2001 From: nic-chen Date: Thu, 24 Dec 2020 14:54:59 +0800 Subject: [PATCH 2/5] fix: use reasonable timestamp --- api/test/e2e/route_with_management_fileds_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/api/test/e2e/route_with_management_fileds_test.go b/api/test/e2e/route_with_management_fileds_test.go index e65279a617..03bda5356f 100644 --- a/api/test/e2e/route_with_management_fileds_test.go +++ b/api/test/e2e/route_with_management_fileds_test.go @@ -373,7 +373,7 @@ func TestRoute_With_Create_Time(t *testing.T) { Method: http.MethodPut, Body: `{ "uri": "/hello", - "create_time": 1, + "create_time": 1608792721, "upstream": { "nodes": { "172.16.238.20:1980": 1 @@ -391,7 +391,7 @@ func TestRoute_With_Create_Time(t *testing.T) { Method: http.MethodPut, Body: `{ "uri": "/hello", - "update_time": 1, + "update_time": 1608792721, "upstream": { "nodes": { "172.16.238.20:1980": 1 @@ -409,8 +409,8 @@ func TestRoute_With_Create_Time(t *testing.T) { Method: http.MethodPut, Body: `{ "uri": "/hello", - "create_time": 1, - "update_time": 1, + "create_time": 1608792721, + "update_time": 1608792721, "upstream": { "nodes": { "172.16.238.20:1980": 1 From d64c81d912f1ccaa63da5d527bf2bfc8fa09cac8 Mon Sep 17 00:00:00 2001 From: nic-chen Date: Thu, 24 Dec 2020 15:09:39 +0800 Subject: [PATCH 3/5] chore: update error message --- api/filter/schema.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/filter/schema.go b/api/filter/schema.go index 5153c3a455..2baaa7aa76 100644 --- a/api/filter/schema.go +++ b/api/filter/schema.go @@ -112,10 +112,10 @@ func handleSpecialField(resource string, reqBody []byte) ([]byte, error) { return reqBody, fmt.Errorf("read request body failed: %s", err) } if _, ok := bodyMap["create_time"]; ok { - return reqBody, fmt.Errorf("not support specifying create_time") + return reqBody, fmt.Errorf("we don't accept create_time from client") } if _, ok := bodyMap["update_time"]; ok { - return reqBody, fmt.Errorf("not support specifying update_time") + return reqBody, fmt.Errorf("we don't accept update_time from client") } // remove script, because it's a map, and need to be parsed into lua code From 9f2d573109956069af290d940ad806c635d9000a Mon Sep 17 00:00:00 2001 From: nic-chen Date: Thu, 24 Dec 2020 20:06:36 +0800 Subject: [PATCH 4/5] test: update test cases --- .../e2e/route_with_management_fileds_test.go | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/api/test/e2e/route_with_management_fileds_test.go b/api/test/e2e/route_with_management_fileds_test.go index 7bfcca8d37..207cba92a2 100644 --- a/api/test/e2e/route_with_management_fileds_test.go +++ b/api/test/e2e/route_with_management_fileds_test.go @@ -367,10 +367,10 @@ func TestRoute_search_by_label(t *testing.T) { func TestRoute_With_Create_Time(t *testing.T) { tests := []HttpTestCase{ { - caseDesc: "create route with create_time", - Object: ManagerApiExpect(t), - Path: "/apisix/admin/routes/r1", - Method: http.MethodPut, + Desc: "create route with create_time", + Object: ManagerApiExpect(t), + Path: "/apisix/admin/routes/r1", + Method: http.MethodPut, Body: `{ "uri": "/hello", "create_time": 1608792721, @@ -385,10 +385,10 @@ func TestRoute_With_Create_Time(t *testing.T) { ExpectStatus: http.StatusBadRequest, }, { - caseDesc: "create route with update_time", - Object: ManagerApiExpect(t), - Path: "/apisix/admin/routes/r1", - Method: http.MethodPut, + Desc: "create route with update_time", + Object: ManagerApiExpect(t), + Path: "/apisix/admin/routes/r1", + Method: http.MethodPut, Body: `{ "uri": "/hello", "update_time": 1608792721, @@ -403,10 +403,10 @@ func TestRoute_With_Create_Time(t *testing.T) { ExpectStatus: http.StatusBadRequest, }, { - caseDesc: "create route with create_time and update_time", - Object: ManagerApiExpect(t), - Path: "/apisix/admin/routes/r1", - Method: http.MethodPut, + Desc: "create route with create_time and update_time", + Object: ManagerApiExpect(t), + Path: "/apisix/admin/routes/r1", + Method: http.MethodPut, Body: `{ "uri": "/hello", "create_time": 1608792721, @@ -422,7 +422,7 @@ func TestRoute_With_Create_Time(t *testing.T) { ExpectStatus: http.StatusBadRequest, }, { - caseDesc: "make sure the route not created", + Desc: "make sure the route not created", Object: APISIXExpect(t), Method: http.MethodGet, Path: "/hello", @@ -432,6 +432,6 @@ func TestRoute_With_Create_Time(t *testing.T) { } for _, tc := range tests { - testCaseCheck(tc) + testCaseCheck(tc, t) } } From ec69b20c9d5f09a0cdedc732131aaa29a338eda5 Mon Sep 17 00:00:00 2001 From: nic-chen Date: Thu, 24 Dec 2020 21:52:07 +0800 Subject: [PATCH 5/5] fix: according to review --- api/filter/schema.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/api/filter/schema.go b/api/filter/schema.go index 2baaa7aa76..687b21ab17 100644 --- a/api/filter/schema.go +++ b/api/filter/schema.go @@ -112,10 +112,10 @@ func handleSpecialField(resource string, reqBody []byte) ([]byte, error) { return reqBody, fmt.Errorf("read request body failed: %s", err) } if _, ok := bodyMap["create_time"]; ok { - return reqBody, fmt.Errorf("we don't accept create_time from client") + return reqBody, errors.New("we don't accept create_time from client") } if _, ok := bodyMap["update_time"]; ok { - return reqBody, fmt.Errorf("we don't accept update_time from client") + return reqBody, errors.New("we don't accept update_time from client") } // remove script, because it's a map, and need to be parsed into lua code