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/10 08:45:48 UTC
[GitHub] [apisix] jxhecong opened a new pull request #2690: fix(init_etcd): add Authorization header while enable etcd auth
jxhecong opened a new pull request #2690:
URL: https://github.com/apache/apisix/pull/2690
### What this PR does / why we need it:
https://github.com/apache/apisix/issues/2670
### Pre-submission checklist:
* [ ] Did you explain what problem does this PR solve? Or what new features have been added?
* [ ] Have you added corresponding test cases?
* [ ] Have you modified the corresponding document?
* [ ] Is this PR backward compatible?
----------------------------------------------------------------
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] jxhecong commented on pull request #2690: fix(init_etcd): add Authorization header while enable etcd auth
Posted by GitBox <gi...@apache.org>.
jxhecong commented on pull request #2690:
URL: https://github.com/apache/apisix/pull/2690#issuecomment-724675704
> You need to provide a test in https://github.com/apache/apisix/blob/master/.travis/apisix_cli_test.sh like
>
> https://github.com/apache/apisix/blob/1ea912d75268e9a244aeea82397db011c766124d/t/core/etcd-auth.t#L29
sorry, i have no idea how to do this and why is it needed.
----------------------------------------------------------------
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] membphis commented on a change in pull request #2690: fix(init_etcd): add Authorization header while enable etcd auth
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2690:
URL: https://github.com/apache/apisix/pull/2690#discussion_r523747590
##########
File path: bin/apisix
##########
@@ -560,6 +560,23 @@ local function init_etcd(show_output)
for index, host in ipairs(yaml_conf.etcd.host) do
local is_success = true
+ local token_head = ""
+ if etcd_conf.user and etcd_conf.password then
+ local uri_auth = host .. "/v3/auth/authenticate"
+ local post_json_auth = '{"name": "' .. etcd_conf.user .. '", "password": "' .. etcd_conf.password .. '"}'
Review comment:
yes, `dkjson` is more safe.
----------------------------------------------------------------
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] jxhecong commented on a change in pull request #2690: fix(init_etcd): add Authorization header while enable etcd auth
Posted by GitBox <gi...@apache.org>.
jxhecong commented on a change in pull request #2690:
URL: https://github.com/apache/apisix/pull/2690#discussion_r524048203
##########
File path: bin/apisix
##########
@@ -560,6 +560,23 @@ local function init_etcd(show_output)
for index, host in ipairs(yaml_conf.etcd.host) do
local is_success = true
+ local token_head = ""
+ if etcd_conf.user and etcd_conf.password then
+ local uri_auth = host .. "/v3/auth/authenticate"
+ local post_json_auth = '{"name": "' .. etcd_conf.user .. '", "password": "' .. etcd_conf.password .. '"}'
Review comment:
Done
----------------------------------------------------------------
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] wfnuser commented on a change in pull request #2690: fix(CLI): add Authorization header while enable etcd auth
Posted by GitBox <gi...@apache.org>.
wfnuser commented on a change in pull request #2690:
URL: https://github.com/apache/apisix/pull/2690#discussion_r529510734
##########
File path: .travis/apisix_cli_test.sh
##########
@@ -494,3 +494,68 @@ fi
make stop
echo "passed: access log with JSON format"
+
+# check etcd while enable auth
+git checkout conf/config.yaml
+
+export ETCDCTL_API=3
+etcdctl version
+etcdctl --endpoints=127.0.0.1:2379 user add "root:apache-api6"
+etcdctl --endpoints=127.0.0.1:2379 role add root
+etcdctl --endpoints=127.0.0.1:2379 user grant-role root root
+etcdctl --endpoints=127.0.0.1:2379 user get root
+etcdctl --endpoints=127.0.0.1:2379 auth enable
+etcdctl --endpoints=127.0.0.1:2379 --user=root:apache-api6 del /apisix --prefix
+
+echo '
+etcd:
+ host:
+ - "http://127.0.0.1:2379"
+ prefix: "/apisix"
+ timeout: 30
+ user: root
+ password: apache-api6
+' > conf/config.yaml
+
+make init
+cmd_res=`etcdctl --endpoints=127.0.0.1:2379 --user=root:apache-api6 get /apisix --prefix`
+etcdctl --endpoints=127.0.0.1:2379 --user=root:apache-api6 auth disable
+etcdctl --endpoints=127.0.0.1:2379 role delete root
+etcdctl --endpoints=127.0.0.1:2379 user delete root
+
+init_kv=(
+/apisix/consumers/
Review comment:
I see... You mean we should only check part of this key exists? But there might be more keys, and not in the same order?
----------------------------------------------------------------
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 #2690: fix(init_etcd): add Authorization header while enable etcd auth
Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #2690:
URL: https://github.com/apache/apisix/pull/2690#issuecomment-724648836
You need to provide a test in https://github.com/apache/apisix/blob/master/.travis/apisix_cli_test.sh like https://github.com/apache/apisix/blob/1ea912d75268e9a244aeea82397db011c766124d/t/core/etcd-auth.t#L29
----------------------------------------------------------------
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] jxhecong commented on pull request #2690: fix(init_etcd): add Authorization header while enable etcd auth
Posted by GitBox <gi...@apache.org>.
jxhecong commented on pull request #2690:
URL: https://github.com/apache/apisix/pull/2690#issuecomment-724642209
> Can we calculate the token from user and password?
you mean calculate the token in local, no need to request etcd server?
----------------------------------------------------------------
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 #2690: fix(init_etcd): add Authorization header while enable etcd auth
Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #2690:
URL: https://github.com/apache/apisix/pull/2690#issuecomment-724646999
>
>
> > Can we calculate the token from user and password?
>
> you mean calculate the token in local, no need to request etcd server?
I read the doc of etcd. It is impossible. There can be better than your way.
----------------------------------------------------------------
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] jxhecong commented on a change in pull request #2690: fix(init_etcd): add Authorization header while enable etcd auth
Posted by GitBox <gi...@apache.org>.
jxhecong commented on a change in pull request #2690:
URL: https://github.com/apache/apisix/pull/2690#discussion_r520509095
##########
File path: bin/apisix
##########
@@ -560,6 +560,21 @@ local function init_etcd(show_output)
for index, host in ipairs(yaml_conf.etcd.host) do
local is_success = true
+ local token_head = ""
+ if etcd_conf.user and etcd_conf.password then
+ local uri_auth = host .. "/v3/auth/authenticate"
+ local post_json_auth = '{"name": "' .. etcd_conf.user .. '", "password": "' .. etcd_conf.password .. '"}'
+ local cmd_auth = "curl -s " .. uri_auth .. " -X POST -d '" .. post_json_auth
+ .. "' --connect-timeout " .. timeout
+ .. " --max-time " .. timeout * 2 .. " --retry 1 2>&1"
+
+ local res_auth = execute_cmd(cmd_auth)
+ local body_auth, _, err_auth = dkjson.decode(res_auth)
+ if not err_auth then
+ token_head = " -H 'Authorization: " .. body_auth.token .. "'"
Review comment:
Error message shoud be show, but I think it's not necessary to stop when the command failed
----------------------------------------------------------------
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] membphis commented on a change in pull request #2690: fix(CLI): add Authorization header while enable etcd auth
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2690:
URL: https://github.com/apache/apisix/pull/2690#discussion_r524838230
##########
File path: bin/apisix
##########
@@ -560,6 +560,27 @@ local function init_etcd(show_output)
for index, host in ipairs(yaml_conf.etcd.host) do
local is_success = true
+ local token_head = ""
+ if etcd_conf.user and etcd_conf.password then
+ local uri_auth = host .. "/v3/auth/authenticate"
+ local json_auth = {
+ name = etcd_conf.user,
+ password = etcd_conf.password
+ }
+ local post_json_auth = dkjson.encode(json_auth)
+ local cmd_auth = "curl -s " .. uri_auth .. " -X POST -d '" .. post_json_auth
+ .. "' --connect-timeout " .. timeout
Review comment:
bad indentation
##########
File path: bin/apisix
##########
@@ -560,6 +560,27 @@ local function init_etcd(show_output)
for index, host in ipairs(yaml_conf.etcd.host) do
local is_success = true
+ local token_head = ""
+ if etcd_conf.user and etcd_conf.password then
+ local uri_auth = host .. "/v3/auth/authenticate"
+ local json_auth = {
+ name = etcd_conf.user,
+ password = etcd_conf.password
+ }
+ local post_json_auth = dkjson.encode(json_auth)
+ local cmd_auth = "curl -s " .. uri_auth .. " -X POST -d '" .. post_json_auth
+ .. "' --connect-timeout " .. timeout
+ .. " --max-time " .. timeout * 2 .. " --retry 1 2>&1"
+
+ local res_auth = execute_cmd(cmd_auth)
+ local body_auth, _, err_auth = dkjson.decode(res_auth)
+ if not err_auth then
Review comment:
bad style, here is a good style, please take a look at:
```lua
if err_auth then
error(....)
end
...
```
----------------------------------------------------------------
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] jxhecong commented on a change in pull request #2690: fix(init_etcd): add Authorization header while enable etcd auth
Posted by GitBox <gi...@apache.org>.
jxhecong commented on a change in pull request #2690:
URL: https://github.com/apache/apisix/pull/2690#discussion_r524047061
##########
File path: bin/apisix
##########
@@ -560,6 +560,21 @@ local function init_etcd(show_output)
for index, host in ipairs(yaml_conf.etcd.host) do
local is_success = true
+ local token_head = ""
+ if etcd_conf.user and etcd_conf.password then
+ local uri_auth = host .. "/v3/auth/authenticate"
+ local post_json_auth = '{"name": "' .. etcd_conf.user .. '", "password": "' .. etcd_conf.password .. '"}'
+ local cmd_auth = "curl -s " .. uri_auth .. " -X POST -d '" .. post_json_auth
+ .. "' --connect-timeout " .. timeout
+ .. " --max-time " .. timeout * 2 .. " --retry 1 2>&1"
+
+ local res_auth = execute_cmd(cmd_auth)
+ local body_auth, _, err_auth = dkjson.decode(res_auth)
+ if not err_auth then
+ token_head = " -H 'Authorization: " .. body_auth.token .. "'"
Review comment:
Done
----------------------------------------------------------------
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] wfnuser commented on a change in pull request #2690: fix(CLI): add Authorization header while enable etcd auth
Posted by GitBox <gi...@apache.org>.
wfnuser commented on a change in pull request #2690:
URL: https://github.com/apache/apisix/pull/2690#discussion_r529502786
##########
File path: .travis/apisix_cli_test.sh
##########
@@ -494,3 +494,68 @@ fi
make stop
echo "passed: access log with JSON format"
+
+# check etcd while enable auth
+git checkout conf/config.yaml
+
+export ETCDCTL_API=3
+etcdctl version
+etcdctl --endpoints=127.0.0.1:2379 user add "root:apache-api6"
+etcdctl --endpoints=127.0.0.1:2379 role add root
+etcdctl --endpoints=127.0.0.1:2379 user grant-role root root
+etcdctl --endpoints=127.0.0.1:2379 user get root
+etcdctl --endpoints=127.0.0.1:2379 auth enable
+etcdctl --endpoints=127.0.0.1:2379 --user=root:apache-api6 del /apisix --prefix
+
+echo '
+etcd:
+ host:
+ - "http://127.0.0.1:2379"
+ prefix: "/apisix"
+ timeout: 30
+ user: root
+ password: apache-api6
+' > conf/config.yaml
+
+make init
+cmd_res=`etcdctl --endpoints=127.0.0.1:2379 --user=root:apache-api6 get /apisix --prefix`
+etcdctl --endpoints=127.0.0.1:2379 --user=root:apache-api6 auth disable
+etcdctl --endpoints=127.0.0.1:2379 role delete root
+etcdctl --endpoints=127.0.0.1:2379 user delete root
+
+init_kv=(
+/apisix/consumers/
Review comment:
So the problem is which part should we confirm?
----------------------------------------------------------------
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] jxhecong commented on a change in pull request #2690: fix(CLI): add Authorization header while enable etcd auth
Posted by GitBox <gi...@apache.org>.
jxhecong commented on a change in pull request #2690:
URL: https://github.com/apache/apisix/pull/2690#discussion_r526576620
##########
File path: bin/apisix
##########
@@ -560,6 +560,27 @@ local function init_etcd(show_output)
for index, host in ipairs(yaml_conf.etcd.host) do
local is_success = true
+ local token_head = ""
+ if etcd_conf.user and etcd_conf.password then
+ local uri_auth = host .. "/v3/auth/authenticate"
+ local json_auth = {
+ name = etcd_conf.user,
+ password = etcd_conf.password
+ }
+ local post_json_auth = dkjson.encode(json_auth)
+ local cmd_auth = "curl -s " .. uri_auth .. " -X POST -d '" .. post_json_auth
+ .. "' --connect-timeout " .. timeout
+ .. " --max-time " .. timeout * 2 .. " --retry 1 2>&1"
+
+ local res_auth = execute_cmd(cmd_auth)
+ local body_auth, _, err_auth = dkjson.decode(res_auth)
+ if not err_auth then
Review comment:
Done
----------------------------------------------------------------
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] membphis commented on a change in pull request #2690: fix(CLI): add Authorization header while enable etcd auth
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2690:
URL: https://github.com/apache/apisix/pull/2690#discussion_r527367999
##########
File path: .travis/apisix_cli_test.sh
##########
@@ -494,3 +494,68 @@ fi
make stop
echo "passed: access log with JSON format"
+
+# check etcd while enable auth
+git checkout conf/config.yaml
+
+export ETCDCTL_API=3
+etcdctl version
+etcdctl --endpoints=127.0.0.1:2379 user add "root:apache-api6"
+etcdctl --endpoints=127.0.0.1:2379 role add root
+etcdctl --endpoints=127.0.0.1:2379 user grant-role root root
+etcdctl --endpoints=127.0.0.1:2379 user get root
+etcdctl --endpoints=127.0.0.1:2379 auth enable
+etcdctl --endpoints=127.0.0.1:2379 --user=root:apache-api6 del /apisix --prefix
+
+echo '
+etcd:
+ host:
+ - "http://127.0.0.1:2379"
+ prefix: "/apisix"
+ timeout: 30
+ user: root
+ password: apache-api6
+' > conf/config.yaml
+
+make init
+cmd_res=`etcdctl --endpoints=127.0.0.1:2379 --user=root:apache-api6 get /apisix --prefix`
+etcdctl --endpoints=127.0.0.1:2379 --user=root:apache-api6 auth disable
+etcdctl --endpoints=127.0.0.1:2379 role delete root
+etcdctl --endpoints=127.0.0.1:2379 user delete root
+
+init_kv=(
+/apisix/consumers/
Review comment:
I think we only need to confirm part of it. This list may change in the future.
we can do this in a new PR.
----------------------------------------------------------------
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 #2690: fix(CLI): add Authorization header while enable etcd auth
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2690:
URL: https://github.com/apache/apisix/pull/2690#discussion_r526777703
##########
File path: bin/apisix
##########
@@ -560,6 +560,26 @@ local function init_etcd(show_output)
for index, host in ipairs(yaml_conf.etcd.host) do
local is_success = true
+ local token_head = ""
+ if etcd_conf.user and etcd_conf.password then
+ local uri_auth = host .. "/v3/auth/authenticate"
+ local json_auth = {
+ name = etcd_conf.user,
+ password = etcd_conf.password
+ }
+ local post_json_auth = dkjson.encode(json_auth)
+ local cmd_auth = "curl -s " .. uri_auth .. " -X POST -d '" .. post_json_auth
+ .. "' --connect-timeout " .. timeout
+ .. " --max-time " .. timeout * 2 .. " --retry 1 2>&1"
+
+ local res_auth = execute_cmd(cmd_auth)
Review comment:
Should be `util.execute_cmd`
----------------------------------------------------------------
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] jxhecong commented on pull request #2690: fix(CLI): add Authorization header while enable etcd auth
Posted by GitBox <gi...@apache.org>.
jxhecong commented on pull request #2690:
URL: https://github.com/apache/apisix/pull/2690#issuecomment-728774403
> missing CLI test, this is important.
> we need to confirm new CLI can work fine for this case
How far should be checked?
In the check environment, what is etcd's address and passwd of root user?
----------------------------------------------------------------
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 #2690: fix(init_etcd): add Authorization header while enable etcd auth
Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #2690:
URL: https://github.com/apache/apisix/pull/2690#issuecomment-724611961
Can we calculate the token from user and password?
----------------------------------------------------------------
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] membphis merged pull request #2690: fix(CLI): add Authorization header while enable etcd auth
Posted by GitBox <gi...@apache.org>.
membphis merged pull request #2690:
URL: https://github.com/apache/apisix/pull/2690
----------------------------------------------------------------
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] membphis edited a comment on pull request #2690: fix(CLI): add Authorization header while enable etcd auth
Posted by GitBox <gi...@apache.org>.
membphis edited a comment on pull request #2690:
URL: https://github.com/apache/apisix/pull/2690#issuecomment-729040840
> In the check environment, what is etcd's address and passwd of root user?
you can start an etcd instance with as you want by shell. here is an example of `lua-resty-etcd`: https://github.com/api7/lua-resty-etcd/blob/master/t/v3/add-auth.sh
----------------------------------------------------------------
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 #2690: fix(init_etcd): add Authorization header while enable etcd auth
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2690:
URL: https://github.com/apache/apisix/pull/2690#discussion_r520604995
##########
File path: bin/apisix
##########
@@ -560,6 +560,21 @@ local function init_etcd(show_output)
for index, host in ipairs(yaml_conf.etcd.host) do
local is_success = true
+ local token_head = ""
+ if etcd_conf.user and etcd_conf.password then
+ local uri_auth = host .. "/v3/auth/authenticate"
+ local post_json_auth = '{"name": "' .. etcd_conf.user .. '", "password": "' .. etcd_conf.password .. '"}'
+ local cmd_auth = "curl -s " .. uri_auth .. " -X POST -d '" .. post_json_auth
+ .. "' --connect-timeout " .. timeout
+ .. " --max-time " .. timeout * 2 .. " --retry 1 2>&1"
+
+ local res_auth = execute_cmd(cmd_auth)
+ local body_auth, _, err_auth = dkjson.decode(res_auth)
+ if not err_auth then
+ token_head = " -H 'Authorization: " .. body_auth.token .. "'"
Review comment:
@jxhecong
We should stop the command when the authorization is failed. Otherwise, we won't init the etcd correctly, just like what happen 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] membphis commented on pull request #2690: fix(CLI): add Authorization header while enable etcd auth
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2690:
URL: https://github.com/apache/apisix/pull/2690#issuecomment-729040840
> In the check environment, what is etcd's address and passwd of root user?
you can start an etcd instance with as you want by shell.
----------------------------------------------------------------
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] juzhiyuan commented on a change in pull request #2690: fix(init_etcd): add Authorization header while enable etcd auth
Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on a change in pull request #2690:
URL: https://github.com/apache/apisix/pull/2690#discussion_r522200494
##########
File path: bin/apisix
##########
@@ -560,6 +560,23 @@ local function init_etcd(show_output)
for index, host in ipairs(yaml_conf.etcd.host) do
local is_success = true
+ local token_head = ""
+ if etcd_conf.user and etcd_conf.password then
+ local uri_auth = host .. "/v3/auth/authenticate"
+ local post_json_auth = '{"name": "' .. etcd_conf.user .. '", "password": "' .. etcd_conf.password .. '"}'
Review comment:
Could we use some libraries or functions to stringify that object instead of combining string?
https://stackoverflow.com/questions/24908199/convert-json-string-to-lua-table
##########
File path: bin/apisix
##########
@@ -560,6 +560,23 @@ local function init_etcd(show_output)
for index, host in ipairs(yaml_conf.etcd.host) do
local is_success = true
+ local token_head = ""
+ if etcd_conf.user and etcd_conf.password then
+ local uri_auth = host .. "/v3/auth/authenticate"
+ local post_json_auth = '{"name": "' .. etcd_conf.user .. '", "password": "' .. etcd_conf.password .. '"}'
Review comment:
According to https://github.com/apache/apisix/pull/2690/files#diff-0cfc3cc4673fc5c2de7be20b9f8763a8955212292f08a8a7b5d1f459533f5fcdR572 , could we use `dkjson.encode`?
##########
File path: bin/apisix
##########
@@ -560,6 +560,21 @@ local function init_etcd(show_output)
for index, host in ipairs(yaml_conf.etcd.host) do
local is_success = true
+ local token_head = ""
+ if etcd_conf.user and etcd_conf.password then
+ local uri_auth = host .. "/v3/auth/authenticate"
+ local post_json_auth = '{"name": "' .. etcd_conf.user .. '", "password": "' .. etcd_conf.password .. '"}'
+ local cmd_auth = "curl -s " .. uri_auth .. " -X POST -d '" .. post_json_auth
+ .. "' --connect-timeout " .. timeout
+ .. " --max-time " .. timeout * 2 .. " --retry 1 2>&1"
+
+ local res_auth = execute_cmd(cmd_auth)
+ local body_auth, _, err_auth = dkjson.decode(res_auth)
+ if not err_auth then
+ token_head = " -H 'Authorization: " .. body_auth.token .. "'"
Review comment:
Agree, once the authentication failed, we have to terminate the command.
----------------------------------------------------------------
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 #2690: fix(CLI): add Authorization header while enable etcd auth
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2690:
URL: https://github.com/apache/apisix/pull/2690#discussion_r526901780
##########
File path: bin/apisix
##########
@@ -560,6 +560,26 @@ local function init_etcd(show_output)
for index, host in ipairs(yaml_conf.etcd.host) do
local is_success = true
+ local token_head = ""
+ if etcd_conf.user and etcd_conf.password then
+ local uri_auth = host .. "/v3/auth/authenticate"
+ local json_auth = {
+ name = etcd_conf.user,
+ password = etcd_conf.password
+ }
+ local post_json_auth = dkjson.encode(json_auth)
+ local cmd_auth = "curl -s " .. uri_auth .. " -X POST -d '" .. post_json_auth
+ .. "' --connect-timeout " .. timeout
+ .. " --max-time " .. timeout * 2 .. " --retry 1 2>&1"
+
+ local res_auth = execute_cmd(cmd_auth)
Review comment:
@jxhecong
The check failed because `execute_cmd` doesn't exist (it should be `util.execute_cmd`). Let's fix it and see what's next.
----------------------------------------------------------------
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] jxhecong commented on a change in pull request #2690: fix(CLI): add Authorization header while enable etcd auth
Posted by GitBox <gi...@apache.org>.
jxhecong commented on a change in pull request #2690:
URL: https://github.com/apache/apisix/pull/2690#discussion_r526843319
##########
File path: bin/apisix
##########
@@ -560,6 +560,26 @@ local function init_etcd(show_output)
for index, host in ipairs(yaml_conf.etcd.host) do
local is_success = true
+ local token_head = ""
+ if etcd_conf.user and etcd_conf.password then
+ local uri_auth = host .. "/v3/auth/authenticate"
+ local json_auth = {
+ name = etcd_conf.user,
+ password = etcd_conf.password
+ }
+ local post_json_auth = dkjson.encode(json_auth)
+ local cmd_auth = "curl -s " .. uri_auth .. " -X POST -d '" .. post_json_auth
+ .. "' --connect-timeout " .. timeout
+ .. " --max-time " .. timeout * 2 .. " --retry 1 2>&1"
+
+ local res_auth = execute_cmd(cmd_auth)
Review comment:
anything else? 1 failing checks
----------------------------------------------------------------
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] jxhecong commented on a change in pull request #2690: fix(CLI): add Authorization header while enable etcd auth
Posted by GitBox <gi...@apache.org>.
jxhecong commented on a change in pull request #2690:
URL: https://github.com/apache/apisix/pull/2690#discussion_r526576537
##########
File path: bin/apisix
##########
@@ -560,6 +560,27 @@ local function init_etcd(show_output)
for index, host in ipairs(yaml_conf.etcd.host) do
local is_success = true
+ local token_head = ""
+ if etcd_conf.user and etcd_conf.password then
+ local uri_auth = host .. "/v3/auth/authenticate"
+ local json_auth = {
+ name = etcd_conf.user,
+ password = etcd_conf.password
+ }
+ local post_json_auth = dkjson.encode(json_auth)
+ local cmd_auth = "curl -s " .. uri_auth .. " -X POST -d '" .. post_json_auth
+ .. "' --connect-timeout " .. timeout
Review comment:
Done
----------------------------------------------------------------
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] jxhecong commented on a change in pull request #2690: fix(CLI): add Authorization header while enable etcd auth
Posted by GitBox <gi...@apache.org>.
jxhecong commented on a change in pull request #2690:
URL: https://github.com/apache/apisix/pull/2690#discussion_r526843319
##########
File path: bin/apisix
##########
@@ -560,6 +560,26 @@ local function init_etcd(show_output)
for index, host in ipairs(yaml_conf.etcd.host) do
local is_success = true
+ local token_head = ""
+ if etcd_conf.user and etcd_conf.password then
+ local uri_auth = host .. "/v3/auth/authenticate"
+ local json_auth = {
+ name = etcd_conf.user,
+ password = etcd_conf.password
+ }
+ local post_json_auth = dkjson.encode(json_auth)
+ local cmd_auth = "curl -s " .. uri_auth .. " -X POST -d '" .. post_json_auth
+ .. "' --connect-timeout " .. timeout
+ .. " --max-time " .. timeout * 2 .. " --retry 1 2>&1"
+
+ local res_auth = execute_cmd(cmd_auth)
Review comment:
Anything else, what about the changes in .travis/apisix_cli_test.sh?
And there left 1 failing checks, what should i do to fix 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] spacewander commented on a change in pull request #2690: fix(init_etcd): add Authorization header while enable etcd auth
Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2690:
URL: https://github.com/apache/apisix/pull/2690#discussion_r520495542
##########
File path: bin/apisix
##########
@@ -560,6 +560,21 @@ local function init_etcd(show_output)
for index, host in ipairs(yaml_conf.etcd.host) do
local is_success = true
+ local token_head = ""
+ if etcd_conf.user and etcd_conf.password then
+ local uri_auth = host .. "/v3/auth/authenticate"
+ local post_json_auth = '{"name": "' .. etcd_conf.user .. '", "password": "' .. etcd_conf.password .. '"}'
+ local cmd_auth = "curl -s " .. uri_auth .. " -X POST -d '" .. post_json_auth
+ .. "' --connect-timeout " .. timeout
+ .. " --max-time " .. timeout * 2 .. " --retry 1 2>&1"
+
+ local res_auth = execute_cmd(cmd_auth)
+ local body_auth, _, err_auth = dkjson.decode(res_auth)
+ if not err_auth then
+ token_head = " -H 'Authorization: " .. body_auth.token .. "'"
Review comment:
Need to show error message and stop the command when failed.
----------------------------------------------------------------
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] moonming commented on pull request #2690: fix(CLI): add Authorization header while enable etcd auth
Posted by GitBox <gi...@apache.org>.
moonming commented on pull request #2690:
URL: https://github.com/apache/apisix/pull/2690#issuecomment-730800617
@membphis please take a look
----------------------------------------------------------------
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