add make_default endpoint and give it one lil test

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

@@ -21,9 +21,11 @@ use crate::db::model::{
 use crate::db::pagination::paginated;
 use crate::db::pool::DbConnection;
 use crate::db::queries::ip_pool::FilterOverlappingIpRanges;
-use async_bb8_diesel::AsyncRunQueryDsl;
+use crate::db::TransactionError;
+use async_bb8_diesel::{AsyncConnection, AsyncRunQueryDsl};
 use chrono::Utc;
 use diesel::prelude::*;
+use diesel::result::Error as DieselError;
 use ipnetwork::IpNetwork;
 use nexus_db_model::ExternalIp;
 use nexus_db_model::IpPoolResourceType;
@@ -362,6 +364,8 @@ impl DataStore {
         .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))
     }
 
+    // TODO: separate this operation from update so that we can have /link 409
+    // or whatever when the association already exists?
     pub async fn ip_pool_associate_resource(
         &self,
         opctx: &OpContext,
@@ -408,6 +412,84 @@ impl DataStore {
             })
     }
 
+    // TODO: make default should fail when the association doesn't exist.
+    // should it also fail when it's already default? probably not?
+    pub async fn ip_pool_make_default(
+        &self,
+        opctx: &OpContext,
+        authz_ip_pool: &authz::IpPool,
+        authz_silo: &authz::Silo,
+    ) -> UpdateResult<IpPoolResource> {
+        use db::schema::ip_pool_resource::dsl;
+
+        // TODO: correct auth check
+        opctx
+            .authorize(authz::Action::CreateChild, &authz::IP_POOL_LIST)
+            .await?;
+
+        let ip_pool_id = authz_ip_pool.id();
+        let silo_id = authz_silo.id();
+
+        // Errors returned from the below transactions.
+        #[derive(Debug)]
+        enum IpPoolResourceUpdateError {
+            FailedToUnsetDefault(DieselError),
+        }
+        type TxnError = TransactionError<IpPoolResourceUpdateError>;
+
+        let conn = self.pool_connection_authorized(opctx).await?;
+        conn.transaction_async(|conn| async move {
+            // note this is matching the specified silo, but could be any pool
+            let existing_default_for_silo = dsl::ip_pool_resource
+                .filter(dsl::resource_type.eq(IpPoolResourceType::Silo))
+                .filter(dsl::resource_id.eq(silo_id))
+                .filter(dsl::is_default.eq(true))
+                .select(IpPoolResource::as_select())
+                .get_result_async(&conn)
+                .await;
+
+            // if there is an existing default, we need to unset it before we can
+            // set the new default
+            if let Ok(existing_default) = existing_default_for_silo {
+                // if the pool we're making default is already default for this
+                // silo, don't error: just noop
+                if existing_default.ip_pool_id == ip_pool_id {
+                    return Ok(existing_default);
+                }
+
+                let unset_default = diesel::update(dsl::ip_pool_resource)
+                    .filter(dsl::resource_id.eq(existing_default.resource_id))
+                    .filter(dsl::ip_pool_id.eq(existing_default.ip_pool_id))
+                    .filter(
+                        dsl::resource_type.eq(existing_default.resource_type),
+                    )
+                    .set(dsl::is_default.eq(false))
+                    .execute_async(&conn)
+                    .await;
+                if let Err(e) = unset_default {
+                    return Err(TxnError::CustomError(
+                        IpPoolResourceUpdateError::FailedToUnsetDefault(e),
+                    ));
+                }
+            }
+
+            // TODO: test that this errors if the link doesn't exist already
+            let updated_link = diesel::update(dsl::ip_pool_resource)
+                .filter(dsl::resource_id.eq(silo_id))
+                .filter(dsl::ip_pool_id.eq(ip_pool_id))
+                .filter(dsl::resource_type.eq(IpPoolResourceType::Silo))
+                .set(dsl::is_default.eq(true))
+                .returning(IpPoolResource::as_returning())
+                .get_result_async(&conn)
+                .await?;
+            Ok(updated_link)
+        })
+        .await
+        .map_err(|e| {
+            Error::internal_error(&format!("Transaction error: {:?}", e))
+        })
+    }
+
     // TODO: write a test for this
     async fn ensure_no_ips_outstanding(
         &self,

nexus/db-queries/src/db/queries/external_ip.rs

@@ -886,7 +886,7 @@ mod tests {
             self.db_datastore
                 .ip_pool_associate_resource(&self.opctx, association)
                 .await
-                .expect("Failed to associate IP dool with silo");
+                .expect("Failed to associate IP pool with silo");
 
             self.initialize_ip_pool(name, range).await;
         }
@@ -1712,6 +1712,8 @@ mod tests {
             Ipv4Addr::new(10, 0, 0, 6),
         ))
         .unwrap();
