You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by ju...@apache.org on 2020/10/22 07:12:57 UTC

[apisix-dashboard] branch master updated: fix: add custom checking for resource's conf (#584)

This is an automated email from the ASF dual-hosted git repository.

juzhiyuan 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 c816059  fix: add custom checking for resource's conf (#584)
c816059 is described below

commit c816059cf818c2bbe655015bf789bd37bedc1dbf
Author: nic-chen <33...@users.noreply.github.com>
AuthorDate: Thu Oct 22 15:12:49 2020 +0800

    fix: add custom checking for resource's conf (#584)
    
    * fix: add custom checking for resource's conf
    
    * fix: add custom checking for resource's conf
    
    * test: add test cases
    
    * test: add test case
    
    * test: add test cases via handler
    
    * fix: remove debug log
---
 api/internal/core/store/validate.go      | 114 ++++++++++++++++++++++++++++++-
 api/internal/core/store/validate_test.go | 110 +++++++++++++++++++++++++++++
 api/internal/handler/handler.go          |   2 +
 api/internal/handler/route/route_test.go |  43 ++++++++++++
 api/internal/utils/consts/api_error.go   |   2 +-
 5 files changed, 268 insertions(+), 3 deletions(-)

diff --git a/api/internal/core/store/validate.go b/api/internal/core/store/validate.go
index 4496ee7..ad3b978 100644
--- a/api/internal/core/store/validate.go
+++ b/api/internal/core/store/validate.go
@@ -75,7 +75,7 @@ type APISIXJsonSchemaValidator struct {
 func NewAPISIXJsonSchemaValidator(jsonPath string) (Validator, error) {
 	schemaDef := conf.Schema.Get(jsonPath).String()
 	if schemaDef == "" {
-		return nil, fmt.Errorf("schema not found")
+		return nil, fmt.Errorf("scheme validate failed: schema not found, path: %s", jsonPath)
 	}
 
 	s, err := gojsonschema.NewSchema(gojsonschema.NewStringLoader(schemaDef))
@@ -102,6 +102,111 @@ func getPlugins(reqBody interface{}) map[string]interface{} {
 	return nil
 }
 
+func cHashKeySchemaCheck(upstream *entity.UpstreamDef) error {
+	if upstream.HashOn == "consumer" {
+		return nil
+	}
+	if upstream.HashOn != "vars" &&
+		upstream.HashOn != "header" &&
+		upstream.HashOn != "cookie" {
+		fmt.Errorf("invalid hash_on type: %s", upstream.HashOn)
+	}
+
+	var schemaDef string
+	if upstream.HashOn == "vars" {
+		schemaDef = conf.Schema.Get("main.upstream_hash_vars_schema").String()
+		if schemaDef == "" {
+			return fmt.Errorf("scheme validate failed: schema not found, patch: main.upstream_hash_vars_schema")
+		}
+	}
+
+	if upstream.HashOn == "header" || upstream.HashOn == "cookie" {
+		schemaDef = conf.Schema.Get("main.upstream_hash_header_schema").String()
+		if schemaDef == "" {
+			return fmt.Errorf("scheme validate failed: schema not found, path: main.upstream_hash_header_schema")
+		}
+	}
+
+	s, err := gojsonschema.NewSchema(gojsonschema.NewStringLoader(schemaDef))
+	if err != nil {
+		return fmt.Errorf("scheme validate failed: %w", err)
+	}
+
+	ret, err := s.Validate(gojsonschema.NewGoLoader(upstream.Key))
+	if err != nil {
+		return fmt.Errorf("scheme validate failed: %w", err)
+	}
+
+	if !ret.Valid() {
+		errString := buffer.Buffer{}
+		for i, vErr := range ret.Errors() {
+			if i != 0 {
+				errString.AppendString("\n")
+			}
+			errString.AppendString(vErr.String())
+		}
+		return fmt.Errorf("scheme validate failed: %s", errString.String())
+	}
+
+	return nil
+}
+
+func checkUpstream(upstream *entity.UpstreamDef) error {
+	if upstream == nil {
+		return nil
+	}
+
+	if upstream.PassHost == "node" && upstream.Nodes != nil {
+		if nodes := entity.NodesFormat(upstream.Nodes); len(nodes.([]*entity.Node)) != 1 {
+			return fmt.Errorf("only support single node for `node` mode currently")
+		}
+	}
+
+	if upstream.PassHost == "rewrite" && upstream.UpstreamHost == "" {
+		return fmt.Errorf("`upstream_host` can't be empty when `pass_host` is `rewrite`")
+	}
+
+	if upstream.Type != "chash" {
+		return nil
+	}
+
+	//to confirm
+	if upstream.HashOn == "" {
+		upstream.HashOn = "vars"
+	}
+
+	if upstream.HashOn != "consumer" && upstream.Key == "" {
+		return fmt.Errorf("missing key")
+	}
+
+	if err := cHashKeySchemaCheck(upstream); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func checkConf(reqBody interface{}) error {
+	switch reqBody.(type) {
+	case *entity.Route:
+		route := reqBody.(*entity.Route)
+		if err := checkUpstream(route.Upstream); err != nil {
+			return err
+		}
+	case *entity.Service:
+		service := reqBody.(*entity.Service)
+		if err := checkUpstream(service.Upstream); err != nil {
+			return err
+		}
+	case *entity.Upstream:
+		upstream := reqBody.(*entity.Upstream)
+		if err := checkUpstream(&upstream.UpstreamDef); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 func (v *APISIXJsonSchemaValidator) Validate(obj interface{}) error {
 	ret, err := v.schema.Validate(gojsonschema.NewGoLoader(obj))
 	if err != nil {
@@ -119,13 +224,18 @@ func (v *APISIXJsonSchemaValidator) Validate(obj interface{}) error {
 		return fmt.Errorf("scheme validate fail: %s", errString.String())
 	}
 
+	//custom check
+	if err := checkConf(obj); err != nil {
+		return err
+	}
+
 	//check plugin json schema
 	plugins := getPlugins(obj)
 	if plugins != nil {
 		for pluginName, pluginConf := range plugins {
 			schemaDef := conf.Schema.Get("plugins." + pluginName).String()
 			if schemaDef == "" {
-				return fmt.Errorf("scheme validate failed: schema not found")
+				return fmt.Errorf("scheme validate failed: schema not found, path: %s", "plugins."+pluginName)
 			}
 
 			s, err := gojsonschema.NewSchema(gojsonschema.NewStringLoader(schemaDef))
diff --git a/api/internal/core/store/validate_test.go b/api/internal/core/store/validate_test.go
index 1e26384..f2494dc 100644
--- a/api/internal/core/store/validate_test.go
+++ b/api/internal/core/store/validate_test.go
@@ -138,3 +138,113 @@ func TestAPISIXJsonSchemaValidator_Validate(t *testing.T) {
 	assert.EqualError(t, err, "scheme validate failed: (root): count is required")
 
 }
+
+func TestAPISIXJsonSchemaValidator_checkUpstream(t *testing.T) {
+	validator, err := NewAPISIXJsonSchemaValidator("main.route")
+	assert.Nil(t, err)
+
+	// type:chash, hash_on: consumer, missing key, ok
+	route := &entity.Route{}
+	reqBody := `{
+      "id": "1",
+      "methods": ["GET"],
+      "upstream": {
+          "nodes": {
+              "127.0.0.1:8080": 1
+          },
+          "type": "chash",
+          "hash_on":"consumer"
+      },
+      "desc": "new route",
+      "uri": "/index.html"
+  }`
+	json.Unmarshal([]byte(reqBody), route)
+
+	err = validator.Validate(route)
+	assert.Nil(t, err)
+
+	// type:chash, hash_on: default(vars), missing key
+	route2 := &entity.Route{}
+	reqBody = `{
+      "id": "1",
+      "methods": ["GET"],
+      "upstream": {
+          "nodes": {
+              "127.0.0.1:8080": 1
+          },
+          "type": "chash"
+      },
+      "desc": "new route",
+      "uri": "/index.html"
+  }`
+	json.Unmarshal([]byte(reqBody), route2)
+
+	err = validator.Validate(route2)
+	assert.NotNil(t, err)
+	assert.EqualError(t, err, "missing key")
+
+	//type:chash, hash_on: header, missing key
+	route3 := &entity.Route{}
+	reqBody = `{
+      "id": "1",
+      "methods": ["GET"],
+      "upstream": {
+          "nodes": {
+              "127.0.0.1:8080": 1
+          },
+          "type": "chash",
+          "hash_on":"header"
+      },
+      "desc": "new route",
+      "uri": "/index.html"
+  }`
+	json.Unmarshal([]byte(reqBody), route3)
+
+	err = validator.Validate(route3)
+	assert.NotNil(t, err)
+	assert.EqualError(t, err, "missing key")
+
+	//type:chash, hash_on: cookie, missing key
+	route4 := &entity.Route{}
+	reqBody = `{
+      "id": "1",
+      "methods": ["GET"],
+      "upstream": {
+          "nodes": {
+              "127.0.0.1:8080": 1
+          },
+          "type": "chash",
+          "hash_on":"cookie"
+      },
+      "desc": "new route",
+      "uri": "/index.html"
+  }`
+	json.Unmarshal([]byte(reqBody), route4)
+
+	err = validator.Validate(route4)
+	assert.NotNil(t, err)
+	assert.EqualError(t, err, "missing key")
+
+	//type:chash, hash_on: vars, wrong key
+	route5 := &entity.Route{}
+	reqBody = `{
+      "id": "1",
+      "methods": ["GET"],
+      "upstream": {
+          "nodes": {
+              "127.0.0.1:8080": 1
+          },
+          "type": "chash",
+          "hash_on":"vars",
+          "key": "not_support"
+      },
+      "desc": "new route",
+      "uri": "/index.html"
+  }`
+	json.Unmarshal([]byte(reqBody), route5)
+
+	err = validator.Validate(route5)
+	assert.NotNil(t, err)
+	assert.EqualError(t, err, "scheme 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_-]+)$'")
+
+}
diff --git a/api/internal/handler/handler.go b/api/internal/handler/handler.go
index db2c0a5..81efb66 100644
--- a/api/internal/handler/handler.go
+++ b/api/internal/handler/handler.go
@@ -34,6 +34,8 @@ func SpecCodeResponse(err error) *data.SpecCodeResponse {
 	errMsg := err.Error()
 	if strings.Contains(errMsg, "required") ||
 		strings.Contains(errMsg, "conflicted") ||
+		strings.Contains(errMsg, "invalid") ||
+		strings.Contains(errMsg, "missing") ||
 		strings.Contains(errMsg, "scheme validate fail") {
 		return &data.SpecCodeResponse{StatusCode: http.StatusBadRequest}
 	}
diff --git a/api/internal/handler/route/route_test.go b/api/internal/handler/route/route_test.go
index 9269d34..0b23a49 100644
--- a/api/internal/handler/route/route_test.go
+++ b/api/internal/handler/route/route_test.go
@@ -864,4 +864,47 @@ func TestRoute(t *testing.T) {
 	assert.NotNil(t, err)
 	assert.Equal(t, http.StatusBadRequest, ret.(*data.SpecCodeResponse).StatusCode)
 
+	//type:chash, hash_on: vars, wrong key
+	route5 := &entity.Route{}
+	reqBody = `{
+      "id": "1",
+      "methods": ["GET"],
+      "upstream": {
+          "nodes": {
+              "127.0.0.1:8080": 1
+          },
+          "type": "chash",
+          "hash_on":"vars",
+          "key": "not_support"
+      },
+      "desc": "new route",
+      "uri": "/index.html"
+  }`
+	json.Unmarshal([]byte(reqBody), route5)
+	ctx.SetInput(route5)
+	ret, err = handler.Create(ctx)
+	assert.NotNil(t, err)
+	assert.Equal(t, http.StatusBadRequest, ret.(*data.SpecCodeResponse).StatusCode)
+
+	//type:chash, hash_on: cookie, missing key
+	route6 := &entity.Route{}
+	reqBody = `{
+      "id": "1",
+      "methods": ["GET"],
+      "upstream": {
+          "nodes": {
+              "127.0.0.1:8080": 1
+          },
+          "type": "chash",
+          "hash_on":"cookie"
+      },
+      "desc": "new route",
+      "uri": "/index.html"
+  }`
+	json.Unmarshal([]byte(reqBody), route6)
+	ctx.SetInput(route6)
+	ret, err = handler.Create(ctx)
+	assert.NotNil(t, err)
+	assert.Equal(t, http.StatusBadRequest, ret.(*data.SpecCodeResponse).StatusCode)
+
 }
diff --git a/api/internal/utils/consts/api_error.go b/api/internal/utils/consts/api_error.go
index cd9bc32..99844b8 100644
--- a/api/internal/utils/consts/api_error.go
+++ b/api/internal/utils/consts/api_error.go
@@ -31,7 +31,7 @@ func ErrorWrapper(handle WrapperHandle) gin.HandlerFunc {
 			c.JSON(apiError.Status, apiError)
 			return
 		}
-		c.JSON(http.StatusOK, gin.H{"data": data, "code": 200, "message": "success"})
+		c.JSON(http.StatusOK, gin.H{"data": data, "code": 0, "message": "success"})
 	}
 }