Skip to content

Commit

Permalink
Allow bind of subnet to custom router in create/update
Browse files Browse the repository at this point in the history
  • Loading branch information
FelixMcFelix committed May 24, 2024
1 parent 08c982e commit 821e241
Show file tree
Hide file tree
Showing 13 changed files with 1,645 additions and 304 deletions.
14 changes: 13 additions & 1 deletion nexus/db-model/src/vpc_router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@

use super::{impl_enum_type, Generation, Name, RouterRoute};
use crate::collection::DatastoreCollectionConfig;
use crate::schema::{router_route, vpc_router};
use crate::schema::{router_route, vpc_router, vpc_subnet};
use crate::{DatastoreAttachTargetConfig, VpcSubnet};
use chrono::{DateTime, Utc};
use db_macros::Resource;
use nexus_types::external_api::params;
Expand Down Expand Up @@ -99,3 +100,14 @@ impl From<params::VpcRouterUpdate> for VpcRouterUpdate {
}
}
}

impl DatastoreAttachTargetConfig<VpcSubnet> for VpcRouter {
type Id = Uuid;

type CollectionIdColumn = vpc_router::dsl::id;
type CollectionTimeDeletedColumn = vpc_router::dsl::time_deleted;

type ResourceIdColumn = vpc_subnet::dsl::id;
type ResourceCollectionIdColumn = vpc_subnet::dsl::custom_router_id;
type ResourceTimeDeletedColumn = vpc_subnet::dsl::time_deleted;
}
77 changes: 77 additions & 0 deletions nexus/db-queries/src/db/datastore/vpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ use super::SQL_BATCH_SIZE;
use crate::authz;
use crate::context::OpContext;
use crate::db;
use crate::db::collection_attach::AttachError;
use crate::db::collection_attach::DatastoreAttachTarget;
use crate::db::collection_insert::AsyncInsertError;
use crate::db::collection_insert::DatastoreCollection;
use crate::db::error::public_error_from_diesel;
Expand Down Expand Up @@ -935,6 +937,81 @@ impl DataStore {
Ok(out)
}

pub async fn vpc_subnet_set_custom_router(
&self,
opctx: &OpContext,
authz_subnet: &authz::VpcSubnet,
authz_router: &authz::VpcRouter,
) -> Result<VpcSubnet, Error> {
opctx.authorize(authz::Action::Modify, authz_subnet).await?;
opctx.authorize(authz::Action::Read, authz_router).await?;

use db::schema::vpc_router::dsl as router_dsl;
use db::schema::vpc_subnet::dsl as subnet_dsl;

let query = VpcRouter::attach_resource(
authz_router.id(),
authz_subnet.id(),
router_dsl::vpc_router
.into_boxed()
.filter(router_dsl::kind.eq(VpcRouterKind::Custom)),
subnet_dsl::vpc_subnet.into_boxed(),
u32::MAX,
diesel::update(subnet_dsl::vpc_subnet).set((
subnet_dsl::time_modified.eq(Utc::now()),
subnet_dsl::custom_router_id.eq(authz_router.id()),
)),
);

query
.attach_and_get_result_async(
&*self.pool_connection_authorized(opctx).await?,
)
.await
.map(|(_, resource)| resource)
.map_err(|e| match e {
AttachError::CollectionNotFound => Error::not_found_by_id(
ResourceType::VpcRouter,
&authz_router.id(),
),
AttachError::ResourceNotFound => Error::not_found_by_id(
ResourceType::VpcSubnet,
&authz_subnet.id(),
),
// The only other failure reason can be an attempt to use a system router.
AttachError::NoUpdate { .. } => Error::invalid_request(
"cannot attach a system router to a VPC subnet",
),
AttachError::DatabaseError(e) => {
public_error_from_diesel(e, ErrorHandler::Server)
}
})
}

pub async fn vpc_subnet_unset_custom_router(
&self,
opctx: &OpContext,
authz_subnet: &authz::VpcSubnet,
) -> Result<VpcSubnet, Error> {
opctx.authorize(authz::Action::Modify, authz_subnet).await?;

use db::schema::vpc_subnet::dsl;

diesel::update(dsl::vpc_subnet)
.filter(dsl::time_deleted.is_null())
.filter(dsl::id.eq(authz_subnet.id()))
.set(dsl::custom_router_id.eq(Option::<Uuid>::None))
.returning(VpcSubnet::as_returning())
.get_result_async(&*self.pool_connection_authorized(opctx).await?)
.await
.map_err(|e| {
public_error_from_diesel(
e,
ErrorHandler::NotFoundByResource(authz_subnet),
)
})
}

