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


##########
apisix/admin/kms.lua:
##########
@@ -0,0 +1,207 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+local core = require("apisix.core")
+local utils = require("apisix.admin.utils")
+local type = type
+local tostring = tostring
+
+
+local _M = {
+    need_v3_filter = true,
+}
+
+
+local function check_conf(id, conf, need_id, typ)
+    if not conf then
+        return nil, {error_msg = "missing configurations"}
+    end
+
+    id = id or conf.id
+    if need_id and not id then
+        return nil, {error_msg = "missing id"}
+    end
+
+    if not need_id and id then
+        return nil, {error_msg = "wrong id, do not need it"}
+    end
+
+    if need_id and conf.id and tostring(conf.id) ~= tostring(id) then
+        return nil, {error_msg = "wrong id"}
+    end
+
+    conf.id = id
+
+    core.log.info("conf: ", core.json.delay_encode(conf))
+    local ok, err = core.schema.check(core.schema["kms_" .. typ], conf)
+    if not ok then
+        return nil, {error_msg = "invalid configuration: " .. err}
+    end
+
+    return true
+end
+
+
+local function split_typ_and_id(id, sub_path)
+    local uri_segs = core.utils.split_uri(sub_path)
+    local typ = id
+    local id = nil
+    if #uri_segs > 0 then
+        id = uri_segs[1]
+    end
+    return typ, id
+end
+
+
+function _M.put(id, conf, sub_path)
+    local typ, id = split_typ_and_id(id, sub_path)
+    if not id then
+        return 400, {error_msg = "no kms id in uri"}
+    end
+
+    local ok, err = check_conf(typ .. "/" .. id, conf, true, typ)
+    if not ok then
+        return 400, {error_msg = err}
+    end
+
+    local key = "/kms/" .. typ .. "/" .. id
+
+    local ok, err = utils.inject_conf_with_prev_conf("kms", key, conf)
+    if not ok then
+        return 503, {error_msg = err}
+    end
+
+    local res, err = core.etcd.set(key, conf)
+    if not res then
+        core.log.error("failed to put kms [", key, "]: ", err)
+        return 503, {error_msg = err}
+    end
+
+    return res.status, res.body
+end
+
+
+function _M.get(id, conf, sub_path)
+    local typ, id = split_typ_and_id(id, sub_path)
+
+    local key = "/kms/"
+    if id then
+        key = key .. typ
+        key = key .. "/" .. id
+    end
+
+    local res, err = core.etcd.get(key, not id)
+    if not res then
+        core.log.error("failed to get kms [", key, "]: ", err)
+        return 503, {error_msg = err}
+    end
+
+    utils.fix_count(res.body, id)
+    return res.status, res.body
+end
+
+
+function _M.delete(id, conf, sub_path)
+    local typ, id = split_typ_and_id(id, sub_path)
+    if not id then
+        return 400, {error_msg = "no kms id in uri"}
+    end
+
+    if not id then

Review Comment:
   Why check the id twice?



##########
docs/en/latest/admin-api.md:
##########
@@ -1119,3 +1120,65 @@ Route used in the [Stream Proxy](./stream-proxy.md).
 To learn more about filtering in stream proxies, check 
