Copilot commented on code in PR #13519:
URL: https://github.com/apache/apisix/pull/13519#discussion_r3396203740
##########
apisix/secret/aws.lua:
##########
@@ -93,47 +95,68 @@ local function make_request_to_aws(conf, key)
end
if res.status ~= 200 then
+ local err_type = type(res.body) == "table" and res.body.__type
+ local not_found = type(err_type) == "string"
+ and find(err_type, "ResourceNotFoundException") ~=
nil
+
local data = core.json.encode(res.body)
if data then
- return nil, "invalid status code " .. res.status .. ", " .. data
+ return nil, "invalid status code " .. res.status .. ", " .. data,
not_found
end
- return nil, "invalid status code " .. res.status
+ return nil, "invalid status code " .. res.status, not_found
end
return res.body.SecretString
end
--- key is the aws secretId
+-- key is the aws secretId, optionally followed by a JSON field name.
+-- As AWS secret names may contain slashes, the boundary between the secret
+-- name and the field name is ambiguous. Try the longest possible secret name
+-- first, then on ResourceNotFound move path segments from the right into the
+-- field position, e.g. for "a/b/c": ("a/b/c"), ("a/b", "c"), ("a", "b/c").
function _M.get(conf, key)
core.log.info("fetching data from aws for key: ", key)
- local idx = find(key, '/')
-
- local main_key = idx and sub(key, 1, idx - 1) or key
- if main_key == "" then
+ if key == "" or find(key, '/') == 1 then
return nil, "can't find main key, key: " .. key
end
- local sub_key = idx and sub(key, idx + 1) or nil
+ local main_key = key
+ local sub_key
+ local last_err
+ while true do
+ core.log.info("main: ", main_key, sub_key and ", sub: " .. sub_key or
"")
- core.log.info("main: ", main_key, sub_key and ", sub: " .. sub_key or "")
+ local res, err, not_found = make_request_to_aws(conf, main_key)
+ if res then
+ if not sub_key then
+ return res
+ end
- local res, err = make_request_to_aws(conf, main_key)
- if not res then
- return nil, "failed to retrtive data from aws secret manager: " .. err
- end
+ local data, err = core.json.decode(res)
+ if not data then
+ return nil, "failed to decode result, res: " .. res .. ", err:
" .. err
+ end
- if not sub_key then
- return res
- end
+ return data[sub_key]
+ end
+
+ last_err = err
+ if not not_found then
+ break
+ end
+
+ local idx = rfind_char(main_key, '/')
+ if not idx then
+ break
+ end
- local data, err = core.json.decode(res)
- if not data then
- return nil, "failed to decode result, res: " .. res .. ", err: " .. err
+ main_key = sub(key, 1, idx - 1)
+ sub_key = sub(key, idx + 1)
end
- return data[sub_key]
+ return nil, "failed to retrtive data from aws secret manager: " .. last_err
Review Comment:
User-facing error message has a typo: "retrtive" should be "retrieve".
##########
docs/zh/latest/terminology/secret.md:
##########
@@ -227,6 +227,8 @@ $secret://$manager/$id/$secret_name/$key
- secret_name: 密钥管理服务中的密钥名称
- key：当密钥的值是 JSON 字符串时，获取某个属性的值
+注意：AWS Secrets Manager 中的密钥名称本身可以包含斜杠（例如 `john/secret`），因此 `secret_name` 与
`key` 之间的边界存在歧义。APISIX 会优先尝试最长的密钥名称：先将剩余路径整体作为密钥名称查询，如果返回
`ResourceNotFound`，则从右侧逐段将路径移入 `key` 位置，直到查询成功。例如
`$secret://aws/1/john/secret/john-key-auth` 会先尝试名为 `john/secret/john-key-auth`
的密钥，再尝试名为 `john/secret` 的密钥并取其中的 `john-key-auth` 字段，最后尝试名为 `john` 的密钥并取其中的
`secret/john-key-auth` 字段。当存在多种可能的解释时，最长匹配的密钥名称优先。
Review Comment:
这里写的是 `ResourceNotFound`，但 AWS Secrets Manager
的错误类型（且代码里检查的也是）`ResourceNotFoundException`。用准确的错误名更方便用户对照 AWS 返回/日志理解行为。
##########
apisix/secret/aws.lua:
##########
@@ -93,47 +95,68 @@ local function make_request_to_aws(conf, key)
end
if res.status ~= 200 then
+ local err_type = type(res.body) == "table" and res.body.__type
+ local not_found = type(err_type) == "string"
+ and find(err_type, "ResourceNotFoundException") ~=
nil
+
local data = core.json.encode(res.body)
if data then
- return nil, "invalid status code " .. res.status .. ", " .. data
+ return nil, "invalid status code " .. res.status .. ", " .. data,
not_found
end
- return nil, "invalid status code " .. res.status
+ return nil, "invalid status code " .. res.status, not_found
end
return res.body.SecretString
end
--- key is the aws secretId
+-- key is the aws secretId, optionally followed by a JSON field name.
+-- As AWS secret names may contain slashes, the boundary between the secret
+-- name and the field name is ambiguous. Try the longest possible secret name
+-- first, then on ResourceNotFound move path segments from the right into the
+-- field position, e.g. for "a/b/c": ("a/b/c"), ("a/b", "c"), ("a", "b/c").
Review Comment:
The comment describes the fallback as happening on `ResourceNotFound`, but
the implementation checks for `ResourceNotFoundException`. Using the exact AWS
error name here avoids confusion for readers/debugging.
##########
docs/en/latest/terminology/secret.md:
##########
@@ -224,6 +224,8 @@ $secret://$manager/$id/$secret_name/$key
- secret_name: the secret name in the secrets management service
- key: get the value of a property when the value of the secret is a JSON
string
+Note that the secret name in AWS Secrets Manager may itself contain slashes
(e.g. `john/secret`), which makes the boundary between `secret_name` and `key`
ambiguous. APISIX resolves the reference by trying the longest possible secret
name first: it treats the whole remaining path as the secret name, and on
`ResourceNotFound` it moves path segments from the right into the `key`
position until the lookup succeeds. For example,
`$secret://aws/1/john/secret/john-key-auth` tries the secret named
`john/secret/john-key-auth` first, then the secret `john/secret` with key
`john-key-auth`, and finally the secret `john` with key `secret/john-key-auth`.
When multiple interpretations exist, the longest matching secret name takes
precedence.
Review Comment:
This note refers to `ResourceNotFound`, but AWS Secrets Manager’s error type
(and the implementation check) is `ResourceNotFoundException`. Using the exact
name makes it easier to correlate behavior with AWS responses/logs.
--
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]
