change API to allow associating silo by name. it's beautiful

nexus/db-model/src/ip_pool.rs

@@ -89,15 +89,6 @@ impl_enum_type!(
      Silo => b"silo"
 );
 
-impl From<params::IpPoolResourceType> for IpPoolResourceType {
-    fn from(typ: params::IpPoolResourceType) -> Self {
-        match typ {
-            params::IpPoolResourceType::Fleet => IpPoolResourceType::Fleet,
-            params::IpPoolResourceType::Silo => IpPoolResourceType::Silo,
-        }
-    }
-}
-
 #[derive(Queryable, Insertable, Selectable, Clone, Debug)]
 #[diesel(table_name = ip_pool_resource)]
 pub struct IpPoolResource {

nexus/src/app/ip_pool.rs

@@ -10,6 +10,7 @@ use ipnetwork::IpNetwork;
 use nexus_db_queries::authz;
 use nexus_db_queries::context::OpContext;
 use nexus_db_queries::db;
+use nexus_db_queries::db::fixed_data::FLEET_ID;
 // use nexus_db_queries::db::fixed_data::silo::INTERNAL_SILO_ID;
 use nexus_db_queries::db::lookup;
 use nexus_db_queries::db::lookup::LookupPath;
@@ -75,35 +76,40 @@ impl super::Nexus {
         &self,
         opctx: &OpContext,
         pool_lookup: &lookup::IpPool<'_>,
-        ip_pool_resource: &params::IpPoolAssociationCreate,
+        assoc_create: &params::IpPoolAssociationCreate,
     ) -> CreateResult<db::model::IpPoolResource> {
         // TODO: check for perms on specified resource? or unnecessary because this is an operator action?
         let (.., authz_pool) =
             pool_lookup.lookup_for(authz::Action::Modify).await?;
-        match ip_pool_resource.resource_type {
-            params::IpPoolResourceType::Silo => {
-                self.silo_lookup(
-                    &opctx,
-                    NameOrId::Id(ip_pool_resource.resource_id),
-                )?
-                .lookup_for(authz::Action::Read)
-                .await?;
+        let (resource_type, resource_id, is_default) = match assoc_create {
+            params::IpPoolAssociationCreate::Silo(assoc_silo) => {
+                let (silo,) = self
+                    .silo_lookup(&opctx, assoc_silo.silo.clone())?
+                    .lookup_for(authz::Action::Read)
+                    .await?;
+                (
+                    db::model::IpPoolResourceType::Silo,
+                    silo.id(),
+                    assoc_silo.is_default,
+                )
             }
-            params::IpPoolResourceType::Fleet => {
-                // hope we don't need to be assured of the fleet's existence
+            params::IpPoolAssociationCreate::Fleet(assoc_fleet) => {
+                // we don't need to be assured of the fleet's existence
+                (
+                    db::model::IpPoolResourceType::Fleet,
+                    *FLEET_ID,
+                    assoc_fleet.is_default,
+                )
             }
         };
         self.db_datastore
             .ip_pool_associate_resource(
                 opctx,
                 db::model::IpPoolResource {
                     ip_pool_id: authz_pool.id(),
-                    resource_type: ip_pool_resource
-                        .resource_type
-                        .clone()
-                        .into(),
-                    resource_id: ip_pool_resource.resource_id,
-                    is_default: ip_pool_resource.is_default,
+                    resource_type,
+                    resource_id,
+                    is_default,
                 },
             )
             .await

nexus/test-utils/src/resource_helpers.rs

@@ -12,7 +12,6 @@ use dropshot::test_util::ClientTestContext;
 use dropshot::HttpErrorResponseBody;
 use dropshot::Method;
 use http::StatusCode;
-use nexus_db_queries::db::fixed_data::FLEET_ID;
 use nexus_test_interface::NexusServer;
 use nexus_types::external_api::params;
 use nexus_types::external_api::params::PhysicalDiskKind;
@@ -149,11 +148,9 @@ pub async fn create_ip_pool(
     let _assoc: views::IpPoolResource = object_create(
         client,
         &format!("/v1/system/ip-pools/{pool_name}/association"),
-        &params::IpPoolAssociationCreate {
-            resource_id: *FLEET_ID,
-            resource_type: params::IpPoolResourceType::Fleet,
+        &params::IpPoolAssociationCreate::Fleet(params::IpPoolAssociateFleet {
             is_default: false,
-        },
+        }),
     )
     .await;
 

nexus/tests/integration_tests/endpoints.rs

@@ -469,11 +469,10 @@ lazy_static! {
         };
     pub static ref DEMO_IP_POOL_ASSOC_URL: String = format!("{}/association", *DEMO_IP_POOL_URL);
     pub static ref DEMO_IP_POOL_ASSOC_BODY: params::IpPoolAssociationCreate =
-        params::IpPoolAssociationCreate {
-            resource_id:  DEFAULT_SILO.identity().id,
-            resource_type: params::IpPoolResourceType::Silo,
+        params::IpPoolAssociationCreate::Silo(params::IpPoolAssociateSilo {
+            silo: NameOrId::Id(DEFAULT_SILO.identity().id),
             is_default: false,
-        };
+        });
     pub static ref DEMO_IP_POOL_RANGE: IpRange = IpRange::V4(Ipv4Range::new(
         std::net::Ipv4Addr::new(10, 0, 0, 0),
         std::net::Ipv4Addr::new(10, 0, 0, 255),

nexus/tests/integration_tests/instances.rs

@@ -44,6 +44,7 @@ use omicron_common::api::external::InstanceNetworkInterface;
 use omicron_common::api::external::InstanceState;
 use omicron_common::api::external::Ipv4Net;
 use omicron_common::api::external::Name;
+use omicron_common::api::external::NameOrId;
 use omicron_common::api::external::Vni;
 use omicron_nexus::app::MAX_MEMORY_BYTES_PER_INSTANCE;
 use omicron_nexus::app::MAX_VCPU_PER_INSTANCE;
@@ -3493,11 +3494,11 @@ async fn test_instance_ephemeral_ip_from_correct_pool(
         },
     )
     .await;
-    let params = params::IpPoolAssociationCreate {
-        resource_id: DEFAULT_SILO.id(),
-        resource_type: params::IpPoolResourceType::Silo,
-        is_default: true,
-    };
+    let params =
+        params::IpPoolAssociationCreate::Silo(params::IpPoolAssociateSilo {
+            silo: NameOrId::Id(DEFAULT_SILO.id()),
+            is_default: true,
+        });
     let assoc_url = format!("/v1/system/ip-pools/{pool_name}/association");
     let _ = NexusRequest::objects_post(client, &assoc_url, &params)
         .authn_as(AuthnMode::PrivilegedUser)

nexus/tests/integration_tests/ip_pools.rs

@@ -9,7 +9,6 @@ use dropshot::HttpErrorResponseBody;
 use http::method::Method;
 use http::StatusCode;
 use nexus_db_queries::db::datastore::SERVICE_IP_POOL_NAME;
-use nexus_db_queries::db::fixed_data::FLEET_ID;
 use nexus_test_utils::http_testing::AuthnMode;
 use nexus_test_utils::http_testing::NexusRequest;
 use nexus_test_utils::http_testing::RequestBuilder;
@@ -33,7 +32,7 @@ use nexus_types::external_api::views::IpPool;
 use nexus_types::external_api::views::IpPoolRange;
 use nexus_types::external_api::views::IpPoolResource;
 use omicron_common::api::external::IdentityMetadataUpdateParams;
-// use omicron_common::api::external::NameOrId;
+use omicron_common::api::external::NameOrId;
 use omicron_common::api::external::{IdentityMetadataCreateParams, Name};
 use omicron_nexus::TestInterfaces;
 use sled_agent_client::TestInterfaces as SledTestInterfaces;
@@ -360,11 +359,11 @@ async fn test_ip_pool_with_silo(cptestctx: &ControlPlaneTestContext) {
 
     // expect 404 on association if the specified silo doesn't exist
     let nonexistent_silo_id = Uuid::new_v4();
-    let params = params::IpPoolAssociationCreate {
-        resource_id: nonexistent_silo_id,
-        resource_type: params::IpPoolResourceType::Silo,
-        is_default: false,
-    };
+    let params =
+        params::IpPoolAssociationCreate::Silo(params::IpPoolAssociateSilo {
+            silo: NameOrId::Id(nonexistent_silo_id),
+            is_default: false,
+        });
     let error = NexusRequest::new(
         RequestBuilder::new(
             client,
@@ -387,11 +386,9 @@ async fn test_ip_pool_with_silo(cptestctx: &ControlPlaneTestContext) {
     );
 
     // associate with silo that exists
-    // let params = params::IpPoolAssociate {
-    //     resource_id: *FLEET_ID,
-    //     resource_type: params::IpPoolResourceType::Silo,
+    // let params = params::IpPoolAssociationCreate::Fleet(params::IpPoolAssociateFleet {
     //     is_default: false,
-    // };
+    // });
     // let _: IpPoolResource = object_create(
     //     client,
     //     &format!("/v1/system/ip-pools/p1/association"),
@@ -822,11 +819,10 @@ async fn test_ip_pool_list_usable_by_project(
     // add to fleet since we can't add to project yet
     // TODO: could do silo, might as well? need the ID, though. at least
     // until I make it so you can specify the resource by name
-    let params = params::IpPoolAssociationCreate {
-        resource_id: *FLEET_ID,
-        resource_type: params::IpPoolResourceType::Fleet,
-        is_default: false,
-    };
+    let params =
+        params::IpPoolAssociationCreate::Fleet(params::IpPoolAssociateFleet {
+            is_default: false,
+        });
     let _: IpPoolResource = object_create(
         client,
         &format!("/v1/system/ip-pools/{mypool_name}/association"),

nexus/types/src/external_api/params.rs

@@ -741,38 +741,23 @@ pub struct IpPoolUpdate {
 }
 
 #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
-#[serde(rename_all = "snake_case")]
-pub enum IpPoolResourceType {
-    Fleet,
-    Silo,
+pub struct IpPoolAssociateSilo {
+    pub silo: NameOrId,
+    pub is_default: bool,
 }
 
-/// Parameters for associating an IP pool with a resource (fleet, silo)
 #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
-pub struct IpPoolAssociationCreate {
-    pub resource_id: Uuid,
-    pub resource_type: IpPoolResourceType,
+pub struct IpPoolAssociateFleet {
     pub is_default: bool,
 }
 
-// #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
-// pub struct IpPoolAssociateSilo {
-//     pub silo: NameOrId,
-//     pub is_default: bool,
-// }
-//
-// #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
-// pub struct IpPoolAssociateFleet {
-//     pub is_default: bool,
-// }
-//
-// TODO: IpPoolAssociate as tagged enum
-// #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
-// #[serde(tag = "resource_type", rename_all = "snake_case")]
-// pub enum IpPoolAssociate {
-//     Fleet(IpPoolAssociateFleet),
-//     Silo(IpPoolAssociateFleet),
-// }
+/// Parameters for associating an IP pool with a resource (fleet, silo)
+#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
+#[serde(tag = "resource_type", rename_all = "snake_case")]
+pub enum IpPoolAssociationCreate {
+    Silo(IpPoolAssociateSilo),
+    Fleet(IpPoolAssociateFleet),
+}
 
 #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
 pub struct IpPoolAssociationDelete {

openapi/nexus.json

@@ -11627,23 +11627,47 @@
       },
       "IpPoolAssociationCreate": {
         "description": "Parameters for associating an IP pool with a resource (fleet, silo)",
-        "type": "object",
-        "properties": {
-          "is_default": {
-            "type": "boolean"
-          },
-          "resource_id": {
-            "type": "string",
-            "format": "uuid"
+        "oneOf": [
+          {
+            "type": "object",
+            "properties": {
+              "is_default": {
+                "type": "boolean"
+              },
+              "resource_type": {
+                "type": "string",
+                "enum": [
+                  "silo"
+                ]
+              },
+              "silo": {
+                "$ref": "#/components/schemas/NameOrId"
+              }
+            },
+            "required": [
+              "is_default",
+              "resource_type",
+              "silo"
+            ]
           },
-          "resource_type": {
-            "$ref": "#/components/schemas/IpPoolResourceType"
+          {
+            "type": "object",
+            "properties": {
+              "is_default": {
+                "type": "boolean"
+              },
+              "resource_type": {
+                "type": "string",
+                "enum": [
+                  "fleet"
+                ]
+              }
+            },
+            "required": [
+              "is_default",
+              "resource_type"
+            ]
           }
-        },
-        "required": [
-          "is_default",
-          "resource_id",
-          "resource_type"
         ]
       },
       "IpPoolCreate": {
@@ -11712,13 +11736,6 @@
       "IpPoolResource": {
         "type": "object"
       },
-      "IpPoolResourceType": {
-        "type": "string",
-        "enum": [
-          "fleet",
-          "silo"
-        ]
-      },
       "IpPoolResultsPage": {
         "description": "A single page of results",
         "type": "object",