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/11/29 15:40:09 UTC
[GitHub] [apisix-dashboard] idbeta opened a new pull request #874: test: add route with invalid remote_addr
idbeta opened a new pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874
Please answer these questions before submitting a pull request
- Why submit this pull request?
- [x] E2E Testcase
- [ ] New feature provided
- [ ] Improve performance
- Related issues
https://github.com/apache/apisix-dashboard/issues/631
___
### Bugfix
- Description
- How to fix?
___
### New feature or improvement
- Describe the details and related test reports.
----------------------------------------------------------------
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] idbeta commented on a change in pull request #874: test: add route with invalid remote_addr
Posted by GitBox <gi...@apache.org>.
idbeta commented on a change in pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#discussion_r532602261
##########
File path: api/test/e2e/route_e2e_remote_addr_test.go
##########
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+//case 1: config route with invalid remote_addr
+func TestRoute_add_with_invalid_remote_addr(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/test_uri",
+ "remote_addr": "127.0.0.",
Review comment:
I think we should not trust json schema completely, so it is reasonable to cover it in e2e.
##########
File path: api/test/e2e/route_remote_addr_test.go
##########
@@ -42,6 +42,7 @@ func TestRoute_add_with_invalid_remote_addr(t *testing.T) {
}`,
Headers: map[string]string{"Authorization": token},
ExpectStatus: http.StatusBadRequest,
+ ExpectBody: "scheme validate fail",
Review comment:
ping @moonming
----------------------------------------------------------------
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 commented on a change in pull request #874: test: add E2E testcase about add route with invalid request body
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#discussion_r530933243
##########
File path: api/test/e2e/route_remote_addr_test.go
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+func TestRoute_add_with_invalid_remote_addr(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/test_uri",
+ "remote_addr": "127.0.0.",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "172.16.238.20:1980": 1
+ }
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ },
+ {
+ caseDesc: "verify route",
+ Object: APISIXExpect(t),
+ Method: http.MethodGet,
+ Path: "/test_uri",
+ Headers: map[string]string{"Authorization": token},
Review comment:
We can remove this line
##########
File path: api/test/e2e/route_remote_addr_test.go
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+func TestRoute_add_with_invalid_remote_addr(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/test_uri",
+ "remote_addr": "127.0.0.",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "172.16.238.20:1980": 1
+ }
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ },
+ {
+ caseDesc: "verify route",
+ Object: APISIXExpect(t),
+ Method: http.MethodGet,
+ Path: "/test_uri",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusNotFound,
+ Sleep: sleepTime,
+ },
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/test_uri",
+ "remote_addr": "127.0.0.aa",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "172.16.238.20:1980": 1
+ }
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ },
+ {
+ caseDesc: "verify route",
+ Object: APISIXExpect(t),
+ Method: http.MethodGet,
+ Path: "/test_uri",
+ Headers: map[string]string{"Authorization": token},
Review comment:
ditto
----------------------------------------------------------------
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 merged pull request #874: test: add route with invalid remote_addr (E2E)
Posted by GitBox <gi...@apache.org>.
membphis merged pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874
----------------------------------------------------------------
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 commented on a change in pull request #874: test: add E2E testcase about add route with invalid request body
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#discussion_r531049077
##########
File path: api/test/e2e/route_remote_addr_test.go
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+func TestRoute_add_with_invalid_remote_addr(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/test_uri",
+ "remote_addr": "127.0.0.",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "172.16.238.20:1980": 1
+ }
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ },
+ {
+ caseDesc: "verify route",
+ Object: APISIXExpect(t),
+ Method: http.MethodGet,
+ Path: "/test_uri",
+ Headers: map[string]string{"Authorization": token},
Review comment:
sorry, I think we should keep this
----------------------------------------------------------------
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] moonming commented on a change in pull request #874: test: add E2E testcase about add route with invalid request body
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#discussion_r530709845
##########
File path: api/test/e2e/route_e2e_remote_addr_test.go
##########
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+//case 1: config route with invalid remote_addr
+func TestRoute_add_with_invalid_remote_addr(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/test_uri",
+ "remote_addr": "127.0.0.",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "172.16.238.20:1980": 1
Review comment:
we not support this format anymore
##########
File path: api/test/e2e/route_e2e_remote_addr_test.go
##########
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+//case 1: config route with invalid remote_addr
+func TestRoute_add_with_invalid_remote_addr(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/test_uri",
+ "remote_addr": "127.0.0.",
Review comment:
is this test json chema?
##########
File path: api/test/e2e/route_e2e_remote_addr_test.go
##########
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+//case 1: config route with invalid remote_addr
+func TestRoute_add_with_invalid_remote_addr(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/test_uri",
+ "remote_addr": "127.0.0.",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "172.16.238.20:1980": 1
+ }
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ },
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/test_uri",
+ "remote_addr": "127.0.0.aa",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "172.16.238.20:1980": 1
+ }
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
Review comment:
not check data plane
----------------------------------------------------------------
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] idbeta commented on a change in pull request #874: test: add E2E testcase about add route with invalid request body
Posted by GitBox <gi...@apache.org>.
idbeta commented on a change in pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#discussion_r530920622
##########
File path: api/test/e2e/route_e2e_remote_addr_test.go
##########
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+//case 1: config route with invalid remote_addr
+func TestRoute_add_with_invalid_remote_addr(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/test_uri",
+ "remote_addr": "127.0.0.",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "172.16.238.20:1980": 1
Review comment:
I already use the new format now.
----------------------------------------------------------------
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 edited a comment on pull request #874: test: add E2E testcase about add route with invalid request body
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#issuecomment-733660890
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/874?src=pr&el=h1) Report
> Merging [#874](https://codecov.io/gh/apache/apisix-dashboard/pull/874?src=pr&el=desc) (36daab8) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/0dd68b9b2261e521ad5d46fc2d0ac7623cf16d92?el=desc) (0dd68b9) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/874?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #874 +/- ##
=======================================
Coverage 43.03% 43.03%
=======================================
Files 18 18
Lines 1271 1271
=======================================
Hits 547 547
Misses 632 632
Partials 92 92
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/874?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/874?src=pr&el=footer). Last update [0dd68b9...36daab8](https://codecov.io/gh/apache/apisix-dashboard/pull/874?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
[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #874: test: add route with invalid remote_addr
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#issuecomment-733660890
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/874?src=pr&el=h1) Report
> Merging [#874](https://codecov.io/gh/apache/apisix-dashboard/pull/874?src=pr&el=desc) (dcc0c1e) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/0dd68b9b2261e521ad5d46fc2d0ac7623cf16d92?el=desc) (0dd68b9) will **decrease** coverage by `0.21%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/874?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #874 +/- ##
==========================================
- Coverage 43.03% 42.82% -0.22%
==========================================
Files 18 18
Lines 1271 1289 +18
==========================================
+ Hits 547 552 +5
- Misses 632 645 +13
Partials 92 92
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/874?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/utils/utils.go](https://codecov.io/gh/apache/apisix-dashboard/pull/874/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL3V0aWxzL3V0aWxzLmdv) | `51.61% <0.00%> (-7.65%)` | :arrow_down: |
| [api/internal/handler/service/service.go](https://codecov.io/gh/apache/apisix-dashboard/pull/874/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvc2VydmljZS9zZXJ2aWNlLmdv) | `27.65% <0.00%> (-0.61%)` | :arrow_down: |
| [api/internal/core/entity/query.go](https://codecov.io/gh/apache/apisix-dashboard/pull/874/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvZW50aXR5L3F1ZXJ5Lmdv) | `0.00% <0.00%> (ø)` | |
| [api/internal/core/entity/entity.go](https://codecov.io/gh/apache/apisix-dashboard/pull/874/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvZW50aXR5L2VudGl0eS5nbw==) | `0.00% <0.00%> (ø)` | |
| [api/internal/handler/consumer/consumer.go](https://codecov.io/gh/apache/apisix-dashboard/pull/874/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvY29uc3VtZXIvY29uc3VtZXIuZ28=) | `34.92% <0.00%> (ø)` | |
| [api/internal/handler/route/route.go](https://codecov.io/gh/apache/apisix-dashboard/pull/874/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2hhbmRsZXIvcm91dGUvcm91dGUuZ28=) | `46.56% <0.00%> (+0.11%)` | :arrow_up: |
| [api/internal/core/store/store.go](https://codecov.io/gh/apache/apisix-dashboard/pull/874/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmUuZ28=) | `79.22% <0.00%> (+0.27%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/874?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/874?src=pr&el=footer). Last update [0dd68b9...dcc0c1e](https://codecov.io/gh/apache/apisix-dashboard/pull/874?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
[GitHub] [apisix-dashboard] idbeta commented on a change in pull request #874: test: add route with invalid remote_addr
Posted by GitBox <gi...@apache.org>.
idbeta commented on a change in pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#discussion_r533027857
##########
File path: api/test/e2e/route_remote_addr_test.go
##########
@@ -0,0 +1,121 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+func TestRoute_add_with_invalid_remote_addr(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello",
+ "remote_addr": "127.0.0.",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }]
+ }
Review comment:
fixed
##########
File path: api/test/e2e/route_remote_addr_test.go
##########
@@ -0,0 +1,121 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+func TestRoute_add_with_invalid_remote_addr(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello",
+ "remote_addr": "127.0.0.",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }]
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ ExpectBody: "remote_addr: Does not match pattern",
+ },
+ {
+ caseDesc: "verify route",
+ Object: APISIXExpect(t),
+ Method: http.MethodGet,
+ Path: "/hello",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusNotFound,
+ Sleep: sleepTime,
+ },
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello",
+ "remote_addr": "127.0.0.aa",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }]
+ }
Review comment:
fixed
##########
File path: api/test/e2e/route_remote_addr_test.go
##########
@@ -0,0 +1,121 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+func TestRoute_add_with_invalid_remote_addr(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello",
+ "remote_addr": "127.0.0.",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }]
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ ExpectBody: "remote_addr: Does not match pattern",
+ },
+ {
+ caseDesc: "verify route",
+ Object: APISIXExpect(t),
+ Method: http.MethodGet,
+ Path: "/hello",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusNotFound,
+ Sleep: sleepTime,
+ },
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello",
+ "remote_addr": "127.0.0.aa",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }]
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ ExpectBody: "remote_addr: Does not match pattern",
+ },
+ {
+ caseDesc: "verify route",
+ Object: APISIXExpect(t),
+ Method: http.MethodGet,
+ Path: "/hello",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusNotFound,
+ Sleep: sleepTime,
+ },
+ {
+ caseDesc: "config route with invalid remote_addrs",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello",
+ "remote_addrs": ["127.0.0.1","192.168.0."],
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }]
+ }
+ }`,
Review comment:
fixed
##########
File path: api/test/e2e/route_remote_addr_test.go
##########
@@ -0,0 +1,121 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+func TestRoute_add_with_invalid_remote_addr(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello",
+ "remote_addr": "127.0.0.",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }]
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ ExpectBody: "remote_addr: Does not match pattern",
Review comment:
fixed
----------------------------------------------------------------
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] idbeta edited a comment on pull request #874: test: add route with invalid remote_addr
Posted by GitBox <gi...@apache.org>.
idbeta edited a comment on pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#issuecomment-735412898
> > ping @moonming
>
> As I said days before, these checks should be the work of json chema.
> Is there a problem with the json schema that can't be verified?
I think we should not trust json schema completely, so it is reasonable to cover it in e2e.
----------------------------------------------------------------
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 commented on pull request #874: test: add E2E testcase about add route with invalid request body
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#issuecomment-734516024
> the PR title should have a better one.
agree, why do you submit two PRs about `route + remote_addr`?
----------------------------------------------------------------
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] moonming removed a comment on pull request #874: test: add route with invalid remote_addr
Posted by GitBox <gi...@apache.org>.
moonming removed a comment on pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#issuecomment-735532579
no resposne from @idbeta
----------------------------------------------------------------
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 commented on pull request #874: test: add route with invalid remote_addr
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#issuecomment-735532368
> I think we should not trust json schema completely, so it is reasonable to cover it in e2e.
@moonming do you have time to review this PR again? PR was blocked now
----------------------------------------------------------------
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 #874: test: add E2E testcase about #631
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#issuecomment-733660890
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/874?src=pr&el=h1) Report
> Merging [#874](https://codecov.io/gh/apache/apisix-dashboard/pull/874?src=pr&el=desc) (6ff4ab7) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/0dd68b9b2261e521ad5d46fc2d0ac7623cf16d92?el=desc) (0dd68b9) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/874?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #874 +/- ##
=======================================
Coverage 43.03% 43.03%
=======================================
Files 18 18
Lines 1271 1271
=======================================
Hits 547 547
Misses 632 632
Partials 92 92
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/874?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/874?src=pr&el=footer). Last update [0dd68b9...6ff4ab7](https://codecov.io/gh/apache/apisix-dashboard/pull/874?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
[GitHub] [apisix-dashboard] idbeta commented on a change in pull request #874: test: add E2E testcase about add route with invalid request body
Posted by GitBox <gi...@apache.org>.
idbeta commented on a change in pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#discussion_r530729252
##########
File path: api/test/e2e/route_e2e_remote_addr_test.go
##########
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+//case 1: config route with invalid remote_addr
+func TestRoute_add_with_invalid_remote_addr(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/test_uri",
+ "remote_addr": "127.0.0.",
Review comment:
as caseDesc said, this is test for `add route with invalid remote_addr`
----------------------------------------------------------------
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] idbeta edited a comment on pull request #874: test: add route with invalid remote_addr
Posted by GitBox <gi...@apache.org>.
idbeta edited a comment on pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#issuecomment-735412898
> > ping @moonming
>
> As I said days before, these checks should be the work of json chema.
> Is there a problem with the json schema that can't be verified?
I think don’t trust json schema completely, so it is reasonable to cover it in e2e.
----------------------------------------------------------------
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 commented on pull request #874: test: add E2E testcase for #631
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#issuecomment-733759477
bad title, I can not know what are you doing from title and description
----------------------------------------------------------------
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] moonming commented on a change in pull request #874: test: add route with invalid remote_addr
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#discussion_r532606587
##########
File path: api/test/e2e/route_remote_addr_test.go
##########
@@ -0,0 +1,121 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+func TestRoute_add_with_invalid_remote_addr(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello",
+ "remote_addr": "127.0.0.",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }]
+ }
Review comment:
indentation
##########
File path: api/test/e2e/route_remote_addr_test.go
##########
@@ -0,0 +1,121 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+func TestRoute_add_with_invalid_remote_addr(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello",
+ "remote_addr": "127.0.0.",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }]
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ ExpectBody: "remote_addr: Does not match pattern",
+ },
+ {
+ caseDesc: "verify route",
+ Object: APISIXExpect(t),
+ Method: http.MethodGet,
+ Path: "/hello",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusNotFound,
+ Sleep: sleepTime,
+ },
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello",
+ "remote_addr": "127.0.0.aa",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }]
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ ExpectBody: "remote_addr: Does not match pattern",
+ },
+ {
+ caseDesc: "verify route",
+ Object: APISIXExpect(t),
+ Method: http.MethodGet,
+ Path: "/hello",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusNotFound,
+ Sleep: sleepTime,
+ },
+ {
+ caseDesc: "config route with invalid remote_addrs",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello",
+ "remote_addrs": ["127.0.0.1","192.168.0."],
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }]
+ }
+ }`,
Review comment:
ditto
##########
File path: api/test/e2e/route_remote_addr_test.go
##########
@@ -0,0 +1,121 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+func TestRoute_add_with_invalid_remote_addr(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello",
+ "remote_addr": "127.0.0.",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }]
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ ExpectBody: "remote_addr: Does not match pattern",
Review comment:
we need to check the complete body, not part of it
##########
File path: api/test/e2e/route_remote_addr_test.go
##########
@@ -0,0 +1,121 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+func TestRoute_add_with_invalid_remote_addr(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello",
+ "remote_addr": "127.0.0.",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }]
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ ExpectBody: "remote_addr: Does not match pattern",
+ },
+ {
+ caseDesc: "verify route",
+ Object: APISIXExpect(t),
+ Method: http.MethodGet,
+ Path: "/hello",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusNotFound,
+ Sleep: sleepTime,
+ },
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello",
+ "remote_addr": "127.0.0.aa",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }]
+ }
Review comment:
ditto
----------------------------------------------------------------
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 commented on pull request #874: test: add E2E testcase about add route with invalid request body
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#issuecomment-734333308
@moonming take a look when you have time
----------------------------------------------------------------
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 pull request #874: test: add route with invalid remote_addr
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#issuecomment-735320051
ping @moonming
----------------------------------------------------------------
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 edited a comment on pull request #874: test: add route with invalid remote_addr (E2E)
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#issuecomment-733660890
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/874?src=pr&el=h1) Report
> Merging [#874](https://codecov.io/gh/apache/apisix-dashboard/pull/874?src=pr&el=desc) (5f2aa34) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/96e3ad1ec87f35d52d75f0a560dc152843e21e23?el=desc) (96e3ad1) will **decrease** coverage by `0.07%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/874?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #874 +/- ##
==========================================
- Coverage 42.82% 42.74% -0.08%
==========================================
Files 18 18
Lines 1289 1289
==========================================
- Hits 552 551 -1
- Misses 645 646 +1
Partials 92 92
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/874?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/core/store/store.go](https://codecov.io/gh/apache/apisix-dashboard/pull/874/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmUuZ28=) | `78.57% <0.00%> (-0.65%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/874?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/874?src=pr&el=footer). Last update [96e3ad1...5f2aa34](https://codecov.io/gh/apache/apisix-dashboard/pull/874?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
[GitHub] [apisix-dashboard] idbeta commented on a change in pull request #874: test: add route with invalid remote_addr
Posted by GitBox <gi...@apache.org>.
idbeta commented on a change in pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#discussion_r533054080
##########
File path: api/test/e2e/route_remote_addr_test.go
##########
@@ -0,0 +1,121 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+func TestRoute_add_with_invalid_remote_addr(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello",
+ "remote_addr": "127.0.0.",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }]
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ ExpectBody: "remote_addr: Does not match pattern",
Review comment:
ping @moonming Can you review it when you have time?
----------------------------------------------------------------
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 edited a comment on pull request #874: test: add route with invalid remote_addr
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#issuecomment-733660890
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/874?src=pr&el=h1) Report
> Merging [#874](https://codecov.io/gh/apache/apisix-dashboard/pull/874?src=pr&el=desc) (397a070) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/0dd68b9b2261e521ad5d46fc2d0ac7623cf16d92?el=desc) (0dd68b9) will **decrease** coverage by `0.07%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/874?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #874 +/- ##
==========================================
- Coverage 43.03% 42.95% -0.08%
==========================================
Files 18 18
Lines 1271 1271
==========================================
- Hits 547 546 -1
- Misses 632 633 +1
Partials 92 92
```
| [Impacted Files](https://codecov.io/gh/apache/apisix-dashboard/pull/874?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [api/internal/core/store/store.go](https://codecov.io/gh/apache/apisix-dashboard/pull/874/diff?src=pr&el=tree#diff-YXBpL2ludGVybmFsL2NvcmUvc3RvcmUvc3RvcmUuZ28=) | `78.28% <0.00%> (-0.66%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/874?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/874?src=pr&el=footer). Last update [0dd68b9...397a070](https://codecov.io/gh/apache/apisix-dashboard/pull/874?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
[GitHub] [apisix-dashboard] moonming commented on a change in pull request #874: test: add route with invalid remote_addr
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#discussion_r532337892
##########
File path: api/test/e2e/route_remote_addr_test.go
##########
@@ -0,0 +1,118 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+func TestRoute_add_with_invalid_remote_addr(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello",
+ "remote_addr": "127.0.0.",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }]
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ },
+ {
+ caseDesc: "verify route",
+ Object: APISIXExpect(t),
+ Method: http.MethodGet,
+ Path: "/hello",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusNotFound,
+ Sleep: sleepTime,
+ },
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello",
+ "remote_addr": "127.0.0.aa",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }]
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
Review comment:
we need to check the response body.
----------------------------------------------------------------
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] moonming commented on a change in pull request #874: test: add route with invalid remote_addr
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#discussion_r532155367
##########
File path: api/test/e2e/route_e2e_remote_addr_test.go
##########
@@ -0,0 +1,86 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+//case 1: config route with invalid remote_addr
+func TestRoute_add_with_invalid_remote_addr(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/test_uri",
+ "remote_addr": "127.0.0.",
Review comment:
Isn't this the job of json chema? why should e2e to cover?
----------------------------------------------------------------
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] idbeta commented on a change in pull request #874: test: add route with invalid remote_addr
Posted by GitBox <gi...@apache.org>.
idbeta commented on a change in pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#discussion_r532383816
##########
File path: api/test/e2e/route_remote_addr_test.go
##########
@@ -42,6 +42,7 @@ func TestRoute_add_with_invalid_remote_addr(t *testing.T) {
}`,
Headers: map[string]string{"Authorization": token},
ExpectStatus: http.StatusBadRequest,
+ ExpectBody: "scheme validate fail",
Review comment:
fixed.
----------------------------------------------------------------
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] idbeta commented on pull request #874: test: add route with invalid remote_addr
Posted by GitBox <gi...@apache.org>.
idbeta commented on pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#issuecomment-735412898
> > ping @moonming
>
> As I said days before, these checks should be the work of json chema.
> Is there a problem with the json schema that can't be verified?
----------------------------------------------------------------
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 commented on a change in pull request #874: test: add E2E testcase about add route with invalid request body
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#discussion_r530745302
##########
File path: api/test/e2e/route_e2e_remote_addr_test.go
##########
@@ -0,0 +1,118 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+//case 1: config route with invalid remote_addr
+func TestRoute_add_with_invalid_remote_addr(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/test_uri",
+ "remote_addr": "127.0.0.",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "172.16.238.20:1980": 1
+ }
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ },
+ {
+ caseDesc: "verify route",
+ Object: APISIXExpect(t),
+ Method: http.MethodGet,
+ Path: "/test_uri",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusNotFound,
+ Sleep: sleepTime,
+ },
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/test_uri",
+ "remote_addr": "127.0.0.aa",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "172.16.238.20:1980": 1
+ }
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ },
+ {
+ caseDesc: "verify route",
+ Object: APISIXExpect(t),
+ Method: http.MethodGet,
+ Path: "/test_uri",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusNotFound,
+ Sleep: sleepTime,
+ },
+ {
+ caseDesc: "config route with invalid remote_addrs",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/test_uri",
+ "remote_addrs": ["127.0.0.1","192.168.0."],
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "172.16.238.20:1980": 1
+ }
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ },
+ {
+ caseDesc: "verify route",
+ Object: APISIXExpect(t),
+ Method: http.MethodGet,
+ Path: "/test_uri",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusNotFound,
+ Sleep: sleepTime,
+ },
+
+ // TODO it's a bug here, see: https://github.com/apache/apisix-dashboard/issues/872
Review comment:
we can remove line 109 and 111 too, we have the issues
##########
File path: api/test/e2e/route_e2e_remote_addr_test.go
##########
@@ -0,0 +1,118 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+//case 1: config route with invalid remote_addr
Review comment:
I think this comment is useless, pls remove it
----------------------------------------------------------------
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] idbeta commented on a change in pull request #874: test: add route with invalid remote_addr
Posted by GitBox <gi...@apache.org>.
idbeta commented on a change in pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#discussion_r532340419
##########
File path: api/test/e2e/route_remote_addr_test.go
##########
@@ -0,0 +1,118 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+func TestRoute_add_with_invalid_remote_addr(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello",
+ "remote_addr": "127.0.0.",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }]
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ },
+ {
+ caseDesc: "verify route",
+ Object: APISIXExpect(t),
+ Method: http.MethodGet,
+ Path: "/hello",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusNotFound,
+ Sleep: sleepTime,
+ },
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello",
+ "remote_addr": "127.0.0.aa",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }]
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
Review comment:
fixed.
----------------------------------------------------------------
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] moonming commented on a change in pull request #874: test: add route with invalid remote_addr
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#discussion_r532338155
##########
File path: api/test/e2e/route_remote_addr_test.go
##########
@@ -0,0 +1,118 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+func TestRoute_add_with_invalid_remote_addr(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello",
+ "remote_addr": "127.0.0.",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }]
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ },
+ {
+ caseDesc: "verify route",
+ Object: APISIXExpect(t),
+ Method: http.MethodGet,
+ Path: "/hello",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusNotFound,
+ Sleep: sleepTime,
+ },
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/hello",
+ "remote_addr": "127.0.0.aa",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": [{
+ "host": "172.16.238.20",
+ "port": 1980,
+ "weight": 1
+ }]
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
Review comment:
Otherwise, there is no way to determine whether the json schema really works
----------------------------------------------------------------
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] nic-chen commented on pull request #874: test: add E2E testcase about add route with invalid request body
Posted by GitBox <gi...@apache.org>.
nic-chen commented on pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#issuecomment-734357333
need test cases for correct remote addr
----------------------------------------------------------------
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] nic-chen commented on pull request #874: test: add E2E testcase about add route with invalid request body
Posted by GitBox <gi...@apache.org>.
nic-chen commented on pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#issuecomment-734358219
the PR title should have a better one.
----------------------------------------------------------------
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] moonming commented on a change in pull request #874: test: add route with invalid remote_addr
Posted by GitBox <gi...@apache.org>.
moonming commented on a change in pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#discussion_r532359862
##########
File path: api/test/e2e/route_remote_addr_test.go
##########
@@ -42,6 +42,7 @@ func TestRoute_add_with_invalid_remote_addr(t *testing.T) {
}`,
Headers: map[string]string{"Authorization": token},
ExpectStatus: http.StatusBadRequest,
+ ExpectBody: "scheme validate fail",
Review comment:
`scheme` -> schema, `fail` -> `failed`
Is there any specific failed reason?
----------------------------------------------------------------
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 commented on a change in pull request #874: test: add E2E testcase for #631
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#discussion_r530444072
##########
File path: api/test/e2e/route_e2e_remote_addr_test.go
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+ // "time"
Review comment:
remove useless code, keep code tidy
##########
File path: api/test/e2e/route_e2e_remote_addr_test.go
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+ // "time"
+ // "github.com/stretchr/testify/assert"
+ // "github.com/tidwall/gjson"
+)
+
+//case 1: config route with invalid remote_addr
+func TestRoute_add_with_invalid_remote_addr(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/test_uri",
+ "remote_addr": "127.0.0.",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "172.16.238.20:1980": 1
+ }
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ },
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/test_uri",
+ "remote_addr": "127.0.0.aa",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "172.16.238.20:1980": 1
+ }
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ },
+ {
+ caseDesc: "config route with invalid remote_addrs",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/test_uri",
+ "remote_addrs": ["127.0.0.1","192.168.0."],
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "172.16.238.20:1980": 1
+ }
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ },
+ }
+
+ for _, tc := range tests {
+ testCaseCheck(tc)
+ }
+}
+
+//Teardown
+func TestConsumer_teardown(t *testing.T) {
+ _ = []HttpTestCase{
+ {
+ caseDesc: "delete route",
+ Object: MangerApiExpect(t),
+ Method: http.MethodDelete,
+ Path: "/apisix/admin/routes/r1",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusOK,
Review comment:
should 404 here
----------------------------------------------------------------
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] moonming commented on pull request #874: test: add route with invalid remote_addr
Posted by GitBox <gi...@apache.org>.
moonming commented on pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#issuecomment-735532579
no resposne from @idbeta
----------------------------------------------------------------
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 commented on a change in pull request #874: test: add E2E testcase about add route with invalid request body
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#discussion_r530745302
##########
File path: api/test/e2e/route_e2e_remote_addr_test.go
##########
@@ -0,0 +1,118 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package e2e
+
+import (
+ "net/http"
+ "testing"
+)
+
+//case 1: config route with invalid remote_addr
+func TestRoute_add_with_invalid_remote_addr(t *testing.T) {
+ tests := []HttpTestCase{
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/test_uri",
+ "remote_addr": "127.0.0.",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "172.16.238.20:1980": 1
+ }
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ },
+ {
+ caseDesc: "verify route",
+ Object: APISIXExpect(t),
+ Method: http.MethodGet,
+ Path: "/test_uri",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusNotFound,
+ Sleep: sleepTime,
+ },
+ {
+ caseDesc: "config route with invalid remote_addr",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/test_uri",
+ "remote_addr": "127.0.0.aa",
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "172.16.238.20:1980": 1
+ }
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ },
+ {
+ caseDesc: "verify route",
+ Object: APISIXExpect(t),
+ Method: http.MethodGet,
+ Path: "/test_uri",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusNotFound,
+ Sleep: sleepTime,
+ },
+ {
+ caseDesc: "config route with invalid remote_addrs",
+ Object: MangerApiExpect(t),
+ Method: http.MethodPut,
+ Path: "/apisix/admin/routes/r1",
+ Body: `{
+ "uri": "/test_uri",
+ "remote_addrs": ["127.0.0.1","192.168.0."],
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "172.16.238.20:1980": 1
+ }
+ }
+ }`,
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusBadRequest,
+ },
+ {
+ caseDesc: "verify route",
+ Object: APISIXExpect(t),
+ Method: http.MethodGet,
+ Path: "/test_uri",
+ Headers: map[string]string{"Authorization": token},
+ ExpectStatus: http.StatusNotFound,
+ Sleep: sleepTime,
+ },
+
+ // TODO it's a bug here, see: https://github.com/apache/apisix-dashboard/issues/872
Review comment:
we can remove line 109 and 111 too, we have the issues already
----------------------------------------------------------------
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] idbeta commented on pull request #874: test: add E2E testcase about add route with invalid request body
Posted by GitBox <gi...@apache.org>.
idbeta commented on pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#issuecomment-734383470
> need test cases for correct remote addr
yes, you can check this PR https://github.com/apache/apisix-dashboard/pull/884
----------------------------------------------------------------
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] moonming commented on pull request #874: test: add route with invalid remote_addr
Posted by GitBox <gi...@apache.org>.
moonming commented on pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#issuecomment-735343347
> ping @moonming
As I said days before, these checks should be the work of json chema.
Is there a problem with the json schema that can't be verified?
----------------------------------------------------------------
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] idbeta closed pull request #874: test: add route with invalid remote_addr
Posted by GitBox <gi...@apache.org>.
idbeta closed pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874
----------------------------------------------------------------
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 edited a comment on pull request #874: test: add E2E testcase about add route with invalid request body
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#issuecomment-733660890
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/874?src=pr&el=h1) Report
> Merging [#874](https://codecov.io/gh/apache/apisix-dashboard/pull/874?src=pr&el=desc) (1c8ef35) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/0dd68b9b2261e521ad5d46fc2d0ac7623cf16d92?el=desc) (0dd68b9) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/874?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #874 +/- ##
=======================================
Coverage 43.03% 43.03%
=======================================
Files 18 18
Lines 1271 1271
=======================================
Hits 547 547
Misses 632 632
Partials 92 92
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/874?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/874?src=pr&el=footer). Last update [0dd68b9...1c8ef35](https://codecov.io/gh/apache/apisix-dashboard/pull/874?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
[GitHub] [apisix-dashboard] codecov-io edited a comment on pull request #874: test: add E2E testcase about add route with invalid request body
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#issuecomment-733660890
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/874?src=pr&el=h1) Report
> Merging [#874](https://codecov.io/gh/apache/apisix-dashboard/pull/874?src=pr&el=desc) (1c16183) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/0dd68b9b2261e521ad5d46fc2d0ac7623cf16d92?el=desc) (0dd68b9) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/874?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #874 +/- ##
=======================================
Coverage 43.03% 43.03%
=======================================
Files 18 18
Lines 1271 1271
=======================================
Hits 547 547
Misses 632 632
Partials 92 92
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/874?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/874?src=pr&el=footer). Last update [0dd68b9...1c16183](https://codecov.io/gh/apache/apisix-dashboard/pull/874?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
[GitHub] [apisix-dashboard] idbeta commented on a change in pull request #874: test: add route with invalid remote_addr
Posted by GitBox <gi...@apache.org>.
idbeta commented on a change in pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#discussion_r532399295
##########
File path: api/test/e2e/route_remote_addr_test.go
##########
@@ -42,6 +42,7 @@ func TestRoute_add_with_invalid_remote_addr(t *testing.T) {
}`,
Headers: map[string]string{"Authorization": token},
ExpectStatus: http.StatusBadRequest,
+ ExpectBody: "scheme validate fail",
Review comment:
I have raised the issue of typo https://github.com/apache/apisix-dashboard/issues/914
----------------------------------------------------------------
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 edited a comment on pull request #874: test: add E2E testcase about add route with invalid request body
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #874:
URL: https://github.com/apache/apisix-dashboard/pull/874#issuecomment-733660890
# [Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/874?src=pr&el=h1) Report
> Merging [#874](https://codecov.io/gh/apache/apisix-dashboard/pull/874?src=pr&el=desc) (5ec5d97) into [master](https://codecov.io/gh/apache/apisix-dashboard/commit/0dd68b9b2261e521ad5d46fc2d0ac7623cf16d92?el=desc) (0dd68b9) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/apisix-dashboard/pull/874?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #874 +/- ##
=======================================
Coverage 43.03% 43.03%
=======================================
Files 18 18
Lines 1271 1271
=======================================
Hits 547 547
Misses 632 632
Partials 92 92
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/apisix-dashboard/pull/874?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/874?src=pr&el=footer). Last update [0dd68b9...5ec5d97](https://codecov.io/gh/apache/apisix-dashboard/pull/874?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