oxidecomputer / oxidecomputer/omicron
In-memory result of inserting multiple `BgpPeerConfig`s with unnumbered peers may have incorrect communities and import/export policies
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 572
- Forks
- 97
- Avg merge
- 2d 12h
- Merged PRs (30d)
- 96
Description
When performing the database inserts for a SwitchPortSettingsCreate request, we assemble an in-memory map keyed by IP address: https://github.com/oxidecomputer/omicron/blob/eb0bb82820e46f2b114f3ae0b620333bfe0109bc/nexus/db-queries/src/db/datastore/switch_port.rs#L1410-L1411
For unnumbered peers, we use the sentinel value 0.0.0.0 as the key: https://github.com/oxidecomputer/omicron/blob/eb0bb82820e46f2b114f3ae0b620333bfe0109bc/nexus/db-queries/src/db/datastore/switch_port.rs#L1416-L1419
After performing the inserts (which do record the correct data for each peer - this bug only affects the in-memory result we return), we reassemble the communities and import/export policies by matching up the inserted row against its corresponding entry in peer_by_addr: https://github.com/oxidecomputer/omicron/blob/eb0bb82820e46f2b114f3ae0b620333bfe0109bc/nexus/db-queries/src/db/datastore/switch_port.rs#L1535-L1561
However, unnumbered peers all share the same 0.0.0.0 key, so we only have one possible value, and we reuse that for all unnumbered peers. If we insert two different unnumbered peers associated with two different links and those two peers have different communities or import/export policies, the result we return will be incorrect: we'll return the same communities and import/export policies for all unnumbered peers (the exact value being whichever was inserted into the peer_by_addr map last).
This came up during cleanup work for #9832, when reworking the database representations for unnumbered peers to get rid of the need for a sentinel value. The upcoming PR that does that rework will fix this.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in nexus/db-queries/src/db/datastore/switch_port.rs at the peer_by_addr assembly around lines 1410-1419 and the result reconstruction around lines 1535-1561. Trace SwitchPortSettingsCreate with two unnumbered peers on different links and different communities or import/export policies. Done means the returned in-memory result preserves each peer's own values, while the database rows remain correct.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 48/100