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 2021/05/26 11:10:33 UTC
[GitHub] [apisix] Demogorgon314 opened a new pull request #4313: feat: nacos discovery support namespace
Demogorgon314 opened a new pull request #4313:
URL: https://github.com/apache/apisix/pull/4313
### What this PR does / why we need it:
#4306
### Pre-submission checklist:
* [x] Did you explain what problem does this PR solve? Or what new features have been added?
* [x] Have you added corresponding test cases?
* [x] Have you modified the corresponding document?
* [x] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) first**
--
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] spacewander merged pull request #4313: feat: nacos discovery support namespace
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #4313:
URL: https://github.com/apache/apisix/pull/4313
--
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] Firstsawyou commented on a change in pull request #4313: feat: nacos discovery support namespace
Posted by GitBox <gi...@apache.org>.
Firstsawyou commented on a change in pull request #4313:
URL: https://github.com/apache/apisix/pull/4313#discussion_r639826338
##########
File path: t/discovery/nacos.t
##########
@@ -251,3 +251,134 @@ discovery:
]
--- no_error_log
[error]
+
+
+
+=== TEST 8: get APISIX-NACOS info from NACOS - no auth with namespace
+--- yaml_config eval: $::yaml_config
+--- apisix_yaml
+routes:
+ -
+ uri: /hello
+ upstream:
+ service_name: APISIX-NACOS
+ discovery_type: nacos
+ type: roundrobin
+ nacos_namespace_id: test_ns
+#END
+--- pipelined_requests eval
+[
+ "GET /hello",
+ "GET /hello",
+]
+--- response_body_like eval
+[
+ qr/server [1-2]/,
+ qr/server [1-2]/,
+]
+--- no_error_log
+[error, error]
Review comment:
```suggestion
[error]
```
##########
File path: apisix/discovery/nacos.lua
##########
@@ -254,10 +264,10 @@ local function fetch_full_registry(premature)
applications = up_apps
return
end
-
local data, err
- for _, service_name in ipairs(infos) do
- data, err = get_url(base_uri, instance_list_path .. service_name .. token_param)
+ for _, service_info in ipairs(infos) do
+ local namespace_param = get_namespace_param(service_info.nacos_namespace_id);
+ data, err = get_url(base_uri, instance_list_path .. service_info.service_name .. token_param .. namespace_param)
Review comment:
```suggestion
data, err = get_url(base_uri, instance_list_path .. service_info.service_name ..
token_param .. namespace_param)
```
##########
File path: t/discovery/nacos.t
##########
@@ -251,3 +251,134 @@ discovery:
]
--- no_error_log
[error]
+
+
+
+=== TEST 8: get APISIX-NACOS info from NACOS - no auth with namespace
+--- yaml_config eval: $::yaml_config
+--- apisix_yaml
+routes:
+ -
+ uri: /hello
+ upstream:
+ service_name: APISIX-NACOS
+ discovery_type: nacos
+ type: roundrobin
+ nacos_namespace_id: test_ns
+#END
+--- pipelined_requests eval
+[
+ "GET /hello",
+ "GET /hello",
+]
+--- response_body_like eval
+[
+ qr/server [1-2]/,
+ qr/server [1-2]/,
+]
+--- no_error_log
+[error, error]
+
+
+
+=== TEST 9: get APISIX-NACOS info from NACOS - configured in services with namespace
+--- yaml_config eval: $::yaml_config
+--- apisix_yaml
+routes:
+ -
+ uri: /hello
+ service_id: 1
+services:
+ -
+ id: 1
+ upstream:
+ service_name: APISIX-NACOS
+ discovery_type: nacos
+ type: roundrobin
+ nacos_namespace_id: test_ns
+#END
+--- pipelined_requests eval
+[
+ "GET /hello",
+ "GET /hello",
+]
+--- response_body_like eval
+[
+ qr/server [1-2]/,
+ qr/server [1-2]/,
+]
+--- no_error_log
+[error]
+
+
+
+=== TEST 10: get APISIX-NACOS info from NACOS - configured in upstreams + etcd with namespace
+--- extra_yaml_config
+discovery:
+ nacos:
+ host:
+ - "http://127.0.0.1:8858"
+ fetch_interval: 1
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/upstreams/1',
+ ngx.HTTP_PUT,
+ [[{
+ "service_name": "APISIX-NACOS",
+ "discovery_type": "nacos",
+ "type": "roundrobin",
+ "nacos_namespace_id": "test_ns"
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ ngx.say(body)
+ return
+ end
+
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/hello",
+ "upstream_id": 1
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ end
+
+ ngx.say(body)
+ }
+ }
+--- request
+GET /t
+--- response_body
+passed
+--- no_error_log
+[error]
+
+
+
+=== TEST 11: hit with namespace
+--- extra_yaml_config
+discovery:
+ nacos:
+ host:
+ - "http://127.0.0.1:8858"
+ fetch_interval: 1
+--- pipelined_requests eval
+[
+ "GET /hello",
+ "GET /hello",
+]
+--- response_body_like eval
+[
+ qr/server [1-2]/,
+ qr/server [1-2]/,
+]
+--- no_error_log
+[error]
Review comment:
A blank line is required at the end of the file.
--
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] Demogorgon314 commented on a change in pull request #4313: feat: nacos discovery support namespace
Posted by GitBox <gi...@apache.org>.
Demogorgon314 commented on a change in pull request #4313:
URL: https://github.com/apache/apisix/pull/4313#discussion_r640304176
##########
File path: apisix/schema_def.lua
##########
@@ -389,6 +389,10 @@ local upstream_schema = {
description = "discovery type",
type = "string",
},
+ nacos_namespace_id = {
Review comment:
Yes, the would be better！
--
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] Demogorgon314 commented on a change in pull request #4313: feat: nacos discovery support namespace
Posted by GitBox <gi...@apache.org>.
Demogorgon314 commented on a change in pull request #4313:
URL: https://github.com/apache/apisix/pull/4313#discussion_r640305750
##########
File path: t/discovery/nacos.t
##########
@@ -251,3 +251,134 @@ discovery:
]
--- no_error_log
[error]
+
+
+
+=== TEST 8: get APISIX-NACOS info from NACOS - no auth with namespace
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] spacewander commented on a change in pull request #4313: feat: nacos discovery support namespace
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4313:
URL: https://github.com/apache/apisix/pull/4313#discussion_r640276937
##########
File path: t/discovery/nacos.t
##########
@@ -251,3 +251,134 @@ discovery:
]
--- no_error_log
[error]
+
+
+
+=== TEST 8: get APISIX-NACOS info from NACOS - no auth with namespace
Review comment:
Need another test that the request gets 503 because the namespace is incorrect.
##########
File path: apisix/schema_def.lua
##########
@@ -389,6 +389,10 @@ local upstream_schema = {
description = "discovery type",
type = "string",
},
+ nacos_namespace_id = {
Review comment:
I think it would be better if we move the `nacos_namespace_id` into `discovery_args`, like:
```
discovery_args = {
type = "object",
properties = {
namespace_id = {
description = "namespace id",
type = "string",
},
}
},
```
This change will make the schema tidy.
--
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] Demogorgon314 commented on pull request #4313: feat: nacos discovery support namespace
Posted by GitBox <gi...@apache.org>.
Demogorgon314 commented on pull request #4313:
URL: https://github.com/apache/apisix/pull/4313#issuecomment-849291705
> The CI is always pending on "wait nacos reg..."
I changed nacos-test-service Dockerfile , should be related to this @spacewander
https://github.com/api7/nacos-test-service/pull/1
--
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] Demogorgon314 commented on a change in pull request #4313: feat: nacos discovery support namespace
Posted by GitBox <gi...@apache.org>.
Demogorgon314 commented on a change in pull request #4313:
URL: https://github.com/apache/apisix/pull/4313#discussion_r640209799
##########
File path: apisix/discovery/nacos.lua
##########
@@ -254,10 +264,10 @@ local function fetch_full_registry(premature)
applications = up_apps
return
end
-
local data, err
- for _, service_name in ipairs(infos) do
- data, err = get_url(base_uri, instance_list_path .. service_name .. token_param)
+ for _, service_info in ipairs(infos) do
+ local namespace_param = get_namespace_param(service_info.nacos_namespace_id);
+ data, err = get_url(base_uri, instance_list_path .. service_info.service_name .. token_param .. namespace_param)
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] tokers commented on a change in pull request #4313: feat: nacos discovery support namespace
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #4313:
URL: https://github.com/apache/apisix/pull/4313#discussion_r640240356
##########
File path: docs/en/latest/discovery/nacos.md
##########
@@ -99,3 +99,46 @@ The format response as below:
"action": "set"
}
```
+
+Example of routing a request with a URL of "/nacosWithNamespaceId/*" to a service which name, namespaceId "http://192.168.33.1:8848/nacos/v1/ns/instance/list?serviceName=APISIX-NACOS&namespaceId=test_ns" and use nacos discovery client in the registry :
Review comment:
```suggestion
Example of routing a request with a URL of "/nacosWithNamespaceId/*" to a service which name, namespaceId "http://192.168.33.1:8848/nacos/v1/ns/instance/list?serviceName=APISIX-NACOS&namespaceId=test_ns" and use nacos discovery client in the registry:
```
##########
File path: docs/en/latest/discovery/nacos.md
##########
@@ -99,3 +99,46 @@ The format response as below:
"action": "set"
}
```
+
+Example of routing a request with a URL of "/nacosWithNamespaceId/*" to a service which name, namespaceId "http://192.168.33.1:8848/nacos/v1/ns/instance/list?serviceName=APISIX-NACOS&namespaceId=test_ns" and use nacos discovery client in the registry :
+
+```shell
+$ curl http://127.0.0.1:9080/apisix/admin/routes/2 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -i -d '
+{
+ "uri": "/nacosWithNamespaceId/*",
+ "upstream": {
+ "service_name": "APISIX-NACOS",
+ "type": "roundrobin",
+ "discovery_type": "nacos",
+ "nacos_namespace_id": "test_ns"
+ }
+}'
+```
+
+The format response as below:
Review comment:
```suggestion
The formatted response as below:
```
--
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] Demogorgon314 commented on a change in pull request #4313: feat: nacos discovery support namespace
Posted by GitBox <gi...@apache.org>.
Demogorgon314 commented on a change in pull request #4313:
URL: https://github.com/apache/apisix/pull/4313#discussion_r640502067
##########
File path: docs/en/latest/discovery/nacos.md
##########
@@ -99,3 +99,46 @@ The format response as below:
"action": "set"
}
```
+
+Example of routing a request with a URL of "/nacosWithNamespaceId/*" to a service which name, namespaceId "http://192.168.33.1:8848/nacos/v1/ns/instance/list?serviceName=APISIX-NACOS&namespaceId=test_ns" and use nacos discovery client in the registry :
+
+```shell
+$ curl http://127.0.0.1:9080/apisix/admin/routes/2 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -i -d '
+{
+ "uri": "/nacosWithNamespaceId/*",
+ "upstream": {
+ "service_name": "APISIX-NACOS",
+ "type": "roundrobin",
+ "discovery_type": "nacos",
+ "nacos_namespace_id": "test_ns"
+ }
+}'
+```
+
+The format response as below:
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] Demogorgon314 edited a comment on pull request #4313: feat: nacos discovery support namespace
Posted by GitBox <gi...@apache.org>.
Demogorgon314 edited a comment on pull request #4313:
URL: https://github.com/apache/apisix/pull/4313#issuecomment-849291705
> The CI is always pending on "wait nacos reg..."
I changed nacos-test-service Dockerfile , should be related to this @spacewander
https://github.com/api7/nacos-test-service/pull/1
--
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] spacewander commented on pull request #4313: feat: nacos discovery support namespace
Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #4313:
URL: https://github.com/apache/apisix/pull/4313#issuecomment-849288831
The CI is always pending on "wait nacos reg..."
--
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