diff --git a/Cargo.lock b/Cargo.lock index ec73c65101..95dadc608e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1258,6 +1258,7 @@ dependencies = [ "librms", "mac_address", "mockito", + "moka", "mqttea", "nras", "num_cpus", diff --git a/crates/admin-cli/src/expected_machines/add/args.rs b/crates/admin-cli/src/expected_machines/add/args.rs index 67fc82bbe5..ab6db99771 100644 --- a/crates/admin-cli/src/expected_machines/add/args.rs +++ b/crates/admin-cli/src/expected_machines/add/args.rs @@ -188,6 +188,13 @@ pub struct Args { help = "If true, do not lock down the server as part of lifecycle management within the state machine. If unset or false, preserve the default behavior of locking down the server after configuring the BIOS." )] pub disable_lockdown: Option, + + #[clap( + long = "bmc-vendor-override", + value_name = "BMC_VENDOR_OVERRIDE", + help = "Pin the Redfish BMC vendor for this host. Once set it governs how NICo talks to this BMC -- which libredfish driver each Redfish client dispatches on, the vendor recorded by Site Explorer, and therefore the firmware config lookup, the IPMI-vs-Redfish restart choice and the BMC console transport. Host lifecycle decisions keyed to the host's own DMI data are unaffected. A RedfishVendor variant name, case-sensitive (e.g. Dell, Supermicro, NvidiaDpu, Hpe, Lenovo). Unset means automatic detection. Not applied to credential rotation or factory bootstrap, which must reach a BMC before its vendor is usable." + )] + pub bmc_vendor_override: Option, } impl Args { @@ -240,6 +247,7 @@ impl TryFrom for rpc::forge::ExpectedMachine { disable_lockdown: Some(dl), } }), + bmc_vendor_override: value.bmc_vendor_override, }) } } diff --git a/crates/admin-cli/src/expected_machines/common.rs b/crates/admin-cli/src/expected_machines/common.rs index 9089df031f..8d0f7048de 100644 --- a/crates/admin-cli/src/expected_machines/common.rs +++ b/crates/admin-cli/src/expected_machines/common.rs @@ -166,6 +166,10 @@ pub struct ExpectedMachineJson { /// Per-host lifecycle profile for settings that affect state-machine progression. #[serde(default)] pub host_lifecycle_profile: Option, + /// Operator pinned Redfish BMC vendor, a `RedfishVendor` variant name such as + /// `Dell`. Absent or empty means automatic detection. + #[serde(default)] + pub bmc_vendor_override: Option, } impl ExpectedMachineJson { diff --git a/crates/admin-cli/src/expected_machines/patch/args.rs b/crates/admin-cli/src/expected_machines/patch/args.rs index 38d376609a..e232b1f1ee 100644 --- a/crates/admin-cli/src/expected_machines/patch/args.rs +++ b/crates/admin-cli/src/expected_machines/patch/args.rs @@ -52,6 +52,7 @@ use crate::expected_machines::common::HostDpuPolicy; "bmc_ip_allocation", "dpf_enabled", "host_nics", +"bmc_vendor_override", ])))] #[command(after_long_help = "\ EXAMPLES: @@ -76,6 +77,14 @@ Retain the BMC's auto-allocated DHCP address as a static one (never expires): $ nico-admin-cli expected-machine patch --bmc-mac-address 00:11:22:33:44:55 \ --bmc-ip-allocation retained +Pin the Redfish BMC vendor for a host: + $ nico-admin-cli expected-machine patch --bmc-mac-address 00:11:22:33:44:55 \ + --bmc-vendor-override Dell + +Clear the Redfish BMC vendor override (return to automatic detection): + $ nico-admin-cli expected-machine patch --bmc-mac-address 00:11:22:33:44:55 \ + --bmc-vendor-override \"\" + ")] pub struct Args { #[clap(short = 'a', long, help = "BMC MAC Address of the expected machine")] @@ -218,6 +227,14 @@ pub struct Args { help = "If true, do not lock down the server as part of lifecycle management within the state machine. If unset or false, preserve the default behavior of locking down the server after configuring the BIOS." )] pub disable_lockdown: Option, + + #[clap( + long = "bmc-vendor-override", + value_name = "BMC_VENDOR_OVERRIDE", + group = "group", + help = "Pin the Redfish BMC vendor for this host. Once set it governs how NICo talks to this BMC -- which libredfish driver each Redfish client dispatches on, the vendor recorded by Site Explorer, and therefore the firmware config lookup, the IPMI-vs-Redfish restart choice and the BMC console transport. Host lifecycle decisions keyed to the host's own DMI data are unaffected. A RedfishVendor variant name, case-sensitive (e.g. Dell, Supermicro, NvidiaDpu, Hpe, Lenovo). Redfish clients pick it up within a minute; the recorded vendor changes at the next successful exploration. Pass an empty string to clear the override (return to automatic detection). Not applied to credential rotation or factory bootstrap, which must reach a BMC before its vendor is usable." + )] + pub bmc_vendor_override: Option, } impl Args { @@ -246,8 +263,9 @@ impl Args { && self.dpu_policy.is_none() && self.bmc_ip_allocation.is_none() && self.host_nics.is_none() + && self.bmc_vendor_override.is_none() { - return Err(CarbideCliError::GenericError("One of the following options must be specified: bmc-username and bmc-password or chassis-serial-number or fallback-dpu-serial-number or bmc-ip-address or dpu-policy or bmc-ip-allocation or dpf-enabled or host_nics".to_string())); + return Err(CarbideCliError::GenericError("One of the following options must be specified: bmc-username and bmc-password or chassis-serial-number or fallback-dpu-serial-number or bmc-ip-address or dpu-policy or bmc-ip-allocation or dpf-enabled or host_nics or bmc-vendor-override".to_string())); } if self .fallback_dpu_serial_numbers diff --git a/crates/admin-cli/src/expected_machines/patch/mod.rs b/crates/admin-cli/src/expected_machines/patch/mod.rs index 3066176c5f..9cf1a0b2ad 100644 --- a/crates/admin-cli/src/expected_machines/patch/mod.rs +++ b/crates/admin-cli/src/expected_machines/patch/mod.rs @@ -57,6 +57,7 @@ impl Run for Args { disable_lockdown: Some(dl), }), self.host_nics, + self.bmc_vendor_override, ) .await?; Ok(()) diff --git a/crates/admin-cli/src/expected_machines/update/mod.rs b/crates/admin-cli/src/expected_machines/update/mod.rs index 4a95f9dde6..7987d52b7b 100644 --- a/crates/admin-cli/src/expected_machines/update/mod.rs +++ b/crates/admin-cli/src/expected_machines/update/mod.rs @@ -79,6 +79,7 @@ impl Run for Args { // TODO: file-based update preserves existing host_nics; wire in // expected_machine.host_nics to honor the file's list None, + expected_machine.bmc_vendor_override, ) .await?; Ok(()) diff --git a/crates/admin-cli/src/rpc.rs b/crates/admin-cli/src/rpc.rs index 759ca09bd7..609b3986c5 100644 --- a/crates/admin-cli/src/rpc.rs +++ b/crates/admin-cli/src/rpc.rs @@ -834,6 +834,7 @@ impl ApiClient { bmc_ip_allocation: Option<::rpc::forge::BmcIpAllocationType>, host_lifecycle_profile: Option<::rpc::forge::HostLifecycleProfile>, host_nics: Option, + bmc_vendor_override: Option, ) -> Result<(), CarbideCliError> { let get_req = match (bmc_mac_address, id) { (Some(_), Some(_)) => { @@ -927,6 +928,9 @@ impl ApiClient { .or(expected_machine.bmc_ip_allocation), host_lifecycle_profile: host_lifecycle_profile .or(expected_machine.host_lifecycle_profile), + // Patch semantics. Use the flag when given, where an empty string + // clears the override, and otherwise keep the stored value. + bmc_vendor_override: bmc_vendor_override.or(expected_machine.bmc_vendor_override), }; Ok(self.0.update_expected_machine(request).await?) @@ -970,6 +974,7 @@ impl ApiClient { disable_lockdown: hlp.disable_lockdown, } }), + bmc_vendor_override: machine.bmc_vendor_override, }) .collect(), }; diff --git a/crates/api-core/Cargo.toml b/crates/api-core/Cargo.toml index c6d76d3f13..97d375b9b6 100644 --- a/crates/api-core/Cargo.toml +++ b/crates/api-core/Cargo.toml @@ -111,6 +111,7 @@ lazy_static = { workspace = true } libredfish = { workspace = true } librms = { workspace = true } mac_address = { workspace = true } +moka = { workspace = true } num_cpus = { workspace = true } nv-redfish = { workspace = true } opentelemetry = { workspace = true, features = ["logs"] } diff --git a/crates/api-core/src/handlers/expected_machine.rs b/crates/api-core/src/handlers/expected_machine.rs index be84a37cc1..36e5ec392a 100644 --- a/crates/api-core/src/handlers/expected_machine.rs +++ b/crates/api-core/src/handlers/expected_machine.rs @@ -128,10 +128,39 @@ pub(crate) fn validate_expected_machine_for_insert( .bmc_ip_allocation .validate(machine.data.bmc_ip_address.is_some()) .map_err(|msg| CarbideError::InvalidArgument(msg.to_string()))?; + validate_bmc_vendor_override(machine)?; Ok(()) } +/// Reject a `bmc_vendor_override` libredfish could not use, so a typo fails the +/// write rather than being stored as a pin that never applies. `Unknown` is +/// refused too, being the pool sentinel for an uninitialized client. An empty +/// string means no pin and is accepted, since the JSON import can carry one. +fn validate_bmc_vendor_override(machine: &ExpectedMachine) -> Result<(), CarbideError> { + use libredfish::model::service_root::RedfishVendor; + + let Some(name) = machine + .data + .bmc_vendor_override + .as_deref() + .filter(|name| !name.is_empty()) + else { + return Ok(()); + }; + + match carbide_redfish::libredfish::conv::redfish_vendor_from_str(name) { + Some(vendor) if vendor != RedfishVendor::Unknown => Ok(()), + _ => Err(CarbideError::InvalidArgument(format!( + // xtask:allow-error-case: the vendor spellings are case-sensitive values + "bmc_vendor_override {name:?} is not a usable Redfish vendor name, \ + which is the exact case-sensitive variant spelling \ + such as \"Dell\", \"Supermicro\" or \"NvidiaDpu\". Pass an empty \ + string to clear the override and fall back to automatic detection" + ))), + } +} + /// Create missing expected_machines that aren't already in the database, /// calling `validate_expected_machine_for_insert` for each new entry. This is currently /// purely used by the expected_machines.json import path only, but lives @@ -229,6 +258,7 @@ pub(crate) async fn update( .bmc_ip_allocation .validate(machine.data.bmc_ip_address.is_some()) .map_err(|msg| CarbideError::InvalidArgument(msg.to_string()))?; + validate_bmc_vendor_override(&machine)?; let mut txn = api.txn_begin().await?; @@ -458,6 +488,8 @@ async fn update_expected_machine( data, }; + validate_bmc_vendor_override(&expected_machine)?; + if let Some(bmc_ip) = expected_machine.data.bmc_ip_address { update_preallocated_machine_interface( txn, @@ -715,4 +747,42 @@ mod tests { let too_long = "A".repeat(65); assert!(!CHASSIS_SERIAL_REGEX.is_match(&too_long)); } + + /// A pin libredfish cannot use has to fail the write. Storing it would leave + /// an operator believing the vendor is pinned while every client keeps + /// detecting, with only a server side warn to say otherwise. + #[test] + fn bmc_vendor_override_rejects_names_libredfish_cannot_use() { + fn validate(stored: Option<&str>) -> Result<(), CarbideError> { + validate_bmc_vendor_override(&ExpectedMachine { + id: None, + bmc_mac_address: "02:00:00:00:00:01".parse().expect("valid test MAC"), + data: ExpectedMachineData { + bmc_vendor_override: stored.map(str::to_string), + ..Default::default() + }, + }) + } + + assert!(validate(None).is_ok(), "no pin is always valid"); + assert!( + validate(Some("")).is_ok(), + "an empty string clears the pin, expected_machines.json can carry it \ + literally, and failing it would bail API startup" + ); + assert!(validate(Some("Dell")).is_ok(), "an exact variant name"); + assert!( + validate(Some("NvidiaDpu")).is_ok(), + "a multi-word variant name" + ); + + // Case matters, so the near miss an operator is most likely to type has + // to be rejected rather than silently stored. + assert!(validate(Some("dell")).is_err(), "wrong case"); + assert!(validate(Some("Del")).is_err(), "typo"); + assert!( + validate(Some("Unknown")).is_err(), + "the uninitialized-client sentinel is not a pinnable vendor" + ); + } } diff --git a/crates/api-core/src/handlers/instance.rs b/crates/api-core/src/handlers/instance.rs index 2b3dfad671..a8bb556357 100644 --- a/crates/api-core/src/handlers/instance.rs +++ b/crates/api-core/src/handlers/instance.rs @@ -20,7 +20,7 @@ use std::str::FromStr; use ::rpc::errors::RpcDataConversionError; use ::rpc::forge::{self as rpc, AdminForceDeleteMachineResponse}; use ::rpc::model::RpcTryFrom; -use carbide_redfish::libredfish::RedfishAuth; +use carbide_redfish::libredfish::{RedfishAuth, VendorSelection}; use carbide_secrets::credentials::{BmcCredentialType, CredentialKey}; use carbide_uuid::infiniband::IBPartitionId; use carbide_uuid::instance::InstanceId; @@ -1102,7 +1102,7 @@ pub(crate) async fn invoke_power( RedfishAuth::Key(CredentialKey::BmcCredentials { credential_type: BmcCredentialType::BmcRoot { bmc_mac_address }, }), - None, + VendorSelection::Detect, ) .await .map_err(|e| CarbideError::internal(e.to_string()))?; diff --git a/crates/api-core/src/handlers/machine.rs b/crates/api-core/src/handlers/machine.rs index e29f7eda25..9f1e4bfd15 100644 --- a/crates/api-core/src/handlers/machine.rs +++ b/crates/api-core/src/handlers/machine.rs @@ -20,7 +20,7 @@ use std::collections::HashMap; use ::rpc::errors::RpcDataConversionError; use ::rpc::forge as rpc; use ::rpc::model::machine::ManagedHostStateSnapshotRpc; -use carbide_redfish::libredfish::RedfishAuth; +use carbide_redfish::libredfish::{RedfishAuth, VendorSelection}; use carbide_secrets::credentials::{BmcCredentialType, CredentialKey, Credentials}; use carbide_uuid::machine::MachineId; use libredfish::SystemPowerControl; @@ -515,7 +515,7 @@ pub(crate) async fn admin_force_delete_machine( RedfishAuth::Key(CredentialKey::BmcCredentials { credential_type: BmcCredentialType::BmcRoot { bmc_mac_address }, }), - None, + VendorSelection::Detect, ) .await { diff --git a/crates/api-core/src/lib.rs b/crates/api-core/src/lib.rs index 612527ae0d..4bb63ba8e1 100644 --- a/crates/api-core/src/lib.rs +++ b/crates/api-core/src/lib.rs @@ -67,6 +67,7 @@ mod machine_validation; mod measured_boot; mod mqtt_state_change_hook; mod network_segment; +mod redfish_vendor_override; mod scout_stream; pub mod secrets; mod setup; diff --git a/crates/api-core/src/redfish_vendor_override.rs b/crates/api-core/src/redfish_vendor_override.rs new file mode 100644 index 0000000000..bd52f7df91 --- /dev/null +++ b/crates/api-core/src/redfish_vendor_override.rs @@ -0,0 +1,237 @@ +/* + * SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +//! Database backed lookup for the operator BMC vendor pin. +//! +//! Lives here because it bridges two sibling crates, one holding the trait and +//! the other the query, with neither depending on the other. + +use std::net::IpAddr; +use std::sync::Arc; +use std::time::Duration; + +use async_trait::async_trait; +use carbide_redfish::libredfish::conv; +use carbide_redfish::libredfish::vendor_override::{ + BmcVendorOverrideResolver, VendorOverrideError, +}; +use carbide_utils::redfish::parse_uri_host_ip; +use libredfish::model::service_root::RedfishVendor; +use sqlx::PgPool; + +/// How long a resolved pin, or the absence of one, is reused before a fresh read. +/// +/// Bounded staleness beats invalidating on write, because writes key on BMC MAC +/// while this cache keys on BMC IP and a future write path could skip a hook. +const DEFAULT_PIN_CACHE_TTL: Duration = Duration::from_secs(60); + +/// Upper bound on cached BMC addresses, so probing unknown ones cannot grow the +/// cache without limit. +const PIN_CACHE_CAPACITY: u64 = 4096; + +/// Resolves `bmc_vendor_override` for a BMC address, with a short lived cache. +pub struct DbBmcVendorOverrideResolver { + db_pool: PgPool, + /// Caches the absence of a pin as well as its presence, which is the point. + /// Almost no BMC has a pin and `create_client` runs many times per BMC per + /// sweep, so without negative caching the common path would query Postgres + /// on every client construction. + cache: moka::future::Cache>, +} + +impl DbBmcVendorOverrideResolver { + pub fn new(db_pool: PgPool) -> Self { + Self::with_cache_ttl(db_pool, DEFAULT_PIN_CACHE_TTL) + } + + /// Same, with an explicit TTL so tests can observe expiry without sleeping. + pub fn with_cache_ttl(db_pool: PgPool, ttl: Duration) -> Self { + Self { + db_pool, + cache: moka::future::Cache::builder() + .max_capacity(PIN_CACHE_CAPACITY) + .time_to_live(ttl) + .build(), + } + } +} + +#[async_trait] +impl BmcVendorOverrideResolver for DbBmcVendorOverrideResolver { + async fn vendor_override( + &self, + host: &str, + ) -> Result, VendorOverrideError> { + // Pins resolve against `machine_interface_addresses.address`, so a BMC + // addressed by hostname has nothing to match on. That is a supported way + // to reach a BMC, just not one a pin can be keyed to, so report no pin. + let Some(ip) = parse_uri_host_ip(host) else { + return Ok(None); + }; + + // `try_get_with` collapses concurrent lookups for one BMC into a single + // query and, unlike `get_with`, does not cache failures, so a recovered + // database is picked up on the next call rather than after the TTL. + self.cache + .try_get_with(ip, async { + let raw = db::machine_interface::find_bmc_vendor_override_by_ip( + &self.db_pool, + ip, + ) + .await + .map_err(|error| { + tracing::warn!( + bmc = %ip, + %error, + "Failed to read bmc_vendor_override, BMC clients will use automatic detection" + ); + VendorOverrideError::new(ip.to_string(), Box::new(error)) + }); + + // A failure is remembered as "no pin" for the TTL. `try_get_with` + // caches only successes, and BMC client construction used to touch + // no database at all, so retrying per client would turn an outage + // into one blocking pool acquire per Redfish call. + if raw.is_err() { + self.cache.insert(ip, None).await; + } + let raw = raw?; + + // Parsing, the warning for an unusable name, and the refusal of + // `Unknown` all live in `redfish_vendor_override`, the single + // place a stored name is interpreted. + Ok(conv::redfish_vendor_override(host, raw.as_deref())) + }) + .await + .map_err(|error: Arc| VendorOverrideError { + host: error.host.clone(), + // The Arc belongs to moka, so wrap it again and let callers see + // an owned error without the cache sharing showing through. + source: Box::new(std::io::Error::other(error.source.to_string())), + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Seed one BMC interface at `ip` whose expected machine pins `pin`. + async fn seed_pinned_bmc(pool: &PgPool, ip: IpAddr, mac: &str, pin: &str) { + use model::network_segment::{ + AllocationStrategy, NetworkSegmentControllerState, NetworkSegmentType, + NewNetworkSegment, + }; + + // Built through the real persist path rather than a hand written INSERT + // so this fixture cannot drift from the segment schema. + let segment_id = carbide_uuid::network::NetworkSegmentId::new(); + let mut txn = db::Transaction::begin(pool).await.expect("txn"); + db::network_segment::persist( + NewNetworkSegment { + id: segment_id, + name: "pin-cache-segment".to_string(), + subdomain_id: None, + vpc_id: None, + mtu: 1500, + prefixes: Vec::new(), + vlan_id: None, + vni: None, + segment_type: NetworkSegmentType::HostInband, + can_stretch: Some(false), + allocation_strategy: AllocationStrategy::Reserved, + }, + txn.as_pgconn(), + NetworkSegmentControllerState::Ready, + ) + .await + .expect("segment"); + txn.commit().await.expect("commit segment"); + + let interface: uuid::Uuid = sqlx::query_scalar( + "INSERT INTO machine_interfaces \ + (segment_id, mac_address, primary_interface, hostname, interface_type) \ + VALUES ($1, $2::macaddr, false, 'pin-cache-bmc', 'Bmc') RETURNING id", + ) + .bind(segment_id) + .bind(mac) + .fetch_one(pool) + .await + .expect("interface"); + + sqlx::query( + "INSERT INTO machine_interface_addresses (interface_id, address) VALUES ($1, $2::inet)", + ) + .bind(interface) + .bind(ip) + .execute(pool) + .await + .expect("address"); + + sqlx::query( + "INSERT INTO expected_machines (serial_number, bmc_mac_address, bmc_username, \ + bmc_password, bmc_vendor_override) VALUES ('PINCACHE1', $1::macaddr, 'root', 'p', $2)", + ) + .bind(mac) + .bind(pin) + .execute(pool) + .await + .expect("expected machine"); + } + + /// A BMC reached by hostname has no address row to match, so it reports no + /// pin rather than failing the client construction that asked. + #[crate::sqlx_test] + async fn a_hostname_addressed_bmc_has_no_pin( + pool: sqlx::PgPool, + ) -> Result<(), Box> { + let resolver = DbBmcVendorOverrideResolver::new(pool); + assert_eq!(resolver.vendor_override("bmc-01.example.test").await?, None); + Ok(()) + } + + /// The cache is what makes a lookup per `create_client` affordable, so assert + /// it caches, including the far more common answer of no pin at all. + #[crate::sqlx_test] + async fn resolved_pins_and_their_absence_are_both_cached( + pool: sqlx::PgPool, + ) -> Result<(), Box> { + let ip: IpAddr = "192.0.2.77".parse()?; + + // Nothing seeded yet, so both resolvers observe no pin. + let cached = DbBmcVendorOverrideResolver::new(pool.clone()); + let uncached = DbBmcVendorOverrideResolver::with_cache_ttl(pool.clone(), Duration::ZERO); + assert_eq!(cached.vendor_override(&ip.to_string()).await?, None); + assert_eq!(uncached.vendor_override(&ip.to_string()).await?, None); + + seed_pinned_bmc(&pool, ip, "7A:7B:7C:7D:7E:77", "Dell").await; + + assert_eq!( + cached.vendor_override(&ip.to_string()).await?, + None, + "the negative answer must still come from cache, the property that \ + keeps this off the hot path" + ); + assert_eq!( + uncached.vendor_override(&ip.to_string()).await?, + Some(RedfishVendor::Dell), + "a resolver whose entries have expired must see the new pin" + ); + + Ok(()) + } +} diff --git a/crates/api-core/src/setup.rs b/crates/api-core/src/setup.rs index 8c23c63477..c573439c23 100644 --- a/crates/api-core/src/setup.rs +++ b/crates/api-core/src/setup.rs @@ -108,6 +108,7 @@ use crate::mqtt_state_change_hook::hook::MqttStateChangeHook; use crate::mqtt_state_change_hook::republisher::{ ManagedHostStateRepublisher, ManagedHostStateRepublisherParams, }; +use crate::redfish_vendor_override::DbBmcVendorOverrideResolver; use crate::scout_stream::ConnectionRegistry; use crate::{CarbideError, attestation, db_init, ethernet_virtualization, listener}; @@ -135,6 +136,7 @@ pub fn create_ipmi_tool( fn create_redfish_pool( carbide_config: &CarbideConfig, credential_manager: Arc, + db_pool: PgPool, ) -> eyre::Result> { let pool = libredfish::RedfishClientPool::builder() .danger_accept_invalid_certs() @@ -183,6 +185,9 @@ fn create_redfish_pool( credential_manager, pool, carbide_config.site_explorer.bmc_proxy.clone(), + // Supplies the per BMC operator vendor pin to every client this pool + // builds, so no call site has to resolve it itself. + Arc::new(DbBmcVendorOverrideResolver::new(db_pool)), )) } @@ -203,7 +208,8 @@ pub(crate) async fn start_runtime( cancel_token: CancellationToken, ready_channel: Sender<()>, ) -> eyre::Result<()> { - let shared_redfish_pool = create_redfish_pool(&carbide_config, credential_manager.clone())?; + let shared_redfish_pool = + create_redfish_pool(&carbide_config, credential_manager.clone(), db_pool.clone())?; let shared_nv_redfish_pool = carbide_redfish::nv_redfish::new_pool(carbide_config.site_explorer.bmc_proxy.clone()); diff --git a/crates/api-core/src/tests/expected_machine.rs b/crates/api-core/src/tests/expected_machine.rs index 663c381ad7..040767a049 100644 --- a/crates/api-core/src/tests/expected_machine.rs +++ b/crates/api-core/src/tests/expected_machine.rs @@ -638,6 +638,7 @@ async fn test_add_expected_machine_dpu_serials(pool: sqlx::PgPool) { dpu_mode: None, bmc_ip_allocation: None, host_lifecycle_profile: None, + bmc_vendor_override: None, #[allow(deprecated)] dpf_enabled: true, }; diff --git a/crates/api-core/src/tests/sku.rs b/crates/api-core/src/tests/sku.rs index 53a5c92658..a70be6cf8b 100644 --- a/crates/api-core/src/tests/sku.rs +++ b/crates/api-core/src/tests/sku.rs @@ -837,6 +837,7 @@ pub mod tests { dpu_policy: Default::default(), bmc_ip_allocation: Default::default(), host_lifecycle_profile: Default::default(), + bmc_vendor_override: None, }, }, ) @@ -933,6 +934,7 @@ pub mod tests { dpu_policy: Default::default(), bmc_ip_allocation: Default::default(), host_lifecycle_profile: Default::default(), + bmc_vendor_override: None, }, }, ) @@ -1004,6 +1006,7 @@ pub mod tests { dpu_policy: Default::default(), bmc_ip_allocation: Default::default(), host_lifecycle_profile: Default::default(), + bmc_vendor_override: None, }, }, ) @@ -1092,6 +1095,7 @@ pub mod tests { dpu_policy: Default::default(), bmc_ip_allocation: Default::default(), host_lifecycle_profile: Default::default(), + bmc_vendor_override: None, }, }, ) @@ -1537,6 +1541,7 @@ pub mod tests { dpu_policy: Default::default(), bmc_ip_allocation: Default::default(), host_lifecycle_profile: Default::default(), + bmc_vendor_override: None, }, }, ) diff --git a/crates/api-db/migrations/20260727120000_expected_machine_bmc_vendor_override.sql b/crates/api-db/migrations/20260727120000_expected_machine_bmc_vendor_override.sql new file mode 100644 index 0000000000..9419f5ed23 --- /dev/null +++ b/crates/api-db/migrations/20260727120000_expected_machine_bmc_vendor_override.sql @@ -0,0 +1,4 @@ +-- Add bmc_vendor_override to expected_machines so an operator can pin the +-- Redfish BMC vendor for a host. NULL means automatic detection. The value is a +-- RedfishVendor variant name, forced into libredfish when a client is built. +ALTER TABLE expected_machines ADD COLUMN bmc_vendor_override text; diff --git a/crates/api-db/src/expected_machine.rs b/crates/api-db/src/expected_machine.rs index 38bedf03c3..0399b3fe65 100644 --- a/crates/api-db/src/expected_machine.rs +++ b/crates/api-db/src/expected_machine.rs @@ -263,9 +263,9 @@ pub async fn create( ) -> DatabaseResult { let id = machine.id.unwrap_or_else(Uuid::new_v4); let query = "INSERT INTO expected_machines - (id, bmc_mac_address, bmc_username, bmc_password, serial_number, fallback_dpu_serial_numbers, metadata_name, metadata_description, metadata_labels, sku_id, host_nics, rack_id, default_pause_ingestion_and_poweron, dpf_enabled, bmc_ip_address, bmc_retain_credentials, dpu_mode, bmc_ip_allocation, host_lifecycle_profile) + (id, bmc_mac_address, bmc_username, bmc_password, serial_number, fallback_dpu_serial_numbers, metadata_name, metadata_description, metadata_labels, sku_id, host_nics, rack_id, default_pause_ingestion_and_poweron, dpf_enabled, bmc_ip_address, bmc_retain_credentials, dpu_mode, bmc_ip_allocation, host_lifecycle_profile, bmc_vendor_override) VALUES - ($1::uuid, $2::macaddr, $3::varchar, $4::varchar, $5::varchar, $6::text[], $7, $8, $9::jsonb, $10::varchar, $11::jsonb, $12, $13, $14, $15::inet, $16, $17, $18, $19::jsonb) RETURNING *"; + ($1::uuid, $2::macaddr, $3::varchar, $4::varchar, $5::varchar, $6::text[], $7, $8, $9::jsonb, $10::varchar, $11::jsonb, $12, $13, $14, $15::inet, $16, $17, $18, $19::jsonb, $20::text) RETURNING *"; sqlx::query_as(query) .bind(id) @@ -292,6 +292,7 @@ pub async fn create( .bind(machine.data.dpu_policy) .bind(machine.data.bmc_ip_allocation) .bind(sqlx::types::Json(&machine.data.host_lifecycle_profile)) + .bind(&machine.data.bmc_vendor_override) .fetch_one(txn) .await .map_err(|err: sqlx::Error| match err { @@ -399,7 +400,8 @@ pub async fn update(txn: &mut PgConnection, machine: &ExpectedMachine) -> Databa bmc_retain_credentials=COALESCE($14, bmc_retain_credentials), \ dpu_mode=$15, \ bmc_ip_allocation=$16, \ - host_lifecycle_profile=COALESCE($17, host_lifecycle_profile) \ + host_lifecycle_profile=COALESCE($17, host_lifecycle_profile), \ + bmc_vendor_override=$18 \ WHERE ", $where_clause, ) @@ -408,11 +410,11 @@ pub async fn update(txn: &mut PgConnection, machine: &ExpectedMachine) -> Databa let (query, target_id) = match machine.id { Some(id) => ( - update_expected_machine_query!("id=$18::uuid"), + update_expected_machine_query!("id=$19::uuid"), id.to_string(), ), None => ( - update_expected_machine_query!("bmc_mac_address=$18::macaddr"), + update_expected_machine_query!("bmc_mac_address=$19::macaddr"), machine.bmc_mac_address.to_string(), ), }; @@ -438,6 +440,7 @@ pub async fn update(txn: &mut PgConnection, machine: &ExpectedMachine) -> Databa (!machine.data.host_lifecycle_profile.is_empty()) .then_some(sqlx::types::Json(&machine.data.host_lifecycle_profile)), ) + .bind(&machine.data.bmc_vendor_override) .bind(&target_id) .execute(&mut *txn) .await diff --git a/crates/api-db/src/expected_machine/tests.rs b/crates/api-db/src/expected_machine/tests.rs index 8dc6988638..f2ae461d0d 100644 --- a/crates/api-db/src/expected_machine/tests.rs +++ b/crates/api-db/src/expected_machine/tests.rs @@ -154,6 +154,7 @@ async fn test_duplicate_fail_create(pool: sqlx::PgPool) -> Result<(), Box, ) -> DatabaseResult { + // The operator vendor pin is deliberately not read here. It resolves inside + // `create_client`, which every BMC client goes through, so a copy here would + // be a second source of truth able to disagree with what a client used. let mac_address = find_by_ip(db, ip) .await? .ok_or_else(|| DatabaseError::NotFoundError { @@ -448,6 +451,30 @@ pub async fn lookup_bmc_access_info( }) } +/// Resolve the operator pinned Redfish vendor for the BMC reachable at `ip`. +/// +/// The pin is keyed by BMC MAC while consumers hold a BMC address, so this +/// bridges the two. Having no pin is the normal case, never an error. +pub async fn find_bmc_vendor_override_by_ip( + db: impl DbReader<'_>, + ip: IpAddr, +) -> DatabaseResult> { + let query = r"SELECT em.bmc_vendor_override + FROM machine_interface_addresses mia + INNER JOIN machine_interfaces mi ON mi.id = mia.interface_id + INNER JOIN expected_machines em ON em.bmc_mac_address = mi.mac_address + WHERE mia.address = $1::inet + AND mi.interface_type = 'Bmc' + AND em.bmc_vendor_override IS NOT NULL + LIMIT 1"; + sqlx::query_scalar(query) + .bind(ip) + .fetch_optional(db) + .await + .map(Option::flatten) + .map_err(|e| DatabaseError::query(query, e)) +} + pub async fn find_by_ip( txn: impl DbReader<'_>, ip: IpAddr, diff --git a/crates/api-db/src/machine_interface/tests.rs b/crates/api-db/src/machine_interface/tests.rs index 754cb7d57a..271c55ea77 100644 --- a/crates/api-db/src/machine_interface/tests.rs +++ b/crates/api-db/src/machine_interface/tests.rs @@ -401,3 +401,138 @@ async fn test_retain_bmc_address_pins_dhcp_and_survives_expiry( Ok(()) } + +/// The pin is written on the expected machine and keyed by BMC MAC, while every +/// consumer builds clients from a BMC address, so this query bridges the two. If +/// the join matched nothing every pin would silently do nothing and no other test +/// would fail, so assert the value arrives and that each empty shape is `None`. +#[crate::sqlx_test] +async fn find_bmc_vendor_override_by_ip_bridges_mac_keyed_pin_to_bmc_address( + pool: sqlx::PgPool, +) -> Result<(), Box> { + let segment = create_test_segment(&pool, "bmc-vendor-override-segment").await?; + + // Mac, ip, hostname and the pin, where `None` means no expected machine row. + let fixtures: [(MacAddress, IpAddr, &str, Option<&str>); 5] = [ + ( + "7A:7B:7C:7D:7E:51".parse()?, + "192.0.2.51".parse()?, + "pinned", + Some("Dell"), + ), + ( + "7A:7B:7C:7D:7E:52".parse()?, + "192.0.2.52".parse()?, + "no-machine", + None, + ), + ( + "7A:7B:7C:7D:7E:53".parse()?, + "192.0.2.53".parse()?, + "null-pin", + Some(""), + ), + ( + "7A:7B:7C:7D:7E:54".parse()?, + "192.0.2.54".parse()?, + "bogus-pin", + Some("NotAVendor"), + ), + ( + "7A:7B:7C:7D:7E:55".parse()?, + "2001:db8::55".parse()?, + "v6-pinned", + Some("NvidiaDpu"), + ), + ]; + + let mut txn = db::Transaction::begin(&pool).await?; + for (index, (mac, ip, hostname, pin)) in fixtures.iter().enumerate() { + let interface_id: uuid::Uuid = sqlx::query_scalar( + "INSERT INTO machine_interfaces + (segment_id, mac_address, primary_interface, hostname, interface_type) + VALUES ($1, $2, false, $3, 'Bmc') RETURNING id", + ) + .bind(segment) + .bind(mac) + .bind(hostname) + .fetch_one(txn.as_pgconn()) + .await?; + + sqlx::query( + "INSERT INTO machine_interface_addresses (interface_id, address) VALUES ($1, $2::inet)", + ) + .bind(interface_id) + .bind(ip) + .execute(txn.as_pgconn()) + .await?; + + if let Some(pin) = pin { + // An empty string stores as SQL NULL, covering the unset shape too. + sqlx::query( + "INSERT INTO expected_machines + (serial_number, bmc_mac_address, bmc_username, bmc_password, bmc_vendor_override) + VALUES ($1, $2, 'root', 'pass', NULLIF($3, ''))", + ) + .bind(format!("VVG121G{index}")) + .bind(mac) + .bind(pin) + .execute(txn.as_pgconn()) + .await?; + } + } + + for (_, ip, hostname, expected) in fixtures.iter() { + // The query hands back the raw string. Parsing and rejecting unusable + // names happens a layer up, so a bogus name survives this one intact. + let expected = expected.filter(|pin| !pin.is_empty()).map(str::to_string); + assert_eq!( + find_bmc_vendor_override_by_ip(txn.as_pgconn(), *ip).await?, + expected, + "pin lookup for {hostname} at {ip}" + ); + } + + // An address with no interface row at all has no pin, and is not an error. + assert_eq!( + find_bmc_vendor_override_by_ip(txn.as_pgconn(), "192.0.2.99".parse()?).await?, + None, + "an unknown BMC address simply has no pin" + ); + + // A pin belongs to a BMC, so a data interface sharing an expected machine MAC + // must not hand its pin to whatever else answers on that address. + let data_mac: MacAddress = "7A:7B:7C:7D:7E:56".parse()?; + let data_ip: IpAddr = "192.0.2.56".parse()?; + let data_interface: uuid::Uuid = sqlx::query_scalar( + "INSERT INTO machine_interfaces + (segment_id, mac_address, primary_interface, hostname, interface_type) + VALUES ($1, $2, false, 'data-nic', 'Data') RETURNING id", + ) + .bind(segment) + .bind(data_mac) + .fetch_one(txn.as_pgconn()) + .await?; + sqlx::query( + "INSERT INTO machine_interface_addresses (interface_id, address) VALUES ($1, $2::inet)", + ) + .bind(data_interface) + .bind(data_ip) + .execute(txn.as_pgconn()) + .await?; + sqlx::query( + "INSERT INTO expected_machines + (serial_number, bmc_mac_address, bmc_username, bmc_password, bmc_vendor_override) + VALUES ('VVG121GZ', $1, 'root', 'pass', 'Dell')", + ) + .bind(data_mac) + .execute(txn.as_pgconn()) + .await?; + assert_eq!( + find_bmc_vendor_override_by_ip(txn.as_pgconn(), data_ip).await?, + None, + "a non BMC interface must never resolve a pin" + ); + + Ok(()) +} diff --git a/crates/api-model/src/expected_machine.rs b/crates/api-model/src/expected_machine.rs index 6852ace1a1..4592ff90ad 100644 --- a/crates/api-model/src/expected_machine.rs +++ b/crates/api-model/src/expected_machine.rs @@ -262,6 +262,12 @@ pub struct ExpectedMachineData { /// knobs should be added here rather than as new flat columns. #[serde(default)] pub host_lifecycle_profile: HostLifecycleProfile, + /// Operator pinned Redfish BMC vendor for this host, a `RedfishVendor` variant + /// name such as `Dell`, forced into libredfish instead of detection. Absent or + /// empty means automatic detection. NICo keeps no vendor list of its own and + /// lets libredfish match the name. + #[serde(default)] + pub bmc_vendor_override: Option, } // Important : new fields for expected machine (and data) should be optional _and_ serde(default), // unless you want to go update all the files in each production deployment that autoload @@ -338,6 +344,7 @@ impl<'r> FromRow<'r, PgRow> for ExpectedMachine { host_lifecycle_profile: row .try_get::, _>("host_lifecycle_profile") .map(|j| j.0)?, + bmc_vendor_override: row.try_get("bmc_vendor_override")?, }, }) } diff --git a/crates/api-model/src/site_explorer/mod.rs b/crates/api-model/src/site_explorer/mod.rs index 0ea4b8030c..bdd60d39be 100644 --- a/crates/api-model/src/site_explorer/mod.rs +++ b/crates/api-model/src/site_explorer/mod.rs @@ -64,7 +64,10 @@ pub struct EndpointExplorationReport { /// The time it took to explore the endpoint in the last site explorer run #[serde(default, skip_serializing_if = "Option::is_none")] pub last_exploration_latency: Option, - /// Vendor as reported by Redfish + /// The BMC vendor, either what Redfish reported or the operator pin. + /// + /// The value the rest of NICo acts on, selecting the firmware key, the restart + /// path and the console transport, so a pin has to win here too. #[serde(default, skip_serializing_if = "Option::is_none")] pub vendor: Option, /// `Managers` reported by Redfish diff --git a/crates/bmc-explorer-cli/src/main.rs b/crates/bmc-explorer-cli/src/main.rs index 437a723d13..9f6f34978b 100644 --- a/crates/bmc-explorer-cli/src/main.rs +++ b/crates/bmc-explorer-cli/src/main.rs @@ -90,6 +90,9 @@ async fn main() -> Result<(), Box> { credential_provider.clone(), rf_pool, proxy_address.clone(), + // This CLI explores a BMC straight from its arguments and has no expected + // machine store to read a vendor pin from, so it opts out. + Arc::new(carbide_redfish::libredfish::NoBmcVendorOverrides), ); let mode = match args.mode.as_str() { "libredfish" => SiteExplorerExploreMode::LibRedfish, diff --git a/crates/component-manager/src/core_compute_manager.rs b/crates/component-manager/src/core_compute_manager.rs index ec857c10dd..50add99e23 100644 --- a/crates/component-manager/src/core_compute_manager.rs +++ b/crates/component-manager/src/core_compute_manager.rs @@ -3,7 +3,7 @@ use std::sync::Arc; -use carbide_redfish::libredfish::RedfishClientPool; +use carbide_redfish::libredfish::{RedfishClientPool, VendorSelection}; use carbide_secrets::credentials::Credentials; use model::component_manager::{ComputeTrayComponent, PowerAction}; @@ -31,14 +31,18 @@ impl CoreComputeTrayManager { } } -fn map_vendor(vendor: ComputeTrayVendor) -> Option { +/// Translate the tray recorded vendor into a client construction request. +/// +/// The value comes from DMI, so it is a hint an operator pin outranks. A tray +/// whose vendor cannot be named falls back to detection. +fn map_vendor(vendor: ComputeTrayVendor) -> VendorSelection { use libredfish::model::service_root::RedfishVendor; match vendor { - ComputeTrayVendor::Dell => Some(RedfishVendor::Dell), - ComputeTrayVendor::Hpe => Some(RedfishVendor::Hpe), - ComputeTrayVendor::Lenovo => Some(RedfishVendor::Lenovo), - ComputeTrayVendor::Supermicro => Some(RedfishVendor::Supermicro), - ComputeTrayVendor::Nvidia | ComputeTrayVendor::Unknown => None, + ComputeTrayVendor::Dell => VendorSelection::Hint(RedfishVendor::Dell), + ComputeTrayVendor::Hpe => VendorSelection::Hint(RedfishVendor::Hpe), + ComputeTrayVendor::Lenovo => VendorSelection::Hint(RedfishVendor::Lenovo), + ComputeTrayVendor::Supermicro => VendorSelection::Hint(RedfishVendor::Supermicro), + ComputeTrayVendor::Nvidia | ComputeTrayVendor::Unknown => VendorSelection::Detect, } } @@ -145,16 +149,16 @@ mod tests { #[test] fn compute_tray_vendor_maps_to_redfish_vendor() { value_scenarios!(map_vendor: - "supported Redfish vendors" { - ComputeTrayVendor::Dell => Some(RedfishVendor::Dell), - ComputeTrayVendor::Hpe => Some(RedfishVendor::Hpe), - ComputeTrayVendor::Lenovo => Some(RedfishVendor::Lenovo), - ComputeTrayVendor::Supermicro => Some(RedfishVendor::Supermicro), + "supported Redfish vendors become a hint an operator pin can outrank" { + ComputeTrayVendor::Dell => VendorSelection::Hint(RedfishVendor::Dell), + ComputeTrayVendor::Hpe => VendorSelection::Hint(RedfishVendor::Hpe), + ComputeTrayVendor::Lenovo => VendorSelection::Hint(RedfishVendor::Lenovo), + ComputeTrayVendor::Supermicro => VendorSelection::Hint(RedfishVendor::Supermicro), } - "vendors without a Redfish adapter" { - ComputeTrayVendor::Nvidia => None, - ComputeTrayVendor::Unknown => None, + "vendors without a Redfish adapter fall back to detection" { + ComputeTrayVendor::Nvidia => VendorSelection::Detect, + ComputeTrayVendor::Unknown => VendorSelection::Detect, } ); } diff --git a/crates/machine-a-tron/src/api_client.rs b/crates/machine-a-tron/src/api_client.rs index d30c64561b..443dfa6239 100644 --- a/crates/machine-a-tron/src/api_client.rs +++ b/crates/machine-a-tron/src/api_client.rs @@ -525,6 +525,7 @@ impl ApiClient { dpu_mode: dpu_policy.map(|policy| rpc::forge::DpuMode::from(policy) as i32), bmc_ip_allocation: None, host_lifecycle_profile: None, + bmc_vendor_override: None, }) .await .map_err(ClientApiError::InvocationError) diff --git a/crates/machine-controller/src/handler/test_machine_setup.rs b/crates/machine-controller/src/handler/test_machine_setup.rs index dcc25d7613..9a7a57775b 100644 --- a/crates/machine-controller/src/handler/test_machine_setup.rs +++ b/crates/machine-controller/src/handler/test_machine_setup.rs @@ -18,7 +18,7 @@ use std::collections::HashMap; use carbide_redfish::libredfish::test_support::{RedfishSim, RedfishSimAction}; -use carbide_redfish::libredfish::{RedfishAuth, RedfishClientPool}; +use carbide_redfish::libredfish::{RedfishAuth, RedfishClientPool, VendorSelection}; use carbide_secrets::credentials::{CredentialKey, CredentialType}; use libredfish::BiosProfileType; use libredfish::model::service_root::RedfishVendor; @@ -57,7 +57,7 @@ async fn test_oem_manager_profiles_passed_to_machine_setup() { RedfishAuth::Key(CredentialKey::HostRedfish { credential_type: CredentialType::SiteDefault, }), - None, + VendorSelection::Detect, ) .await .unwrap(); diff --git a/crates/redfish/src/libredfish/conv.rs b/crates/redfish/src/libredfish/conv.rs index 4377bffdbc..9ef414c30c 100644 --- a/crates/redfish/src/libredfish/conv.rs +++ b/crates/redfish/src/libredfish/conv.rs @@ -196,6 +196,39 @@ impl IntoModel for libredfish::model::component_integrity::Evidence { } } +/// Parse a vendor name into libredfish's `RedfishVendor` via its own +/// `Deserialize` impl, so NICo keeps no vendor list of its own. Returns `None` +/// when the name matches no variant. +pub fn redfish_vendor_from_str( + name: &str, +) -> Option { + use serde::Deserialize; + let de = serde::de::value::StrDeserializer::::new(name); + libredfish::model::service_root::RedfishVendor::deserialize(de).ok() +} + +/// Resolve an operator stored override string into a forced `RedfishVendor`. +/// +/// Absent, empty, or unmatched means no override, warning under `host`. `Unknown` +/// is refused too, since it asks the pool for an uninitialized client. +pub fn redfish_vendor_override( + host: &str, + raw: Option<&str>, +) -> Option { + use libredfish::model::service_root::RedfishVendor; + + let name = raw.filter(|s| !s.is_empty())?; + let vendor = redfish_vendor_from_str(name).filter(|vendor| *vendor != RedfishVendor::Unknown); + if vendor.is_none() { + tracing::warn!( + bmc = %host, + bmc_vendor_override = %name, + "bmc_vendor_override matches no usable Redfish vendor, using automatic detection" + ); + } + vendor +} + #[cfg(test)] mod tests { use carbide_test_support::value_scenarios; @@ -203,6 +236,50 @@ mod tests { use super::*; + #[test] + fn parses_known_variant_names() { + assert_eq!(redfish_vendor_from_str("Dell"), Some(RedfishVendor::Dell)); + assert_eq!( + redfish_vendor_from_str("NvidiaDpu"), + Some(RedfishVendor::NvidiaDpu) + ); + } + + #[test] + fn rejects_unknown_or_miscased_names() { + assert_eq!(redfish_vendor_from_str("dell"), None); + assert_eq!(redfish_vendor_from_str("NotARealVendor"), None); + assert_eq!(redfish_vendor_from_str(""), None); + } + + #[test] + fn override_helper_handles_empty_and_unmatched() { + assert_eq!(redfish_vendor_override("bmc", None), None); + assert_eq!(redfish_vendor_override("bmc", Some("")), None); + assert_eq!( + redfish_vendor_override("bmc", Some("Dell")), + Some(RedfishVendor::Dell) + ); + assert_eq!(redfish_vendor_override("bmc", Some("bogus")), None); + } + + /// `Unknown` is a real variant so the parser matches it, but it is the client + /// pool sentinel for an uninitialized client rather than a pinnable vendor, so + /// the override resolver has to drop it and let detection run. + #[test] + fn override_helper_rejects_unknown_sentinel() { + assert_eq!( + redfish_vendor_from_str("Unknown"), + Some(RedfishVendor::Unknown), + "the parser reports whichever variant the name matched" + ); + assert_eq!( + redfish_vendor_override("bmc", Some("Unknown")), + None, + "the override resolver falls back to automatic detection" + ); + } + #[test] fn redfish_vendors_map_to_bmc_vendors() { value_scenarios!(bmc_vendor: diff --git a/crates/redfish/src/libredfish/implementation.rs b/crates/redfish/src/libredfish/implementation.rs index 12cca3ac5d..25df9dd6c1 100644 --- a/crates/redfish/src/libredfish/implementation.rs +++ b/crates/redfish/src/libredfish/implementation.rs @@ -30,7 +30,11 @@ use libredfish::model::service_root::RedfishVendor; use libredfish::{Endpoint, Redfish}; use crate::libredfish::instrumented::{InstrumentedRedfish, REDFISH_BACKEND}; -use crate::libredfish::{RedfishAuth, RedfishClientCreationError, RedfishClientPool}; +use crate::libredfish::vendor_selection::ClientMode; +use crate::libredfish::{ + BmcVendorOverrideResolver, RedfishAuth, RedfishClientCreationError, RedfishClientPool, + VendorSelection, +}; /// Formats a host for the URL authority that `libredfish` constructs internally. /// @@ -49,6 +53,7 @@ pub struct RedfishClientPoolImpl { pool: libredfish::RedfishClientPool, credential_reader: Arc, proxy_address: Arc>>, + vendor_override_resolver: Arc, } impl RedfishClientPoolImpl { @@ -56,11 +61,31 @@ impl RedfishClientPoolImpl { credential_reader: Arc, pool: libredfish::RedfishClientPool, proxy_address: Arc>>, + vendor_override_resolver: Arc, ) -> Self { RedfishClientPoolImpl { credential_reader, pool, proxy_address, + vendor_override_resolver, + } + } + + /// The operator pinned vendor for `host`, or `None` for detection. + /// + /// The single place a resolver failure is handled, and deliberately has no + /// error variant a later refactor could turn into a hard failure. + async fn pinned_vendor(&self, host: &str) -> Option { + match self.vendor_override_resolver.vendor_override(host).await { + Ok(vendor) => vendor, + Err(error) => { + tracing::debug!( + bmc = %host, + %error, + "bmc_vendor_override unresolved, using automatic detection" + ); + None + } } } } @@ -72,7 +97,7 @@ impl RedfishClientPool for RedfishClientPoolImpl { host: &str, port: Option, auth: RedfishAuth, - vendor: Option, + vendor: VendorSelection, ) -> Result, RedfishClientCreationError> { let original_host = host; @@ -134,11 +159,19 @@ impl RedfishClientPool for RedfishClientPoolImpl { Vec::default() }; + // Resolve the operator pin only for the modes it can apply to. The + // uninitialized mode is matched first and returns before any lookup + // happens, because forcing a vendor there breaks factory BMC bootstrap. + let mode = match vendor { + VendorSelection::Uninitialized => ClientMode::Uninitialized, + selection => selection.resolve(self.pinned_vendor(original_host).await), + }; + // The initializing paths below make HTTP calls of their own, so they // are metered like any other Redfish operation. - let client = match vendor { + let client = match mode { // Auto-detect vendor from the service root. - None => red::instrumented( + ClientMode::Detect => red::instrumented( REDFISH_BACKEND, "create_client", self.pool @@ -146,20 +179,17 @@ impl RedfishClientPool for RedfishClientPoolImpl { ) .await .map_err(RedfishClientCreationError::RedfishError)?, - // Unknown means "no vendor" — return a standard client without - // making any HTTP calls (used by the anonymous probe client). - // This restores the behavior of the old `initialize: false` path - // which called create_standard_client. The full initialization - // path (create_client_with_vendor) makes HTTP calls to /Systems, - // /Managers, etc. that fail with 401 on BMCs requiring auth. - // With no I/O here, there is no external call to meter either. - Some(RedfishVendor::Unknown) => self + // No vendor, so return a standard client without making any HTTP + // calls, as the anonymous probe client needs. Full initialization + // fetches /Systems and /Managers, which answer 401 on a BMC that + // requires auth. With no I/O there is nothing to meter either. + ClientMode::Uninitialized => self .pool .create_standard_client_with_custom_headers(endpoint, custom_headers) .map_err(RedfishClientCreationError::RedfishError) .map(|c| c as Box)?, - // Use the provided vendor directly. - Some(vendor) => red::instrumented( + // Use the resolved vendor directly. + ClientMode::Vendor(vendor) => red::instrumented( REDFISH_BACKEND, "create_client", self.pool @@ -177,6 +207,10 @@ impl RedfishClientPool for RedfishClientPoolImpl { fn credential_reader(&self) -> &dyn CredentialReader { &*self.credential_reader } + + async fn pinned_bmc_vendor(&self, host: &str) -> Option { + self.pinned_vendor(host).await + } } #[cfg(test)] diff --git a/crates/redfish/src/libredfish/instrumented.rs b/crates/redfish/src/libredfish/instrumented.rs index 1939dcfa59..f85fa44591 100644 --- a/crates/redfish/src/libredfish/instrumented.rs +++ b/crates/redfish/src/libredfish/instrumented.rs @@ -424,7 +424,7 @@ mod tests { use super::*; use crate::libredfish::test_support::RedfishSim; - use crate::libredfish::{RedfishAuth, RedfishClientPool}; + use crate::libredfish::{RedfishAuth, RedfishClientPool, VendorSelection}; async fn sim_client(sim: &RedfishSim) -> InstrumentedRedfish { let client = sim @@ -434,7 +434,7 @@ mod tests { RedfishAuth::Key(CredentialKey::HostRedfish { credential_type: CredentialType::SiteDefault, }), - None, + VendorSelection::Detect, ) .await .expect("sim client"); diff --git a/crates/redfish/src/libredfish/mod.rs b/crates/redfish/src/libredfish/mod.rs index 5b25e8d7a0..449dbf592c 100644 --- a/crates/redfish/src/libredfish/mod.rs +++ b/crates/redfish/src/libredfish/mod.rs @@ -24,6 +24,8 @@ pub mod dpu_bios; pub mod error; #[cfg(feature = "test-support")] pub mod test_support; +pub mod vendor_override; +pub mod vendor_selection; use std::net::SocketAddr; use std::sync::Arc; @@ -38,6 +40,8 @@ use carbide_utils::redfish::BmcAccessInfo; pub use error::RedfishClientCreationError; use libredfish::Redfish; use libredfish::model::service_root::RedfishVendor; +pub use vendor_override::{BmcVendorOverrideResolver, NoBmcVendorOverrides, VendorOverrideError}; +pub use vendor_selection::VendorSelection; #[derive(Debug, Clone, Copy, PartialEq, Eq, LabelValue)] enum DpuUefiPasswordSetupSkipReason { @@ -99,15 +103,21 @@ fn emit_dpu_uefi_password_setup_skipped_if_needed( true } +/// Builds the production client pool. +/// +/// `vendor_override_resolver` supplies the per BMC operator vendor pin. Pass +/// [`NoBmcVendorOverrides`] to opt out explicitly. pub fn new_pool( credential_reader: Arc, pool: libredfish::RedfishClientPool, proxy_address: Arc>>, + vendor_override_resolver: Arc, ) -> Arc { Arc::new(implementation::RedfishClientPoolImpl::new( credential_reader, pool, proxy_address, + vendor_override_resolver, )) } @@ -117,21 +127,26 @@ pub trait RedfishClientPool: Send + Sync + 'static { // MARK: - Required methods /// Creates a new Redfish client for a Machines BMC. - /// `host` is the IP address or hostname of the BMC. - /// `vendor` allows you to pre-assign the underlying - /// RedfishVendor to use for the client, saving the - /// service root call to auto-detect the vendor. + /// + /// Every BMC client is built here, which is what lets an operator vendor pin + /// apply everywhere without each call site resolving it. async fn create_client( &self, host: &str, port: Option, auth: RedfishAuth, - vendor: Option, + vendor: VendorSelection, ) -> Result, RedfishClientCreationError>; /// Returns a CredentialReader for use in setting credentials in the UEFI/BMC. fn credential_reader(&self) -> &dyn CredentialReader; + /// The operator pinned vendor for this BMC, or `None` for detection. + /// + /// For callers needing the pin as a value rather than a client. They read it + /// here so none can disagree with the vendor `create_client` applied. + async fn pinned_bmc_vendor(&self, host: &str) -> Option; + // MARK: - Default (helper) methods async fn probe_redfish_endpoint( @@ -143,7 +158,7 @@ pub trait RedfishClientPool: Send + Sync + 'static { &bmc_ip_address.ip().to_string(), Some(bmc_ip_address.port()), RedfishAuth::Anonymous, - Some(RedfishVendor::Unknown), + VendorSelection::Uninitialized, ) .await?; @@ -155,6 +170,10 @@ pub trait RedfishClientPool: Send + Sync + 'static { Ok(()) } + /// Builds a client for a BMC described by a [`BmcAccessInfo`]. + /// + /// Asks for detection rather than resolving the pin here. The pin is applied + /// inside `create_client`, so this path and every other one share it. async fn client_by_info( &self, access: &BmcAccessInfo, @@ -163,7 +182,7 @@ pub trait RedfishClientPool: Send + Sync + 'static { &access.host, access.port, RedfishAuth::for_bmc_mac(access.mac_address), - None, + VendorSelection::Detect, ) .await } @@ -327,7 +346,7 @@ pub trait RedfishClientPool: Send + Sync + 'static { host, port, RedfishAuth::Direct(curr_user.clone(), curr_password.clone()), - Some(RedfishVendor::Unknown), + VendorSelection::Uninitialized, ) .await?; @@ -417,7 +436,7 @@ pub trait RedfishClientPool: Send + Sync + 'static { host, port, RedfishAuth::Direct(curr_user.to_string(), new_password), - Some(vendor), + VendorSelection::Hint(vendor), ) .await?; @@ -446,7 +465,7 @@ pub trait RedfishClientPool: Send + Sync + 'static { host, port, RedfishAuth::Direct(root_user.clone(), root_password.clone()), - Some(RedfishVendor::Unknown), + VendorSelection::Uninitialized, ) .await?; @@ -496,7 +515,7 @@ pub trait RedfishClientPool: Send + Sync + 'static { host, port, RedfishAuth::Direct(username, password), - Some(RedfishVendor::Unknown), + VendorSelection::Uninitialized, ) .await?; @@ -532,7 +551,7 @@ pub trait RedfishClientPool: Send + Sync + 'static { host, port, RedfishAuth::Anonymous, - Some(RedfishVendor::Unknown), + VendorSelection::Uninitialized, ) .await?; @@ -547,7 +566,12 @@ pub trait RedfishClientPool: Send + Sync + 'static { // shelves) that don't expose a recognized vendor in the service root. let Credentials::UsernamePassword { username, password } = credentials; let client = self - .create_client(host, port, RedfishAuth::Direct(username, password), None) + .create_client( + host, + port, + RedfishAuth::Direct(username, password), + VendorSelection::Detect, + ) .await?; let chassis_ids = client @@ -882,7 +906,7 @@ mod tests { RedfishAuth::Key(CredentialKey::HostRedfish { credential_type: CredentialType::SiteDefault, }), - None, + VendorSelection::Detect, ) .await .unwrap(); @@ -901,7 +925,7 @@ mod tests { RedfishAuth::Key(CredentialKey::HostRedfish { credential_type: CredentialType::SiteDefault, }), - None, + VendorSelection::Detect, ) .await .unwrap(); @@ -950,6 +974,68 @@ mod tests { .collect() } + /// `client_by_info` is the shared entry point for BMC clients, so the pin has + /// to arrive at `create_client` as the forced vendor. It asks for detection + /// and the pool applies the pin, covering every other detecting call site. + #[tokio::test] + async fn client_by_info_applies_the_operator_pin() { + const HOST: &str = "127.0.0.1"; + + async fn vendor_passed_for(pin: Option) -> Option { + let sim = RedfishSim::default(); + if let Some(pin) = pin { + sim.set_vendor_override(HOST, pin); + } + let access = BmcAccessInfo { + host: HOST.to_string(), + port: Some(443), + mac_address: mac_address::MacAddress::new([0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0x01]), + }; + + sim.client_by_info(&access).await.expect("sim client"); + + let calls = sim.create_client_calls(); + assert_eq!(calls.len(), 1, "one client per client_by_info call"); + assert_eq!( + calls[0].selection, + VendorSelection::Detect, + "client_by_info must delegate the pin to the pool, not resolve it itself" + ); + calls[0].vendor + } + + assert_eq!( + vendor_passed_for(Some(RedfishVendor::Dell)).await, + Some(RedfishVendor::Dell), + "a pinned vendor must reach create_client" + ); + assert_eq!( + vendor_passed_for(None).await, + None, + "no pin leaves automatic detection in place" + ); + } + + /// A pin must not apply to any BMC other than the one it was set on. + #[tokio::test] + async fn operator_pin_is_scoped_to_its_own_host() { + let sim = RedfishSim::default(); + sim.set_vendor_override("127.0.0.1", RedfishVendor::Dell); + + let access = BmcAccessInfo { + host: "127.0.0.2".to_string(), + port: Some(443), + mac_address: mac_address::MacAddress::new([0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0x02]), + }; + sim.client_by_info(&access).await.expect("sim client"); + + assert_eq!( + sim.create_client_calls()[0].vendor, + None, + "an unpinned BMC must still auto-detect" + ); + } + #[tokio::test] async fn set_bf4_dpu_service_password_changes_service_account() { let sim = RedfishSim::default(); diff --git a/crates/redfish/src/libredfish/test_support.rs b/crates/redfish/src/libredfish/test_support.rs index c519ce5ffd..750fab09fd 100644 --- a/crates/redfish/src/libredfish/test_support.rs +++ b/crates/redfish/src/libredfish/test_support.rs @@ -42,7 +42,10 @@ use libredfish::{ }; use mac_address::MacAddress; -use crate::libredfish::{RedfishAuth, RedfishClientCreationError, RedfishClientPool}; +use crate::libredfish::vendor_selection::ClientMode; +use crate::libredfish::{ + RedfishAuth, RedfishClientCreationError, RedfishClientPool, VendorSelection, +}; const TRIGGER_EVIDENCE_TASK_ID: &str = "SpdmTriggerEvidenceTaskId"; @@ -67,6 +70,10 @@ struct RedfishSimState { /// Records every call to `RedfishClientPool::create_client` so tests can /// assert what vendor was passed at each call site. create_client_calls: Vec, + /// Operator pinned vendors by BMC host, standing in for the stored + /// `bmc_vendor_override`. Empty by default so existing tests see no pin. + /// Seed it to assert a pin reaches a given call site. + vendor_overrides: HashMap, /// When set, `change_password` fails with /// [`RedfishError::PasswordChangeRequired`] to model a factory BMC (e.g. /// Viking) that refuses the by-username change until the initial @@ -121,6 +128,11 @@ fn sim_http_error(status: http::StatusCode, url: &str, body: &str) -> RedfishErr #[derive(Debug, Clone, PartialEq)] pub struct CreateClientCall { pub host: String, + /// What the call site asked for, before any pin was applied. Assert on this + /// to prove a probe path stayed uninitialized. + pub selection: VendorSelection, + /// The vendor the client was built with, after the pin was applied. `None` + /// means detection. Assert on this to prove a pin took effect. pub vendor: Option, } @@ -363,6 +375,17 @@ impl RedfishSim { self.state.lock().unwrap().service_root_vendor = vendor; } + /// Pin an operator vendor for `host`, standing in for the stored value. + /// + /// `host` must match what the call site passes, the BMC IP without its port. + pub fn set_vendor_override(&self, host: &str, vendor: RedfishVendor) { + self.state + .lock() + .unwrap() + .vendor_overrides + .insert(host.to_string(), vendor); + } + /// Override the `Manufacturer` reported by `get_chassis`, so tests can /// drive `probe_bmc_vendor`'s Lite-On/Delta chassis fallback. pub fn set_chassis_manufacturer(&self, manufacturer: Option) { @@ -2229,13 +2252,25 @@ impl RedfishClientPool for RedfishSim { host: &str, port: Option, auth: RedfishAuth, - vendor: Option, + vendor: VendorSelection, ) -> Result, RedfishClientCreationError> { { let mut state = self.state.lock().unwrap(); + // Apply the seeded pin as the production pool does, so a test + // asserting on `vendor` sees what a real client would dispatch on. + // `resolve` guarantees a pin cannot reach the uninitialized mode. + let pin = state.vendor_overrides.get(host).copied(); + let applied = match vendor.resolve(pin) { + ClientMode::Detect => None, + ClientMode::Vendor(vendor) => Some(vendor), + // Recorded as `Unknown` to match how this mode was spelled + // before `VendorSelection`, which the rotation assertions read. + ClientMode::Uninitialized => Some(RedfishVendor::Unknown), + }; state.create_client_calls.push(CreateClientCall { host: host.to_string(), - vendor, + selection: vendor, + vendor: applied, }); let default_lockdown = state.default_lockdown.unwrap_or(EnabledDisabled::Disabled); state @@ -2264,6 +2299,15 @@ impl RedfishClientPool for RedfishSim { &self.credential_manager } + async fn pinned_bmc_vendor(&self, host: &str) -> Option { + self.state + .lock() + .unwrap() + .vendor_overrides + .get(host) + .copied() + } + async fn uefi_setup( &self, _client: &dyn Redfish, diff --git a/crates/redfish/src/libredfish/vendor_override.rs b/crates/redfish/src/libredfish/vendor_override.rs new file mode 100644 index 0000000000..1e99a3d6ba --- /dev/null +++ b/crates/redfish/src/libredfish/vendor_override.rs @@ -0,0 +1,76 @@ +/* + * SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +use async_trait::async_trait; +use libredfish::model::service_root::RedfishVendor; + +/// Resolves the operator pinned Redfish vendor for a BMC. +/// +/// Injected so this crate need not reach the database. Keyed by host because that +/// is the only identity reaching every BMC client construction. +#[async_trait] +pub trait BmcVendorOverrideResolver: Send + Sync + 'static { + /// The pinned vendor for the BMC at `host`, or `None` when it has no pin. + /// + /// `host` is the real BMC host, an IP literal or a hostname, never the site + /// wide BMC proxy substituted in its place. + async fn vendor_override( + &self, + host: &str, + ) -> Result, VendorOverrideError>; +} + +/// A pin lookup that failed. +/// +/// Advisory, so the pool warns and falls back to detection. The source is boxed +/// to keep a database dependency out of this crate. +#[derive(Debug, thiserror::Error)] +#[error("failed to resolve bmc_vendor_override for {host}: {source}")] +pub struct VendorOverrideError { + pub host: String, + #[source] + pub source: Box, +} + +impl VendorOverrideError { + pub fn new( + host: impl Into, + source: impl Into>, + ) -> Self { + Self { + host: host.into(), + source: source.into(), + } + } +} + +/// A resolver that never pins anything. +/// +/// For binaries with no expected machine store to consult. Named rather than +/// left absent so that having no pins is a visible choice at the call site. +#[derive(Debug, Default, Clone, Copy)] +pub struct NoBmcVendorOverrides; + +#[async_trait] +impl BmcVendorOverrideResolver for NoBmcVendorOverrides { + async fn vendor_override( + &self, + _host: &str, + ) -> Result, VendorOverrideError> { + Ok(None) + } +} diff --git a/crates/redfish/src/libredfish/vendor_selection.rs b/crates/redfish/src/libredfish/vendor_selection.rs new file mode 100644 index 0000000000..0320c5a6a3 --- /dev/null +++ b/crates/redfish/src/libredfish/vendor_selection.rs @@ -0,0 +1,107 @@ +/* + * SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +use libredfish::model::service_root::RedfishVendor; + +/// How `create_client` picks the vendor implementation a client dispatches on. +/// +/// Replaced a bare `Option`, which encoded three unrelated intents +/// in two states and kept them apart only by convention. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum VendorSelection { + /// Detect the vendor from the BMC service root. + /// + /// An operator pin is used instead when the host has one, because a pin + /// exists precisely for hosts whose detection result is wrong. + Detect, + /// A vendor the caller resolved itself, from DMI or a probe. + /// + /// An operator pin outranks it. Every value arriving here is itself a + /// detection result, and a pin is the operator correction to detection. + Hint(RedfishVendor), + /// Probe and bootstrap mode, an uninitialized client that fetches nothing. + /// + /// Never pinned, carrying no vendor so pinning it cannot be expressed. Factory + /// GBx00 BMCs refuse `/Systems` until rotation, so a vendor here deadlocks. + Uninitialized, +} + +/// The client construction mode after any operator pin has been applied. +/// +/// Separate from [`VendorSelection`] so the precedence rule is one pure function +/// that can be tested exhaustively, leaving `create_client` to dispatch only. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum ClientMode { + /// Let libredfish detect the vendor from the service root. + Detect, + /// Build a client for this specific vendor. + Vendor(RedfishVendor), + /// Return a standard client with no vendor implementation and no fetches. + Uninitialized, +} + +impl VendorSelection { + /// Apply the operator pin to this selection. + /// + /// A pin wins over `Detect` and `Hint`, and is ignored by `Uninitialized`. + pub(crate) fn resolve(self, pin: Option) -> ClientMode { + match (self, pin) { + // Never pinnable. The caller asked for a client that can reach a BMC + // no vendor client can initialize against yet. + (Self::Uninitialized, _) => ClientMode::Uninitialized, + (Self::Detect | Self::Hint(_), Some(pinned)) => ClientMode::Vendor(pinned), + (Self::Detect, None) => ClientMode::Detect, + (Self::Hint(vendor), None) => ClientMode::Vendor(vendor), + } + } +} + +#[cfg(test)] +mod tests { + use carbide_test_support::value_scenarios; + + use super::*; + + #[test] + fn pin_outranks_detection_but_never_the_uninitialized_mode() { + value_scenarios!( + run = |(selection, pin): (VendorSelection, Option)| selection + .resolve(pin); + + "no pin leaves the caller's intent intact" { + (VendorSelection::Detect, None) => ClientMode::Detect, + (VendorSelection::Hint(RedfishVendor::Dell), None) + => ClientMode::Vendor(RedfishVendor::Dell), + (VendorSelection::Uninitialized, None) => ClientMode::Uninitialized, + } + + "a pin wins over detection and over a caller-resolved vendor" { + (VendorSelection::Detect, Some(RedfishVendor::Dell)) + => ClientMode::Vendor(RedfishVendor::Dell), + (VendorSelection::Hint(RedfishVendor::NvidiaGBx00), Some(RedfishVendor::Dell)) + => ClientMode::Vendor(RedfishVendor::Dell), + } + + // The rule factory BMC bootstrap and credential rotation depend on. + // If this row ever flips, rotation deadlocks on factory GBx00 BMCs. + "a pin never reaches the uninitialized mode" { + (VendorSelection::Uninitialized, Some(RedfishVendor::Dell)) + => ClientMode::Uninitialized, + } + ); + } +} diff --git a/crates/rpc/proto/forge.proto b/crates/rpc/proto/forge.proto index cb892447b3..e188130524 100644 --- a/crates/rpc/proto/forge.proto +++ b/crates/rpc/proto/forge.proto @@ -6269,6 +6269,15 @@ message ExpectedMachine { // Per-host control over how this BMC's IP is assigned and retained. See // `BmcIpAllocationType` above. Unset means "use the default" (AUTO). optional BmcIpAllocationType bmc_ip_allocation = 18; + // Field 19 is intentionally left unused so the internal `dpu_policy` name can + // never leak onto the wire. Guarded by the model test + // `host_dpu_policy_descriptor_retains_compatibility_surface`. + reserved 19; + // Operator-pinned Redfish BMC vendor for this host. When set, it is passed to + // libredfish as the forced vendor instead of detection from the service root. + // Empty or unset means automatic detection. Holds a `RedfishVendor` variant + // name (e.g. "Dell"). + optional string bmc_vendor_override = 20; } message ExpectedMachineRequest { diff --git a/crates/rpc/src/model/expected_machine.rs b/crates/rpc/src/model/expected_machine.rs index 7518575847..0eb182bba8 100644 --- a/crates/rpc/src/model/expected_machine.rs +++ b/crates/rpc/src/model/expected_machine.rs @@ -219,6 +219,7 @@ impl From for rpc::forge::ExpectedMachine { .host_lifecycle_profile .disable_lockdown, }), + bmc_vendor_override: expected_machine.data.bmc_vendor_override, } } } @@ -300,6 +301,12 @@ impl TryFrom for ExpectedMachineData { disable_lockdown: hlp.disable_lockdown, }) .unwrap_or_default(), + // An empty proto string clears the override, matching an empty value + // passed to the CLI flag. + bmc_vendor_override: match em.bmc_vendor_override.as_deref() { + None | Some("") => None, + Some(_) => em.bmc_vendor_override, + }, }) } } diff --git a/crates/site-explorer/src/bmc_endpoint_explorer.rs b/crates/site-explorer/src/bmc_endpoint_explorer.rs index b8965c17bb..80c870a67d 100644 --- a/crates/site-explorer/src/bmc_endpoint_explorer.rs +++ b/crates/site-explorer/src/bmc_endpoint_explorer.rs @@ -21,6 +21,7 @@ use std::sync::Arc; use std::sync::atomic::{AtomicBool, Ordering}; use std::time::Duration; +use crate::redfish::ResolvedVendor; use bmc_explorer::Product; use carbide_ipmi::IPMITool; use carbide_redfish::boot_interface::BootInterfaceTarget; @@ -285,7 +286,47 @@ impl BmcEndpointExplorer { .await } + /// Produce the exploration report, then let an operator pin override the + /// vendor the BMC reported. + /// + /// Applied after the mode dispatch, the only layer knowing pin from probe. pub async fn generate_exploration_report( + &self, + bmc_ip_address: SocketAddr, + credentials: Credentials, + boot_interface: Option<&BootInterfaceTarget>, + vendor: Option, + ) -> Result { + let client_vendor = vendor.map(ResolvedVendor::vendor); + let report = self + .generate_reported_exploration_report( + bmc_ip_address, + credentials, + boot_interface, + client_vendor, + ) + .await; + + let Some(pin) = vendor.and_then(ResolvedVendor::pinned) else { + return report; + }; + report.map(|mut report| { + let pinned = carbide_redfish::libredfish::conv::bmc_vendor(pin); + if report.vendor != Some(pinned) { + tracing::info!( + %bmc_ip_address, + reported = ?report.vendor, + %pinned, + redfish_vendor = %pin, + "Recording the operator-pinned BMC vendor instead of the reported one" + ); + } + report.vendor = Some(pinned); + report + }) + } + + async fn generate_reported_exploration_report( &self, bmc_ip_address: SocketAddr, credentials: Credentials, @@ -676,63 +717,84 @@ impl EndpointExplorer for BmcEndpointExplorer { } let bmc_mac_address = interface.mac_address; - let vendor = match self.redfish_client.get_redfish_vendor(bmc_ip_address).await { - Ok(vendor) => vendor, - Err(e) => { - tracing::error!( + + // An operator pinned vendor replaces detection outright, which is the + // whole point of the override. Read from the client pool rather than from + // `expected` so this value and the vendor every client for this BMC + // dispatches on can never disagree. + let resolved_vendor = match self.redfish_client.pinned_bmc_vendor(bmc_ip_address).await { + Some(vendor) => { + // Debug rather than info, because this runs for a pinned BMC on + // every exploration iteration and the log further below was + // demoted for that same reason in #4149. + tracing::debug!( %bmc_ip_address, - error = %e, - "Failed to probe Redfish service root endpoint" + %bmc_mac_address, + %vendor, + "Using operator-pinned BMC vendor, skipping service root detection" ); + ResolvedVendor::Pinned(vendor) + } + None => match self.redfish_client.get_redfish_vendor(bmc_ip_address).await { + Ok(vendor) => ResolvedVendor::Detected(vendor), + Err(e) => { + tracing::error!( + %bmc_ip_address, + error = %e, + "Failed to probe Redfish service root endpoint" + ); - // Lite-On power shelf BMCs don't expose Vendor details in the - // service root, so we fall back to probing the Chassis endpoint. - // Only attempt this for power shelf endpoints — machines and - // switches should never need this workaround. - // - // In the future, if we want to expand this to other kinds of trays we can - // expand the pattern matching logic below. - let Some(ExpectedEntity::PowerShelf(eps)) = expected else { - return Err(e); - }; + // Lite-On power shelf BMCs don't expose Vendor details in the + // service root, so we fall back to probing the Chassis endpoint. + // Only attempt this for power shelf endpoints — machines and + // switches should never need this workaround. + // + // In the future, if we want to expand this to other kinds of trays we can + // expand the pattern matching logic below. + let Some(ExpectedEntity::PowerShelf(eps)) = expected else { + return Err(e); + }; - let (username, password) = - match self.get_bmc_root_credentials(bmc_mac_address).await { - Ok(Credentials::UsernamePassword { username, password }) => { - (username, password) + let (username, password) = + match self.get_bmc_root_credentials(bmc_mac_address).await { + Ok(Credentials::UsernamePassword { username, password }) => { + (username, password) + } + Err(_) => (eps.bmc_username.clone(), eps.bmc_password.clone()), + }; + + // Lite-On and Delta power shelf BMCs don't expose vendor + // details in the service root, so we fall back to checking the + // Manufacturer field across all Chassis entries. + let vendor = match self + .redfish_client + .probe_vendor_name_from_chassis(bmc_ip_address, username, password) + .await + { + Ok(v) => v, + Err(chassis_err) => { + tracing::error!( + %bmc_ip_address, + error = %chassis_err, + "Failed to probe vendor from chassis" + ); + return Err(e); } - Err(_) => (eps.bmc_username.clone(), eps.bmc_password.clone()), }; - - // Lite-On and Delta power shelf BMCs don't expose vendor - // details in the service root, so we fall back to checking the - // Manufacturer field across all Chassis entries. - let vendor = match self - .redfish_client - .probe_vendor_name_from_chassis(bmc_ip_address, username, password) - .await - { - Ok(v) => v, - Err(chassis_err) => { - tracing::error!( - %bmc_ip_address, - error = %chassis_err, - "Failed to probe vendor from chassis" - ); + let vendor_lc = vendor.to_lowercase(); + if vendor_lc.contains("lite-on") { + ResolvedVendor::Detected(RedfishVendor::LiteOnPowerShelf) + } else if vendor_lc.contains("delta") { + ResolvedVendor::Detected(RedfishVendor::DeltaPowerShelf) + } else { return Err(e); } - }; - let vendor_lc = vendor.to_lowercase(); - if vendor_lc.contains("lite-on") { - RedfishVendor::LiteOnPowerShelf - } else if vendor_lc.contains("delta") { - RedfishVendor::DeltaPowerShelf - } else { - return Err(e); } - } + }, }; + let vendor = resolved_vendor.vendor(); + tracing::debug!( target: "carbide_diagnostics::bmc_redfish_supported", %bmc_ip_address, @@ -752,7 +814,7 @@ impl EndpointExplorer for BmcEndpointExplorer { bmc_ip_address, credentials, boot_interface, - Some(vendor), + Some(resolved_vendor), ) .await { @@ -917,7 +979,7 @@ impl EndpointExplorer for BmcEndpointExplorer { bmc_ip_address, bmc_credentials, boot_interface, - Some(vendor), + Some(resolved_vendor), ) .await? } @@ -1871,6 +1933,138 @@ mod tests { Ok(sim.machine_setup_status_targets(&bmc_ip_address.ip().to_string())) } + /// Explore a host whose service root reports `service_root_vendor`, with `pin` + /// seeded as the operator override. + /// + /// Returns the vendor every client used and the vendor the report recorded. + async fn explore_with_vendor_pin( + service_root_vendor: Option<&str>, + pin: Option, + ) -> Result<(Vec>, EndpointExplorationReport), String> { + let sim = Arc::new(RedfishSim::default()); + sim.set_service_root_vendor(service_root_vendor.map(str::to_string)); + sim.seed_user("root", "factory-password"); + let bmc_ip_address: SocketAddr = "127.0.0.1:443".parse().expect("valid test BMC address"); + if let Some(pin) = pin { + // Seeded on the pool, where production reads it from, so this + // exercises the same path rather than a test only shortcut. + sim.set_vendor_override(&bmc_ip_address.ip().to_string(), pin); + } + let proxy_address = Arc::new(ArcSwap::new(Arc::new(None))); + let explorer = BmcEndpointExplorer::new( + sim.clone(), + Arc::new(NvRedfishClientPool::new(proxy_address)), + carbide_ipmi::test_support(), + Arc::new(TestCredentialManager::default()), + Arc::new(AtomicBool::new(false)), + SiteExplorerExploreMode::LibRedfish, + None, + ); + let bmc_mac_address: MacAddress = "02:00:00:00:00:02".parse().expect("valid test BMC MAC"); + let interface = MachineInterfaceSnapshot::mock_with_mac(bmc_mac_address); + let expected = ExpectedEntity::Machine(ExpectedMachine { + id: None, + bmc_mac_address, + data: ExpectedMachineData { + bmc_username: "root".to_string(), + bmc_password: "factory-password".to_string(), + serial_number: "vendor-pin-host".to_string(), + bmc_retain_credentials: Some(true), + ..Default::default() + }, + }); + + let report = explorer + .explore_endpoint(bmc_ip_address, &interface, Some(&expected), None, None) + .await + .map_err(|error| error.to_string())?; + + Ok(( + sim.create_client_calls() + .into_iter() + .map(|call| call.vendor) + .collect(), + report, + )) + } + + /// The motivating case for the override, a BMC whose service root does not + /// identify it at all. Detection alone fails that host outright, so the pin + /// has to rescue the exploration, reach the client, and land in the persisted + /// report, which is what console access and firmware selection read. + #[tokio::test] + async fn explore_endpoint_honors_a_pinned_vendor_when_detection_fails() { + const UNIDENTIFIABLE: Option<&str> = Some("TotallyNotAKnownVendor"); + + let detected_only = explore_with_vendor_pin(UNIDENTIFIABLE, None).await; + assert!( + detected_only.is_err(), + "an unidentifiable BMC must fail exploration without a pin, got {detected_only:?}" + ); + + let (client_vendors, report) = + explore_with_vendor_pin(UNIDENTIFIABLE, Some(RedfishVendor::Dell)) + .await + .expect("a pinned vendor should carry the exploration past detection"); + assert!( + client_vendors.contains(&Some(RedfishVendor::Dell)), + "the pinned vendor must reach create_client, got {client_vendors:?}" + ); + assert_eq!( + report.vendor, + Some(bmc_vendor::BMCVendor::Dell), + "the pin must be what gets persisted; the BMC reported nothing usable" + ); + } + + /// A pin outranks what the BMC reports, so an operator can also correct a + /// host that misidentifies itself and not only one that stays silent. + #[tokio::test] + async fn explore_endpoint_prefers_a_pinned_vendor_over_detection() { + let (client_vendors, report) = + explore_with_vendor_pin(Some("Nvidia"), Some(RedfishVendor::Dell)) + .await + .expect("exploration should succeed"); + assert!( + client_vendors.contains(&Some(RedfishVendor::Dell)), + "the pin must win over the detected vendor, got {client_vendors:?}" + ); + assert_eq!( + report.vendor, + Some(bmc_vendor::BMCVendor::Dell), + "the pin must overwrite the vendor the BMC reported, not just the client" + ); + + let (client_vendors, report) = explore_with_vendor_pin(Some("Nvidia"), None) + .await + .expect("exploration should succeed"); + assert!( + !client_vendors.contains(&Some(RedfishVendor::Dell)), + "without a pin nothing should force Dell, got {client_vendors:?}" + ); + assert_eq!( + report.vendor, + Some(bmc_vendor::BMCVendor::Nvidia), + "an unpinned host must keep recording what it reported" + ); + } + + /// The pin is stored as a `RedfishVendor` name but persisted as the coarser + /// `BMCVendor`, so the mapping is part of the contract. Pinning a Lenovo GB300 + /// records `LenovoAMI`, which the firmware key and console transport use. + #[tokio::test] + async fn a_pinned_vendor_is_persisted_through_the_bmc_vendor_mapping() { + let (_, report) = explore_with_vendor_pin(Some("Nvidia"), Some(RedfishVendor::LenovoGB300)) + .await + .expect("exploration should succeed"); + assert_eq!(report.vendor, Some(bmc_vendor::BMCVendor::LenovoAMI)); + + // The persisted spelling and not just the enum, because the report JSON + // and the `desired_firmware` join both compare this string. + let wire = serde_json::to_value(&report).expect("report serializes"); + assert_eq!(wire["Vendor"], serde_json::json!("LenovoAMI")); + } + #[tokio::test] async fn credential_bootstrap_preserves_the_evaluated_boot_interface() { let mac_address = MacAddress::new([0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0x01]); diff --git a/crates/site-explorer/src/redfish.rs b/crates/site-explorer/src/redfish.rs index 9bd1b6a51f..9757e61f3e 100644 --- a/crates/site-explorer/src/redfish.rs +++ b/crates/site-explorer/src/redfish.rs @@ -25,7 +25,9 @@ use carbide_network::deserialize_input_mac_to_address; use carbide_redfish::boot_interface::BootInterfaceTarget; use carbide_redfish::libredfish::conv::{IntoModel, bmc_vendor}; use carbide_redfish::libredfish::dpu_bios::is_dpu_bios_attributes_not_ready; -use carbide_redfish::libredfish::{RedfishAuth, RedfishClientCreationError, RedfishClientPool}; +use carbide_redfish::libredfish::{ + RedfishAuth, RedfishClientCreationError, RedfishClientPool, VendorSelection, +}; use carbide_redfish::nv_redfish::NvRedfishClientPool; use carbide_secrets::credentials::Credentials; use libredfish::model::ODataId; @@ -46,6 +48,38 @@ use regex::Regex; const NOT_FOUND: u16 = 404; const BF4_NDF0_TO_BASE_MAC_OFFSET: u64 = 0x10; +/// The vendor NICo resolved for a BMC, and how it got there. +/// +/// Both carry the same vendor, because the report must record what the client +/// dispatched on. Only a pin may overwrite what the BMC reported. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum ResolvedVendor { + /// Set by an operator via `bmc_vendor_override`. + /// + /// Authoritative for client construction and for the persisted vendor, since + /// letting detection overwrite it would defeat the pin. + Pinned(RedfishVendor), + /// Probed from the service root or the power shelf Chassis fallback. + Detected(RedfishVendor), +} + +impl ResolvedVendor { + pub fn vendor(self) -> RedfishVendor { + match self { + Self::Pinned(vendor) | Self::Detected(vendor) => vendor, + } + } + + /// `Some` only when an operator set this vendor, meaning it may overwrite + /// what the BMC reported. + pub fn pinned(self) -> Option { + match self { + Self::Pinned(vendor) => Some(vendor), + Self::Detected(_) => None, + } + } +} + // RedfishClient is a wrapper around a redfish client pool and implements redfish utility functions that the site explorer utilizes. // TODO: In the future, we should refactor a lot of this client's work to api/src/redfish.rs because other components in carbide can utilize this functionality. // Eventually, this file should only have code related to generating the site exploration report. @@ -69,7 +103,7 @@ impl RedfishClient { &self, bmc_ip_address: SocketAddr, auth: RedfishAuth, - vendor: Option, + vendor: VendorSelection, ) -> Result, RedfishClientCreationError> { self.redfish_client_pool .create_client( @@ -85,15 +119,13 @@ impl RedfishClient { &self, bmc_ip_address: SocketAddr, ) -> Result, RedfishClientCreationError> { - // This currently uses a "standard" client without any vendor - // specific implementations. If we end up ever needing vendor - // specific support for a caller using this, we could simply - // just drop in vendor: Option to support an - // override of using RedfishVendor::Unknown. + // A standard client with no vendor implementation, all the service root + // and product probes need, and the only kind that works on a factory BMC + // that refuses reads until its password is rotated. Not pinnable. self.create_redfish_client( bmc_ip_address, RedfishAuth::Anonymous, - Some(RedfishVendor::Unknown), + VendorSelection::Uninitialized, ) .await } @@ -102,7 +134,7 @@ impl RedfishClient { &self, bmc_ip_address: SocketAddr, Credentials::UsernamePassword { username, password }: Credentials, - vendor: Option, + vendor: VendorSelection, ) -> Result, RedfishClientCreationError> { self.create_redfish_client( bmc_ip_address, @@ -112,12 +144,26 @@ impl RedfishClient { .await } + /// The operator pinned vendor for this BMC, if it has one. + /// + /// For the caller needing it as a value. Read from the pool so it matches + /// what `create_client` applies. + pub async fn pinned_bmc_vendor(&self, bmc_ip_address: SocketAddr) -> Option { + self.redfish_client_pool + .pinned_bmc_vendor(&bmc_ip_address.ip().to_string()) + .await + } + + /// The sole constructor for authenticated Site Explorer clients. + /// + /// The single place the operator vendor pin enters every BMC operation here, + /// so a new operation inherits it just by calling this. async fn create_authenticated_redfish_client( &self, bmc_ip_address: SocketAddr, credentials: Credentials, ) -> Result, RedfishClientCreationError> { - self.create_direct_redfish_client(bmc_ip_address, credentials, None) + self.create_direct_redfish_client(bmc_ip_address, credentials, VendorSelection::Detect) .await } @@ -199,7 +245,11 @@ impl RedfishClient { credentials: Credentials, ) -> Result<(), EndpointExplorationError> { let client = self - .create_direct_redfish_client(bmc_ip_address, credentials, Some(RedfishVendor::Unknown)) + .create_direct_redfish_client( + bmc_ip_address, + credentials, + VendorSelection::Uninitialized, + ) .await .map_err(map_redfish_client_creation_error)?; @@ -277,12 +327,21 @@ impl RedfishClient { vendor: Option, ) -> Result { let client = self - .create_direct_redfish_client(bmc_ip_address, credentials, vendor) + .create_direct_redfish_client( + bmc_ip_address, + credentials, + // Caller resolved, so a hint the pool may still override. + vendor.map_or(VendorSelection::Detect, VendorSelection::Hint), + ) .await .map_err(map_redfish_client_creation_error)?; let service_root = client.get_service_root().await.map_err(map_redfish_error)?; - let vendor = service_root.vendor().map(bmc_vendor); + // What the BMC reports about itself. An operator pin overrides this one + // layer up. The `vendor` parameter may be an anonymous probe result, which + // can disagree with the authenticated root, so substituting it here would + // change the recorded vendor for unpinned Lenovo AMI hosts. + let reported_vendor = service_root.vendor().map(bmc_vendor); let manager = fetch_manager(client.as_ref()) .await @@ -367,7 +426,7 @@ impl RedfishClient { systems: vec![system], chassis, service, - vendor, + vendor: reported_vendor, versions: HashMap::default(), model: None, power_shelf_id: None, @@ -1577,7 +1636,8 @@ mod tests { use super::{ BootInterfaceTarget, EndpointExplorationError, MachineSetupStatus, RedfishClient, - fetch_machine_setup_status, nv_bmc_explore_config, record_evaluated_boot_interface, + VendorSelection, fetch_machine_setup_status, nv_bmc_explore_config, + record_evaluated_boot_interface, }; fn test_addr() -> SocketAddr { @@ -1601,7 +1661,12 @@ mod tests { > { let sim = RedfishSim::default(); let client = sim - .create_client("test-host", None, RedfishAuth::Anonymous, None) + .create_client( + "test-host", + None, + RedfishAuth::Anonymous, + VendorSelection::Detect, + ) .await .map_err(|error| error.to_string())?; let status = fetch_machine_setup_status(client.as_ref(), target.as_ref()) @@ -1766,4 +1831,257 @@ mod tests { ) .await; } + + /// Every authenticated operation must honor the pin and stay unaffected + /// without one. + /// + /// Driven twice, so the second half is the no regression guarantee. + #[tokio::test] + async fn every_authenticated_operation_honors_the_pin_and_is_unchanged_without_one() { + const PIN: RedfishVendor = RedfishVendor::Dell; + + let boot_interface = + BootInterfaceTarget::MacOnly(MacAddress::new([0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0x01])); + + // Each entry drives one operation and discards its result. Only the + // vendor the client was constructed with matters here. + #[allow(clippy::type_complexity)] + let operations: Vec<( + &str, + Box< + dyn for<'a> Fn( + &'a RedfishClient, + SocketAddr, + Credentials, + ) + -> std::pin::Pin + 'a>>, + >, + )> = vec![ + ( + "reset_bmc", + Box::new(|c: &RedfishClient, a, k| { + Box::pin(async move { + let _ = c.reset_bmc(a, k).await; + }) + }), + ), + ( + "get_power_state", + Box::new(|c: &RedfishClient, a, k| { + Box::pin(async move { + let _ = c.get_power_state(a, k).await; + }) + }), + ), + ( + "power", + Box::new(|c: &RedfishClient, a, k| { + Box::pin(async move { + let _ = c.power(a, k, libredfish::SystemPowerControl::On).await; + }) + }), + ), + ( + "disable_secure_boot", + Box::new(|c: &RedfishClient, a, k| { + Box::pin(async move { + let _ = c.disable_secure_boot(a, k).await; + }) + }), + ), + ( + "lockdown", + Box::new(|c: &RedfishClient, a, k| { + Box::pin(async move { + let _ = c + .lockdown(a, k, libredfish::EnabledDisabled::Disabled) + .await; + }) + }), + ), + ( + "lockdown_status", + Box::new(|c: &RedfishClient, a, k| { + Box::pin(async move { + let _ = c.lockdown_status(a, k).await; + }) + }), + ), + ( + "enable_infinite_boot", + Box::new(|c: &RedfishClient, a, k| { + Box::pin(async move { + let _ = c.enable_infinite_boot(a, k).await; + }) + }), + ), + ( + "is_infinite_boot_enabled", + Box::new(|c: &RedfishClient, a, k| { + Box::pin(async move { + let _ = c.is_infinite_boot_enabled(a, k).await; + }) + }), + ), + ( + "machine_setup", + Box::new(|c: &RedfishClient, a, k| { + Box::pin(async move { + let _ = c.machine_setup(a, k, None).await; + }) + }), + ), + ( + "is_viking", + Box::new(|c: &RedfishClient, a, k| { + Box::pin(async move { + let _ = c.is_viking(a, k).await; + }) + }), + ), + ( + "clear_nvram", + Box::new(|c: &RedfishClient, a, k| { + Box::pin(async move { + let _ = c.clear_nvram(a, k).await; + }) + }), + ), + ( + "set_nic_mode", + Box::new(|c: &RedfishClient, a, k| { + Box::pin(async move { + let _ = c.set_nic_mode(a, k, super::NicMode::Dpu).await; + }) + }), + ), + ( + "create_bmc_user", + Box::new(|c: &RedfishClient, a, k| { + Box::pin(async move { + let _ = c + .create_bmc_user(a, k, "u", "p", libredfish::RoleId::Administrator) + .await; + }) + }), + ), + ( + "delete_bmc_user", + Box::new(|c: &RedfishClient, a, k| { + Box::pin(async move { + let _ = c.delete_bmc_user(a, k, "u").await; + }) + }), + ), + ( + "probe_vendor_name_from_chassis", + Box::new(|c: &RedfishClient, a, _k| { + Box::pin(async move { + let _ = c + .probe_vendor_name_from_chassis( + a, + "root".to_string(), + "pass".to_string(), + ) + .await; + }) + }), + ), + ]; + + // Pin to seed, then the vendor every client must be built with. `None` + // is the behavior before pins existed, leaving the vendor to detection. + let expectations = [(Some(PIN), Some(PIN)), (None, None)]; + + for (name, drive) in operations { + for (pin, expected) in expectations { + let sim = Arc::new(RedfishSim::default()); + if let Some(pin) = pin { + sim.set_vendor_override(&test_addr().ip().to_string(), pin); + } + let client = build_redfish_client(sim.clone()); + + drive(&client, test_addr(), Credentials::new("root", "pass")).await; + + let vendors: Vec> = sim + .create_client_calls() + .into_iter() + .map(|call| call.vendor) + .collect(); + assert!( + !vendors.is_empty(), + "{name} built no Redfish client, so this row proves nothing" + ); + assert!( + vendors.iter().all(|vendor| *vendor == expected), + "{name} with pin {pin:?} must build every client with {expected:?}, got {vendors:?}" + ); + } + } + + // `set_boot_order_dpu_first` takes its boot interface by reference and so + // does not fit the closure shape used above. + for (pin, expected) in expectations { + let sim = Arc::new(RedfishSim::default()); + if let Some(pin) = pin { + sim.set_vendor_override(&test_addr().ip().to_string(), pin); + } + let client = build_redfish_client(sim.clone()); + let _ = client + .set_boot_order_dpu_first( + test_addr(), + Credentials::new("root", "pass"), + &boot_interface, + ) + .await; + assert!( + sim.create_client_calls() + .iter() + .all(|call| call.vendor == expected), + "set_boot_order_dpu_first with pin {pin:?} must use {expected:?}" + ); + } + } + + /// The probe and credential bootstrap paths must stay unpinnable. + /// + /// They need an uninitialized client, which factory BMCs require, so a pin + /// reaching them would deadlock rotation. + #[tokio::test] + async fn probe_paths_ignore_the_operator_pin() { + // Both directions matter. With a pin because it must not reach these, + // and without one because nothing about them changed. + for pin in [Some(RedfishVendor::Dell), None] { + probe_paths_stay_uninitialized(pin).await; + } + } + + async fn probe_paths_stay_uninitialized(pin: Option) { + let sim = Arc::new(RedfishSim::default()); + if let Some(pin) = pin { + sim.set_vendor_override(&test_addr().ip().to_string(), pin); + } + let client = build_redfish_client(sim.clone()); + + let _ = client.get_redfish_product(test_addr()).await; + let _ = client.get_redfish_vendor(test_addr()).await; + let _ = client + .validate_bmc_credentials(test_addr(), Credentials::new("root", "pass")) + .await; + + let calls = sim.create_client_calls(); + assert!(!calls.is_empty(), "expected probe clients to be built"); + for call in calls { + assert_eq!( + call.selection, + VendorSelection::Uninitialized, + "a probe path must ask for an uninitialized client" + ); + assert_eq!( + call.vendor, + Some(RedfishVendor::Unknown), + "a probe client must stay uninitialized (pin {pin:?})" + ); + } + } } diff --git a/crates/site-explorer/tests/integration/site_explorer.rs b/crates/site-explorer/tests/integration/site_explorer.rs index 87ef92519c..bda5cf6197 100644 --- a/crates/site-explorer/tests/integration/site_explorer.rs +++ b/crates/site-explorer/tests/integration/site_explorer.rs @@ -789,6 +789,7 @@ async fn test_expected_machine_device_type_metrics( dpu_policy: Default::default(), bmc_ip_allocation: Default::default(), host_lifecycle_profile: Default::default(), + bmc_vendor_override: None, }, }, ) @@ -815,6 +816,7 @@ async fn test_expected_machine_device_type_metrics( dpu_policy: Default::default(), bmc_ip_allocation: Default::default(), host_lifecycle_profile: Default::default(), + bmc_vendor_override: None, }, }, ) @@ -841,6 +843,7 @@ async fn test_expected_machine_device_type_metrics( dpu_policy: Default::default(), bmc_ip_allocation: Default::default(), host_lifecycle_profile: Default::default(), + bmc_vendor_override: None, }, }, ) @@ -2350,6 +2353,7 @@ async fn test_fallback_dpu_serial(pool: PgPool) -> Result<(), Box Result<(), Box Result<(), Box* Host NICs as a JSON array of ExpectedHostNic objects (fields: -mac_address, nic_type, fixed_ip, fixed_mask, fixed_gateway, primary) +mac_address, network_segment_type, fixed_ip, fixed_mask, fixed_gateway, +primary; legacy: nic_type) **--rack_id** *\* Rack ID for this machine @@ -73,9 +75,9 @@ Rack ID for this machine **--default_pause_ingestion_and_poweron** *\* Optional flag to pause machines ingestion and power on. False - dont pause, true - will pause it. The actual mutable state is stored in -explored_endpoints.\ +explored_endpoints. -\ + *Possible values:* - true @@ -83,9 +85,9 @@ explored_endpoints.\ - false **--dpf-enabled** *\* -DPF enable/disable for this machine. Default is updated as true.\ +DPF enable/disable for this machine. Default is updated as true. -\ + *Possible values:* - true @@ -105,25 +107,25 @@ as expected switches) **--bmc-retain-credentials** *\* When true, site-explorer skips BMC password rotation and stores -factory-default credentials in Vault as-is\ +factory-default credentials in Vault as-is -\ + *Possible values:* - true - false -**--dpu-policy** *\*\ +**--dpu-policy** *\* Per-host DPU policy. \`manage\` (default): inherit the site policy, -which defaults to managing DPUs; \`nic\`: configure DPU hardware -as plain NICs; \`ignore\`: do not configure or attach DPU hardware. -Unset defers to the site-wide \`\[site_explorer\] dpu_policy\` setting. -The previous \`use-as-nic\` value remains accepted as an alias. The legacy -\`--dpu-mode\` flag also remains accepted: \`dpu-mode\` maps to \`manage\`, -\`nic-mode\` to \`nic\`, and \`no-dpu\` to \`ignore\`.\ - -\ +which defaults to managing DPUs; \`nic\`: configure DPU hardware as +plain NICs; \`ignore\`: do not configure or attach DPU hardware. Unset +defers to the site-wide \`\[site_explorer\] dpu_policy\` setting. The +previous \`use-as-nic\` value remains accepted as an alias. The legacy +\`--dpu-mode\` flag also remains accepted: \`dpu-mode\` maps to +\`manage\`, \`nic-mode\` to \`nic\`, and \`no-dpu\` to \`ignore\`. + + *Possible values:* - manage @@ -132,22 +134,55 @@ The previous \`use-as-nic\` value remains accepted as an alias. The legacy - ignore +**--bmc-ip-allocation** *\* +Per-host control over how this BMCs IP is assigned and retained. +\`auto\` (default): infer from \`--bmc-ip-address\` -- a configured +address is \`fixed\`, no address is \`retained\`; \`dynamic\`: a normal +DHCP lease that may expire and change; \`fixed\`: the operator-specified +\`--bmc-ip-address\` (static); \`retained\`: an auto-allocated address +pinned as static (never expires). Unset defers to the server default +(\`auto\`). + + +*Possible values:* + +- unspecified + +- auto + +- dynamic + +- fixed + +- retained + **--disable-lockdown** *\* If true, do not lock down the server as part of lifecycle management within the state machine. If unset or false, preserve the default -behavior of locking down the server after configuring the BIOS.\ +behavior of locking down the server after configuring the BIOS. -\ + *Possible values:* - true - false +**--bmc-vendor-override** *\* +Pin the Redfish BMC vendor for this host. Once set it governs how NICo +talks to this BMC -- which libredfish driver each Redfish client +dispatches on, the vendor recorded by Site Explorer, and therefore the +firmware config lookup, the IPMI-vs-Redfish restart choice and the BMC +console transport. Host lifecycle decisions keyed to the host's own DMI +data are unaffected. A RedfishVendor variant name, case-sensitive (e.g. +Dell, Supermicro, NvidiaDpu, Hpe, Lenovo). Unset means automatic +detection. Not applied to credential rotation or factory bootstrap, +which must reach a BMC before its vendor is usable. + **--sort-by** *\* \[default: primary-id\] -Sort output by specified field\ +Sort output by specified field -\ + *Possible values:* - primary-id: Sort by the primary id @@ -164,6 +199,7 @@ nico-admin-cli expected-machine add --bmc-mac-address 00:11:22:33:44:55 --bmc-us nico-admin-cli expected-machine add --bmc-mac-address 00:11:22:33:44:55 --bmc-username admin --bmc-password mypassword --chassis-serial-number sample_serial-1 --meta-name MyMachine --label DATACENTER:XYZ --sku-id DGX-H100-640GB nico-admin-cli expected-machine add --bmc-mac-address 00:11:22:33:44:55 --bmc-username admin --bmc-password mypassword --chassis-serial-number sample_serial-1 --bmc-ip-address 192.0.2.20 nico-admin-cli expected-machine add --bmc-mac-address 00:11:22:33:44:55 --bmc-username admin --bmc-password mypassword --chassis-serial-number sample_serial-1 --dpu-policy nic +nico-admin-cli expected-machine add --bmc-mac-address 00:11:22:33:44:55 --bmc-username admin --bmc-password mypassword --chassis-serial-number sample_serial-1 --bmc-ip-allocation retained ``` --- diff --git a/docs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-patch.md b/docs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-patch.md index c08148966c..81530a83fb 100644 --- a/docs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-patch.md +++ b/docs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-patch.md @@ -18,7 +18,9 @@ update, preserves unprovided fields). \[**--rack-id**\] \[**--default_pause_ingestion_and_poweron**\] \[**--dpf-enabled**\] \[**--bmc-ip-address**\] \[**--extended**\] \[**--bmc-retain-credentials**\] \[**--dpu-policy**\] -\[**--disable-lockdown**\] \[**--sort-by**\] \[**-h**\|**--help**\] +\[**--bmc-ip-allocation**\] \[**--host_nics**\] +\[**--disable-lockdown**\] \[**--bmc-vendor-override**\] +\[**--sort-by**\] \[**-h**\|**--help**\] ## DESCRIPTION @@ -79,9 +81,9 @@ A RACK ID that will be added for the newly created Machine. **--default_pause_ingestion_and_poweron** *\* Optional flag to pause machines ingestion and power on. False - dont pause, true - will pause it. The actual mutable state is stored in -explored_endpoints.\ +explored_endpoints. -\ + *Possible values:* - true @@ -89,9 +91,9 @@ explored_endpoints.\ - false **--dpf-enabled** *\* -DPF enable/disable for this machine. Default is updated as true.\ +DPF enable/disable for this machine. Default is updated as true. -\ + *Possible values:* - true @@ -111,25 +113,25 @@ internal UUIDs that are used to associate instances. **--bmc-retain-credentials** *\* When true, site-explorer skips BMC password rotation and stores -factory-default credentials in Vault as-is\ +factory-default credentials in Vault as-is -\ + *Possible values:* - true - false -**--dpu-policy** *\*\ +**--dpu-policy** *\* Per-host DPU policy. \`manage\`: inherit the site policy, which defaults to managing DPUs; \`nic\`: configure DPU hardware as plain NICs; \`ignore\`: do not configure or attach DPU hardware. Unset preserves the -existing per-host value. The previous \`use-as-nic\` value remains accepted -as an alias. The legacy \`--dpu-mode\` flag also remains accepted: -\`dpu-mode\` maps to \`manage\`, \`nic-mode\` to \`nic\`, and \`no-dpu\` -to \`ignore\`.\ +existing per-host value. The previous \`use-as-nic\` value remains +accepted as an alias. The legacy \`--dpu-mode\` flag also remains +accepted: \`dpu-mode\` maps to \`manage\`, \`nic-mode\` to \`nic\`, and +\`no-dpu\` to \`ignore\`. -\ + *Possible values:* - manage @@ -138,22 +140,62 @@ to \`ignore\`.\ - ignore +**--bmc-ip-allocation** *\* +Per-host control over how this BMCs IP is assigned and retained. +\`auto\` (default): infer from \`--bmc-ip-address\` -- a configured +address is \`fixed\`, no address is \`retained\`; \`dynamic\`: a normal +DHCP lease that may expire and change; \`fixed\`: the operator-specified +\`--bmc-ip-address\` (static); \`retained\`: an auto-allocated address +pinned as static (never expires). Unset preserves the existing per-host +value. + + +*Possible values:* + +- unspecified + +- auto + +- dynamic + +- fixed + +- retained + +**--host_nics** *\* +Host NICs as a JSON array of ExpectedHostNic objects (fields: +mac_address, network_segment_type, fixed_ip, fixed_mask, fixed_gateway, +primary; legacy: nic_type). Replaces the machines full host NIC list. + **--disable-lockdown** *\* If true, do not lock down the server as part of lifecycle management within the state machine. If unset or false, preserve the default -behavior of locking down the server after configuring the BIOS.\ +behavior of locking down the server after configuring the BIOS. -\ + *Possible values:* - true - false +**--bmc-vendor-override** *\* +Pin the Redfish BMC vendor for this host. Once set it governs how NICo +talks to this BMC -- which libredfish driver each Redfish client +dispatches on, the vendor recorded by Site Explorer, and therefore the +firmware config lookup, the IPMI-vs-Redfish restart choice and the BMC +console transport. Host lifecycle decisions keyed to the host's own DMI +data are unaffected. A RedfishVendor variant name, case-sensitive (e.g. +Dell, Supermicro, NvidiaDpu, Hpe, Lenovo). Redfish clients pick it up +within a minute; the recorded vendor changes at the next successful +exploration. Pass an empty string to clear the override (return to +automatic detection). Not applied to credential rotation or factory +bootstrap, which must reach a BMC before its vendor is usable. + **--sort-by** *\* \[default: primary-id\] -Sort output by specified field\ +Sort output by specified field -\ + *Possible values:* - primary-id: Sort by the primary id @@ -170,6 +212,9 @@ nico-admin-cli expected-machine patch --bmc-mac-address 00:11:22:33:44:55 --sku- nico-admin-cli expected-machine patch --id 12345678-1234-5678-90ab-cdef01234567 --sku-id DGX-H100-640GB nico-admin-cli expected-machine patch --bmc-mac-address 00:11:22:33:44:55 --bmc-username admin --bmc-password mynewpassword nico-admin-cli expected-machine patch --bmc-mac-address 00:11:22:33:44:55 --dpu-policy ignore +nico-admin-cli expected-machine patch --bmc-mac-address 00:11:22:33:44:55 --bmc-ip-allocation retained +nico-admin-cli expected-machine patch --bmc-mac-address 00:11:22:33:44:55 --bmc-vendor-override Dell +nico-admin-cli expected-machine patch --bmc-mac-address 00:11:22:33:44:55 --bmc-vendor-override "" ``` --- diff --git a/rest-api/proto/core/gen/v1/nico_nico.pb.go b/rest-api/proto/core/gen/v1/nico_nico.pb.go index 97a0514bc3..d7a940c9f4 100644 --- a/rest-api/proto/core/gen/v1/nico_nico.pb.go +++ b/rest-api/proto/core/gen/v1/nico_nico.pb.go @@ -35326,6 +35326,11 @@ type ExpectedMachine struct { // Per-host control over how this BMC's IP is assigned and retained. See // `BmcIpAllocationType` above. Unset means "use the default" (AUTO). BmcIpAllocation *BmcIpAllocationType `protobuf:"varint,18,opt,name=bmc_ip_allocation,json=bmcIpAllocation,proto3,enum=forge.BmcIpAllocationType,oneof" json:"bmc_ip_allocation,omitempty"` + // Operator-pinned Redfish BMC vendor for this host. When set, it is passed to + // libredfish as the forced vendor instead of detection from the service root. + // Empty or unset means automatic detection. Holds a `RedfishVendor` variant + // name (e.g. "Dell"). + BmcVendorOverride *string `protobuf:"bytes,20,opt,name=bmc_vendor_override,json=bmcVendorOverride,proto3,oneof" json:"bmc_vendor_override,omitempty"` // WARNING: Following fields are not present in Core, but added directly in REST snapshot Name *string `protobuf:"bytes,21,opt,name=name,proto3,oneof" json:"name,omitempty"` Manufacturer *string `protobuf:"bytes,22,opt,name=manufacturer,proto3,oneof" json:"manufacturer,omitempty"` @@ -35496,6 +35501,13 @@ func (x *ExpectedMachine) GetBmcIpAllocation() BmcIpAllocationType { return BmcIpAllocationType_BMC_IP_ALLOCATION_TYPE_UNSPECIFIED } +func (x *ExpectedMachine) GetBmcVendorOverride() string { + if x != nil && x.BmcVendorOverride != nil { + return *x.BmcVendorOverride + } + return "" +} + func (x *ExpectedMachine) GetName() string { if x != nil && x.Name != nil { return *x.Name @@ -64294,7 +64306,7 @@ const file_nico_nico_proto_rawDesc = "" + "\x15_network_segment_type\"[\n" + "\x14HostLifecycleProfile\x12.\n" + "\x10disable_lockdown\x18\x01 \x01(\bH\x00R\x0fdisableLockdown\x88\x01\x01B\x13\n" + - "\x11_disable_lockdown\"\xe2\v\n" + + "\x11_disable_lockdown\"\xb5\f\n" + "\x0fExpectedMachine\x12&\n" + "\x0fbmc_mac_address\x18\x01 \x01(\tR\rbmcMacAddress\x12!\n" + "\fbmc_username\x18\x02 \x01(\tR\vbmcUsername\x12!\n" + @@ -64315,16 +64327,17 @@ const file_nico_nico_proto_rawDesc = "" + "\x16bmc_retain_credentials\x18\x0f \x01(\bH\x06R\x14bmcRetainCredentials\x88\x01\x01\x12.\n" + "\bdpu_mode\x18\x10 \x01(\x0e2\x0e.forge.DpuModeH\aR\adpuMode\x88\x01\x01\x12V\n" + "\x16host_lifecycle_profile\x18\x11 \x01(\v2\x1b.forge.HostLifecycleProfileH\bR\x14hostLifecycleProfile\x88\x01\x01\x12K\n" + - "\x11bmc_ip_allocation\x18\x12 \x01(\x0e2\x1a.forge.BmcIpAllocationTypeH\tR\x0fbmcIpAllocation\x88\x01\x01\x12\x17\n" + - "\x04name\x18\x15 \x01(\tH\n" + - "R\x04name\x88\x01\x01\x12'\n" + - "\fmanufacturer\x18\x16 \x01(\tH\vR\fmanufacturer\x88\x01\x01\x12\x19\n" + - "\x05model\x18\x17 \x01(\tH\fR\x05model\x88\x01\x01\x12%\n" + - "\vdescription\x18\x18 \x01(\tH\rR\vdescription\x88\x01\x01\x12.\n" + - "\x10firmware_version\x18\x19 \x01(\tH\x0eR\x0ffirmwareVersion\x88\x01\x01\x12\x1c\n" + - "\aslot_id\x18\x1a \x01(\x05H\x0fR\x06slotId\x88\x01\x01\x12\x1e\n" + - "\btray_idx\x18\x1b \x01(\x05H\x10R\atrayIdx\x88\x01\x01\x12\x1c\n" + - "\ahost_id\x18\x1c \x01(\x05H\x11R\x06hostId\x88\x01\x01B\t\n" + + "\x11bmc_ip_allocation\x18\x12 \x01(\x0e2\x1a.forge.BmcIpAllocationTypeH\tR\x0fbmcIpAllocation\x88\x01\x01\x123\n" + + "\x13bmc_vendor_override\x18\x14 \x01(\tH\n" + + "R\x11bmcVendorOverride\x88\x01\x01\x12\x17\n" + + "\x04name\x18\x15 \x01(\tH\vR\x04name\x88\x01\x01\x12'\n" + + "\fmanufacturer\x18\x16 \x01(\tH\fR\fmanufacturer\x88\x01\x01\x12\x19\n" + + "\x05model\x18\x17 \x01(\tH\rR\x05model\x88\x01\x01\x12%\n" + + "\vdescription\x18\x18 \x01(\tH\x0eR\vdescription\x88\x01\x01\x12.\n" + + "\x10firmware_version\x18\x19 \x01(\tH\x0fR\x0ffirmwareVersion\x88\x01\x01\x12\x1c\n" + + "\aslot_id\x18\x1a \x01(\x05H\x10R\x06slotId\x88\x01\x01\x12\x1e\n" + + "\btray_idx\x18\x1b \x01(\x05H\x11R\atrayIdx\x88\x01\x01\x12\x1c\n" + + "\ahost_id\x18\x1c \x01(\x05H\x12R\x06hostId\x88\x01\x01B\t\n" + "\a_sku_idB\x05\n" + "\x03_idB\n" + "\n" + @@ -64335,7 +64348,8 @@ const file_nico_nico_proto_rawDesc = "" + "\x17_bmc_retain_credentialsB\v\n" + "\t_dpu_modeB\x19\n" + "\x17_host_lifecycle_profileB\x14\n" + - "\x12_bmc_ip_allocationB\a\n" + + "\x12_bmc_ip_allocationB\x16\n" + + "\x14_bmc_vendor_overrideB\a\n" + "\x05_nameB\x0f\n" + "\r_manufacturerB\b\n" + "\x06_modelB\x0e\n" + @@ -64345,7 +64359,7 @@ const file_nico_nico_proto_rawDesc = "" + "\b_slot_idB\v\n" + "\t_tray_idxB\n" + "\n" + - "\b_host_id\"j\n" + + "\b_host_idJ\x04\b\x13\x10\x14\"j\n" + "\x16ExpectedMachineRequest\x12&\n" + "\x0fbmc_mac_address\x18\x01 \x01(\tR\rbmcMacAddress\x12!\n" + "\x02id\x18\x02 \x01(\v2\f.common.UUIDH\x00R\x02id\x88\x01\x01B\x05\n" + diff --git a/rest-api/proto/core/src/v1/nico_nico.proto b/rest-api/proto/core/src/v1/nico_nico.proto index f9b083e091..f52eaeeb1e 100644 --- a/rest-api/proto/core/src/v1/nico_nico.proto +++ b/rest-api/proto/core/src/v1/nico_nico.proto @@ -6270,6 +6270,15 @@ message ExpectedMachine { // Per-host control over how this BMC's IP is assigned and retained. See // `BmcIpAllocationType` above. Unset means "use the default" (AUTO). optional BmcIpAllocationType bmc_ip_allocation = 18; + // Field 19 is intentionally left unused so the internal `dpu_policy` name can + // never leak onto the wire. Guarded by the model test + // `host_dpu_policy_descriptor_retains_compatibility_surface`. + reserved 19; + // Operator-pinned Redfish BMC vendor for this host. When set, it is passed to + // libredfish as the forced vendor instead of detection from the service root. + // Empty or unset means automatic detection. Holds a `RedfishVendor` variant + // name (e.g. "Dell"). + optional string bmc_vendor_override = 20; // WARNING: Following fields are not present in Core, but added directly in REST snapshot optional string name = 21;