Skip to content

Commit

Permalink
let's add an endpoint!
Browse files Browse the repository at this point in the history
  • Loading branch information
david-crespo committed Oct 11, 2023
1 parent 94d0ebf commit 1de49fc
Show file tree
Hide file tree
Showing 8 changed files with 139 additions and 52 deletions.
31 changes: 31 additions & 0 deletions nexus/src/external_api/http_entrypoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ pub(crate) fn external_api() -> NexusApiDescription {
// Operator-Accessible IP Pools API
api.register(ip_pool_list)?;
api.register(ip_pool_create)?;
api.register(ip_pool_associate)?;
api.register(ip_pool_view)?;
api.register(ip_pool_delete)?;
api.register(ip_pool_update)?;
Expand Down Expand Up @@ -1300,6 +1301,36 @@ async fn ip_pool_update(
apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await
}

// TODO: associate just seems like the wrong word and I'd like to change it
// across the board. What I really mean is "make available to" or "make availale
// for use in"
/// Associate an IP Pool with a silo or project
#[endpoint {
method = POST,
path = "/v1/system/ip-pools/{pool}/associate",
tags = ["system/networking"],
}]
async fn ip_pool_associate(
rqctx: RequestContext<Arc<ServerContext>>,
path_params: Path<params::IpPoolPath>,
resource_assoc: TypedBody<params::IpPoolResource>,
// TODO: what does this return? Returning the association record seems silly
) -> Result<HttpResponseUpdatedNoContent, 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 resource_assoc = resource_assoc.into_inner();
let pool_lookup = nexus.ip_pool_lookup(&opctx, &path.pool)?;
nexus
.ip_pool_associate_resource(&opctx, &pool_lookup, &resource_assoc)
.await?;
Ok(HttpResponseUpdatedNoContent())
};
apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await
}

/// Fetch the IP pool used for Oxide services
#[endpoint {
method = GET,
Expand Down
14 changes: 14 additions & 0 deletions nexus/test-utils/src/resource_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ 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;
Expand Down Expand Up @@ -143,6 +144,19 @@ pub async fn create_ip_pool(
},
)
.await;

// match previous behavior, makes pool available for use anywhere in fleet
let _: () = object_create(
client,
&format!("/v1/system/ip-pools/{pool_name}/associate"),
&params::IpPoolResource {
resource_id: *FLEET_ID,
resource_type: params::IpPoolResourceType::Fleet,
is_default: false,
},
)
.await;

// TODO: associate with fleet as a non-default like before?
let range = populate_ip_pool(client, pool_name, ip_range).await;
(pool, range)
Expand Down
2 changes: 0 additions & 2 deletions nexus/tests/integration_tests/endpoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -456,8 +456,6 @@ lazy_static! {
name: DEMO_IP_POOL_NAME.clone(),
description: String::from("an IP pool"),
},
silo: None,
is_default: true,
};
pub static ref DEMO_IP_POOL_PROJ_URL: String =
format!("/v1/ip-pools/{}?project={}", *DEMO_IP_POOL_NAME, *DEMO_PROJECT_NAME);
Expand Down
4 changes: 2 additions & 2 deletions nexus/tests/integration_tests/instances.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3416,8 +3416,8 @@ async fn test_instance_ephemeral_ip_from_correct_pool(
name: pool_name.parse().unwrap(),
description: String::from("an ip pool"),
},
silo: Some(NameOrId::Id(DEFAULT_SILO.id())),
is_default: true,
// silo: Some(NameOrId::Id(DEFAULT_SILO.id())),
// is_default: true,
},
)
.await;
Expand Down
46 changes: 21 additions & 25 deletions nexus/tests/integration_tests/ip_pools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use nexus_types::external_api::shared::Ipv6Range;
use nexus_types::external_api::views::IpPool;
use nexus_types::external_api::views::IpPoolRange;
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;
Expand Down Expand Up @@ -61,8 +61,8 @@ async fn test_ip_pool_basic_crud(cptestctx: &ControlPlaneTestContext) {
assert_eq!(ip_pools.len(), 1, "Expected to see default IP pool");

assert_eq!(ip_pools[0].identity.name, "default",);
assert_eq!(ip_pools[0].silo_id, None);
assert!(ip_pools[0].is_default);
// assert_eq!(ip_pools[0].silo_id, None);
// assert!(ip_pools[0].is_default);

// Verify 404 if the pool doesn't exist yet, both for creating or deleting
let error: HttpErrorResponseBody = NexusRequest::expect_failure(
Expand Down Expand Up @@ -105,8 +105,8 @@ async fn test_ip_pool_basic_crud(cptestctx: &ControlPlaneTestContext) {
name: String::from(pool_name).parse().unwrap(),
description: String::from(description),
},
silo: None,
is_default: false,
// silo: None,
// is_default: false,
};
let created_pool: IpPool =
NexusRequest::objects_post(client, ip_pools_url, &params)
Expand All @@ -118,7 +118,7 @@ async fn test_ip_pool_basic_crud(cptestctx: &ControlPlaneTestContext) {
.unwrap();
assert_eq!(created_pool.identity.name, pool_name);
assert_eq!(created_pool.identity.description, description);
assert_eq!(created_pool.silo_id, None);
// assert_eq!(created_pool.silo_id, None);

let list = NexusRequest::iter_collection_authn::<IpPool>(
client,
Expand Down Expand Up @@ -288,35 +288,33 @@ async fn test_ip_pool_with_silo(cptestctx: &ControlPlaneTestContext) {
// silo: Some(NameOrId::Name(cptestctx.silo_name.clone())),
// is_default: false,
};
let created_pool = create_pool(client, &params).await;
assert_eq!(created_pool.identity.name, "p0");
let _created_pool = create_pool(client, &params).await;

let silo_id =
created_pool.silo_id.expect("Expected pool to have a silo_id");
// let silo_id =
// created_pool.silo_id.expect("Expected pool to have a silo_id");

// now we'll create another IP pool using that silo ID
let params = IpPoolCreate {
identity: IdentityMetadataCreateParams {
name: String::from("p1").parse().unwrap(),
description: String::from(""),
},
silo: Some(NameOrId::Id(silo_id)),
is_default: false,
// silo: Some(NameOrId::Id(silo_id)),
// is_default: false,
};
let created_pool = create_pool(client, &params).await;
assert_eq!(created_pool.identity.name, "p1");
assert_eq!(created_pool.silo_id.unwrap(), silo_id);
let _created_pool = create_pool(client, &params).await;
// assert_eq!(created_pool.silo_id.unwrap(), silo_id);

// expect 404 if the specified silo doesn't exist
let bad_silo_params = IpPoolCreate {
identity: IdentityMetadataCreateParams {
name: String::from("p2").parse().unwrap(),
description: String::from(""),
},
silo: Some(NameOrId::Name(
String::from("not-a-thing").parse().unwrap(),
)),
is_default: false,
// silo: Some(NameOrId::Name(
// String::from("not-a-thing").parse().unwrap(),
// )),
// is_default: false,
};
let error: HttpErrorResponseBody = NexusRequest::new(
RequestBuilder::new(client, Method::POST, "/v1/system/ip-pools")
Expand Down Expand Up @@ -374,8 +372,8 @@ async fn test_ip_pool_range_overlapping_ranges_fails(
name: String::from(pool_name).parse().unwrap(),
description: String::from(description),
},
silo: None,
is_default: false,
// silo: None,
// is_default: false,
};
let created_pool: IpPool =
NexusRequest::objects_post(client, ip_pools_url, &params)
Expand Down Expand Up @@ -557,8 +555,6 @@ async fn test_ip_pool_range_pagination(cptestctx: &ControlPlaneTestContext) {
name: String::from(pool_name).parse().unwrap(),
description: String::from(description),
},
silo: None,
is_default: false,
};
let created_pool: IpPool =
NexusRequest::objects_post(client, ip_pools_url, &params)
Expand Down Expand Up @@ -695,8 +691,8 @@ async fn test_ip_pool_list_usable_by_project(
name: String::from(mypool_name).parse().unwrap(),
description: String::from("right on cue"),
},
silo: None,
is_default: false,
// silo: None,
// is_default: false,
};
NexusRequest::objects_post(client, ip_pools_url, &params)
.authn_as(AuthnMode::PrivilegedUser)
Expand Down
1 change: 1 addition & 0 deletions nexus/tests/output/nexus_tags.txt
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ system_metric GET /v1/system/metrics/{metric_nam

API operations found with tag "system/networking"
OPERATION ID METHOD URL PATH
ip_pool_associate POST /v1/system/ip-pools/{pool}/associate
ip_pool_create POST /v1/system/ip-pools
ip_pool_delete DELETE /v1/system/ip-pools/{pool}
ip_pool_list GET /v1/system/ip-pools
Expand Down
1 change: 1 addition & 0 deletions nexus/types/src/external_api/params.rs
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,7 @@ pub struct IpPoolUpdate {
}

#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
#[serde(rename_all = "snake_case")]
pub enum IpPoolResourceType {
Fleet,
Silo,
Expand Down
92 changes: 69 additions & 23 deletions openapi/nexus.json
Original file line number Diff line number Diff line change
Expand Up @@ -4577,6 +4577,47 @@
}
}
},
"/v1/system/ip-pools/{pool}/associate": {
"post": {
"tags": [
"system/networking"
],
"summary": "Associate an IP Pool with a silo or project",
"operationId": "ip_pool_associate",
"parameters": [
{
"in": "path",
"name": "pool",
"description": "Name or ID of the IP pool",
"required": true,
"schema": {
"$ref": "#/components/schemas/NameOrId"
}
}
],
"requestBody": {
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/IpPoolResource"
}
}
},
"required": true
},
"responses": {
"204": {
"description": "resource updated"
},
"4XX": {
"$ref": "#/components/responses/Error"
},
"5XX": {
"$ref": "#/components/responses/Error"
}
}
}
},
"/v1/system/ip-pools/{pool}/ranges": {
"get": {
"tags": [
Expand Down Expand Up @@ -11512,9 +11553,6 @@
"type": "string",
"format": "uuid"
},
"is_default": {
"type": "boolean"
},
"name": {
"description": "unique, mutable, user-controlled identifier for each resource",
"allOf": [
Expand All @@ -11523,11 +11561,6 @@
}
]
},
"silo_id": {
"nullable": true,
"type": "string",
"format": "uuid"
},
"time_created": {
"description": "timestamp when this resource was created",
"type": "string",
Expand All @@ -11542,7 +11575,6 @@
"required": [
"description",
"id",
"is_default",
"name",
"time_created",
"time_modified"
Expand All @@ -11555,22 +11587,8 @@
"description": {
"type": "string"
},
"is_default": {
"description": "Whether the IP pool is considered a default pool for its scope (fleet or silo). If a pool is marked default and is associated with a silo, instances created in that silo will draw IPs from that pool unless another pool is specified at instance create time.",
"default": false,
"type": "boolean"
},
"name": {
"$ref": "#/components/schemas/Name"
},
"silo": {
"nullable": true,
"description": "If an IP pool is associated with a silo, instance IP allocations in that silo can draw from that pool.",
"allOf": [
{
"$ref": "#/components/schemas/NameOrId"
}
]
}
},
"required": [
Expand Down Expand Up @@ -11625,6 +11643,34 @@
"items"
]
},
"IpPoolResource": {
"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"
},
"resource_type": {
"$ref": "#/components/schemas/IpPoolResourceType"
}
},
"required": [
"is_default",
"resource_id",
"resource_type"
]
},
"IpPoolResourceType": {
"type": "string",
"enum": [
"fleet",
"silo"
]
},
"IpPoolResultsPage": {
"description": "A single page of results",
"type": "object",
Expand Down

0 comments on commit 1de49fc

Please sign in to comment.