From c463ff603b83b90a6a8fd3ea372a108ef1d63504 Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Mon, 20 Jul 2026 15:28:24 +0800 Subject: [PATCH 1/7] fix(watch): do not drop events when a read carries multiple responses read_watch() splits one body read into several watch responses, decodes the events of each of them into all_events, and then attaches all_events to `body`, which points at the last decoded response. The attach was guarded by `#all_events > 1`, so a read holding [response with 1 event, response without events] returned the last response as is and the event was lost, silently and unrecoverably for the caller. Today etcd never produces that layout because the only event-less responses are `created` and `canceled`, which never trail an event response. It does as soon as the watch is created with progress_notify, which apache/apisix needs to advance start_revision safely (apache/apisix#13067). Attach the events whenever any were collected. Also guard on body.result, which is nil when the last response is an error body with an http code below 500 - reachable before this change too, and a nil index in the watch loop. --- lib/resty/etcd/v3.lua | 7 +++- t/v3/watch.t | 76 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 t/v3/watch.t diff --git a/lib/resty/etcd/v3.lua b/lib/resty/etcd/v3.lua index 8c9b2572..be5898d5 100644 --- a/lib/resty/etcd/v3.lua +++ b/lib/resty/etcd/v3.lua @@ -939,7 +939,12 @@ local function request_chunk(self, method, path, opts, timeout) end end - if #all_events > 1 then + -- one read may carry several watch responses separated by "\n", and the + -- last one is not necessarily the one holding the events, e.g. + -- [response with events, progress notification]. `body` points at the + -- last decoded response, so the collected events must always be + -- attached to it, not only when more than one event was collected. + if #all_events > 0 and body.result then body.result.events = all_events end return body diff --git a/t/v3/watch.t b/t/v3/watch.t new file mode 100644 index 00000000..d75cf88b --- /dev/null +++ b/t/v3/watch.t @@ -0,0 +1,76 @@ +use Test::Nginx::Socket::Lua; + +log_level('info'); +no_long_string(); +repeat_each(1); +plan 'no_plan'; + +our $HttpConfig = <<'_EOC_'; + lua_socket_log_errors off; + lua_package_path 'lib/?.lua;/usr/local/share/lua/5.3/?.lua;/usr/share/lua/5.1/?.lua;;'; + + # A fake etcd watch endpoint replying with two watch responses in a single + # body chunk: one carrying an event, followed by a progress notification. + # etcd coalesces responses like this whenever they are written close enough + # together, and a real etcd cannot be driven into that state reliably. + server { + listen 1985; + + location /v3/watch { + content_by_lua_block { + ngx.print('{"result":{"header":{"revision":"7"},"events":' .. + '[{"type":"PUT","kv":{"key":"L3Rlc3Q=","value":"ImFiYyI=",' .. + '"mod_revision":"7"}}]}}\n' .. + '{"result":{"header":{"revision":"9"}}}\n') + } + } + } +_EOC_ + +run_tests(); + +__DATA__ + +=== TEST 1: a coalesced read keeps the events of every response it contains +--- http_config eval: $::HttpConfig +--- config + location /t { + content_by_lua_block { + local etcd, err = require "resty.etcd" .new({ + protocol = "v3", + http_host = "http://127.0.0.1:1985", + }) + if not etcd then + ngx.say("failed to new: ", err) + return + end + + local res_func, err = etcd:watchdir("/test", {timeout = 5}) + if not res_func then + ngx.say("failed to watchdir: ", err) + return + end + + local res, err = res_func() + if not res then + ngx.say("failed to read watch: ", err) + return + end + + -- the last response of the read is the progress notification + ngx.say("revision: ", res.result.header.revision) + local events = res.result.events + ngx.say("events: ", events and #events or 0) + if events and events[1] then + ngx.say("key: ", events[1].kv.key) + ngx.say("value: ", events[1].kv.value) + end + } + } +--- response_body +revision: 9 +events: 1 +key: /test +value: abc +--- no_error_log +[error] From fa940dde8621a240cb26cf411f7ad301a92a6e76 Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Mon, 27 Jul 2026 13:02:59 +0800 Subject: [PATCH 2/7] ci: bump runner from retired ubuntu-20.04 to ubuntu-22.04 The ubuntu-20.04 hosted runner image has been retired, so jobs pinned to it queue indefinitely with no runner to pick them up. Move to ubuntu-22.04. --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ec45b012..e92e2ab8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,7 +22,7 @@ jobs: - version: 3.4.0 conf: Procfile-single-enable-mtls - runs-on: "ubuntu-20.04" + runs-on: "ubuntu-22.04" env: OPENRESTY_PREFIX: "/usr/local/openresty" AUTH_ENDPOINT_V3: "127.0.0.1:12379" From 4ab60af155c6a6e7f064ad0e23ab30ca85ec32f3 Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Mon, 27 Jul 2026 13:49:56 +0800 Subject: [PATCH 3/7] ci: bump Go to 1.22 and use 'go install' for goreman goreman's transitive dependency golang.org/x/sys v0.46.0 now imports the stdlib "slices" package (Go >= 1.21), so building it under the pinned Go 1.17 fails at the install step. Bump the toolchain to 1.22. Since 'go get' no longer installs binaries on Go >= 1.18, switch to 'go install github.com/mattn/goreman@latest'. --- .github/workflows/ci.yml | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e92e2ab8..278a98a4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -35,10 +35,11 @@ jobs: - uses: actions/checkout@v2 - name: setup go - uses: actions/setup-go@v2.1.3 + uses: actions/setup-go@v5 with: - # goreman requires 1.17 - go-version: "1.17" + # goreman's transitive deps (golang.org/x/sys) now import the + # stdlib "slices" package, which needs Go >= 1.21 + go-version: "1.22" - name: get dependencies run: sudo apt install -y cpanminus build-essential libncurses5-dev libreadline-dev libssl-dev perl @@ -68,7 +69,7 @@ jobs: tar xf etcd-v$ETCD_VER-linux-amd64.tar.gz # run etcd local cluster, startup at localhost:2379, localhost:22379, and localhost:32379 # see more https://github.com/etcd-io/etcd/blob/master/Documentation/dev-guide/local_cluster.md - go get github.com/mattn/goreman + go install github.com/mattn/goreman@latest - name: script if: matrix.conf != 'Procfile-single-enable-mtls' From bbe2fe82be34e362f1be89404f0ccd8816e40c7e Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Mon, 27 Jul 2026 14:22:01 +0800 Subject: [PATCH 4/7] test(watch): add missing request line to coalesced-read test The new t/v3/watch.t TEST 1 had no '--- request' section, so Test::Nginx had no request to send to the /t location and bailed out with 'Request line should be non-empty' before the assertions ran. --- t/v3/watch.t | 2 ++ 1 file changed, 2 insertions(+) diff --git a/t/v3/watch.t b/t/v3/watch.t index d75cf88b..cc1c7b34 100644 --- a/t/v3/watch.t +++ b/t/v3/watch.t @@ -67,6 +67,8 @@ __DATA__ end } } +--- request +GET /t --- response_body revision: 9 events: 1 From 40fe2ba1d2b8d0f2890de04a1514c353343e1a20 Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Mon, 27 Jul 2026 14:50:59 +0800 Subject: [PATCH 5/7] test(tls): accept OpenSSL 3.x 'self-signed' wording OpenSSL 3.x reworded the X509 verify error from 'self signed certificate' to 'self-signed certificate' (hyphen). Broaden the TEST 2 pattern to 'self[ -]signed' so it matches both the 1.1.x and 3.x spellings. --- t/v3/tls.t | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/v3/tls.t b/t/v3/tls.t index c34cfde0..19ea5331 100644 --- a/t/v3/tls.t +++ b/t/v3/tls.t @@ -103,7 +103,7 @@ GET /t --- no_error_log [error] --- response_body eval -qr/18: self signed certificate/ +qr/18: self[ -]signed certificate/ From c7e0e1da482da25e26977637ce2706cd22e0ae87 Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Mon, 27 Jul 2026 15:06:40 +0800 Subject: [PATCH 6/7] ci: pin goreman to v0.3.15 and forbid implicit toolchain download Addresses review feedback (CodeRabbit/Copilot): 'go install goreman@latest' is non-deterministic, and goreman >= v0.3.19 declares 'go 1.25' in its module, so Go's default GOTOOLCHAIN=auto would silently fetch a 1.25 toolchain and defeat the setup-go 1.22 pin. Pin to v0.3.15 (module 'go 1.19', golang.org/x/sys v0.6.0) which builds with the 1.22 toolchain, and set GOTOOLCHAIN=local so a future pin bump that needs a newer toolchain fails loudly instead of downloading one. --- .github/workflows/ci.yml | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 278a98a4..b7c45d5f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -37,8 +37,9 @@ jobs: - name: setup go uses: actions/setup-go@v5 with: - # goreman's transitive deps (golang.org/x/sys) now import the - # stdlib "slices" package, which needs Go >= 1.21 + # Keep this in sync with the goreman pin in the install step: it must + # be >= goreman's module `go` directive, otherwise the toolchain gets + # downloaded implicitly (see GOTOOLCHAIN=local note below). go-version: "1.22" - name: get dependencies @@ -69,7 +70,12 @@ jobs: tar xf etcd-v$ETCD_VER-linux-amd64.tar.gz # run etcd local cluster, startup at localhost:2379, localhost:22379, and localhost:32379 # see more https://github.com/etcd-io/etcd/blob/master/Documentation/dev-guide/local_cluster.md - go install github.com/mattn/goreman@latest + # Pin goreman to a fixed release for reproducible CI. v0.3.15 declares + # `go 1.19` and pulls golang.org/x/sys v0.6.0, both of which build with + # the pinned toolchain above. GOTOOLCHAIN=local forbids Go from + # silently downloading a newer toolchain (goreman >= v0.3.19 declares + # `go 1.25`, which would otherwise defeat the setup-go pin). + GOTOOLCHAIN=local go install github.com/mattn/goreman@v0.3.15 - name: script if: matrix.conf != 'Procfile-single-enable-mtls' From 73865df75cc2aa684d487b7917be53ed135e79a9 Mon Sep 17 00:00:00 2001 From: AlinsRan Date: Mon, 27 Jul 2026 15:12:55 +0800 Subject: [PATCH 7/7] test(watch): cover [progress, event] and interleaved coalescing orders The fix's premise is that the response holding events is not necessarily the last one in a coalesced read, so cover both orderings and an interleaving, not just [event, progress]: - TEST 2: [progress, event] -> event is the last response - TEST 3: [event, progress, event] -> both events survive, in order Also document why TEST 1 reports the trailing progress notification's revision while still delivering the earlier event. --- t/v3/watch.t | 157 +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 151 insertions(+), 6 deletions(-) diff --git a/t/v3/watch.t b/t/v3/watch.t index cc1c7b34..5ca06893 100644 --- a/t/v3/watch.t +++ b/t/v3/watch.t @@ -5,23 +5,63 @@ no_long_string(); repeat_each(1); plan 'no_plan'; +# etcd coalesces watch responses into a single body read whenever they are +# written close enough together (very common once progress_notify is on), and a +# real etcd cannot be driven into that state reliably -- so each test below uses +# a fake endpoint that emits a fixed, hand-crafted coalesced read. The point of +# the fix under test is that the response holding the events is NOT necessarily +# the last one in the read, so we exercise both orderings and an interleaving. + +# [event, progress]: event (rev 7) followed by a progress notification (rev 9). our $HttpConfig = <<'_EOC_'; lua_socket_log_errors off; lua_package_path 'lib/?.lua;/usr/local/share/lua/5.3/?.lua;/usr/share/lua/5.1/?.lua;;'; + server { + listen 1985; + location /v3/watch { + content_by_lua_block { + ngx.print('{"result":{"header":{"revision":"7"},"events":' .. + '[{"type":"PUT","kv":{"key":"L3Rlc3Q=","value":"ImFiYyI=",' .. + '"mod_revision":"7"}}]}}\n' .. + '{"result":{"header":{"revision":"9"}}}\n') + } + } + } +_EOC_ - # A fake etcd watch endpoint replying with two watch responses in a single - # body chunk: one carrying an event, followed by a progress notification. - # etcd coalesces responses like this whenever they are written close enough - # together, and a real etcd cannot be driven into that state reliably. +# [progress, event]: progress notification (rev 7) followed by an event (rev 9). +our $HttpConfigProgressFirst = <<'_EOC_'; + lua_socket_log_errors off; + lua_package_path 'lib/?.lua;/usr/local/share/lua/5.3/?.lua;/usr/share/lua/5.1/?.lua;;'; server { listen 1985; + location /v3/watch { + content_by_lua_block { + ngx.print('{"result":{"header":{"revision":"7"}}}\n' .. + '{"result":{"header":{"revision":"9"},"events":' .. + '[{"type":"PUT","kv":{"key":"L3Rlc3Q=","value":"ImFiYyI=",' .. + '"mod_revision":"9"}}]}}\n') + } + } + } +_EOC_ +# [event, progress, event]: an event (rev 7), a progress notification (rev 8), +# and a second event (rev 9) all in one read. +our $HttpConfigInterleaved = <<'_EOC_'; + lua_socket_log_errors off; + lua_package_path 'lib/?.lua;/usr/local/share/lua/5.3/?.lua;/usr/share/lua/5.1/?.lua;;'; + server { + listen 1985; location /v3/watch { content_by_lua_block { ngx.print('{"result":{"header":{"revision":"7"},"events":' .. '[{"type":"PUT","kv":{"key":"L3Rlc3Q=","value":"ImFiYyI=",' .. '"mod_revision":"7"}}]}}\n' .. - '{"result":{"header":{"revision":"9"}}}\n') + '{"result":{"header":{"revision":"8"}}}\n' .. + '{"result":{"header":{"revision":"9"},"events":' .. + '[{"type":"PUT","kv":{"key":"L3Rlc3Qy","value":"ImJjZCI=",' .. + '"mod_revision":"9"}}]}}\n') } } } @@ -57,7 +97,11 @@ __DATA__ return end - -- the last response of the read is the progress notification + -- The read is [event(rev 7), progress(rev 9)]. We report the + -- progress notification's revision (9, the last response in the + -- read) while still delivering the rev-7 event: a progress + -- notification guarantees nothing happened between the event and + -- it, so the caller can safely advance start_revision past 9. ngx.say("revision: ", res.result.header.revision) local events = res.result.events ngx.say("events: ", events and #events or 0) @@ -76,3 +120,104 @@ key: /test value: abc --- no_error_log [error] + + + +=== TEST 2: a coalesced read keeps the event when it is the last response +--- http_config eval: $::HttpConfigProgressFirst +--- config + location /t { + content_by_lua_block { + local etcd, err = require "resty.etcd" .new({ + protocol = "v3", + http_host = "http://127.0.0.1:1985", + }) + if not etcd then + ngx.say("failed to new: ", err) + return + end + + local res_func, err = etcd:watchdir("/test", {timeout = 5}) + if not res_func then + ngx.say("failed to watchdir: ", err) + return + end + + local res, err = res_func() + if not res then + ngx.say("failed to read watch: ", err) + return + end + + -- The read is [progress(rev 7), event(rev 9)]. The event is the + -- last response, so its revision (9) is reported and its event + -- delivered. + ngx.say("revision: ", res.result.header.revision) + local events = res.result.events + ngx.say("events: ", events and #events or 0) + if events and events[1] then + ngx.say("key: ", events[1].kv.key) + ngx.say("value: ", events[1].kv.value) + end + } + } +--- request +GET /t +--- response_body +revision: 9 +events: 1 +key: /test +value: abc +--- no_error_log +[error] + + + +=== TEST 3: a coalesced read keeps events from every response when interleaved +--- http_config eval: $::HttpConfigInterleaved +--- config + location /t { + content_by_lua_block { + local etcd, err = require "resty.etcd" .new({ + protocol = "v3", + http_host = "http://127.0.0.1:1985", + }) + if not etcd then + ngx.say("failed to new: ", err) + return + end + + local res_func, err = etcd:watchdir("/test", {timeout = 5}) + if not res_func then + ngx.say("failed to watchdir: ", err) + return + end + + local res, err = res_func() + if not res then + ngx.say("failed to read watch: ", err) + return + end + + -- The read is [event(rev 7), progress(rev 8), event(rev 9)]. Both + -- events must survive, in order, under the highest revision (9). + ngx.say("revision: ", res.result.header.revision) + local events = res.result.events + ngx.say("events: ", events and #events or 0) + if events then + for i = 1, #events do + ngx.say("key", i, ": ", events[i].kv.key, + " value", i, ": ", events[i].kv.value) + end + end + } + } +--- request +GET /t +--- response_body +revision: 9 +events: 2 +key1: /test value1: abc +key2: /test2 value2: bcd +--- no_error_log +[error]