bootstore type reorg PR 5/?: Split internal/external API types: Add ExternalImportExportPolicy

[jgallagher]

ImportExportPolicy was defined in omicron_common::api::external, but was used in the external API, a few internal APIs, and serialized in the bootstore. This PR introduces ExternalImportExportPolicy (external API only) and moves ImportExportPolicy (the rest) into omicron_common::api::internal::shared - future work in this chain of PRs will move it again (along with other serialized-in-the-bootstore types), but it has to land there for now.

This makes no changes to any internal or external OpenAPI specs - it's only adjusting internal representations.

Part of #9801. Staged on top of #9909.

I'm a lot less sure of some of the types here than with previous PRs in this stack because I'm finding the various BgpPeer{,Config} types pretty confusing. Some notes on types affected by this change:

• common::api::internal::shared::BgpPeerConfig is the type serialized in the bootstore and used in internal APIs, so it uses the now-internal ImportExportPolicy
• nexus_types::external_api::networking::BgpPeer looks like the previous type (the internal BgpPeerConfig), but the fields aren't 1-to-1. This is an external type, so it uses ExternalImportExportPolicy. (In principle this is fine, and is the point of this PR itself: the external types don't have to map 1-to-1 to internal types! But the names are quite confusing, largely because of the next two bullets.)
• nexus_types::external_api::networking::BgpPeerConfig also exists but it's a container for a vec of nexus_types::external_api::networking::BgpPeers and doesn't look anything at all like the internal BgpPeerConfig in the first bullet
• nexus/db-queries/src/db/datastore/switch_port.rs also defines a public (!) BgpPeerConfig struct that has many common fields with the internal BgpPeerConfig and the external BgpPeer, but the types of the fields are database flavored (e.g., SqlU32 instead of u32). This struct is included as a field in the public SwitchPortSettingsCombinedResult type defined in the same module that's consumed by a couple of Nexus background tasks. I'm not sure whether this should use the internal or external ImportExportPolicy. I originally had it using the external one (because of the way I thought it was used). In principle it seems like we should limit the external type to the external API, so in 924ca7e I changed it to use the internal ImportExportPolicy, but feel free to tell me to revert that commit and go back to external. Weirdly (as you can see in the commit), this change did not affect any of the consumers of this BgpPeerConfig, and rust-analyzer agrees that these fields are never read by anything. I don't know if they could be removed entirely, or if the fact that they're not being used is an oversight somewhere else.

I'm a little tempted to throw up my hands and move ImportExportPolicy back to omicron_common and share one type everywhere, but that doesn't seem like the right direction - maybe it would be better to invest time in cleaning up the organization of these BGP types instead? Feedback very welcome.

[jgallagher]

Moving this to draft for now based on #9906 (comment).

[rcgoodfellow]

maybe it would be better to invest time in cleaning up the organization of these BGP types instead? Feedback very welcome.

Sorry for the rather vexing set of types here. I think part of the confusion here comes from the fact that when a user defines a BGP peer configuration, it references a higher level overall BGP configuration. These are distinct objects that the user creates and are also distinct tables in the database. When it comes time to send configure BGP through Maghemite, the type representation needs to be changed a bit based on how the data structures are hooked together in Maghemite. It's no longer a peer configuration referencing a top level BGP configuration by UUID, but BGP peer configs are tied to a top level configuration through ASNs.

I'm also coming to think that putting BGP configuration and routes in the switch port settings object was a mistake. They should probably be their own top level things.

I think part of the multiplicity we have in the data structures comes from the era in which these data structures landed about 3 years ago. Back then it was challenging to have common data structures across omicron without getting in a circular dependency mess with and an inability to generate openapi specs.

This is all to say, yes I think it would be very worthwhile to reorganize and clean things up. There are parts of the BGP data structures that just aren't relevant at all. A rather large complexity down payment was made with the idea that breakout ports would soon become a thing, but that never happened and those interfaces just make the API and use of internal data structures much more confusing than it needs to be.

cc: @internet-diglett