[this](./stream-proxy.md#more-route-match-options) document.
 
 [Back to TOC](#table-of-contents)
+
+## kms
+
+**API**: /apisix/admin/kms/{secretmanager}/{id}
+
+kms means `Secrets Management`, which could use any secret manager supported, 
e.g. `vault`.
+
+### Request Methods
+
+| Method | Request URI                        | Request Body | Description     
                                  |
+| ------ | ---------------------------------- | ------------ | 
------------------------------------------------- |
+| GET    | /apisix/admin/kms            | NULL         | Fetches a list of all 
kms.                  |
+| GET    | /apisix/admin/kms/{secretmanager}/{id} | NULL         | Fetches 
specified kms by id.           |
+| PUT    | /apisix/admin/kms/{secretmanager}            | {...}        | 
Create new kms configuration.                              |
+| DELETE | /apisix/admin/kms/{secretmanager}/{id} | NULL         | Removes the 
kms with the specified id. |
+| PATCH  | /apisix/admin/kms/{secretmanager}/{id}        | {...}        | 
Updates the selected attributes of the specified, existing kms. To delete an 
attribute, set value of attribute set to null. |
+| PATCH  | /apisix/admin/kms/{secretmanager}/{id}/{path} | {...}        | 
Updates the attribute specified in the path. The values of other attributes 
remain unchanged.                                 |
+
+### Request Body Parameters
+
+When `{secretmanager}` is `vault`:
+
+| Parameter   | Required | Type        | Description                           
                                                                             | 
Example                                          |
+| ----------- | -------- | ----------- | 
------------------------------------------------------------------------------------------------------------------
 | ------------------------------------------------ |
+| uri    | True     | URI        | URI of the vault server.                    
                                                                          |     
                                             |
+| prefix    | True    | string        | key prefix
+| token     | True    | string      | vault token. |                           
                       |
+| desc        | False    | Auxiliary   | Description of usage scenarios.       
                                                                             | 
kms xxxx                                    |
+| labels      | False    | Match Rules | Attributes of the kms specified as 
key-value pairs.                                                           | 
{"version":"v2","build":"16","env":"production"} |

Review Comment:
   Does kms have these fields?



##########
apisix/admin/kms.lua:
##########
@@ -0,0 +1,207 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one or more
+-- contributor license agreements.  See the NOTICE file distributed with
+-- this work for additional information regarding copyright ownership.
+-- The ASF licenses this file to You under the Apache License, Version 2.0
+-- (the "License"); you may not use this file except in compliance with
+-- the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+local core = require("apisix.core")
+local utils = require("apisix.admin.utils")
+local type = type
+local tostring = tostring
+
+
+local _M = {
+    need_v3_filter = true,
+}
+
+
+local function check_conf(id, conf, need_id, typ)
+    if not conf then
+        return nil, {error_msg = "missing configurations"}
+    end
+
+    id = id or conf.id
+    if need_id and not id then
+        return nil, {error_msg = "missing id"}
+    end
+
+    if not need_id and id then
+        return nil, {error_msg = "wrong id, do not need it"}
+    end
+
+    if need_id and conf.id and tostring(conf.id) ~= tostring(id) then
+        return nil, {error_msg = "wrong id"}
+    end
+
+    conf.id = id
+
+    core.log.info("conf: ", core.json.delay_encode(conf))
+    local ok, err = core.schema.check(core.schema["kms_" .. typ], conf)
+    if not ok then
+        return nil, {error_msg = "invalid configuration: " .. err}
+    end
+
+    return true
+end
+
+
+local function split_typ_and_id(id, sub_path)
+    local uri_segs = core.utils.split_uri(sub_path)
+    local typ = id
+    local id = nil
+    if #uri_segs > 0 then
+        id = uri_segs[1]
+    end
+    return typ, id
+end
+
+
+function _M.put(id, conf, sub_path)
+    local typ, id = split_typ_and_id(id, sub_path)
+    if not id then
+        return 400, {error_msg = "no kms id in uri"}
+    end
+
+    local ok, err = check_conf(typ .. "/" .. id, conf, true, typ)
+    if not ok then
+        return 400, {error_msg = err}

Review Comment:
   The err from check_conf already contains error_msg field.



##########
docs/en/latest/admin-api.md:
##########
@@ -1119,3 +1120,65 @@ Route used in the [Stream Proxy](./stream-proxy.md).
 To learn more about filtering in stream proxies, check 
[this](./stream-proxy.md#more-route-match-options) document.
 
 [Back to TOC](#table-of-contents)
+
+## kms
+
+**API**: /apisix/admin/kms/{secretmanager}/{id}
+
+kms means `Secrets Management`, which could use any secret manager supported, 
e.g. `vault`.

Review Comment:
   I am curious why kms means `Secrets Management`. `kms` is not the 
abbreviation of Secrets Management.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to