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 2022/09/07 08:32:50 UTC

[GitHub] [apisix] spacewander commented on a diff in pull request #7860: change: move etcd conf under deployment

spacewander commented on code in PR #7860:
URL: https://github.com/apache/apisix/pull/7860#discussion_r964546893


##########
apisix/cli/snippet.lua:
##########
@@ -70,6 +70,15 @@ function _M.generate_conf_server(env, conf)
         end
     end
 
+    local ssl_trusted_certificate
+    local etcd_tls_verify = etcd.tls.verify

Review Comment:
   Should we check if etcd.tls is nil?



##########
t/core/etcd-mtls.t:
##########
@@ -24,6 +24,12 @@ if ($out !~ m/function:/) {
     plan('no_plan');
 }
 
+Test::Nginx::Socket::set_http_config_filter(sub {

Review Comment:
   Do we need a separate generator for this test file?



##########
apisix/cli/snippet.lua:
##########
@@ -70,6 +70,15 @@ function _M.generate_conf_server(env, conf)
         end
     end
 
+    local ssl_trusted_certificate
+    local etcd_tls_verify = etcd.tls.verify
+    if enable_https and etcd_tls_verify then
+        if not conf.apisix.ssl.ssl_trusted_certificate then
+            return nil, "should set ssl_trusted_certificate if etcd tls verify is enabled"

Review Comment:
   Now we can remove:
   https://github.com/apache/apisix/blob/c397ede02d0b304b8b3a1d5049f6c1d8a83d3804/apisix/cli/snippet.lua#L42
   ?



-- 
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