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/07/29 09:54:01 UTC

[GitHub] [apisix] zll2 opened a new pull request #4702: feat: support aliyun mse nacos ram

zll2 opened a new pull request #4702:
URL: https://github.com/apache/apisix/pull/4702


   support aliyun mse nacos ram authentication
   
   ### What this PR does / why we need it:
   <!--- Why is this change required? What problem does it solve? -->
   <!--- 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?
   * [ ] Have you added corresponding test cases?
   * [ ] Have you modified the corresponding document?
   * [ ] 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] zll2 commented on a change in pull request #4702: feat: support aliyun mse nacos ram

Posted by GitBox <gi...@apache.org>.
zll2 commented on a change in pull request #4702:
URL: https://github.com/apache/apisix/pull/4702#discussion_r679117027



##########
File path: apisix/discovery/nacos.lua
##########
@@ -211,7 +272,7 @@ local function get_base_uri()
         url = url .. '/'
     end
 
-    return url, username, password
+    return url, username, password, access_key, secret_key, authorization_type

Review comment:
       好的，我改下




-- 
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 #4702: feat: support aliyun mse nacos ram

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4702:
URL: https://github.com/apache/apisix/pull/4702#discussion_r679096840



##########
File path: apisix/discovery/nacos.lua
##########
@@ -211,7 +272,7 @@ local function get_base_uri()
         url = url .. '/'
     end
 
-    return url, username, password
+    return url, username, password, access_key, secret_key, authorization_type

Review comment:
       There is a typo in my suggestion...
   It should be
   ```
   url, authorization_type, username, password
   or
   url, authorization_type, access_key, secret_key
   ```
   
   It seems that each authorization_type only uses two of them, so we can return only four values instead of six.




-- 
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] juzhiyuan commented on pull request #4702: feat: support aliyun mse nacos ram

Posted by GitBox <gi...@apache.org>.
juzhiyuan commented on pull request #4702:
URL: https://github.com/apache/apisix/pull/4702#issuecomment-888984648


   @zll2 Hi, please have a check at the CI https://github.com/apache/apisix/pull/4702/checks?check_run_id=3190661841


-- 
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] zll2 commented on a change in pull request #4702: feat: support aliyun mse nacos ram

Posted by GitBox <gi...@apache.org>.
zll2 commented on a change in pull request #4702:
URL: https://github.com/apache/apisix/pull/4702#discussion_r679087490



##########
File path: apisix/discovery/nacos.lua
##########
@@ -211,7 +272,7 @@ local function get_base_uri()
         url = url .. '/'
     end
 
-    return url, username, password
+    return url, username, password, access_key, secret_key, authorization_type

Review comment:
       这里的建议是 authorization_type 返回顺序往前提？
   
   1、username, password
   2、access_key, secret_key
   
   1 和 2 是两两组合使用，用于支持 2 种不同的认证方式




-- 
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] zll2 commented on a change in pull request #4702: feat: support aliyun mse nacos ram

Posted by GitBox <gi...@apache.org>.
zll2 commented on a change in pull request #4702:
URL: https://github.com/apache/apisix/pull/4702#discussion_r679116787



##########
File path: docs/en/latest/discovery/nacos.md
##########
@@ -38,6 +38,10 @@ discovery:
   nacos:
     host:
       - "http://${username}:${password}@${host1}:${port1}"
+    authorization:
+      type: "basic_auth"  # default basic_auth
+      access_key: ""      # default empty

Review comment:
       好的，我重新提交一个 PR，这个 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.

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 #4702: feat: support aliyun mse nacos ram

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4702:
URL: https://github.com/apache/apisix/pull/4702#discussion_r679098717



##########
File path: docs/en/latest/discovery/nacos.md
##########
@@ -38,6 +38,10 @@ discovery:
   nacos:
     host:
       - "http://${username}:${password}@${host1}:${port1}"
+    authorization:
+      type: "basic_auth"  # default basic_auth
+      access_key: ""      # default empty

Review comment:
       The official docs don't give a name to this authorization: https://nacos.io/zh-cn/docs/auth.html
   
   What about just call it "default"? The default authorization of Nacos.




-- 
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 #4702: feat: support aliyun mse nacos ram

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #4702:
URL: https://github.com/apache/apisix/pull/4702#discussion_r679059871



##########
File path: apisix/discovery/nacos.lua
##########
@@ -211,7 +272,7 @@ local function get_base_uri()
         url = url .. '/'
     end
 
-    return url, username, password
+    return url, username, password, access_key, secret_key, authorization_type