+        // TODO: failing because I changed create_ip_pool to make it
+        // default for the silo, and there is already a default
         context.create_ip_pool("p1", second_range).await;
 
         // Allocating an address on an instance in the second pool should be

nexus/src/app/ip_pool.rs

@@ -135,6 +135,21 @@ impl super::Nexus {
             .await
     }
 
+    pub(crate) async fn ip_pool_make_default(
+        &self,
+        opctx: &OpContext,
+        pool_lookup: &lookup::IpPool<'_>,
+        silo_lookup: &lookup::Silo<'_>,
+    ) -> CreateResult<db::model::IpPoolResource> {
+        let (.., authz_pool) =
+            pool_lookup.lookup_for(authz::Action::Modify).await?;
+        let (.., authz_silo) =
+            silo_lookup.lookup_for(authz::Action::Read).await?;
+        self.db_datastore
+            .ip_pool_make_default(opctx, &authz_pool, &authz_silo)
+            .await
+    }
+
     pub(crate) async fn ip_pools_list(
         &self,
         opctx: &OpContext,

nexus/src/external_api/http_entrypoints.rs

@@ -125,6 +125,7 @@ pub(crate) fn external_api() -> NexusApiDescription {
         api.register(ip_pool_silo_list)?;
         api.register(ip_pool_silo_link)?;
         api.register(ip_pool_silo_unlink)?;
+        api.register(ip_pool_make_default)?;
         api.register(ip_pool_view)?;
         api.register(ip_pool_delete)?;
         api.register(ip_pool_update)?;
@@ -1399,7 +1400,7 @@ async fn ip_pool_silo_link(
 async fn ip_pool_silo_unlink(
     rqctx: RequestContext<Arc<ServerContext>>,
     path_params: Path<params::IpPoolPath>,
-    query_params: Query<params::IpPoolSiloUnlink>,
+    query_params: Query<params::SiloSelector>,
 ) -> Result<HttpResponseUpdatedNoContent, HttpError> {
     let apictx = rqctx.context();
     let handler = async {
@@ -1418,6 +1419,36 @@ async fn ip_pool_silo_unlink(
     apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await
 }
 
+// TODO: change this to PUT /ip-pools/{pool}/silos/{silo} so
+// it can be used for both set default true and false
+
+/// Make an IP pool the default for a silo
+#[endpoint {
+    method = POST,
+    path = "/v1/system/ip-pools/{pool}/make_default",
+    tags = ["system/networking"],
+}]
+async fn ip_pool_make_default(
+    rqctx: RequestContext<Arc<ServerContext>>,
+    path_params: Path<params::IpPoolPath>,
+    silo_selector: TypedBody<params::SiloSelector>,
+) -> Result<HttpResponseCreated<views::IpPoolSilo>, HttpError> {
+    let apictx = rqctx.context();
+    let handler = async {
+        let opctx = crate::context::op_context_for_external_api(&rqctx).await?;
+        let nexus = &apictx.nexus;
+        let path = path_params.into_inner();
+        let silo_selector = silo_selector.into_inner();
+        let pool_lookup = nexus.ip_pool_lookup(&opctx, &path.pool)?;
+        let silo_lookup = nexus.silo_lookup(&opctx, silo_selector.silo)?;
+        let assoc = nexus
+            .ip_pool_make_default(&opctx, &pool_lookup, &silo_lookup)
+            .await?;
+        Ok(HttpResponseCreated(assoc.into()))
+    };
+    apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await
+}
+
 /// Fetch the IP pool used for Oxide services
 #[endpoint {
     method = GET,

nexus/tests/integration_tests/instances.rs

@@ -3583,35 +3583,45 @@ async fn test_instance_ephemeral_ip_from_correct_pool(
     );
 
     // make first pool the default for the priv user's silo
-    let link = params::IpPoolSiloLink {
-        silo: NameOrId::Id(DEFAULT_SILO.id()),
-        is_default: true,
-    };
-    create_ip_pool(&client, "default", Some(range1), Some(link)).await;
+    dbg!(DEFAULT_SILO.id());
+    let silo = NameOrId::Id(DEFAULT_SILO.id());
+    let link = params::IpPoolSiloLink { silo: silo.clone(), is_default: true };
+    create_ip_pool(&client, "pool1", Some(range1), Some(link)).await;
 
     // second pool is associated with the silo but not default
-    let link = params::IpPoolSiloLink {
-        silo: NameOrId::Id(DEFAULT_SILO.id()),
-        is_default: false,
-    };
-    create_ip_pool(&client, "other-pool", Some(range2), Some(link)).await;
+    let link = params::IpPoolSiloLink { silo: silo.clone(), is_default: false };
+    create_ip_pool(&client, "pool2", Some(range2), Some(link)).await;
 
     // Create an instance with pool name blank, expect IP from default pool
-    create_instance_with_pool(client, "default-pool-inst", None).await;
+    create_instance_with_pool(client, "pool1-inst", None).await;
 
-    let ip = fetch_instance_ephemeral_ip(client, "default-pool-inst").await;
+    let ip = fetch_instance_ephemeral_ip(client, "pool1-inst").await;
     assert!(
         ip.ip >= range1.first_address() && ip.ip <= range1.last_address(),
-        "Expected ephemeral IP to come from default pool"
+        "Expected ephemeral IP to come from pool1"
     );
 
     // Create an instance explicitly using the non-default "other-pool".
-    create_instance_with_pool(client, "other-pool-inst", Some("other-pool"))
-        .await;
-    let ip = fetch_instance_ephemeral_ip(client, "other-pool-inst").await;
+    create_instance_with_pool(client, "pool2-inst", Some("pool2")).await;
+    let ip = fetch_instance_ephemeral_ip(client, "pool2-inst").await;
+    assert!(
+        ip.ip >= range2.first_address() && ip.ip <= range2.last_address(),
+        "Expected ephemeral IP to come from pool2"
+    );
+
+    // make pool2 default and create instance with default pool. check that it now it comes from pool2
+    let _: views::IpPoolSilo = object_create(
+        client,
+        "/v1/system/ip-pools/pool2/make_default",
+        &params::SiloSelector { silo: silo.clone() },
+    )
+    .await;
+
+    create_instance_with_pool(client, "pool2-inst2", None).await;
+    let ip = fetch_instance_ephemeral_ip(client, "pool2-inst2").await;
     assert!(
         ip.ip >= range2.first_address() && ip.ip <= range2.last_address(),
-        "Expected ephemeral IP to come from other pool"
+        "Expected ephemeral IP to come from pool2"
     );
 }
 

nexus/tests/integration_tests/ip_pools.rs

@@ -30,6 +30,7 @@ use nexus_types::external_api::params::InstanceNetworkInterfaceAttachment;
 use nexus_types::external_api::params::IpPoolCreate;
 use nexus_types::external_api::params::IpPoolSiloLink;
 use nexus_types::external_api::params::IpPoolUpdate;
+use nexus_types::external_api::params::SiloSelector;
 use nexus_types::external_api::shared::IpRange;
 use nexus_types::external_api::shared::Ipv4Range;
 use nexus_types::external_api::shared::Ipv6Range;
@@ -351,7 +352,7 @@ async fn test_ip_pool_with_silo(cptestctx: &ControlPlaneTestContext) {
     let p1 = create_pool(client, "p1").await;
 
     // there should be no associations
-    let assocs_p0 = get_associations(client, "p0").await;
+    let assocs_p0 = silos_for_pool(client, "p0").await;
     assert_eq!(assocs_p0.items.len(), 0);
 
     // expect 404 on association if the specified silo doesn't exist
@@ -383,46 +384,62 @@ async fn test_ip_pool_with_silo(cptestctx: &ControlPlaneTestContext) {
     );
 
     // associate by name with silo that exists
-    let params = params::IpPoolSiloLink {
-        silo: NameOrId::Name(cptestctx.silo_name.clone()),
-        is_default: false,
-    };
+    let silo = NameOrId::Name(cptestctx.silo_name.clone());
+    let params =
+        params::IpPoolSiloLink { silo: silo.clone(), is_default: false };
     let _: IpPoolSilo =
         object_create(client, "/v1/system/ip-pools/p0/silos", &params).await;
 
     // get silo ID so we can test association by ID as well
     let silo_url = format!("/v1/system/silos/{}", cptestctx.silo_name);
-    let silo = NexusRequest::object_get(client, &silo_url)
+    let silo_id = NexusRequest::object_get(client, &silo_url)
         .authn_as(AuthnMode::PrivilegedUser)
         .execute_and_parse_unwrap::<Silo>()
-        .await;
-    let silo_id = silo.identity.id;
+        .await
+        .identity
+        .id;
 
-    let assocs_p0 = get_associations(client, "p0").await;
-    let silo_assoc =
+    let assocs_p0 = silos_for_pool(client, "p0").await;
+    let silo_link =
         IpPoolSilo { ip_pool_id: p0.identity.id, silo_id, is_default: false };
     assert_eq!(assocs_p0.items.len(), 1);
-    assert_eq!(assocs_p0.items[0], silo_assoc);
+    assert_eq!(assocs_p0.items[0], silo_link);
 
     // TODO: dissociate silo
     // TODO: confirm dissociation
 
     // associate same silo to other pool by ID
     let params = params::IpPoolSiloLink {
-        silo: NameOrId::Id(silo.identity.id),
+        silo: NameOrId::Id(silo_id),
         is_default: false,
     };
     let _: IpPoolSilo =
         object_create(client, "/v1/system/ip-pools/p1/silos", &params).await;
 
     // association should look the same as the other one, except different pool ID
-    let assocs_p1 = get_associations(client, "p1").await;
-    assert_eq!(assocs_p1.items.len(), 1);
+    let silos_p1 = silos_for_pool(client, "p1").await;
+    assert_eq!(silos_p1.items.len(), 1);
     assert_eq!(
-        assocs_p1.items[0],
-        IpPoolSilo { ip_pool_id: p1.identity.id, ..silo_assoc }
+        silos_p1.items[0],
+        IpPoolSilo { ip_pool_id: p1.identity.id, ..silo_link }
     );
 
+    // make p0's pool default and show that it changes
+    let params = SiloSelector { silo: silo.clone() };
+    let _: IpPoolSilo =
+        object_create(client, "/v1/system/ip-pools/p0/make_default", &params)
+            .await;
+    // making the same one default again is not an error
+    let _: IpPoolSilo =
+        object_create(client, "/v1/system/ip-pools/p0/make_default", &params)
+            .await;
+
+    let silos_p0 = silos_for_pool(client, "p0").await;
+    assert_eq!(silos_p0.items.len(), 1);
+    assert_eq!(silos_p0.items[0].is_default, true);
+
+    // TODO: unset default
+
     // TODO: associating a resource that is already associated should be a noop
     // and return a success message
 
@@ -471,15 +488,12 @@ fn get_names(pools: Vec<IpPool>) -> Vec<String> {
     pools.iter().map(|p| p.identity.name.to_string()).collect()
 }
 
-async fn get_associations(
+async fn silos_for_pool(
     client: &ClientTestContext,
     id: &str,
 ) -> ResultsPage<IpPoolSilo> {
-    objects_list_page_authz::<IpPoolSilo>(
-        client,
-        &format!("/v1/system/ip-pools/{}/silos", id),
-    )
-    .await
+    let url = format!("/v1/system/ip-pools/{}/silos", id);
+    objects_list_page_authz::<IpPoolSilo>(client, &url).await
 }
 
 async fn create_pool(client: &ClientTestContext, name: &str) -> IpPool {

nexus/types/src/external_api/params.rs

@@ -763,11 +763,6 @@ pub struct IpPoolSiloLink {
     pub is_default: bool,
 }
 
-#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
-pub struct IpPoolSiloUnlink {
-    pub silo: NameOrId,
-}
-
 // INSTANCES
 
 /// Describes an attachment of an `InstanceNetworkInterface` to an `Instance`,