This is an automated email from the ASF dual-hosted git repository. starsz 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 f4f27d1 feat: check if the service is used by route when deleting (#1598) f4f27d1 is described below commit f4f27d1a7b2f2a712c15dd922a30afd7e5d671db Author: Peter Zhu <starsz...@gmail.com> AuthorDate: Fri Mar 19 09:21:18 2021 +0800 feat: check if the service is used by route when deleting (#1598) --- api/internal/handler/service/service.go | 30 +++++++- api/internal/handler/service/service_test.go | 81 +++++++++++++++++++-- api/test/e2enew/service/service_test.go | 105 +++++++++++++++++++++++++++ 3 files changed, 207 insertions(+), 9 deletions(-) diff --git a/api/internal/handler/service/service.go b/api/internal/handler/service/service.go index 184c409..fe382b0 100644 --- a/api/internal/handler/service/service.go +++ b/api/internal/handler/service/service.go @@ -38,12 +38,14 @@ import ( type Handler struct { serviceStore store.Interface upstreamStore store.Interface + routeStore store.Interface } func NewHandler() (handler.RouteRegister, error) { return &Handler{ serviceStore: store.GetStore(store.HubKeyService), upstreamStore: store.GetStore(store.HubKeyUpstream), + routeStore: store.GetStore(store.HubKeyRoute), }, nil } @@ -232,8 +234,34 @@ type BatchDelete struct { func (h *Handler) BatchDelete(c droplet.Context) (interface{}, error) { input := c.Input().(*BatchDelete) + ids := strings.Split(input.IDs, ",") + mp := make(map[string]struct{}) + for _, id := range ids { + mp[id] = struct{}{} + } + + ret, err := h.routeStore.List(c.Context(), store.ListInput{ + Predicate: func(obj interface{}) bool { + route := obj.(*entity.Route) + if _, exist := mp[utils.InterfaceToString(route.ServiceID)]; exist { + return true + } + + return false + }, + PageSize: 0, + PageNumber: 0, + }) + if err != nil { + return handler.SpecCodeResponse(err), err + } + + if ret.TotalSize > 0 { + return &data.SpecCodeResponse{StatusCode: http.StatusBadRequest}, + fmt.Errorf("route: %s is using this service", ret.Rows[0].(*entity.Route).Name) + } - if err := h.serviceStore.BatchDelete(c.Context(), strings.Split(input.IDs, ",")); err != nil { + if err := h.serviceStore.BatchDelete(c.Context(), ids); err != nil { return handler.SpecCodeResponse(err), err } diff --git a/api/internal/handler/service/service_test.go b/api/internal/handler/service/service_test.go index 55c47b5..3c508d0 100644 --- a/api/internal/handler/service/service_test.go +++ b/api/internal/handler/service/service_test.go @@ -18,6 +18,7 @@ package service import ( + "errors" "fmt" "net/http" "testing" @@ -819,12 +820,15 @@ func TestService_Patch(t *testing.T) { func TestServices_Delete(t *testing.T) { tests := []struct { - caseDesc string - giveInput *BatchDelete - giveErr error - wantInput []string - wantErr error - wantRet interface{} + caseDesc string + giveInput *BatchDelete + giveErr error + wantInput []string + wantErr error + wantRet interface{} + routeMockData []*entity.Route + routeMockErr error + getCalled bool }{ { caseDesc: "delete success", @@ -832,6 +836,7 @@ func TestServices_Delete(t *testing.T) { IDs: "s1", }, wantInput: []string{"s1"}, + getCalled: true, }, { caseDesc: "batch delete success", @@ -839,6 +844,7 @@ func TestServices_Delete(t *testing.T) { IDs: "s1,s2", }, wantInput: []string{"s1", "s2"}, + getCalled: true, }, { caseDesc: "delete failed", @@ -849,6 +855,45 @@ func TestServices_Delete(t *testing.T) { wantInput: []string{"s1"}, wantRet: handler.SpecCodeResponse(fmt.Errorf("delete error")), wantErr: fmt.Errorf("delete error"), + getCalled: true, + }, + { + caseDesc: "delete failed, route is using", + giveInput: &BatchDelete{ + IDs: "s1", + }, + wantInput: []string{"s1"}, + routeMockData: []*entity.Route{ + &entity.Route{ + BaseInfo: entity.BaseInfo{ + ID: "r1", + CreateTime: 1609746531, + }, + Name: "route1", + Desc: "test_route", + UpstreamID: "u1", + ServiceID: "s1", + Labels: map[string]string{ + "version": "v1", + }, + }, + }, + routeMockErr: nil, + getCalled: false, + wantRet: &data.SpecCodeResponse{StatusCode: 400}, + wantErr: errors.New("route: route1 is using this service"), + }, + { + caseDesc: "delete failed, route list error", + giveInput: &BatchDelete{ + IDs: "s1", + }, + wantInput: []string{"s1"}, + routeMockData: nil, + routeMockErr: errors.New("route list error"), + wantRet: handler.SpecCodeResponse(errors.New("route list error")), + wantErr: errors.New("route list error"), + getCalled: false, }, } @@ -862,11 +907,31 @@ func TestServices_Delete(t *testing.T) { assert.Equal(t, tc.wantInput, input) }).Return(tc.giveErr) - h := Handler{serviceStore: serviceStore} + routeStore := &store.MockInterface{} + routeStore.On("List", mock.Anything).Return(func(input store.ListInput) *store.ListOutput { + var returnData []interface{} + for _, c := range tc.routeMockData { + if input.Predicate(c) { + if input.Format == nil { + returnData = append(returnData, c) + continue + } + + returnData = append(returnData, input.Format(c)) + } + } + + return &store.ListOutput{ + Rows: returnData, + TotalSize: len(returnData), + } + }, tc.routeMockErr) + + h := Handler{serviceStore: serviceStore, routeStore: routeStore} ctx := droplet.NewContext() ctx.SetInput(tc.giveInput) ret, err := h.BatchDelete(ctx) - assert.True(t, getCalled) + assert.Equal(t, tc.getCalled, getCalled) assert.Equal(t, tc.wantRet, ret) assert.Equal(t, tc.wantErr, err) }) diff --git a/api/test/e2enew/service/service_test.go b/api/test/e2enew/service/service_test.go index 3c35866..a8201db 100644 --- a/api/test/e2enew/service/service_test.go +++ b/api/test/e2enew/service/service_test.go @@ -25,6 +25,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/apisix/manager-api/test/e2enew/base" + "github.com/onsi/ginkgo/extensions/table" ) var _ = ginkgo.Describe("create service without plugin", func() { @@ -518,3 +519,107 @@ var _ = ginkgo.Describe("service update use patch method", func() { }) }) }) + +var _ = ginkgo.Describe("test service delete", func() { + t := ginkgo.GinkgoT() + var createServiceBody map[string]interface{} = map[string]interface{}{ + "name": "testservice", + "upstream": map[string]interface{}{ + "type": "roundrobin", + "nodes": []map[string]interface{}{ + { + "host": base.UpstreamIp, + "port": 1980, + "weight": 1, + }, + }, + }, + } + _createServiceBody, err := json.Marshal(createServiceBody) + assert.Nil(t, err) + + table.DescribeTable("test service delete", + func(tc base.HttpTestCase) { + base.RunTestCase(tc) + }, + table.Entry("create service without plugin", base.HttpTestCase{ + Desc: "create service without plugin", + Object: base.ManagerApiExpect(), + Method: http.MethodPut, + Path: "/apisix/admin/services/s1", + Headers: map[string]string{"Authorization": base.GetToken()}, + Body: string(_createServiceBody), + ExpectStatus: http.StatusOK, + ExpectBody: "\"name\":\"testservice\",\"upstream\":{\"nodes\":[{\"host\":\"" + base.UpstreamIp + "\",\"port\":1980,\"weight\":1}],\"type\":\"roundrobin\"}}", + }), + table.Entry("create route use service s1", base.HttpTestCase{ + Desc: "create route use service s1", + Object: base.ManagerApiExpect(), + Method: http.MethodPut, + Path: "/apisix/admin/routes/r1", + Body: `{ + "id": "r1", + "name": "route1", + "uri": "/hello", + "upstream": { + "type": "roundrobin", + "nodes": { + "` + base.UpstreamIp + `:1980": 1 + } + }, + "service_id": "s1" + }`, + Headers: map[string]string{"Authorization": base.GetToken()}, + ExpectStatus: http.StatusOK, + ExpectBody: "\"service_id\":\"s1\"", + }), + table.Entry("hit route on apisix", base.HttpTestCase{ + Object: base.APISIXExpect(), + Method: http.MethodGet, + Path: "/hello", + ExpectStatus: http.StatusOK, + ExpectBody: "hello world", + Sleep: base.SleepTime, + }), + table.Entry("delete service failed", base.HttpTestCase{ + Desc: "delete service failed", + Object: base.ManagerApiExpect(), + Method: http.MethodDelete, + Path: "/apisix/admin/services/s1", + Headers: map[string]string{"Authorization": base.GetToken()}, + ExpectStatus: http.StatusBadRequest, + ExpectBody: "route: route1 is using this service", + }), + table.Entry("delete route first", base.HttpTestCase{ + Desc: "delete route first", + Object: base.ManagerApiExpect(), + Method: http.MethodDelete, + Path: "/apisix/admin/routes/r1", + Headers: map[string]string{"Authorization": base.GetToken()}, + ExpectStatus: http.StatusOK, + }), + table.Entry("check route exist", base.HttpTestCase{ + Desc: "check route exist", + Object: base.ManagerApiExpect(), + Method: http.MethodDelete, + Path: "/apisix/admin/routes/r1", + Headers: map[string]string{"Authorization": base.GetToken()}, + ExpectStatus: http.StatusNotFound, + }), + table.Entry("delete service success", base.HttpTestCase{ + Desc: "delete service success", + Object: base.ManagerApiExpect(), + Method: http.MethodDelete, + Path: "/apisix/admin/services/s1", + Headers: map[string]string{"Authorization": base.GetToken()}, + ExpectStatus: http.StatusOK, + }), + table.Entry("check the service exist", base.HttpTestCase{ + Desc: "check the exist", + Object: base.ManagerApiExpect(), + Method: http.MethodGet, + Path: "/apisix/admin/services/s1", + Headers: map[string]string{"Authorization": base.GetToken()}, + ExpectStatus: http.StatusNotFound, + })) +})