Master#3994
Conversation
fix: merge deduplicated egress routes for shared relay peer keys
Release v1.5.1
|
Tenki Code Review - Complete Files Reviewed: 2 By Severity:
The PR improves Files Reviewed (2 files) |
There was a problem hiding this comment.
Overview
This PR changes deduplicateEgressRoutes in logic/peers.go from a simple drop-duplicate strategy to a merge strategy: when two EgressNetworkRoutes entries share the same PeerKey+Network key, their EgressRanges and EgressRangesWithMetric slices are now combined rather than the second entry being discarded. A new helper mergeUniqueEgressRangeMetrics handles the metrics merging, and a test file logic/peers_test.go is added to cover the new behavior.
Finding: Incomplete deduplication in mergeUniqueEgressRangeMetrics
In logic/peers.go (line 960), mergeUniqueEgressRangeMetrics uses the full models.EgressRangeMetric struct directly as a map key:
seen := make(map[models.EgressRangeMetric]struct{}, len(existing)+len(incoming))EgressRangeMetric contains EgressID and EgressName fields (tagged json:"-" but actively populated in logic/egress.go with distinct per-egress-gateway values). This means two metrics representing the same network range but originating from different egress gateway sources will have different EgressID values, causing them to be treated as distinct entries and both retained in the merged slice.
By contrast, EgressRanges is deduplicated by UniqueStrings purely on the CIDR string — so after merging, EgressRanges will correctly contain one entry for a network range while EgressRangesWithMetric may contain multiple entries for the same range. This inconsistency can cause the client to receive duplicate routing metric entries for a single network.
The existing test does not exercise this case (its test metrics have EgressID: "" and EgressName: ""), so the test passes despite the latent inconsistency.
Fix: Change the map key from models.EgressRangeMetric to string (the Network field), mirroring the string-based deduplication of EgressRanges.
Describe your changes
Provide Issue ticket number if applicable/not in title
Provide testing steps
Checklist before requesting a review