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 all 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 @@
{
"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"
}
}
}
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]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
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":{"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)
})
}
}
16 changes: 9 additions & 7 deletions api/internal/core/store/storehub.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
32 changes: 16 additions & 16 deletions api/internal/core/store/test_case.json
Original file line number Diff line number Diff line change
@@ -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
}
76 changes: 76 additions & 0 deletions api/internal/core/store/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
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
}
1 change: 1 addition & 0 deletions api/test/docker/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down