fix(server)!: reject a wildcard bind with no advertised address - #3923
Open
chengxilo wants to merge 15 commits into
Open
fix(server)!: reject a wildcard bind with no advertised address#3923chengxilo wants to merge 15 commits into
chengxilo wants to merge 15 commits into
Conversation
|
Thanks for the PR. It is labeled Slash commands (own line, regular comment) move it around the queue:
See CONTRIBUTING.md for details. |
Closes apache#3890 A server whose TCP listener binds a wildcard reported that wildcard as its own client-facing address in GetClusterMetadata. SDKs collect roster addresses into their reconnect candidates, so 0.0.0.0:8090 became a dial target that retry would eventually pick and never reach. A bind address answers which interfaces a node accepts on. It is not an answer to where a client reaches it, and for the unspecified address the two have no relation. With a roster each node already answers the second question through cluster.nodes.advertised_address; without one the server had no way to be told it at all. node.advertised_address supplies it, named to match its roster counterpart, and the server now refuses to start when a wildcard bind leaves the question unanswered. A concrete bind address still needs no declaration: it already names an interface a client can reach. Only what an operator declares is validated. A bind address is never held to being routable, which is the mistake that made Kafka reject wildcard binds that had always been valid (KAFKA-18281). The declared values are held to it in both modes, so a roster ip, advertised address or per-network selector that names the unspecified address stops the boot rather than the cluster: peers dialing 0.0.0.0 reach their own host, which is how a cluster comes up with every node believing it is alone while its containers report healthy. Resolution is now infallible past construction. A roster node is built through TryFrom, so an address that does not parse fails there instead of leaving every consumer to carry a fallback, and the fallbacks are gone: metadata no longer publishes a raw unparsed string, forwarding no longer treats a missing replica ip as "no target", and a selector whose CIDR does not parse is no longer dropped in silence. The two listeners also resolve the self address once between them, rather than each deriving it from its own bind address and disagreeing whenever http.address and tcp.address differ. BREAKING CHANGE: a server that binds a wildcard address without a roster now refuses to start until node.advertised_address names where clients reach it. Deployments that bind 0.0.0.0 must declare one: the Helm chart derives it from the Service DNS name and the shipped compose files name their service, but an external deployment needs the hostname or load balancer address its clients dial. A cluster.nodes ip must now be a literal IP, and no declared address may be the unspecified one.
chengxilo
force-pushed
the
fix-unreachable-roster-candidates
branch
from
August 19, 2026 05:11
60f1345 to
2b6fc1a
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3923 +/- ##
=============================================
- Coverage 84.03% 71.27% -12.76%
Complexity 1358 1358
=============================================
Files 1215 1215
Lines 170820 156068 -14752
Branches 138576 123826 -14750
=============================================
- Hits 143547 111242 -32305
- Misses 23411 40992 +17581
+ Partials 3862 3834 -28
🚀 New features to boost your workflow:
|
mmodzelewski
requested changes
Aug 23, 2026
mmodzelewski
left a comment
mmodzelewski
left a comment
Member
There was a problem hiding this comment.
The core logic is sound, there are a few things worth fixing before merge, though. Besides the comments in-line, here are the ones that don't have a direct reference in the changed code:
- docker-compose.yml (root) — builds the Dockerfile (wildcard binds at Dockerfile:45-48), has no environment: block, and cluster is off by default, so the README-documented docker compose up quickstart now refuses boot. Needs IGGY_NODE_ADVERTISED_ADDRESS=localhost like the eight other compose files patched in this PR.
- foreign/java/java-sdk/src/test/java/org/apache/iggy/client/BaseIntegrationTest.java:92 — the testcontainer sets IGGY_TCP_ADDRESS=0.0.0.0:8090 with no advertised var; it will refuse boot once apache/iggy:edge ships this change. The C# fixture got the fix (VsrCluster.cs:294); the Java one was missed. Dormant today only because CI takes the USE_EXTERNAL_SERVER path.
- core/server/src/args.rs:57 — the --help example IGGY_TCP_ADDRESS=0.0.0.0:8090 is now boot-refusing when copy-pasted standalone; the READMEs were updated but the help text was missed.
| if let Some(http_addr) = topology.http_listen_addr { | ||
| // One host for all four transports, resolved from the client-facing | ||
| // TCP bind so both listeners publish the same node address. | ||
| let self_advertised = self_advertised_address( |
Member
There was a problem hiding this comment.
This changes the source of the HTTP metadata self address: before this PR, http.rs published the HTTP listener's own bound address (self_ip: bound_addr.ip().to_string()); now it derives from the TCP bind. With the shipped default tcp.address = 127.0.0.1:8090 and HTTP bound to a concrete external interface, the published address silently flips from the external IP to 127.0.0.1 - no boot refusal, no warning. Consider warning on (or validating) the interface mismatch between the TCP and HTTP binds.
| }; | ||
| let replica_ip = config.ip.parse().ok(); | ||
| Self { | ||
| impl TryFrom<ClusterNodeConfig> for ResolvedClusterNode { |
Member
There was a problem hiding this comment.
TryFrom checks only that the sources parse; it doesn't reject unspecified addresses the way the validator does for roster ip, catch-all, and selectors. The "always dialable" invariant therefore holds only because validation runs before conversion - a direct TryFrom caller can still build a node advertising 0.0.0.0. Mirroring the unspecified rejection here (on the canonical form) would make the type self-defending.
github-actions
Bot
added
S-waiting-on-author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR address?
Closes #3890
This PR block #3650, see #3890 for detail.
Rationale
A wildcard bind says which interfaces a node accepts on, not where a client reaches it, so publishing it as an address hands clients a target they cannot dial.
What changed?
node.advertised_addresssupplies it, and the server now refuses to start when server is configured with wildcard bind while leaves it unset.Breaking:
deployments binding
0.0.0.0without a roster must declare an address. The Helm chart can derives it from the Service DNS name and the shipped compose files name their service; anything else needsIGGY_NODE_ADVERTISED_ADDRESS. Acluster.nodesip must now be a literal IP, and no declared address may be the unspecified one.Some detail regarding new behavior:
When would it boot?
Standalone (
cluster.enabled = false)tcp.addressnode.advertised_address127.0.0.1:8090127.0.0.1(derived from the bind)127.0.0.1:8090broker.example.combroker.example.com(declared wins)0.0.0.0:80900.0.0.0:8090broker.example.combroker.example.com0.0.0.0/::broker:8090(carries a port)Cluster (
cluster.enabled = true)tcp.addressonly picks the bind interface here — ports come from the roster — so a wildcard is perfectlynormal.
nodes[].ip172.28.0.101nodes[].ip0.0.0.0/::nodes[].ipiggy-server(any hostname)advertised_addressadvertised_addressipadvertised_addressbroker.example.comadvertised_address0.0.0.0/::address0.0.0.0/::node.advertised_addressWhich address a client is told (
advertised_for):Both modes
tcp.address:8090(empty host)SocketAddrhas no such spellinglocalhost:8090(hostname)Edge cases
cluster.enabled = falsewith[[cluster.nodes]]left behindThat last row is a side fix: a malformed selector used to be swallowed, surfacing only as "clients on one network
get the catch-all address" with nothing to debug.
Local Execution
AI Usage