You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by ch...@apache.org on 2020/12/08 13:00:02 UTC
[apisix-dashboard] branch master updated: feat: support disable
property for json schema according to APISIX's change (#904)
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 <33...@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)
+ }
+}