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 b9a0227 fix: Support string type for the script field in Route (#1289)
b9a0227 is described below
commit b9a0227f30ac3f2bf843ab01f37e240a8bd30730
Author: Joey <majunj...@gmail.com>
AuthorDate: Fri Jan 15 12:45:29 2021 +0800
fix: Support string type for the script field in Route (#1289)
* Support string type for script field in Route
Signed-off-by: imjoey <majunj...@gmail.com>
* Add validating lua code when create/update routes
also improve the test case in unittest and e2e
Signed-off-by: imjoey <majunj...@gmail.com>
* typo fix and style format
Signed-off-by: imjoey <majunj...@gmail.com>
* Improve testcases
Signed-off-by: imjoey <majunj...@gmail.com>
* Addtional check the Script via log in APISIX
Signed-off-by: imjoey <majunj...@gmail.com>
* ngx.log print log in error.log, instead of access.log
Signed-off-by: imjoey <majunj...@gmail.com>
* Use ngx.WARN instead of INFO to enable output
Signed-off-by: imjoey <majunj...@gmail.com>
---
api/internal/handler/route/route.go | 50 +++++---
api/internal/handler/route/route_test.go | 149 ++++++++++++++++++++++-
api/internal/utils/utils.go | 8 ++
api/internal/utils/utils_test.go | 11 ++
api/test/e2e/base.go | 31 +++++
api/test/e2e/route_with_script_luacode_test.go | 156 +++++++++++++++++++++++++
6 files changed, 389 insertions(+), 16 deletions(-)
diff --git a/api/internal/handler/route/route.go
b/api/internal/handler/route/route.go
index 89083ed..73f0fa6 100644
--- a/api/internal/handler/route/route.go
+++ b/api/internal/handler/route/route.go
@@ -30,8 +30,9 @@ import (
"github.com/shiningrush/droplet"
"github.com/shiningrush/droplet/data"
"github.com/shiningrush/droplet/wrapper"
- "github.com/yuin/gopher-lua"
wgin "github.com/shiningrush/droplet/wrapper/gin"
+ lua "github.com/yuin/gopher-lua"
+
"github.com/apisix/manager-api/internal/conf"
"github.com/apisix/manager-api/internal/core/entity"
"github.com/apisix/manager-api/internal/core/store"
@@ -327,12 +328,25 @@ func (h *Handler) Create(c droplet.Context) (interface{},
error) {
script := &entity.Script{}
script.ID = utils.InterfaceToString(input.ID)
script.Script = input.Script
- //to lua
+
var err error
- input.Script, err =
generateLuaCode(input.Script.(map[string]interface{}))
- if err != nil {
- return nil, err
+ // 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.
+ if scriptConf, ok := input.Script.(map[string]interface{}); ok {
+ // For lua code of map type, syntax validation is done
by
+ // the generateLuaCode function
+ input.Script, err = generateLuaCode(scriptConf)
+ if err != nil {
+ return &data.SpecCodeResponse{StatusCode:
http.StatusBadRequest}, err
+ }
+ } else {
+ // For lua code of string type, use utility func to
syntax validation
+ err = utils.ValidateLuaCode(input.Script.(string))
+ if err != nil {
+ return &data.SpecCodeResponse{StatusCode:
http.StatusBadRequest}, err
+ }
}
+
//save original conf
if err = h.scriptStore.Create(c.Context(), script); err != nil {
return nil, err
@@ -392,17 +406,25 @@ func (h *Handler) Update(c droplet.Context) (interface{},
error) {
script := &entity.Script{}
script.ID = input.ID
script.Script = input.Script
- //to lua
+
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
+ // 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.
+ if scriptConf, ok := input.Script.(map[string]interface{}); ok {
+ // For lua code of map type, syntax validation is done
by
+ // the generateLuaCode function
+ input.Route.Script, err = generateLuaCode(scriptConf)
+ if err != nil {
+ return &data.SpecCodeResponse{StatusCode:
http.StatusBadRequest}, err
+ }
+ } else {
+ // For lua code of string type, use utility func to
syntax validation
+ err = utils.ValidateLuaCode(input.Script.(string))
+ if err != nil {
+ return &data.SpecCodeResponse{StatusCode:
http.StatusBadRequest}, err
+ }
}
+
//save original conf
if err = h.scriptStore.Update(c.Context(), script, true); err
!= nil {
//if not exists, create
diff --git a/api/internal/handler/route/route_test.go
b/api/internal/handler/route/route_test.go
index 4618755..5630d31 100644
--- a/api/internal/handler/route/route_test.go
+++ b/api/internal/handler/route/route_test.go
@@ -938,7 +938,7 @@ func TestRoute(t *testing.T) {
dataPage = retPage.(*store.ListOutput)
assert.Equal(t, len(dataPage.Rows), 1)
- //sleep
+ //sleep
time.Sleep(time.Duration(100) * time.Millisecond)
// list search and status not match
@@ -1197,7 +1197,7 @@ func TestRoute(t *testing.T) {
assert.Nil(t, err)
}
-func Test_Route_With_Script(t *testing.T) {
+func Test_Route_With_Script_Dag2lua(t *testing.T) {
// init
err := storage.InitETCDClient(conf.ETCDConfig)
assert.Nil(t, err)
@@ -1349,3 +1349,148 @@ 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.WARN,\"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.WARN,\"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.WARN,\"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.WARN,\"hit access phase\")"
+ }`
+ err = json.Unmarshal([]byte(reqBody), route)
+ assert.Nil(t, err)
+ ctx.SetInput(route)
+ ret, err = handler.Create(ctx)
+ assert.NotNil(t, err)
+ assert.EqualError(t, err, "<string> at EOF: syntax error\n")
+ assert.Equal(t, http.StatusBadRequest,
ret.(*data.SpecCodeResponse).StatusCode)
+}
diff --git a/api/internal/utils/utils.go b/api/internal/utils/utils.go
index 6b7ea10..0a612c2 100644
--- a/api/internal/utils/utils.go
+++ b/api/internal/utils/utils.go
@@ -26,6 +26,7 @@ import (
"strings"
"github.com/sony/sonyflake"
+ "github.com/yuin/gopher-lua/parse"
)
var _sf *sonyflake.Sonyflake
@@ -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 passed, otherwise a non-nil error will be returned
+func ValidateLuaCode(code string) error {
+ _, err := parse.Parse(strings.NewReader(code), "<string>")
+ return err
+}
diff --git a/api/internal/utils/utils_test.go b/api/internal/utils/utils_test.go
index 1d7c154..5cec1e8 100644
--- a/api/internal/utils/utils_test.go
+++ b/api/internal/utils/utils_test.go
@@ -117,3 +117,14 @@ func TestLabelContains(t *testing.T) {
}
assert.True(t, LabelContains(mp, reqMap))
}
+
+func TestValidateLuaCode(t *testing.T) {
+ validLuaCode := "local _M = {} \n function _M.access(api_ctx) \n
ngx.log(ngx.WARN,\"hit access phase\") \n end \nreturn _M"
+ err := ValidateLuaCode(validLuaCode)
+ assert.Nil(t, err)
+
+ invalidLuaCode := "local _M = {} \n function _M.access(api_ctx) \n
ngx.log(ngx.WARN,\"hit access phase\")"
+ err = ValidateLuaCode(invalidLuaCode)
+ assert.NotNil(t, err)
+ assert.Equal(t, "<string> at EOF: syntax error\n", err.Error())
+}
diff --git a/api/test/e2e/base.go b/api/test/e2e/base.go
index 8f7b23a..08fa71e 100644
--- a/api/test/e2e/base.go
+++ b/api/test/e2e/base.go
@@ -307,3 +307,34 @@ func CleanAPISIXErrorLog(t *testing.T) {
}
assert.Nil(t, err)
}
+
+// ReadAPISIXAccessLog reads the access log of APISIX.
+func ReadAPISIXAccessLog(t *testing.T) string {
+ cmd := exec.Command("pwd")
+ pwdByte, err := cmd.CombinedOutput()
+ pwd := string(pwdByte)
+ pwd = strings.Replace(pwd, "\n", "", 1)
+ pwd = pwd[:strings.Index(pwd, "/e2e")]
+ bytes, err := ioutil.ReadFile(pwd + "/docker/apisix_logs/access.log")
+ assert.Nil(t, err)
+ logContent := string(bytes)
+
+ return logContent
+}
+
+// CleanAPISIXAccessLog cleans the access log of APISIX.
+// It's always recommended to call this function before checking
+// its content.
+func CleanAPISIXAccessLog(t *testing.T) {
+ cmd := exec.Command("pwd")
+ pwdByte, err := cmd.CombinedOutput()
+ pwd := string(pwdByte)
+ pwd = strings.Replace(pwd, "\n", "", 1)
+ pwd = pwd[:strings.Index(pwd, "/e2e")]
+ cmd = exec.Command("sudo", "echo", " > ",
pwd+"/docker/apisix_logs/access.log")
+ _, err = cmd.CombinedOutput()
+ if err != nil {
+ fmt.Println("cmd error:", err.Error())
+ }
+ assert.Nil(t, err)
+}
diff --git a/api/test/e2e/route_with_script_luacode_test.go
b/api/test/e2e/route_with_script_luacode_test.go
new file mode 100644
index 0000000..fdfb212
--- /dev/null
+++ b/api/test/e2e/route_with_script_luacode_test.go
@@ -0,0 +1,156 @@
+/*
+ * 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"
+
+ "github.com/stretchr/testify/assert"
+)
+
+func TestRoute_with_script_lucacode(t *testing.T) {
+
+ // clean error log
+ CleanAPISIXErrorLog(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.WARN,\"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: "hit the route",
+ Object: APISIXExpect(t),
+ Method: http.MethodGet,
+ Path: "/hello",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusOK,
+ ExpectBody: "hello world\n",
+ },
+ {
+ 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.WARN,\"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.WARN,\"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.WARN,\"hit access phase\")"
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ },
+ }
+
+ for _, tc := range tests {
+ testCaseCheck(tc, t)
+ }
+
+ // sleep for process log
+ time.Sleep(1500 * time.Millisecond)
+
+ // verify the log generated by script set in Step-3 above
+ logContent := ReadAPISIXErrorLog(t)
+ assert.Contains(t, logContent, "hit access phase")
+
+ // clean log
+ CleanAPISIXErrorLog(t)
+}
