starsz commented on a change in pull request #1289:
URL: https://github.com/apache/apisix-dashboard/pull/1289#discussion_r556994094
##########
File path: api/internal/handler/route/route_test.go
##########
@@ -1349,3 +1349,146 @@ func Test_Route_With_Script(t *testing.T) {
_, err = handler.BatchDelete(ctx)
assert.Nil(t, err)
}
+
+func Test_Route_With_Script_Luacode(t *testing.T) {
+ // init
+ err := storage.InitETCDClient(conf.ETCDConfig)
+ assert.Nil(t, err)
+ err = store.InitStores()
+ assert.Nil(t, err)
+
+ handler := &Handler{
+ routeStore: store.GetStore(store.HubKeyRoute),
+ svcStore: store.GetStore(store.HubKeyService),
+ upstreamStore: store.GetStore(store.HubKeyUpstream),
+ scriptStore: store.GetStore(store.HubKeyScript),
+ }
+ assert.NotNil(t, handler)
+
+ // create with script of valid lua syntax
+ ctx := droplet.NewContext()
+ route := &entity.Route{}
+ reqBody := `{
+ "id": "1",
+ "uri": "/index.html",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "www.a.com",
+ "port": 80,
+ "weight": 1
+ }]
+ },
+ "script": "local _M = {} \n function _M.access(api_ctx) \n
ngx.log(ngx.INFO,\"hit access phase\") \n end \nreturn _M"
+ }`
Review comment:
Style: JSON format looks not very good.
I can see different JSON format in your code, maybe you should format them
like
https://github.com/apache/apisix-dashboard/pull/1289/files#diff-69dbad33b4168d98e52aecaff8a2997d5101e9936a45adf502a3cde9cb9f1763R1406-R1418
##########
File path: api/internal/utils/utils.go
##########
@@ -161,3 +162,10 @@ func LabelContains(labels map[string]string, reqLabels
map[string]struct{}) bool
return false
}
+
+// ValidateLuaCode validates lua syntax for input code, return nil
+// if passwd, otherwise a non-nil error will be returned
Review comment:
passwd? Is this a typo?
##########
File path: api/internal/handler/route/route_test.go
##########
@@ -1349,3 +1349,146 @@ func Test_Route_With_Script(t *testing.T) {
_, err = handler.BatchDelete(ctx)
assert.Nil(t, err)
}
+
+func Test_Route_With_Script_Luacode(t *testing.T) {
+ // init
+ err := storage.InitETCDClient(conf.ETCDConfig)
+ assert.Nil(t, err)
+ err = store.InitStores()
+ assert.Nil(t, err)
+
+ handler := &Handler{
+ routeStore: store.GetStore(store.HubKeyRoute),
+ svcStore: store.GetStore(store.HubKeyService),
+ upstreamStore: store.GetStore(store.HubKeyUpstream),
+ scriptStore: store.GetStore(store.HubKeyScript),
+ }
+ assert.NotNil(t, handler)
+
+ // create with script of valid lua syntax
+ ctx := droplet.NewContext()
+ route := &entity.Route{}
+ reqBody := `{
+ "id": "1",
+ "uri": "/index.html",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "www.a.com",
+ "port": 80,
+ "weight": 1
+ }]
+ },
+ "script": "local _M = {} \n function _M.access(api_ctx) \n
ngx.log(ngx.INFO,\"hit access phase\") \n end \nreturn _M"
+ }`
+ err = json.Unmarshal([]byte(reqBody), route)
+ assert.Nil(t, err)
+ ctx.SetInput(route)
+ _, err = handler.Create(ctx)
+ assert.Nil(t, err)
+
+ // sleep
+ time.Sleep(time.Duration(20) * time.Millisecond)
+
+ // get
+ input := &GetInput{}
+ input.ID = "1"
+ ctx.SetInput(input)
+ ret, err := handler.Get(ctx)
+ stored := ret.(*entity.Route)
+ assert.Nil(t, err)
+ assert.Equal(t, stored.ID, route.ID)
+ assert.Equal(t, "local _M = {} \n function _M.access(api_ctx) \n
ngx.log(ngx.INFO,\"hit access phase\") \n end \nreturn _M", stored.Script)
+
+ // update via empty script
+ route2 := &UpdateInput{}
+ route2.ID = "1"
+ reqBody = `{
+ "id": "1",
+ "uri": "/index.html",
+ "enable_websocket": true,
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "www.a.com",
+ "port": 80,
+ "weight": 1
+ }]
+ }
+ }`
+
+ err = json.Unmarshal([]byte(reqBody), route2)
+ assert.Nil(t, err)
+ ctx.SetInput(route2)
+ _, err = handler.Update(ctx)
+ assert.Nil(t, err)
+
+ //sleep
+ time.Sleep(time.Duration(100) * time.Millisecond)
+
+ //get, script should be nil
+ input = &GetInput{}
+ input.ID = "1"
+ ctx.SetInput(input)
+ ret, err = handler.Get(ctx)
+ stored = ret.(*entity.Route)
+ assert.Nil(t, err)
+ assert.Equal(t, stored.ID, route.ID)
+ assert.Nil(t, stored.Script)
+
+ // 2nd update via invalid script
+ input3 := &UpdateInput{}
+ input3.ID = "1"
+ reqBody = `{
+ "id": "1",
+ "uri": "/index.html",
+ "enable_websocket": true,
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "www.a.com",
+ "port": 80,
+ "weight": 1
+ }]
+ },
+ "script": "local _M = {} \n function _M.access(api_ctx) \n
ngx.log(ngx.INFO,\"hit access phase\")"
+ }`
+
+ err = json.Unmarshal([]byte(reqBody), input3)
+ assert.Nil(t, err)
+ ctx.SetInput(input3)
+ _, err = handler.Update(ctx)
+ // err should NOT be nil
+ assert.NotNil(t, err)
+
+ // delete test data
+ inputDel := &BatchDelete{}
+ reqBody = `{"ids": "1"}`
+ err = json.Unmarshal([]byte(reqBody), inputDel)
+ assert.Nil(t, err)
+ ctx.SetInput(inputDel)
+ _, err = handler.BatchDelete(ctx)
+ assert.Nil(t, err)
+
+ // 2nd create with script of invalid lua syntax
+ ctx = droplet.NewContext()
+ route = &entity.Route{}
+ reqBody = `{
+ "id": "1",
+ "uri": "/index.html",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "www.a.com",
+ "port": 80,
+ "weight": 1
+ }]
+ },
+ "script": "local _M = {} \n function _M.access(api_ctx) \n
ngx.log(ngx.INFO,\"hit access phase\")"
+ }`
Review comment:
Here
##########
File path: api/internal/handler/route/route.go
##########
@@ -392,16 +398,14 @@ func (h *Handler) Update(c droplet.Context) (interface{},
error) {
script := &entity.Script{}
script.ID = input.ID
script.Script = input.Script
- //to lua
+ // Explicitly to lua if input script is of the map type,
otherwise
+ // it will always represent a piece of lua code of the string
type.
var err error
- scriptConf, ok := input.Script.(map[string]interface{})
- if !ok {
- return &data.SpecCodeResponse{StatusCode:
http.StatusBadRequest},
- fmt.Errorf("invalid `script`")
- }
- input.Route.Script, err = generateLuaCode(scriptConf)
- if err != nil {
- return &data.SpecCodeResponse{StatusCode:
http.StatusInternalServerError}, err
+ if scriptConf, ok := input.Script.(map[string]interface{}); ok {
+ input.Route.Script, err = generateLuaCode(scriptConf)
+ if err != nil {
+ return &data.SpecCodeResponse{StatusCode:
http.StatusInternalServerError}, err
+ }
Review comment:
Well done! Thanks a lot.
##########
File path: api/test/e2e/route_with_script_luacode_test.go
##########
@@ -0,0 +1,130 @@
+/*
+ * 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"
+)
+
+func TestRoute_with_script_lucacode(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ Desc: "create route with script of valid lua code",
+ Object: ManagerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }]
+ },
+ "script": "local _M = {} \n function
_M.access(api_ctx) \n ngx.log(ngx.INFO,\"hit access phase\") \n end \nreturn _M"
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusOK,
+ },
+ {
+ Desc: "get the route",
+ Object: ManagerApiExpect(t),
+ Method: http.MethodGet,
+ Path: "/apisix/admin/routes/r1",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusOK,
+ ExpectBody: "hit access phase",
+ Sleep: sleepTime,
+ },
+ {
+ Desc: "update route with script of valid lua code",
+ 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
+ }]
+ },
+ "script": "local _M = {} \n function
_M.access(api_ctx) \n ngx.log(ngx.INFO,\"hit access phase\") \n end \nreturn _M"
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusOK,
+ },
+ {
+ Desc: "update route with script of invalid lua code",
+ Object: ManagerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }]
+ },
+ "script": "local _M = {} \n function
_M.access(api_ctx) \n ngx.log(ngx.INFO,\"hit access phase\")"
+ }`,
Review comment:
Ditto.
##########
File path: api/test/e2e/route_with_script_luacode_test.go
##########
@@ -0,0 +1,130 @@
+/*
+ * 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"
+)
+
+func TestRoute_with_script_lucacode(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ Desc: "create route with script of valid lua code",
+ Object: ManagerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }]
+ },
+ "script": "local _M = {} \n function
_M.access(api_ctx) \n ngx.log(ngx.INFO,\"hit access phase\") \n end \nreturn _M"
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusOK,
+ },
+ {
+ Desc: "get the route",
+ Object: ManagerApiExpect(t),
+ Method: http.MethodGet,
+ Path: "/apisix/admin/routes/r1",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusOK,
+ ExpectBody: "hit access phase",
+ Sleep: sleepTime,
+ },
+ {
+ Desc: "update route with script of valid lua code",
+ 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
+ }]
+ },
+ "script": "local _M = {} \n function
_M.access(api_ctx) \n ngx.log(ngx.INFO,\"hit access phase\") \n end \nreturn _M"
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusOK,
+ },
+ {
+ Desc: "update route with script of invalid lua code",
+ Object: ManagerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }]
+ },
+ "script": "local _M = {} \n function
_M.access(api_ctx) \n ngx.log(ngx.INFO,\"hit access phase\")"
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ },
+ {
+ Desc: "delete the route (r1)",
+ Object: ManagerApiExpect(t),
+ Method: http.MethodDelete,
+ Path: "/apisix/admin/routes/r1",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusOK,
+ Sleep: sleepTime,
+ },
+ {
+ Desc: "create route with script of invalid lua code",
+ Object: ManagerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }]
+ },
+ "script": "local _M = {} \n function
_M.access(api_ctx) \n ngx.log(ngx.INFO,\"hit access phase\")"
+ }`,
Review comment:
Ditto.
##########
File path: api/test/e2e/route_with_script_luacode_test.go
##########
@@ -0,0 +1,130 @@
+/*
+ * 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"
+)
+
+func TestRoute_with_script_lucacode(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ Desc: "create route with script of valid lua code",
+ Object: ManagerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }]
+ },
+ "script": "local _M = {} \n function
_M.access(api_ctx) \n ngx.log(ngx.INFO,\"hit access phase\") \n end \nreturn _M"
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusOK,
+ },
+ {
+ Desc: "get the route",
+ Object: ManagerApiExpect(t),
+ Method: http.MethodGet,
+ Path: "/apisix/admin/routes/r1",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusOK,
+ ExpectBody: "hit access phase",
+ Sleep: sleepTime,
+ },
+ {
+ Desc: "update route with script of valid lua code",
+ 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
+ }]
+ },
+ "script": "local _M = {} \n function
_M.access(api_ctx) \n ngx.log(ngx.INFO,\"hit access phase\") \n end \nreturn _M"
+ }`,
Review comment:
Ditto.
##########
File path: api/test/e2e/route_with_script_luacode_test.go
##########
@@ -0,0 +1,130 @@
+/*
+ * 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"
+)
+
+func TestRoute_with_script_lucacode(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ Desc: "create route with script of valid lua code",
+ Object: ManagerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }]
+ },
+ "script": "local _M = {} \n function
_M.access(api_ctx) \n ngx.log(ngx.INFO,\"hit access phase\") \n end \nreturn _M"
+ }`,
Review comment:
Ditto.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