pub async fn subnet_list_instance_network_interfaces(
&self,
opctx: &OpContext,
Expand Down
93 changes: 91 additions & 2 deletions nexus/src/app/vpc_subnet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ impl super::Nexus {
// See <https://github.com/oxidecomputer/omicron/issues/685> for
// details.
let subnet_id = Uuid::new_v4();
let out = match params.ipv6_block {
let mut out = match params.ipv6_block {
None => {
const NUM_RETRIES: usize = 2;
let mut retry = 0;
Expand Down Expand Up @@ -213,6 +213,23 @@ impl super::Nexus {
}
}?;

// XX: rollback the creation if this fails?
if let Some(custom_router) = &params.custom_router {
let (.., authz_subnet) = LookupPath::new(opctx, &self.db_datastore)
.vpc_subnet_id(out.id())
.lookup_for(authz::Action::Modify)
.await?;

out = self
.vpc_subnet_update_custom_router(
opctx,
&authz_vpc,
&authz_subnet,
Some(custom_router),
)
.await?;
}

self.vpc_needed_notify_sleds();

Ok(out)
Expand All @@ -235,8 +252,18 @@ impl super::Nexus {
vpc_subnet_lookup: &lookup::VpcSubnet<'_>,
params: &params::VpcSubnetUpdate,
) -> UpdateResult<VpcSubnet> {
let (.., authz_subnet) =
let (.., authz_vpc, authz_subnet) =
vpc_subnet_lookup.lookup_for(authz::Action::Modify).await?;

// Updating the subnet is a separate action.
self.vpc_subnet_update_custom_router(
opctx,
&authz_vpc,
&authz_subnet,
params.custom_router.as_ref(),
)
.await?;

let out = self
.db_datastore
.vpc_update_subnet(&opctx, &authz_subnet, params.clone().into())
Expand All @@ -247,6 +274,68 @@ impl super::Nexus {
Ok(out)
}

async fn vpc_subnet_update_custom_router(
&self,
opctx: &OpContext,
authz_vpc: &authz::Vpc,
authz_subnet: &authz::VpcSubnet,
custom_router: Option<&NameOrId>,
) -> UpdateResult<VpcSubnet> {
// Resolve the VPC router, if specified.
let router_lookup = match custom_router {
Some(key @ NameOrId::Name(_)) => self
.vpc_router_lookup(
opctx,
params::RouterSelector {
project: None,
vpc: Some(NameOrId::Id(authz_vpc.id())),
router: key.clone(),
},
)
.map(Some),
Some(key @ NameOrId::Id(_)) => self
.vpc_router_lookup(
opctx,
params::RouterSelector {
project: None,
vpc: None,
router: key.clone(),
},
)
.map(Some),
None => Ok(None),
}?;

let router_lookup = if let Some(l) = router_lookup {
let (.., rtr_authz_vpc, authz_router) =
l.lookup_for(authz::Action::Read).await?;

if authz_vpc.id() != rtr_authz_vpc.id() {
return Err(Error::invalid_request(
"router and subnet must belong to the same VPC",
));
}

Some(authz_router)
} else {
None
};

if let Some(authz_router) = router_lookup {
self.db_datastore
.vpc_subnet_set_custom_router(
opctx,
&authz_subnet,
&authz_router,
)
.await
} else {
self.db_datastore
.vpc_subnet_unset_custom_router(opctx, &authz_subnet)
.await
}
}

pub(crate) async fn vpc_delete_subnet(
&self,
opctx: &OpContext,
Expand Down
10 changes: 0 additions & 10 deletions nexus/src/external_api/http_entrypoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5446,7 +5446,6 @@ async fn vpc_firewall_rules_update(
method = GET,
path = "/v1/vpc-routers",
tags = ["vpcs"],
unpublished = true,
}]
async fn vpc_router_list(
rqctx: RequestContext<ApiContext>,
Expand Down Expand Up @@ -5486,7 +5485,6 @@ async fn vpc_router_list(
method = GET,
path = "/v1/vpc-routers/{router}",
tags = ["vpcs"],
unpublished = true,
}]
async fn vpc_router_view(
rqctx: RequestContext<ApiContext>,
Expand Down Expand Up @@ -5520,7 +5518,6 @@ async fn vpc_router_view(
method = POST,
path = "/v1/vpc-routers",
tags = ["vpcs"],
unpublished = true,
}]
async fn vpc_router_create(
rqctx: RequestContext<ApiContext>,
Expand Down Expand Up @@ -5556,7 +5553,6 @@ async fn vpc_router_create(
method = DELETE,
path = "/v1/vpc-routers/{router}",
tags = ["vpcs"],
unpublished = true,
}]
async fn vpc_router_delete(
rqctx: RequestContext<ApiContext>,
Expand Down Expand Up @@ -5590,7 +5586,6 @@ async fn vpc_router_delete(
method = PUT,
path = "/v1/vpc-routers/{router}",
tags = ["vpcs"],
unpublished = true,
}]
async fn vpc_router_update(
rqctx: RequestContext<ApiContext>,
Expand Down Expand Up @@ -5630,7 +5625,6 @@ async fn vpc_router_update(
method = GET,
path = "/v1/vpc-router-routes",
tags = ["vpcs"],
unpublished = true,
}]
async fn vpc_router_route_list(
rqctx: RequestContext<ApiContext>,
Expand Down Expand Up @@ -5672,7 +5666,6 @@ async fn vpc_router_route_list(
method = GET,
path = "/v1/vpc-router-routes/{route}",
tags = ["vpcs"],
unpublished = true,
}]
async fn vpc_router_route_view(
rqctx: RequestContext<ApiContext>,
Expand Down Expand Up @@ -5709,7 +5702,6 @@ async fn vpc_router_route_view(
method = POST,
path = "/v1/vpc-router-routes",
tags = ["vpcs"],
unpublished = true,
}]
async fn vpc_router_route_create(
rqctx: RequestContext<ApiContext>,
Expand Down Expand Up @@ -5745,7 +5737,6 @@ async fn vpc_router_route_create(
method = DELETE,
path = "/v1/vpc-router-routes/{route}",
tags = ["vpcs"],
unpublished = true,
}]
async fn vpc_router_route_delete(
rqctx: RequestContext<ApiContext>,
Expand Down Expand Up @@ -5781,7 +5772,6 @@ async fn vpc_router_route_delete(
method = PUT,
path = "/v1/vpc-router-routes/{route}",
tags = ["vpcs"],
unpublished = true,
}]
async fn vpc_router_route_update(
rqctx: RequestContext<ApiContext>,
Expand Down
2 changes: 2 additions & 0 deletions nexus/tests/integration_tests/endpoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ pub static DEMO_VPC_SUBNET_CREATE: Lazy<params::VpcSubnetCreate> =
},
ipv4_block: Ipv4Net("10.1.2.3/8".parse().unwrap()),
ipv6_block: None,
custom_router: None,
});

// VPC Router used for testing
Expand Down Expand Up @@ -1506,6 +1507,7 @@ pub static VERIFY_ENDPOINTS: Lazy<Vec<VerifyEndpoint>> = Lazy::new(|| {
name: None,
description: Some("different".to_string())
},
custom_router: None,
}).unwrap()
),
AllowedMethod::Delete,
Expand Down
3 changes: 3 additions & 0 deletions nexus/tests/integration_tests/instances.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1724,6 +1724,7 @@ async fn test_instance_with_new_custom_network_interfaces(
},
ipv4_block: Ipv4Net("172.31.0.0/24".parse().unwrap()),
ipv6_block: None,
custom_router: None,
};
let _response = NexusRequest::objects_post(
client,
Expand Down Expand Up @@ -1870,6 +1871,7 @@ async fn test_instance_create_delete_network_interface(
},
ipv4_block: Ipv4Net("172.31.0.0/24".parse().unwrap()),
ipv6_block: None,
custom_router: None,
};
let _response = NexusRequest::objects_post(
client,
Expand Down Expand Up @@ -2111,6 +2113,7 @@ async fn test_instance_update_network_interfaces(
},
ipv4_block: Ipv4Net("172.31.0.0/24".parse().unwrap()),
ipv6_block: None,
custom_router: None,
};
let _response = NexusRequest::objects_post(
client,
Expand Down
1 change: 1 addition & 0 deletions nexus/tests/integration_tests/subnet_allocation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ async fn test_subnet_allocation(cptestctx: &ControlPlaneTestContext) {
// Use the minimum subnet size
ipv4_block: Ipv4Net(subnet),
ipv6_block: None,
custom_router: None,
};
NexusRequest::objects_post(client, &subnets_url, &Some(&subnet_create))
.authn_as(AuthnMode::PrivilegedUser)
Expand Down
Loading

0 comments on commit 821e241

Please sign in to comment.