Return a more specific http status code on conflict.

jmcarp · jmcarp · commit b77feb2e5490 · 2026-04-01T22:17:48.000-04:00
diff --git a/common/src/api/external/error.rs b/common/src/api/external/error.rs
@@ -420,8 +420,9 @@ impl From<Error> for HttpError {
 
             Error::ObjectAlreadyExists { type_name: t, object_name: n } => {
                 let message = format!("already exists: {} \"{}\"", t, n);
-                HttpError::for_bad_request(
+                HttpError::for_client_error(
                     Some(String::from("ObjectAlreadyExists")),
+                    dropshot::ClientErrorStatusCode::CONFLICT,
                     message,
                 )
             }
diff --git a/nexus/tests/integration_tests/affinity.rs b/nexus/tests/integration_tests/affinity.rs
@@ -704,7 +704,7 @@ async fn test_group_crud<T: AffinityGroupish>(client: &ClientTestContext) {
     // We can now create a group and observe it
     project_api.group_create(GROUP_NAME).await;
     let response = project_api
-        .group_create_expect_error(GROUP_NAME, StatusCode::BAD_REQUEST)
+        .group_create_expect_error(GROUP_NAME, StatusCode::CONFLICT)
         .await;
     assert_eq!(
         response.message,
@@ -726,7 +726,7 @@ async fn test_group_crud<T: AffinityGroupish>(client: &ClientTestContext) {
         .group_member_add_expect_error(
             GROUP_NAME,
             &instance_name,
-            StatusCode::BAD_REQUEST,
+            StatusCode::CONFLICT,
         )
         .await;
     assert_eq!(
@@ -858,7 +858,7 @@ async fn test_group_project_selector<T: AffinityGroupish>(
 
     // We can only create a group within a project
     no_project_api
-        .group_create_expect_error(GROUP_NAME, StatusCode::BAD_REQUEST)
+        .group_create_expect_error(GROUP_NAME, StatusCode::CONFLICT)
         .await;
     let group = project_api.group_create(GROUP_NAME).await;
 
diff --git a/nexus/tests/integration_tests/basic.rs b/nexus/tests/integration_tests/basic.rs
@@ -372,7 +372,7 @@ async fn test_projects_basic(cptestctx: &ControlPlaneTestContext) {
     let error = NexusRequest::new(
         RequestBuilder::new(client, Method::POST, &projects_url)
             .body(Some(&project_create))
-            .expect_status(Some(StatusCode::BAD_REQUEST)),
+            .expect_status(Some(StatusCode::CONFLICT)),
     )
     .authn_as(AuthnMode::PrivilegedUser)
     .execute()
diff --git a/nexus/tests/integration_tests/certificates.rs b/nexus/tests/integration_tests/certificates.rs
@@ -97,7 +97,7 @@ async fn cert_create_expect_error(
 
     NexusRequest::expect_failure_with_body(
         client,
-        StatusCode::BAD_REQUEST,
+        StatusCode::CONFLICT,
         Method::POST,
         &url,
         &params,
diff --git a/nexus/tests/integration_tests/disks.rs b/nexus/tests/integration_tests/disks.rs
@@ -386,7 +386,7 @@ async fn test_disk_create_disk_that_already_exists_fails(
     let error: HttpErrorResponseBody = NexusRequest::new(
         RequestBuilder::new(client, Method::POST, &disks_url)
             .body(Some(&new_disk))
-            .expect_status(Some(StatusCode::BAD_REQUEST)),
+            .expect_status(Some(StatusCode::CONFLICT)),
     )
     .authn_as(AuthnMode::PrivilegedUser)
     .execute()
diff --git a/nexus/tests/integration_tests/external_ips.rs b/nexus/tests/integration_tests/external_ips.rs
@@ -616,7 +616,7 @@ async fn test_floating_ip_create_name_in_use(
                 },
             },
         }))
-        .expect_status(Some(StatusCode::BAD_REQUEST)),
+        .expect_status(Some(StatusCode::CONFLICT)),
     )
     .authn_as(AuthnMode::PrivilegedUser)
     .execute()
diff --git a/nexus/tests/integration_tests/external_subnets.rs b/nexus/tests/integration_tests/external_subnets.rs
@@ -866,7 +866,7 @@ async fn external_subnet_create_name_conflict(
         client,
         &external_subnets_url(PROJECT_NAME),
         &create_params,
-        StatusCode::BAD_REQUEST,
+        StatusCode::CONFLICT,
     )
     .await;
     assert_eq!(error.error_code.as_deref(), Some("ObjectAlreadyExists"));
diff --git a/nexus/tests/integration_tests/images.rs b/nexus/tests/integration_tests/images.rs
@@ -414,7 +414,7 @@ async fn test_image_promotion(cptestctx: &ControlPlaneTestContext) {
 
     NexusRequest::new(
         RequestBuilder::new(client, http::Method::POST, &promote_url)
-            .expect_status(Some(StatusCode::BAD_REQUEST)),
+            .expect_status(Some(StatusCode::CONFLICT)),
     )
     .authn_as(AuthnMode::PrivilegedUser)
     .execute()
diff --git a/nexus/tests/integration_tests/instances.rs b/nexus/tests/integration_tests/instances.rs
@@ -390,7 +390,7 @@ async fn test_instances_create_reboot_halt(
                 anti_affinity_groups: Vec::new(),
                 multicast_groups: Vec::new(),
             }))
-            .expect_status(Some(StatusCode::BAD_REQUEST)),
+            .expect_status(Some(StatusCode::CONFLICT)),
     )
     .authn_as(AuthnMode::PrivilegedUser)
     .execute()
diff --git a/nexus/tests/integration_tests/ip_pools.rs b/nexus/tests/integration_tests/ip_pools.rs
@@ -134,7 +134,7 @@ async fn test_ip_pool_basic_crud(cptestctx: &ControlPlaneTestContext) {
             },
             IpVersion::V4,
         ),
-        StatusCode::BAD_REQUEST,
+        StatusCode::CONFLICT,
     )
     .await;
 
@@ -508,7 +508,7 @@ async fn test_ip_pool_silo_link(cptestctx: &ControlPlaneTestContext) {
         client,
         "/v1/system/ip-pools/p0/silos",
         &params,
-        StatusCode::BAD_REQUEST,
+        StatusCode::CONFLICT,
     )
     .await;
     assert_eq!(error.error_code.unwrap(), "ObjectAlreadyExists");
@@ -560,13 +560,9 @@ async fn test_ip_pool_silo_link(cptestctx: &ControlPlaneTestContext) {
     // creating a third pool and trying to link it as default: true should fail
     create_ipv4_pool(client, "p2").await;
     let url = "/v1/system/ip-pools/p2/silos";
-    let error = object_create_error(
-        client,
-        &url,
-        &link_params,
-        StatusCode::BAD_REQUEST,
-    )
-    .await;
+    let error =
+        object_create_error(client, &url, &link_params, StatusCode::CONFLICT)
+            .await;
     assert_eq!(error.error_code.unwrap(), "ObjectAlreadyExists");
 
     // unlink p1 from silo (doesn't matter that it's a default)
diff --git a/nexus/tests/integration_tests/silos.rs b/nexus/tests/integration_tests/silos.rs
@@ -67,7 +67,7 @@ async fn test_silos(cptestctx: &ControlPlaneTestContext) {
     let error: dropshot::HttpErrorResponseBody =
         NexusRequest::expect_failure_with_body(
             client,
-            StatusCode::BAD_REQUEST,
+            StatusCode::CONFLICT,
             Method::POST,
             "/v1/system/silos",
             &silo::SiloCreate {
diff --git a/nexus/tests/integration_tests/ssh_keys.rs b/nexus/tests/integration_tests/ssh_keys.rs
@@ -71,7 +71,7 @@ async fn test_ssh_keys(cptestctx: &ControlPlaneTestContext) {
     let error: dropshot::HttpErrorResponseBody =
         NexusRequest::expect_failure_with_body(
             client,
-            http::StatusCode::BAD_REQUEST,
+            http::StatusCode::CONFLICT,
             http::Method::POST,
             "/v1/me/ssh-keys",
             &SshKeyCreate {
diff --git a/nexus/tests/integration_tests/vpc_routers.rs b/nexus/tests/integration_tests/vpc_routers.rs
@@ -135,7 +135,7 @@ async fn test_vpc_routers_crud_operations(cptestctx: &ControlPlaneTestContext) {
                     description: String::from("this is not a router"),
                 },
             }))
-            .expect_status(Some(StatusCode::BAD_REQUEST)),
+            .expect_status(Some(StatusCode::CONFLICT)),
     )
     .authn_as(AuthnMode::PrivilegedUser)
     .execute()
diff --git a/nexus/tests/integration_tests/vpc_subnets.rs b/nexus/tests/integration_tests/vpc_subnets.rs
@@ -268,7 +268,7 @@ async fn test_vpc_subnets(cptestctx: &ControlPlaneTestContext) {
     };
     let error: dropshot::HttpErrorResponseBody = NexusRequest::new(
         RequestBuilder::new(client, Method::POST, &subnets_url)
-            .expect_status(Some(StatusCode::BAD_REQUEST))
+            .expect_status(Some(StatusCode::CONFLICT))
             .body(Some(&new_subnet)),
     )
     .authn_as(AuthnMode::PrivilegedUser)
diff --git a/nexus/tests/integration_tests/vpcs.rs b/nexus/tests/integration_tests/vpcs.rs
@@ -130,7 +130,7 @@ async fn test_vpcs(cptestctx: &ControlPlaneTestContext) {
         &client,
         PROJECT_NAME,
         vpc_name,
-        StatusCode::BAD_REQUEST,
+        StatusCode::CONFLICT,
     )
     .await;
     assert_eq!(error.message, "already exists: vpc \"just-rainsticks\"");
diff --git a/nexus/tests/integration_tests/webhooks.rs b/nexus/tests/integration_tests/webhooks.rs
@@ -446,7 +446,7 @@ async fn test_webhook_receiver_names_are_unique(
             secrets: vec![MY_COOL_SECRET.to_string()],
             subscriptions: vec!["test.foo.bar".parse().unwrap()],
         },
-        http::StatusCode::BAD_REQUEST,
+        http::StatusCode::CONFLICT,
     )
     .await;
     assert_eq!(

Original file line number	Diff line number	Diff line change
`@@ -420,8 +420,9 @@ impl From<Error> for HttpError {`
`420`	`420`
`421`	`421`	`Error::ObjectAlreadyExists { type_name: t, object_name: n } => {`
`422`	`422`	`let message = format!("already exists: {} \"{}\"", t, n);`
`423`		`- HttpError::for_bad_request(`
	`423`	`+ HttpError::for_client_error(`
`424`	`424`	`Some(String::from("ObjectAlreadyExists")),`
	`425`	`+ dropshot::ClientErrorStatusCode::CONFLICT,`
`425`	`426`	`message,`
`426`	`427`	`)`
`427`	`428`	`}`
Original file line number	Diff line number	Diff line change
`@@ -372,7 +372,7 @@ async fn test_projects_basic(cptestctx: &ControlPlaneTestContext) {`
`372`	`372`	`let error = NexusRequest::new(`
`373`	`373`	`RequestBuilder::new(client, Method::POST, &projects_url)`
`374`	`374`	`.body(Some(&project_create))`
`375`		`- .expect_status(Some(StatusCode::BAD_REQUEST)),`
	`375`	`+ .expect_status(Some(StatusCode::CONFLICT)),`
`376`	`376`	`)`
`377`	`377`	`.authn_as(AuthnMode::PrivilegedUser)`
`378`	`378`	`.execute()`
Original file line number	Diff line number	Diff line change
`@@ -386,7 +386,7 @@ async fn test_disk_create_disk_that_already_exists_fails(`
`386`	`386`	`let error: HttpErrorResponseBody = NexusRequest::new(`
`387`	`387`	`RequestBuilder::new(client, Method::POST, &disks_url)`
`388`	`388`	`.body(Some(&new_disk))`
`389`		`- .expect_status(Some(StatusCode::BAD_REQUEST)),`
	`389`	`+ .expect_status(Some(StatusCode::CONFLICT)),`
`390`	`390`	`)`
`391`	`391`	`.authn_as(AuthnMode::PrivilegedUser)`
`392`	`392`	`.execute()`
Original file line number	Diff line number	Diff line change
`@@ -616,7 +616,7 @@ async fn test_floating_ip_create_name_in_use(`
`616`	`616`	`},`
`617`	`617`	`},`
`618`	`618`	`}))`
`619`		`- .expect_status(Some(StatusCode::BAD_REQUEST)),`
	`619`	`+ .expect_status(Some(StatusCode::CONFLICT)),`
`620`	`620`	`)`
`621`	`621`	`.authn_as(AuthnMode::PrivilegedUser)`
`622`	`622`	`.execute()`
Original file line number	Diff line number	Diff line change
`@@ -866,7 +866,7 @@ async fn external_subnet_create_name_conflict(`
`866`	`866`	`client,`
`867`	`867`	`&external_subnets_url(PROJECT_NAME),`
`868`	`868`	`&create_params,`
`869`		`- StatusCode::BAD_REQUEST,`
	`869`	`+ StatusCode::CONFLICT,`
`870`	`870`	`)`
`871`	`871`	`.await;`
`872`	`872`	`assert_eq!(error.error_code.as_deref(), Some("ObjectAlreadyExists"));`
Original file line number	Diff line number	Diff line change
`@@ -414,7 +414,7 @@ async fn test_image_promotion(cptestctx: &ControlPlaneTestContext) {`
`414`	`414`
`415`	`415`	`NexusRequest::new(`
`416`	`416`	`RequestBuilder::new(client, http::Method::POST, &promote_url)`
`417`		`- .expect_status(Some(StatusCode::BAD_REQUEST)),`
	`417`	`+ .expect_status(Some(StatusCode::CONFLICT)),`
`418`	`418`	`)`
`419`	`419`	`.authn_as(AuthnMode::PrivilegedUser)`
`420`	`420`	`.execute()`
Original file line number	Diff line number	Diff line change
`@@ -390,7 +390,7 @@ async fn test_instances_create_reboot_halt(`
`390`	`390`	`anti_affinity_groups: Vec::new(),`
`391`	`391`	`multicast_groups: Vec::new(),`
`392`	`392`	`}))`
`393`		`- .expect_status(Some(StatusCode::BAD_REQUEST)),`
	`393`	`+ .expect_status(Some(StatusCode::CONFLICT)),`
`394`	`394`	`)`
`395`	`395`	`.authn_as(AuthnMode::PrivilegedUser)`
`396`	`396`	`.execute()`