From a964db4c12afbe2c4d91565ac12d55d05683aab0 Mon Sep 17 00:00:00 2001 From: LetsGO Date: Tue, 12 Apr 2022 09:26:21 +0800 Subject: [PATCH] chore: use json schema instead hard code (#2399) --- api/build.sh | 1 + 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 | 16 ++-- api/internal/core/store/test_case.json | 32 ++++---- api/internal/core/store/validate_test.go | 76 +++++++++++++++++++ .../handler/system_config/system_config.go | 23 ------ api/test/docker/Dockerfile | 1 + 9 files changed, 247 insertions(+), 51 deletions(-) create mode 100644 api/conf/customize_schema.json create mode 100644 api/internal/conf/conf_test.go 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" diff --git a/api/conf/customize_schema.json b/api/conf/customize_schema.json new file mode 100644 index 0000000000..bde2563183 --- /dev/null +++ b/api/conf/customize_schema.json @@ -0,0 +1,33 @@ +{ + "main": { + "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..20074238fc 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]map[string]interface{} + customizeSchemaMap map[string]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["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["main"] { + apisixSchemaMap["main"][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..1a1a1c3ef3 --- /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":{"b":1}}`), + }, + wantErr: true, + 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(`{"main":{"c":3}}`), + }, + wantErr: false, + wantRes: []byte(`{"main":{"a":1,"b":2,"c":3},"plugins":{"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..9efc685511 100644 --- a/api/internal/core/store/storehub.go +++ b/api/internal/core/store/storehub.go @@ -49,14 +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, + HubKeyConsumer: true, + HubKeyRoute: true, + HubKeySsl: true, + HubKeyService: true, + HubKeyUpstream: true, + HubKeyGlobalRule: true, + HubKeyStreamRoute: true, + HubKeySystemConfig: true, } + 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 index 950edf34d9..616dd5514f 100644 --- a/api/internal/core/store/test_case.json +++ b/api/internal/core/store/test_case.json @@ -1,19 +1,19 @@ { - "$schema": "http://json-schema.org/draft-04/schema#", - "type": "object", - "properties": { - "name": { - "type": "string", - "minLength": 10 + "$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 + } }, - "email": { - "type": "string", - "maxLength": 10 - }, - "age": { - "type": "integer", - "minimum": 0 - } - }, - "additionalProperties": false + "additionalProperties": false } diff --git a/api/internal/core/store/validate_test.go b/api/internal/core/store/validate_test.go index 805049975a..c85e39a5a3 100644 --- a/api/internal/core/store/validate_test.go +++ b/api/internal/core/store/validate_test.go @@ -428,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/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 \