Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: use json schema instead hard code #2399

Merged
merged 3 commits into from
Apr 12, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions api/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
33 changes: 33 additions & 0 deletions api/conf/customize_schema.json
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
53 changes: 48 additions & 5 deletions api/internal/conf/conf.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package conf

import (
"encoding/json"
"fmt"
"io/ioutil"
"os"
Expand Down Expand Up @@ -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
Expand Down
63 changes: 63 additions & 0 deletions api/internal/conf/conf_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
}
13 changes: 13 additions & 0 deletions api/internal/core/store/storehub.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not put this under "main"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

schema.json is output from apisix, my idea is to use different names for distinguishing them

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

if err != nil {
return err
}
opt.Validator = validator
}

if _, ok := hubsNeedCheck[key]; ok {
validator, err := NewAPISIXJsonSchemaValidator("main." + string(key))
if err != nil {
Expand Down
19 changes: 0 additions & 19 deletions api/internal/core/store/test_case.json

This file was deleted.

11 changes: 5 additions & 6 deletions api/internal/core/store/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ import (
"encoding/json"
"errors"
"fmt"
"io/ioutil"

"github.com/xeipuuv/gojsonschema"
"go.uber.org/zap/buffer"

Expand All @@ -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)
}
Expand Down
77 changes: 53 additions & 24 deletions api/internal/core/store/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion api/internal/handler/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}
}

Expand Down
23 changes: 0 additions & 23 deletions api/internal/handler/system_config/system_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package system_config

import (
"errors"
"reflect"
"time"

Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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
}
Loading