diff --git a/nexus/db-model/src/external_ip.rs b/nexus/db-model/src/external_ip.rs index e79dabb612..40ad3e2b92 100644 --- a/nexus/db-model/src/external_ip.rs +++ b/nexus/db-model/src/external_ip.rs @@ -372,7 +372,7 @@ impl TryFrom for views::ExternalIp { } } -impl TryFrom for views::FloatingIp { +impl TryFrom for FloatingIp { type Error = Error; fn try_from(ip: ExternalIp) -> Result { @@ -402,23 +402,35 @@ impl TryFrom for views::FloatingIp { "database schema guarantees ID metadata for non-service FIP", ))?; - let identity = IdentityMetadata { + let identity = FloatingIpIdentity { id: ip.id, name, description, time_created: ip.time_created, time_modified: ip.time_modified, + time_deleted: ip.time_deleted, }; - Ok(views::FloatingIp { - ip: ip.ip.ip(), + Ok(FloatingIp { + ip: ip.ip, identity, project_id, - instance_id: ip.parent_id, + ip_pool_id: ip.ip_pool_id, + ip_pool_range_id: ip.ip_pool_range_id, + is_service: ip.is_service, + parent_id: ip.parent_id, }) } } +impl TryFrom for views::FloatingIp { + type Error = Error; + + fn try_from(ip: ExternalIp) -> Result { + FloatingIp::try_from(ip).map(Into::into) + } +} + impl From for views::FloatingIp { fn from(ip: FloatingIp) -> Self { let identity = IdentityMetadata { diff --git a/nexus/db-queries/src/db/datastore/external_ip.rs b/nexus/db-queries/src/db/datastore/external_ip.rs index 9ac72705a8..a624e5f5b7 100644 --- a/nexus/db-queries/src/db/datastore/external_ip.rs +++ b/nexus/db-queries/src/db/datastore/external_ip.rs @@ -32,7 +32,7 @@ use omicron_common::api::external::DeleteResult; use omicron_common::api::external::Error; use omicron_common::api::external::ListResultVec; use omicron_common::api::external::LookupResult; -use omicron_common::api::external::NameOrId; +use omicron_common::api::external::ResourceType; use omicron_common::api::external::UpdateResult; use ref_cast::RefCast; use std::net::IpAddr; @@ -153,8 +153,6 @@ impl DataStore { self.ip_pools_fetch_default(opctx).await?.id() }; - // XXX: Verify that chosen pool comes from my silo. - let data = if let Some(ip) = ip { IncompleteExternalIp::for_floating_explicit( ip_id, @@ -174,61 +172,7 @@ impl DataStore { ) }; - // TODO: need to disambiguate no IP and/or IP taken - // from resource name collision, and expose those in - // a nice way. self.allocate_external_ip(opctx, data).await - // .map_err(|e| { - // public_error_from_diesel( - // e, - // ErrorHandler::Conflict(todo!(), name.as_str()) - // ) - // }) - } - - /// Allocates a floating IP address for instance usage. - pub async fn attach_floating_ip_to_instance( - &self, - opctx: &OpContext, - _instance_id: Uuid, - _ip_id: &NameOrId, - _project: &authz::Project, - ) -> UpdateResult { - // use db::schema::external_ip::dsl; - // TODO: scope by project - // opctx.authorize(authz::Action::CreateChild, authz_project).await?; - let _conn = self.pool_connection_authorized(opctx).await?; - - // let ip_id = match ip_id { - // NameOrId::Id(id) => *id, - // NameOrId::Name(name) => { - // diesel::select(dsl::external_ip) - // .filter(dsl::time_deleted.is_null()) - // .filter(dsl::name.eq(&name)) - // .execute_and_check(&conn) - // .await - // .map(|m| m.) - // } - // }; - - // opctx.authn. - // todo!() - - // diesel::update(dsl::external_ip) - // .filter(dsl::time_deleted.is_null()) - // .filter(dsl::id.eq(Some(ip_id))) - // .filter(dsl::parent_id.is_null()) - // .set(dsl::parent_id.eq(instance_id)) - // .check_if_exists::(ip_id) - // .execute_and_check(&*self.pool_connection_authorized(opctx).await?) - // .await - // .map(|r| match r.status { - // UpdateStatus::Updated => true, - // UpdateStatus::NotUpdatedButExists => false, - // }) - // .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) - - todo!() } async fn allocate_external_ip( @@ -246,8 +190,12 @@ impl DataStore { conn: &async_bb8_diesel::Connection, data: IncompleteExternalIp, ) -> CreateResult { + // Name needs to be cloned out here (if present) to give users a + // sensible error message on name collision. + let name = data.name().clone(); let explicit_ip = data.explicit_ip().is_some(); NextExternalIp::new(data).get_result_async(conn).await.map_err(|e| { + use diesel::result::Error::DatabaseError; use diesel::result::Error::NotFound; match e { NotFound => { @@ -261,6 +209,17 @@ impl DataStore { ) } } + DatabaseError(..) if name.is_some() => { + public_error_from_diesel( + e, + ErrorHandler::Conflict( + ResourceType::FloatingIp, + name.as_ref() + .map(|m| m.as_str()) + .unwrap_or_default(), + ), + ) + } _ => crate::db::queries::external_ip::from_diesel(e), } }) @@ -476,4 +435,96 @@ impl DataStore { } Ok(()) } + + /// Attaches a Floating IP address to an instance. + pub async fn floating_ip_attach( + &self, + opctx: &OpContext, + authz_fip: &authz::FloatingIp, + db_fip: &FloatingIp, + instance_id: Uuid, + ) -> UpdateResult { + use db::schema::external_ip::dsl; + + // Verify this FIP is not attached to any instances/services. + if db_fip.parent_id.is_some() { + return Err(Error::invalid_request( + "Floating IP cannot be attached to one instance while still attached to another", + )); + } + + let (.., authz_instance, _db_instance) = LookupPath::new(&opctx, self) + .instance_id(instance_id) + .fetch_for(authz::Action::Modify) + .await?; + + opctx.authorize(authz::Action::Modify, authz_fip).await?; + opctx.authorize(authz::Action::Modify, &authz_instance).await?; + + diesel::update(dsl::external_ip) + .filter(dsl::id.eq(db_fip.id())) + .filter(dsl::kind.eq(IpKind::Floating)) + .filter(dsl::time_deleted.is_null()) + .filter(dsl::parent_id.is_null()) + .set(( + dsl::parent_id.eq(Some(instance_id)), + dsl::time_modified.eq(Utc::now()), + )) + .returning(ExternalIp::as_returning()) + .get_result_async(&*self.pool_connection_authorized(opctx).await?) + .await + .map_err(|e| { + public_error_from_diesel( + e, + ErrorHandler::NotFoundByResource(authz_fip), + ) + }) + .and_then(|r| FloatingIp::try_from(r)) + .map_err(|e| Error::internal_error(&format!("{e}"))) + } + + /// Detaches a Floating IP address from an instance. + pub async fn floating_ip_detach( + &self, + opctx: &OpContext, + authz_fip: &authz::FloatingIp, + db_fip: &FloatingIp, + ) -> UpdateResult { + use db::schema::external_ip::dsl; + + let Some(instance_id) = db_fip.parent_id else { + return Err(Error::invalid_request( + "Floating IP is not attached to an instance", + )); + }; + + let (.., authz_instance, _db_instance) = LookupPath::new(&opctx, self) + .instance_id(instance_id) + .fetch_for(authz::Action::Modify) + .await?; + + opctx.authorize(authz::Action::Modify, authz_fip).await?; + opctx.authorize(authz::Action::Modify, &authz_instance).await?; + + diesel::update(dsl::external_ip) + .filter(dsl::id.eq(db_fip.id())) + .filter(dsl::kind.eq(IpKind::Floating)) + .filter(dsl::time_deleted.is_null()) + .filter(dsl::parent_id.eq(instance_id)) + .set(( + dsl::parent_id.eq(Option::::None), + dsl::time_modified.eq(Utc::now()), + )) + .returning(ExternalIp::as_returning()) + .get_result_async(&*self.pool_connection_authorized(opctx).await?) + .await + .map_err(|e| { + public_error_from_diesel( + e, + ErrorHandler::NotFoundByResource(authz_fip), + ) + }) + .and_then(|r| FloatingIp::try_from(r)) + .map_err(|e| Error::internal_error(&format!("{e}"))) + } } diff --git a/nexus/src/app/sagas/instance_create.rs b/nexus/src/app/sagas/instance_create.rs index 87d9366780..e1c4f273a7 100644 --- a/nexus/src/app/sagas/instance_create.rs +++ b/nexus/src/app/sagas/instance_create.rs @@ -620,23 +620,35 @@ async fn sic_allocate_instance_external_ip( let ip_id = repeat_saga_params.new_id; // Collect the possible pool name for this IP address - let pool_name = match ip_params { + match ip_params { params::ExternalIpCreate::Ephemeral { ref pool_name } => { - pool_name.as_ref().map(|name| db::model::Name(name.clone())) + let pool_name = + pool_name.as_ref().map(|name| db::model::Name(name.clone())); + datastore + .allocate_instance_ephemeral_ip( + &opctx, + ip_id, + instance_id, + pool_name, + ) + .await + .map_err(ActionError::action_failed)?; } - params::ExternalIpCreate::Floating { ref floating_ip } => { - // In floating case, need: - // floating IP does not belong to another instance - // floating IP belongs to the parent project. - return Err(ActionError::action_failed(format!( - "can't yet bind floating ip {floating_ip:?} to instance" - ))); + params::ExternalIpCreate::Floating { ref floating_ip_name } => { + let floating_ip_name = db::model::Name(floating_ip_name.clone()); + let (.., authz_fip, db_fip) = LookupPath::new(&opctx, &datastore) + .project_id(saga_params.project_id) + .floating_ip_name(&floating_ip_name) + .fetch_for(authz::Action::Modify) + .await + .map_err(ActionError::action_failed)?; + + datastore + .floating_ip_attach(&opctx, &authz_fip, &db_fip, instance_id) + .await + .map_err(ActionError::action_failed)?; } - }; - datastore - .allocate_instance_ephemeral_ip(&opctx, ip_id, instance_id, pool_name) - .await - .map_err(ActionError::action_failed)?; + } Ok(()) } diff --git a/nexus/src/app/sagas/instance_delete.rs b/nexus/src/app/sagas/instance_delete.rs index 1605465c74..4160bcaea2 100644 --- a/nexus/src/app/sagas/instance_delete.rs +++ b/nexus/src/app/sagas/instance_delete.rs @@ -127,6 +127,11 @@ async fn sid_deallocate_external_ip( ) .await .map_err(ActionError::action_failed)?; + osagactx + .datastore() + .detach_floating_ips_by_instance_id(&opctx, params.authz_instance.id()) + .await + .map_err(ActionError::action_failed)?; Ok(()) } diff --git a/nexus/types/src/external_api/params.rs b/nexus/types/src/external_api/params.rs index 0c24570e57..12dd01299b 100644 --- a/nexus/types/src/external_api/params.rs +++ b/nexus/types/src/external_api/params.rs @@ -851,7 +851,7 @@ pub enum ExternalIpCreate { /// an existing Floating IP object assigned to the current project. /// /// The floating IP must not be in use by another instance or service. - Floating { floating_ip: NameOrId }, + Floating { floating_ip_name: Name }, } /// Create-time parameters for an `Instance` diff --git a/openapi/nexus.json b/openapi/nexus.json index 1b55b7f580..cb45018a99 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -10447,8 +10447,8 @@ "description": "An IP address providing both inbound and outbound access. The address is an existing Floating IP object assigned to the current project.\n\nThe floating IP must not be in use by another instance or service.", "type": "object", "properties": { - "floating_ip": { - "$ref": "#/components/schemas/NameOrId" + "floating_ip_name": { + "$ref": "#/components/schemas/Name" }, "type": { "type": "string", @@ -10458,7 +10458,7 @@ } }, "required": [ - "floating_ip", + "floating_ip_name", "type" ] }