AlinsRan opened a new pull request, #13885:
URL: https://github.com/apache/apisix/pull/13885
### Description
Two defects in the data-plane write guard added by #12241.
**1. The guard fails closed on the one case it does not target.**
```lua
local function disable_write_if_data_plane()
local data_plane, err = is_data_plane()
if err then
log.error("failed to check data plane role: ", err)
return true, err -- blocks the write
end
if data_plane then
-- current only warn, will be return false in future releases
log.warn(NOT_ALLOW_WRITE_ETCD_WARN)
return false -- allows the write
end
```
A confirmed `data_plane` role only warns, but a failure to read the local
config blocks every wrapped write. That is backwards, and it is reachable:
`get_etcd_cli()` caches the client in an upvalue, so once the client exists
`fetch_local_conf()` is never called again from `new()` — only from this guard.
If `local_conf()` starts failing afterwards, a control plane with a perfectly
usable cached client stops being able to write, while the role the guard exists
for keeps writing.
The guard is in its warn-only phase, so it now returns `false` there too.
**2. `wrap_etcd_client` errors are dropped.**
```lua
etcd_cli = wrap_etcd_client(etcd_cli)
return etcd_cli, prefix
```
`wrap_etcd_client` returns `nil, "method ... not found in etcd client"` when
the client is missing one of the wrapped methods. The second value is
discarded, so `_new` returns `nil, prefix`, `get_etcd_cli` returns `nil, nil,
nil`, and callers such as `_M.get` do `return nil, err` with `err == nil` — an
undiagnosable failure. The error is now propagated.
Also fixes the six-space indentation in `is_data_plane`.
### Tests
`t/core/etcd-write.t` TEST 34 and TEST 35. Both fail on `master` and pass
with this change:
```
TEST 34 got: second set: failed: mocked local conf failure expected:
second set: ok
TEST 35 got: res: nil, err: nil expected: res:
nil, err: method setnx not found in etcd client
```
The full file (35 tests) passes with the change.
### Checklist
- [x] I have explained the need for this PR and the problem it solves
- [x] I have explained the changes or the new features added to this PR
- [x] I have added tests corresponding to this change
- [x] I have updated the documentation accordingly
- [x] I have verified that the change is backward compatible
--
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]
