You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by ju...@apache.org on 2020/12/25 09:16:35 UTC
[apisix-dashboard] branch master updated: fix: when create a route
without ID in request body by Admin API,
it can't be displayed in Manager API (#1063)
This is an automated email from the ASF dual-hosted git repository.
juzhiyuan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/apisix-dashboard.git
The following commit(s) were added to refs/heads/master by this push:
new 95ca6e0 fix: when create a route without ID in request body by Admin API, it can't be displayed in Manager API (#1063)
95ca6e0 is described below
commit 95ca6e0436b23e9b65cb50efe8b527960f740b2e
Author: nic-chen <33...@users.noreply.github.com>
AuthorDate: Fri Dec 25 17:16:26 2020 +0800
fix: when create a route without ID in request body by Admin API, it can't be displayed in Manager API (#1063)
* fix: when the ID in body of a resource is empty, it cannot be displayed in the list
* test: add test case
* fix: unit test fail
* fix: error
* test: update test cases
* test: update test cases
* chore: naming style
* test: add test cases for create route by POST
* fix: unit test failed
* fix: manager api host port for e2e
* chore: sleep more time
* chore: code refactor
* fix: error
* fix: unit test failed
---
api/internal/core/entity/entity.go | 6 +
api/internal/core/storage/etcd.go | 10 +-
api/internal/core/storage/storage.go | 7 +-
api/internal/core/storage/storage_mock.go | 26 ++++-
api/internal/core/store/store.go | 24 ++--
api/internal/core/store/store_test.go | 53 +++++++--
api/internal/core/store/validate_mock.go | 25 +++--
api/test/e2e/base.go | 19 ++--
api/test/e2e/id_compatible_test.go | 180 ++++++++++++++++++++++++++----
9 files changed, 287 insertions(+), 63 deletions(-)
diff --git a/api/internal/core/entity/entity.go b/api/internal/core/entity/entity.go
index 1af128e..0e68078 100644
--- a/api/internal/core/entity/entity.go
+++ b/api/internal/core/entity/entity.go
@@ -52,6 +52,12 @@ func (info *BaseInfo) Updating(storedInfo *BaseInfo) {
info.UpdateTime = time.Now().Unix()
}
+func (info *BaseInfo) KeyCompat(key string) {
+ if info.ID == nil && key != "" {
+ info.ID = key
+ }
+}
+
type BaseInfoSetter interface {
GetBaseInfo() *BaseInfo
}
diff --git a/api/internal/core/storage/etcd.go b/api/internal/core/storage/etcd.go
index f378ba4..e73bf00 100644
--- a/api/internal/core/storage/etcd.go
+++ b/api/internal/core/storage/etcd.go
@@ -73,15 +73,19 @@ func (s *EtcdV3Storage) Get(ctx context.Context, key string) (string, error) {
return string(resp.Kvs[0].Value), nil
}
-func (s *EtcdV3Storage) List(ctx context.Context, key string) ([]string, error) {
+func (s *EtcdV3Storage) List(ctx context.Context, key string) ([]Keypair, error) {
resp, err := Client.Get(ctx, key, clientv3.WithPrefix())
if err != nil {
log.Errorf("etcd get failed: %s", err)
return nil, fmt.Errorf("etcd get failed: %s", err)
}
- var ret []string
+ var ret []Keypair
for i := range resp.Kvs {
- ret = append(ret, string(resp.Kvs[i].Value))
+ data := Keypair{
+ Key: string(resp.Kvs[i].Key),
+ Value: string(resp.Kvs[i].Value),
+ }
+ ret = append(ret, data)
}
return ret, nil
diff --git a/api/internal/core/storage/storage.go b/api/internal/core/storage/storage.go
index c4b5399..ba30772 100644
--- a/api/internal/core/storage/storage.go
+++ b/api/internal/core/storage/storage.go
@@ -20,7 +20,7 @@ import "context"
type Interface interface {
Get(ctx context.Context, key string) (string, error)
- List(ctx context.Context, key string) ([]string, error)
+ List(ctx context.Context, key string) ([]Keypair, error)
Create(ctx context.Context, key, val string) error
Update(ctx context.Context, key, val string) error
BatchDelete(ctx context.Context, keys []string) error
@@ -33,6 +33,11 @@ type WatchResponse struct {
Canceled bool
}
+type Keypair struct {
+ Key string
+ Value string
+}
+
type Event struct {
Type EventType
Key string
diff --git a/api/internal/core/storage/storage_mock.go b/api/internal/core/storage/storage_mock.go
index c5063a9..f85d8b7 100644
--- a/api/internal/core/storage/storage_mock.go
+++ b/api/internal/core/storage/storage_mock.go
@@ -1,5 +1,19 @@
-// Code generated by mockery v1.0.0. DO NOT EDIT.
-
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
package storage
import (
@@ -63,15 +77,15 @@ func (_m *MockInterface) Get(ctx context.Context, key string) (string, error) {
}
// List provides a mock function with given fields: ctx, key
-func (_m *MockInterface) List(ctx context.Context, key string) ([]string, error) {
+func (_m *MockInterface) List(ctx context.Context, key string) ([]Keypair, error) {
ret := _m.Called(ctx, key)
- var r0 []string
- if rf, ok := ret.Get(0).(func(context.Context, string) []string); ok {
+ var r0 []Keypair
+ if rf, ok := ret.Get(0).(func(context.Context, string) []Keypair); ok {
r0 = rf(ctx, key)
} else {
if ret.Get(0) != nil {
- r0 = ret.Get(0).([]string)
+ r0 = ret.Get(0).([]Keypair)
}
}
diff --git a/api/internal/core/store/store.go b/api/internal/core/store/store.go
index ee3fa50..435f3a1 100644
--- a/api/internal/core/store/store.go
+++ b/api/internal/core/store/store.go
@@ -96,13 +96,15 @@ func (s *GenericStore) Init() error {
return err
}
for i := range ret {
- if ret[i] == "init_dir" {
+ if ret[i].Value == "init_dir" {
continue
}
- objPtr, err := s.StringToObjPtr(ret[i])
+ key := ret[i].Key[len(s.opt.BasePath)+1:]
+ objPtr, err := s.StringToObjPtr(ret[i].Value, key)
if err != nil {
return err
}
+
s.cache.Store(s.opt.KeyFunc(objPtr), objPtr)
}
@@ -117,12 +119,13 @@ func (s *GenericStore) Init() error {
for i := range event.Events {
switch event.Events[i].Type {
case storage.EventTypePut:
- objPtr, err := s.StringToObjPtr(event.Events[i].Value)
+ key := event.Events[i].Key[len(s.opt.BasePath)+1:]
+ objPtr, err := s.StringToObjPtr(event.Events[i].Value, key)
if err != nil {
log.Warnf("value convert to obj failed: %s", err)
continue
}
- s.cache.Store(event.Events[i].Key[len(s.opt.BasePath)+1:], objPtr)
+ s.cache.Store(key, objPtr)
case storage.EventTypeDelete:
s.cache.Delete(event.Events[i].Key[len(s.opt.BasePath)+1:])
}
@@ -320,15 +323,22 @@ func (s *GenericStore) Close() error {
return nil
}
-func (s *GenericStore) StringToObjPtr(str string) (interface{}, error) {
+func (s *GenericStore) StringToObjPtr(str, key string) (interface{}, error) {
objPtr := reflect.New(s.opt.ObjType)
- err := json.Unmarshal([]byte(str), objPtr.Interface())
+ ret := objPtr.Interface()
+ err := json.Unmarshal([]byte(str), ret)
+ fmt.Println("ret:", ret, "s.opt.ObjType", s.opt.ObjType)
if err != nil {
log.Errorf("json marshal failed: %s", err)
return nil, fmt.Errorf("json unmarshal failed: %s", err)
}
- return objPtr.Interface(), nil
+ if setter, ok := ret.(entity.BaseInfoSetter); ok {
+ info := setter.GetBaseInfo()
+ info.KeyCompat(key)
+ }
+
+ return ret, nil
}
func (s *GenericStore) GetObjStorageKey(obj interface{}) string {
diff --git a/api/internal/core/store/store_test.go b/api/internal/core/store/store_test.go
index 20c5f7b..9bc1159 100644
--- a/api/internal/core/store/store_test.go
+++ b/api/internal/core/store/store_test.go
@@ -115,7 +115,7 @@ func TestGenericStore_Init(t *testing.T) {
caseDesc string
giveStore *GenericStore
giveListErr error
- giveListRet []string
+ giveListRet []storage.Keypair
giveWatchCh chan storage.WatchResponse
giveResp storage.WatchResponse
wantErr error
@@ -134,9 +134,15 @@ func TestGenericStore_Init(t *testing.T) {
},
},
},
- giveListRet: []string{
- `{"Field1":"demo1-f1", "Field2":"demo1-f2"}`,
- `{"Field1":"demo2-f1", "Field2":"demo2-f2"}`,
+ giveListRet: []storage.Keypair{
+ {
+ Key: "test/demo1-f1",
+ Value: `{"Field1":"demo1-f1", "Field2":"demo1-f2"}`,
+ },
+ {
+ Key: "test/demo2-f1",
+ Value: `{"Field1":"demo2-f1", "Field2":"demo2-f2"}`,
+ },
},
giveWatchCh: make(chan storage.WatchResponse),
giveResp: storage.WatchResponse{
@@ -154,12 +160,14 @@ func TestGenericStore_Init(t *testing.T) {
},
wantCache: map[string]interface{}{
"demo2-f1": &TestStruct{
- Field1: "demo2-f1",
- Field2: "demo2-f2",
+ BaseInfo: entity.BaseInfo{ID: "demo2-f1"},
+ Field1: "demo2-f1",
+ Field2: "demo2-f2",
},
"demo3-f1": &TestStruct{
- Field1: "demo3-f1",
- Field2: "demo3-f2",
+ BaseInfo: entity.BaseInfo{ID: "demo3-f1"},
+ Field1: "demo3-f1",
+ Field2: "demo3-f2",
},
},
wantListCalled: true,
@@ -183,9 +191,15 @@ func TestGenericStore_Init(t *testing.T) {
},
},
},
- giveListRet: []string{
- `{"Field1","demo1-f1", "Field2":"demo1-f2"}`,
- `{"Field1":"demo2-f1", "Field2":"demo2-f2"}`,
+ giveListRet: []storage.Keypair{
+ {
+ Key: "test/demo1-f1",
+ Value: `{"Field1","demo1-f1", "Field2":"demo1-f2"}`,
+ },
+ {
+ Key: "test/demo2-f1",
+ Value: `{"Field1":"demo2-f1", "Field2":"demo2-f2"}`,
+ },
},
wantErr: fmt.Errorf("json unmarshal failed: invalid character ',' after object key"),
wantListCalled: true,
@@ -765,3 +779,20 @@ func TestGenericStore_Delete(t *testing.T) {
assert.Equal(t, tc.wantErr, err, tc.caseDesc)
}
}
+
+func TestGenericStore_StringToObjPtr(t *testing.T) {
+ s, err := NewGenericStore(GenericStoreOption{
+ BasePath: "test",
+ ObjType: reflect.TypeOf(entity.SSL{}),
+ KeyFunc: func(obj interface{}) string {
+ r := obj.(*entity.Route)
+ return utils.InterfaceToString(r.ID)
+ },
+ })
+ assert.Nil(t, err)
+ id := "1"
+ sslStr := `{"key":"test_key", "cert":"test_cert"}`
+ sslInterface, err := s.StringToObjPtr(sslStr, id)
+ ssl := sslInterface.(*entity.SSL)
+ assert.Equal(t, id, ssl.ID)
+}
diff --git a/api/internal/core/store/validate_mock.go b/api/internal/core/store/validate_mock.go
index 643ba78..ad6922c 100644
--- a/api/internal/core/store/validate_mock.go
+++ b/api/internal/core/store/validate_mock.go
@@ -1,5 +1,19 @@
-// Code generated by mockery v1.0.0. DO NOT EDIT.
-
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
package store
import mock "github.com/stretchr/testify/mock"
@@ -13,12 +27,9 @@ type MockValidator struct {
func (_m *MockValidator) Validate(obj interface{}) error {
ret := _m.Called(obj)
- var r0 error
if rf, ok := ret.Get(0).(func(interface{}) error); ok {
- r0 = rf(obj)
- } else {
- r0 = ret.Error(0)
+ return rf(obj)
}
- return r0
+ return ret.Error(0)
}
diff --git a/api/test/e2e/base.go b/api/test/e2e/base.go
index 7077a85..5c79f85 100644
--- a/api/test/e2e/base.go
+++ b/api/test/e2e/base.go
@@ -20,7 +20,7 @@ import (
"bytes"
"context"
"crypto/tls"
- "github.com/stretchr/testify/assert"
+
"io/ioutil"
"net"
"net/http"
@@ -28,11 +28,16 @@ import (
"time"
"github.com/gavv/httpexpect/v2"
+ "github.com/stretchr/testify/assert"
"github.com/tidwall/gjson"
)
var token string
+
+var APISIXHost = "http://127.0.0.1:9080"
var APISIXInternalUrl = "http://172.16.238.30:9080"
+var APISIXSingleWorkerHost = "http://127.0.0.1:9081"
+var ManagerAPIHost = "http://127.0.0.1:9000"
func init() {
//login to get auth token
@@ -41,7 +46,7 @@ func init() {
"password": "admin"
}`)
- url := "http://127.0.0.1:9000/apisix/admin/user/login"
+ url := ManagerAPIHost + "/apisix/admin/user/login"
req, err := http.NewRequest(http.MethodPost, url, bytes.NewBuffer(requestBody))
if err != nil {
panic(err)
@@ -86,11 +91,11 @@ func httpGet(url string) ([]byte, int, error) {
}
func ManagerApiExpect(t *testing.T) *httpexpect.Expect {
- return httpexpect.New(t, "http://127.0.0.1:9000")
+ return httpexpect.New(t, ManagerAPIHost)
}
func APISIXExpect(t *testing.T) *httpexpect.Expect {
- return httpexpect.New(t, "http://127.0.0.1:9080")
+ return httpexpect.New(t, APISIXHost)
}
func APISIXHTTPSExpect(t *testing.T) *httpexpect.Expect {
@@ -117,10 +122,8 @@ func APISIXHTTPSExpect(t *testing.T) *httpexpect.Expect {
return e
}
-var singleWorkerAPISIXHost = "http://127.0.0.1:9081"
-
func BatchTestServerPort(t *testing.T, times int) map[string]int {
- url := singleWorkerAPISIXHost + "/server_port"
+ url := APISIXSingleWorkerHost + "/server_port"
req, err := http.NewRequest(http.MethodGet, url, nil)
assert.Nil(t, err)
@@ -197,6 +200,8 @@ func testCaseCheck(tc HttpTestCase, t *testing.T) {
if tc.Sleep != 0 {
time.Sleep(tc.Sleep)
+ } else {
+ time.Sleep(time.Duration(50) * time.Millisecond)
}
if tc.Query != "" {
diff --git a/api/test/e2e/id_compatible_test.go b/api/test/e2e/id_compatible_test.go
index 21b6558..90c3322 100644
--- a/api/test/e2e/id_compatible_test.go
+++ b/api/test/e2e/id_compatible_test.go
@@ -17,8 +17,14 @@
package e2e
import (
+ "fmt"
+ "io/ioutil"
"net/http"
"testing"
+ "time"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/tidwall/gjson"
)
func TestID_Using_Int(t *testing.T) {
@@ -29,13 +35,13 @@ func TestID_Using_Int(t *testing.T) {
Method: http.MethodPut,
Path: "/apisix/admin/upstreams",
Body: `{
- "id": 1,
- "nodes": [{
- "host": "172.16.238.20",
- "port": 1980,
- "weight": 1
- }],
- "type": "roundrobin"
+ "id": 1,
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }],
+ "type": "roundrobin"
}`,
Headers: map[string]string{"Authorization": token},
ExpectStatus: http.StatusOK,
@@ -162,13 +168,13 @@ func TestID_Using_String(t *testing.T) {
Method: http.MethodPut,
Path: "/apisix/admin/upstreams",
Body: `{
- "id": "2",
- "nodes": [{
- "host": "172.16.238.20",
- "port": 1980,
- "weight": 1
- }],
- "type": "roundrobin"
+ "id": "2",
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }],
+ "type": "roundrobin"
}`,
Headers: map[string]string{"Authorization": token},
ExpectStatus: http.StatusOK,
@@ -210,6 +216,7 @@ func TestID_Using_String(t *testing.T) {
Path: "/apisix/admin/upstreams/2",
Headers: map[string]string{"Authorization": token},
ExpectStatus: http.StatusOK,
+ Sleep: sleepTime,
},
{
Desc: "make sure the upstream has been deleted",
@@ -243,13 +250,13 @@ func TestID_Crossing(t *testing.T) {
Method: http.MethodPut,
Path: "/apisix/admin/upstreams",
Body: `{
- "id": 3,
- "nodes": [{
- "host": "172.16.238.20",
- "port": 1980,
- "weight": 1
- }],
- "type": "roundrobin"
+ "id": 3,
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }],
+ "type": "roundrobin"
}`,
Headers: map[string]string{"X-API-KEY": "edd1c9f034335f136f87ad84b625c8f1"},
ExpectStatus: http.StatusCreated,
@@ -311,6 +318,7 @@ func TestID_Crossing(t *testing.T) {
Path: "/apisix/admin/upstreams/3",
Headers: map[string]string{"Authorization": token},
ExpectStatus: http.StatusOK,
+ Sleep: sleepTime,
},
{
Desc: "make sure the upstream has been deleted",
@@ -335,3 +343,133 @@ func TestID_Crossing(t *testing.T) {
testCaseCheck(tc, t)
}
}
+
+func TestID_Not_In_Body(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ Desc: "make sure the route is not created",
+ Object: APISIXExpect(t),
+ Method: http.MethodGet,
+ Path: "/hello",
+ ExpectStatus: http.StatusNotFound,
+ Sleep: sleepTime,
+ },
+ {
+ Desc: "create route that has no ID in request body by admin api",
+ Object: APISIXExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "172.16.238.20:1980": 1
+ }
+ }
+ }`,
+ Headers: map[string]string{"X-API-KEY": "edd1c9f034335f136f87ad84b625c8f1"},
+ ExpectStatus: http.StatusCreated,
+ Sleep: sleepTime,
+ },
+ {
+ Desc: "verify that the route is available for manager api",
+ Object: ManagerApiExpect(t),
+ Method: http.MethodGet,
+ Path: "/apisix/admin/routes/r1",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusOK,
+ ExpectBody: `"id":"r1"`,
+ Sleep: sleepTime,
+ },
+ {
+ Desc: "hit the route just created",
+ Object: APISIXExpect(t),
+ Method: http.MethodGet,
+ Path: "/hello",
+ ExpectStatus: http.StatusOK,
+ ExpectBody: "hello world",
+ Sleep: sleepTime,
+ },
+ {
+ Desc: "delete the route",
+ Object: ManagerApiExpect(t),
+ Method: http.MethodDelete,
+ Path: "/apisix/admin/routes/r1",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusOK,
+ },
+ {
+ Desc: "hit deleted route",
+ Object: APISIXExpect(t),
+ Method: http.MethodGet,
+ Path: "/hello",
+ ExpectStatus: http.StatusNotFound,
+ Sleep: sleepTime,
+ },
+ {
+ Desc: "create route that has no ID in request body by admin api (POST)",
+ Object: APISIXExpect(t),
+ Method: http.MethodPost,
+ Path: "/apisix/admin/routes",
+ Body: `{
+ "uri": "/hello",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "172.16.238.20:1980": 1
+ }
+ }
+ }`,
+ Headers: map[string]string{"X-API-KEY": "edd1c9f034335f136f87ad84b625c8f1"},
+ ExpectStatus: http.StatusCreated,
+ Sleep: sleepTime,
+ },
+ {
+ Desc: "verify that the route is available for manager api",
+ Object: ManagerApiExpect(t),
+ Method: http.MethodGet,
+ Path: "/apisix/admin/routes",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusOK,
+ ExpectBody: `"uri":"/hello"`,
+ Sleep: sleepTime,
+ },
+ {
+ Desc: "hit the route just created",
+ Object: APISIXExpect(t),
+ Method: http.MethodGet,
+ Path: "/hello",
+ ExpectStatus: http.StatusOK,
+ ExpectBody: "hello world",
+ Sleep: sleepTime,
+ },
+ }
+
+ for _, tc := range tests {
+ testCaseCheck(tc, t)
+ }
+
+ // delete the route created by POST
+ time.Sleep(time.Duration(100) * time.Millisecond)
+ request, _ := http.NewRequest("GET", ManagerAPIHost+"/apisix/admin/routes", nil)
+ request.Header.Add("Authorization", token)
+ resp, err := http.DefaultClient.Do(request)
+ assert.Nil(t, err)
+ defer resp.Body.Close()
+ respBody, _ := ioutil.ReadAll(resp.Body)
+ fmt.Println("string(respBody)", string(respBody))
+ list := gjson.Get(string(respBody), "data.rows").Value().([]interface{})
+ for _, item := range list {
+ route := item.(map[string]interface{})
+ tc := HttpTestCase{
+ Desc: "delete the route",
+ Object: ManagerApiExpect(t),
+ Method: http.MethodDelete,
+ Path: "/apisix/admin/routes/" + route["id"].(string),
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusOK,
+ }
+ testCaseCheck(tc, t)
+ }
+}