mirror of
https://github.com/NVIDIA/OpenShell.git
synced 2026-10-21 06:11:31 +08:00
fix(server): validate portable resource requirements
Validate typed CPU and memory quantities and reject legacy CPU or memory entries in the historical inline template resource passthrough with migration guidance. Part of #2838 Signed-off-by: Evan Lezar <elezar@nvidia.com>
This commit is contained in:
@@ -197,8 +197,8 @@ pub(super) fn validate_sandbox_spec(name: &str, spec: &SandboxSpec) -> Result<()
|
||||
validate_env_entries(&tmpl.environment, "spec.template.environment")?;
|
||||
}
|
||||
|
||||
// --- spec.resource_requirements.gpu ---
|
||||
validate_gpu_request_fields(spec)?;
|
||||
// --- spec.resource_requirements ---
|
||||
validate_resource_requirement_fields(spec)?;
|
||||
|
||||
if !spec.command.is_empty() {
|
||||
validate_main_process_command(&spec.command)?;
|
||||
@@ -296,7 +296,7 @@ fn validate_main_process_command(command: &[String]) -> Result<(), Status> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn validate_gpu_request_fields(spec: &SandboxSpec) -> Result<(), Status> {
|
||||
fn validate_resource_requirement_fields(spec: &SandboxSpec) -> Result<(), Status> {
|
||||
if openshell_core::gpu::sandbox_gpu_count(spec.resource_requirements.as_ref()) == Some(0) {
|
||||
return Err(invalid_argument(
|
||||
"spec.resource_requirements.gpu.count",
|
||||
@@ -304,9 +304,47 @@ fn validate_gpu_request_fields(spec: &SandboxSpec) -> Result<(), Status> {
|
||||
));
|
||||
}
|
||||
|
||||
if let Some(cpu) = spec
|
||||
.resource_requirements
|
||||
.as_ref()
|
||||
.and_then(|requirements| requirements.cpu.as_ref())
|
||||
{
|
||||
if let Some(quantity) = cpu.quantity.as_deref() {
|
||||
validate_cpu_quantity(quantity, "spec.resource_requirements.cpu.quantity")?;
|
||||
} else {
|
||||
return Err(Status::invalid_argument(
|
||||
"spec.resource_requirements.cpu must set quantity",
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
if let Some(memory) = spec
|
||||
.resource_requirements
|
||||
.as_ref()
|
||||
.and_then(|requirements| requirements.memory.as_ref())
|
||||
{
|
||||
if let Some(quantity) = memory.quantity.as_deref() {
|
||||
validate_memory_quantity(quantity, "spec.resource_requirements.memory.quantity")?;
|
||||
} else {
|
||||
return Err(Status::invalid_argument(
|
||||
"spec.resource_requirements.memory must set quantity",
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn validate_cpu_quantity(value: &str, field_name: &str) -> Result<(), Status> {
|
||||
openshell_core::quantity::validate_cpu_quantity(value, field_name)
|
||||
.map_err(Status::invalid_argument)
|
||||
}
|
||||
|
||||
fn validate_memory_quantity(value: &str, field_name: &str) -> Result<(), Status> {
|
||||
openshell_core::quantity::validate_memory_quantity(value, field_name)
|
||||
.map_err(Status::invalid_argument)
|
||||
}
|
||||
|
||||
/// Validate template-level field sizes.
|
||||
fn validate_sandbox_template(tmpl: &SandboxTemplate) -> Result<(), Status> {
|
||||
// String fields.
|
||||
@@ -360,6 +398,7 @@ fn validate_sandbox_template(tmpl: &SandboxTemplate) -> Result<(), Status> {
|
||||
),
|
||||
));
|
||||
}
|
||||
reject_legacy_template_cpu_memory_resources(s)?;
|
||||
}
|
||||
if let Some(ref s) = tmpl.driver_config {
|
||||
let size = s.encoded_len();
|
||||
@@ -401,6 +440,31 @@ fn reject_gateway_owned_driver_config_keys(config: &prost_types::Struct) -> Resu
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn reject_legacy_template_cpu_memory_resources(
|
||||
resources: &prost_types::Struct,
|
||||
) -> Result<(), Status> {
|
||||
for section_name in ["limits", "requests"] {
|
||||
let Some(value) = resources.fields.get(section_name) else {
|
||||
continue;
|
||||
};
|
||||
let Some(prost_types::value::Kind::StructValue(section)) = value.kind.as_ref() else {
|
||||
return Err(Status::invalid_argument(format!(
|
||||
"template.resources.{section_name} must be an object"
|
||||
)));
|
||||
};
|
||||
|
||||
for resource_name in ["cpu", "memory"] {
|
||||
if section.fields.contains_key(resource_name) {
|
||||
return Err(Status::invalid_argument(format!(
|
||||
"template.resources.{section_name}.{resource_name} is no longer supported; use spec.resource_requirements.{resource_name}.quantity"
|
||||
)));
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
@@ -1188,6 +1252,8 @@ mod tests {
|
||||
let spec = SandboxSpec {
|
||||
resource_requirements: Some(openshell_core::proto::ResourceRequirements {
|
||||
gpu: Some(openshell_core::proto::GpuResourceRequirements { count: None }),
|
||||
cpu: None,
|
||||
memory: None,
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
@@ -1199,6 +1265,8 @@ mod tests {
|
||||
let spec = SandboxSpec {
|
||||
resource_requirements: Some(openshell_core::proto::ResourceRequirements {
|
||||
gpu: Some(openshell_core::proto::GpuResourceRequirements { count: Some(2) }),
|
||||
cpu: None,
|
||||
memory: None,
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
@@ -1210,6 +1278,8 @@ mod tests {
|
||||
let spec = SandboxSpec {
|
||||
resource_requirements: Some(openshell_core::proto::ResourceRequirements {
|
||||
gpu: Some(openshell_core::proto::GpuResourceRequirements { count: Some(0) }),
|
||||
cpu: None,
|
||||
memory: None,
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
@@ -1218,6 +1288,160 @@ mod tests {
|
||||
assert!(err.message().contains("gpu count must be greater than 0"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn validate_sandbox_spec_accepts_cpu_and_memory_requirements() {
|
||||
let spec = SandboxSpec {
|
||||
resource_requirements: Some(openshell_core::proto::ResourceRequirements {
|
||||
gpu: None,
|
||||
cpu: Some(openshell_core::proto::CpuResourceRequirements {
|
||||
quantity: Some("500m".to_string()),
|
||||
}),
|
||||
memory: Some(openshell_core::proto::MemoryResourceRequirements {
|
||||
quantity: Some("2Gi".to_string()),
|
||||
}),
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
assert!(validate_sandbox_spec("compute-sandbox", &spec).is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn validate_sandbox_spec_rejects_invalid_cpu_requirements() {
|
||||
let spec = SandboxSpec {
|
||||
resource_requirements: Some(openshell_core::proto::ResourceRequirements {
|
||||
gpu: None,
|
||||
cpu: Some(openshell_core::proto::CpuResourceRequirements {
|
||||
quantity: Some("0".to_string()),
|
||||
}),
|
||||
memory: Some(openshell_core::proto::MemoryResourceRequirements {
|
||||
quantity: Some("2Gi".to_string()),
|
||||
}),
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let err = validate_sandbox_spec("compute-sandbox", &spec).unwrap_err();
|
||||
assert_eq!(err.code(), Code::InvalidArgument);
|
||||
assert!(
|
||||
err.message()
|
||||
.contains("spec.resource_requirements.cpu.quantity")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn validate_sandbox_spec_accepts_cpu_and_memory_quantities() {
|
||||
let spec = SandboxSpec {
|
||||
resource_requirements: Some(openshell_core::proto::ResourceRequirements {
|
||||
gpu: None,
|
||||
cpu: Some(openshell_core::proto::CpuResourceRequirements {
|
||||
quantity: Some("250m".to_string()),
|
||||
}),
|
||||
memory: Some(openshell_core::proto::MemoryResourceRequirements {
|
||||
quantity: Some("512Mi".to_string()),
|
||||
}),
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
assert!(validate_sandbox_spec("compute-sandbox", &spec).is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn validate_sandbox_spec_rejects_invalid_memory_quantity() {
|
||||
let spec = SandboxSpec {
|
||||
resource_requirements: Some(openshell_core::proto::ResourceRequirements {
|
||||
gpu: None,
|
||||
cpu: None,
|
||||
memory: Some(openshell_core::proto::MemoryResourceRequirements {
|
||||
quantity: Some("0".to_string()),
|
||||
}),
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let err = validate_sandbox_spec("compute-sandbox", &spec).unwrap_err();
|
||||
assert_eq!(err.code(), Code::InvalidArgument);
|
||||
assert!(
|
||||
err.message()
|
||||
.contains("spec.resource_requirements.memory.quantity")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn validate_sandbox_spec_rejects_legacy_template_cpu_memory_resources() {
|
||||
use prost_types::{Struct, Value, value::Kind};
|
||||
|
||||
let mut limits = std::collections::BTreeMap::new();
|
||||
limits.insert(
|
||||
"cpu".to_string(),
|
||||
Value {
|
||||
kind: Some(Kind::StringValue("500m".to_string())),
|
||||
},
|
||||
);
|
||||
let mut fields = std::collections::BTreeMap::new();
|
||||
fields.insert(
|
||||
"limits".to_string(),
|
||||
Value {
|
||||
kind: Some(Kind::StructValue(Struct { fields: limits })),
|
||||
},
|
||||
);
|
||||
let spec = SandboxSpec {
|
||||
template: Some(SandboxTemplate {
|
||||
resources: Some(Struct { fields }),
|
||||
..Default::default()
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let err = validate_sandbox_spec("legacy-resources", &spec).unwrap_err();
|
||||
assert_eq!(err.code(), Code::InvalidArgument);
|
||||
assert!(err.message().contains("template.resources.limits.cpu"));
|
||||
assert!(
|
||||
err.message()
|
||||
.contains("spec.resource_requirements.cpu.quantity")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn validate_sandbox_spec_rejects_non_object_legacy_resource_sections() {
|
||||
use prost_types::{ListValue, Struct, Value, value::Kind};
|
||||
|
||||
let invalid_sections = [
|
||||
(
|
||||
"limits",
|
||||
"string",
|
||||
Some(Kind::StringValue("500m".to_string())),
|
||||
),
|
||||
(
|
||||
"requests",
|
||||
"list",
|
||||
Some(Kind::ListValue(ListValue { values: Vec::new() })),
|
||||
),
|
||||
("limits", "null", None),
|
||||
];
|
||||
|
||||
for (section_name, name, kind) in invalid_sections {
|
||||
let mut fields = std::collections::BTreeMap::new();
|
||||
fields.insert(section_name.to_string(), Value { kind });
|
||||
let spec = SandboxSpec {
|
||||
template: Some(SandboxTemplate {
|
||||
resources: Some(Struct { fields }),
|
||||
..Default::default()
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
let message = format!("{name} {section_name} section should be rejected");
|
||||
let err = validate_sandbox_spec("legacy-resources", &spec).expect_err(&message);
|
||||
assert_eq!(err.code(), Code::InvalidArgument, "{name}");
|
||||
assert_eq!(
|
||||
err.message(),
|
||||
format!("template.resources.{section_name} must be an object")
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn validate_sandbox_spec_accepts_empty_defaults() {
|
||||
assert!(validate_sandbox_spec("", &default_spec()).is_ok());
|
||||
|
||||
Reference in New Issue
Block a user