Re: [PR] feat(admin): make /configs/validate available in all modes [apisix]
nic-6443 merged PR #13220: URL: https://github.com/apache/apisix/pull/13220 -- 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(admin): make /configs/validate available in all modes [apisix]
nic-6443 commented on code in PR #13220:
URL: https://github.com/apache/apisix/pull/13220#discussion_r3076987018
##
apisix/admin/config_validate.lua:
##
@@ -0,0 +1,237 @@
+--
+-- 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.
+--
+
+--- Batch configuration validation module.
+-- Validates APISIX declarative configurations (routes, services, consumers,
etc.)
+-- including resource-level JSON Schema and plugin check_schema() advanced
validation.
+-- Used by both standalone mode and etcd mode via POST
/apisix/admin/configs/validate.
+
+local type = type
+local pairs= pairs
+local ipairs = ipairs
+local tostring = tostring
+local pcall= pcall
+local str_find = string.find
+local str_sub = string.sub
+local table_insert = table.insert
+local yaml = require("lyaml")
+local core = require("apisix.core")
+local tbl_deepcopy = require("apisix.core.table").deepcopy
+local constants= require("apisix.constants")
+
+local _M = {}
+
+local resources = {
+routes = require("apisix.admin.routes"),
+services= require("apisix.admin.services"),
+upstreams = require("apisix.admin.upstreams"),
+consumers = require("apisix.admin.consumers"),
+credentials = require("apisix.admin.credentials"),
+schema = require("apisix.admin.schema"),
+ssls= require("apisix.admin.ssl"),
+plugins = require("apisix.admin.plugins"),
+protos = require("apisix.admin.proto"),
+global_rules= require("apisix.admin.global_rules"),
+stream_routes = require("apisix.admin.stream_routes"),
+plugin_metadata = require("apisix.admin.plugin_metadata"),
+plugin_configs = require("apisix.admin.plugin_config"),
+consumer_groups = require("apisix.admin.consumer_group"),
+secrets = require("apisix.admin.secrets"),
+}
+
+local CONF_VERSION_KEY_SUFFIX = "_conf_version"
+local ALL_RESOURCE_KEYS = {}
+for dir in pairs(constants.HTTP_ETCD_DIRECTORY) do
+local key = str_sub(dir, 2)
+ALL_RESOURCE_KEYS[key] = key .. CONF_VERSION_KEY_SUFFIX
+end
+for dir in pairs(constants.STREAM_ETCD_DIRECTORY) do
+local key = str_sub(dir, 2)
+ALL_RESOURCE_KEYS[key] = key .. CONF_VERSION_KEY_SUFFIX
+end
+
+
+local function check_duplicate(item, key, id_set)
+local identifier, identifier_type
+if key == "consumers" then
+identifier = item.id or item.username
+identifier_type = item.id and "credential id" or "username"
+else
+identifier = item.id
+identifier_type = "id"
+end
+
+if id_set[identifier] then
+return true, "found duplicate " .. identifier_type .. " " ..
identifier .. " in " .. key
+end
+id_set[identifier] = true
+return false
Review Comment:
Fixed. Added a nil guard — `check_duplicate` now returns `false` early when
identifier is nil, so resources without `id` skip duplicate checking instead of
crashing.
##
apisix/admin/config_validate.lua:
##
@@ -0,0 +1,237 @@
+--
+-- 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.
+--
+
+--- Batch configuration validation module.
+-- Validates APISIX declarative configurations (routes, services, consumers,
etc.)
+-- including resource-level JSON Schema and plugin check_schema() advanced
validation.
+-- Used by both standalone mode and etcd mode via POST
/apisix/admin/configs/validate.
+
+local type = type
+local
Re: [PR] feat(admin): make /configs/validate available in all modes [apisix]
nic-6443 commented on code in PR #13220:
URL: https://github.com/apache/apisix/pull/13220#discussion_r3076987559
##
apisix/admin/init.lua:
##
@@ -423,6 +424,12 @@ local function standalone_run()
end
+local function validate_configs()
+set_ctx_and_check_token()
+return config_validate.validate()
Review Comment:
Fixed — removed the unused `ctx` parameter from `validate()`. Both call
sites (standalone.lua and init.lua) now call `config_validate.validate()` with
no args.
##
apisix/admin/config_validate.lua:
##
@@ -0,0 +1,237 @@
+--
+-- 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.
+--
+
+--- Batch configuration validation module.
+-- Validates APISIX declarative configurations (routes, services, consumers,
etc.)
+-- including resource-level JSON Schema and plugin check_schema() advanced
validation.
+-- Used by both standalone mode and etcd mode via POST
/apisix/admin/configs/validate.
+
+local type = type
+local pairs= pairs
+local ipairs = ipairs
+local tostring = tostring
+local pcall= pcall
+local str_find = string.find
+local str_sub = string.sub
+local table_insert = table.insert
+local yaml = require("lyaml")
+local core = require("apisix.core")
+local tbl_deepcopy = require("apisix.core.table").deepcopy
+local constants= require("apisix.constants")
+
+local _M = {}
+
+local resources = {
+routes = require("apisix.admin.routes"),
+services= require("apisix.admin.services"),
+upstreams = require("apisix.admin.upstreams"),
+consumers = require("apisix.admin.consumers"),
+credentials = require("apisix.admin.credentials"),
+schema = require("apisix.admin.schema"),
+ssls= require("apisix.admin.ssl"),
+plugins = require("apisix.admin.plugins"),
+protos = require("apisix.admin.proto"),
+global_rules= require("apisix.admin.global_rules"),
+stream_routes = require("apisix.admin.stream_routes"),
+plugin_metadata = require("apisix.admin.plugin_metadata"),
+plugin_configs = require("apisix.admin.plugin_config"),
+consumer_groups = require("apisix.admin.consumer_group"),
+secrets = require("apisix.admin.secrets"),
+}
+
+local CONF_VERSION_KEY_SUFFIX = "_conf_version"
+local ALL_RESOURCE_KEYS = {}
+for dir in pairs(constants.HTTP_ETCD_DIRECTORY) do
+local key = str_sub(dir, 2)
+ALL_RESOURCE_KEYS[key] = key .. CONF_VERSION_KEY_SUFFIX
+end
+for dir in pairs(constants.STREAM_ETCD_DIRECTORY) do
+local key = str_sub(dir, 2)
+ALL_RESOURCE_KEYS[key] = key .. CONF_VERSION_KEY_SUFFIX
+end
+
+
+local function check_duplicate(item, key, id_set)
+local identifier, identifier_type
+if key == "consumers" then
+identifier = item.id or item.username
+identifier_type = item.id and "credential id" or "username"
+else
+identifier = item.id
+identifier_type = "id"
+end
+
+if id_set[identifier] then
+return true, "found duplicate " .. identifier_type .. " " ..
identifier .. " in " .. key
+end
+id_set[identifier] = true
+return false
+end
+
+
+local function check_conf(checker, schema, item, typ)
+if not checker then
+return true
+end
+local str_id = tostring(item.id)
+if typ == "consumers" and
+core.string.find(str_id, "/credentials/") then
+local credential_checker = resources.credentials.checker
+local credential_schema = resources.credentials.schema
+return credential_checker(item.id, item, false, credential_schema, {
+skip_references_check = true,
+})
+end
+
+local secret_type
+if typ == "secrets" then
+local idx = str_find(str_id or "", "/")
+if not idx then
+return false, {
+error_msg = "invalid secret id: " .. (str_id or "")
+}
+end
+secret_type = str_sub(str_id, 1, idx - 1)
+end
+return checker(item.id, item, false, schema, {
+secret_type = secret_type,
+skip_references_check = true,
+})
+end
+
+
+function _M.validate_configuration(req_body, collect_
Re: [PR] feat(admin): make /configs/validate available in all modes [apisix]
Copilot commented on code in PR #13220:
URL: https://github.com/apache/apisix/pull/13220#discussion_r3076950830
##
apisix/admin/config_validate.lua:
##
@@ -0,0 +1,237 @@
+--
+-- 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.
+--
+
+--- Batch configuration validation module.
+-- Validates APISIX declarative configurations (routes, services, consumers,
etc.)
+-- including resource-level JSON Schema and plugin check_schema() advanced
validation.
+-- Used by both standalone mode and etcd mode via POST
/apisix/admin/configs/validate.
+
+local type = type
+local pairs= pairs
+local ipairs = ipairs
+local tostring = tostring
+local pcall= pcall
+local str_find = string.find
+local str_sub = string.sub
+local table_insert = table.insert
+local yaml = require("lyaml")
+local core = require("apisix.core")
+local tbl_deepcopy = require("apisix.core.table").deepcopy
+local constants= require("apisix.constants")
+
+local _M = {}
+
+local resources = {
+routes = require("apisix.admin.routes"),
+services= require("apisix.admin.services"),
+upstreams = require("apisix.admin.upstreams"),
+consumers = require("apisix.admin.consumers"),
+credentials = require("apisix.admin.credentials"),
+schema = require("apisix.admin.schema"),
+ssls= require("apisix.admin.ssl"),
+plugins = require("apisix.admin.plugins"),
+protos = require("apisix.admin.proto"),
+global_rules= require("apisix.admin.global_rules"),
+stream_routes = require("apisix.admin.stream_routes"),
+plugin_metadata = require("apisix.admin.plugin_metadata"),
+plugin_configs = require("apisix.admin.plugin_config"),
+consumer_groups = require("apisix.admin.consumer_group"),
+secrets = require("apisix.admin.secrets"),
+}
+
+local CONF_VERSION_KEY_SUFFIX = "_conf_version"
+local ALL_RESOURCE_KEYS = {}
+for dir in pairs(constants.HTTP_ETCD_DIRECTORY) do
+local key = str_sub(dir, 2)
+ALL_RESOURCE_KEYS[key] = key .. CONF_VERSION_KEY_SUFFIX
+end
+for dir in pairs(constants.STREAM_ETCD_DIRECTORY) do
+local key = str_sub(dir, 2)
+ALL_RESOURCE_KEYS[key] = key .. CONF_VERSION_KEY_SUFFIX
+end
+
+
+local function check_duplicate(item, key, id_set)
+local identifier, identifier_type
+if key == "consumers" then
+identifier = item.id or item.username
+identifier_type = item.id and "credential id" or "username"
+else
+identifier = item.id
+identifier_type = "id"
+end
+
+if id_set[identifier] then
+return true, "found duplicate " .. identifier_type .. " " ..
identifier .. " in " .. key
+end
+id_set[identifier] = true
+return false
Review Comment:
`check_duplicate()` indexes `id_set[identifier]` where `identifier` can be
`nil` for resource types whose schema doesn't require `id` (e.g.,
routes/services in declarative config). In Lua this raises `table index is
nil`, turning an otherwise-valid payload into a 500. Please guard against
missing identifiers (skip duplicate checking when there is no stable key, or
derive one per resource type) so validation always returns a 4xx with a useful
error instead of crashing.
##
apisix/admin/config_validate.lua:
##
@@ -0,0 +1,237 @@
+--
+-- 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.
+--
+
+--- Batch configuration validat
