In-memory result of inserting multiple `BgpPeerConfig`s with unnumbered peers may have incorrect communities and import/export policies

[jgallagher]

When performing the database inserts for a SwitchPortSettingsCreate request, we assemble an in-memory map keyed by IP address:

omicron/nexus/db-queries/src/db/datastore/switch_port.rs

Lines 1410 to 1411 in eb0bb82

	let mut peer_by_addr: BTreeMap<IpAddr, &networking::BgpPeer> =
	BTreeMap::new();

For unnumbered peers, we use the sentinel value 0.0.0.0 as the key:

omicron/nexus/db-queries/src/db/datastore/switch_port.rs

Lines 1416 to 1419 in eb0bb82

	// Track peers for policy lookup. For unnumbered peers (addr is None),
	// use UNSPECIFIED as the sentinel value.
	let lookup_addr =
	p.addr.unwrap_or(IpAddr::V4(Ipv4Addr::UNSPECIFIED));

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:

omicron/nexus/db-queries/src/db/datastore/switch_port.rs

Lines 1535 to 1561 in eb0bb82

	// Lookup policies and communities for peers. For unnumbered peers (addr
	// is None), use UNSPECIFIED as the lookup key.
	let lookup_addr =
	p.addr.map(|a| a.ip()).unwrap_or(IpAddr::V4(Ipv4Addr::UNSPECIFIED));
	let (allowed_import, allowed_export, communities) = (
	peer_by_addr
	.get(&lookup_addr)
	.map(|x| x.allowed_import.clone())
	.unwrap_or(ImportExportPolicy::NoFiltering),
	peer_by_addr
	.get(&lookup_addr)
	.map(|x| x.allowed_export.clone())
	.unwrap_or(ImportExportPolicy::NoFiltering),
	peer_by_addr
	.get(&lookup_addr)
	.map(|x| x.communities.clone())
	.unwrap_or(Vec::new()),
	);
	result.bgp_peers.push(
	BgpPeerFromDbBuilder {
	peer_config: &p,
	communities,
	allowed_import,
	allowed_export,
	}
	.build(),
	);

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.

[jgallagher]

Example from a test failure: inserting multiple unnumbered peers associated with different links, like this:

        let peer_phy0 = BgpPeer {
            addr: None,
            interface_name: "phy0".parse().unwrap(),
            communities: vec![10, 20],
            allowed_export: ImportExportPolicy::Allow(vec![
                "10.0.0.0/8".parse().unwrap(),
            ]),
            allowed_import: ImportExportPolicy::NoFiltering,
            // ... rest of the fields elided for brevity ...
        };

        let peer_phy1 = BgpPeer {
            addr: None,
            interface_name: "phy1".parse().unwrap(),
            communities: vec![30, 40],
            allowed_export: ImportExportPolicy::NoFiltering,
            allowed_import: ImportExportPolicy::Allow(vec![
                "172.16.0.0/12".parse().unwrap(),
            ]),
            // ... rest of the fields elided for brevity ...
        };

does result in correct data being inserted:

root@[::1]:32221/omicron> select * from switch_port_settings_bgp_peer_config_communities;
            port_settings_id           | interface_name |  addr   | community
---------------------------------------+----------------+---------+------------
  c4fbcb65-3d48-4ffc-9a85-d170e81f672f | phy0           | 0.0.0.0 |        10
  c4fbcb65-3d48-4ffc-9a85-d170e81f672f | phy0           | 0.0.0.0 |        20
  c4fbcb65-3d48-4ffc-9a85-d170e81f672f | phy1           | 0.0.0.0 |        30
  c4fbcb65-3d48-4ffc-9a85-d170e81f672f | phy1           | 0.0.0.0 |        40
(4 rows)


Time: 8ms total (execution 8ms / network 0ms)

root@[::1]:32221/omicron> select * from switch_port_settings_bgp_peer_config_allow_export;
            port_settings_id           | interface_name |  addr   |   prefix
---------------------------------------+----------------+---------+-------------
  c4fbcb65-3d48-4ffc-9a85-d170e81f672f | phy0           | 0.0.0.0 | 10.0.0.0/8
(1 row)


Time: 7ms total (execution 7ms / network 0ms)

root@[::1]:32221/omicron> select * from switch_port_settings_bgp_peer_config_allow_import;
            port_settings_id           | interface_name |  addr   |    prefix
---------------------------------------+----------------+---------+----------------
  c4fbcb65-3d48-4ffc-9a85-d170e81f672f | phy1           | 0.0.0.0 | 172.16.0.0/12
(1 row)

but the returned result reports the same communities and import/export policies for both peers:

    assertion `left == right` failed
      left: [10, 20]
     right: [30, 40]