Skip to content

[FIXED] Missing leaf connect advisory for solicited connections#8015

Merged
neilalexander merged 1 commit intomainfrom
maurice/leaf-conn-advisory
Apr 9, 2026
Merged

[FIXED] Missing leaf connect advisory for solicited connections#8015
neilalexander merged 1 commit intomainfrom
maurice/leaf-conn-advisory

Conversation

@MauriceVanVeen
Copy link
Copy Markdown
Member

Resolves #7998

Signed-off-by: Maurice van Veen github@mauricevanveen.com

@MauriceVanVeen MauriceVanVeen requested a review from a team as a code owner April 7, 2026 16:02
@synadia-claude-reviewer
Copy link
Copy Markdown

synadia-claude-reviewer bot commented Apr 7, 2026

Claude finished @MauriceVanVeen's task in 1m 33s —— View job


PR Review: Missing leaf connect advisory for solicited connections

  • Read CLAUDE.md for repo guidelines
  • Examine PR diff
  • Read relevant source context in leafnode.go
  • Analyze test coverage
  • Post review

Verdict: LGTM

The fix is correct, minimal, and well-tested.

Root Cause

For hub-side (accepted) leaf connections, accountConnectEvent is already fired via isClientAuthorized in auth.go:398 (which runs for inbound LEAF connections during auth). For spoke-side (solicited) connections, the connection is initiated outbound and doesn't go through isClientAuthorized, so accountConnectEvent was never called — causing the missing advisory.

The single-line addition in leafNodeFinishConnectProcess (which is only reachable for solicited/spoke connections — the accepting side returns early at leafnode.go:1504 when !didSolicit) correctly closes this gap without risk of double-firing.

Test

TestLeafNodeSpokeConnectAdvisory covers the scenario well: it validates both the DISCONNECT advisory (already working) and the CONNECT advisory (the bug) across a hub shutdown/restart cycle for the USER account leaf, confirming the fix works on reconnect too.

No issues found.

Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
@MauriceVanVeen MauriceVanVeen force-pushed the maurice/leaf-conn-advisory branch from d33f556 to 1751487 Compare April 7, 2026 16:10
Copy link
Copy Markdown
Member

@neilalexander neilalexander left a comment

Choose a reason for hiding this comment

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

LGTM

@neilalexander neilalexander merged commit e58dea1 into main Apr 9, 2026
71 of 72 checks passed
@neilalexander neilalexander deleted the maurice/leaf-conn-advisory branch April 9, 2026 11:49
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.

Leaf node CONNECT advisory not emitted for solicited connections

2 participants