From 274ea691c77a36c62b9bd0724c5392e2dd562575 Mon Sep 17 00:00:00 2001 From: nic-chen Date: Tue, 15 Dec 2020 17:17:10 +0800 Subject: [PATCH 01/15] feat: support global rules --- api/internal/core/entity/entity.go | 6 + api/internal/core/store/storehub.go | 13 +- .../handler/global_rule/global_rule.go | 140 ++++++++ .../handler/global_rule/global_rule_test.go | 338 ++++++++++++++++++ api/internal/route.go | 3 + 5 files changed, 494 insertions(+), 6 deletions(-) create mode 100644 api/internal/handler/global_rule/global_rule.go create mode 100644 api/internal/handler/global_rule/global_rule_test.go diff --git a/api/internal/core/entity/entity.go b/api/internal/core/entity/entity.go index e1270c83c2..7f25d93e99 100644 --- a/api/internal/core/entity/entity.go +++ b/api/internal/core/entity/entity.go @@ -225,3 +225,9 @@ type Script struct { ID string `json:"id"` Script interface{} `json:"script,omitempty"` } + +// swagger:model GlobalPlugins +type GlobalPlugins struct { + ID interface{} `json:"id"` + Plugins map[string]interface{} `json:"plugins,omitempty"` +} diff --git a/api/internal/core/store/storehub.go b/api/internal/core/store/storehub.go index 25a5f56d44..ac46a1229c 100644 --- a/api/internal/core/store/storehub.go +++ b/api/internal/core/store/storehub.go @@ -28,12 +28,13 @@ import ( type HubKey string const ( - HubKeyConsumer HubKey = "consumer" - HubKeyRoute HubKey = "route" - HubKeyService HubKey = "service" - HubKeySsl HubKey = "ssl" - HubKeyUpstream HubKey = "upstream" - HubKeyScript HubKey = "script" + HubKeyConsumer HubKey = "consumer" + HubKeyRoute HubKey = "route" + HubKeyService HubKey = "service" + HubKeySsl HubKey = "ssl" + HubKeyUpstream HubKey = "upstream" + HubKeyScript HubKey = "script" + HubKeyGlobalRule HubKey = "global_rules" ) var ( diff --git a/api/internal/handler/global_rule/global_rule.go b/api/internal/handler/global_rule/global_rule.go new file mode 100644 index 0000000000..eb8b7a25a1 --- /dev/null +++ b/api/internal/handler/global_rule/global_rule.go @@ -0,0 +1,140 @@ +/* + * 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 global_rule + +import ( + "github.com/apisix/manager-api/internal/core/entity" + "github.com/apisix/manager-api/internal/core/store" + "reflect" + "strings" + + "github.com/gin-gonic/gin" + "github.com/shiningrush/droplet" + "github.com/shiningrush/droplet/wrapper" + wgin "github.com/shiningrush/droplet/wrapper/gin" + + "github.com/apisix/manager-api/internal/handler" +) + +type Handler struct { + globalRuleStore store.Interface +} + +func NewHandler() (handler.RouteRegister, error) { + return &Handler{ + globalRuleStore: store.GetStore(store.HubKeyGlobalRule), + }, nil +} + +func (h *Handler) ApplyRoute(r *gin.Engine) { + // global plugins + r.GET("/apisix/admin/global_rules/:id", wgin.Wraps(h.Get, + wrapper.InputType(reflect.TypeOf(GetInput{})))) + r.GET("/apisix/admin/global_rules", wgin.Wraps(h.List, + wrapper.InputType(reflect.TypeOf(ListInput{})))) + r.PUT("/apisix/admin/global_rules/:id", wgin.Wraps(h.Set, + wrapper.InputType(reflect.TypeOf(entity.GlobalPlugins{})))) + r.PUT("/apisix/admin/global_rules", wgin.Wraps(h.Set, + wrapper.InputType(reflect.TypeOf(entity.GlobalPlugins{})))) + r.DELETE("/apisix/admin/global_rules/:ids", wgin.Wraps(h.BatchDelete, + wrapper.InputType(reflect.TypeOf(BatchDeleteInput{})))) +} + +type GetInput struct { + ID string `auto_read:"id,path" validate:"required"` +} + +func (h *Handler) Get(c droplet.Context) (interface{}, error) { + input := c.Input().(*GetInput) + + r, err := h.globalRuleStore.Get(input.ID) + if err != nil { + return handler.SpecCodeResponse(err), err + } + return r, nil +} + +type ListInput struct { + store.Pagination +} + +// swagger:operation GET /apisix/admin/global_rules getGlobalRuleList +// +// Return the global rule list according to the specified page number and page size. +// +// --- +// produces: +// - application/json +// parameters: +// - name: page +// in: query +// description: page number +// required: false +// type: integer +// - name: page_size +// in: query +// description: page size +// required: false +// type: integer +// responses: +// '0': +// description: list response +// schema: +// type: array +// items: +// "$ref": "#/definitions/GlobalPlugins" +// default: +// description: unexpected error +// schema: +// "$ref": "#/definitions/ApiError" +func (h *Handler) List(c droplet.Context) (interface{}, error) { + input := c.Input().(*ListInput) + + ret, err := h.globalRuleStore.List(store.ListInput{ + PageSize: input.PageSize, + PageNumber: input.PageNumber, + }) + if err != nil { + return nil, err + } + + return ret, nil +} + +func (h *Handler) Set(c droplet.Context) (interface{}, error) { + input := c.Input().(*entity.GlobalPlugins) + + if err := h.globalRuleStore.Create(c.Context(), input); err != nil { + return handler.SpecCodeResponse(err), err + } + + return nil, nil +} + +type BatchDeleteInput struct { + IDs string `auto_read:"ids,path"` +} + +func (h *Handler) BatchDelete(c droplet.Context) (interface{}, error) { + input := c.Input().(*BatchDeleteInput) + + if err := h.globalRuleStore.BatchDelete(c.Context(), strings.Split(input.IDs, ",")); err != nil { + return handler.SpecCodeResponse(err), err + } + + return nil, nil +} diff --git a/api/internal/handler/global_rule/global_rule_test.go b/api/internal/handler/global_rule/global_rule_test.go new file mode 100644 index 0000000000..c7b15fdded --- /dev/null +++ b/api/internal/handler/global_rule/global_rule_test.go @@ -0,0 +1,338 @@ +/* + * 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 global_rule + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "testing" + + "github.com/gin-gonic/gin" + "github.com/shiningrush/droplet" + "github.com/shiningrush/droplet/data" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + + "github.com/apisix/manager-api/internal/core/entity" + "github.com/apisix/manager-api/internal/core/store" +) + +func performRequest(r http.Handler, method, path string) *httptest.ResponseRecorder { + req := httptest.NewRequest(method, path, nil) + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + return w +} + +func TestHandler_ApplyRoute(t *testing.T) { + mStore := &store.MockInterface{} + giveRet := `{ + "id": "test", + "plugins": { + "jwt-auth": {} + } + }` + mStore.On("Get", mock.Anything).Run(func(args mock.Arguments) { + assert.Equal(t, "test", args.Get(0)) + }).Return(giveRet, nil) + + h := Handler{globalRuleStore: mStore} + r := gin.New() + h.ApplyRoute(r) + + w := performRequest(r, "GET", "/apisix/admin/global_rules/test") + assert.Equal(t, 200, w.Code) +} + +func TestHandler_Get(t *testing.T) { + tests := []struct { + caseDesc string + giveInput *GetInput + giveRet interface{} + giveErr error + wantErr error + wantGetKey string + wantRet interface{} + }{ + { + caseDesc: "normal", + giveInput: &GetInput{ID: "test"}, + wantGetKey: "test", + giveRet: `{ + "id": "test", + "plugins": { + "jwt-auth": {} + } + }`, + wantRet: `{ + "id": "test", + "plugins": { + "jwt-auth": {} + } + }`, + }, + { + caseDesc: "store get failed", + giveInput: &GetInput{ID: "non-existent-key"}, + wantGetKey: "non-existent-key", + giveErr: fmt.Errorf("data not found"), + wantErr: fmt.Errorf("data not found"), + wantRet: &data.SpecCodeResponse{ + StatusCode: http.StatusNotFound, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.caseDesc, func(t *testing.T) { + getCalled := true + mStore := &store.MockInterface{} + mStore.On("Get", mock.Anything).Run(func(args mock.Arguments) { + getCalled = true + assert.Equal(t, tc.wantGetKey, args.Get(0)) + }).Return(tc.giveRet, tc.giveErr) + + h := Handler{globalRuleStore: mStore} + ctx := droplet.NewContext() + ctx.SetInput(tc.giveInput) + ret, err := h.Get(ctx) + assert.True(t, getCalled) + assert.Equal(t, tc.wantRet, ret) + assert.Equal(t, tc.wantErr, err) + }) + } +} + +func TestHandler_List(t *testing.T) { + tests := []struct { + caseDesc string + giveInput *ListInput + giveData []*entity.GlobalPlugins + giveErr error + wantErr error + wantInput store.ListInput + wantRet interface{} + }{ + { + caseDesc: "list all condition", + giveInput: &ListInput{ + Pagination: store.Pagination{ + PageSize: 10, + PageNumber: 1, + }, + }, + wantInput: store.ListInput{ + PageSize: 10, + PageNumber: 1, + }, + giveData: []*entity.GlobalPlugins{ + {ID: "global-rules-1"}, + {ID: "global-rules-2"}, + {ID: "global-rules-3"}, + }, + wantRet: &store.ListOutput{ + Rows: []interface{}{ + &entity.GlobalPlugins{ID: "global-rules-1"}, + &entity.GlobalPlugins{ID: "global-rules-2"}, + &entity.GlobalPlugins{ID: "global-rules-3"}, + }, + TotalSize: 3, + }, + }, + { + caseDesc: "store list failed", + giveInput: &ListInput{ + Pagination: store.Pagination{ + PageSize: 10, + PageNumber: 10, + }, + }, + wantInput: store.ListInput{ + PageSize: 10, + PageNumber: 10, + }, + giveData: []*entity.GlobalPlugins{}, + giveErr: fmt.Errorf("list failed"), + wantErr: fmt.Errorf("list failed"), + }, + } + + for _, tc := range tests { + t.Run(tc.caseDesc, func(t *testing.T) { + getCalled := true + mStore := &store.MockInterface{} + mStore.On("List", mock.Anything).Run(func(args mock.Arguments) { + getCalled = true + input := args.Get(0).(store.ListInput) + assert.Equal(t, tc.wantInput.PageSize, input.PageSize) + assert.Equal(t, tc.wantInput.PageNumber, input.PageNumber) + }).Return(func(input store.ListInput) *store.ListOutput { + var returnData []interface{} + for _, c := range tc.giveData { + if input.Predicate == nil || input.Predicate(c) { + returnData = append(returnData, c) + } + } + return &store.ListOutput{ + Rows: returnData, + TotalSize: len(returnData), + } + }, tc.giveErr) + + h := Handler{globalRuleStore: mStore} + ctx := droplet.NewContext() + ctx.SetInput(tc.giveInput) + ret, err := h.List(ctx) + assert.True(t, getCalled) + assert.Equal(t, tc.wantRet, ret) + assert.Equal(t, tc.wantErr, err) + }) + } +} + +func TestHandler_Set(t *testing.T) { + tests := []struct { + caseDesc string + giveInput *entity.GlobalPlugins + giveCtx context.Context + giveErr error + wantErr error + wantInput *entity.GlobalPlugins + wantRet interface{} + wantCalled bool + }{ + { + caseDesc: "normal", + giveInput: &entity.GlobalPlugins{ + ID: "name", + Plugins: map[string]interface{}{ + "jwt-auth": map[string]interface{}{}, + }, + }, + giveCtx: context.WithValue(context.Background(), "test", "value"), + wantInput: &entity.GlobalPlugins{ + ID: "name", + Plugins: map[string]interface{}{ + "jwt-auth": map[string]interface{}{}, + }, + }, + wantRet: nil, + wantCalled: true, + }, + { + caseDesc: "store create failed", + giveInput: &entity.GlobalPlugins{ + ID: "name", + Plugins: nil, + }, + giveErr: fmt.Errorf("create failed"), + wantInput: &entity.GlobalPlugins{ + ID: "name", + Plugins: map[string]interface{}(nil), + }, + wantErr: fmt.Errorf("create failed"), + wantRet: &data.SpecCodeResponse{ + StatusCode: http.StatusInternalServerError, + }, + wantCalled: true, + }, + } + + for _, tc := range tests { + t.Run(tc.caseDesc, func(t *testing.T) { + methodCalled := true + mStore := &store.MockInterface{} + mStore.On("Create", mock.Anything, mock.Anything).Run(func(args mock.Arguments) { + methodCalled = true + assert.Equal(t, tc.giveCtx, args.Get(0)) + assert.Equal(t, tc.wantInput, args.Get(1)) + }).Return(tc.giveErr) + + h := Handler{globalRuleStore: mStore} + ctx := droplet.NewContext() + ctx.SetInput(tc.giveInput) + ctx.SetContext(tc.giveCtx) + ret, err := h.Set(ctx) + assert.Equal(t, tc.wantCalled, methodCalled) + assert.Equal(t, tc.wantRet, ret) + assert.Equal(t, tc.wantErr, err) + }) + } +} + +func TestHandler_BatchDelete(t *testing.T) { + tests := []struct { + caseDesc string + giveInput *BatchDeleteInput + giveCtx context.Context + giveErr error + wantErr error + wantInput []string + wantRet interface{} + }{ + { + caseDesc: "normal", + giveInput: &BatchDeleteInput{ + IDs: "user1,user2", + }, + giveCtx: context.WithValue(context.Background(), "test", "value"), + wantInput: []string{ + "user1", + "user2", + }, + }, + { + caseDesc: "store delete failed", + giveInput: &BatchDeleteInput{ + IDs: "user1,user2", + }, + giveCtx: context.WithValue(context.Background(), "test", "value"), + giveErr: fmt.Errorf("delete failed"), + wantInput: []string{ + "user1", + "user2", + }, + wantErr: fmt.Errorf("delete failed"), + wantRet: &data.SpecCodeResponse{ + StatusCode: http.StatusInternalServerError, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.caseDesc, func(t *testing.T) { + methodCalled := true + mStore := &store.MockInterface{} + mStore.On("BatchDelete", mock.Anything, mock.Anything, mock.Anything).Run(func(args mock.Arguments) { + methodCalled = true + assert.Equal(t, tc.giveCtx, args.Get(0)) + assert.Equal(t, tc.wantInput, args.Get(1)) + }).Return(tc.giveErr) + + h := Handler{globalRuleStore: mStore} + ctx := droplet.NewContext() + ctx.SetInput(tc.giveInput) + ctx.SetContext(tc.giveCtx) + ret, err := h.BatchDelete(ctx) + assert.True(t, methodCalled) + assert.Equal(t, tc.wantErr, err) + assert.Equal(t, tc.wantRet, ret) + }) + } +} diff --git a/api/internal/route.go b/api/internal/route.go index a57e5c0e0a..141c6d49de 100644 --- a/api/internal/route.go +++ b/api/internal/route.go @@ -18,6 +18,7 @@ package internal import ( "fmt" + "github.com/gin-contrib/pprof" "github.com/gin-contrib/sessions" "github.com/gin-contrib/sessions/cookie" @@ -29,6 +30,7 @@ import ( "github.com/apisix/manager-api/internal/handler" "github.com/apisix/manager-api/internal/handler/authentication" "github.com/apisix/manager-api/internal/handler/consumer" + "github.com/apisix/manager-api/internal/handler/global_rule" "github.com/apisix/manager-api/internal/handler/healthz" "github.com/apisix/manager-api/internal/handler/plugin" "github.com/apisix/manager-api/internal/handler/route" @@ -63,6 +65,7 @@ func SetUpRouter() *gin.Engine { plugin.NewHandler, healthz.NewHandler, authentication.NewHandler, + global_rule.NewHandler, } for i := range factories { From 794b5384e9e17e343f9f11589e9537c54f4694db Mon Sep 17 00:00:00 2001 From: nic-chen Date: Tue, 15 Dec 2020 19:06:41 +0800 Subject: [PATCH 02/15] test: add e2e test cases for global rule --- api/filter/schema.go | 1 + api/internal/core/store/storehub.go | 18 ++- .../handler/global_rule/global_rule.go | 4 +- api/test/e2e/global_rule_test.go | 147 ++++++++++++++++++ 4 files changed, 165 insertions(+), 5 deletions(-) create mode 100644 api/test/e2e/global_rule_test.go diff --git a/api/filter/schema.go b/api/filter/schema.go index a294c46496..22b5166923 100644 --- a/api/filter/schema.go +++ b/api/filter/schema.go @@ -41,6 +41,7 @@ var resources = map[string]string{ "services": "service", "consumers": "consumer", "ssl": "ssl", + "global_rules": "global_rule", } func parseCert(crt, key string) ([]string, error) { diff --git a/api/internal/core/store/storehub.go b/api/internal/core/store/storehub.go index ac46a1229c..1d85794c62 100644 --- a/api/internal/core/store/storehub.go +++ b/api/internal/core/store/storehub.go @@ -34,7 +34,7 @@ const ( HubKeySsl HubKey = "ssl" HubKeyUpstream HubKey = "upstream" HubKeyScript HubKey = "script" - HubKeyGlobalRule HubKey = "global_rules" + HubKeyGlobalRule HubKey = "global_rule" ) var ( @@ -42,8 +42,8 @@ var ( ) func InitStore(key HubKey, opt GenericStoreOption) error { - if key == HubKeyConsumer || key == HubKeyRoute || - key == HubKeyService || key == HubKeySsl || key == HubKeyUpstream { + if key == HubKeyConsumer || key == HubKeyRoute || key == HubKeySsl || + key == HubKeyService || key == HubKeyUpstream || key == HubKeyGlobalRule { validator, err := NewAPISIXJsonSchemaValidator("main." + string(key)) if err != nil { return err @@ -145,5 +145,17 @@ func InitStores() error { return err } + err = InitStore(HubKeyGlobalRule, GenericStoreOption{ + BasePath: "/apisix/global_rules", + ObjType: reflect.TypeOf(entity.GlobalPlugins{}), + KeyFunc: func(obj interface{}) string { + r := obj.(*entity.GlobalPlugins) + return utils.InterfaceToString(r.ID) + }, + }) + if err != nil { + return err + } + return nil } diff --git a/api/internal/handler/global_rule/global_rule.go b/api/internal/handler/global_rule/global_rule.go index eb8b7a25a1..33cb2561ca 100644 --- a/api/internal/handler/global_rule/global_rule.go +++ b/api/internal/handler/global_rule/global_rule.go @@ -17,8 +17,6 @@ package global_rule import ( - "github.com/apisix/manager-api/internal/core/entity" - "github.com/apisix/manager-api/internal/core/store" "reflect" "strings" @@ -27,6 +25,8 @@ import ( "github.com/shiningrush/droplet/wrapper" wgin "github.com/shiningrush/droplet/wrapper/gin" + "github.com/apisix/manager-api/internal/core/entity" + "github.com/apisix/manager-api/internal/core/store" "github.com/apisix/manager-api/internal/handler" ) diff --git a/api/test/e2e/global_rule_test.go b/api/test/e2e/global_rule_test.go new file mode 100644 index 0000000000..3f9e78a4ed --- /dev/null +++ b/api/test/e2e/global_rule_test.go @@ -0,0 +1,147 @@ +/* + * 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 e2e + +import ( + "net/http" + "testing" + "time" +) + +func TestGlobalRule(t *testing.T) { + tests := []HttpTestCase{ + { + caseDesc: "make sure the route is not created ", + Object: APISIXExpect(t), + Method: http.MethodGet, + Path: "/hello", + ExpectStatus: http.StatusNotFound, + ExpectBody: `{"error_msg":"404 Route Not Found"}`, + }, + { + caseDesc: "create route", + Object: ManagerApiExpect(t), + Method: http.MethodPut, + Path: "/apisix/admin/routes/r1", + Body: `{ + "uri": "/hello", + "upstream": { + "type": "roundrobin", + "nodes": [{ + "host": "172.16.238.20", + "port": 1981, + "weight": 1 + }] + } + }`, + Headers: map[string]string{"Authorization": token}, + ExpectStatus: http.StatusOK, + }, + { + caseDesc: "create global rule", + Object: ManagerApiExpect(t), + Path: "/apisix/admin/global_rules/1", + Method: http.MethodPut, + Body: `{ + "id": "1", + "plugins": { + "limit-count": { + "count": 2, + "time_window": 2, + "rejected_code": 503, + "key": "remote_addr" + } + } + }`, + Headers: map[string]string{"Authorization": token}, + ExpectStatus: http.StatusOK, + }, + { + caseDesc: "verify route that should not be limited", + Object: APISIXExpect(t), + Method: http.MethodGet, + Path: "/hello", + ExpectStatus: http.StatusOK, + ExpectBody: "hello world", + Sleep: sleepTime, + }, + { + caseDesc: "verify route that should not be limited 2", + Object: APISIXExpect(t), + Method: http.MethodGet, + Path: "/hello", + ExpectStatus: http.StatusOK, + ExpectBody: "hello world", + }, + { + caseDesc: "verify route that should be limited", + Object: APISIXExpect(t), + Method: http.MethodGet, + Path: "/hello", + ExpectStatus: http.StatusServiceUnavailable, + ExpectBody: "503 Service Temporarily Unavailable", + }, + { + caseDesc: "verify route that should not be limited since time window pass", + Object: APISIXExpect(t), + Method: http.MethodGet, + Path: "/hello", + ExpectStatus: http.StatusOK, + ExpectBody: "hello world", + Sleep: 3 * time.Second, + }, + { + caseDesc: "delete global rule", + Object: ManagerApiExpect(t), + Method: http.MethodDelete, + Path: "/apisix/admin/global_rules/1", + Headers: map[string]string{"Authorization": token}, + ExpectStatus: http.StatusOK, + }, + { + caseDesc: "make sure the global rule has been deleted", + Object: ManagerApiExpect(t), + Method: http.MethodGet, + Path: "/apisix/admin/global_rules/1", + Headers: map[string]string{"Authorization": token}, + ExpectStatus: http.StatusNotFound, + ExpectBody: `{"code":10001,"message":"data not found"`, + Sleep: sleepTime, + }, + { + caseDesc: "delete route", + Object: ManagerApiExpect(t), + Method: http.MethodDelete, + Path: "/apisix/admin/routes/r1", + Headers: map[string]string{"Authorization": token}, + ExpectStatus: http.StatusOK, + }, + { + caseDesc: "make sure the route has been deleted", + Object: APISIXExpect(t), + Method: http.MethodGet, + Path: "/hello", + ExpectStatus: http.StatusNotFound, + ExpectBody: `{"error_msg":"404 Route Not Found"}`, + Sleep: sleepTime, + }, + } + + for _, tc := range tests { + testCaseCheck(tc) + } +} \ No newline at end of file From 1f24f9384c8ff3cbb948ff05f11ba43276725268 Mon Sep 17 00:00:00 2001 From: nic-chen Date: Tue, 15 Dec 2020 19:08:27 +0800 Subject: [PATCH 03/15] fix: code format --- api/test/e2e/global_rule_test.go | 208 +++++++++++++++---------------- 1 file changed, 104 insertions(+), 104 deletions(-) diff --git a/api/test/e2e/global_rule_test.go b/api/test/e2e/global_rule_test.go index 3f9e78a4ed..195fb9efc8 100644 --- a/api/test/e2e/global_rule_test.go +++ b/api/test/e2e/global_rule_test.go @@ -17,27 +17,27 @@ package e2e import ( - "net/http" - "testing" - "time" + "net/http" + "testing" + "time" ) func TestGlobalRule(t *testing.T) { - tests := []HttpTestCase{ - { - caseDesc: "make sure the route is not created ", - Object: APISIXExpect(t), - Method: http.MethodGet, - Path: "/hello", - ExpectStatus: http.StatusNotFound, - ExpectBody: `{"error_msg":"404 Route Not Found"}`, - }, - { - caseDesc: "create route", - Object: ManagerApiExpect(t), - Method: http.MethodPut, - Path: "/apisix/admin/routes/r1", - Body: `{ + tests := []HttpTestCase{ + { + caseDesc: "make sure the route is not created ", + Object: APISIXExpect(t), + Method: http.MethodGet, + Path: "/hello", + ExpectStatus: http.StatusNotFound, + ExpectBody: `{"error_msg":"404 Route Not Found"}`, + }, + { + caseDesc: "create route", + Object: ManagerApiExpect(t), + Method: http.MethodPut, + Path: "/apisix/admin/routes/r1", + Body: `{ "uri": "/hello", "upstream": { "type": "roundrobin", @@ -48,15 +48,15 @@ func TestGlobalRule(t *testing.T) { }] } }`, - Headers: map[string]string{"Authorization": token}, - ExpectStatus: http.StatusOK, - }, - { - caseDesc: "create global rule", - Object: ManagerApiExpect(t), - Path: "/apisix/admin/global_rules/1", - Method: http.MethodPut, - Body: `{ + Headers: map[string]string{"Authorization": token}, + ExpectStatus: http.StatusOK, + }, + { + caseDesc: "create global rule", + Object: ManagerApiExpect(t), + Path: "/apisix/admin/global_rules/1", + Method: http.MethodPut, + Body: `{ "id": "1", "plugins": { "limit-count": { @@ -67,81 +67,81 @@ func TestGlobalRule(t *testing.T) { } } }`, - Headers: map[string]string{"Authorization": token}, - ExpectStatus: http.StatusOK, - }, - { - caseDesc: "verify route that should not be limited", - Object: APISIXExpect(t), - Method: http.MethodGet, - Path: "/hello", - ExpectStatus: http.StatusOK, - ExpectBody: "hello world", - Sleep: sleepTime, - }, - { - caseDesc: "verify route that should not be limited 2", - Object: APISIXExpect(t), - Method: http.MethodGet, - Path: "/hello", - ExpectStatus: http.StatusOK, - ExpectBody: "hello world", - }, - { - caseDesc: "verify route that should be limited", - Object: APISIXExpect(t), - Method: http.MethodGet, - Path: "/hello", - ExpectStatus: http.StatusServiceUnavailable, - ExpectBody: "503 Service Temporarily Unavailable", - }, - { - caseDesc: "verify route that should not be limited since time window pass", - Object: APISIXExpect(t), - Method: http.MethodGet, - Path: "/hello", - ExpectStatus: http.StatusOK, - ExpectBody: "hello world", - Sleep: 3 * time.Second, - }, - { - caseDesc: "delete global rule", - Object: ManagerApiExpect(t), - Method: http.MethodDelete, - Path: "/apisix/admin/global_rules/1", - Headers: map[string]string{"Authorization": token}, - ExpectStatus: http.StatusOK, - }, - { - caseDesc: "make sure the global rule has been deleted", - Object: ManagerApiExpect(t), - Method: http.MethodGet, - Path: "/apisix/admin/global_rules/1", - Headers: map[string]string{"Authorization": token}, - ExpectStatus: http.StatusNotFound, - ExpectBody: `{"code":10001,"message":"data not found"`, - Sleep: sleepTime, - }, - { - caseDesc: "delete route", - Object: ManagerApiExpect(t), - Method: http.MethodDelete, - Path: "/apisix/admin/routes/r1", - Headers: map[string]string{"Authorization": token}, - ExpectStatus: http.StatusOK, - }, - { - caseDesc: "make sure the route has been deleted", - Object: APISIXExpect(t), - Method: http.MethodGet, - Path: "/hello", - ExpectStatus: http.StatusNotFound, - ExpectBody: `{"error_msg":"404 Route Not Found"}`, - Sleep: sleepTime, - }, - } + Headers: map[string]string{"Authorization": token}, + ExpectStatus: http.StatusOK, + }, + { + caseDesc: "verify route that should not be limited", + Object: APISIXExpect(t), + Method: http.MethodGet, + Path: "/hello", + ExpectStatus: http.StatusOK, + ExpectBody: "hello world", + Sleep: sleepTime, + }, + { + caseDesc: "verify route that should not be limited 2", + Object: APISIXExpect(t), + Method: http.MethodGet, + Path: "/hello", + ExpectStatus: http.StatusOK, + ExpectBody: "hello world", + }, + { + caseDesc: "verify route that should be limited", + Object: APISIXExpect(t), + Method: http.MethodGet, + Path: "/hello", + ExpectStatus: http.StatusServiceUnavailable, + ExpectBody: "503 Service Temporarily Unavailable", + }, + { + caseDesc: "verify route that should not be limited since time window pass", + Object: APISIXExpect(t), + Method: http.MethodGet, + Path: "/hello", + ExpectStatus: http.StatusOK, + ExpectBody: "hello world", + Sleep: 3 * time.Second, + }, + { + caseDesc: "delete global rule", + Object: ManagerApiExpect(t), + Method: http.MethodDelete, + Path: "/apisix/admin/global_rules/1", + Headers: map[string]string{"Authorization": token}, + ExpectStatus: http.StatusOK, + }, + { + caseDesc: "make sure the global rule has been deleted", + Object: ManagerApiExpect(t), + Method: http.MethodGet, + Path: "/apisix/admin/global_rules/1", + Headers: map[string]string{"Authorization": token}, + ExpectStatus: http.StatusNotFound, + ExpectBody: `{"code":10001,"message":"data not found"`, + Sleep: sleepTime, + }, + { + caseDesc: "delete route", + Object: ManagerApiExpect(t), + Method: http.MethodDelete, + Path: "/apisix/admin/routes/r1", + Headers: map[string]string{"Authorization": token}, + ExpectStatus: http.StatusOK, + }, + { + caseDesc: "make sure the route has been deleted", + Object: APISIXExpect(t), + Method: http.MethodGet, + Path: "/hello", + ExpectStatus: http.StatusNotFound, + ExpectBody: `{"error_msg":"404 Route Not Found"}`, + Sleep: sleepTime, + }, + } - for _, tc := range tests { - testCaseCheck(tc) - } -} \ No newline at end of file + for _, tc := range tests { + testCaseCheck(tc) + } +} From a6333ecbafc0ba834f73ba5b2027a3b4f8b2da7b Mon Sep 17 00:00:00 2001 From: nic-chen Date: Fri, 18 Dec 2020 12:45:41 +0800 Subject: [PATCH 04/15] test: add test cases to confirm global rule has been removed. --- api/test/e2e/global_rule_test.go | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/api/test/e2e/global_rule_test.go b/api/test/e2e/global_rule_test.go index 195fb9efc8..b257418583 100644 --- a/api/test/e2e/global_rule_test.go +++ b/api/test/e2e/global_rule_test.go @@ -122,6 +122,31 @@ func TestGlobalRule(t *testing.T) { ExpectBody: `{"code":10001,"message":"data not found"`, Sleep: sleepTime, }, + { + caseDesc: "verify route that should not be limited", + Object: APISIXExpect(t), + Method: http.MethodGet, + Path: "/hello", + ExpectStatus: http.StatusOK, + ExpectBody: "hello world", + Sleep: sleepTime, + }, + { + caseDesc: "verify route that should not be limited 2", + Object: APISIXExpect(t), + Method: http.MethodGet, + Path: "/hello", + ExpectStatus: http.StatusOK, + ExpectBody: "hello world", + }, + { + caseDesc: "verify route that should not be limited 3 (no plugin is enabled)", + Object: APISIXExpect(t), + Method: http.MethodGet, + Path: "/hello", + ExpectStatus: http.StatusOK, + ExpectBody: "hello world", + }, { caseDesc: "delete route", Object: ManagerApiExpect(t), From b6443a66934c78fb13c2078ddec1d40be6af1acc Mon Sep 17 00:00:00 2001 From: nic-chen Date: Fri, 18 Dec 2020 13:13:08 +0800 Subject: [PATCH 05/15] fix: according to review --- api/filter/schema.go | 10 +++++----- api/internal/core/store/storehub.go | 11 +++++++++-- api/test/e2e/global_rule_test.go | 2 +- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/api/filter/schema.go b/api/filter/schema.go index 22b5166923..ab87c86bd2 100644 --- a/api/filter/schema.go +++ b/api/filter/schema.go @@ -36,11 +36,11 @@ import ( ) var resources = map[string]string{ - "routes": "route", - "upstreams": "upstream", - "services": "service", - "consumers": "consumer", - "ssl": "ssl", + "routes": "route", + "upstreams": "upstream", + "services": "service", + "consumers": "consumer", + "ssl": "ssl", "global_rules": "global_rule", } diff --git a/api/internal/core/store/storehub.go b/api/internal/core/store/storehub.go index 1d85794c62..6cec1ad78e 100644 --- a/api/internal/core/store/storehub.go +++ b/api/internal/core/store/storehub.go @@ -42,8 +42,15 @@ var ( ) func InitStore(key HubKey, opt GenericStoreOption) error { - if key == HubKeyConsumer || key == HubKeyRoute || key == HubKeySsl || - key == HubKeyService || key == HubKeyUpstream || key == HubKeyGlobalRule { + hubsNeedCheck := map[HubKey]bool{ + HubKeyConsumer: true, + HubKeyRoute: true, + HubKeySsl: true, + HubKeyService: true, + HubKeyUpstream: true, + HubKeyGlobalRule: true, + } + if _, ok := hubsNeedCheck[key]; ok { validator, err := NewAPISIXJsonSchemaValidator("main." + string(key)) if err != nil { return err diff --git a/api/test/e2e/global_rule_test.go b/api/test/e2e/global_rule_test.go index b257418583..7d7dfdc340 100644 --- a/api/test/e2e/global_rule_test.go +++ b/api/test/e2e/global_rule_test.go @@ -25,7 +25,7 @@ import ( func TestGlobalRule(t *testing.T) { tests := []HttpTestCase{ { - caseDesc: "make sure the route is not created ", + caseDesc: "make sure the route doesn't exist", Object: APISIXExpect(t), Method: http.MethodGet, Path: "/hello", From b9b24864aae435e723c1e58c161aa13190c89a06 Mon Sep 17 00:00:00 2001 From: nic-chen Date: Sun, 20 Dec 2020 12:12:51 +0800 Subject: [PATCH 06/15] fix: lint --- api/internal/core/store/storehub.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/api/internal/core/store/storehub.go b/api/internal/core/store/storehub.go index f6c823aa36..3b56612f3d 100644 --- a/api/internal/core/store/storehub.go +++ b/api/internal/core/store/storehub.go @@ -153,14 +153,17 @@ func InitStores() error { return err } - err = InitStore(HubKeyGlobalRule, GenericStoreOption{ + err = InitStore(HubKeyGlobalRule, GenericStoreOption{ BasePath: "/apisix/global_rules", ObjType: reflect.TypeOf(entity.GlobalPlugins{}), KeyFunc: func(obj interface{}) string { r := obj.(*entity.GlobalPlugins) - return utils.InterfaceToString(r.ID) - }, - }) + return utils.InterfaceToString(r.ID) + }, + }) + if err != nil { + return err + } err = InitStore(HubKeyServerInfo, GenericStoreOption{ BasePath: "/apisix/data_plane/server_info", From 3025af59419644b86b0e8e7b78b33767c24c3e5e Mon Sep 17 00:00:00 2001 From: nic-chen Date: Sun, 20 Dec 2020 20:54:34 +0800 Subject: [PATCH 07/15] test: update test cases for plugins configured both in route and global rule --- api/test/e2e/global_rule_test.go | 119 +++++++++++++++++-------------- 1 file changed, 67 insertions(+), 52 deletions(-) diff --git a/api/test/e2e/global_rule_test.go b/api/test/e2e/global_rule_test.go index 7d7dfdc340..f829ee781e 100644 --- a/api/test/e2e/global_rule_test.go +++ b/api/test/e2e/global_rule_test.go @@ -19,7 +19,6 @@ package e2e import ( "net/http" "testing" - "time" ) func TestGlobalRule(t *testing.T) { @@ -59,50 +58,82 @@ func TestGlobalRule(t *testing.T) { Body: `{ "id": "1", "plugins": { - "limit-count": { - "count": 2, - "time_window": 2, - "rejected_code": 503, - "key": "remote_addr" - } + "response-rewrite": { + "headers": { + "X-VERSION":"1.0" + } + }, + "uri-blocker": { + "block_rules": ["select.+(from|limit)", "(?:(union(.*?)select))"] + } } }`, Headers: map[string]string{"Authorization": token}, ExpectStatus: http.StatusOK, }, { - caseDesc: "verify route that should not be limited", - Object: APISIXExpect(t), - Method: http.MethodGet, - Path: "/hello", - ExpectStatus: http.StatusOK, - ExpectBody: "hello world", - Sleep: sleepTime, + caseDesc: "verify route with header", + Object: APISIXExpect(t), + Method: http.MethodGet, + Path: "/hello", + ExpectStatus: http.StatusOK, + ExpectBody: "hello world", + ExpectHeaders: map[string]string{"X-VERSION": "1.0"}, + Sleep: sleepTime, }, { - caseDesc: "verify route that should not be limited 2", - Object: APISIXExpect(t), - Method: http.MethodGet, - Path: "/hello", + caseDesc: "verify route that should be blocked", + Object: APISIXExpect(t), + Method: http.MethodGet, + Path: "/hello", + Query: "name=;select%20from%20sys", + ExpectStatus: http.StatusForbidden, + ExpectHeaders: map[string]string{"X-VERSION": "1.0"}, + }, + { + caseDesc: "update route with same plugin response-rewrite", + Object: ManagerApiExpect(t), + Method: http.MethodPut, + Path: "/apisix/admin/routes/r1", + Body: `{ + "uri": "/hello", + "plugins": { + "response-rewrite": { + "headers": { + "X-VERSION":"2.0" + } + } + }, + "upstream": { + "type": "roundrobin", + "nodes": [{ + "host": "172.16.238.20", + "port": 1981, + "weight": 1 + }] + } + }`, + Headers: map[string]string{"Authorization": token}, ExpectStatus: http.StatusOK, - ExpectBody: "hello world", }, { - caseDesc: "verify route that should be limited", - Object: APISIXExpect(t), - Method: http.MethodGet, - Path: "/hello", - ExpectStatus: http.StatusServiceUnavailable, - ExpectBody: "503 Service Temporarily Unavailable", + caseDesc: "verify route that header should be the same as the route config", + Object: APISIXExpect(t), + Method: http.MethodGet, + Path: "/hello", + ExpectStatus: http.StatusOK, + ExpectBody: "hello world", + ExpectHeaders: map[string]string{"X-VERSION": "2.0"}, + Sleep: sleepTime, }, { - caseDesc: "verify route that should not be limited since time window pass", + caseDesc: "the uncovered global plugin should works", Object: APISIXExpect(t), Method: http.MethodGet, Path: "/hello", - ExpectStatus: http.StatusOK, - ExpectBody: "hello world", - Sleep: 3 * time.Second, + Query: "name=;select%20from%20sys", + ExpectStatus: http.StatusForbidden, + //ExpectHeaders: map[string]string{"X-VERSION":"2.0"}, }, { caseDesc: "delete global rule", @@ -123,29 +154,13 @@ func TestGlobalRule(t *testing.T) { Sleep: sleepTime, }, { - caseDesc: "verify route that should not be limited", - Object: APISIXExpect(t), - Method: http.MethodGet, - Path: "/hello", - ExpectStatus: http.StatusOK, - ExpectBody: "hello world", - Sleep: sleepTime, - }, - { - caseDesc: "verify route that should not be limited 2", - Object: APISIXExpect(t), - Method: http.MethodGet, - Path: "/hello", - ExpectStatus: http.StatusOK, - ExpectBody: "hello world", - }, - { - caseDesc: "verify route that should not be limited 3 (no plugin is enabled)", - Object: APISIXExpect(t), - Method: http.MethodGet, - Path: "/hello", - ExpectStatus: http.StatusOK, - ExpectBody: "hello world", + caseDesc: "verify route that should not be blocked", + Object: APISIXExpect(t), + Method: http.MethodGet, + Path: "/hello", + Query: "name=;select%20from%20sys", + ExpectStatus: http.StatusOK, + ExpectHeaders: map[string]string{"X-VERSION": "2.0"}, }, { caseDesc: "delete route", From 3af747b047341b735d76dd4cfdf54f8df32cfd45 Mon Sep 17 00:00:00 2001 From: nic-chen Date: Tue, 22 Dec 2020 17:45:01 +0800 Subject: [PATCH 08/15] fix: resful style --- api/internal/handler/global_rule/global_rule.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/api/internal/handler/global_rule/global_rule.go b/api/internal/handler/global_rule/global_rule.go index 33cb2561ca..20d50321f4 100644 --- a/api/internal/handler/global_rule/global_rule.go +++ b/api/internal/handler/global_rule/global_rule.go @@ -18,7 +18,6 @@ package global_rule import ( "reflect" - "strings" "github.com/gin-gonic/gin" "github.com/shiningrush/droplet" @@ -50,7 +49,7 @@ func (h *Handler) ApplyRoute(r *gin.Engine) { wrapper.InputType(reflect.TypeOf(entity.GlobalPlugins{})))) r.PUT("/apisix/admin/global_rules", wgin.Wraps(h.Set, wrapper.InputType(reflect.TypeOf(entity.GlobalPlugins{})))) - r.DELETE("/apisix/admin/global_rules/:ids", wgin.Wraps(h.BatchDelete, + r.DELETE("/apisix/admin/global_rules/:id", wgin.Wraps(h.BatchDelete, wrapper.InputType(reflect.TypeOf(BatchDeleteInput{})))) } @@ -126,13 +125,13 @@ func (h *Handler) Set(c droplet.Context) (interface{}, error) { } type BatchDeleteInput struct { - IDs string `auto_read:"ids,path"` + ID string `auto_read:"id,path"` } func (h *Handler) BatchDelete(c droplet.Context) (interface{}, error) { input := c.Input().(*BatchDeleteInput) - if err := h.globalRuleStore.BatchDelete(c.Context(), strings.Split(input.IDs, ",")); err != nil { + if err := h.globalRuleStore.BatchDelete(c.Context(), []string{input.ID}); err != nil { return handler.SpecCodeResponse(err), err } From 0f83dd19208f09858eafe6f6c2dcdb9cfe3c8c45 Mon Sep 17 00:00:00 2001 From: nic-chen Date: Tue, 22 Dec 2020 17:49:31 +0800 Subject: [PATCH 09/15] fix: global rule unit test --- api/internal/handler/global_rule/global_rule_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/api/internal/handler/global_rule/global_rule_test.go b/api/internal/handler/global_rule/global_rule_test.go index c7b15fdded..62a719d8fb 100644 --- a/api/internal/handler/global_rule/global_rule_test.go +++ b/api/internal/handler/global_rule/global_rule_test.go @@ -289,23 +289,21 @@ func TestHandler_BatchDelete(t *testing.T) { { caseDesc: "normal", giveInput: &BatchDeleteInput{ - IDs: "user1,user2", + ID: "user1", }, giveCtx: context.WithValue(context.Background(), "test", "value"), wantInput: []string{ "user1", - "user2", }, }, { caseDesc: "store delete failed", giveInput: &BatchDeleteInput{ - IDs: "user1,user2", + ID: "user2", }, giveCtx: context.WithValue(context.Background(), "test", "value"), giveErr: fmt.Errorf("delete failed"), wantInput: []string{ - "user1", "user2", }, wantErr: fmt.Errorf("delete failed"), From 3c58e5cd2fe0b6ee3dace3dc28e240a569cb47a0 Mon Sep 17 00:00:00 2001 From: nic-chen Date: Thu, 24 Dec 2020 20:28:46 +0800 Subject: [PATCH 10/15] test: update test cases --- api/test/e2e/global_rule_test.go | 46 ++++++++++++++++---------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/api/test/e2e/global_rule_test.go b/api/test/e2e/global_rule_test.go index f829ee781e..19ecaf6033 100644 --- a/api/test/e2e/global_rule_test.go +++ b/api/test/e2e/global_rule_test.go @@ -24,7 +24,7 @@ import ( func TestGlobalRule(t *testing.T) { tests := []HttpTestCase{ { - caseDesc: "make sure the route doesn't exist", + Desc: "make sure the route doesn't exist", Object: APISIXExpect(t), Method: http.MethodGet, Path: "/hello", @@ -32,10 +32,10 @@ func TestGlobalRule(t *testing.T) { ExpectBody: `{"error_msg":"404 Route Not Found"}`, }, { - caseDesc: "create route", - Object: ManagerApiExpect(t), - Method: http.MethodPut, - Path: "/apisix/admin/routes/r1", + Desc: "create route", + Object: ManagerApiExpect(t), + Method: http.MethodPut, + Path: "/apisix/admin/routes/r1", Body: `{ "uri": "/hello", "upstream": { @@ -51,10 +51,10 @@ func TestGlobalRule(t *testing.T) { ExpectStatus: http.StatusOK, }, { - caseDesc: "create global rule", - Object: ManagerApiExpect(t), - Path: "/apisix/admin/global_rules/1", - Method: http.MethodPut, + Desc: "create global rule", + Object: ManagerApiExpect(t), + Path: "/apisix/admin/global_rules/1", + Method: http.MethodPut, Body: `{ "id": "1", "plugins": { @@ -72,7 +72,7 @@ func TestGlobalRule(t *testing.T) { ExpectStatus: http.StatusOK, }, { - caseDesc: "verify route with header", + Desc: "verify route with header", Object: APISIXExpect(t), Method: http.MethodGet, Path: "/hello", @@ -82,7 +82,7 @@ func TestGlobalRule(t *testing.T) { Sleep: sleepTime, }, { - caseDesc: "verify route that should be blocked", + Desc: "verify route that should be blocked", Object: APISIXExpect(t), Method: http.MethodGet, Path: "/hello", @@ -91,10 +91,10 @@ func TestGlobalRule(t *testing.T) { ExpectHeaders: map[string]string{"X-VERSION": "1.0"}, }, { - caseDesc: "update route with same plugin response-rewrite", - Object: ManagerApiExpect(t), - Method: http.MethodPut, - Path: "/apisix/admin/routes/r1", + Desc: "update route with same plugin response-rewrite", + Object: ManagerApiExpect(t), + Method: http.MethodPut, + Path: "/apisix/admin/routes/r1", Body: `{ "uri": "/hello", "plugins": { @@ -117,7 +117,7 @@ func TestGlobalRule(t *testing.T) { ExpectStatus: http.StatusOK, }, { - caseDesc: "verify route that header should be the same as the route config", + Desc: "verify route that header should be the same as the route config", Object: APISIXExpect(t), Method: http.MethodGet, Path: "/hello", @@ -127,7 +127,7 @@ func TestGlobalRule(t *testing.T) { Sleep: sleepTime, }, { - caseDesc: "the uncovered global plugin should works", + Desc: "the uncovered global plugin should works", Object: APISIXExpect(t), Method: http.MethodGet, Path: "/hello", @@ -136,7 +136,7 @@ func TestGlobalRule(t *testing.T) { //ExpectHeaders: map[string]string{"X-VERSION":"2.0"}, }, { - caseDesc: "delete global rule", + Desc: "delete global rule", Object: ManagerApiExpect(t), Method: http.MethodDelete, Path: "/apisix/admin/global_rules/1", @@ -144,7 +144,7 @@ func TestGlobalRule(t *testing.T) { ExpectStatus: http.StatusOK, }, { - caseDesc: "make sure the global rule has been deleted", + Desc: "make sure the global rule has been deleted", Object: ManagerApiExpect(t), Method: http.MethodGet, Path: "/apisix/admin/global_rules/1", @@ -154,7 +154,7 @@ func TestGlobalRule(t *testing.T) { Sleep: sleepTime, }, { - caseDesc: "verify route that should not be blocked", + Desc: "verify route that should not be blocked", Object: APISIXExpect(t), Method: http.MethodGet, Path: "/hello", @@ -163,7 +163,7 @@ func TestGlobalRule(t *testing.T) { ExpectHeaders: map[string]string{"X-VERSION": "2.0"}, }, { - caseDesc: "delete route", + Desc: "delete route", Object: ManagerApiExpect(t), Method: http.MethodDelete, Path: "/apisix/admin/routes/r1", @@ -171,7 +171,7 @@ func TestGlobalRule(t *testing.T) { ExpectStatus: http.StatusOK, }, { - caseDesc: "make sure the route has been deleted", + Desc: "make sure the route has been deleted", Object: APISIXExpect(t), Method: http.MethodGet, Path: "/hello", @@ -182,6 +182,6 @@ func TestGlobalRule(t *testing.T) { } for _, tc := range tests { - testCaseCheck(tc) + testCaseCheck(tc, t) } } From f4207a3a52e6ac5799ef54bef7c85958600ac2c7 Mon Sep 17 00:00:00 2001 From: nic-chen Date: Fri, 25 Dec 2020 15:21:13 +0800 Subject: [PATCH 11/15] feat: support patch for global rule --- .../handler/global_rule/global_rule.go | 36 ++++++++++++++ api/test/e2e/global_rule_test.go | 47 +++++++++++++++++++ 2 files changed, 83 insertions(+) diff --git a/api/internal/handler/global_rule/global_rule.go b/api/internal/handler/global_rule/global_rule.go index 20d50321f4..81b1877c75 100644 --- a/api/internal/handler/global_rule/global_rule.go +++ b/api/internal/handler/global_rule/global_rule.go @@ -17,6 +17,7 @@ package global_rule import ( + "encoding/json" "reflect" "github.com/gin-gonic/gin" @@ -27,6 +28,8 @@ import ( "github.com/apisix/manager-api/internal/core/entity" "github.com/apisix/manager-api/internal/core/store" "github.com/apisix/manager-api/internal/handler" + "github.com/apisix/manager-api/internal/utils" + "github.com/apisix/manager-api/internal/utils/consts" ) type Handler struct { @@ -49,6 +52,10 @@ func (h *Handler) ApplyRoute(r *gin.Engine) { wrapper.InputType(reflect.TypeOf(entity.GlobalPlugins{})))) r.PUT("/apisix/admin/global_rules", wgin.Wraps(h.Set, wrapper.InputType(reflect.TypeOf(entity.GlobalPlugins{})))) + + r.PATCH("/apisix/admin/global_rules/:id", consts.ErrorWrapper(Patch)) + r.PATCH("/apisix/admin/global_rules/:id/*path", consts.ErrorWrapper(Patch)) + r.DELETE("/apisix/admin/global_rules/:id", wgin.Wraps(h.BatchDelete, wrapper.InputType(reflect.TypeOf(BatchDeleteInput{})))) } @@ -124,6 +131,35 @@ func (h *Handler) Set(c droplet.Context) (interface{}, error) { return nil, nil } +func Patch(c *gin.Context) (interface{}, error) { + reqBody, _ := c.GetRawData() + ID := c.Param("id") + subPath := c.Param("path") + + routeStore := store.GetStore(store.HubKeyGlobalRule) + stored, err := routeStore.Get(ID) + if err != nil { + return handler.SpecCodeResponse(err), err + } + + res, err := utils.MergePatch(stored, subPath, reqBody) + if err != nil { + return handler.SpecCodeResponse(err), err + } + + var globalRule entity.GlobalPlugins + err = json.Unmarshal(res, &globalRule) + if err != nil { + return handler.SpecCodeResponse(err), err + } + + if err := routeStore.Update(c, &globalRule, false); err != nil { + return handler.SpecCodeResponse(err), err + } + + return nil, nil +} + type BatchDeleteInput struct { ID string `auto_read:"id,path"` } diff --git a/api/test/e2e/global_rule_test.go b/api/test/e2e/global_rule_test.go index 19ecaf6033..19a59b3961 100644 --- a/api/test/e2e/global_rule_test.go +++ b/api/test/e2e/global_rule_test.go @@ -135,6 +135,53 @@ func TestGlobalRule(t *testing.T) { ExpectStatus: http.StatusForbidden, //ExpectHeaders: map[string]string{"X-VERSION":"2.0"}, }, + { + Desc: "route patch to enable key-auth", + Object: ManagerApiExpect(t), + Method: http.MethodPatch, + Path: "/apisix/admin/global_rules/1/plugins", + Body: `{ + "response-rewrite": { + "headers": { + "X-VERSION":"1.0" + } + }, + "key-auth": {} + }`, + Headers: map[string]string{"Authorization": token}, + ExpectStatus: http.StatusOK, + }, + { + Desc: "make sure that patch succeeded", + Object: APISIXExpect(t), + Method: http.MethodGet, + Path: "/hello", + ExpectStatus: http.StatusUnauthorized, + Sleep: sleepTime, + }, + { + Desc: "route patch to disable key-auth", + Object: ManagerApiExpect(t), + Method: http.MethodPatch, + Path: "/apisix/admin/global_rules/1", + Body: `{ + "plugins": { + "key-auth": null + } + }`, + Headers: map[string]string{"Authorization": token}, + ExpectStatus: http.StatusOK, + }, + { + Desc: "make sure that patch succeeded", + Object: APISIXExpect(t), + Method: http.MethodGet, + Path: "/hello", + ExpectStatus: http.StatusOK, + ExpectBody: "hello world", + ExpectHeaders: map[string]string{"X-VERSION": "2.0"}, + Sleep: sleepTime, + }, { Desc: "delete global rule", Object: ManagerApiExpect(t), From 78141e14a9b727b6d68fb3600975a9762631eba2 Mon Sep 17 00:00:00 2001 From: nic-chen Date: Sun, 27 Dec 2020 22:47:29 +0800 Subject: [PATCH 12/15] fix CI failed --- api/test/e2e/base.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/api/test/e2e/base.go b/api/test/e2e/base.go index 5c79f857c1..8cc9e0e610 100644 --- a/api/test/e2e/base.go +++ b/api/test/e2e/base.go @@ -174,6 +174,13 @@ type HttpTestCase struct { } func testCaseCheck(tc HttpTestCase, t *testing.T) { + + if tc.Sleep != 0 { + time.Sleep(tc.Sleep) + } else { + time.Sleep(time.Duration(50) * time.Millisecond) + } + t.Run(tc.Desc, func(t *testing.T) { //init expectObj := tc.Object @@ -198,12 +205,6 @@ func testCaseCheck(tc HttpTestCase, t *testing.T) { panic("fail to init request") } - if tc.Sleep != 0 { - time.Sleep(tc.Sleep) - } else { - time.Sleep(time.Duration(50) * time.Millisecond) - } - if tc.Query != "" { req.WithQueryString(tc.Query) } From c3e19f664dceab11906fc42d823e89713fe159cd Mon Sep 17 00:00:00 2001 From: nic-chen Date: Sun, 27 Dec 2020 23:21:41 +0800 Subject: [PATCH 13/15] fix CI failed --- api/test/e2e/base.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/api/test/e2e/base.go b/api/test/e2e/base.go index 8cc9e0e610..9d6bfbe917 100644 --- a/api/test/e2e/base.go +++ b/api/test/e2e/base.go @@ -173,14 +173,8 @@ type HttpTestCase struct { Sleep time.Duration //ms } -func testCaseCheck(tc HttpTestCase, t *testing.T) { - - if tc.Sleep != 0 { - time.Sleep(tc.Sleep) - } else { - time.Sleep(time.Duration(50) * time.Millisecond) - } +func testCaseCheck(tc HttpTestCase, t *testing.T) { t.Run(tc.Desc, func(t *testing.T) { //init expectObj := tc.Object @@ -205,6 +199,12 @@ func testCaseCheck(tc HttpTestCase, t *testing.T) { panic("fail to init request") } + if tc.Sleep != 0 { + time.Sleep(tc.Sleep) + } else { + time.Sleep(time.Duration(50) * time.Millisecond) + } + if tc.Query != "" { req.WithQueryString(tc.Query) } @@ -239,5 +239,4 @@ func testCaseCheck(tc HttpTestCase, t *testing.T) { resp.Body().Contains(tc.ExpectBody) } }) - } From 749d956f4a0b6c3b26846277bcb60c485a691a06 Mon Sep 17 00:00:00 2001 From: nic-chen Date: Mon, 28 Dec 2020 15:07:27 +0800 Subject: [PATCH 14/15] fix CI failed --- api/test/e2e/base.go | 107 +++++++++++++++++++++---------------------- 1 file changed, 52 insertions(+), 55 deletions(-) diff --git a/api/test/e2e/base.go b/api/test/e2e/base.go index 9d6bfbe917..87eb77b022 100644 --- a/api/test/e2e/base.go +++ b/api/test/e2e/base.go @@ -173,70 +173,67 @@ type HttpTestCase struct { Sleep time.Duration //ms } - func testCaseCheck(tc HttpTestCase, t *testing.T) { - t.Run(tc.Desc, func(t *testing.T) { - //init - expectObj := tc.Object - var req *httpexpect.Request - switch tc.Method { - case http.MethodGet: - req = expectObj.GET(tc.Path) - case http.MethodPut: - req = expectObj.PUT(tc.Path) - case http.MethodPost: - req = expectObj.POST(tc.Path) - case http.MethodDelete: - req = expectObj.DELETE(tc.Path) - case http.MethodPatch: - req = expectObj.PATCH(tc.Path) - case http.MethodOptions: - req = expectObj.OPTIONS(tc.Path) - default: - } + //init + expectObj := tc.Object + var req *httpexpect.Request + switch tc.Method { + case http.MethodGet: + req = expectObj.GET(tc.Path) + case http.MethodPut: + req = expectObj.PUT(tc.Path) + case http.MethodPost: + req = expectObj.POST(tc.Path) + case http.MethodDelete: + req = expectObj.DELETE(tc.Path) + case http.MethodPatch: + req = expectObj.PATCH(tc.Path) + case http.MethodOptions: + req = expectObj.OPTIONS(tc.Path) + default: + } - if req == nil { - panic("fail to init request") - } + if req == nil { + panic("fail to init request") + } - if tc.Sleep != 0 { - time.Sleep(tc.Sleep) - } else { - time.Sleep(time.Duration(50) * time.Millisecond) - } + if tc.Sleep != 0 { + time.Sleep(tc.Sleep) + } else { + time.Sleep(time.Duration(50) * time.Millisecond) + } - if tc.Query != "" { - req.WithQueryString(tc.Query) - } + if tc.Query != "" { + req.WithQueryString(tc.Query) + } - //set header - for key, val := range tc.Headers { - req.WithHeader(key, val) - } + //set header + for key, val := range tc.Headers { + req.WithHeader(key, val) + } - //set body - if tc.Body != "" { - req.WithText(tc.Body) - } + //set body + if tc.Body != "" { + req.WithText(tc.Body) + } - //respond check - resp := req.Expect() + //respond check + resp := req.Expect() - //match http status - if tc.ExpectStatus != 0 { - resp.Status(tc.ExpectStatus) - } + //match http status + if tc.ExpectStatus != 0 { + resp.Status(tc.ExpectStatus) + } - //match headers - if tc.ExpectHeaders != nil { - for key, val := range tc.ExpectHeaders { - resp.Header(key).Equal(val) - } + //match headers + if tc.ExpectHeaders != nil { + for key, val := range tc.ExpectHeaders { + resp.Header(key).Equal(val) } + } - //match body - if tc.ExpectBody != "" { - resp.Body().Contains(tc.ExpectBody) - } - }) + //match body + if tc.ExpectBody != "" { + resp.Body().Contains(tc.ExpectBody) + } } From 56a549a5a6ec25cbc89e249389614e0f94d9ea21 Mon Sep 17 00:00:00 2001 From: nic-chen Date: Mon, 28 Dec 2020 15:16:11 +0800 Subject: [PATCH 15/15] fix CI failed --- api/test/e2e/base.go | 106 ++++++++++++++++++++++--------------------- 1 file changed, 54 insertions(+), 52 deletions(-) diff --git a/api/test/e2e/base.go b/api/test/e2e/base.go index 87eb77b022..4bed9cb1d7 100644 --- a/api/test/e2e/base.go +++ b/api/test/e2e/base.go @@ -174,66 +174,68 @@ type HttpTestCase struct { } func testCaseCheck(tc HttpTestCase, t *testing.T) { - //init - expectObj := tc.Object - var req *httpexpect.Request - switch tc.Method { - case http.MethodGet: - req = expectObj.GET(tc.Path) - case http.MethodPut: - req = expectObj.PUT(tc.Path) - case http.MethodPost: - req = expectObj.POST(tc.Path) - case http.MethodDelete: - req = expectObj.DELETE(tc.Path) - case http.MethodPatch: - req = expectObj.PATCH(tc.Path) - case http.MethodOptions: - req = expectObj.OPTIONS(tc.Path) - default: - } + t.Run(tc.Desc, func(t *testing.T) { + //init + expectObj := tc.Object + var req *httpexpect.Request + switch tc.Method { + case http.MethodGet: + req = expectObj.GET(tc.Path) + case http.MethodPut: + req = expectObj.PUT(tc.Path) + case http.MethodPost: + req = expectObj.POST(tc.Path) + case http.MethodDelete: + req = expectObj.DELETE(tc.Path) + case http.MethodPatch: + req = expectObj.PATCH(tc.Path) + case http.MethodOptions: + req = expectObj.OPTIONS(tc.Path) + default: + } - if req == nil { - panic("fail to init request") - } + if req == nil { + panic("fail to init request") + } - if tc.Sleep != 0 { - time.Sleep(tc.Sleep) - } else { - time.Sleep(time.Duration(50) * time.Millisecond) - } + if tc.Sleep != 0 { + time.Sleep(tc.Sleep) + } else { + time.Sleep(time.Duration(50) * time.Millisecond) + } - if tc.Query != "" { - req.WithQueryString(tc.Query) - } + if tc.Query != "" { + req.WithQueryString(tc.Query) + } - //set header - for key, val := range tc.Headers { - req.WithHeader(key, val) - } + //set header + for key, val := range tc.Headers { + req.WithHeader(key, val) + } - //set body - if tc.Body != "" { - req.WithText(tc.Body) - } + //set body + if tc.Body != "" { + req.WithText(tc.Body) + } - //respond check - resp := req.Expect() + //respond check + resp := req.Expect() - //match http status - if tc.ExpectStatus != 0 { - resp.Status(tc.ExpectStatus) - } + //match http status + if tc.ExpectStatus != 0 { + resp.Status(tc.ExpectStatus) + } - //match headers - if tc.ExpectHeaders != nil { - for key, val := range tc.ExpectHeaders { - resp.Header(key).Equal(val) + //match headers + if tc.ExpectHeaders != nil { + for key, val := range tc.ExpectHeaders { + resp.Header(key).Equal(val) + } } - } - //match body - if tc.ExpectBody != "" { - resp.Body().Contains(tc.ExpectBody) - } + //match body + if tc.ExpectBody != "" { + resp.Body().Contains(tc.ExpectBody) + } + }) }