Review comment:
       What about returning:
   ```
   url, authorization_type, username, password, access_key, secret_key
   or
   url, authorization_type, access_key, secret_key
   ```

##########
File path: apisix/discovery/nacos.lua
##########
@@ -92,14 +105,59 @@ local function discovery_nacos_callback(data, event, source, pid)
                ", application: ", core.json.encode(applications, true))
 end
 
-local function request(request_uri, path, body, method, basic_auth)
+local function get_aliyun_ram_sign_headers(param_values, access_key, secret_key)
+    if not param_values then
+        return {}
+    end
+
+    local namespace_id = param_values['namespace_id']
+    local group_name = param_values['group_name']
+    local time_ngx = ngx.utctime()
+    local time_change = string.gsub(time_ngx, " ", "T")
+    local time_utc = table.concat({time_change, "Z"})

Review comment:
       Why not use `time_change .. "Z"` directly?

##########
File path: docs/en/latest/discovery/nacos.md
##########
@@ -38,6 +38,10 @@ discovery:
   nacos:
     host:
       - "http://${username}:${password}@${host1}:${port1}"
+    authorization:
+      type: "basic_auth"  # default basic_auth
+      access_key: ""      # default empty

Review comment:
       We can omit the key for this auth type.
   BTW, basic_auth makes me think about this: https://developer.mozilla.org/en-US/docs/Web/HTTP/Authentication. Is there a better name?




-- 
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] zll2 closed pull request #4702: feat: support aliyun mse nacos ram

Posted by GitBox <gi...@apache.org>.
zll2 closed pull request #4702:
URL: https://github.com/apache/apisix/pull/4702


   


-- 
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] zll2 commented on a change in pull request #4702: feat: support aliyun mse nacos ram

Posted by GitBox <gi...@apache.org>.
zll2 commented on a change in pull request #4702:
URL: https://github.com/apache/apisix/pull/4702#discussion_r679082183



##########
File path: docs/en/latest/discovery/nacos.md
##########
@@ -38,6 +38,10 @@ discovery:
   nacos:
     host:
       - "http://${username}:${password}@${host1}:${port1}"
+    authorization:
+      type: "basic_auth"  # default basic_auth
+      access_key: ""      # default empty

Review comment:
       basic_auth 是原先 nacos.lua 里的分支判断，enreka 是使用了该判断，我看了下实际是不需要，就给去掉了；为了区分和 阿里云 MSE Nacos 的认证鉴权类型区分，然后就拿 basic_auth 作为默认的 type 了；开源版的 Nacos 自建的话，可以使用 用户名 及 密码进行加强安全验证，也可以不使用，但 阿里云 MSE Nacos 的商业版本在安全认证的方面仅支持 AccessKey & Access Secret 进行 OpenAPI 交互；
   
   我看 APISIX 插件中的也是叫 basic—auth，你有更好的建议吗？




-- 
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] zll2 commented on a change in pull request #4702: feat: support aliyun mse nacos ram

Posted by GitBox <gi...@apache.org>.
zll2 commented on a change in pull request #4702:
URL: https://github.com/apache/apisix/pull/4702#discussion_r679076879



##########
File path: apisix/discovery/nacos.lua
##########
@@ -92,14 +105,59 @@ local function discovery_nacos_callback(data, event, source, pid)
                ", application: ", core.json.encode(applications, true))
 end
 
-local function request(request_uri, path, body, method, basic_auth)
+local function get_aliyun_ram_sign_headers(param_values, access_key, secret_key)
+    if not param_values then
+        return {}
+    end
+
+    local namespace_id = param_values['namespace_id']
+    local group_name = param_values['group_name']
+    local time_ngx = ngx.utctime()
+    local time_change = string.gsub(time_ngx, " ", "T")
+    local time_utc = table.concat({time_change, "Z"})

Review comment:
       哈哈哈，第一次写 Lua，有些语法和函数上就不是特别了解

##########
File path: apisix/discovery/nacos.lua
##########
@@ -92,14 +105,59 @@ local function discovery_nacos_callback(data, event, source, pid)
                ", application: ", core.json.encode(applications, true))
 end
 
-local function request(request_uri, path, body, method, basic_auth)
+local function get_aliyun_ram_sign_headers(param_values, access_key, secret_key)
+    if not param_values then
+        return {}
+    end
+
+    local namespace_id = param_values['namespace_id']
+    local group_name = param_values['group_name']
+    local time_ngx = ngx.utctime()
+    local time_change = string.gsub(time_ngx, " ", "T")
+    local time_utc = table.concat({time_change, "Z"})

Review comment:
       我改改掉




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