Skip to content

fix(kubernetes): validate sandbox names against RFC 1123 requirements#2295

Draft
2000krysztof wants to merge 1 commit into
NVIDIA:mainfrom
2000krysztof:fix/2115-k8s-name-validation
Draft

fix(kubernetes): validate sandbox names against RFC 1123 requirements#2295
2000krysztof wants to merge 1 commit into
NVIDIA:mainfrom
2000krysztof:fix/2115-k8s-name-validation

Conversation

@2000krysztof

@2000krysztof 2000krysztof commented Jul 15, 2026

Copy link
Copy Markdown

Summary

This PR adds a call to the validate_kubernetes_dns1123_label inside of create_sandbox function.

Related Issue

Fixes #2115

Changes

  • display errors if the name is invalid
  • calls the validation function inside of the create_sandbox function to intercept the name before the creation happens to display the error

Testing

  • mise run pre-commit passes
  • Unit tests added/updated
  • E2E tests added/updated (if applicable)
  • This was also tested on an openshift 4.21 cluster to verify if the changes actually work

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable)

@copy-pr-bot

copy-pr-bot Bot commented Jul 15, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions

Copy link
Copy Markdown

Thank you for your submission! We ask that you sign our Developer Certificate of Origin before we can accept your contribution. You can sign the DCO by adding a comment below using this text:


I have read the DCO document and I hereby sign the DCO.


You can retrigger this bot by commenting recheck in this Pull Request. Posted by the DCO Assistant Lite bot.

@elezar elezar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adding validation before the Kubernetes API call. I think this should reuse the DNS-1123 validation we already have instead of introducing a second sandbox-name validator.

crates/openshell-driver-kubernetes/src/driver.rs already defines validate_kubernetes_dns1123_label() and validate_kubernetes_dns1123_subdomain() near the existing driver volume validation code. The label helper enforces the same shape needed here: non-empty, max 63 chars, lowercase alphanumeric or -, and start/end alphanumeric. It is already covered by the driver-config DNS-1123 tests.

Please replace the new validate_k8s_name() implementation with a call like:

validate_kubernetes_dns1123_label(&sandbox.name, "sandbox.name")
    .map_err(KubernetesDriverError::InvalidArgument)?;

That keeps the Kubernetes driver on one validation primitive and avoids two slightly different RFC 1123 implementations drifting over time. If we want better sandbox-specific wording, we should add that around the existing helper or promote a shared helper into openshell_core, rather than duplicating the parsing logic.

Signed-off-by: Krzysztof Malczuk <kmalczuk@redhat.com>
@2000krysztof 2000krysztof force-pushed the fix/2115-k8s-name-validation branch from e1841c5 to e96b64d Compare July 15, 2026 15:38
@2000krysztof

Copy link
Copy Markdown
Author

I have read the DCO document and I hereby sign the DCO.

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.

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

2 participants