From 93c91da750724ebdb6f44fad5b5f33c7e8065dc5 Mon Sep 17 00:00:00 2001 From: Wei Jiang Date: Thu, 31 Mar 2022 17:43:59 +0800 Subject: [PATCH 1/3] chore: use json schema insted hard code Signed-off-by: Wei Jiang --- api/conf/customize_schema.json | 33 ++++++++ api/internal/conf/conf.go | 53 +++++++++++-- api/internal/conf/conf_test.go | 63 +++++++++++++++ api/internal/core/store/storehub.go | 13 ++++ api/internal/core/store/test_case.json | 19 ----- api/internal/core/store/validate.go | 11 ++- api/internal/core/store/validate_test.go | 77 +++++++++++++------ api/internal/handler/handler.go | 3 +- .../handler/system_config/system_config.go | 23 ------ api/test/docker/Dockerfile | 1 + 10 files changed, 218 insertions(+), 78 deletions(-) create mode 100644 api/conf/customize_schema.json create mode 100644 api/internal/conf/conf_test.go delete mode 100644 api/internal/core/store/test_case.json diff --git a/api/conf/customize_schema.json b/api/conf/customize_schema.json new file mode 100644 index 0000000000..55055b726b --- /dev/null +++ b/api/conf/customize_schema.json @@ -0,0 +1,33 @@ +{ + "customize": { + "system_config": { + "properties": { + "config_name": { + "maxLength":100, + "minLength":1, + "pattern":"^[a-zA-Z0-9_]+$", + "type":"string" + }, + "desc": { + "maxLength":256, + "type":"string" + }, + "payload": { + "type":"object", + "minProperties":1 + }, + "create_time": { + "type":"integer" + }, + "update_time": { + "type":"integer" + } + }, + "required": [ + "config_name", + "payload" + ], + "type":"object" + } + } +} diff --git a/api/internal/conf/conf.go b/api/internal/conf/conf.go index 5087edf77d..920eb0c6b8 100644 --- a/api/internal/conf/conf.go +++ b/api/internal/conf/conf.go @@ -17,6 +17,7 @@ package conf import ( + "encoding/json" "fmt" "io/ioutil" "os" @@ -289,12 +290,54 @@ func initPlugins(plugins []string) { } func initSchema() { - filePath := WorkDir + "/conf/schema.json" - if schemaContent, err := ioutil.ReadFile(filePath); err != nil { - panic(fmt.Sprintf("fail to read configuration: %s", filePath)) - } else { - Schema = gjson.ParseBytes(schemaContent) + var ( + apisixSchemaPath = WorkDir + "/conf/schema.json" + customizeSchemaPath = WorkDir + "/conf/customize_schema.json" + apisixSchemaContent []byte + customizeSchemaContent []byte + err error + ) + + if apisixSchemaContent, err = ioutil.ReadFile(apisixSchemaPath); err != nil { + panic(fmt.Errorf("fail to read configuration: %s, error: %s", apisixSchemaPath, err.Error())) + } + + if customizeSchemaContent, err = ioutil.ReadFile(customizeSchemaPath); err != nil { + panic(fmt.Errorf("fail to read configuration: %s, error: %s", customizeSchemaPath, err.Error())) + } + + content, err := mergeSchema(apisixSchemaContent, customizeSchemaContent) + if err != nil { + panic(err) + } + + Schema = gjson.ParseBytes(content) +} + +func mergeSchema(apisixSchema, customizeSchema []byte) ([]byte, error) { + var ( + apisixSchemaMap map[string]interface{} + customizeSchemaMap map[string]interface{} + ) + + if err := json.Unmarshal(apisixSchema, &apisixSchemaMap); err != nil { + return nil, err + } + if err := json.Unmarshal(customizeSchema, &customizeSchemaMap); err != nil { + return nil, err } + + for key := range apisixSchemaMap { + if _, ok := customizeSchemaMap[key]; ok { + return nil, fmt.Errorf("duplicates key: %s between schema.json and customize_schema.json", key) + } + } + + for k, v := range customizeSchemaMap { + apisixSchemaMap[k] = v + } + + return json.Marshal(apisixSchemaMap) } // initialize etcd config diff --git a/api/internal/conf/conf_test.go b/api/internal/conf/conf_test.go new file mode 100644 index 0000000000..ef58784995 --- /dev/null +++ b/api/internal/conf/conf_test.go @@ -0,0 +1,63 @@ +package conf + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" +) + +func Test_mergeSchema(t *testing.T) { + type args struct { + apisixSchema []byte + customizeSchema []byte + } + tests := []struct { + name string + args args + wantRes []byte + wantErr bool + wantErrMessage string + }{ + { + name: "should failed when have duplicates key", + args: args{ + apisixSchema: []byte(`{"main":{"a":1,"b":2},"plugins":{"a":1}}`), + customizeSchema: []byte(`{"main":{"c":1},"customize":{"a":1}}`), + }, + wantErr: true, + wantErrMessage: "duplicates key: main between schema.json and customize_schema.json", + }, + { + name: "should success", + args: args{ + apisixSchema: []byte(`{"main":{"a":1,"b":2},"plugins":{"a":1}}`), + customizeSchema: []byte(`{"customize":{"a":1}}`), + }, + wantErr: false, + wantRes: []byte(`{"main":{"a":1,"b":2},"plugins":{"a":1},"customize":{"a":1}}`), + }, + } + for _, tt := range tests { + + t.Run(tt.name, func(t *testing.T) { + var ( + wantMap map[string]interface{} + gotMap map[string]interface{} + ) + + got, err := mergeSchema(tt.args.apisixSchema, tt.args.customizeSchema) + if tt.wantErr { + assert.Equal(t, tt.wantErrMessage, err.Error()) + return + } + + assert.NoError(t, err) + err = json.Unmarshal(got, &gotMap) + assert.NoError(t, err) + err = json.Unmarshal(tt.wantRes, &wantMap) + assert.NoError(t, err) + assert.Equal(t, wantMap, gotMap) + }) + } +} diff --git a/api/internal/core/store/storehub.go b/api/internal/core/store/storehub.go index 76d15fe4cc..ec4001a12f 100644 --- a/api/internal/core/store/storehub.go +++ b/api/internal/core/store/storehub.go @@ -57,6 +57,19 @@ func InitStore(key HubKey, opt GenericStoreOption) error { HubKeyGlobalRule: true, HubKeyStreamRoute: true, } + + hubsNeedCheckByCustomize := map[HubKey]bool{ + HubKeySystemConfig: true, + } + + if _, ok := hubsNeedCheckByCustomize[key]; ok { + validator, err := NewJsonSchemaValidator("customize." + string(key)) + if err != nil { + return err + } + opt.Validator = validator + } + if _, ok := hubsNeedCheck[key]; ok { validator, err := NewAPISIXJsonSchemaValidator("main." + string(key)) if err != nil { diff --git a/api/internal/core/store/test_case.json b/api/internal/core/store/test_case.json deleted file mode 100644 index 950edf34d9..0000000000 --- a/api/internal/core/store/test_case.json +++ /dev/null @@ -1,19 +0,0 @@ -{ - "$schema": "http://json-schema.org/draft-04/schema#", - "type": "object", - "properties": { - "name": { - "type": "string", - "minLength": 10 - }, - "email": { - "type": "string", - "maxLength": 10 - }, - "age": { - "type": "integer", - "minimum": 0 - } - }, - "additionalProperties": false -} diff --git a/api/internal/core/store/validate.go b/api/internal/core/store/validate.go index abf5933185..83f8a1f640 100644 --- a/api/internal/core/store/validate.go +++ b/api/internal/core/store/validate.go @@ -20,8 +20,6 @@ import ( "encoding/json" "errors" "fmt" - "io/ioutil" - "github.com/xeipuuv/gojsonschema" "go.uber.org/zap/buffer" @@ -38,11 +36,12 @@ type JsonSchemaValidator struct { } func NewJsonSchemaValidator(jsonPath string) (Validator, error) { - bs, err := ioutil.ReadFile(jsonPath) - if err != nil { - return nil, fmt.Errorf("get abs path failed: %s", err) + schemaDef := conf.Schema.Get(jsonPath).String() + if schemaDef == "" { + log.Errorf("schema validate failed: schema not found, path: %s", jsonPath) + return nil, fmt.Errorf("schema validate failed: schema not found, path: %s", jsonPath) } - s, err := gojsonschema.NewSchema(gojsonschema.NewStringLoader(string(bs))) + s, err := gojsonschema.NewSchema(gojsonschema.NewStringLoader(schemaDef)) if err != nil { return nil, fmt.Errorf("new schema failed: %s", err) } diff --git a/api/internal/core/store/validate_test.go b/api/internal/core/store/validate_test.go index 805049975a..00fcd34283 100644 --- a/api/internal/core/store/validate_test.go +++ b/api/internal/core/store/validate_test.go @@ -26,48 +26,77 @@ import ( "github.com/apisix/manager-api/internal/core/entity" ) -type TestObj struct { - Name string `json:"name"` - Email string `json:"email"` - Age int `json:"age"` -} - func TestJsonSchemaValidator_Validate(t *testing.T) { tests := []struct { + name string givePath string giveObj interface{} - wantNewErr error - wantValidateErr []error + wantNewErr bool + wantValidateErr bool + wantErrMessage string }{ { - givePath: "./test_case.json", - giveObj: TestObj{ - Name: "lessName", - Email: "too long name greater than 10", - Age: 12, + name: "new json schema validator failed", + givePath: "customize.xxx", + wantNewErr: true, + wantErrMessage: "schema validate failed: schema not found, path: customize.xxx", + }, + { + name: "invalid configName (configName is empty)", + givePath: "customize.system_config", + giveObj: &entity.SystemConfig{ + Payload: map[string]interface{}{"a": 1}, + }, + wantValidateErr: true, + wantErrMessage: "config_name: String length must be greater than or equal to 1\nconfig_name: Does not match pattern '^[a-zA-Z0-9_]+$'", + }, + { + name: "invalid configName (configName do not match regex)", + givePath: "customize.system_config", + giveObj: &entity.SystemConfig{ + ConfigName: "1@2", + Payload: map[string]interface{}{"a": 1}, + }, + wantValidateErr: true, + wantErrMessage: "config_name: Does not match pattern '^[a-zA-Z0-9_]+$'", + }, + { + name: "invalid payload", + givePath: "customize.system_config", + giveObj: &entity.SystemConfig{ + ConfigName: "cc", }, - wantValidateErr: []error{ - fmt.Errorf("name: String length must be greater than or equal to 10\nemail: String length must be less than or equal to 10"), - fmt.Errorf("email: String length must be less than or equal to 10\nname: String length must be greater than or equal to 10"), + wantValidateErr: true, + wantErrMessage: "(root): payload is required", + }, + { + name: "validate should succeed", + givePath: "customize.system_config", + giveObj: &entity.SystemConfig{ + ConfigName: "aaa", + Payload: map[string]interface{}{"a": 1}, }, }, } for _, tc := range tests { v, err := NewJsonSchemaValidator(tc.givePath) - if err != nil { - assert.Equal(t, tc.wantNewErr, err) + if tc.wantNewErr { + assert.Error(t, err) + assert.Equal(t, tc.wantErrMessage, err.Error()) continue } + + assert.NotNil(t, v) err = v.Validate(tc.giveObj) - ret := false - for _, wantErr := range tc.wantValidateErr { - if wantErr.Error() == err.Error() { - ret = true - } + if tc.wantValidateErr { + assert.Error(t, err) + assert.Equal(t, tc.wantErrMessage, err.Error()) + continue } - assert.True(t, ret) + assert.NoError(t, err) } + } func TestAPISIXJsonSchemaValidator_Validate(t *testing.T) { diff --git a/api/internal/handler/handler.go b/api/internal/handler/handler.go index af5c9dc220..ea032c99b0 100644 --- a/api/internal/handler/handler.go +++ b/api/internal/handler/handler.go @@ -62,7 +62,8 @@ func SpecCodeResponse(err error) *data.SpecCodeResponse { strings.Contains(errMsg, "conflicted") || strings.Contains(errMsg, "invalid") || strings.Contains(errMsg, "missing") || - strings.Contains(errMsg, "schema validate failed") { + strings.Contains(errMsg, "schema validate failed") || + strings.Contains(errMsg, "not match pattern") { return &data.SpecCodeResponse{StatusCode: http.StatusBadRequest} } diff --git a/api/internal/handler/system_config/system_config.go b/api/internal/handler/system_config/system_config.go index b74acb3866..154c029da9 100644 --- a/api/internal/handler/system_config/system_config.go +++ b/api/internal/handler/system_config/system_config.go @@ -17,7 +17,6 @@ package system_config import ( - "errors" "reflect" "time" @@ -72,11 +71,6 @@ func (h *Handler) Post(c droplet.Context) (interface{}, error) { input.CreateTime = time.Now().Unix() input.UpdateTime = time.Now().Unix() - // TODO use json schema to do it - if err := h.checkSystemConfig(input); err != nil { - return handler.SpecCodeResponse(err), err - } - // create res, err := h.systemConfig.Create(c.Context(), input) if err != nil { @@ -90,11 +84,6 @@ func (h *Handler) Put(c droplet.Context) (interface{}, error) { input := c.Input().(*entity.SystemConfig) input.UpdateTime = time.Now().Unix() - // TODO use json schema to do it - if err := h.checkSystemConfig(input); err != nil { - return handler.SpecCodeResponse(err), err - } - // update res, err := h.systemConfig.Update(c.Context(), input, false) if err != nil { @@ -118,15 +107,3 @@ func (h *Handler) Delete(c droplet.Context) (interface{}, error) { return nil, nil } - -func (h *Handler) checkSystemConfig(input *entity.SystemConfig) error { - if len(input.ConfigName) < 1 || len(input.ConfigName) > 100 { - return errors.New("invalid params: config_name length must be between 1 and 100") - } - - if len(input.Payload) < 1 { - return errors.New("invalid params: payload is required") - } - - return nil -} diff --git a/api/test/docker/Dockerfile b/api/test/docker/Dockerfile index 4ef6eaae55..072960b27c 100644 --- a/api/test/docker/Dockerfile +++ b/api/test/docker/Dockerfile @@ -26,6 +26,7 @@ RUN mkdir -p /go/manager-api/conf \ && mv /go/src/github.com/apisix/manager-api/entry.sh /go/manager-api/ \ && mv /go/src/github.com/apisix/manager-api/conf/conf.yaml /go/manager-api/conf/conf.yaml \ && mv /go/src/github.com/apisix/manager-api/conf/schema.json /go/manager-api/conf/schema.json \ + && mv /go/src/github.com/apisix/manager-api/conf/customize_schema.json /go/manager-api/conf/customize_schema.json \ && rm -rf /go/src/github.com/apisix/manager-api \ && rm -rf /etc/localtime \ && ln -s /usr/share/zoneinfo/Hongkong /etc/localtime \ From 26bf689fadabaa37d1c707ac3cc0ade64562a9f9 Mon Sep 17 00:00:00 2001 From: Wei Jiang Date: Thu, 31 Mar 2022 18:11:20 +0800 Subject: [PATCH 2/3] fix Signed-off-by: Wei Jiang --- api/build.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/api/build.sh b/api/build.sh index 4df783f49a..db74cb20c1 100755 --- a/api/build.sh +++ b/api/build.sh @@ -45,6 +45,7 @@ fi cd ./api && go build -o ../output/manager-api -ldflags "${GOLDFLAGS}" ./main.go && cd .. cp ./api/conf/schema.json ./output/conf/schema.json +cp ./api/conf/customize_schema.json ./output/conf/customize_schema.json cp ./api/conf/conf*.yaml ./output/conf/ echo "Build the Manager API successfully" From 3e49805d2433544e2365c35b7c43a2477ae423f7 Mon Sep 17 00:00:00 2001 From: Wei Jiang Date: Sat, 2 Apr 2022 15:35:20 +0800 Subject: [PATCH 3/3] fix Signed-off-by: Wei Jiang --- api/conf/customize_schema.json | 2 +- api/internal/conf/conf.go | 14 +-- api/internal/conf/conf_test.go | 8 +- api/internal/core/store/storehub.go | 25 ++-- api/internal/core/store/test_case.json | 19 +++ api/internal/core/store/validate.go | 11 +- api/internal/core/store/validate_test.go | 153 +++++++++++++++-------- api/internal/handler/handler.go | 3 +- 8 files changed, 145 insertions(+), 90 deletions(-) create mode 100644 api/internal/core/store/test_case.json diff --git a/api/conf/customize_schema.json b/api/conf/customize_schema.json index 55055b726b..bde2563183 100644 --- a/api/conf/customize_schema.json +++ b/api/conf/customize_schema.json @@ -1,5 +1,5 @@ { - "customize": { + "main": { "system_config": { "properties": { "config_name": { diff --git a/api/internal/conf/conf.go b/api/internal/conf/conf.go index 920eb0c6b8..20074238fc 100644 --- a/api/internal/conf/conf.go +++ b/api/internal/conf/conf.go @@ -316,8 +316,8 @@ func initSchema() { func mergeSchema(apisixSchema, customizeSchema []byte) ([]byte, error) { var ( - apisixSchemaMap map[string]interface{} - customizeSchemaMap map[string]interface{} + apisixSchemaMap map[string]map[string]interface{} + customizeSchemaMap map[string]map[string]interface{} ) if err := json.Unmarshal(apisixSchema, &apisixSchemaMap); err != nil { @@ -327,14 +327,14 @@ func mergeSchema(apisixSchema, customizeSchema []byte) ([]byte, error) { return nil, err } - for key := range apisixSchemaMap { - if _, ok := customizeSchemaMap[key]; ok { - return nil, fmt.Errorf("duplicates key: %s between schema.json and customize_schema.json", key) + for key := range apisixSchemaMap["main"] { + if _, ok := customizeSchemaMap["main"][key]; ok { + return nil, fmt.Errorf("duplicates key: main.%s between schema.json and customize_schema.json", key) } } - for k, v := range customizeSchemaMap { - apisixSchemaMap[k] = v + for k, v := range customizeSchemaMap["main"] { + apisixSchemaMap["main"][k] = v } return json.Marshal(apisixSchemaMap) diff --git a/api/internal/conf/conf_test.go b/api/internal/conf/conf_test.go index ef58784995..1a1a1c3ef3 100644 --- a/api/internal/conf/conf_test.go +++ b/api/internal/conf/conf_test.go @@ -23,19 +23,19 @@ func Test_mergeSchema(t *testing.T) { name: "should failed when have duplicates key", args: args{ apisixSchema: []byte(`{"main":{"a":1,"b":2},"plugins":{"a":1}}`), - customizeSchema: []byte(`{"main":{"c":1},"customize":{"a":1}}`), + customizeSchema: []byte(`{"main":{"b":1}}`), }, wantErr: true, - wantErrMessage: "duplicates key: main between schema.json and customize_schema.json", + wantErrMessage: "duplicates key: main.b between schema.json and customize_schema.json", }, { name: "should success", args: args{ apisixSchema: []byte(`{"main":{"a":1,"b":2},"plugins":{"a":1}}`), - customizeSchema: []byte(`{"customize":{"a":1}}`), + customizeSchema: []byte(`{"main":{"c":3}}`), }, wantErr: false, - wantRes: []byte(`{"main":{"a":1,"b":2},"plugins":{"a":1},"customize":{"a":1}}`), + wantRes: []byte(`{"main":{"a":1,"b":2,"c":3},"plugins":{"a":1}}`), }, } for _, tt := range tests { diff --git a/api/internal/core/store/storehub.go b/api/internal/core/store/storehub.go index ec4001a12f..9efc685511 100644 --- a/api/internal/core/store/storehub.go +++ b/api/internal/core/store/storehub.go @@ -49,27 +49,16 @@ var ( func InitStore(key HubKey, opt GenericStoreOption) error { hubsNeedCheck := map[HubKey]bool{ - HubKeyConsumer: true, - HubKeyRoute: true, - HubKeySsl: true, - HubKeyService: true, - HubKeyUpstream: true, - HubKeyGlobalRule: true, - HubKeyStreamRoute: true, - } - - hubsNeedCheckByCustomize := map[HubKey]bool{ + HubKeyConsumer: true, + HubKeyRoute: true, + HubKeySsl: true, + HubKeyService: true, + HubKeyUpstream: true, + HubKeyGlobalRule: true, + HubKeyStreamRoute: true, HubKeySystemConfig: true, } - if _, ok := hubsNeedCheckByCustomize[key]; ok { - validator, err := NewJsonSchemaValidator("customize." + string(key)) - if err != nil { - return err - } - opt.Validator = validator - } - if _, ok := hubsNeedCheck[key]; ok { validator, err := NewAPISIXJsonSchemaValidator("main." + string(key)) if err != nil { diff --git a/api/internal/core/store/test_case.json b/api/internal/core/store/test_case.json new file mode 100644 index 0000000000..616dd5514f --- /dev/null +++ b/api/internal/core/store/test_case.json @@ -0,0 +1,19 @@ +{ + "$schema": "http://json-schema.org/draft-04/schema#", + "type": "object", + "properties": { + "name": { + "type": "string", + "minLength": 10 + }, + "email": { + "type": "string", + "maxLength": 10 + }, + "age": { + "type": "integer", + "minimum": 0 + } + }, + "additionalProperties": false +} diff --git a/api/internal/core/store/validate.go b/api/internal/core/store/validate.go index 83f8a1f640..abf5933185 100644 --- a/api/internal/core/store/validate.go +++ b/api/internal/core/store/validate.go @@ -20,6 +20,8 @@ import ( "encoding/json" "errors" "fmt" + "io/ioutil" + "github.com/xeipuuv/gojsonschema" "go.uber.org/zap/buffer" @@ -36,12 +38,11 @@ type JsonSchemaValidator struct { } func NewJsonSchemaValidator(jsonPath string) (Validator, error) { - schemaDef := conf.Schema.Get(jsonPath).String() - if schemaDef == "" { - log.Errorf("schema validate failed: schema not found, path: %s", jsonPath) - return nil, fmt.Errorf("schema validate failed: schema not found, path: %s", jsonPath) + bs, err := ioutil.ReadFile(jsonPath) + if err != nil { + return nil, fmt.Errorf("get abs path failed: %s", err) } - s, err := gojsonschema.NewSchema(gojsonschema.NewStringLoader(schemaDef)) + s, err := gojsonschema.NewSchema(gojsonschema.NewStringLoader(string(bs))) if err != nil { return nil, fmt.Errorf("new schema failed: %s", err) } diff --git a/api/internal/core/store/validate_test.go b/api/internal/core/store/validate_test.go index 00fcd34283..c85e39a5a3 100644 --- a/api/internal/core/store/validate_test.go +++ b/api/internal/core/store/validate_test.go @@ -26,77 +26,48 @@ import ( "github.com/apisix/manager-api/internal/core/entity" ) +type TestObj struct { + Name string `json:"name"` + Email string `json:"email"` + Age int `json:"age"` +} + func TestJsonSchemaValidator_Validate(t *testing.T) { tests := []struct { - name string givePath string giveObj interface{} - wantNewErr bool - wantValidateErr bool - wantErrMessage string + wantNewErr error + wantValidateErr []error }{ { - name: "new json schema validator failed", - givePath: "customize.xxx", - wantNewErr: true, - wantErrMessage: "schema validate failed: schema not found, path: customize.xxx", - }, - { - name: "invalid configName (configName is empty)", - givePath: "customize.system_config", - giveObj: &entity.SystemConfig{ - Payload: map[string]interface{}{"a": 1}, + givePath: "./test_case.json", + giveObj: TestObj{ + Name: "lessName", + Email: "too long name greater than 10", + Age: 12, }, - wantValidateErr: true, - wantErrMessage: "config_name: String length must be greater than or equal to 1\nconfig_name: Does not match pattern '^[a-zA-Z0-9_]+$'", - }, - { - name: "invalid configName (configName do not match regex)", - givePath: "customize.system_config", - giveObj: &entity.SystemConfig{ - ConfigName: "1@2", - Payload: map[string]interface{}{"a": 1}, - }, - wantValidateErr: true, - wantErrMessage: "config_name: Does not match pattern '^[a-zA-Z0-9_]+$'", - }, - { - name: "invalid payload", - givePath: "customize.system_config", - giveObj: &entity.SystemConfig{ - ConfigName: "cc", - }, - wantValidateErr: true, - wantErrMessage: "(root): payload is required", - }, - { - name: "validate should succeed", - givePath: "customize.system_config", - giveObj: &entity.SystemConfig{ - ConfigName: "aaa", - Payload: map[string]interface{}{"a": 1}, + wantValidateErr: []error{ + fmt.Errorf("name: String length must be greater than or equal to 10\nemail: String length must be less than or equal to 10"), + fmt.Errorf("email: String length must be less than or equal to 10\nname: String length must be greater than or equal to 10"), }, }, } for _, tc := range tests { v, err := NewJsonSchemaValidator(tc.givePath) - if tc.wantNewErr { - assert.Error(t, err) - assert.Equal(t, tc.wantErrMessage, err.Error()) + if err != nil { + assert.Equal(t, tc.wantNewErr, err) continue } - - assert.NotNil(t, v) err = v.Validate(tc.giveObj) - if tc.wantValidateErr { - assert.Error(t, err) - assert.Equal(t, tc.wantErrMessage, err.Error()) - continue + ret := false + for _, wantErr := range tc.wantValidateErr { + if wantErr.Error() == err.Error() { + ret = true + } } - assert.NoError(t, err) + assert.True(t, ret) } - } func TestAPISIXJsonSchemaValidator_Validate(t *testing.T) { @@ -457,6 +428,82 @@ func TestAPISIXJsonSchemaValidator_Route_checkRemoteAddr(t *testing.T) { } } +func TestAPISIXSchemaValidator_SystemConfig(t *testing.T) { + tests := []struct { + name string + givePath string + giveObj interface{} + wantNewErr bool + wantValidateErr bool + wantErrMessage string + }{ + { + name: "new json schema validator failed", + givePath: "main.xxx", + wantNewErr: true, + wantErrMessage: "schema validate failed: schema not found, path: main.xxx", + }, + { + name: "invalid configName (configName is empty)", + givePath: "main.system_config", + giveObj: &entity.SystemConfig{ + Payload: map[string]interface{}{"a": 1}, + }, + wantValidateErr: true, + wantErrMessage: "schema validate failed: config_name: String length must be greater than or equal to 1\nconfig_name: Does not match pattern '^[a-zA-Z0-9_]+$'", + }, + { + name: "invalid configName (configName do not match regex)", + givePath: "main.system_config", + giveObj: &entity.SystemConfig{ + ConfigName: "1@2", + Payload: map[string]interface{}{"a": 1}, + }, + wantValidateErr: true, + wantErrMessage: "schema validate failed: config_name: Does not match pattern '^[a-zA-Z0-9_]+$'", + }, + { + name: "invalid payload", + givePath: "main.system_config", + giveObj: &entity.SystemConfig{ + ConfigName: "cc", + }, + wantValidateErr: true, + wantErrMessage: "schema validate failed: (root): payload is required", + }, + { + name: "validate should succeed", + givePath: "main.system_config", + giveObj: &entity.SystemConfig{ + ConfigName: "aaa", + Payload: map[string]interface{}{"a": 1}, + }, + }, + } + + for _, tc := range tests { + validator, err := NewAPISIXSchemaValidator(tc.givePath) + if tc.wantNewErr { + assert.Error(t, err) + assert.Equal(t, tc.wantErrMessage, err.Error()) + continue + } + + assert.NoError(t, err) + assert.NotNil(t, validator) + + req, err := json.Marshal(tc.giveObj) + assert.NoError(t, err) + err = validator.Validate(req) + if tc.wantValidateErr { + assert.Error(t, err) + assert.Equal(t, tc.wantErrMessage, err.Error()) + continue + } + assert.NoError(t, err) + } +} + func TestAPISIXSchemaValidator_Validate(t *testing.T) { validator, err := NewAPISIXSchemaValidator("main.consumer") assert.Nil(t, err) diff --git a/api/internal/handler/handler.go b/api/internal/handler/handler.go index ea032c99b0..af5c9dc220 100644 --- a/api/internal/handler/handler.go +++ b/api/internal/handler/handler.go @@ -62,8 +62,7 @@ func SpecCodeResponse(err error) *data.SpecCodeResponse { strings.Contains(errMsg, "conflicted") || strings.Contains(errMsg, "invalid") || strings.Contains(errMsg, "missing") || - strings.Contains(errMsg, "schema validate failed") || - strings.Contains(errMsg, "not match pattern") { + strings.Contains(errMsg, "schema validate failed") { return &data.SpecCodeResponse{StatusCode: http.StatusBadRequest} }