review feedback: underscores to hyphens, extra comment

Author: davepacheco

common/src/api/external/mod.rs

@@ -313,9 +313,14 @@ impl Name {
 )]
 #[display("{resource_type}.{role_name}")]
 pub struct RoleName {
-    #[from_str(regex = "[a-z_]+")]
+    // "resource_type" is generally the String value of one of the
+    // `ResourceType` variants.  We could store the parsed `ResourceType`
+    // instead, but it's useful to be able to represent RoleNames for resource
+    // types that we don't know about.  That could happen if we happen to find
+    // them in the database, for example.
+    #[from_str(regex = "[a-z-]+")]
     resource_type: String,
-    #[from_str(regex = "[a-z_]+")]
+    #[from_str(regex = "[a-z-]+")]
     role_name: String,
 }
 
@@ -365,7 +370,7 @@ impl JsonSchema for RoleName {
             string: Some(Box::new(schemars::schema::StringValidation {
                 max_length: Some(63),
                 min_length: None,
-                pattern: Some("[a-z_]+\\.[a-z_]+".to_string()),
+                pattern: Some("[a-z-]+\\.[a-z-]+".to_string()),
             })),
             array: None,
             object: None,
@@ -557,7 +562,7 @@ impl TryFrom<i64> for Generation {
     PartialOrd,
     SerializeDisplay,
 )]
