You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/12/02 03:47:50 UTC

[GitHub] [apisix-dashboard] nic-chen opened a new pull request #946: fix: sync json schema to fix the checking of `remote_addrs`

nic-chen opened a new pull request #946:
URL: https://github.com/apache/apisix-dashboard/pull/946


   Please answer these questions before submitting a pull request
   
   - Why submit this pull request?
   - [x] Bugfix
   - [ ] New feature provided
   - [ ] Improve performance
   
   - Related issues
   
   related #940 
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [apisix-dashboard] juzhiyuan commented on a change in pull request #946: fix: sync json schema to fix the checking of `remote_addrs`

Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #946:
URL: https://github.com/apache/apisix-dashboard/pull/946#discussion_r533882192



##########
File path: api/internal/core/store/validate_test.go
##########
@@ -247,5 +247,147 @@ func TestAPISIXJsonSchemaValidator_checkUpstream(t *testing.T) {
 	err = validator.Validate(route5)
 	assert.NotNil(t, err)
 	assert.EqualError(t, err, "schema 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_-]+)$'")
-
 }
+
+
+func TestAPISIXJsonSchemaValidator_Route_checkRemoteAddr(t *testing.T) {
+
+	tests := []struct {
+		caseDesc  string
+		giveContent     string
+		wantNewErr      error
+		wantValidateErr []error
+	}{
+		{
+			caseDesc: "correct remote_addr",
+			giveContent: `{
+				"id": "1",
+				"uri": "/*",
+				"upstream": {
+					"nodes": [{
+						"host": "127.0.0.1",
+						"port": 8080,
+						"weight": 1
+					}],
+					"type": "roundrobin"
+				},
+				"remote_addr": "127.0.0.1"
+			}`,
+		},
+		{
+			caseDesc: "correct remote_addr (CIDR)",
+			giveContent: `{
+				"id": "1",
+				"uri": "/*",
+				"upstream": {
+					"nodes": [{
+						"host": "127.0.0.1",
+						"port": 8080,
+						"weight": 1
+					}],
+					"type": "roundrobin"
+				},
+				"remote_addr": "192.168.1.0/24"
+			}`,
+		},
+		{
+			caseDesc: "invalid remote_addr",
+			giveContent: `{
+				"id": "1",
+				"uri": "/*",
+				"upstream": {
+					"nodes": [{
+						"host": "127.0.0.1",
+						"port": 8080,
+						"weight": 1
+					}],
+					"type": "roundrobin"
+				},
+				"remote_addr": "127.0.0."
+			}`,
+			wantValidateErr: []error{
+				fmt.Errorf("schema validate failed: remote_addr: Must validate at least one schema (anyOf)\nremote_addr: Does not match format 'ipv4'"),
+			},
+		},
+		{
+			caseDesc: "correct remote_addrs",
+			giveContent: `{
+				"id": "1",
+				"uri": "/*",
+				"upstream": {
+					"nodes": [{
+						"host": "127.0.0.1",
+						"port": 8080,
+						"weight": 1
+					}],
+					"type": "roundrobin"
+				},
+				"remote_addrs": ["127.0.0.1", "192.0.0.0/8", "::1", "fe80::1/64"]
+			}`,
+		},
+		{
+			caseDesc: "invalid remote_addrs",
+			giveContent: `{
+				"id": "1",
+				"uri": "/*",
+				"upstream": {
+					"nodes": [{
+						"host": "127.0.0.1",
+						"port": 8080,
+						"weight": 1
+					}],
+					"type": "roundrobin"
+				},
+				"remote_addrs": ["127.0.0.", "192.0.0.0/128", "::1"]
+			}`,
+			wantValidateErr: []error{
+				fmt.Errorf("schema validate failed: remote_addrs.0: Must validate at least one schema (anyOf)\nremote_addrs.0: Does not match format 'ipv4'\nremote_addrs.1: Must validate at least one schema (anyOf)\nremote_addrs.1: Does not match format 'ipv4'"),
+			},
+		},
+		{
+			caseDesc: "invalid remote_addrs (an empty string item)",
+			giveContent: `{
+				"id": "1",
+				"uri": "/*",
+				"upstream": {
+					"nodes": [{
+						"host": "127.0.0.1",
+						"port": 8080,
+						"weight": 1
+					}],
+					"type": "roundrobin"
+				},
+				"remote_addrs": [""]
+			}`,
+			wantValidateErr: []error{
+				fmt.Errorf("schema validate failed: remote_addrs.0: Must validate at least one schema (anyOf)\nremote_addrs.0: Does not match format 'ipv4'"),
+			},
+		},
+	}
+
+	for _, tc := range tests {
+		validator, err := NewAPISIXJsonSchemaValidator("main.route")
+		if err != nil {
+			assert.Equal(t, tc.wantNewErr, err, tc.caseDesc)
+			continue
+		}
+		route := &entity.Route{}
+		err = json.Unmarshal([]byte(tc.giveContent), route)
+		assert.Nil(t, err, tc.caseDesc)
+
+		err = validator.Validate(route)
+		if tc.wantValidateErr == nil {
+			assert.Equal(t, nil, err, tc.caseDesc)
+			continue
+		}
+
+		ret := false
+		for _, wantErr := range tc.wantValidateErr {
+			if wantErr.Error() == err.Error() {
+				ret = true
+			}
+		}
+		assert.True(t, ret, tc.caseDesc)
+	}
+
+}

