bug(K8s): sandbox create does not validate name against Kubernetes naming constraints #2115

Activity

1. github-actions

   added

   state:triage-neededOpened without agent diagnostics and needs triageOpened without agent diagnostics and needs triage

   on Jul 2, 2026
2. varshaprasad96

   changed the title [-]bug(kubernetes): sandbox create does not validate name against Kubernetes naming constraints[/-] [+]bug(K8s): sandbox create does not validate name against Kubernetes naming constraints[/+] on Jul 2, 2026
3. drew

   added

   area:gatewayGateway server and control-plane workGateway server and control-plane work

   and removed

   state:triage-neededOpened without agent diagnostics and needs triageOpened without agent diagnostics and needs triage

   on Jul 9, 2026
4. drew

   added the

   Bug

   issue type on Jul 9, 2026
5. 2000krysztof commented on Jul 13, 2026

   

   2000krysztof

   on Jul 13, 2026

   Contributor

   More actions

   I would like to work on this issue.
6. TaylorMutch commented on Jul 13, 2026

   

   TaylorMutch

   on Jul 13, 2026

   Collaborator

   More actions

   @2000krysztof I approved your vouch, thanks for volunteering!
7. Hokonoken commented on Jul 14, 2026

   

   Hokonoken

   on Jul 14, 2026

   More actions

   Welcome @2000krysztof, and good luck with this one! I looked into it earlier, so a few edge cases that might be worth covering, if they help:

   • Validate the name as a single DNS-1123 label (≤63 chars, ^[a-z0-9]([a-z0-9-]*[a-z0-9])?$) rather than a full subdomain. The sandbox name is the leading hostname label in service routing (the routing host is {sandbox}.{base_domain}, and service endpoints are keyed {sandbox}--{service}), so a dotted-but-otherwise-valid subdomain name would still break routing.
   • Call the validation first in validate_sandbox_create, so the friendly error precedes the raw RFC-1123 API error the issue is about.
   • I'd leave the -- service separator to the routing layer rather than banning it in name validation (it's cross-driver), unless you find a concrete reason to reject it here.

   Happy to share the two unit tests I drafted (a valid-name case and an invalid dotted/too-long case) if useful — otherwise, all yours.
8. 2000krysztof commented on Jul 14, 2026

   

   2000krysztof

   on Jul 14, 2026

   Last edited by 2000krysztof

   Contributor

   More actions

   @Hokonoken I think that 63 characters is fine for this, because it gives enough of a margine. I would be happy to take a look at your tests that you made.
9. Hokonoken commented on Jul 15, 2026

   

   Hokonoken

   on Jul 15, 2026

   More actions

   Here they are — two unit tests plus the small validator they exercise, so the block is self-contained. Take them as-is or adapt freely, no attribution needed.

   I re-ran both today on current main (e6f319c7): the only adaptation needed was the call site, since validate_sandbox_create now starts with self.validate_driver_config_for_sandbox(sandbox) — I put validate_sandbox_name(&sandbox.name)?; as the first line before it. Both tests pass there (cargo test -p openshell-driver-kubernetes sandbox_name_validation).

   // crates/openshell-driver-kubernetes/src/driver.rs
   
   const MAX_SANDBOX_NAME_LEN: usize = 63;
   
   /// Reject sandbox names Kubernetes cannot accept before they reach the API.
   fn validate_sandbox_name(name: &str) -> Result<(), tonic::Status> {
       let valid = !name.is_empty()
           && name.len() <= MAX_SANDBOX_NAME_LEN
           && !name.starts_with('-')
           && !name.ends_with('-')
           && name
               .bytes()
               .all(|byte| byte.is_ascii_lowercase() || byte.is_ascii_digit() || byte == b'-');
       if valid {
           return Ok(());
       }
       Err(tonic::Status::invalid_argument(format!(
           "sandbox name {name:?} is not a valid Kubernetes object name: use at most \
            {MAX_SANDBOX_NAME_LEN} lowercase letters, digits, or '-', starting and ending \
            with a letter or digit"
       )))
   }
   
   #[test]
   fn sandbox_name_validation_accepts_kubernetes_safe_names() {
       let max_len = "a".repeat(MAX_SANDBOX_NAME_LEN);
       for name in ["crack-krait", "a", "abc123", "123abc", max_len.as_str()] {
           assert!(
               validate_sandbox_name(name).is_ok(),
               "expected {name:?} to be accepted"
           );
       }
   }
   
   #[test]
   fn sandbox_name_validation_rejects_names_kubernetes_would_refuse() {
       let over_len = "a".repeat(MAX_SANDBOX_NAME_LEN + 1);
       for name in [
           "my_sandbox",
           "MySandbox",
           "",
           "-leading",
           "trailing-",
           "dotted.name",
           "has space",
           over_len.as_str(),
       ] {
           let status = validate_sandbox_name(name)
               .expect_err(&format!("expected {name:?} to be rejected"));
           assert_eq!(status.code(), tonic::Code::InvalidArgument, "{name:?}");
       }
   }

   Two deliberate choices encoded in the reject list, in case you want to change them:

   • dotted.name is rejected even though the Kubernetes API would accept it for metadata.name — that's the label-vs-subdomain point from my earlier comment.
   • There is no -- rejection (a--b passes): Kubernetes accepts it, and the {sandbox}--{service} separator collision felt like the routing layer's call rather than the driver's, but that one is arguable either way.

   Good luck with the PR!
10. 2000krysztof commented on Jul 15, 2026

    

    2000krysztof

    on Jul 15, 2026

    Contributor

    More actions

    @Hokonoken Thank you for the code snippet I will add some of these things into my solution.
11. elezar

    closed this as completedin #2295on Jul 17, 2026