Re: [PR] feat: enhance encrypt_fields to support nested structures [apisix]
Baoyuantop merged PR #13192: URL: https://github.com/apache/apisix/pull/13192 -- 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]
Re: [PR] feat: enhance encrypt_fields to support nested structures [apisix]
nic-6443 commented on code in PR #13192:
URL: https://github.com/apache/apisix/pull/13192#discussion_r3057714480
##
apisix/plugin.lua:
##
@@ -988,6 +988,88 @@ local function get_plugin_schema_for_gde(name, schema_type)
end
+-- Process a single encrypt_field path on the given config table.
+-- Supports:
+-- - Arbitrary depth dotted paths (e.g., "a.b.c.d")
+-- - Array traversal at intermediate nodes (iterate each element)
+-- - Leaf type dispatch: string, array of strings, map of strings
+local function process_encrypt_field(conf, key_path, operation, plugin_name,
op_name)
+local dot_pos = core.string.find(key_path, ".")
Review Comment:
`core.string.find` is APISIX's wrapper around `string.find` that always
passes `true` as the 4th argument (plain matching mode). See
`apisix/core/string.lua`:
```lua
function _M.find(haystack, needle, from)
return str_find(haystack, needle, from or 1, true)
end
```
So `"."` is already treated as a literal dot, not a pattern. No change
needed.
##
apisix/plugin.lua:
##
@@ -988,6 +988,88 @@ local function get_plugin_schema_for_gde(name, schema_type)
end
+-- Process a single encrypt_field path on the given config table.
+-- Supports:
+-- - Arbitrary depth dotted paths (e.g., "a.b.c.d")
+-- - Array traversal at intermediate nodes (iterate each element)
+-- - Leaf type dispatch: string, array of strings, map of strings
+local function process_encrypt_field(conf, key_path, operation, plugin_name,
op_name)
+local dot_pos = core.string.find(key_path, ".")
+
+if not dot_pos then
+-- leaf segment
+local val = conf[key_path]
+if val == nil then
+return
+end
+
+if type(val) == "string" then
+local result, err = operation(val, "data_encrypt")
+if not result then
+core.log.warn("failed to ", op_name, " the conf of plugin [",
+ plugin_name, "] key [", key_path, "], err: ",
err)
+else
+conf[key_path] = result
+end
+
+elseif type(val) == "table" then
+if core.table.isarray(val) then
+-- array of strings
+for i, item in ipairs(val) do
+if type(item) == "string" then
+local result, err = operation(item, "data_encrypt")
+if not result then
+core.log.warn("failed to ", op_name, " the conf of
plugin [",
+ plugin_name, "] key [", key_path,
+ "] index [", i, "], err: ", err)
+else
+val[i] = result
+end
+end
+end
+else
+-- map of strings
+for k, v in pairs(val) do
+if type(v) == "string" then
+local result, err = operation(v, "data_encrypt")
+if not result then
+core.log.warn("failed to ", op_name, " the conf of
plugin [",
+ plugin_name, "] key [", key_path,
+ ".", k, "], err: ", err)
+else
+val[k] = result
+end
+end
+end
+end
+end
+
+else
+-- intermediate segment: split on first dot and recurse
+local segment = key_path:sub(1, dot_pos - 1)
+local rest = key_path:sub(dot_pos + 1)
+local val = conf[segment]
+
+if val == nil or type(val) ~= "table" then
+return
+end
+
+if core.table.isarray(val) then
+-- array: iterate each element and recurse
+for _, item in ipairs(val) do
+if type(item) == "table" then
+process_encrypt_field(item, rest, operation, plugin_name,
op_name)
+end
+end
+else
+-- map: recurse into it
+process_encrypt_field(val, rest, operation, plugin_name, op_name)
+end
+end
+end
+_M.process_encrypt_field = process_encrypt_field
Review Comment:
This is intentionally exported for testing — `t/node/data_encrypt3.t` TEST 7
calls it directly to test array-of-strings leaf encryption without needing to
set up a full plugin with that schema shape. This follows the same pattern used
elsewhere in the codebase for testing internal helpers. The underscore prefix
convention (`_M._process_encrypt_field`) is not used in this codebase.
##
t/node/data_encrypt3.t:
##
@@ -0,0 +1,537 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
Re: [PR] feat: enhance encrypt_fields to support nested structures [apisix]
Copilot commented on code in PR #13192:
URL: https://github.com/apache/apisix/pull/13192#discussion_r3057707185
##
apisix/plugin.lua:
##
@@ -988,6 +988,88 @@ local function get_plugin_schema_for_gde(name, schema_type)
end
+-- Process a single encrypt_field path on the given config table.
+-- Supports:
+-- - Arbitrary depth dotted paths (e.g., "a.b.c.d")
+-- - Array traversal at intermediate nodes (iterate each element)
+-- - Leaf type dispatch: string, array of strings, map of strings
+local function process_encrypt_field(conf, key_path, operation, plugin_name,
op_name)
+local dot_pos = core.string.find(key_path, ".")
Review Comment:
`core.string.find(key_path, ".")` is very likely treating `"."` as a pattern
(match-any) rather than a literal dot, which would make `dot_pos` non-nil for
almost any non-empty key (breaking flat keys and path splitting). Use a literal
search (e.g., `key_path:find(".", 1, true)` or an escaped pattern like `"%."`)
so dotted-path detection only triggers when a real dot exists.
```suggestion
local dot_pos = key_path:find(".", 1, true)
```
##
apisix/plugin.lua:
##
@@ -988,6 +988,88 @@ local function get_plugin_schema_for_gde(name, schema_type)
end
+-- Process a single encrypt_field path on the given config table.
+-- Supports:
+-- - Arbitrary depth dotted paths (e.g., "a.b.c.d")
+-- - Array traversal at intermediate nodes (iterate each element)
+-- - Leaf type dispatch: string, array of strings, map of strings
+local function process_encrypt_field(conf, key_path, operation, plugin_name,
op_name)
+local dot_pos = core.string.find(key_path, ".")
+
+if not dot_pos then
+-- leaf segment
+local val = conf[key_path]
+if val == nil then
+return
+end
+
+if type(val) == "string" then
+local result, err = operation(val, "data_encrypt")
+if not result then
+core.log.warn("failed to ", op_name, " the conf of plugin [",
+ plugin_name, "] key [", key_path, "], err: ",
err)
+else
+conf[key_path] = result
+end
+
+elseif type(val) == "table" then
+if core.table.isarray(val) then
+-- array of strings
+for i, item in ipairs(val) do
+if type(item) == "string" then
+local result, err = operation(item, "data_encrypt")
+if not result then
+core.log.warn("failed to ", op_name, " the conf of
plugin [",
+ plugin_name, "] key [", key_path,
+ "] index [", i, "], err: ", err)
+else
+val[i] = result
+end
+end
+end
+else
+-- map of strings
+for k, v in pairs(val) do
+if type(v) == "string" then
+local result, err = operation(v, "data_encrypt")
+if not result then
+core.log.warn("failed to ", op_name, " the conf of
plugin [",
+ plugin_name, "] key [", key_path,
+ ".", k, "], err: ", err)
+else
+val[k] = result
+end
+end
+end
+end
+end
+
+else
+-- intermediate segment: split on first dot and recurse
+local segment = key_path:sub(1, dot_pos - 1)
+local rest = key_path:sub(dot_pos + 1)
+local val = conf[segment]
+
+if val == nil or type(val) ~= "table" then
+return
+end
+
+if core.table.isarray(val) then
+-- array: iterate each element and recurse
+for _, item in ipairs(val) do
+if type(item) == "table" then
+process_encrypt_field(item, rest, operation, plugin_name,
op_name)
+end
+end
+else
+-- map: recurse into it
+process_encrypt_field(val, rest, operation, plugin_name, op_name)
+end
+end
+end
+_M.process_encrypt_field = process_encrypt_field
Review Comment:
`process_encrypt_field` is exported on `_M`, which makes it part of the
module’s public API surface (even if it is only intended as an internal
helper). If this isn’t meant to be a supported external API, consider keeping
it local (or clearly marking it internal, e.g., `_M._process_encrypt_field`)
and adjusting `t/node/data_encrypt3.t` to validate behavior through the normal
encrypt/decrypt paths rather than calling the helper directly.
```suggestion
_M._process_encrypt_field = process_encrypt_field
```
