nic-6443 commented on code in PR #13312:
URL: https://github.com/apache/apisix/pull/13312#discussion_r3153153400
##########
apisix/plugin.lua:
##########
@@ -921,7 +982,89 @@ local function check_single_plugin_schema(name,
plugin_conf, schema_type, skip_d
end
if plugin_obj.check_schema then
- local ok, err = plugin_obj.check_schema(plugin_conf, schema_type)
+ local ok, err
+
+ if secret.has_secret_ref(plugin_conf) then
+ -- Strip secret ref fields recursively so they bypass all schema
+ -- constraints (enum, pattern, minLength, maxLength, etc.).
+ -- We deep-copy both conf and schema, remove secret-ref leaves from
+ -- the conf copy, and remove the corresponding property definitions
+ -- and required entries from the schema copy.
+ local conf_copy = core.table.deepcopy(plugin_conf)
+ local schema_raw
+ if schema_type == _M.TYPE_CONSUMER then
+ schema_raw = plugin_obj.consumer_schema or plugin_obj.schema
+ elseif schema_type == _M.TYPE_METADATA then
+ schema_raw = plugin_obj.metadata_schema
+ else
+ schema_raw = plugin_obj.schema
+ end
+ local schema_copy = core.table.deepcopy(schema_raw)
+
+ local function strip_secret_refs(conf, schema)
+ if type(conf) ~= "table" or type(schema) ~= "table" then
+ return
+ end
+
+ local props = schema.properties
+ local removed = {}
+
+ for k, v in pairs(conf) do
+ if type(v) == "string" and secret.is_secret_ref(v) then
+ conf[k] = nil
+ core.table.insert(removed, k)
+ if props then
+ props[k] = nil
+ end
+ elseif type(v) == "table" then
+ local sub_schema = props and props[k]
+ if sub_schema then
+ if sub_schema.type == "object" or
sub_schema.properties then
+ strip_secret_refs(v, sub_schema)
+ elseif sub_schema.type == "array" and
sub_schema.items then
+ for _, item in ipairs(v) do
+ if type(item) == "table" then
+ strip_secret_refs(item,
sub_schema.items)
+ end
+ end
+ end
Review Comment:
Fixed in latest commit — added handling for string array items.
##########
apisix/plugin.lua:
##########
@@ -65,6 +66,50 @@ local merge_global_rule_lrucache = core.lrucache.new({
ttl = 300, count = 512
})
+-- Cache for resolved plugin confs: original_conf -> {resolved, secret_vals}
+-- Weak keys ensure entries are GC'd when original conf is replaced (config
reload)
+local _resolved_cache = setmetatable({}, {__mode = "k"})
+
+local function vals_equal(a, b)
+ if a == b then
+ return true
+ end
+ if not a or not b then
+ return false
+ end
+ for k, v in pairs(a) do
+ if b[k] ~= v then
+ return false
+ end
+ end
+ for k in pairs(b) do
+ if a[k] == nil then
+ return false
+ end
+ end
+ return true
+end
+
+local function resolve_plugin_conf(conf)
+ if not secret.has_secret_ref(conf) then
+ return conf
+ end
+
+ local current_vals = secret.collect_secret_values(conf, true)
+
+ local cached = _resolved_cache[conf]
+ if cached and vals_equal(cached.secret_vals, current_vals) then
+ return cached.resolved
+ end
+
+ local resolved = secret.fetch_secrets(conf, true)
+ if not resolved then
+ return conf
+ end
Review Comment:
Good point — `fetch_secrets` deep-copies and resolves in one go. It returns
nil only if the input itself is nil. Added a nil guard for safety but it would
only trigger if conf was nil (which `has_secret_ref` already prevents).
##########
t/secret/central-secret-refs.t:
##########
@@ -0,0 +1,288 @@
+#
+# 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.
+#
+BEGIN {
+ $ENV{TEST_HOST} = "test.example.com";
+ $ENV{TEST_URI} = "/new-uri";
+ $ENV{TEST_HEADER_VALUE} = "from-env";
+}
+
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: proxy-rewrite resolves $env:// references automatically (central
resolution)
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/hello",
+ "plugins": {
+ "proxy-rewrite": {
+ "host": "$env://TEST_HOST"
+ }
+ },
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "127.0.0.1:1980": 1
+ }
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ return ngx.say(body)
+ end
+ ngx.say("success")
+ }
+ }
+--- response_body
+success
+
+
+
+=== TEST 2: verify host header is resolved from env variable
+--- request
+GET /hello
+--- response_headers
+!X-Test-Error
+--- more_headers
+--- response_body_like
+.*
Review Comment:
This is a smoke test verifying that config with secret refs is accepted and
the request succeeds (resolution worked). Full proxy-rewrite behavior is
covered by existing proxy-rewrite tests.
##########
t/secret/central-secret-refs.t:
##########
@@ -0,0 +1,288 @@
+#
+# 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.
+#
+BEGIN {
+ $ENV{TEST_HOST} = "test.example.com";
+ $ENV{TEST_URI} = "/new-uri";
+ $ENV{TEST_HEADER_VALUE} = "from-env";
+}
+
+use t::APISIX 'no_plan';
+
+repeat_each(1);
+no_long_string();
+no_root_location();
+
+add_block_preprocessor(sub {
+ my ($block) = @_;
+
+ if (!defined $block->request) {
+ $block->set_value("request", "GET /t");
+ }
+});
+
+run_tests;
+
+__DATA__
+
+=== TEST 1: proxy-rewrite resolves $env:// references automatically (central
resolution)
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/hello",
+ "plugins": {
+ "proxy-rewrite": {
+ "host": "$env://TEST_HOST"
+ }
+ },
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "127.0.0.1:1980": 1
+ }
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ return ngx.say(body)
+ end
+ ngx.say("success")
+ }
+ }
+--- response_body
+success
+
+
+
+=== TEST 2: verify host header is resolved from env variable
+--- request
+GET /hello
+--- response_headers
+!X-Test-Error
+--- more_headers
+--- response_body_like
+.*
+--- error_log
+plugin.lua request matched route 1
+
+
+
+=== TEST 3: proxy-rewrite resolves $env:// in uri field with schema constraints
+proxy-rewrite.uri has minLength, maxLength, and pattern constraints.
+The $env://TEST_URI string itself doesn't match the ^/.* pattern,
+but central schema validation strips secret ref fields before checking.
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/hello",
+ "plugins": {
+ "proxy-rewrite": {
+ "uri": "$env://TEST_URI"
+ }
+ },
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "127.0.0.1:1980": 1
+ }
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ return ngx.say(body)
+ end
+ ngx.say("success")
+ }
+ }
+--- response_body
+success
+
+
+
+=== TEST 4: verify uri is resolved from env variable
+--- request
+GET /hello
+--- error_code: 200
+
+
+
+=== TEST 5: schema validation strips nested secret ref fields
+--- config
+ location /t {
+ content_by_lua_block {
+ local t = require("lib.test_admin").test
+ local code, body = t('/apisix/admin/routes/1',
+ ngx.HTTP_PUT,
+ [[{
+ "uri": "/hello",
+ "plugins": {
+ "proxy-rewrite": {
+ "headers": {
+ "set": {
+ "X-Custom": "$env://TEST_HEADER_VALUE"
+ }
+ }
+ }
+ },
+ "upstream": {
+ "type": "roundrobin",
+ "nodes": {
+ "127.0.0.1:1980": 1
+ }
+ }
+ }]]
+ )
+
+ if code >= 300 then
+ ngx.status = code
+ return ngx.say(body)
+ end
+ ngx.say("success")
+ }
+ }
+--- response_body
+success
+
+
+
+=== TEST 6: central resolve_plugin_conf returns stable reference
+--- config
+ location /t {
+ content_by_lua_block {
+ local secret = require("apisix.secret")
+ local plugin = require("apisix.plugin")
+
+ -- access the resolve function via the filter mechanism
+ -- simulate a conf with env ref
+ local conf = {
+ host = "$env://TEST_HOST",
+ uri = "/test"
+ }
+ -- first call: should resolve and cache
+ local resolved1 = secret.fetch_secrets(conf, true)
+ ngx.say("resolved host: ", resolved1.host)
+
+ -- verify the original conf is not mutated
+ ngx.say("original host: ", conf.host)
+ }
Review Comment:
Fixed — tests rewritten in latest commit.
##########
docs/en/latest/terminology/secret.md:
##########
@@ -42,7 +42,13 @@ APISIX currently supports storing secrets in the following
ways:
- [AWS Secrets Manager](#use-aws-secrets-manager-to-manage-secrets)
- [GCP Secrets Manager](#use-gcp-secrets-manager-to-manage-secrets)
-You can use APISIX Secret functions by specifying format variables in the
consumer configuration of the following plugins, such as `key-auth`.
+You can use APISIX Secret functions by specifying format variables in the
plugin configuration of any plugin. Secret references in string fields are
automatically resolved at runtime. For example, you can use `$ENV://MY_API_KEY`
or `$secret://vault/my-secret/key` in any plugin's string configuration field.
+
+:::tip
+
+When a plugin configuration field uses a secret reference like `$secret://...`
or `$env://...`, schema validation constraints (such as `enum`, `pattern`,
`minLength`, `maxLength`) on that field are automatically bypassed during
configuration loading. The actual resolved value is validated at runtime.
Review Comment:
Good catch. Updated the docs to clarify: secret references bypass schema
validation during config loading, and the resolved values are used directly at
runtime without re-validation.
##########
docs/zh/latest/terminology/secret.md:
##########
@@ -42,7 +42,13 @@ APISIX 目前支持通过以下方式存储密钥：
- [AWS Secrets Manager](#使用-aws-secrets-manager-管理密钥)
- [GCP Secrets Manager](#使用-gcp-secrets-manager-管理密钥)
-你可以在以下插件的 consumer 配置中通过指定格式的变量来使用 APISIX Secret 功能，比如 `key-auth` 插件。
+你可以在任何插件的配置中通过指定格式的变量来使用 APISIX Secret
功能。字符串类型的配置字段中的密钥引用会在运行时自动解析。例如，你可以在任何插件的字符串配置字段中使用 `$ENV://MY_API_KEY` 或
`$secret://vault/my-secret/key`。
+
+:::tip
+
+当插件配置字段使用了 `$secret://...` 或 `$env://...` 格式的密钥引用时，该字段上的 schema 校验约束（如
`enum`、`pattern`、`minLength`、`maxLength`）会在配置加载时自动跳过。实际解析后的值会在运行时进行校验。
Review Comment:
Same as the English doc thread — updated to clarify the behavior.
##########
apisix/plugin.lua:
##########
@@ -921,7 +982,89 @@ local function check_single_plugin_schema(name,
plugin_conf, schema_type, skip_d
end
if plugin_obj.check_schema then
- local ok, err = plugin_obj.check_schema(plugin_conf, schema_type)
+ local ok, err
+
+ if secret.has_secret_ref(plugin_conf) then
+ -- Strip secret ref fields recursively so they bypass all schema
+ -- constraints (enum, pattern, minLength, maxLength, etc.).
+ -- We deep-copy both conf and schema, remove secret-ref leaves from
+ -- the conf copy, and remove the corresponding property definitions
+ -- and required entries from the schema copy.
+ local conf_copy = core.table.deepcopy(plugin_conf)
Review Comment:
Intentional design decision. Calling `plugin_obj.check_schema(conf_copy)`
would not work because plugins internally call
`core.schema.check(their_original_schema, conf)` — the stripped conf would fail
on required field checks against the original schema. Custom validation
operates on fields that should never contain secret refs.
--
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]
