AlinsRan commented on code in PR #13886:
URL: https://github.com/apache/apisix/pull/13886#discussion_r3891314840
##########
apisix/admin/config_validate.lua:
##########
@@ -125,6 +146,15 @@ function _M.validate_configuration(req_body,
collect_all_errors)
local is_valid = true
local validation_results = {}
+ local shape_ok, shape_err = core.schema.check(config_schema, req_body)
Review Comment:
Confirmed and fixed in a9ea603. `yaml.load("~")` returns lyaml's sentinel,
which is a table with no keys, so it passed the object schema and `update()`
then bumped every `*_conf_version` with no items — a 202 that wiped the
configuration. JSON `null` was already rejected, so the two are consistent now:
```lua
if not ok or type(result) ~= "table" or result == yaml.null then
err = "invalid yaml request body"
```
Applied to both the PUT path and `/configs/validate`. Covered by
standalone.t TEST 25 (and TEST 27 asserts the digest is untouched).
##########
apisix/admin/init.lua:
##########
@@ -554,12 +563,16 @@ function _M.init_worker()
events = require("apisix.events")
events:register(reload_plugins, reload_event, "PUT")
- if plugins_conf_ver_dict and not is_yaml_config_provider then
+ if plugins_conf_ver_dict then
Review Comment:
Confirmed and fixed in a9ea603. The two paths use different events:
`/v1/plugins/reload` posts `control-api-plugin-reload`, handled in
`control/router.lua`, which cannot reach `applied_plugins_conf_version` here —
so the timer saw the bumped version and reloaded again within 1s. This module
now acknowledges that event:
```lua
events:register(ack_plugins_reload,
require("apisix.control.v1").RELOAD_EVENT, "PUT")
```
It only records the version; the load stays in the control handler. A worker
that missed the broadcast misses the ack too, so the timer still reconciles it.
Note this predates the PR — the timer already ran in etcd mode — the fix covers
both.
##########
apisix/admin/standalone.lua:
##########
@@ -100,15 +103,19 @@ local function update(ctx)
-- parse the request body
local data
if core.string.has_prefix(content_type, "application/yaml") then
- data = yaml.load(req_body, { all = false })
- if not data or type(data) ~= "table" then
+ -- yaml.load raises on a malformed document, it does not return an
error
+ local ok, result = pcall(yaml.load, req_body, { all = false })
+ if not ok or type(result) ~= "table" then
Review Comment:
Added, standalone.t TEST 24 (`PUT`) and TEST 26 (`/configs/validate`), both
with `Content-Type: application/yaml` and a `routes: [` body, asserting 400 and
`invalid yaml request body`.
##########
apisix/cli/file.lua:
##########
@@ -266,7 +266,9 @@ function _M.read_yaml_conf(apisix_home)
if not is_empty_file then
local user_conf = yaml.load(user_conf_yaml)
- if not user_conf then
+ -- lyaml returns a scalar for a document such as `foo`, which would
blow
+ -- up in resolve_conf_var's pairs() below
+ if type(user_conf) ~= "table" then
Review Comment:
Added to t/cli/test_deployment_null_sections.sh: a `config.yaml` containing
`just-a-scalar`, asserting `invalid config.yaml file` is reported and that no
`bad argument` appears. That is the actual pre-fix error — `lyaml` returns the
scalar as a string and `resolve_conf_var` calls `pairs()` on it — not an index
error.
--
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]