Review comment:
       ,




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [apisix-dashboard] membphis closed pull request #946: fix: sync json schema to fix the checking of `remote_addrs`

Posted by GitBox <gi...@apache.org>.
membphis closed pull request #946:
URL: https://github.com/apache/apisix-dashboard/pull/946


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [apisix-dashboard] juzhiyuan commented on a change in pull request #946: fix: sync json schema to fix the checking of `remote_addrs`

Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #946:
URL: https://github.com/apache/apisix-dashboard/pull/946#discussion_r533882306



##########
File path: api/internal/core/store/validate_test.go
##########
@@ -121,17 +121,17 @@ func TestAPISIXJsonSchemaValidator_Validate(t *testing.T) {
 	//plugin schema fail
 	consumer3 := &entity.Consumer{}
 	reqBody = `{
-      "id": "jack",
-      "username": "jack",
-      "plugins": {
-          "limit-count": {
-              "time_window": 60,
-              "rejected_code": 503,
-              "key": "remote_addr"
-          }
-      },
-    "desc": "test description"
-  }`
+		"id": "jack",

Review comment:
       ```suggestion
   usually use 2 spaces for json
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [apisix-dashboard] codecov-io commented on pull request #946: fix: sync json schema to fix the checking of `remote_addrs`

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #946:
URL: https://github.com/apache/apisix-dashboard/pull/946#issuecomment-736983170


   # [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/946?src=pr&el=h1) Report
   > Merging [#946](https://codecov.io/gh/apache/apisix-dashboard/pull/946?src=pr&el=desc) (3c8747c) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/47d67d61ef98b5ce8ae482dc0bf135f015e09a4f?el=desc) (47d67d6) will **increase** coverage by `0.15%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/apisix-dashboard/pull/946/graphs/tree.svg?width=650&height=150&src=pr&token=Q1HERXN96P)](https://codecov.io/gh/apache/apisix-dashboard/pull/946?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master     #946      +/-   ##
   ==========================================
   + Coverage   43.02%   43.17%   +0.15%     
   ==========================================
     Files          18       18              
     Lines        1290     1290              
   ==========================================
   + Hits          555      557       +2     
   + Misses        643      642       -1     
   + Partials       92       91       -1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/946?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [api/internal/core/store/validate.go](https://codecov.io/gh/apache/apisix-dashboard/pull/946/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvdmFsaWRhdGUuZ28=) | `56.52% <0.00%> (+1.44%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/946?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/946?src=pr&el=footer). Last update [47d67d6...3c8747c](https://codecov.io/gh/apache/apisix-dashboard/pull/946?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org