This is an automated email from the ASF dual-hosted git repository.

chenjunxu 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 4d1301f  feat: support disable property for json schema according to 
APISIX's change (#904)
4d1301f is described below

commit 4d1301f71c8e34afc1715e44e26fafbcaa8db920
Author: nic-chen <33000667+nic-c...@users.noreply.github.com>
AuthorDate: Tue Dec 8 20:59:52 2020 +0800

    feat: support disable property for json schema according to APISIX's change 
(#904)
    
    * feat: support disable property for json schema according to APISIX's 
change
---
 api/internal/core/store/validate.go          |  43 +++++---
 api/internal/core/store/validate_test.go     |  61 ++++++++++-
 api/test/e2e/route_with_limit_plugin_test.go | 151 +++++++++++++++++++++++++++
 3 files changed, 242 insertions(+), 13 deletions(-)

diff --git a/api/internal/core/store/validate.go 
b/api/internal/core/store/validate.go
index c47b634..ea7d1f5 100644
--- a/api/internal/core/store/validate.go
+++ b/api/internal/core/store/validate.go
@@ -17,10 +17,10 @@
 package store
 
 import (
+       "encoding/json"
        "errors"
        "fmt"
        "io/ioutil"
-       "regexp"
 
        "github.com/xeipuuv/gojsonschema"
        "go.uber.org/zap/buffer"
@@ -252,31 +252,50 @@ func (v *APISIXJsonSchemaValidator) Validate(obj 
interface{}) error {
        }
 
        plugins, schemaType := getPlugins(obj)
-       //fix lua json.encode transform lua{properties={}} to 
json{"properties":[]}
-       reg := regexp.MustCompile(`\"properties\":\[\]`)
        for pluginName, pluginConf := range plugins {
-               var schemaDef string
-               schemaDef = conf.Schema.Get("plugins." + pluginName + "." + 
schemaType).String()
-               if schemaDef == "" && schemaType == "consumer_schema" {
-                       schemaDef = conf.Schema.Get("plugins." + pluginName + 
".schema").String()
+               schemaValue := conf.Schema.Get("plugins." + pluginName + "." + 
schemaType).Value()
+               if schemaValue == nil && schemaType == "consumer_schema" {
+                       schemaValue = conf.Schema.Get("plugins." + pluginName + 
".schema").Value()
                }
-               if schemaDef == "" {
-                       log.Warnf("schema validate failed: schema not found, 
path: %s", "plugins."+pluginName)
+               if schemaValue == nil {
+                       log.Warnf("schema validate failed: schema not found,  
%s, %s", "plugins."+pluginName, schemaType)
                        return fmt.Errorf("schema validate failed: schema not 
found, path: %s", "plugins."+pluginName)
                }
+               schemaMap := schemaValue.(map[string]interface{})
+               schemaByte, err := json.Marshal(schemaMap)
+               if err != nil {
+                       log.Warnf("schema validate failed: schema json encode 
failed, path: %s, %w", "plugins."+pluginName, err)
+                       return fmt.Errorf("schema validate failed: schema json 
encode failed, path: %s, %w", "plugins."+pluginName, err)
+               }
 
-               schemaDef = reg.ReplaceAllString(schemaDef, `"properties":{}`)
-               s, err := 
gojsonschema.NewSchema(gojsonschema.NewStringLoader(schemaDef))
+               s, err := 
gojsonschema.NewSchema(gojsonschema.NewBytesLoader(schemaByte))
                if err != nil {
                        log.Warnf("init schema validate failed: %w", err)
                        return fmt.Errorf("schema validate failed: %w", err)
                }
 
-               ret, err := s.Validate(gojsonschema.NewGoLoader(pluginConf))
+               // check property disable, if is bool, remove from json schema 
checking
+               conf := pluginConf.(map[string]interface{})
+               var exchange bool
+               disable, ok := conf["disable"]
+               if ok {
+                       if fmt.Sprintf("%T", disable) == "bool" {
+                               delete(conf, "disable")
+                               exchange = true
+                       }
+               }
+
+               // check schema
+               ret, err := s.Validate(gojsonschema.NewGoLoader(conf))
                if err != nil {
                        return fmt.Errorf("schema validate failed: %w", err)
                }
 
+               // put the value back to the property disable
+               if exchange {
+                       conf["disable"] = disable
+               }
+
                if !ret.Valid() {
                        errString := buffer.Buffer{}
                        for i, vErr := range ret.Errors() {
diff --git a/api/internal/core/store/validate_test.go 
b/api/internal/core/store/validate_test.go
index 369329f..72a398e 100644
--- a/api/internal/core/store/validate_test.go
+++ b/api/internal/core/store/validate_test.go
@@ -137,7 +137,6 @@ func TestAPISIXJsonSchemaValidator_Validate(t *testing.T) {
        err = validator.Validate(consumer3)
        assert.NotNil(t, err)
        assert.EqualError(t, err, "schema validate failed: (root): count is 
required")
-
 }
 
 func TestAPISIXJsonSchemaValidator_checkUpstream(t *testing.T) {
@@ -249,6 +248,66 @@ func TestAPISIXJsonSchemaValidator_checkUpstream(t 
*testing.T) {
        assert.EqualError(t, err, "schema validate failed: (root): Does not 
match pattern 
'^((uri|server_name|server_addr|request_uri|remote_port|remote_addr|query_string|host|hostname)|arg_[0-9a-zA-z_-]+)$'")
 }
 
+func TestAPISIXJsonSchemaValidator_Plugin(t *testing.T) {
+       validator, err := NewAPISIXJsonSchemaValidator("main.route")
+       assert.Nil(t, err)
+
+       // validate plugin's schema which has no `properties` or empty 
`properties`
+       route := &entity.Route{}
+       reqBody := `{
+               "id": "1",
+               "uri": "/hello",
+               "plugins": {
+                       "prometheus": {
+                               "disable": false
+                       },
+                       "key-auth": {
+                               "disable": true
+                       }
+               }
+       }`
+       err = json.Unmarshal([]byte(reqBody), route)
+       assert.Nil(t, err)
+       err = validator.Validate(route)
+       assert.Nil(t, err)
+
+       // validate plugin's schema which use `oneOf`
+       reqBody = `{
+               "id": "1",
+               "uri": "/hello",
+               "plugins": {
+                        "ip-restriction": {
+                            "blacklist": [
+                                "127.0.0.0/24"
+                            ],
+                            "disable": true
+                        }
+               }
+       }`
+       err = json.Unmarshal([]byte(reqBody), route)
+       assert.Nil(t, err)
+       err = validator.Validate(route)
+       assert.Nil(t, err)
+
+       // validate plugin's schema with invalid type for `disable`
+       reqBody = `{
+               "id": "1",
+               "uri": "/hello",
+               "plugins": {
+                        "ip-restriction": {
+                            "blacklist": [
+                                "127.0.0.0/24"
+                            ],
+                            "disable": 1
+                        }
+               }
+       }`
+       err = json.Unmarshal([]byte(reqBody), route)
+       assert.Nil(t, err)
+       err = validator.Validate(route)
+       assert.Equal(t, fmt.Errorf("schema validate failed: (root): Must 
validate one and only one schema (oneOf)\n(root): Additional property disable 
is not allowed"), err)
+}
+
 func TestAPISIXJsonSchemaValidator_Route_checkRemoteAddr(t *testing.T) {
        tests := []struct {
                caseDesc        string
diff --git a/api/test/e2e/route_with_limit_plugin_test.go 
b/api/test/e2e/route_with_limit_plugin_test.go
index be1fcfb..39f0fb7 100644
--- a/api/test/e2e/route_with_limit_plugin_test.go
+++ b/api/test/e2e/route_with_limit_plugin_test.go
@@ -302,3 +302,154 @@ func TestRoute_With_Limit_Plugin_By_Consumer(t 
*testing.T) {
                testCaseCheck(tc)
        }
 }
+
+func TestRoute_With_Limit_Count_And_Disable(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",
+                               "plugins": {
+                                       "limit-count": {
+                                               "count": 2,
+                                               "time_window": 2,
+                                               "rejected_code": 503,
+                                               "key": "remote_addr",
+                                               "disable": false
+                                       }
+                               },
+                               "upstream": {
+                                       "type": "roundrobin",
+                                       "nodes": [{
+                                               "host": "172.16.238.20",
+                                               "port": 1981,
+                                               "weight": 1
+                                       }]
+                               }
+                       }`,
+                       Headers:      map[string]string{"Authorization": token},
+                       ExpectStatus: http.StatusOK,
+                       ExpectBody:   `"code":0`,
+               },
+               {
+                       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:        2 * time.Second,
+               },
+               {
+                       caseDesc: "update route to disable plugin limit-count",
+                       Object:   ManagerApiExpect(t),
+                       Method:   http.MethodPut,
+                       Path:     "/apisix/admin/routes/r1",
+                       Body: `{
+                               "uri": "/hello",
+                               "plugins": {
+                                       "limit-count": {
+                                               "count": 1,
+                                               "time_window": 2,
+                                               "rejected_code": 503,
+                                               "key": "remote_addr",
+                                               "disable": true
+                                       }
+                               },
+                               "upstream": {
+                                       "type": "roundrobin",
+                                       "nodes": [{
+                                               "host": "172.16.238.20",
+                                               "port": 1981,
+                                               "weight": 1
+                                       }]
+                               }
+                       }`,
+                       Headers:      map[string]string{"Authorization": token},
+                       ExpectStatus: http.StatusOK,
+                       ExpectBody:   `"code":0`,
+               },
+               {
+                       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 
(exceed config count)",
+                       Object:       APISIXExpect(t),
+                       Method:       http.MethodGet,
+                       Path:         "/hello",
+                       ExpectStatus: http.StatusOK,
+                       ExpectBody:   "hello world",
+               },
+               {
+                       caseDesc:     "verify route that should not be limited 
(exceed config count again)",
+                       Object:       APISIXExpect(t),
+                       Method:       http.MethodGet,
+                       Path:         "/hello",
+                       ExpectStatus: http.StatusOK,
+                       ExpectBody:   "hello world",
+               },
+               {
+                       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)
+       }
+}

Reply via email to