Skip to content

refactor(api): share VM name validation across service and gateway - #341

Open
tholop wants to merge 1 commit into
0.7from
feat/shared-vm-name-rule
Open

tholop wants to merge 1 commit into
0.7from
feat/shared-vm-name-rule

Conversation

@tholop

@tholop tholop commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

This is a tiny fix to remove duplicated code I found. Actually there is a third near-identical copy of the same logic here:

pub(crate) fn safe_id(value: &str) -> bool {
!value.is_empty()
&& value.len() <= 64
&& value
.bytes()
.all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'-' | b'_'))
}
fn safe_dns_label(value: &str) -> bool {
value.len() <= 63
&& safe_id(value)
&& !value.contains('_')
&& value.as_bytes().first().is_some_and(u8::is_ascii_alphanumeric)
&& value.as_bytes().last().is_some_and(u8::is_ascii_alphanumeric)
. But somehow that third variant allows names that start with non-alphanum characters. Not sure if this is intentional.

-- Pierre

Summary

  • Moves the 1..=64 ASCII alphanumeric/hyphen/underscore VM identifier rule (^[A-Za-z0-9][A-Za-z0-9_-]{0,63}$) into capsem_api::validate_name (crates/capsem-api/src/lifecycle.rs).
  • Updates capsem-service (crates/capsem-service/src/naming.rs validate_vm_name) and capsem-gateway (crates/capsem-gateway/src/stream.rs validate_vm_id) to delegate to capsem_api::validate_name while preserving their existing error messages (+66 / -32, 4 files).

@codecov-commenter

codecov-commenter commented Oct 7, 2026 •

Copy link
Copy Markdown

❌ 5 Tests Failed:

Tests completed Failed Passed Skipped
6039 5 6034 0
View the top 3 failed test(s) by shortest run time
capsem-sdk::operations::tests::every_contract_operation_has_http_cases
Stack Traces | 0.069s run time
thread 'operations::tests::every_contract_operation_has_http_cases' (262141) panicked at .../src/operations/tests.rs:108:5:
assertion `left == right` failed
  left: {"attachNetworkMember", "callMcpTool", "createNetwork", "createVm", "createVmExposure", "createVmPreviewSession", "deleteNetwork", "deleteVm", "deleteVmExposure", "detachNetworkMember", "downloadVmFile", "execVm", "exportVmBodies", "forkVm", "getAssetStatus", "getHypervisorInfo", "getHypervisorLogs", "getMcpDefault", "getMcpInfo", "getNetwork", "getNetworkLogs", "getPanics", "getTriage", "getUpdateStatus", "getVmContainer", "getVmEventBodies", "getVmHistory", "getVmInfo", "getVmLogs", "getVmStatsDetail", "getVmStatsSummary", "getVmStatus", "getVmTimeline", "injectCredential", "listImages", "listMcpServers", "listMcpTools", "listNetworks", "listVmExposures", "listVmFiles", "listVms", "pauseVm", "persistVm", "pullImage", "purgeVms", "refreshMcpServer", "restartHypervisor", "resumeVm", "revokeVmPreviewSessions", "runVm", "startVm", "stopVm", "updateHypervisor", "uploadVmFile"}
 right: {"attachNetworkMember", "callMcpTool", "createNetwork", "createVm", "createVmExposure", "createVmPreviewSession", "deleteNetwork", "deleteVm", "deleteVmExposure", "detachNetworkMember", "downloadVmFile", "execVm", "exportVmBodies", "forkVm", "getAssetStatus", "getHypervisorInfo", "getHypervisorLogs", "getMcpDefault", "getMcpInfo", "getNetwork", "getNetworkLogs", "getPanics", "getTriage", "getUpdateStatus", "getVmContainer", "getVmEventBodies", "getVmHistory", "getVmInfo", "getVmLogs", "getVmStatsDetail", "getVmStatsSummary", "getVmStatus", "getVmTimeline", "listImages", "listMcpServers", "listMcpTools", "listNetworks", "listVmExposures", "listVmFiles", "listVms", "pauseVm", "persistVm", "pullImage", "purgeVms", "refreshMcpServer", "restartHypervisor", "resumeVm", "revokeVmPreviewSessions", "runVm", "startVm", "stopVm", "updateHypervisor", "uploadVmFile"}
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
capsem-assets::oci::worker::tests::inventory_worker_publishes_only_quiet_bounded_observations_and_tracks_nested_changes
Stack Traces | 2.36s run time
thread 'oci::worker::tests::inventory_worker_publishes_only_quiet_bounded_observations_and_tracks_nested_changes' (136773) panicked at .../oci/worker/tests.rs:27:6:
called `Result::unwrap()` on an `Err` value: TimedOut { label: "inventory-observed", attempts: 8, timeout: 2s }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
capsem-service::bin/capsem-service::container_setup::tests::registry::catalog_observation_schedules_one_owned_worker_and_refuses_rebinding
Stack Traces | 2.6s run time
thread 'container_setup::tests::registry::catalog_observation_schedules_one_owned_worker_and_refuses_rebinding' (270658) panicked at .../container_setup/tests/registry.rs:27:6:
called `Result::unwrap()` on an `Err` value: TimedOut { label: "service-cache-observed", attempts: 8, timeout: 2s }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
capsem-assets::oci::worker::tests::incompatible_receipts_are_observed_once_until_their_metadata_changes
Stack Traces | 2.73s run time
thread 'oci::worker::tests::incompatible_receipts_are_observed_once_until_their_metadata_changes' (136710) panicked at .../oci/worker/tests.rs:246:10:
called `Result::unwrap()` on an `Err` value: TimedOut { label: "foreign-cache-reobserved", attempts: 8, timeout: 2s }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
capsem-core::fs_monitor::tests::a_workspace_swapped_for_a_host_link_is_never_walked_or_read
Stack Traces | 360s run time
No failure message available

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

Move the 1..=64 ASCII alphanumeric/hyphen/underscore VM identifier rule into
capsem_api::validate_name so capsem-service and capsem-gateway enforce a
single definition while preserving their respective error messages.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants