From 1cfd3d47acd0270427158c0950e8587c83c17e24 Mon Sep 17 00:00:00 2001 From: Jakhongir Rakhmonov Date: Mon, 13 Jul 2026 11:51:15 +0000 Subject: [PATCH] feat(firmware): allow custom version detection regexes --- crates/api-core/src/handlers/firmware.rs | 257 +++++++++++++++++- .../src/tests/host_firmware_config.rs | 64 +++++ crates/rpc/proto/forge.proto | 1 + .../api/pkg/api/model/hostfirmwareconfig.go | 23 +- .../pkg/api/model/hostfirmwareconfig_test.go | 42 +++ rest-api/docs/index.html | 12 +- rest-api/openapi/spec.yaml | 9 + rest-api/openapitools.json | 7 + rest-api/proto/core/gen/v1/nico_nico.pb.go | 16 +- rest-api/proto/core/src/v1/nico_nico.proto | 1 + .../model_host_firmware_component_config.go | 37 +++ 11 files changed, 448 insertions(+), 21 deletions(-) create mode 100644 rest-api/openapitools.json diff --git a/crates/api-core/src/handlers/firmware.rs b/crates/api-core/src/handlers/firmware.rs index 5e40e23592..5da4a6f018 100644 --- a/crates/api-core/src/handlers/firmware.rs +++ b/crates/api-core/src/handlers/firmware.rs @@ -33,6 +33,8 @@ use url::Url; use crate::CarbideError; use crate::api::{Api, log_request_data, log_request_data_redacted}; +const MAX_CURRENT_VERSION_REPORTED_AS_LENGTH: usize = 1024; + pub(crate) async fn set_firmware_update_time_window( api: &Api, request: Request, @@ -236,7 +238,7 @@ struct HostFirmwareConfigPatch { } struct FirmwareComponentPatch { - current_version_reported_as: Regex, + current_version_reported_as: Option, preingest_upgrade_when_below: Option, known_firmware: Vec, } @@ -247,16 +249,26 @@ struct FirmwareEntryPatch { } impl FirmwareComponentPatch { - fn into_component(self) -> FirmwareComponent { - FirmwareComponent { - current_version_reported_as: Some(self.current_version_reported_as), + fn into_component( + self, + vendor: bmc_vendor::BMCVendor, + model: &str, + component_type: FirmwareComponentType, + ) -> Result { + let current_version_reported_as = match self.current_version_reported_as { + Some(regex) => regex, + None => component_regex(vendor, model, component_type)?, + }; + + Ok(FirmwareComponent { + current_version_reported_as: Some(current_version_reported_as), preingest_upgrade_when_below: self.preingest_upgrade_when_below, known_firmware: self .known_firmware .into_iter() .map(FirmwareEntryPatch::into_entry) .collect(), - } + }) } } @@ -292,7 +304,12 @@ impl HostFirmwareConfigPatch { ))); } - let current_version_reported_as = component_regex(vendor, &model, component_type)?; + let current_version_reported_as = parse_component_regex( + vendor, + &model, + component_type, + component.current_version_reported_as.as_deref(), + )?; let preingest_upgrade_when_below = parse_preingest_upgrade_when_below( component_type, component.preingest_upgrade_when_below.as_deref(), @@ -345,9 +362,12 @@ fn merge_host_firmware_config_patch( if let Some(existing_component) = runtime_config.components.get_mut(&component_type) { merge_host_firmware_component(existing_component, incoming_component); } else { - runtime_config - .components - .insert(component_type, incoming_component.into_component()); + let component = incoming_component.into_component( + runtime_config.vendor, + &runtime_config.model, + component_type, + )?; + runtime_config.components.insert(component_type, component); } } @@ -364,8 +384,9 @@ fn merge_host_firmware_component( existing_component: &mut FirmwareComponent, incoming_component: FirmwareComponentPatch, ) { - existing_component.current_version_reported_as = - Some(incoming_component.current_version_reported_as); + if let Some(regex) = incoming_component.current_version_reported_as { + existing_component.current_version_reported_as = Some(regex); + } if incoming_component.preingest_upgrade_when_below.is_some() { existing_component.preingest_upgrade_when_below = incoming_component.preingest_upgrade_when_below; @@ -505,6 +526,34 @@ fn parse_vendor(vendor: &str) -> Result { Ok(parsed) } +fn parse_component_regex( + vendor: bmc_vendor::BMCVendor, + model: &str, + component_type: FirmwareComponentType, + value: Option<&str>, +) -> Result, CarbideError> { + value + .map(|value| { + if value.trim().is_empty() { + return Err(CarbideError::InvalidArgument(format!( + "current_version_reported_as is empty for {vendor} {model} {component_type}" + ))); + } + if value.chars().count() > MAX_CURRENT_VERSION_REPORTED_AS_LENGTH { + return Err(CarbideError::InvalidArgument(format!( + "current_version_reported_as exceeds {MAX_CURRENT_VERSION_REPORTED_AS_LENGTH} characters for {vendor} {model} {component_type}" + ))); + } + + Regex::new(value).map_err(|error| { + CarbideError::InvalidArgument(format!( + "invalid current_version_reported_as for {vendor} {model} {component_type}: {error}" + )) + }) + }) + .transpose() +} + fn component_regex( vendor: bmc_vendor::BMCVendor, model: &str, @@ -837,6 +886,59 @@ mod tests { ); } + #[test] + fn parse_component_regex_validates_supplied_values() { + let regex = parse_component_regex( + bmc_vendor::BMCVendor::Supermicro, + "SYS-121H-TNR", + FirmwareComponentType::Bmc, + Some("^BMC-Firmware$"), + ) + .unwrap() + .expect("supplied regex"); + assert_eq!(regex.as_str(), "^BMC-Firmware$"); + + assert!( + parse_component_regex( + bmc_vendor::BMCVendor::Supermicro, + "SYS-121H-TNR", + FirmwareComponentType::Bmc, + None, + ) + .unwrap() + .is_none() + ); + assert!( + parse_component_regex( + bmc_vendor::BMCVendor::Supermicro, + "SYS-121H-TNR", + FirmwareComponentType::Bmc, + Some(" "), + ) + .is_err() + ); + assert!( + parse_component_regex( + bmc_vendor::BMCVendor::Supermicro, + "SYS-121H-TNR", + FirmwareComponentType::Bmc, + Some("("), + ) + .is_err() + ); + + let oversized = "a".repeat(MAX_CURRENT_VERSION_REPORTED_AS_LENGTH + 1); + assert!( + parse_component_regex( + bmc_vendor::BMCVendor::Supermicro, + "SYS-121H-TNR", + FirmwareComponentType::Bmc, + Some(&oversized), + ) + .is_err() + ); + } + #[test] fn parse_host_firmware_config_model_trims_and_rejects_empty_values() { scenarios!(run = |input| parse_host_firmware_config_model(input).map_err(drop); @@ -909,6 +1011,7 @@ mod tests { preingestion_exclusive_config: None, }], preingest_upgrade_when_below: Some("1.0.0".to_string()), + current_version_reported_as: None, }], explicit_start_needed: Some(true), ordering: vec![rpc::HostFirmwareComponentType::Bmc as i32], @@ -1045,6 +1148,134 @@ mod tests { ); } + #[test] + fn merge_host_firmware_config_preserves_omitted_component_regex() { + let existing = test_firmware(HashMap::from([( + FirmwareComponentType::Bmc, + test_component( + "^existing-bmc$", + vec![test_entry( + "7.20", + true, + "https://firmware.example.invalid/bmc-7.20.bin", + )], + ), + )])); + let mut patch = test_patch(HashMap::from([( + FirmwareComponentType::Bmc, + test_component( + "^ignored$", + vec![test_entry( + "7.30", + true, + "https://firmware.example.invalid/bmc-7.30.bin", + )], + ), + )])); + patch + .components + .get_mut(&FirmwareComponentType::Bmc) + .unwrap() + .current_version_reported_as = None; + + let merged = merge_host_firmware_config_patch(Some(existing), patch).unwrap(); + + assert_eq!( + merged.components[&FirmwareComponentType::Bmc] + .current_version_reported_as + .as_ref() + .unwrap() + .as_str(), + "^existing-bmc$" + ); + } + + #[test] + fn merge_host_firmware_config_uses_catalog_regex_for_new_component() { + let mut patch = test_patch(HashMap::from([( + FirmwareComponentType::Cx7, + test_component( + "^ignored$", + vec![test_entry( + "28.47.2682", + true, + "https://firmware.example.invalid/cx7.bin", + )], + ), + )])); + patch.ordering = vec![FirmwareComponentType::Cx7]; + patch + .components + .get_mut(&FirmwareComponentType::Cx7) + .unwrap() + .current_version_reported_as = None; + + let merged = merge_host_firmware_config_patch(None, patch).unwrap(); + + assert_eq!( + merged.components[&FirmwareComponentType::Cx7] + .current_version_reported_as + .as_ref() + .unwrap() + .as_str(), + "^CX7_[0-9]+$" + ); + } + + #[test] + fn merge_host_firmware_config_accepts_custom_regex_without_catalog_mapping() { + let mut patch = test_patch(HashMap::from([( + FirmwareComponentType::Bmc, + test_component( + "^BMC-Firmware$", + vec![test_entry( + "1.0.0", + true, + "https://firmware.example.invalid/bmc.bin", + )], + ), + )])); + patch.vendor = bmc_vendor::BMCVendor::Supermicro; + patch.model = "SYS-121H-TNR".to_string(); + patch.ordering = vec![FirmwareComponentType::Bmc]; + + let merged = merge_host_firmware_config_patch(None, patch).unwrap(); + + assert_eq!( + merged.components[&FirmwareComponentType::Bmc] + .current_version_reported_as + .as_ref() + .unwrap() + .as_str(), + "^BMC-Firmware$" + ); + } + + #[test] + fn merge_host_firmware_config_rejects_new_component_without_regex_mapping() { + let mut patch = test_patch(HashMap::from([( + FirmwareComponentType::Bmc, + test_component( + "^ignored$", + vec![test_entry( + "1.0.0", + true, + "https://firmware.example.invalid/bmc.bin", + )], + ), + )])); + patch.vendor = bmc_vendor::BMCVendor::Supermicro; + patch.model = "SYS-121H-TNR".to_string(); + patch.ordering = vec![FirmwareComponentType::Bmc]; + patch + .components + .get_mut(&FirmwareComponentType::Bmc) + .unwrap() + .current_version_reported_as = None; + + assert!(merge_host_firmware_config_patch(None, patch).is_err()); + } + #[test] fn merge_host_firmware_config_replaces_existing_version_and_preserves_default() { let existing = test_firmware(HashMap::from([( @@ -1480,7 +1711,7 @@ mod tests { .collect(); FirmwareComponentPatch { - current_version_reported_as: component.current_version_reported_as.unwrap(), + current_version_reported_as: component.current_version_reported_as, preingest_upgrade_when_below: component.preingest_upgrade_when_below, known_firmware: firmware, } @@ -1491,7 +1722,7 @@ mod tests { entries: Vec<(FirmwareEntry, Option)>, ) -> FirmwareComponentPatch { FirmwareComponentPatch { - current_version_reported_as: Regex::new(regex).unwrap(), + current_version_reported_as: Some(Regex::new(regex).unwrap()), preingest_upgrade_when_below: None, known_firmware: entries .into_iter() diff --git a/crates/api-core/src/tests/host_firmware_config.rs b/crates/api-core/src/tests/host_firmware_config.rs index ce8fb9185d..0f2b35ef2b 100644 --- a/crates/api-core/src/tests/host_firmware_config.rs +++ b/crates/api-core/src/tests/host_firmware_config.rs @@ -73,6 +73,10 @@ async fn upsert_host_firmware_config_creates_and_merges_versions( ); let cx7 = response_component(&response.components, HostFirmwareComponentType::Cx7); + assert_eq!( + cx7.current_version_reported_as.as_deref(), + Some("^CX7_[0-9]+$") + ); assert_eq!( cx7.preingest_upgrade_when_below.as_deref(), Some("28.48.1000") @@ -121,6 +125,56 @@ async fn upsert_host_firmware_config_creates_and_merges_versions( Ok(()) } +#[crate::sqlx_test] +async fn upsert_host_firmware_config_accepts_and_preserves_custom_component_regex( + pool: sqlx::PgPool, +) -> Result<(), Box> { + let env = create_test_env(pool).await; + + env.api + .upsert_host_firmware_config(Request::new(upsert_request_for( + "Supermicro", + "SYS-121H-TNR", + vec![component_config_with_regex( + HostFirmwareComponentType::Bmc, + vec![version_config("1.0.0", true)], + None, + Some("^BMC-Firmware$"), + )], + vec![HostFirmwareComponentType::Bmc], + Some(false), + ))) + .await?; + + let response = env + .api + .upsert_host_firmware_config(Request::new(upsert_request_for( + "Supermicro", + "SYS-121H-TNR", + vec![component_config( + HostFirmwareComponentType::Bmc, + vec![version_config("1.1.0", true)], + None, + )], + Vec::new(), + None, + ))) + .await? + .into_inner(); + + let bmc = response_component(&response.components, HostFirmwareComponentType::Bmc); + assert_eq!( + bmc.current_version_reported_as.as_deref(), + Some("^BMC-Firmware$") + ); + assert_eq!( + firmware_defaults(bmc), + vec![("1.0.0", false), ("1.1.0", true)] + ); + + Ok(()) +} + #[crate::sqlx_test] async fn upsert_host_firmware_config_rejects_create_without_ordering( pool: sqlx::PgPool, @@ -394,11 +448,21 @@ fn component_config( component_type: HostFirmwareComponentType, firmware: Vec, preingest_upgrade_when_below: Option<&str>, +) -> UpsertHostFirmwareComponentConfig { + component_config_with_regex(component_type, firmware, preingest_upgrade_when_below, None) +} + +fn component_config_with_regex( + component_type: HostFirmwareComponentType, + firmware: Vec, + preingest_upgrade_when_below: Option<&str>, + current_version_reported_as: Option<&str>, ) -> UpsertHostFirmwareComponentConfig { UpsertHostFirmwareComponentConfig { r#type: component_type as i32, firmware, preingest_upgrade_when_below: preingest_upgrade_when_below.map(str::to_string), + current_version_reported_as: current_version_reported_as.map(str::to_string), } } diff --git a/crates/rpc/proto/forge.proto b/crates/rpc/proto/forge.proto index ee83b9e32d..8055e02b78 100644 --- a/crates/rpc/proto/forge.proto +++ b/crates/rpc/proto/forge.proto @@ -7983,6 +7983,7 @@ message UpsertHostFirmwareComponentConfig { HostFirmwareComponentType type = 1; repeated HostFirmwareVersionConfig firmware = 2; optional string preingest_upgrade_when_below = 3; + optional string current_version_reported_as = 4; } message HostFirmwareComponentConfigResponse { diff --git a/rest-api/api/pkg/api/model/hostfirmwareconfig.go b/rest-api/api/pkg/api/model/hostfirmwareconfig.go index ace8edd7b1..6b89d26225 100644 --- a/rest-api/api/pkg/api/model/hostfirmwareconfig.go +++ b/rest-api/api/pkg/api/model/hostfirmwareconfig.go @@ -8,6 +8,7 @@ import ( "fmt" "strings" "time" + "unicode/utf8" camu "github.com/NVIDIA/infra-controller/rest-api/api/pkg/api/model/util" cutil "github.com/NVIDIA/infra-controller/rest-api/common/pkg/util" @@ -31,6 +32,8 @@ const ( HostFirmwareComponentTypeCombinedBmcUefi HostFirmwareComponentType = "CombinedBmcUefi" HostFirmwareComponentTypeGPU HostFirmwareComponentType = "GPU" HostFirmwareComponentTypeCx7 HostFirmwareComponentType = "Cx7" + + maxCurrentVersionDetectionRegExLength = 1024 ) var hostFirmwareComponentTypeChoiceMap = map[HostFirmwareComponentType]corev1.HostFirmwareComponentType{ @@ -104,9 +107,10 @@ type APIHostFirmwareVersionConfig struct { // APIHostFirmwareComponentConfig is one component entry in an upsert request. type APIHostFirmwareComponentConfig struct { - Type HostFirmwareComponentType `json:"type"` - Firmware []APIHostFirmwareVersionConfig `json:"firmware"` - PreingestUpgradeWhenBelow *string `json:"preingestUpgradeWhenBelow,omitempty"` + Type HostFirmwareComponentType `json:"type"` + Firmware []APIHostFirmwareVersionConfig `json:"firmware"` + PreingestUpgradeWhenBelow *string `json:"preingestUpgradeWhenBelow,omitempty"` + CurrentVersionDetectionRegEx *string `json:"currentVersionDetectionRegEx,omitempty"` } // APIHostFirmwareComponent is one component entry in a HostFirmwareConfig response. @@ -203,6 +207,16 @@ func (c APIHostFirmwareComponentConfig) Validate() error { if c.PreingestUpgradeWhenBelow != nil && strings.TrimSpace(*c.PreingestUpgradeWhenBelow) == "" { return validation.Errors{"preingestUpgradeWhenBelow": errors.New("must not be empty when provided")} } + if c.CurrentVersionDetectionRegEx != nil { + if strings.TrimSpace(*c.CurrentVersionDetectionRegEx) == "" { + return validation.Errors{"currentVersionDetectionRegEx": errors.New("must not be empty when provided")} + } + if utf8.RuneCountInString(*c.CurrentVersionDetectionRegEx) > maxCurrentVersionDetectionRegExLength { + return validation.Errors{ + "currentVersionDetectionRegEx": fmt.Errorf("must not exceed %d characters", maxCurrentVersionDetectionRegExLength), + } + } + } if len(c.Firmware) == 0 { return validation.Errors{"firmware": errors.New("at least one firmware version is required")} } @@ -223,6 +237,9 @@ func (c APIHostFirmwareComponentConfig) ToProto() *corev1.UpsertHostFirmwareComp trimmed := strings.TrimSpace(*c.PreingestUpgradeWhenBelow) protoComponent.PreingestUpgradeWhenBelow = &trimmed } + if c.CurrentVersionDetectionRegEx != nil { + protoComponent.CurrentVersionReportedAs = c.CurrentVersionDetectionRegEx + } for _, firmware := range c.Firmware { protoComponent.Firmware = append(protoComponent.Firmware, firmware.ToProto()) } diff --git a/rest-api/api/pkg/api/model/hostfirmwareconfig_test.go b/rest-api/api/pkg/api/model/hostfirmwareconfig_test.go index 7887677b61..159c783b40 100644 --- a/rest-api/api/pkg/api/model/hostfirmwareconfig_test.go +++ b/rest-api/api/pkg/api/model/hostfirmwareconfig_test.go @@ -54,6 +54,48 @@ func TestAPIHostFirmwareComponentConfig_ToProto_preingestUpgradeWhenBelow(t *tes assert.Equal(t, "28.48.1000", *proto.PreingestUpgradeWhenBelow) } +func TestAPIHostFirmwareComponentConfig_Validate_currentVersionDetectionRegEx(t *testing.T) { + validComponent := func(regex *string) APIHostFirmwareComponentConfig { + return APIHostFirmwareComponentConfig{ + Type: HostFirmwareComponentTypeBMC, + CurrentVersionDetectionRegEx: regex, + Firmware: []APIHostFirmwareVersionConfig{{ + Version: "1.0.0", + Default: true, + Artifacts: []APIHostFirmwareArtifact{{ + URL: "https://firmware.example.invalid/1.0.0/fw.bin", + }}, + }}, + } + } + + assert.NoError(t, validComponent(nil).Validate()) + assert.NoError(t, validComponent(cutil.GetPtr("^BMC-Firmware$")).Validate()) + + for _, invalid := range []string{"", " ", strings.Repeat("a", maxCurrentVersionDetectionRegExLength+1)} { + err := validComponent(&invalid).Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), "currentVersionDetectionRegEx") + } +} + +func TestAPIHostFirmwareComponentConfig_ToProto_currentVersionDetectionRegEx(t *testing.T) { + proto := APIHostFirmwareComponentConfig{ + Type: HostFirmwareComponentTypeBMC, + CurrentVersionDetectionRegEx: cutil.GetPtr(" ^BMC-Firmware$ "), + Firmware: []APIHostFirmwareVersionConfig{{ + Version: "1.0.0", + Default: true, + Artifacts: []APIHostFirmwareArtifact{{ + URL: "https://firmware.example.invalid/1.0.0/fw.bin", + }}, + }}, + }.ToProto() + + require.NotNil(t, proto.CurrentVersionReportedAs) + assert.Equal(t, " ^BMC-Firmware$ ", *proto.CurrentVersionReportedAs) +} + func TestAPIHostFirmwareVersionConfig_Validate_artifactSha256(t *testing.T) { validSHA := "a1b2c3d4e5f6789012345678901234567890abcdef1234567890abcdef123456" validVersion := func(sha256 *string) APIHostFirmwareVersionConfig { diff --git a/rest-api/docs/index.html b/rest-api/docs/index.html index ea1ec69c3f..4e382a1231 100644 --- a/rest-api/docs/index.html +++ b/rest-api/docs/index.html @@ -14360,10 +14360,18 @@

Typical API Call Flow for Tenant

" class="sc-iJSMbW sc-cBEgGa fiNpIH bAoMjv">

Host firmware component type exposed by the REST API.

required
Array of objects (HostFirmwareVersionConfig) non-empty

Known firmware versions for this component. Versions are merged by version string on update.

-
preingestUpgradeWhenBelow
string non-empty
preingestUpgradeWhenBelow
string non-empty

Optional minimum firmware version required before BMC pre-ingestion can complete. Omitted on update preserves the stored value.

+
currentVersionDetectionRegEx
string [ 1 .. 1024 ] characters

Optional inventory identifier regex used to find this component's +reported firmware version. Omitted on update preserves the stored +value. Omitted for a new component uses Core's built-in mapping when +one exists; otherwise this field is required.

explicitStartNeeded
boolean
Typical API Call Flow for Tenant