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) + } +}