Repository navigation
node: handshake with a transit router the catalog does not list - #667
Merged
Merged
Conversation
A caller reaches a peer through a circuit at one of the peer's routers,
after handshaking there. It recognized a router by its catalog, read from
/info at start and every fifteen minutes: a circuit through a router the
catalog did not list was treated as a relay that is not a router and
dialled as is, and the relay refused it (PERMISSION_DENIED). A member that
starts during a router rollout reads a catalog without the routers that
are restarting, and the deploy's cold-path probe did on 2026-10-11: both
circuits to the SDK agent refused, nothing in the probe's log to say why.
Until the proactive mesh was removed every member handshook with every
router at start, which hid this.
For a relay the catalog does not list the node now re-reads the routers,
at most once in ten seconds, and handshakes with a relay still unlisted
after that, after the listed routers; the relay's own address comes from
the circuit address. A relay that is not a router admits what it
handshakes with on its own terms and its circuit is kept whatever the
handshake said, as before. Each handshake for transit is counted in
agentmesh_node_transit_handshakes_total{router=listed|unlisted,result},
and an unlisted one is logged.
A node without a store, as some tests build, reports that instead of
dereferencing it.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request adds support for transiting routers that are not listed in the control plane's catalog, allowing nodes to handshake with unlisted relays and periodically refresh the catalog. It also introduces a new Prometheus metric agentmesh_node_transit_handshakes_total to track these handshakes, along with corresponding tests and documentation. A critical issue was found in relayTransportAddr where calling AppendComponent on a nil multiaddr.Multiaddr interface will cause a runtime panic; using multiaddr.Join is recommended to safely construct the address.
Review on google#667. Multiaddr is a slice of components in go-multiaddr v0.16, so the relay's address is the components before the /p2p/<relay>/p2p-circuit tail, cloned; TestRelayTransportAddr pins the addressed, bare and direct cases.
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
1 participant
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.
Follow-up to the deploy failure at 14:35 today (
dd17037a, no ladder load): the cold-path probe called the JS agent through its two circuit addresses and both relays answeredPERMISSION_DENIED (202).selectTransitrecognized a router only through the node's catalog, read from/infoat start and every fifteen minutes. A circuit through a router the catalog did not list was treated as a relay that is not a router and dialled unauthenticated, and the relay refused it. A member that starts during a router rollout reads a catalog without the routers that are restarting (their lease is withdrawn for 30 s), which is what the probe is. Before #652 every member handshook with every router at start, which hid this.For a relay the catalog does not list the node now re-reads the routers (at most once in ten seconds) and handshakes with a relay still unlisted after that, after the listed routers, taking the relay's own address from the circuit address. A relay that is not a router admits on its own terms and its circuit is kept as before. Every transit handshake is counted in
agentmesh_node_transit_handshakes_total{router="listed"|"unlisted",result}and an unlisted one is logged, so the next such failure reads from the metrics.Tests:
TestSelectTransitHandshakesWithARouterTheCatalogDoesNotList(two fake routers, node holds one, no control plane to re-read; fails on main with 0 handshakes),TestCallerTransitsARouterItsCatalogMissed(integration: router b and its provider arrive after the caller). AlsorefreshRouterCatalog/loadIdentityForAuthreturn an error instead of dereferencing a nil store, which some unit tests build.