bzp2010 commented on code in PR #12603:
URL: https://github.com/apache/apisix/pull/12603#discussion_r2347784236
##########
apisix/admin/resource.lua:
##########
@@ -123,7 +124,12 @@ function _M:check_conf(id, conf, need_id, typ, allow_time)
core.log.info("schema: ", core.json.delay_encode(self.schema))
end
- local ok, err = self.checker(id, conf, need_id, self.schema, {secret_type
= typ})
+ local conf_for_check = tbl_deepcopy(conf)
+ local ok, err = self.checker(id, conf_for_check, need_id, self.schema,
{secret_type = typ})
Review Comment:
I'm wondering if it's necessary to keep a switch enabled by a specific
querystring to retain the old behavior?
For example, when `fillin_default=true`, still fill in the default value?
This may have some meaning for debugging, otherwise we have to rely only on the
correctness of the document. Historical experience tells me that this is
usually difficult.
##########
apisix/admin/plugin_metadata.lua:
##########
@@ -45,13 +56,8 @@ local function check_conf(plugin_name, conf)
return nil, {error_msg = "invalid plugin name"}
end
- if not plugin_object.metadata_schema then
- plugin_object.metadata_schema = {
- type = "object",
- ['$comment'] = injected_mark,
- properties = {},
- }
- end
+ inject_metadata_schema(plugin_object)
Review Comment:
Although it is not introduced by your PR, it is strange to actively inject
an empty schema and use ordinary schema or metadata_schema according to certain
conditions for configuration verification for some non-plugin metadata.
If a plugin does not indicate that it supports plugin metadata configuration
by actively declaring metadata_schema, it should not be configured in this way.
Specifically, if this happens, it may be more appropriate to reject this
configuration directly. For example, key-auth does not have the meaning of
plugin metadata configuration.
--
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]
