Skip to content

fix(consul): upgrade lua-resty-consul to 0.4.0 - #14118

Merged
AlinsRan merged 3 commits into
apache:masterfrom
AlinsRan:fix/consul-tls-pool-key
Oct 29, 2026
Merged

AlinsRan merged 3 commits into
apache:masterfrom
AlinsRan:fix/consul-tls-pool-key

Conversation

@AlinsRan

@AlinsRan AlinsRan commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Description

The consul discovery client sends its queries through resty.consul. Version 0.3.2 connects with httpc:connect(host, port), which uses the default host:port connection pool, and then calls ssl_handshake, which returns immediately on a reused connection. When two consul registries point at the same https server, one with ssl_verify: false and one with ssl_verify: true, the verifying registry can pick up a pooled connection the other one set up without certificate verification, and fetch nodes from a server whose certificate it would reject.

lua-resty-consul 0.4.0 connects with the table form of lua-resty-http connect (scheme, host, port, ssl_verify, ssl_server_name). That form puts the scheme, SNI and ssl_verify into the pool name and does the handshake itself, so a connection is reused only by a client with the same TLS options. This PR bumps the dependency to 0.4-0.

Other changes in 0.4.0 to lib/resty/consul.lua: none besides the connect rewrite (the rest of the release is CI, test certificates and the README). Its dependency list is unchanged (lua-resty-http, unversioned), and the table form of connect is available in the lua-resty-http version APISIX already ships. The callers, discovery/consul and discovery/consul_kv, always pass a host and port, so the unix socket path (no port) that the connect rewrite affects is not used.

Test: t/discovery/consul-tls.t TEST 7 runs the default registry with ssl_verify: false and a second registry with ssl_verify: true against the same https endpoint, whose certificate is not trusted. With 0.3.2 the second registry resolves the service; with 0.4.0 it fails certificate verification. The other consul / consul_kv discovery tests give the same results with both versions.

The PR also fixes a startup race in TEST 2-5 of the same file: they sent GET /hello right after nginx started, before the privileged agent's first consul fetch, and could get a 503 or miss the expected error log. The /hello route now has a serverless-pre-function that waits until the shared dict has the service (up to 5 seconds, for the test where the fetch is expected to fail).

Which issue(s) this PR fixes:

N/A

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change (no user-facing configuration change)
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

lua-resty-consul 0.3.2 connects through the plain host:port connection pool and
skips the TLS handshake when it gets a reused connection. Two registries
pointing at the same https consul server therefore share their connections,
and one with ssl_verify could send its queries over a connection set up by
another without certificate verification.

0.4.0 connects with the table form of lua-resty-http connect, which keys the
pool on the scheme, the SNI and the ssl_verify setting, so only connections
established with the same TLS options are reused.
@AlinsRan
AlinsRan force-pushed the fix/consul-tls-pool-key branch from a69bd9e to fbb2021 Compare October 28, 2026 09:29
@AlinsRan AlinsRan changed the title fix(consul): do not reuse an unverified TLS connection for a verifying client fix(consul): upgrade lua-resty-consul to 0.4.0 Oct 28, 2026

@membphis membphis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AlinsRan
AlinsRan merged commit f3390e5 into apache:master Oct 29, 2026
20 checks passed
@AlinsRan
AlinsRan deleted the fix/consul-tls-pool-key branch October 29, 2026 03:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants