-
Notifications
You must be signed in to change notification settings - Fork 67
feat: skip default vpc #9750
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: skip default vpc #9750
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,6 +47,7 @@ mod v2026012200; | |
| mod v2026012300; | ||
| mod v2026013000; | ||
| mod v2026013001; | ||
| mod v2026020100; | ||
|
|
||
| #[cfg(test)] | ||
| mod test_utils; | ||
|
|
@@ -79,6 +80,7 @@ api_versions!([ | |
| // | date-based version should be at the top of the list. | ||
| // v | ||
| // (next_yyyymmddnn, IDENT), | ||
| (2026020100, SKIP_DEFAULT_VPC), | ||
| (2026013100, READ_ONLY_DISKS_NULLABLE), | ||
| (2026013001, READ_ONLY_DISKS), | ||
| (2026013000, INSTANCES_EXTERNAL_SUBNETS), | ||
|
|
@@ -942,12 +944,28 @@ pub trait NexusExternalApi { | |
| method = POST, | ||
| path = "/v1/projects", | ||
| tags = ["projects"], | ||
| versions = VERSION_SKIP_DEFAULT_VPC.., | ||
| }] | ||
| async fn project_create( | ||
| rqctx: RequestContext<Self::Context>, | ||
| new_project: TypedBody<params::ProjectCreate>, | ||
| ) -> Result<HttpResponseCreated<views::Project>, HttpError>; | ||
|
|
||
| /// Create project | ||
| #[endpoint { | ||
| method = POST, | ||
| path = "/v1/projects", | ||
| tags = ["projects"], | ||
| operation_id = "project_create", | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm unsure if the |
||
| versions = ..VERSION_SKIP_DEFAULT_VPC, | ||
| }] | ||
| async fn v2026020100_project_create( | ||
| rqctx: RequestContext<Self::Context>, | ||
| new_project: TypedBody<v2026020100::ProjectCreate>, | ||
| ) -> Result<HttpResponseCreated<views::Project>, HttpError> { | ||
| Self::project_create(rqctx, new_project.map(Into::into)).await | ||
| } | ||
|
|
||
| /// Fetch project | ||
| #[endpoint { | ||
| method = GET, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| // This Source Code Form is subject to the terms of the Mozilla Public | ||
| // License, v. 2.0. If a copy of the MPL was not distributed with this | ||
| // file, You can obtain one at https://mozilla.org/MPL/2.0/. | ||
|
|
||
| //! Types that changed in v2026020100. | ||
|
|
||
| use nexus_types::external_api::params; | ||
| use omicron_common::api::external::IdentityMetadataCreateParams; | ||
| use schemars::JsonSchema; | ||
| use serde::Deserialize; | ||
| use serde::Serialize; | ||
|
|
||
| /// Create-time parameters for a `Project` | ||
| #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] | ||
| pub struct ProjectCreate { | ||
| #[serde(flatten)] | ||
| pub identity: IdentityMetadataCreateParams, | ||
| } | ||
|
|
||
| impl From<ProjectCreate> for params::ProjectCreate { | ||
| fn from(ProjectCreate { identity }: ProjectCreate) -> Self { | ||
| Self { identity, skip_default_vpc: false } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,20 +51,24 @@ impl NexusSaga for SagaProjectCreate { | |
| } | ||
|
|
||
| fn make_saga_dag( | ||
| _params: &Self::Params, | ||
| params: &Self::Params, | ||
| mut builder: steno::DagBuilder, | ||
| ) -> Result<steno::Dag, super::SagaInitError> { | ||
| builder.append(project_create_record_action()); | ||
| builder.append(project_create_vpc_params_action()); | ||
|
|
||
| let subsaga_builder = steno::DagBuilder::new(steno::SagaName::new( | ||
| sagas::vpc_create::SagaVpcCreate::NAME, | ||
| )); | ||
| builder.append(steno::Node::subsaga( | ||
| "vpc", | ||
| sagas::vpc_create::create_dag(subsaga_builder)?, | ||
| "vpc_create_params", | ||
| )); | ||
|
|
||
| if !params.project_create.skip_default_vpc { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm unsure if skipping parts of sagas like this is the preferred way to handle such conditional logic but it seemed to be the cleanest method here. |
||
| builder.append(project_create_vpc_params_action()); | ||
|
|
||
| let subsaga_builder = steno::DagBuilder::new(steno::SagaName::new( | ||
| sagas::vpc_create::SagaVpcCreate::NAME, | ||
| )); | ||
| builder.append(steno::Node::subsaga( | ||
| "vpc", | ||
| sagas::vpc_create::create_dag(subsaga_builder)?, | ||
| "vpc_create_params", | ||
| )); | ||
| } | ||
|
|
||
| Ok(builder.build()?) | ||
| } | ||
| } | ||
|
|
@@ -162,24 +166,34 @@ mod test { | |
| ExpressionMethods, OptionalExtension, QueryDsl, SelectableHelper, | ||
| }; | ||
| use nexus_db_queries::{ | ||
| authn::saga::Serialized, authz, context::OpContext, | ||
| authn::saga::Serialized, authz, context::OpContext, db, | ||
| db::datastore::DataStore, | ||
| }; | ||
| use nexus_test_utils_macros::nexus_test; | ||
| use nexus_types::identity::Resource; | ||
| use omicron_common::api::external::IdentityMetadataCreateParams; | ||
|
|
||
| type ControlPlaneTestContext = | ||
| nexus_test_utils::ControlPlaneTestContext<crate::Server>; | ||
|
|
||
| // Helper for creating project create parameters | ||
| fn new_test_params(opctx: &OpContext, authz_silo: authz::Silo) -> Params { | ||
| new_test_params_with_options(opctx, authz_silo, false) | ||
| } | ||
|
|
||
| fn new_test_params_with_options( | ||
| opctx: &OpContext, | ||
| authz_silo: authz::Silo, | ||
| skip_default_vpc: bool, | ||
| ) -> Params { | ||
| Params { | ||
| serialized_authn: Serialized::for_opctx(opctx), | ||
| project_create: params::ProjectCreate { | ||
| identity: IdentityMetadataCreateParams { | ||
| name: "my-project".parse().unwrap(), | ||
| description: "My Project".to_string(), | ||
| }, | ||
| skip_default_vpc, | ||
| }, | ||
| authz_silo, | ||
| } | ||
|
|
@@ -304,4 +318,57 @@ mod test { | |
| ) | ||
| .await; | ||
| } | ||
|
|
||
| #[nexus_test(server = crate::Server)] | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm still figuring out tests for this. It's not pretty currently. |
||
| async fn test_skip_default_vpc_creates_no_vpc( | ||
| cptestctx: &ControlPlaneTestContext, | ||
| ) { | ||
| let nexus = &cptestctx.server.server_context().nexus; | ||
| let datastore = nexus.datastore(); | ||
|
|
||
| // Before running the test, confirm we have no records of any projects. | ||
| verify_clean_slate(datastore).await; | ||
|
|
||
| // Build the saga DAG with skip_default_vpc = true. | ||
| let opctx = test_opctx(&cptestctx); | ||
| let authz_silo = opctx.authn.silo_required().unwrap(); | ||
| let params = | ||
| new_test_params_with_options(&opctx, authz_silo.clone(), true); | ||
| let saga_output = nexus | ||
| .sagas | ||
| .saga_execute::<SagaProjectCreate>(params) | ||
| .await | ||
| .unwrap(); | ||
|
|
||
| // Verify that a project was created. | ||
| let (authz_project, db_project) = saga_output | ||
| .lookup_node_output::<(authz::Project, db::model::Project)>( | ||
| "project", | ||
| ) | ||
| .unwrap(); | ||
| assert_eq!(db_project.name().as_str(), "my-project"); | ||
|
|
||
| // Verify that no VPCs were created for this project. | ||
| use async_bb8_diesel::AsyncRunQueryDsl; | ||
| use diesel::{ExpressionMethods, QueryDsl, SelectableHelper}; | ||
| use nexus_db_queries::db::model::Vpc; | ||
| use nexus_db_schema::schema::vpc::dsl; | ||
|
|
||
| let vpcs = dsl::vpc | ||
| .filter(dsl::project_id.eq(authz_project.id())) | ||
| .filter(dsl::time_deleted.is_null()) | ||
| .select(Vpc::as_select()) | ||
| .load_async::<Vpc>( | ||
| &*datastore.pool_connection_for_tests().await.unwrap(), | ||
| ) | ||
| .await | ||
| .unwrap(); | ||
|
|
||
| assert!( | ||
| vpcs.is_empty(), | ||
| "expected no VPCs for project with skip_default_vpc=true, \ | ||
| found: {:?}", | ||
| vpcs.iter().map(|v| v.name()).collect::<Vec<_>>() | ||
| ); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm unsure if the
operation_idis needed here.