-#[display(style = "snake_case")]
+#[display(style = "kebab-case")]
 pub enum ResourceType {
     Fleet,
     Organization,
@@ -1994,7 +1999,7 @@ mod test {
             // missing role name
             "project.",
             // illegal characters in role name
-            "project.not-good",
+            "project.not_good",
         ];
 
         for input in bad_inputs {
@@ -2018,13 +2023,13 @@ mod test {
         assert_eq!(role_name.resource_type, "barf");
         assert_eq!(role_name.role_name, "admin");
 
-        eprintln!("check name \"organization.super_user\" (expecting success)");
-        let role_name = "organization.super_user"
+        eprintln!("check name \"organization.super-user\" (expecting success)");
+        let role_name = "organization.super-user"
             .parse::<RoleName>()
             .expect("failed to parse");
-        assert_eq!(role_name.to_string(), "organization.super_user");
+        assert_eq!(role_name.to_string(), "organization.super-user");
         assert_eq!(role_name.resource_type, "organization");
-        assert_eq!(role_name.role_name, "super_user");
+        assert_eq!(role_name.role_name, "super-user");
     }
 
     #[test]
@@ -2036,6 +2041,7 @@ mod test {
             "oRgAnIzAtIoN",
             "organisation",
             "vpc subnet",
+            "vpc_subnet",
         ];
         for input in bad_inputs {
             eprintln!("check resource type {:?} (expecting error)", input);
@@ -2051,7 +2057,7 @@ mod test {
         );
         assert_eq!(
             ResourceType::VpcSubnet,
-            "vpc_subnet".parse::<ResourceType>().unwrap()
+            "vpc-subnet".parse::<ResourceType>().unwrap()
         );
     }
 

nexus/tests/integration_tests/vpc_routers.rs

@@ -56,7 +56,7 @@ async fn test_vpc_routers(cptestctx: &ControlPlaneTestContext) {
     let error = client
         .make_request_error(Method::GET, &router_url, StatusCode::NOT_FOUND)
         .await;
-    assert_eq!(error.message, "not found: vpc_router with name \"router1\"");
+    assert_eq!(error.message, "not found: vpc-router with name \"router1\"");
 
     /* Create a VPC Router. */
     let new_router = params::VpcRouterCreate {
@@ -91,7 +91,7 @@ async fn test_vpc_routers(cptestctx: &ControlPlaneTestContext) {
             StatusCode::BAD_REQUEST,
         )
         .await;
-    assert_eq!(error.message, "already exists: vpc_router \"router1\"");
+    assert_eq!(error.message, "already exists: vpc-router \"router1\"");
 
     let router2_name = "router2";
     let router2_url = format!("{}/{}", routers_url, router2_name);
@@ -100,7 +100,7 @@ async fn test_vpc_routers(cptestctx: &ControlPlaneTestContext) {
     let error = client
         .make_request_error(Method::GET, &router2_url, StatusCode::NOT_FOUND)
         .await;
-    assert_eq!(error.message, "not found: vpc_router with name \"router2\"");
+    assert_eq!(error.message, "not found: vpc-router with name \"router2\"");
 
     // create second custom router
     let new_router = params::VpcRouterCreate {
@@ -144,7 +144,7 @@ async fn test_vpc_routers(cptestctx: &ControlPlaneTestContext) {
     let error = client
         .make_request_error(Method::GET, &router_url, StatusCode::NOT_FOUND)
         .await;
-    assert_eq!(error.message, "not found: vpc_router with name \"router1\"");
+    assert_eq!(error.message, "not found: vpc-router with name \"router1\"");
 
     let router_url = format!("{}/{}", routers_url, "new-name");
 
@@ -178,13 +178,13 @@ async fn test_vpc_routers(cptestctx: &ControlPlaneTestContext) {
     let error = client
         .make_request_error(Method::GET, &router_url, StatusCode::NOT_FOUND)
         .await;
-    assert_eq!(error.message, "not found: vpc_router with name \"new-name\"");
+    assert_eq!(error.message, "not found: vpc-router with name \"new-name\"");
 
     // delete router should 404
     let error = client
         .make_request_error(Method::DELETE, &router_url, StatusCode::NOT_FOUND)
         .await;
-    assert_eq!(error.message, "not found: vpc_router with name \"new-name\"");
+    assert_eq!(error.message, "not found: vpc-router with name \"new-name\"");
 
     // Creating a router with the same name in a different VPC is allowed
     let vpc2_name = "vpc2";

nexus/tests/integration_tests/vpc_subnets.rs

@@ -69,7 +69,7 @@ async fn test_vpc_subnets(cptestctx: &ControlPlaneTestContext) {
     let error = client
         .make_request_error(Method::GET, &subnet_url, StatusCode::NOT_FOUND)
         .await;
-    assert_eq!(error.message, "not found: vpc_subnet with name \"subnet1\"");
+    assert_eq!(error.message, "not found: vpc-subnet with name \"subnet1\"");
 
     /* Create a VPC Subnet. */
     let ipv4_block =
@@ -129,7 +129,7 @@ async fn test_vpc_subnets(cptestctx: &ControlPlaneTestContext) {
             StatusCode::BAD_REQUEST,
         )
         .await;
-    assert_eq!(error.message, "already exists: vpc_subnet \"subnet1\"");
+    assert_eq!(error.message, "already exists: vpc-subnet \"subnet1\"");
 
     let subnet2_name = "subnet2";
     let subnet2_url = format!("{}/{}", subnets_url, subnet2_name);
@@ -138,7 +138,7 @@ async fn test_vpc_subnets(cptestctx: &ControlPlaneTestContext) {
     let error = client
         .make_request_error(Method::GET, &subnet2_url, StatusCode::NOT_FOUND)
         .await;
-    assert_eq!(error.message, "not found: vpc_subnet with name \"subnet2\"");
+    assert_eq!(error.message, "not found: vpc-subnet with name \"subnet2\"");
 
     // create second subnet
     let new_subnet = params::VpcSubnetCreate {
@@ -187,7 +187,7 @@ async fn test_vpc_subnets(cptestctx: &ControlPlaneTestContext) {
     let error = client
         .make_request_error(Method::GET, &subnet_url, StatusCode::NOT_FOUND)
         .await;
-    assert_eq!(error.message, "not found: vpc_subnet with name \"subnet1\"");
+    assert_eq!(error.message, "not found: vpc-subnet with name \"subnet1\"");
 
     let subnet_url = format!("{}/{}", subnets_url, "new-name");
 
@@ -221,13 +221,13 @@ async fn test_vpc_subnets(cptestctx: &ControlPlaneTestContext) {
     let error = client
         .make_request_error(Method::GET, &subnet_url, StatusCode::NOT_FOUND)
         .await;
-    assert_eq!(error.message, "not found: vpc_subnet with name \"new-name\"");
+    assert_eq!(error.message, "not found: vpc-subnet with name \"new-name\"");
 
     // delete subnet should 404
     let error = client
         .make_request_error(Method::DELETE, &subnet_url, StatusCode::NOT_FOUND)
         .await;
-    assert_eq!(error.message, "not found: vpc_subnet with name \"new-name\"");
+    assert_eq!(error.message, "not found: vpc-subnet with name \"new-name\"");
 
     // Creating a subnet with the same name in a different VPC is allowed
     let vpc2_name = "vpc2";

openapi/nexus.json

@@ -3773,7 +3773,7 @@
         "title": "A name for a built-in role",
         "description": "Role names consist of two string components separated by dot (\".\").",
         "type": "string",
-        "pattern": "[a-z_]+\\.[a-z_]+",
+        "pattern": "[a-z-]+\\.[a-z-]+",
         "maxLength": 63
       },
       "RoleResultsPage": {