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/10/11 16:27:32 UTC
[GitHub] [apisix] tzssangglass opened a new pull request #5206: feat: support specify custom sni in etcd conf
tzssangglass opened a new pull request #5206:
URL: https://github.com/apache/apisix/pull/5206
Signed-off-by: tzssangglass <tz...@gmail.com>
### What this PR does / why we need it:
<!--- Why is this change required? What problem does it solve? -->
fix: #5155
<!--- If it fixes an open issue, please link to the issue here. -->
### 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?
* [ ] 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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #5206: feat: support specify custom sni in etcd conf
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5206:
URL: https://github.com/apache/apisix/pull/5206#discussion_r727113838
##########
File path: t/core/config_etcd.t
##########
@@ -57,6 +57,11 @@ apisix:
etcd:
host:
- "https://127.0.0.1:2379"
+--- extra_init_by_lua
+local health_check = require("resty.etcd.health_check")
+health_check.get_target_status = function()
Review comment:
Why bypass the get_target_status?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a change in pull request #5206: feat: support specify custom sni in etcd conf
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #5206:
URL: https://github.com/apache/apisix/pull/5206#discussion_r727673582
##########
File path: conf/config-default.yaml
##########
@@ -272,6 +272,7 @@ etcd:
verify: true # whether to verify the etcd endpoint certificate when setup a TLS connection to etcd,
# the default value is true, e.g. the certificate will be verified strictly.
+ #sni: # the SNI fot etcd TLS requests
Review comment:
update
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a change in pull request #5206: feat: support specify custom sni in etcd conf
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #5206:
URL: https://github.com/apache/apisix/pull/5206#discussion_r727249565
##########
File path: t/core/config_etcd.t
##########
@@ -57,6 +57,11 @@ apisix:
etcd:
host:
- "https://127.0.0.1:2379"
+--- extra_init_by_lua
+local health_check = require("resty.etcd.health_check")
+health_check.get_target_status = function()
Review comment:
In the lua-resty-etcd v1.5.5 implementation, there is no passive round-robin health check, so when apisix loads etcd configuration when apisix starts, here:https://github.com/apache/apisix/blob/c7523da7d45b674427205c435444133ff1aca258/apisix/init.lua#L84-L86
even if it get `failed to load the configuration: closed`, then sync data from etcd in ngx.timer will still use `https://127.0.0.1:2379` as the etcd endpoint and get the `handshake failed`.
but in lua-resty-etcd v1.6.0 implementation, the passive round-robin health check is default policy, so when apisix loads etcd configuration when apisix starts, `https://127.0.0.1:2379` is marked as unhealthy endpoint, and then sync data from etcd in ngx.timer would got `has no healthy etcd endpoint available` in `choose_endpoint` function, can not get the `handshake failed`.
bypass `get_target_status` to simulate the behaviour of the original test case.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #5206: feat: support specify custom sni in etcd conf
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5206:
URL: https://github.com/apache/apisix/pull/5206#discussion_r727620614
##########
File path: t/core/config_etcd.t
##########
@@ -57,6 +57,11 @@ apisix:
etcd:
host:
- "https://127.0.0.1:2379"
+--- extra_init_by_lua
+local health_check = require("resty.etcd.health_check")
+health_check.get_target_status = function()
Review comment:
I see.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a change in pull request #5206: feat: support specify custom sni in etcd conf
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #5206:
URL: https://github.com/apache/apisix/pull/5206#discussion_r727068787
##########
File path: conf/config-default.yaml
##########
@@ -272,6 +272,7 @@ etcd:
verify: true # whether to verify the etcd endpoint certificate when setup a TLS connection to etcd,
# the default value is true, e.g. the certificate will be verified strictly.
+ #sni: # the SNI fot etcd TLS requests
Review comment:
Yes, but in lua-resty-etcd sni is same level as tls, which I didn't think of at the time. Now if I want to change it, I need to change the code. I think it can be tolerated
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander merged pull request #5206: feat: support specify custom sni in etcd conf
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #5206:
URL: https://github.com/apache/apisix/pull/5206
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #5206: feat: support specify custom sni in etcd conf
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5206:
URL: https://github.com/apache/apisix/pull/5206#discussion_r727673934
##########
File path: conf/config-default.yaml
##########
@@ -272,6 +272,7 @@ etcd:
verify: true # whether to verify the etcd endpoint certificate when setup a TLS connection to etcd,
# the default value is true, e.g. the certificate will be verified strictly.
+ #sni: # the SNI fot etcd TLS requests
Review comment:
```suggestion
#sni: # the SNI for etcd TLS requests. If missed, the host part of the URL will be used
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #5206: feat: support specify custom sni in etcd conf
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5206:
URL: https://github.com/apache/apisix/pull/5206#discussion_r727111898
##########
File path: conf/config-default.yaml
##########
@@ -272,6 +272,7 @@ etcd:
verify: true # whether to verify the etcd endpoint certificate when setup a TLS connection to etcd,
# the default value is true, e.g. the certificate will be verified strictly.
+ #sni: # the SNI fot etcd TLS requests
Review comment:
I think we can change it in APISIX? If we only change it in APISIX, it would be simpler.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander merged pull request #5206: feat: support specify custom sni in etcd conf
Posted by GitBox <gi...@apache.org>.
spacewander merged pull request #5206:
URL: https://github.com/apache/apisix/pull/5206
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on a change in pull request #5206: feat: support specify custom sni in etcd conf
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #5206:
URL: https://github.com/apache/apisix/pull/5206#discussion_r727620614
##########
File path: t/core/config_etcd.t
##########
@@ -57,6 +57,11 @@ apisix:
etcd:
host:
- "https://127.0.0.1:2379"
+--- extra_init_by_lua
+local health_check = require("resty.etcd.health_check")
+health_check.get_target_status = function()
Review comment:
I see.
##########
File path: conf/config-default.yaml
##########
@@ -272,6 +272,7 @@ etcd:
verify: true # whether to verify the etcd endpoint certificate when setup a TLS connection to etcd,
# the default value is true, e.g. the certificate will be verified strictly.
+ #sni: # the SNI fot etcd TLS requests
Review comment:
```suggestion
#sni: # the SNI for etcd TLS requests. If missed, the host part of the URL will be used
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tokers commented on a change in pull request #5206: feat: support specify custom sni in etcd conf
Posted by GitBox <gi...@apache.org>.
tokers commented on a change in pull request #5206:
URL: https://github.com/apache/apisix/pull/5206#discussion_r726696763
##########
File path: conf/config-default.yaml
##########
@@ -272,6 +272,7 @@ etcd:
verify: true # whether to verify the etcd endpoint certificate when setup a TLS connection to etcd,
# the default value is true, e.g. the certificate will be verified strictly.
+ #sni: # the SNI fot etcd TLS requests
Review comment:
Personally, I think `sni` field should be part of `tls` section.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] tzssangglass commented on a change in pull request #5206: feat: support specify custom sni in etcd conf
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on a change in pull request #5206:
URL: https://github.com/apache/apisix/pull/5206#discussion_r727249565
##########
File path: t/core/config_etcd.t
##########
@@ -57,6 +57,11 @@ apisix:
etcd:
host:
- "https://127.0.0.1:2379"
+--- extra_init_by_lua
+local health_check = require("resty.etcd.health_check")
+health_check.get_target_status = function()
Review comment:
In the lua-resty-etcd v1.5.5 implementation, there is no passive round-robin health check, so when apisix loads etcd configuration when apisix starts, here:https://github.com/apache/apisix/blob/c7523da7d45b674427205c435444133ff1aca258/apisix/init.lua#L84-L86
even if it get `failed to load the configuration: closed`, then sync data from etcd in ngx.timer will still use `https://127.0.0.1:2379` as the etcd endpoint and get the `handshake failed`.
but in lua-resty-etcd v1.6.0 implementation, the passive round-robin health check is default policy, so when apisix loads etcd configuration when apisix starts, `https://127.0.0.1:2379` is marked as unhealthy endpoint, and then sync data from etcd in ngx.timer would got `has no healthy etcd endpoint available` in `choose_endpoint` function, can not get the `handshake failed`.
bypass `get_target_status` to simulate the behaviour of the original test case.
##########
File path: conf/config-default.yaml
##########
@@ -272,6 +272,7 @@ etcd:
verify: true # whether to verify the etcd endpoint certificate when setup a TLS connection to etcd,
# the default value is true, e.g. the certificate will be verified strictly.
+ #sni: # the SNI fot etcd TLS requests
Review comment:
update
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org