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 <[email protected]>
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,
+ }))
+})
