Network Operator BGP API improvements (#6362)

#6081
---
Refactored existing api to list bgp announce sets and added a new api to
list announcements for an announce set

#6244 and #6245
---
Refactored bgp datastore methods to propagate additional error context
as well as emit error types for the external API (404, 403, etc)

#6246
---
Two part fix:
1. Emit an error if operator attempts to create more than one bgp
configuration for the same ASN
2. Emit an error if the operator tries to configure BGP peers for the
same switch that point to bgp configs with _different_ ASNs

#6349
---
Updated bgp datastore methods to always lookup the parent record (not
just when the name needs to be resolved to a UUID). This guards us
against situations where an invalid UUID is accidentally submitted.

Related 
---
Resolving these issues for BGP
- [x] closes #6081
- [x] closes #6244
- [x] closes #6245
- [x] closes #6246
- [x] #6349

nexus/db-model/src/schema.rs

@@ -1889,7 +1889,8 @@ allow_tables_to_appear_in_same_query!(
 
 allow_tables_to_appear_in_same_query!(
     switch_port,
-    switch_port_settings_bgp_peer_config
+    switch_port_settings_bgp_peer_config,
+    bgp_config
 );
 
 allow_tables_to_appear_in_same_query!(disk, virtual_provisioning_resource);

nexus/db-model/src/schema_versions.rs

@@ -17,7 +17,7 @@ use std::collections::BTreeMap;
 ///
 /// This must be updated when you change the database schema.  Refer to
 /// schema/crdb/README.adoc in the root of this repository for details.
-pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(89, 0, 0);
+pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(90, 0, 0);
 
 /// List of all past database schema versions, in *reverse* order
 ///
@@ -29,6 +29,7 @@ static KNOWN_VERSIONS: Lazy<Vec<KnownVersion>> = Lazy::new(|| {
         // |  leaving the first copy as an example for the next person.
         // v
         // KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"),
+        KnownVersion::new(90, "lookup-bgp-config-by-asn"),
         KnownVersion::new(89, "collapse_lldp_settings"),
         KnownVersion::new(88, "route-local-pref"),
         KnownVersion::new(87, "add-clickhouse-server-enum-variants"),

nexus/db-queries/src/db/datastore/mod.rs

@@ -366,6 +366,7 @@ impl DataStore {
     }
 }
 
+#[derive(Clone, Copy, Debug)]
 pub enum UpdatePrecondition<T> {
     DontCare,
     Null,

nexus/src/app/background/tasks/sync_switch_configuration.rs

@@ -565,7 +565,7 @@ impl BackgroundTask for SwitchPortSettingsManager {
                         if !bgp_announce_prefixes.contains_key(&bgp_config.bgp_announce_set_id) {
                             let announcements = match self
                                 .datastore
-                                .bgp_announce_list(
+                                .bgp_announcement_list(
                                     opctx,
                                     &params::BgpAnnounceSetSelector {
                                         name_or_id: bgp_config

nexus/src/app/bgp.rs

@@ -16,13 +16,13 @@ use omicron_common::api::external::{
 use std::net::IpAddr;
 
 impl super::Nexus {
-    pub async fn bgp_config_set(
+    pub async fn bgp_config_create(
         &self,
         opctx: &OpContext,
         config: &params::BgpConfigCreate,
     ) -> CreateResult<BgpConfig> {
         opctx.authorize(authz::Action::Modify, &authz::FLEET).await?;
-        let result = self.db_datastore.bgp_config_set(opctx, config).await?;
+        let result = self.db_datastore.bgp_config_create(opctx, config).await?;
         Ok(result)
     }
 
@@ -69,13 +69,13 @@ impl super::Nexus {
         Ok(result)
     }
 
-    pub async fn bgp_announce_list(
+    pub async fn bgp_announce_set_list(
         &self,
         opctx: &OpContext,
-        sel: &params::BgpAnnounceSetSelector,
-    ) -> ListResultVec<BgpAnnouncement> {
+        pagparams: &PaginatedBy<'_>,
+    ) -> ListResultVec<BgpAnnounceSet> {
         opctx.authorize(authz::Action::Read, &authz::FLEET).await?;
-        self.db_datastore.bgp_announce_list(opctx, sel).await
+        self.db_datastore.bgp_announce_set_list(opctx, pagparams).await
     }
 
     pub async fn bgp_delete_announce_set(
@@ -89,6 +89,15 @@ impl super::Nexus {
         Ok(result)
     }
 
+    pub async fn bgp_announcement_list(
+        &self,
+        opctx: &OpContext,
+        sel: &params::BgpAnnounceSetSelector,
+    ) -> ListResultVec<BgpAnnouncement> {
+        opctx.authorize(authz::Action::Read, &authz::FLEET).await?;
+        self.db_datastore.bgp_announcement_list(opctx, sel).await
+    }
+
     pub async fn bgp_peer_status(
         &self,
         opctx: &OpContext,

nexus/src/app/rack.rs

@@ -510,7 +510,7 @@ impl super::Nexus {
 
             match self
                 .db_datastore
-                .bgp_config_set(
+                .bgp_config_create(
                     &opctx,
                     &BgpConfigCreate {
                         identity: IdentityMetadataCreateParams {

nexus/src/external_api/http_entrypoints.rs

@@ -285,6 +285,8 @@ pub(crate) fn external_api() -> NexusApiDescription {
         api.register(networking_bgp_announce_set_delete)?;
         api.register(networking_bgp_message_history)?;
 
+        api.register(networking_bgp_announcement_list)?;
+
         api.register(networking_bfd_enable)?;
         api.register(networking_bfd_disable)?;
         api.register(networking_bfd_status)?;
@@ -3866,7 +3868,7 @@ async fn networking_bgp_config_create(
         let nexus = &apictx.context.nexus;
         let config = config.into_inner();
         let opctx = crate::context::op_context_for_external_api(&rqctx).await?;
-        let result = nexus.bgp_config_set(&opctx, &config).await?;
+        let result = nexus.bgp_config_create(&opctx, &config).await?;
         Ok(HttpResponseCreated::<BgpConfig>(result.into()))
     };
     apictx
@@ -4044,7 +4046,7 @@ async fn networking_bgp_config_delete(
 /// set with the one specified.
 #[endpoint {
     method = PUT,
-    path = "/v1/system/networking/bgp-announce",
+    path = "/v1/system/networking/bgp-announce-set",
     tags = ["system/networking"],
 }]
 async fn networking_bgp_announce_set_update(
@@ -4066,24 +4068,28 @@ async fn networking_bgp_announce_set_update(
         .await
 }
 
-//TODO pagination? the normal by-name/by-id stuff does not work here
-/// Get originated routes for a BGP configuration
+/// List BGP announce sets
 #[endpoint {
     method = GET,
-    path = "/v1/system/networking/bgp-announce",
+    path = "/v1/system/networking/bgp-announce-set",
     tags = ["system/networking"],
 }]
 async fn networking_bgp_announce_set_list(
     rqctx: RequestContext<ApiContext>,
-    query_params: Query<params::BgpAnnounceSetSelector>,
-) -> Result<HttpResponseOk<Vec<BgpAnnouncement>>, HttpError> {
+    query_params: Query<
+        PaginatedByNameOrId<params::OptionalBgpAnnounceSetSelector>,
+    >,
+) -> Result<HttpResponseOk<Vec<BgpAnnounceSet>>, HttpError> {
     let apictx = rqctx.context();
     let handler = async {
         let nexus = &apictx.context.nexus;
-        let sel = query_params.into_inner();
+        let query = query_params.into_inner();
+        let pag_params = data_page_params_for(&rqctx, &query)?;
+        let scan_params = ScanByNameOrId::from_query(&query)?;
+        let paginated_by = name_or_id_pagination(&pag_params, scan_params)?;
         let opctx = crate::context::op_context_for_external_api(&rqctx).await?;
         let result = nexus
-            .bgp_announce_list(&opctx, &sel)
+            .bgp_announce_set_list(&opctx, &paginated_by)
             .await?
             .into_iter()
             .map(|p| p.into())
@@ -4100,17 +4106,17 @@ async fn networking_bgp_announce_set_list(
 /// Delete BGP announce set
 #[endpoint {
     method = DELETE,
-    path = "/v1/system/networking/bgp-announce",
+    path = "/v1/system/networking/bgp-announce-set/{name_or_id}",
     tags = ["system/networking"],
 }]
 async fn networking_bgp_announce_set_delete(
     rqctx: RequestContext<ApiContext>,
-    selector: Query<params::BgpAnnounceSetSelector>,
+    path_params: Path<params::BgpAnnounceSetSelector>,
 ) -> Result<HttpResponseUpdatedNoContent, HttpError> {
     let apictx = rqctx.context();
     let handler = async {
         let nexus = &apictx.context.nexus;
-        let sel = selector.into_inner();
+        let sel = path_params.into_inner();
         let opctx = crate::context::op_context_for_external_api(&rqctx).await?;
         nexus.bgp_delete_announce_set(&opctx, &sel).await?;
         Ok(HttpResponseUpdatedNoContent {})
@@ -4122,6 +4128,40 @@ async fn networking_bgp_announce_set_delete(
         .await
 }
 
+// TODO: is pagination necessary here? How large do we expect the list of
+// announcements to become in real usage?
+/// Get originated routes for a specified BGP announce set
+#[endpoint {
+    method = GET,
+    path = "/v1/system/networking/bgp-announce-set/{name_or_id}/announcement",
+    tags = ["system/networking"],
+}]
+async fn networking_bgp_announcement_list(
+    rqctx: RequestContext<ApiContext>,
+    path_params: Path<params::BgpAnnounceSetSelector>,
+) -> Result<HttpResponseOk<Vec<BgpAnnouncement>>, HttpError> {
+    let apictx = rqctx.context();
+    let handler = async {
+        let nexus = &apictx.context.nexus;
+        let sel = path_params.into_inner();
+        let opctx = crate::context::op_context_for_external_api(&rqctx).await?;
+
+        let result = nexus
+            .bgp_announcement_list(&opctx, &sel)
+            .await?
+            .into_iter()
+            .map(|p| p.into())
+            .collect();
+
+        Ok(HttpResponseOk(result))
+    };
+    apictx
+        .context
+        .external_latencies
+        .instrument_dropshot_handler(&rqctx, handler)
+        .await
+}
+
 /// Enable a BFD session
 #[endpoint {
     method = POST,

nexus/tests/integration_tests/endpoints.rs

@@ -573,7 +573,7 @@ pub static DEMO_BGP_CONFIG: Lazy<params::BgpConfigCreate> =
         shaper: None,
     });
 pub const DEMO_BGP_ANNOUNCE_SET_URL: &'static str =
-    "/v1/system/networking/bgp-announce?name_or_id=a-bag-of-addrs";
+    "/v1/system/networking/bgp-announce-set";
 pub static DEMO_BGP_ANNOUNCE: Lazy<params::BgpAnnounceSetCreate> =
     Lazy::new(|| params::BgpAnnounceSetCreate {
         identity: IdentityMetadataCreateParams {
@@ -585,6 +585,10 @@ pub static DEMO_BGP_ANNOUNCE: Lazy<params::BgpAnnounceSetCreate> =
             network: "10.0.0.0/16".parse().unwrap(),
         }],
     });
+pub const DEMO_BGP_ANNOUNCE_SET_DELETE_URL: &'static str =
+    "/v1/system/networking/bgp-announce-set/a-bag-of-addrs";
+pub const DEMO_BGP_ANNOUNCEMENT_URL: &'static str =
+    "/v1/system/networking/bgp-announce-set/a-bag-of-addrs/announcement";
 pub const DEMO_BGP_STATUS_URL: &'static str =
     "/v1/system/networking/bgp-status";
 pub const DEMO_BGP_EXPORTED_URL: &'static str =
@@ -2274,6 +2278,7 @@ pub static VERIFY_ENDPOINTS: Lazy<Vec<VerifyEndpoint>> = Lazy::new(|| {
                 AllowedMethod::GetNonexistent
             ],
         },
+
         VerifyEndpoint {
             url: &DEMO_BGP_CONFIG_CREATE_URL,
             visibility: Visibility::Public,
@@ -2295,11 +2300,28 @@ pub static VERIFY_ENDPOINTS: Lazy<Vec<VerifyEndpoint>> = Lazy::new(|| {
                 AllowedMethod::Put(
                     serde_json::to_value(&*DEMO_BGP_ANNOUNCE).unwrap(),
                 ),
-                AllowedMethod::GetNonexistent,
+                AllowedMethod::Get,
+            ],
+        },
+
+        VerifyEndpoint {
+            url: &DEMO_BGP_ANNOUNCE_SET_DELETE_URL,
+            visibility: Visibility::Public,
+            unprivileged_access: UnprivilegedAccess::None,
+            allowed_methods: vec![
                 AllowedMethod::Delete
             ],
         },
 
+        VerifyEndpoint {
+            url: &DEMO_BGP_ANNOUNCEMENT_URL,
+            visibility: Visibility::Public,
+            unprivileged_access: UnprivilegedAccess::None,
+            allowed_methods: vec![
+                AllowedMethod::GetNonexistent,
+            ],
+        },
+
         VerifyEndpoint {
             url: &DEMO_BGP_STATUS_URL,
             visibility: Visibility::Public,

nexus/tests/integration_tests/switch_port.rs

@@ -76,7 +76,7 @@ async fn test_port_settings_basic_crud(ctx: &ControlPlaneTestContext) {
 
     NexusRequest::objects_post(
         client,
-        "/v1/system/networking/bgp-announce",
+        "/v1/system/networking/bgp-announce-set",
         &announce_set,
     )
     .authn_as(AuthnMode::PrivilegedUser)

nexus/tests/output/nexus_tags.txt

@@ -178,9 +178,10 @@ networking_allow_list_view               GET      /v1/system/networking/allow-li
 networking_bfd_disable                   POST     /v1/system/networking/bfd-disable
 networking_bfd_enable                    POST     /v1/system/networking/bfd-enable
 networking_bfd_status                    GET      /v1/system/networking/bfd-status
-networking_bgp_announce_set_delete       DELETE   /v1/system/networking/bgp-announce
-networking_bgp_announce_set_list         GET      /v1/system/networking/bgp-announce
-networking_bgp_announce_set_update       PUT      /v1/system/networking/bgp-announce
+networking_bgp_announce_set_delete       DELETE   /v1/system/networking/bgp-announce-set/{name_or_id}
+networking_bgp_announce_set_list         GET      /v1/system/networking/bgp-announce-set
+networking_bgp_announce_set_update       PUT      /v1/system/networking/bgp-announce-set
+networking_bgp_announcement_list         GET      /v1/system/networking/bgp-announce-set/{name_or_id}/announcement
 networking_bgp_config_create             POST     /v1/system/networking/bgp
 networking_bgp_config_delete             DELETE   /v1/system/networking/bgp
 networking_bgp_config_list               GET      /v1/system/networking/bgp

nexus/types/src/external_api/params.rs

@@ -1629,6 +1629,13 @@ pub struct BgpAnnounceSetCreate {
     pub announcement: Vec<BgpAnnouncementCreate>,
 }
 
+/// Optionally select a BGP announce set by a name or id.
+#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq)]
+pub struct OptionalBgpAnnounceSetSelector {
+    /// A name or id to use when s electing BGP port settings
+    pub name_or_id: Option<NameOrId>,
+}
+
 /// Select a BGP announce set by a name or id.
 #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq)]
 pub struct BgpAnnounceSetSelector {