From 5e739ccb11a065fc0457ec5938242624df0b0e62 Mon Sep 17 00:00:00 2001 From: Hitesh Wadekar Date: Wed, 5 Aug 2026 10:13:19 -0700 Subject: [PATCH 1/6] feat(config): reject unknown fields in NICo API configuration files --- book/src/configuration/configurability.md | 1 - crates/api-core/src/cfg/README.md | 10 + crates/api-core/src/cfg/file.rs | 282 +++++++++++++++++- crates/api-core/src/cfg/load.rs | 61 ++++ .../src/cfg/test_data/full_config.toml | 4 - .../test_data/full_config_post_migration.toml | 4 - .../src/cfg/test_data/site_config.toml | 1 - .../dpu_nic_firmware.rs | 2 +- .../src/test_support/default_config.rs | 1 + .../src/tests/common/api_fixtures/mod.rs | 1 + crates/api-model/src/firmware.rs | 5 + crates/api-model/src/machine/mod.rs | 1 + .../src/network_security_group/mod.rs | 1 + crates/api-model/src/network_segment/mod.rs | 1 + crates/api-model/src/rack_type.rs | 6 + crates/api-model/src/resource_pool/define.rs | 2 + crates/api-model/src/vpc/mod.rs | 1 + crates/api-model/src/vpc/routing_profile.rs | 3 + crates/api-test-helper/src/api_server.rs | 11 - crates/authn/src/config.rs | 2 + crates/component-manager/src/config.rs | 3 + crates/dpa-manager/src/config.rs | 3 + crates/dpf/src/types.rs | 1 + crates/ib-fabric/src/config.rs | 2 + crates/libmlx/src/firmware/config.rs | 73 ++++- crates/libmlx/src/firmware/credentials.rs | 2 +- crates/libmlx/src/profile/serialization.rs | 1 + .../src/config/bom_validation.rs | 1 + .../src/config/controller.rs | 1 + .../src/config/firmware_global.rs | 1 + .../src/config/machine_validation.rs | 2 + crates/machine-controller/src/config/mod.rs | 1 + .../src/config/power_manager.rs | 1 + crates/nras/src/lib.rs | 1 + crates/nvlink-manager/src/config.rs | 1 + crates/rack-controller/src/config.rs | 2 + crates/site-explorer/src/config.rs | 46 +-- .../tests/integration/site_explorer.rs | 1 + crates/state-controller-common/src/config.rs | 1 + .../api/config-files/nico-api-config.toml | 10 +- dev/docker-env/carbide-api-config.toml | 1 - .../nico-api/files/carbide-api-config.toml | 9 +- 42 files changed, 483 insertions(+), 81 deletions(-) diff --git a/book/src/configuration/configurability.md b/book/src/configuration/configurability.md index 1b3021fa5c..58c3d2d8a4 100644 --- a/book/src/configuration/configurability.md +++ b/book/src/configuration/configurability.md @@ -692,7 +692,6 @@ These don't fit any sub-section but show up in production tuning: | `min_dpu_functioning_links` | unset | Minimum healthy DPU links for a machine to report `Healthy`. Unset = all links required. | | `set_http_boot_uri_for_vendors` | `[]` | Vendors for which the state controller pins UEFI HTTP Boot URL on the BMC via Redfish. Empty = rely on DHCP option 67. | | `x86_pxe_boot_url_override` / `arm_pxe_boot_url_override` | unset | Override the default `nico-pxe` boot URL by architecture. Useful when chaining through an external HTTP boot artifact server. | -| `nvue_enabled` | `true` | When `false`, DPU agents write configs directly instead of going through NVUE. | | `anycast_site_prefixes` | `[]` | **Deprecated** — use `[fnn.routing_profiles.].allowed_anycast_prefixes` instead. | | `internet_l3_vni` | `100001` | L3 VNI announced for FNN VPC internet connectivity. Combined with `datacenter_asn` for the route-target. | | `datacenter_asn` | `11414` | Datacenter ASN used by FNN for DC-specific route targets. | diff --git a/crates/api-core/src/cfg/README.md b/crates/api-core/src/cfg/README.md index 218d5e3776..e4c805e690 100644 --- a/crates/api-core/src/cfg/README.md +++ b/crates/api-core/src/cfg/README.md @@ -5,6 +5,16 @@ configuration file, which is deserialized into `NicoConfig` (defined in `file.rs`). Fields are listed in declaration order. Defaults are noted where applicable. +Unknown fields are rejected after the base file, optional site override, and +`CARBIDE_API_` environment values are merged. The startup error reports the +invalid key's full section path. Names inside intentionally dynamic maps, such +as pool names and rack-profile IDs, remain user-defined; fields within each map +value must still match the documented schema. + +The removed `force_dpu_nic_mode` key is the sole compatibility exception: it +is temporarily accepted at the top level and under `[site_explorer]`, ignored, +and reported as a deprecation warning. Use `site_explorer.dpu_policy` instead. + --- ## `NicoConfig` (top-level) diff --git a/crates/api-core/src/cfg/file.rs b/crates/api-core/src/cfg/file.rs index 091cb0e51f..6f4e05b9ef 100644 --- a/crates/api-core/src/cfg/file.rs +++ b/crates/api-core/src/cfg/file.rs @@ -104,6 +104,7 @@ where /// nico-api configuration file content #[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct CarbideConfig { /// Socket address for the gRPC API server, used by /// clients and nico-admin-cli to connect. @@ -351,6 +352,13 @@ pub struct CarbideConfig { #[serde(default)] pub site_explorer: SiteExplorerConfig, + /// Deprecated compatibility key. This setting no longer affects runtime + /// behavior; keep accepting it temporarily so existing site files can be + /// migrated without weakening unknown-field validation. + #[doc(hidden)] + #[serde(default, rename = "force_dpu_nic_mode", skip_serializing)] + pub deprecated_force_dpu_nic_mode: Option, + /// The policy to decide whether two VPCs are allowed to peer with each other based on their /// network virtualization type during creation pub vpc_peering_policy: Option, @@ -830,6 +838,7 @@ pub struct CarbideConfig { /// Global admission limits for business requests handled by nico-api. #[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)] +#[serde(deny_unknown_fields)] pub struct ApiAdmissionControlConfig { /// Whether admission control is active. #[serde(default = "default_to_true")] @@ -1045,6 +1054,7 @@ impl CertificatesConfig { } #[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct TracingConfig { /// Whether to enable OTLP tracing. Default: false #[serde(default)] @@ -1096,6 +1106,7 @@ impl CarbideConfig { /// Observability settings shared across all state controllers. #[derive(Clone, Debug, Default, Serialize, Deserialize, PartialEq)] +#[serde(deny_unknown_fields)] pub struct ObservabilityConfig { /// Health alert classifications for which an additional per-object metric /// (`carbide_object_unhealthy_by_classification_count`) is emitted, @@ -1114,6 +1125,7 @@ pub struct ObservabilityConfig { /// the series cost O(fleet) cardinality, so operators opt in and scrape the /// dedicated endpoint at their own cadence. #[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] +#[serde(deny_unknown_fields)] pub struct PerObjectStateMetricsConfig { /// Whether the per-object state metrics endpoint is enabled. #[serde(default)] @@ -1191,6 +1203,7 @@ impl PerObjectStateMetricObjectType { /// One external tool link rendered in the admin web UI's "Tools" /// sidebar. #[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct ToolLink { /// Stable identifier, must be unique within `tools`. Used /// to look up well-known integrations. @@ -1205,6 +1218,7 @@ pub struct ToolLink { /// (`crate::web::logs`). Bounds memory use and the page size served /// to the browser. #[derive(Clone, Debug, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] pub struct LogHistoryConfig { /// Maximum amount of recent log history to retain in memory, in /// MiB. Oldest lines are evicted once the budget is exceeded. @@ -1515,6 +1529,7 @@ fn default_dpf_pf_total_sf_reserved() -> u32 { } #[derive(Clone, Debug, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] pub struct DpfConfig { /// Enables DPF deployment. #[serde(default)] @@ -1685,6 +1700,14 @@ impl<'de> Deserialize<'de> for DpfMandatoryServicesConfig { let configured = BTreeMap::::deserialize(deserializer)?; let mut services = Self::default(); for (name, configured) in configured { + const SERVICE_FIELDS: &[&str] = &[ + "dts", + "doca_hbn", + "dpu_agent", + "dhcp_server", + "fmds", + "otel", + ]; let service = match name.as_str() { "dts" => &mut services.dts, "doca_hbn" => &mut services.doca_hbn, @@ -1692,12 +1715,20 @@ impl<'de> Deserialize<'de> for DpfMandatoryServicesConfig { "dhcp_server" => &mut services.dhcp_server, "fmds" => &mut services.fmds, "otel" => &mut services.otel, - _ => continue, + _ => return Err(serde::de::Error::unknown_field(&name, SERVICE_FIELDS)), }; - *service = Figment::from(Serialized::defaults(std::mem::take(service))) + let merged = Figment::from(Serialized::defaults(std::mem::take(service))) .merge(Serialized::defaults(configured)) - .extract() - .map_err(serde::de::Error::custom)?; + .extract(); + *service = match merged { + Ok(service) => service, + Err(error) => match error.kind { + figment::error::Kind::UnknownField(field, expected) => { + return Err(serde::de::Error::unknown_field(&field, expected)); + } + _ => return Err(serde::de::Error::custom(error)), + }, + }; } Ok(services) } @@ -1772,6 +1803,7 @@ pub struct DpfResolvedMandatoryServicesConfig { /// Configuration for a single Helm-based DPF service. #[derive(Clone, Debug, Default, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] pub struct DpfServiceConfig { /// Name of the Helm service. pub name: String, @@ -1808,6 +1840,7 @@ pub struct DpfServiceConfig { /// `[dpf.deployments.bf3]` block is absent, via `#[serde(default)]` on the /// `bf3` field of [`DpfDeploymentsConfig`]. #[derive(Clone, Debug, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] pub struct DpfDeploymentConfig { /// URL to the BlueField firmware bundle (BFB) for DPU provisioning /// (BF3-class DPUs). Exactly one of `bfb_url` or `bluefield_software` @@ -1862,6 +1895,7 @@ impl Default for DpfDeploymentConfig { /// [`DpfDeploymentConfig::per_psid_deployment_name`] and /// [`DpfDeploymentConfig::per_psid_node_label_key`]). #[derive(Clone, Debug, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] pub struct DpfBlueFieldSoftwareConfig { /// OS ISO URL used by the DPU OS installation flow (`spec.osIso`). Shared /// across all PSIDs. @@ -1890,6 +1924,7 @@ pub struct DpfDeploymentsConfig { } #[derive(Deserialize)] +#[serde(deny_unknown_fields)] struct DpfDeploymentsConfigDef { #[serde(default)] bf3: DpfDeploymentConfig, @@ -2079,6 +2114,7 @@ impl DpfDeploymentsConfig { /// Machine identity (SPIFFE JWT-SVID) configuration. /// Loaded from `[machine_identity]` section in config. #[derive(Clone, Debug, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] pub struct MachineIdentityConfig { /// Master switch. If false, SetTenantIdentityConfiguration and SignMachineIdentity return 503. #[serde(default = "machine_identity_default_enabled")] @@ -2176,6 +2212,7 @@ impl From for model::tenant::TokenDelegationValidationBou /// SPDM (Security Protocol and Data Model) configuration /// for hardware attestation of DPU components. #[derive(Clone, Debug, Default, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] pub struct SpdmConfig { /// Enables SPDM-based hardware attestation. #[serde(default)] @@ -2189,6 +2226,7 @@ pub struct SpdmConfig { /// Fabric Nearest Neighbor (FNN) configuration for L3 VNI-based overlay networking. #[derive(Clone, Debug, Deserialize, Serialize, PartialEq)] +#[serde(deny_unknown_fields)] pub struct FnnConfig { /// Optional FNN configuration for the admin network VPC. #[serde(default)] @@ -2219,6 +2257,7 @@ pub struct FnnConfig { /// A named routing-profile definition whose unset properties use effective /// defaults unless a VPC supplies an inline override. #[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Default)] +#[serde(deny_unknown_fields)] pub struct FnnRoutingProfileConfig { /// These are used for import policies to import routes /// that match these targets. @@ -2407,6 +2446,7 @@ impl From<&FnnRoutingProfileConfig> for rpc::forge::VpcEffectiveRoutingProfile { /// FNN configuration specific to the admin network. #[derive(Clone, Debug, Deserialize, Serialize, PartialEq)] +#[serde(deny_unknown_fields)] pub struct AdminFnnConfig { /// Whether FNN should be applied to the admin network as well. pub enabled: bool, @@ -2666,6 +2706,7 @@ impl MaxConcurrentUpdates { /// NetworkSegmentStateController related config. #[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] +#[serde(deny_unknown_fields)] pub struct NetworkSegmentStateControllerConfig { /// Common state controller configs #[serde(default = "StateControllerConfig::default")] @@ -2699,6 +2740,7 @@ impl Default for NetworkSegmentStateControllerConfig { /// VpcPrefixStateController related config. #[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] +#[serde(deny_unknown_fields)] pub struct VpcPrefixStateControllerConfig { /// Common state controller configs #[serde(default = "StateControllerConfig::default")] @@ -2736,6 +2778,7 @@ impl Default for VpcPrefixStateControllerConfig { /// IbPartitionStateController related config #[derive(Clone, Debug, Default, Serialize, Deserialize, PartialEq)] +#[serde(deny_unknown_fields)] pub struct IbPartitionStateControllerConfig { /// Common state controller configs #[serde(default = "StateControllerConfig::default")] @@ -2744,6 +2787,7 @@ pub struct IbPartitionStateControllerConfig { /// DpaInterfaceStateController related config #[derive(Clone, Debug, Default, Serialize, Deserialize, PartialEq)] +#[serde(deny_unknown_fields)] pub struct DpaInterfaceStateControllerConfig { /// Common state controller configs #[serde(default = "StateControllerConfig::default")] @@ -2752,6 +2796,7 @@ pub struct DpaInterfaceStateControllerConfig { /// PowerShelfStateController related config #[derive(Clone, Debug, Default, Serialize, Deserialize, PartialEq)] +#[serde(deny_unknown_fields)] pub struct PowerShelfStateControllerConfig { /// Common state controller configs #[serde(default = "StateControllerConfig::default")] @@ -2776,6 +2821,7 @@ pub struct PowerShelfStateControllerConfig { /// RackStateController related config #[derive(Clone, Debug, Default, Serialize, Deserialize, PartialEq)] +#[serde(deny_unknown_fields)] pub struct RackStateControllerConfig { /// Common state controller configs #[serde(default = "StateControllerConfig::default")] @@ -2812,6 +2858,7 @@ impl RackStateControllerConfig { /// SwitchStateController related config #[derive(Clone, Debug, Default, Serialize, Deserialize, PartialEq)] +#[serde(deny_unknown_fields)] pub struct SwitchStateControllerConfig { /// Common state controller configs #[serde(default = "StateControllerConfig::default")] @@ -2848,6 +2895,7 @@ impl SwitchStateControllerConfig { /// SpdmStateController related config #[derive(Clone, Debug, Default, Serialize, Deserialize, PartialEq)] +#[serde(deny_unknown_fields)] pub struct SpdmStateControllerConfig { /// Common state controller configs #[serde(default = "StateControllerConfig::default")] @@ -2855,6 +2903,7 @@ pub struct SpdmStateControllerConfig { } #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] +#[serde(deny_unknown_fields)] pub struct InitialObjectsConfig { /// Resource pools that allocate IPs, VNIs, etc. /// Required, but wrapped in `Option` so partial configs @@ -2869,6 +2918,7 @@ pub struct InitialObjectsConfig { /// TLS certificate and key configuration for securing /// gRPC and HTTP connections. #[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct TlsConfig { /// Path to the root CA certificate file for /// validating client certificates. @@ -2906,6 +2956,7 @@ pub enum ListenMode { /// Authentication related configuration #[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct AuthConfig { /// Enable permissive mode in the authorization enforcer (for development). pub permissive_mode: bool, @@ -3035,6 +3086,7 @@ impl<'de> Deserialize<'de> for DpuConfig { { // Create a temporary struct for partial deserialization #[derive(Deserialize)] + #[serde(deny_unknown_fields)] struct PartialDpuConfig { #[serde(default)] bootstrap_ca_source: Option, @@ -3217,6 +3269,7 @@ impl Default for DpuConfig { } #[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct NetworkSecurityGroupConfig { /// The maximum number of unique rules allowed for /// a network security group after rules are expanded. @@ -3248,6 +3301,7 @@ impl Default for NetworkSecurityGroupConfig { /// Configuration for rolling machine updates and /// maintenance windows. #[derive(Clone, Debug, Default, Deserialize, Serialize, PartialEq)] +#[serde(deny_unknown_fields)] pub struct MachineUpdater { /// Time window during which machines may automatically /// reboot for updates. @@ -3309,6 +3363,7 @@ fn default_tenant_routing_profile() -> String { /// which exports TPM-based boot measurement data as /// Prometheus metrics. #[derive(Clone, Debug, Deserialize, Serialize, PartialEq)] +#[serde(deny_unknown_fields)] pub struct MeasuredBootMetricsCollectorConfig { /// Enables the measured boot metrics monitor. When /// disabled, measured boot metrics are not exported. @@ -3515,6 +3570,7 @@ use model::vpc::VpcDefinition; /// topics, and subscribe to `BMS/v1/PUB/Metadata/#` to learn those routing /// targets. #[derive(Clone, Debug, Default, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct DsxExchangeEventBusConfig { /// Enable/disable the DSX Exchange Event Bus. #[serde(default)] @@ -3615,6 +3671,7 @@ const PUBLISH_INTERVAL_MAX: std::time::Duration = std::time::Duration::from_secs /// self-heal. Republished messages reuse the same topic and JSON payload as /// change-driven events, so consumers handle them identically. #[derive(Clone, Debug, Deserialize, Serialize, PartialEq)] +#[serde(deny_unknown_fields)] pub struct PeriodicStateRepublishConfig { /// Enable periodic republishing. Enabled by default whenever the DSX /// Exchange Event Bus itself is enabled. Change-driven publishing is @@ -3691,6 +3748,7 @@ impl PeriodicStateRepublishConfig { /// Auto machine repair plugin related configuration #[derive(Default, Clone, Copy, Debug, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct AutoMachineRepairPluginConfig { /// Whether automatic machine repair mode is enabled #[serde(default)] @@ -3712,6 +3770,7 @@ pub enum VpcPeeringPolicy { } #[derive(Debug, Clone, PartialEq, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct VmaasConfig { /// Allow VFs on instance creation. defaults to true, but will be disabled when /// using SDN to manage the instance network configuration for VMs @@ -3726,6 +3785,7 @@ pub struct VmaasConfig { } #[derive(Debug, Clone, PartialEq, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct HostRepresentorBridgingConfig { /// The HBN/SFC bridge that host-representor patch ports attach to during provisioning. #[serde(default = "default_hbn_bridge")] @@ -3738,6 +3798,7 @@ pub struct HostRepresentorBridgingConfig { } #[derive(Debug, Clone, PartialEq, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct HostInterceptBridging { /// The name of the bridge (e.g., br-host) that will sit between host PF/VF and br-hbn. /// It will be connected to br-hbn or br-sfc. @@ -3799,10 +3860,11 @@ mod tests { use carbide_authn::config::CertComponent; use carbide_network::virtualization::VpcVirtualizationType; use carbide_site_explorer::config::SiteExplorerExploreMode; - use carbide_test_support::Outcome::{Fails, Yields}; + use carbide_test_support::Outcome::*; use carbide_test_support::{Check, check_values, scenarios, value_scenarios}; use chrono::Datelike; use figment::Figment; + use figment::error::Kind; use figment::providers::{Env, Format, Toml}; use health_report::HealthAlertClassification; use libmlx::variables::value::MlxValueType; @@ -4898,6 +4960,7 @@ mod tests { switches_created_per_run: 9, rotate_switch_nvos_credentials: Arc::new(false.into()), dpu_policy: None, + deprecated_force_dpu_nic_mode: None, explore_mode: SiteExplorerExploreMode::NvRedfish, } ); @@ -4980,6 +5043,18 @@ mod tests { ); } + #[test] + fn deserialize_shipped_deployment_config() { + let path = concat!( + env!("CARGO_MANIFEST_DIR"), + "/../../deploy/nico-base/api/config-files/nico-api-config.toml" + ); + Figment::new() + .merge(Toml::file(path)) + .extract::() + .expect("the shipped deployment config must match the strict schema"); + } + #[test] fn deserialize_full_config() { let config: CarbideConfig = Figment::new() @@ -5115,6 +5190,7 @@ mod tests { switches_created_per_run: 9, rotate_switch_nvos_credentials: Arc::new(false.into()), dpu_policy: None, + deprecated_force_dpu_nic_mode: None, explore_mode: SiteExplorerExploreMode::NvRedfish, } ); @@ -5503,6 +5579,7 @@ mod tests { switches_created_per_run: 9, rotate_switch_nvos_credentials: Arc::new(false.into()), dpu_policy: None, + deprecated_force_dpu_nic_mode: None, explore_mode: SiteExplorerExploreMode::NvRedfish, } ); @@ -5693,6 +5770,54 @@ mod tests { }) } + #[test] + #[allow(clippy::result_large_err)] + fn deserialize_unknown_environment_field_is_rejected() { + figment::Jail::expect_with(|jail| { + jail.set_env("CARBIDE_API_UNKNOWN_FIELD", true); + + let error = Figment::new() + .merge(Toml::file(format!("{TEST_DATA_DIR}/min_config.toml"))) + .merge(Env::prefixed("CARBIDE_API_")) + .extract::() + .unwrap_err(); + + assert!(matches!( + &error.kind, + Kind::UnknownField(field, _) if field == "unknown_field" + )); + assert_eq!(error.path, vec!["unknown_field".to_string()]); + Ok(()) + }) + } + + #[test] + #[allow(clippy::result_large_err)] + fn deserialize_unknown_nested_environment_field_is_rejected() { + figment::Jail::expect_with(|jail| { + jail.set_env("CARBIDE_API_SITE_EXPLORER", "{unknown_nested_field=true}"); + + let error = Figment::new() + .merge(Toml::file(format!("{TEST_DATA_DIR}/min_config.toml"))) + .merge(Env::prefixed("CARBIDE_API_")) + .extract::() + .unwrap_err(); + + assert!(matches!( + &error.kind, + Kind::UnknownField(field, _) if field == "unknown_nested_field" + )); + assert_eq!( + error.path, + vec![ + "site_explorer".to_string(), + "unknown_nested_field".to_string() + ] + ); + Ok(()) + }) + } + #[test] fn site_explorer_serde_defaults_match_core_defaults() -> eyre::Result<()> { // Make sure that if we let serde pick the defaults, it matches Default::default(). @@ -5806,12 +5931,10 @@ mod tests { /// Real-world site TOMLs may still carry the now-removed /// `force_dpu_nic_mode` setting (top-level and/or under - /// `[site_explorer]`). serde silently ignores unknown keys, so - /// those files should keep parsing cleanly after the rip-out -- - /// this is the regression guard for that. + /// `[site_explorer]`). Keep that one compatibility exception explicit. #[test] fn legacy_force_dpu_nic_mode_in_toml_still_parses() { - let _config: CarbideConfig = Figment::new() + let config: CarbideConfig = Figment::new() .merge(Toml::file(format!("{TEST_DATA_DIR}/min_config.toml"))) .merge(Toml::string( "force_dpu_nic_mode = false\n\ @@ -5820,6 +5943,147 @@ mod tests { )) .extract() .expect("legacy force_dpu_nic_mode in TOML must still parse"); + + assert_eq!(config.deprecated_force_dpu_nic_mode, Some(false)); + assert_eq!( + config.site_explorer.deprecated_force_dpu_nic_mode, + Some(true) + ); + } + + #[test] + fn carbide_config_rejects_unknown_fields_across_fixed_schema_levels() { + scenarios!( + run = |patch| Figment::new() + .merge(Toml::file(format!("{TEST_DATA_DIR}/min_config.toml"))) + .merge(Toml::string(patch)) + .extract::() + .map(drop) + .map_err(drop); + + "unknown fields are rejected" { + "unknown_top_level = true" => Fails, + "rapid_iterations = true" => Fails, + "nvue_enabled = false" => Fails, + "[site_explorer]\nunknown_site_explorer_field = true" => Fails, + "[tracing]\nunknown_tracing_field = true" => Fails, + "[machine_state_controller]\nunknown_machine_controller_field = true" => Fails, + "[machine_state_controller.controller]\nunknown_controller_field = true" => Fails, + "[auth]\npermissive_mode = true\nunknown_auth_field = true" => Fails, + "[pools.test-pool]\ntype = \"integer\"\nunknown_pool_field = true" => Fails, + "[dpu_config]\nunknown_dpu_field = true" => Fails, + "[fnn]\nunknown_fnn_field = true" => Fails, + "[fnn.routing_profiles.test]\nunknown_routing_profile_field = true" => Fails, + "[host_health]\nunknown_host_health_field = true" => Fails, + "[firmware_global]\nunknown_firmware_field = true" => Fails, + "[machine_validation_config]\nunknown_validation_field = true" => Fails, + "[network_security_group]\nunknown_nsg_field = true" => Fails, + "[machine_identity]\nunknown_identity_field = true" => Fails, + "[spdm]\nunknown_spdm_field = true" => Fails, + "[component_manager]\nunknown_component_manager_field = true" => Fails, + "[dpa_config]\nunknown_dpa_field = true" => Fails, + "[dsx_exchange_event_bus]\nunknown_event_bus_field = true" => Fails, + "[host_models.test-model]\nvendor = \"Dell\"\nmodel = \"test\"\ncomponents = {}\nunknown_host_model_field = true" => Fails, + "[supernic_firmware_profiles.part-number.psid]\npart_number = \"part-number\"\npsid = \"psid\"\nversion = \"1.0\"\nfirmware_url = \"https://example.com/fw.bin\"\nunknown_supernic_field = true" => Fails, + "[mlx-config-profiles.test]\nname = \"test\"\nregistry_name = \"mlx_generic\"\nconfig = {}\nunknown_mlx_profile_field = true" => Fails, + } + + "dynamic map keys and valid partial sections remain accepted" { + "[pools.an-arbitrary-pool-name]\ntype = \"integer\"" => Yields(()), + "[tracing]\nenabled = true" => Yields(()), + } + ); + } + + #[test] + fn network_security_policy_override_rejects_unknown_rule_fields() { + const VALID_POLICY: &str = r#" +[[network_security_group.policy_overrides]] +src_net = { Prefix = "0.0.0.0/0" } +dst_net = { Prefix = "0.0.0.0/0" } +direction = "Ingress" +ipv6 = false +protocol = "Any" +action = "Deny" +priority = 1 +"#; + + let config = Figment::new() + .merge(Toml::file(format!("{TEST_DATA_DIR}/min_config.toml"))) + .merge(Toml::string(VALID_POLICY)) + .extract::() + .expect("valid policy override parses"); + assert_eq!(config.network_security_group.policy_overrides.len(), 1); + + let invalid_policy = + VALID_POLICY.replace("priority = 1", "priority = 1\nmisspelled_priority = 2"); + let error = Figment::new() + .merge(Toml::file(format!("{TEST_DATA_DIR}/min_config.toml"))) + .merge(Toml::string(&invalid_policy)) + .extract::() + .unwrap_err(); + assert!(matches!( + &error.kind, + Kind::UnknownField(field, _) if field == "misspelled_priority" + )); + assert_eq!( + error.path, + vec![ + "network_security_group".to_string(), + "policy_overrides".to_string(), + "0".to_string(), + "misspelled_priority".to_string() + ] + ); + } + + #[test] + fn unknown_field_error_identifies_key_and_section() { + let error = Figment::new() + .merge(Toml::file(format!("{TEST_DATA_DIR}/min_config.toml"))) + .merge(Toml::string( + "[site_explorer]\nunknown_site_explorer_field = true", + )) + .extract::() + .unwrap_err(); + + assert!(matches!( + &error.kind, + Kind::UnknownField(field, _) if field == "unknown_site_explorer_field" + )); + assert_eq!( + error.path, + vec![ + "site_explorer".to_string(), + "unknown_site_explorer_field".to_string() + ] + ); + assert!( + error + .to_string() + .contains("site_explorer.unknown_site_explorer_field") + ); + } + + #[test] + fn initial_objects_config_rejects_unknown_fields() { + scenarios!( + run = |input| Figment::new() + .merge(Toml::string(input)) + .extract::() + .map(drop) + .map_err(drop); + + "unknown fields are rejected" { + "unknown_top_level = true" => Fails, + "[pools.test-pool]\ntype = \"integer\"\nunknown_pool_field = true" => Fails, + "[networks.admin]\ntype = \"admin\"\nprefix = \"172.20.0.0/24\"\ngateway = \"172.20.0.1\"\nmtu = 9000\nreserve_first = 5\nunknown_network_field = true" => Fails, + } + + "dynamic object names remain accepted" { + "[pools.an-arbitrary-pool-name]\ntype = \"integer\"" => Yields(()), + } + ); } #[test] diff --git a/crates/api-core/src/cfg/load.rs b/crates/api-core/src/cfg/load.rs index 22534424b7..f35506754e 100644 --- a/crates/api-core/src/cfg/load.rs +++ b/crates/api-core/src/cfg/load.rs @@ -109,6 +109,16 @@ pub fn parse_carbide_config( config.config_ctx = Some(merged_config); + if config.deprecated_force_dpu_nic_mode.is_some() + || config.site_explorer.deprecated_force_dpu_nic_mode.is_some() + { + tracing::warn!( + config_key = "force_dpu_nic_mode", + replacement = "site_explorer.dpu_policy", + "Ignoring deprecated configuration key" + ); + } + for (label, _) in config .host_models .iter() @@ -205,3 +215,54 @@ pub fn parse_carbide_config( tracing::trace!(config = ?config.redacted(), "Carbide config"); Ok(Arc::new(config)) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + #[allow(clippy::result_large_err)] + fn unknown_site_override_field_reports_key_and_source_file() { + figment::Jail::expect_with(|jail| { + jail.create_file( + "base.toml", + r#" + database_url = "postgres://test" + listen = "[::]:1081" + asn = 1 + "#, + )?; + jail.create_file( + "site.toml", + "[site_explorer]\nunknown_site_override_field = true", + )?; + + let report = parse_carbide_config(Path::new("base.toml"), Some(Path::new("site.toml"))) + .unwrap_err(); + let error = report + .downcast_ref::() + .expect("configuration error retains the Figment source"); + + assert!(matches!( + &error.kind, + figment::error::Kind::UnknownField(field, _) + if field == "unknown_site_override_field" + )); + assert_eq!( + error.path, + vec![ + "site_explorer".to_string(), + "unknown_site_override_field".to_string() + ] + ); + let source_path = error + .metadata + .as_ref() + .and_then(|metadata| metadata.source.as_ref()) + .and_then(|source| source.file_path()) + .expect("unknown site key is attributed to its source file"); + assert!(source_path.ends_with("site.toml")); + Ok(()) + }) + } +} diff --git a/crates/api-core/src/cfg/test_data/full_config.toml b/crates/api-core/src/cfg/test_data/full_config.toml index 5a92610a04..890d7f6bc4 100644 --- a/crates/api-core/src/cfg/test_data/full_config.toml +++ b/crates/api-core/src/cfg/test_data/full_config.toml @@ -9,7 +9,6 @@ asn = 123 dhcp_servers = ["1.2.3.4", "5.6.7.8"] ntp_servers = ["10.20.30.40", "50.60.70.80"] route_servers = ["9.10.11.12"] -rapid_iterations = false initial_domain_name = "forge.local" initial_dpu_agent_upgrade_policy = "off" machine_update_run_interval = 60 @@ -201,9 +200,6 @@ known_firmware = [ { version = "2.0", filename = "/mnt/persistence/fw/dell/uefi/r750_1.1.fw", url = "https://letmegooglethat.com/?q=dell750uefi", checksum = "f4d4e0954b5635e6c9a8c79ebaa95218", default = true }, ] -[multi_dpu] -enabled = false - [measured_boot_collector] enabled = false run_interval = "555s" diff --git a/crates/api-core/src/cfg/test_data/full_config_post_migration.toml b/crates/api-core/src/cfg/test_data/full_config_post_migration.toml index ec360162b0..cc7bee5f1b 100644 --- a/crates/api-core/src/cfg/test_data/full_config_post_migration.toml +++ b/crates/api-core/src/cfg/test_data/full_config_post_migration.toml @@ -5,7 +5,6 @@ max_database_connections = 1222 asn = 123 dhcp_servers = ["1.2.3.4", "5.6.7.8"] route_servers = ["9.10.11.12"] -rapid_iterations = false initial_domain_name = "forge.local" initial_dpu_agent_upgrade_policy = "off" machine_update_run_interval = 60 @@ -154,9 +153,6 @@ known_firmware = [ { version = "2.0", filename = "/mnt/persistence/fw/dell/uefi/r750_1.1.fw", url = "https://letmegooglethat.com/?q=dell750uefi", checksum = "f4d4e0954b5635e6c9a8c79ebaa95218", default = true }, ] -[multi_dpu] -enabled = false - [measured_boot_collector] enabled = false run_interval = "555s" diff --git a/crates/api-core/src/cfg/test_data/site_config.toml b/crates/api-core/src/cfg/test_data/site_config.toml index 39dace90cd..8df6043632 100644 --- a/crates/api-core/src/cfg/test_data/site_config.toml +++ b/crates/api-core/src/cfg/test_data/site_config.toml @@ -1,6 +1,5 @@ asn = 777 dhcp_servers = ["99.101.102.103"] -rapid_iterations = true max_database_connections = 1333 max_find_by_ids = 50 dpu_network_monitor_pinger_type = "OobNetBind" diff --git a/crates/api-core/src/machine_update_manager/dpu_nic_firmware.rs b/crates/api-core/src/machine_update_manager/dpu_nic_firmware.rs index 6acf5f76e9..aeb1a5bfe0 100644 --- a/crates/api-core/src/machine_update_manager/dpu_nic_firmware.rs +++ b/crates/api-core/src/machine_update_manager/dpu_nic_firmware.rs @@ -41,7 +41,7 @@ use crate::{CarbideResult, DatabaseError}; /// to ensure that DPU NIC firmware matches the expected version of the carbide release. /// /// Config used from [CarbideConfig](crate::cfg::CarbideConfig) -/// * `dpu_nic_firmware_update_version` the version of the DPU NIC firmware that is expected to be running on the DPU. +/// * `dpu_config.dpu_nic_firmware_update_versions` lists the accepted DPU NIC firmware versions. /// /// Note that if the version does not match in either direction, the DPU will be updated. pub(crate) struct DpuNicFirmwareUpdate { diff --git a/crates/api-core/src/test_support/default_config.rs b/crates/api-core/src/test_support/default_config.rs index f1042a291b..00d3257a6b 100644 --- a/crates/api-core/src/test_support/default_config.rs +++ b/crates/api-core/src/test_support/default_config.rs @@ -202,6 +202,7 @@ pub fn get() -> CarbideConfig { create_machines: Arc::new(false.into()), ..Default::default() }, + deprecated_force_dpu_nic_mode: None, vpc_peering_policy: Some(VpcPeeringPolicy::Exclusive), vpc_peering_policy_on_existing: None, attestation_enabled: false, diff --git a/crates/api-core/src/tests/common/api_fixtures/mod.rs b/crates/api-core/src/tests/common/api_fixtures/mod.rs index b5097438bd..c3d5c3a589 100644 --- a/crates/api-core/src/tests/common/api_fixtures/mod.rs +++ b/crates/api-core/src/tests/common/api_fixtures/mod.rs @@ -1748,6 +1748,7 @@ pub(in crate::tests) async fn create_test_env_with_overrides( switches_created_per_run: 1, rotate_switch_nvos_credentials: Arc::new(false.into()), dpu_policy: None, + deprecated_force_dpu_nic_mode: None, // Tests use MockEndpointExplorer. So this doesn't affect anything. explore_mode: SiteExplorerExploreMode::NvRedfish, }, diff --git a/crates/api-model/src/firmware.rs b/crates/api-model/src/firmware.rs index ec9eb0c353..41e86dc9eb 100644 --- a/crates/api-model/src/firmware.rs +++ b/crates/api-model/src/firmware.rs @@ -51,6 +51,7 @@ impl From for DesiredFirmwareVersions { } #[derive(Clone, Debug, Deserialize, Serialize, Default)] +#[serde(deny_unknown_fields)] pub struct Firmware { pub vendor: bmc_vendor::BMCVendor, pub model: String, @@ -224,6 +225,7 @@ impl FirmwareComponentType { } #[derive(Clone, Debug, Deserialize, Serialize, Default)] +#[serde(deny_unknown_fields)] pub struct FirmwareComponent { #[serde(with = "serde_regex")] pub current_version_reported_as: Option, @@ -233,6 +235,7 @@ pub struct FirmwareComponent { } #[derive(Clone, Debug, Deserialize, Serialize, Default)] +#[serde(deny_unknown_fields)] pub struct FirmwareEntry { pub version: String, pub mandatory_upgrade_from_priority: Option, @@ -273,6 +276,7 @@ pub struct FirmwareFileArtifact { } #[derive(Deserialize)] +#[serde(deny_unknown_fields)] struct FirmwareFileArtifactWire { #[serde(default)] filename: Option, @@ -314,6 +318,7 @@ fn firmware_file_artifact_location_is_set(value: &Option) -> bool { } #[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct ScoutConfig { /// Legacy script metadata accepted for backwards-compatible config parsing. /// Scout script selection is inferred from the PXE script registry. diff --git a/crates/api-model/src/machine/mod.rs b/crates/api-model/src/machine/mod.rs index 738fa46a91..e04fb6a19c 100644 --- a/crates/api-model/src/machine/mod.rs +++ b/crates/api-model/src/machine/mod.rs @@ -3173,6 +3173,7 @@ impl Display for PowerState { } #[derive(Clone, Copy, Debug, Serialize, Deserialize, PartialEq)] +#[serde(deny_unknown_fields)] pub struct HostHealthConfig { /// Whether or not to use hardware health reports in aggregate health reports /// and for restricting state transitions. diff --git a/crates/api-model/src/network_security_group/mod.rs b/crates/api-model/src/network_security_group/mod.rs index 2bfd1868aa..0dc9accf43 100644 --- a/crates/api-model/src/network_security_group/mod.rs +++ b/crates/api-model/src/network_security_group/mod.rs @@ -194,6 +194,7 @@ pub enum NetworkSecurityGroupRuleNet { /// single rule that will be applied on a DPU to restrict /// traffic. #[derive(Debug, Clone, Deserialize, Serialize, PartialEq)] +#[serde(deny_unknown_fields)] pub struct NetworkSecurityGroupRule { pub id: Option, pub src_net: NetworkSecurityGroupRuleNet, diff --git a/crates/api-model/src/network_segment/mod.rs b/crates/api-model/src/network_segment/mod.rs index 5a28a6ca3b..f725a2a3d4 100644 --- a/crates/api-model/src/network_segment/mod.rs +++ b/crates/api-model/src/network_segment/mod.rs @@ -77,6 +77,7 @@ pub enum NetworkSegmentDeletionState { // How we specifiy a network segment in the config file #[derive(Debug, Deserialize, Serialize, Clone, PartialEq, Eq)] +#[serde(deny_unknown_fields)] pub struct NetworkDefinition { #[serde(rename = "type")] pub segment_type: NetworkDefinitionSegmentType, diff --git a/crates/api-model/src/rack_type.rs b/crates/api-model/src/rack_type.rs index 9da4d7f58d..8ead9c2565 100644 --- a/crates/api-model/src/rack_type.rs +++ b/crates/api-model/src/rack_type.rs @@ -266,6 +266,7 @@ impl fmt::Display for RackCapabilityType { /// RackCapabilityCompute describes the expected compute tray capability /// for a rack type. #[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] pub struct RackCapabilityCompute { /// Model name of the compute tray (e.g. "GB200"). #[serde(default)] @@ -298,6 +299,7 @@ pub struct RackCapabilityCompute { /// RackCapabilitySwitch describes the expected switch capability /// for a rack type. #[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] pub struct RackCapabilitySwitch { /// Model name of the switch. #[serde(default)] @@ -330,6 +332,7 @@ pub struct RackCapabilitySwitch { /// RackCapabilityPowerShelf describes the expected power shelf capability /// for a rack type. #[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] pub struct RackCapabilityPowerShelf { /// Model name of the power shelf. #[serde(default)] @@ -363,6 +366,7 @@ pub struct RackCapabilityPowerShelf { /// capabilities. It describes what a rack should contain in terms of /// compute trays, switches, and power shelves. #[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] pub struct RackCapabilitiesSet { pub compute: RackCapabilityCompute, pub switch: RackCapabilitySwitch, @@ -379,6 +383,7 @@ pub struct RackCapabilitiesSet { /// uses it as the default firmware request for the profile's compute and switch /// inventory. #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] pub struct RackFirmwareObjectConfig { /// URL from which rack ingestion fetches the SOT JSON document. pub url: url::Url, @@ -404,6 +409,7 @@ impl RackFirmwareObjectConfig { /// capabilities for a class of rack. The profile is referenced by name /// (the map key in the config file) from expected racks and rack configs. #[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] pub struct RackProfile { /// Product family used for product-level component behavior. #[serde(default)] diff --git a/crates/api-model/src/resource_pool/define.rs b/crates/api-model/src/resource_pool/define.rs index 34b7ec7f4b..a2cf604508 100644 --- a/crates/api-model/src/resource_pool/define.rs +++ b/crates/api-model/src/resource_pool/define.rs @@ -17,6 +17,7 @@ use serde::{Deserialize, Serialize}; #[derive(Debug, Deserialize, Serialize, Clone, PartialEq, Eq)] +#[serde(deny_unknown_fields)] pub struct ResourcePoolDef { #[serde(default)] pub ranges: Vec, @@ -29,6 +30,7 @@ pub struct ResourcePoolDef { } #[derive(Debug, Deserialize, Serialize, Clone, PartialEq, Eq)] +#[serde(deny_unknown_fields)] pub struct Range { pub start: String, pub end: String, diff --git a/crates/api-model/src/vpc/mod.rs b/crates/api-model/src/vpc/mod.rs index 639868165b..388a7f6b84 100644 --- a/crates/api-model/src/vpc/mod.rs +++ b/crates/api-model/src/vpc/mod.rs @@ -70,6 +70,7 @@ pub struct Vpc { } #[derive(Debug, Deserialize, Serialize, Clone, PartialEq, Eq)] +#[serde(deny_unknown_fields)] pub struct VpcDefinition { pub organization_id: Option, pub network_virtualization_type: VpcVirtualizationType, diff --git a/crates/api-model/src/vpc/routing_profile.rs b/crates/api-model/src/vpc/routing_profile.rs index c83829d4eb..239784de73 100644 --- a/crates/api-model/src/vpc/routing_profile.rs +++ b/crates/api-model/src/vpc/routing_profile.rs @@ -20,6 +20,7 @@ use serde::{Deserialize, Serialize}; /// A BGP route target used in FNN VRF import/export policies. #[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)] +#[serde(deny_unknown_fields)] pub struct RouteTargetConfig { /// Autonomous System Number component of the route target. #[serde(default)] @@ -32,6 +33,7 @@ pub struct RouteTargetConfig { /// An entry used by a DPU prefix-list policy. #[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)] +#[serde(deny_unknown_fields)] pub struct PrefixFilterPolicyEntry { /// Prefix matched by the policy. pub prefix: IpNetwork, @@ -44,6 +46,7 @@ pub struct PrefixFilterPolicyEntry { /// because VPCs cannot override the base profile's allocation and access /// controls. #[derive(Clone, Debug, Default, Deserialize, Serialize, PartialEq, Eq)] +#[serde(deny_unknown_fields)] pub struct VpcRoutingProfileOverrides { pub route_target_imports: Option>, pub route_targets_on_exports: Option>, diff --git a/crates/api-test-helper/src/api_server.rs b/crates/api-test-helper/src/api_server.rs index fa494a42a7..b61b1f3571 100644 --- a/crates/api-test-helper/src/api_server.rs +++ b/crates/api-test-helper/src/api_server.rs @@ -190,13 +190,6 @@ pub async fn start( mtu = 1490 reserve_first = 0 - [dpu_nic_firmware_update_version] - product_x = "v1" - - [ib_fabric_monitor] - enabled = true - run_interval = "10s" - [site_explorer] enabled = true run_interval = "1s" @@ -204,7 +197,6 @@ pub async fn start( explorations_per_run = 90 create_machines = true machines_created_per_run = 30 - allow_proxy_to_unknown_host = false {bmc_proxy_cfg} reset_rate_limit = "3600s" @@ -293,9 +285,6 @@ pub async fn start( enabled = true vpc_vni = 60100 - [multi_dpu] - enabled = false - [host_health] hardware_health_reports = "Disabled" diff --git a/crates/authn/src/config.rs b/crates/authn/src/config.rs index 480b571418..08c6bb802c 100644 --- a/crates/authn/src/config.rs +++ b/crates/authn/src/config.rs @@ -23,6 +23,7 @@ use crate::SpiffeContext; use crate::spiffe_id::{SpiffeIdError, TrustDomain}; #[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct TrustConfig { /// The SPIFFE trust domain which client certs must adhere to pub spiffe_trust_domain: String, @@ -58,6 +59,7 @@ pub enum CertComponent { } #[derive(Debug, Clone, Deserialize, Serialize, Default)] +#[serde(deny_unknown_fields)] pub struct AllowedCertCriteria { /// These components of the cert must equal the given values to be approved pub required_equals: HashMap, diff --git a/crates/component-manager/src/config.rs b/crates/component-manager/src/config.rs index 1b3b1e75a5..b3f2539658 100644 --- a/crates/component-manager/src/config.rs +++ b/crates/component-manager/src/config.rs @@ -9,6 +9,7 @@ use crate::nv_switch_manager::Backend as NvSwitchBackend; use crate::power_shelf_manager::Backend as PowerShelfBackend; #[derive(Debug, Clone, Default, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct ComponentManagerConfig { #[serde(default)] pub nv_switch_backend: NvSwitchBackend, @@ -128,6 +129,7 @@ pub fn effective_nmx_cluster_switch_mtls_services( } #[derive(Debug, Clone, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct BackendEndpointConfig { pub url: String, #[serde(default)] @@ -140,6 +142,7 @@ pub struct BackendEndpointConfig { /// containing `ca.crt`, `tls.crt`, and `tls.key`. Alternatively, each /// path can be set individually. #[derive(Debug, Clone, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct BackendTlsConfig { /// Directory containing `ca.crt`, `tls.crt`, `tls.key`. /// Individual path fields override files from this directory. diff --git a/crates/dpa-manager/src/config.rs b/crates/dpa-manager/src/config.rs index e63fef32e3..720c92f828 100644 --- a/crates/dpa-manager/src/config.rs +++ b/crates/dpa-manager/src/config.rs @@ -44,6 +44,7 @@ pub enum MqttAuthMode { /// OAuth2 configuration for MQTT broker authentication. #[derive(Clone, Debug, Deserialize, Serialize, PartialEq)] +#[serde(deny_unknown_fields)] pub struct MqttOAuth2Config { /// OAuth2 token endpoint URL. pub token_url: String, @@ -78,6 +79,7 @@ impl MqttOAuth2Config { /// MQTT authentication configuration shared by DPA and DSX event bus. #[derive(Clone, Debug, Default, Deserialize, Serialize, PartialEq)] +#[serde(deny_unknown_fields)] pub struct MqttAuthConfig { /// Authentication mechanism to use for MQTT connections. #[serde(default)] @@ -91,6 +93,7 @@ pub struct MqttAuthConfig { /// Enables DPA, and specifies basic network settings. /// The VNI to be used by DPA will be the same as the parent VPC. #[derive(Clone, Debug, Deserialize, Serialize, PartialEq)] +#[serde(deny_unknown_fields)] pub struct DpaConfig { /// Global enable/disable of Cluster Interconnect Network. #[serde(default)] diff --git a/crates/dpf/src/types.rs b/crates/dpf/src/types.rs index 7a84ff7da6..fee32bd1bc 100644 --- a/crates/dpf/src/types.rs +++ b/crates/dpf/src/types.rs @@ -139,6 +139,7 @@ impl Default for InitDpfResourcesConfig { } #[derive(Clone, Debug, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] pub struct DpfProxyDetails { pub https_proxy: String, #[serde(default)] diff --git a/crates/ib-fabric/src/config.rs b/crates/ib-fabric/src/config.rs index 8b8b9bf195..b2bd239dba 100644 --- a/crates/ib-fabric/src/config.rs +++ b/crates/ib-fabric/src/config.rs @@ -24,6 +24,7 @@ const MAX_IB_PARTITION_PER_TENANT: i32 = 31; /// InfiniBand fabric manager configuration. #[derive(Clone, Debug, Deserialize, Serialize, PartialEq)] +#[serde(deny_unknown_fields)] pub struct IBFabricConfig { /// Maximum InfiniBand partitions per tenant (1-31). #[serde( @@ -141,6 +142,7 @@ impl IBFabricConfig { /// Settings related to an IB fabric #[derive(Debug, Deserialize, Serialize, Clone, PartialEq, Eq)] +#[serde(deny_unknown_fields)] pub struct IbFabricDefinition { /// UFM endpoint address /// These need to be fully qualified, e.g. https://1.2.3.4:443 diff --git a/crates/libmlx/src/firmware/config.rs b/crates/libmlx/src/firmware/config.rs index 046598962c..01671540f4 100644 --- a/crates/libmlx/src/firmware/config.rs +++ b/crates/libmlx/src/firmware/config.rs @@ -30,7 +30,7 @@ use rpc::protos::mlx_device::{ FirmwareFlasherProfile as FirmwareFlasherProfilePb, FirmwareSpec as FirmwareSpecPb, FlashOptions as FlashOptionsPb, FlashSpec as FlashSpecPb, }; -use serde::{Deserialize, Serialize}; +use serde::{Deserialize, Deserializer, Serialize}; use crate::firmware::credentials::Credentials; use crate::firmware::error::{FirmwareError, FirmwareResult}; @@ -177,7 +177,7 @@ impl Default for FlashOptions { // version = "32.43.1014" // firmware_url = "https://..." // reset = true -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize)] pub struct FirmwareFlasherProfile { #[serde(flatten)] pub firmware_spec: FirmwareSpec, @@ -187,6 +187,61 @@ pub struct FirmwareFlasherProfile { pub flash_options: FlashOptions, } +impl<'de> Deserialize<'de> for FirmwareFlasherProfile { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + // `deny_unknown_fields` cannot be combined with `flatten`, so use one + // strict flat representation and rebuild the three public sections. + #[derive(Deserialize)] + #[serde(deny_unknown_fields)] + struct FlatFirmwareFlasherProfile { + part_number: String, + psid: String, + version: String, + firmware_url: String, + firmware_credentials: Option, + device_conf_url: Option, + device_conf_credentials: Option, + #[serde(default)] + verify_from_cache: bool, + cache_dir: Option, + #[serde(default)] + verify_image: bool, + #[serde(default)] + verify_version: bool, + #[serde(default)] + reset: bool, + #[serde(default = "default_reset_level")] + reset_level: u8, + } + + let profile = FlatFirmwareFlasherProfile::deserialize(deserializer)?; + Ok(Self { + firmware_spec: FirmwareSpec { + part_number: profile.part_number, + psid: profile.psid, + version: profile.version, + }, + flash_spec: FlashSpec { + firmware_url: profile.firmware_url, + firmware_credentials: profile.firmware_credentials, + device_conf_url: profile.device_conf_url, + device_conf_credentials: profile.device_conf_credentials, + verify_from_cache: profile.verify_from_cache, + cache_dir: profile.cache_dir, + }, + flash_options: FlashOptions { + verify_image: profile.verify_image, + verify_version: profile.verify_version, + reset: profile.reset, + reset_level: profile.reset_level, + }, + }) + } +} + impl FirmwareFlasherProfile { // from_file reads a FirmwareFlasherProfile from a TOML file. pub fn from_file(path: impl AsRef) -> FirmwareResult { @@ -480,4 +535,18 @@ verify_version = true assert!(profile.flash_options.verify_version); assert_eq!(profile.flash_options.reset_level, 3); // default } + + #[test] + fn profile_toml_rejects_unknown_fields() { + let toml_str = r#" +part_number = "900-9D3B4-00CV-TA0" +psid = "MT_0000000884" +version = "32.43.1014" +firmware_url = "https://artifacts.nvidia.com/fw.bin" +firmware_urll = "https://typo.example.com/fw.bin" +"#; + + let error = FirmwareFlasherProfile::from_toml(toml_str).unwrap_err(); + assert!(error.to_string().contains("firmware_urll")); + } } diff --git a/crates/libmlx/src/firmware/credentials.rs b/crates/libmlx/src/firmware/credentials.rs index 7d02cc4ca2..17e649a881 100644 --- a/crates/libmlx/src/firmware/credentials.rs +++ b/crates/libmlx/src/firmware/credentials.rs @@ -46,7 +46,7 @@ use crate::firmware::error::{FirmwareError, FirmwareResult}; // [firmware_credentials] // type = "ssh_agent" #[derive(Debug, Clone, Serialize, Deserialize)] -#[serde(tag = "type", rename_all = "snake_case")] +#[serde(deny_unknown_fields, tag = "type", rename_all = "snake_case")] pub enum Credentials { // BearerToken uses an Authorization: Bearer header. BearerToken { diff --git a/crates/libmlx/src/profile/serialization.rs b/crates/libmlx/src/profile/serialization.rs index 674320a97e..1342337957 100644 --- a/crates/libmlx/src/profile/serialization.rs +++ b/crates/libmlx/src/profile/serialization.rs @@ -32,6 +32,7 @@ use crate::profile::profile::MlxConfigProfile; // Serializable representation of an MLX configuration profile. // This is the format used for YAML/JSON serialization. #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] pub struct SerializableProfile { // Profile name for identification and documentation pub name: String, diff --git a/crates/machine-controller/src/config/bom_validation.rs b/crates/machine-controller/src/config/bom_validation.rs index 8b3c67c46b..46b2cd29a9 100644 --- a/crates/machine-controller/src/config/bom_validation.rs +++ b/crates/machine-controller/src/config/bom_validation.rs @@ -21,6 +21,7 @@ use serde::{Deserialize, Serialize}; /// MachineValidation related configuration #[derive(Default, Clone, Copy, Debug, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct BomValidationConfig { /// Whether BOM Validation is enabled #[serde(default)] diff --git a/crates/machine-controller/src/config/controller.rs b/crates/machine-controller/src/config/controller.rs index aaa227b87c..e6bb7206ef 100644 --- a/crates/machine-controller/src/config/controller.rs +++ b/crates/machine-controller/src/config/controller.rs @@ -38,6 +38,7 @@ where /// MachineStateController related config. #[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] +#[serde(deny_unknown_fields)] pub struct MachineStateControllerConfig { /// Common state controller configs #[serde(default = "StateControllerConfig::default")] diff --git a/crates/machine-controller/src/config/firmware_global.rs b/crates/machine-controller/src/config/firmware_global.rs index aeae9575ce..6ea7fad7c5 100644 --- a/crates/machine-controller/src/config/firmware_global.rs +++ b/crates/machine-controller/src/config/firmware_global.rs @@ -25,6 +25,7 @@ use serde::{Deserialize, Serialize}; /// Global firmware management settings controlling /// update policies, concurrency, and retry behavior. #[derive(Clone, Debug, Deserialize, Serialize, PartialEq)] +#[serde(deny_unknown_fields)] pub struct FirmwareGlobal { /// Enables automatic host firmware updates via the /// background firmware manager. diff --git a/crates/machine-controller/src/config/machine_validation.rs b/crates/machine-controller/src/config/machine_validation.rs index 56cd6cd1ed..1200321103 100644 --- a/crates/machine-controller/src/config/machine_validation.rs +++ b/crates/machine-controller/src/config/machine_validation.rs @@ -38,6 +38,7 @@ pub enum MachineValidationTestSelectionMode { /// latency, SSD I/O, etc.) run after ingestion to verify /// hardware health. #[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct MachineValidationConfig { /// Enables machine validation testing. #[serde(default)] @@ -79,6 +80,7 @@ pub struct MachineValidationConfig { /// ] /// ``` #[derive(Default, Clone, Debug, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct MachineValidationTestConfig { /// Unique test identifier (e.g., "MmMemLatency"). pub id: String, diff --git a/crates/machine-controller/src/config/mod.rs b/crates/machine-controller/src/config/mod.rs index 73f86872a7..f8f4e3c65e 100644 --- a/crates/machine-controller/src/config/mod.rs +++ b/crates/machine-controller/src/config/mod.rs @@ -92,6 +92,7 @@ impl MachineStateHandlerSiteConfig { /// A UTC time window defined by a start and end timestamp. #[derive(Clone, Debug, Deserialize, Serialize, PartialEq)] +#[serde(deny_unknown_fields)] pub struct TimePeriod { /// Start of the time window (UTC). pub start: chrono::DateTime, diff --git a/crates/machine-controller/src/config/power_manager.rs b/crates/machine-controller/src/config/power_manager.rs index bb3eecc162..18ee1f3620 100644 --- a/crates/machine-controller/src/config/power_manager.rs +++ b/crates/machine-controller/src/config/power_manager.rs @@ -23,6 +23,7 @@ use serde::{Deserialize, Serialize}; /// Power management configuration controlling retry /// intervals and reboot timing. #[derive(Clone, Debug, Default, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] pub struct PowerManagerOptions { /// Master switch to enable or disable power /// management. diff --git a/crates/nras/src/lib.rs b/crates/nras/src/lib.rs index 1c763a4807..e880110d8b 100644 --- a/crates/nras/src/lib.rs +++ b/crates/nras/src/lib.rs @@ -31,6 +31,7 @@ pub use parser::Parser; use serde::{Deserialize, Serialize}; #[derive(Clone, Debug, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] pub struct Config { pub nras_url: String, pub nras_gpu_url_suffix: String, diff --git a/crates/nvlink-manager/src/config.rs b/crates/nvlink-manager/src/config.rs index 2019f2115b..2ee3e6da8f 100644 --- a/crates/nvlink-manager/src/config.rs +++ b/crates/nvlink-manager/src/config.rs @@ -20,6 +20,7 @@ use duration_str::deserialize_duration; use serde::{Deserialize, Serialize}; #[derive(Clone, Debug, Deserialize, Serialize, PartialEq)] +#[serde(deny_unknown_fields)] pub struct NvLinkConfig { /// Enables NvLink partitioning. #[serde(default)] diff --git a/crates/rack-controller/src/config.rs b/crates/rack-controller/src/config.rs index 17da0170f2..470b650417 100644 --- a/crates/rack-controller/src/config.rs +++ b/crates/rack-controller/src/config.rs @@ -48,6 +48,7 @@ pub struct RackConfig { /// run_interval = "60s" /// ``` #[derive(Default, Clone, Debug, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct RackValidationConfig { /// Enables rack validation testing. #[serde(default)] @@ -69,6 +70,7 @@ impl RackValidationConfig { /// Rack Manager Service (RMS) configuration for API connectivity and mTLS. #[derive(Clone, Debug, Default, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct RmsConfig { /// URL of the RMS API for rack-level firmware upgrades and power sequencing. pub api_url: Option, diff --git a/crates/site-explorer/src/config.rs b/crates/site-explorer/src/config.rs index ded94eae3a..bbd9777939 100644 --- a/crates/site-explorer/src/config.rs +++ b/crates/site-explorer/src/config.rs @@ -29,39 +29,9 @@ use duration_str::{deserialize_duration, deserialize_duration_chrono}; use model::expected_machine::HostDpuPolicy; use serde::{Deserialize, Deserializer, Serialize, Serializer}; -#[derive(Deserialize, Serialize)] -struct HostDpuPolicyConfigFields { - #[serde(default)] - dpu_policy: Option, - #[serde(default, skip_serializing)] - dpu_mode: Option, -} - -fn deserialize_host_dpu_policy<'de, D>(deserializer: D) -> Result, D::Error> -where - D: Deserializer<'de>, -{ - let fields = HostDpuPolicyConfigFields::deserialize(deserializer)?; - - Ok(fields.dpu_policy.or(fields.dpu_mode)) -} - -fn serialize_host_dpu_policy( - dpu_policy: &Option, - serializer: S, -) -> Result -where - S: Serializer, -{ - HostDpuPolicyConfigFields { - dpu_policy: *dpu_policy, - dpu_mode: None, - } - .serialize(serializer) -} - /// SiteExplorer related configuration for hardware discovery and ingestion. #[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct SiteExplorerConfig { /// Whether SiteExplorer is enabled. Dynamically toggleable at runtime via SetDynamicConfig. #[serde( @@ -203,13 +173,14 @@ pub struct SiteExplorerConfig { /// /// The legacy `dpu_mode` field and values remain accepted during /// deserialization. - #[serde( - flatten, - deserialize_with = "deserialize_host_dpu_policy", - serialize_with = "serialize_host_dpu_policy" - )] + #[serde(default, alias = "dpu_mode")] pub dpu_policy: Option, + /// Deprecated compatibility key. This setting is ignored; use `dpu_policy`. + #[doc(hidden)] + #[serde(default, rename = "force_dpu_nic_mode", skip_serializing)] + pub deprecated_force_dpu_nic_mode: Option, + /// Controls which Redfish client implementation is used /// for hardware discovery (LibRedfish, NvRedfish, or /// CompareResult for side-by-side validation). @@ -239,6 +210,7 @@ impl Default for SiteExplorerConfig { switches_created_per_run: Self::default_switches_created_per_run(), rotate_switch_nvos_credentials: Self::default_rotate_switch_nvos_credentials(), dpu_policy: None, + deprecated_force_dpu_nic_mode: None, explore_mode: Self::default_explore_mode(), } } @@ -268,6 +240,7 @@ impl PartialEq for SiteExplorerConfig { create_switches, switches_created_per_run, dpu_policy, + deprecated_force_dpu_nic_mode, explore_mode, } = self; @@ -299,6 +272,7 @@ impl PartialEq for SiteExplorerConfig { == other.create_switches.load(AtomicOrdering::Relaxed) && *switches_created_per_run == other.switches_created_per_run && *dpu_policy == other.dpu_policy + && *deprecated_force_dpu_nic_mode == other.deprecated_force_dpu_nic_mode && *explore_mode == other.explore_mode } } diff --git a/crates/site-explorer/tests/integration/site_explorer.rs b/crates/site-explorer/tests/integration/site_explorer.rs index a18d1fdf42..c08407f251 100644 --- a/crates/site-explorer/tests/integration/site_explorer.rs +++ b/crates/site-explorer/tests/integration/site_explorer.rs @@ -2114,6 +2114,7 @@ async fn test_site_explorer_audit_exploration_results( switches_created_per_run: 1, rotate_switch_nvos_credentials: Arc::new(false.into()), dpu_policy: None, + deprecated_force_dpu_nic_mode: None, // Tests use MockEndpointExplorer. So this doesn't affect anything. explore_mode: SiteExplorerExploreMode::NvRedfish, }; diff --git a/crates/state-controller-common/src/config.rs b/crates/state-controller-common/src/config.rs index 005f93af0e..1909806003 100644 --- a/crates/state-controller-common/src/config.rs +++ b/crates/state-controller-common/src/config.rs @@ -22,6 +22,7 @@ use state_controller::config::IterationConfig; /// Common StateController configurations #[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] +#[serde(deny_unknown_fields)] pub struct StateControllerConfig { /// Configures the desired duration for one state controller iteration /// diff --git a/deploy/nico-base/api/config-files/nico-api-config.toml b/deploy/nico-base/api/config-files/nico-api-config.toml index 6b096ede25..8e7a79e1e8 100644 --- a/deploy/nico-base/api/config-files/nico-api-config.toml +++ b/deploy/nico-base/api/config-files/nico-api-config.toml @@ -22,20 +22,20 @@ listen = "[::]:1079" metrics_endpoint = "[::]:1080" -profiler_endpoint = "[::]:1081" database_url = "postgres://replaced-by-env-var" asn = 4294967000 initial_dpu_agent_upgrade_policy = "up_only" -dpu_nic_firmware_intial_update_enabled = false -dpu_nic_firmware_reprovision_update_enabled = true -dpu_nic_firmware_update_version = { "BlueField SoC" = "24.42.1000", "BlueField-3 SmartNIC Main Card" = "32.42.1000" } max_concurrent_machine_updates = 10 -nvue_enabled = true dhcp_servers = [] # Added in per-site-configuration route_servers = [] # Required for FNN. Not yet used attestation_enabled = true bypass_rbac = true +[dpu_config] +dpu_nic_firmware_initial_update_enabled = false +dpu_nic_firmware_reprovision_update_enabled = true +dpu_nic_firmware_update_versions = ["24.42.1000", "32.42.1000"] + [host_health] hardware_health_reports = "MonitorOnly" diff --git a/dev/docker-env/carbide-api-config.toml b/dev/docker-env/carbide-api-config.toml index 88bf265a4f..03d4dc1da1 100644 --- a/dev/docker-env/carbide-api-config.toml +++ b/dev/docker-env/carbide-api-config.toml @@ -23,7 +23,6 @@ enable_route_servers = true dpu_ipmi_tool_impl = "test" initial_domain_name = "forge.local" initial_dpu_agent_upgrade_policy = "off" -nvue_enabled = true attestation_enabled = false bypass_rbac = true allow_insecure_discovery = true diff --git a/helm/charts/nico-api/files/carbide-api-config.toml b/helm/charts/nico-api/files/carbide-api-config.toml index 22391afc63..30ff0306f1 100644 --- a/helm/charts/nico-api/files/carbide-api-config.toml +++ b/helm/charts/nico-api/files/carbide-api-config.toml @@ -7,15 +7,10 @@ listen = "[::]:1079" metrics_endpoint = "[::]:1080" alt_metric_prefix = "nico_" -profiler_endpoint = "[::]:1081" database_url = "postgres://replaced-by-env-var" asn = 4294967000 initial_dpu_agent_upgrade_policy = "up_only" -dpu_nic_firmware_intial_update_enabled = false -dpu_nic_firmware_reprovision_update_enabled = true -dpu_nic_firmware_update_version = { "BlueField SoC" = "24.42.1000", "BlueField-3 SmartNIC Main Card" = "32.42.1000" } max_concurrent_machine_updates = 10 -nvue_enabled = true dhcp_servers = [] # Added in per-site-configuration route_servers = [] # Required for FNN. Not yet used attestation_enabled = true @@ -31,6 +26,10 @@ attestation_enabled = true anycast_site_prefixes = ["0.0.0.0/0"] [dpu_config] +# DPU NIC firmware settings. +dpu_nic_firmware_initial_update_enabled = false +dpu_nic_firmware_reprovision_update_enabled = true +dpu_nic_firmware_update_versions = ["24.42.1000", "32.42.1000"] # Number of VFs configured per DPU PF during BlueField provisioning. num_of_vfs = 16 From 49966130b1b8a5d44c05e0f6557f607fd2a5fb7f Mon Sep 17 00:00:00 2001 From: Hitesh Wadekar Date: Wed, 5 Aug 2026 14:18:17 -0700 Subject: [PATCH 2/6] Fixes suggested by cr --- crates/api-core/src/cfg/file.rs | 154 +++++++++++++++++- crates/libmlx/src/firmware/config.rs | 65 ++++++++ crates/libmlx/src/firmware/credentials.rs | 44 +++-- crates/libmlx/src/firmware/source.rs | 2 +- .../libmlx/tests/firmware/test_credentials.rs | 2 +- crates/scout/src/mlx_device.rs | 2 +- .../api/config-files/nico-api-config.toml | 2 +- deploy/nico-base/api/deployment.yaml | 4 +- deploy/nico-base/api/kustomization.yaml | 2 +- dev/webdev-env/carbide-api-config.toml | 2 + .../nico-api/files/carbide-api-config.toml | 2 +- 11 files changed, 253 insertions(+), 28 deletions(-) diff --git a/crates/api-core/src/cfg/file.rs b/crates/api-core/src/cfg/file.rs index 6f4e05b9ef..7e5af9ee38 100644 --- a/crates/api-core/src/cfg/file.rs +++ b/crates/api-core/src/cfg/file.rs @@ -5043,16 +5043,146 @@ mod tests { ); } - #[test] - fn deserialize_shipped_deployment_config() { - let path = concat!( - env!("CARGO_MANIFEST_DIR"), - "/../../deploy/nico-base/api/config-files/nico-api-config.toml" + fn rendered_helm_api_config() -> String { + let mut config = + include_str!("../../../../helm/charts/nico-api/files/carbide-api-config.toml") + .to_string(); + for (template, rendered) in [ + ( + r#"{{ .Values.auth.adminRootCafilePath | default "/etc/forge/carbide-api/site/admin_root_cert_pem" }}"#, + "/etc/forge/carbide-api/site/admin_root_cert_pem", + ), + ("{{ .Values.auth.permissiveMode | default false }}", "false"), + ("{{ .Values.global.spiffe.trustDomain }}", "example.test"), + ( + r#"{{ .Values.auth.namespace | default (include "nico-api.namespace" .) }}"#, + "nico-system", + ), + ( + "{{ range $i, $cn := .Values.auth.additionalIssuerCns }}{{ if $i }}, {{ end }}{{ $cn | quote }}{{ end }}", + "", + ), + ( + "{{ .Values.service.perObjectStateMetrics.enabled }}", + "false", + ), + ("{{ .Values.service.perObjectStateMetrics.port }}", "9091"), + ( + "{{ default list .Values.service.perObjectStateMetrics.objectTypes | toJson }}", + "[]", + ), + ( + "{{ .Values.componentManager.computeTrayBackend | quote }}", + r#""rms""#, + ), + ( + "{{ .Values.componentManager.nvSwitchBackend | quote }}", + r#""rms""#, + ), + ( + "{{ .Values.componentManager.powerShelfBackend | quote }}", + r#""rms""#, + ), + ( + "{{ .Values.componentManager.nvSwitchUseStateController }}", + "false", + ), + ( + "{{ .Values.componentManager.powerShelfUseStateController }}", + "false", + ), + ( + "{{ .Values.componentManager.computeTrayUseStateController }}", + "false", + ), + ( + "{{ .Values.rms.apiUrl | quote }}", + r#""https://rms.example.test""#, + ), + ("{{ .Values.rms.enforceTls }}", "true"), + ("{{ . | quote }}", r#""/tmp/test.pem""#), + ] { + config = config.replace(template, rendered); + } + + let config = config + .lines() + .filter(|line| !line.trim_start().starts_with("{{-")) + .join("\n"); + assert!( + !config.contains("{{"), + "all Helm template expressions must be rendered for this test" ); - Figment::new() - .merge(Toml::file(path)) + config + } + + fn rendered_deployment_site_config() -> String { + let mut config = + include_str!("../../../../deploy/files/nico-api/nico-api-site-config.toml").to_string(); + for (placeholder, value) in [ + ("MANAGED_HOST_IPMI_POOL_1_GATEWAY_IP", "203.0.113.1"), + ("MANAGED_HOST_IPMI_POOL_1", "203.0.113.0/24"), + ("CONTROL_PLANE_IPMI_POOL_1", "198.51.100.0/24"), + ("ADMIN_NETWORK_GATEWAY_IP", "192.0.2.1"), + ("ADMIN_NETWORK_IP_POOL", "192.0.2.0/24"), + ("DPU_LOOPBACK_START_IP", "10.180.62.1"), + ("DPU_LOOPBACK_END_IP", "10.180.62.62"), + ("NICO_DHCP_EXTERNAL_IP", "192.0.2.10"), + ("SITE_FABRIC_PREFIX_1", "10.0.0.0/8"), + ("ENVIORNMENT_NAME", "test"), + ] { + config = config.replace(placeholder, value); + } + config + } + + #[test] + fn deserialize_shipped_api_configurations() { + let repository_root = concat!(env!("CARGO_MANIFEST_DIR"), "/../.."); + let deploy_path = + format!("{repository_root}/deploy/nico-base/api/config-files/nico-api-config.toml"); + let docker_path = format!("{repository_root}/dev/docker-env/carbide-api-config.toml"); + let webdev_path = format!("{repository_root}/dev/webdev-env/carbide-api-config.toml"); + let site_config = rendered_deployment_site_config(); + let helm_config = rendered_helm_api_config(); + + let deploy_config = Figment::new() + .merge(Toml::file(&deploy_path)) .extract::() .expect("the shipped deployment config must match the strict schema"); + assert_eq!( + deploy_config.dpu_config.dpu_nic_firmware_update_versions, + [BF2_NIC_VERSION.to_string(), BF3_NIC_VERSION.to_string()] + ); + + for (name, figment) in [ + ( + "deployment base plus site override", + Figment::new() + .merge(Toml::file(&deploy_path)) + .merge(Toml::string(&site_config)), + ), + ( + "Docker development", + Figment::new() + .merge(Toml::file(&docker_path)) + .merge(Toml::string( + r#"database_url = "postgres://test:test@localhost/test""#, + )), + ), + ( + "web development", + Figment::new().merge(Toml::file(&webdev_path)), + ), + ( + "rendered Helm", + Figment::new().merge(Toml::string(&helm_config)), + ), + ] { + figment.extract::().unwrap_or_else(|error| { + panic!("{name} config must match the strict schema: {error}") + }); + } } #[test] @@ -5959,7 +6089,10 @@ mod tests { .merge(Toml::string(patch)) .extract::() .map(drop) - .map_err(drop); + .map_err(|error| match error.kind { + Kind::UnknownField(field, _) => field, + other => panic!("expected an unknown-field rejection, got {other:?}"), + }); "unknown fields are rejected" { "unknown_top_level = true" => Fails, @@ -6072,7 +6205,10 @@ priority = 1 .merge(Toml::string(input)) .extract::() .map(drop) - .map_err(drop); + .map_err(|error| match error.kind { + Kind::UnknownField(field, _) => field, + other => panic!("expected an unknown-field rejection, got {other:?}"), + }); "unknown fields are rejected" { "unknown_top_level = true" => Fails, diff --git a/crates/libmlx/src/firmware/config.rs b/crates/libmlx/src/firmware/config.rs index 01671540f4..f1098bab38 100644 --- a/crates/libmlx/src/firmware/config.rs +++ b/crates/libmlx/src/firmware/config.rs @@ -549,4 +549,69 @@ firmware_urll = "https://typo.example.com/fw.bin" let error = FirmwareFlasherProfile::from_toml(toml_str).unwrap_err(); assert!(error.to_string().contains("firmware_urll")); } + + #[test] + fn profile_serde_round_trip_preserves_every_field() { + let original = FirmwareFlasherProfile { + firmware_spec: FirmwareSpec { + part_number: "900-9D3B4-00CV-TA0".to_string(), + psid: "MT_0000000884".to_string(), + version: "32.43.1014".to_string(), + }, + flash_spec: FlashSpec { + firmware_url: "https://artifacts.nvidia.com/fw.bin".to_string(), + firmware_credentials: Some(Credentials::bearer_token("token123")), + device_conf_url: Some("https://artifacts.nvidia.com/debug.conf".to_string()), + device_conf_credentials: Some(Credentials::basic_auth("user", "pass")), + verify_from_cache: true, + cache_dir: Some(PathBuf::from("/var/cache/fw")), + }, + flash_options: FlashOptions { + verify_image: true, + verify_version: true, + reset: true, + reset_level: 5, + }, + }; + + let encoded = toml::to_string(&original).expect("profile serializes"); + let decoded = FirmwareFlasherProfile::from_toml(&encoded) + .expect("serialized profile must deserialize under the strict schema"); + + assert_eq!( + decoded.firmware_spec.part_number, + original.firmware_spec.part_number + ); + assert_eq!(decoded.firmware_spec.psid, original.firmware_spec.psid); + assert_eq!( + decoded.firmware_spec.version, + original.firmware_spec.version + ); + assert_eq!( + decoded.flash_spec.firmware_url, + original.flash_spec.firmware_url + ); + assert_eq!( + decoded.flash_spec.device_conf_url, + original.flash_spec.device_conf_url + ); + assert!(decoded.flash_spec.verify_from_cache); + assert_eq!(decoded.flash_spec.cache_dir, original.flash_spec.cache_dir); + assert!(matches!( + decoded.flash_spec.firmware_credentials, + Some(Credentials::BearerToken { token }) if token == "token123" + )); + assert!(matches!( + decoded.flash_spec.device_conf_credentials, + Some(Credentials::BasicAuth { username, password }) + if username == "user" && password == "pass" + )); + assert!(decoded.flash_options.verify_image); + assert!(decoded.flash_options.verify_version); + assert!(decoded.flash_options.reset); + assert_eq!( + decoded.flash_options.reset_level, + original.flash_options.reset_level + ); + } } diff --git a/crates/libmlx/src/firmware/credentials.rs b/crates/libmlx/src/firmware/credentials.rs index 17e649a881..e84cdb2267 100644 --- a/crates/libmlx/src/firmware/credentials.rs +++ b/crates/libmlx/src/firmware/credentials.rs @@ -71,7 +71,7 @@ pub enum Credentials { }, // SshAgent uses the running SSH agent for authentication. The // agent is reached via the SSH_AUTH_SOCK environment variable. - SshAgent, + SshAgent {}, } impl Credentials { @@ -117,7 +117,7 @@ impl Credentials { // ssh_agent creates an SshAgent credential. pub fn ssh_agent() -> Self { - Self::SshAgent + Self::SshAgent {} } // type_name returns human-readable details for the credential @@ -130,7 +130,7 @@ impl Credentials { Credentials::BasicAuth { .. } => "basic_auth", Credentials::Header { .. } => "header", Credentials::SshKey { .. } => "ssh_key", - Credentials::SshAgent => "ssh_agent", + Credentials::SshAgent {} => "ssh_agent", } } @@ -141,9 +141,11 @@ impl Credentials { Credentials::BearerToken { .. } | Credentials::BasicAuth { .. } | Credentials::Header { .. } => Ok(()), - Credentials::SshKey { .. } | Credentials::SshAgent => Err(FirmwareError::ConfigError( - "SSH credentials cannot be used with HTTP sources".to_string(), - )), + Credentials::SshKey { .. } | Credentials::SshAgent {} => { + Err(FirmwareError::ConfigError( + "SSH credentials cannot be used with HTTP sources".to_string(), + )) + } } } @@ -151,7 +153,7 @@ impl Credentials { // compatible with SSH sources. pub fn validate_ssh(&self) -> FirmwareResult<()> { match self { - Credentials::SshKey { .. } | Credentials::SshAgent => Ok(()), + Credentials::SshKey { .. } | Credentials::SshAgent {} => Ok(()), Credentials::BearerToken { .. } | Credentials::BasicAuth { .. } | Credentials::Header { .. } => Err(FirmwareError::ConfigError( @@ -178,7 +180,7 @@ impl From for FirmwareCredentialsPb { Credentials::SshKey { path, passphrase } => { CredentialTypePb::SshKey(SshKeyCredentialsPb { path, passphrase }) } - Credentials::SshAgent => CredentialTypePb::SshAgent(SshAgentCredentialsPb {}), + Credentials::SshAgent {} => CredentialTypePb::SshAgent(SshAgentCredentialsPb {}), }; FirmwareCredentialsPb { credential_type: Some(credential_type), @@ -207,7 +209,7 @@ impl TryFrom for Credentials { path: sk.path, passphrase: sk.passphrase, }), - CredentialTypePb::SshAgent(_) => Ok(Credentials::SshAgent), + CredentialTypePb::SshAgent(_) => Ok(Credentials::SshAgent {}), } } } @@ -218,7 +220,7 @@ impl TryFrom for forge_ssh::ssh_client::AuthConfig { use forge_ssh::ssh_client::AuthConfig; match value { Credentials::SshKey { path, passphrase } => Ok(AuthConfig::SshKey { path, passphrase }), - Credentials::SshAgent => Ok(AuthConfig::SshAgent), + Credentials::SshAgent {} => Ok(AuthConfig::SshAgent), _ => Err(FirmwareError::ConfigError( "HTTP credentials cannot be used with SSH sources".to_string(), )), @@ -228,6 +230,9 @@ impl TryFrom for forge_ssh::ssh_client::AuthConfig { #[cfg(test)] mod tests { + use carbide_test_support::Outcome::{Fails, Yields}; + use carbide_test_support::scenarios; + use super::*; #[test] @@ -288,6 +293,23 @@ mod tests { let original = Credentials::ssh_agent(); let proto: FirmwareCredentialsPb = original.clone().into(); let converted: Credentials = proto.try_into().unwrap(); - assert!(matches!(converted, Credentials::SshAgent)); + assert!(matches!(converted, Credentials::SshAgent {})); + } + + #[test] + fn ssh_agent_serde_rejects_unknown_fields() { + scenarios!( + run = |input| serde_json::from_str::(input) + .map(|credentials| matches!(credentials, Credentials::SshAgent {})) + .map_err(drop); + + "valid empty struct variant" { + r#"{"type":"ssh_agent"}"# => Yields(true), + } + + "unknown fields are rejected" { + r#"{"type":"ssh_agent","unexpected":true}"# => Fails, + } + ); } } diff --git a/crates/libmlx/src/firmware/source.rs b/crates/libmlx/src/firmware/source.rs index 2fa62e6ff1..09a45dd9a1 100644 --- a/crates/libmlx/src/firmware/source.rs +++ b/crates/libmlx/src/firmware/source.rs @@ -353,7 +353,7 @@ fn credential_type_name(cred: &Credentials) -> &'static str { Credentials::BasicAuth { .. } => "basic_auth", Credentials::Header { .. } => "header", Credentials::SshKey { .. } => "ssh_key", - Credentials::SshAgent => "ssh_agent", + Credentials::SshAgent {} => "ssh_agent", } } diff --git a/crates/libmlx/tests/firmware/test_credentials.rs b/crates/libmlx/tests/firmware/test_credentials.rs index ae80ee3f74..1e57610641 100644 --- a/crates/libmlx/tests/firmware/test_credentials.rs +++ b/crates/libmlx/tests/firmware/test_credentials.rs @@ -113,7 +113,7 @@ fn test_ssh_agent_serde_roundtrip() { let toml = toml::to_string(&cred).unwrap(); let deserialized: Credentials = toml::from_str(&toml).unwrap(); - assert!(matches!(deserialized, Credentials::SshAgent)); + assert!(matches!(deserialized, Credentials::SshAgent {})); } #[test] diff --git a/crates/scout/src/mlx_device.rs b/crates/scout/src/mlx_device.rs index fb66fa9c4c..6a1ab95c9f 100644 --- a/crates/scout/src/mlx_device.rs +++ b/crates/scout/src/mlx_device.rs @@ -247,7 +247,7 @@ fn firmware_credentials(profile: &FirmwareFlasherProfile) -> impl Iterator passphrase.as_deref(), - Credentials::SshAgent => None, + Credentials::SshAgent {} => None, }) } diff --git a/deploy/nico-base/api/config-files/nico-api-config.toml b/deploy/nico-base/api/config-files/nico-api-config.toml index 8e7a79e1e8..f31f080806 100644 --- a/deploy/nico-base/api/config-files/nico-api-config.toml +++ b/deploy/nico-base/api/config-files/nico-api-config.toml @@ -34,7 +34,7 @@ bypass_rbac = true [dpu_config] dpu_nic_firmware_initial_update_enabled = false dpu_nic_firmware_reprovision_update_enabled = true -dpu_nic_firmware_update_versions = ["24.42.1000", "32.42.1000"] +dpu_nic_firmware_update_versions = ["24.47.2682", "32.47.2682"] [host_health] hardware_health_reports = "MonitorOnly" diff --git a/deploy/nico-base/api/deployment.yaml b/deploy/nico-base/api/deployment.yaml index 7c659beb3c..63df2187f5 100644 --- a/deploy/nico-base/api/deployment.yaml +++ b/deploy/nico-base/api/deployment.yaml @@ -50,7 +50,7 @@ spec: - /bin/sh - -c # if you change this command, keep it in sync (as appropriate) with the overlay in local-dev for the local workflow - - exec /opt/carbide/carbide-api run --config-path /etc/forge/carbide-api/carbide-api-config.toml --site-config-path /etc/forge/carbide-api/site/carbide-api-site-config.toml + - exec /opt/carbide/carbide-api run --config-path /etc/forge/carbide-api/nico-api-config.toml --site-config-path /etc/forge/carbide-api/site/nico-api-site-config.toml env: - name: VAULT_ROLE_ID valueFrom: @@ -111,7 +111,7 @@ spec: configMapKeyRef: name: nico-system-nico-database-config key: DB_NAME - - name: CARBIDE_API_DATABASE_URL # Overwrites `database_url` in carbide-api-config.toml + - name: CARBIDE_API_DATABASE_URL # Overwrites `database_url` in nico-api-config.toml value: postgres://$(DATASTORE_USER):$(DATASTORE_PASSWORD)@$(DATASTORE_HOST):$(DATASTORE_PORT)/$(DATASTORE_NAME) - name: CARBIDE_WEB_PRIVATE_COOKIEJAR_KEY value: $(DATASTORE_PASSWORD) diff --git a/deploy/nico-base/api/kustomization.yaml b/deploy/nico-base/api/kustomization.yaml index 93391760ff..f2928a637b 100644 --- a/deploy/nico-base/api/kustomization.yaml +++ b/deploy/nico-base/api/kustomization.yaml @@ -38,7 +38,7 @@ resources: configMapGenerator: - name: nico-api-config-files files: - - config-files/carbide-api-config.toml + - config-files/nico-api-config.toml - config-files/casbin-policy.csv - name: nico-api-site-config-files # Site specific list of files is added in `envs` diff --git a/dev/webdev-env/carbide-api-config.toml b/dev/webdev-env/carbide-api-config.toml index 90a4be68f2..c458feb554 100644 --- a/dev/webdev-env/carbide-api-config.toml +++ b/dev/webdev-env/carbide-api-config.toml @@ -67,6 +67,8 @@ test_selection_mode = "EnableAll" [machine_state_controller] dpu_wait_time = "24h" power_down_wait = "24h" + +[machine_state_controller.controller] iteration_time = "24h" [network_segment_state_controller] diff --git a/helm/charts/nico-api/files/carbide-api-config.toml b/helm/charts/nico-api/files/carbide-api-config.toml index 30ff0306f1..f577156089 100644 --- a/helm/charts/nico-api/files/carbide-api-config.toml +++ b/helm/charts/nico-api/files/carbide-api-config.toml @@ -29,7 +29,7 @@ anycast_site_prefixes = ["0.0.0.0/0"] # DPU NIC firmware settings. dpu_nic_firmware_initial_update_enabled = false dpu_nic_firmware_reprovision_update_enabled = true -dpu_nic_firmware_update_versions = ["24.42.1000", "32.42.1000"] +dpu_nic_firmware_update_versions = ["24.47.2682", "32.47.2682"] # Number of VFs configured per DPU PF during BlueField provisioning. num_of_vfs = 16 From c6ab5090711a11609951ed7d06803915df4bb8ed Mon Sep 17 00:00:00 2001 From: Hitesh Wadekar Date: Thu, 6 Aug 2026 23:44:00 -0700 Subject: [PATCH 3/6] fix(config): warn on unknown fields by default with strict opt-in --- crates/api-core/src/cfg/README.md | 21 +- crates/api-core/src/cfg/file.rs | 8 + crates/api-core/src/cfg/load.rs | 216 +++++++++++++++--- crates/api-core/src/cfg/provenance.rs | 2 +- .../src/test_support/default_config.rs | 1 + crates/api/src/run.rs | 1 + 6 files changed, 208 insertions(+), 41 deletions(-) diff --git a/crates/api-core/src/cfg/README.md b/crates/api-core/src/cfg/README.md index e4c805e690..9bc7aafb66 100644 --- a/crates/api-core/src/cfg/README.md +++ b/crates/api-core/src/cfg/README.md @@ -5,15 +5,17 @@ configuration file, which is deserialized into `NicoConfig` (defined in `file.rs`). Fields are listed in declaration order. Defaults are noted where applicable. -Unknown fields are rejected after the base file, optional site override, and -`CARBIDE_API_` environment values are merged. The startup error reports the -invalid key's full section path. Names inside intentionally dynamic maps, such -as pool names and rack-profile IDs, remain user-defined; fields within each map -value must still match the documented schema. - -The removed `force_dpu_nic_mode` key is the sole compatibility exception: it -is temporarily accepted at the top level and under `[site_explorer]`, ignored, -and reported as a deprecation warning. Use `site_explorer.dpu_policy` instead. +Unknown fields are reported after the base file, optional site override, and +`CARBIDE_API_` environment values are merged. They produce warnings by default +so configuration can be deployed ahead of the supporting binary. Set +`deny_unknown_fields = true` to reject them during startup. Diagnostics include +the invalid key's full section path and source. Names inside intentionally +dynamic maps, such as pool names and rack-profile IDs, remain user-defined; +fields within each map value must still match the documented schema. + +The removed `force_dpu_nic_mode` key is explicitly recognized at the top level +and under `[site_explorer]`, ignored, and reported as a deprecation warning. +Use `site_explorer.dpu_policy` instead. --- @@ -27,6 +29,7 @@ and reported as a deprecation warning. Use `site_explorer.dpu_policy` instead. | `alt_metric_prefix` | `Option` | — | `integrations` | Alternative metric prefix emitted alongside `nico_` for dashboard migration. | | `database_url` | `String` | **required** | `server` | Postgres connection string for all persistent state. | | `max_database_connections` | `u32` | `1000` | `server` | Maximum database connection pool size. | +| `deny_unknown_fields` | `bool` | `false` | `server` | Reject unknown configuration fields instead of logging warnings and continuing. | | `database_pool_acquire_timeout` | `Duration` | `30s` | `server` | How long a caller may wait for a connection from the pool before the attempt fails (sqlx's own default); trips on a stalled database or a saturated pool alike. Must be greater than zero (startup rejects `0`). | | `database_pool_idle_timeout` | `Duration` | `10m` | `server` | Idle time after which the pool closes a connection, keeping the pool's own reaping well inside the Postgres server's 60-minute idle-session reaper. Must be greater than zero (startup rejects `0`). | | `database_pool_max_lifetime` | `Duration` | `30m` | `server` | Maximum age of a pooled connection before it is recycled, so the pool re-balances onto the current primary after a database failover. Must be greater than zero (startup rejects `0`). | diff --git a/crates/api-core/src/cfg/file.rs b/crates/api-core/src/cfg/file.rs index 7e5af9ee38..7b70fcfcf8 100644 --- a/crates/api-core/src/cfg/file.rs +++ b/crates/api-core/src/cfg/file.rs @@ -137,6 +137,13 @@ pub struct CarbideConfig { #[serde(default = "default_max_database_connections")] pub max_database_connections: u32, + /// Whether unknown configuration fields should prevent startup. + /// + /// Defaults to `false`, which logs each unknown field and continues so + /// configuration can be deployed independently of the supporting binary. + #[serde(default)] + pub deny_unknown_fields: bool, + /// How long a caller may wait for a connection from the pool before the /// attempt fails (sqlx's own default). It trips on a stalled database or /// a saturated pool alike. Default is 30s. @@ -4758,6 +4765,7 @@ mod tests { config.max_database_connections, default_max_database_connections() ); + assert!(!config.deny_unknown_fields); // Literals on purpose: these pin the documented defaults (30s/10m/30m // -- sqlx's own), so silently changing a default fn fails here rather // than passing self-referentially. diff --git a/crates/api-core/src/cfg/load.rs b/crates/api-core/src/cfg/load.rs index f35506754e..acf0c799da 100644 --- a/crates/api-core/src/cfg/load.rs +++ b/crates/api-core/src/cfg/load.rs @@ -15,6 +15,7 @@ * limitations under the License. */ +use std::collections::BTreeMap; use std::path::Path; use std::sync::Arc; @@ -22,16 +23,128 @@ use eyre::WrapErr; use figment::providers::{Env, Format, Toml}; use figment::value::{Dict, Map, Value}; use figment::{Figment, Metadata, Profile, Provider}; +use serde::de::DeserializeOwned; use super::file::{CarbideConfig, InitialObjectsConfig}; +#[derive(Clone, Debug, Eq, PartialEq)] +struct UnknownConfigurationField { + path: String, + source: String, +} + +fn remove_value_at_path(value: &mut Value, path: &[String]) -> bool { + let Some((head, tail)) = path.split_first() else { + return false; + }; + + match value { + Value::Dict(_, values) if tail.is_empty() => values.remove(head).is_some(), + Value::Dict(_, values) => values + .get_mut(head) + .is_some_and(|value| remove_value_at_path(value, tail)), + Value::Array(_, values) => head + .parse::() + .ok() + .and_then(|index| values.get_mut(index)) + .is_some_and(|value| remove_value_at_path(value, tail)), + _ => false, + } +} + +fn resolve_error_metadata(mut error: figment::Error, figment: &Figment) -> figment::Error { + if error.metadata.is_none() { + error.metadata = figment.find_metadata(&error.path.join(".")).cloned(); + } + if error.profile.is_none() { + error.profile = Some(figment.profile().clone()); + } + error +} + +#[allow(clippy::result_large_err)] // Figment controls the error representation. +fn extract_with_unknown_fields( + figment: &Figment, +) -> Result<(T, Vec), figment::Error> +where + T: DeserializeOwned, +{ + let mut value = figment.extract::()?; + let mut unknown_fields = BTreeMap::new(); + + loop { + match T::deserialize(&value) { + Ok(config) => return Ok((config, unknown_fields.into_values().collect())), + Err(error) => { + let figment::error::Kind::UnknownField(field, _) = &error.kind else { + return Err(resolve_error_metadata(error, figment)); + }; + + let mut path = error.path.clone(); + if path.last().map(String::as_str) != Some(field) { + path.push(field.clone()); + } + let dotted_path = path.join("."); + let source = figment + .find_metadata(&dotted_path) + .map(super::provenance::source_label) + .unwrap_or_else(|| "configuration".to_string()); + + if !remove_value_at_path(&mut value, &path) { + return Err(resolve_error_metadata(error, figment)); + } + + unknown_fields.insert( + dotted_path.clone(), + UnknownConfigurationField { + path: dotted_path, + source, + }, + ); + } + } + } +} + +fn apply_unknown_field_policy( + unknown_fields: &[UnknownConfigurationField], + deny_unknown_fields: bool, +) -> eyre::Result<()> { + if unknown_fields.is_empty() { + return Ok(()); + } + + if deny_unknown_fields { + let fields = unknown_fields + .iter() + .map(|field| format!("{} ({})", field.path, field.source)) + .collect::>() + .join(", "); + return Err(eyre::eyre!("unknown configuration fields: {fields}")); + } + + for field in unknown_fields { + tracing::warn!( + config_key = %field.path, + config_source = %field.source, + "Ignoring unknown configuration key" + ); + } + Ok(()) +} + /// Parse the `InitialObjectsConfig` file referenced by /// [`CarbideConfig::initial_objects_file`]. -pub fn parse_initial_objects_config(path: &Path) -> eyre::Result { - Figment::new() - .merge(Toml::file(path)) - .extract() - .wrap_err_with(|| format!("while parsing InitialObjectsConfig at {}", path.display())) +pub fn parse_initial_objects_config( + path: &Path, + deny_unknown_fields: bool, +) -> eyre::Result { + let figment = Figment::new().merge(Toml::file(path)); + let (config, unknown_fields) = extract_with_unknown_fields::(&figment) + .wrap_err_with(|| format!("while parsing InitialObjectsConfig at {}", path.display()))?; + apply_unknown_field_policy(&unknown_fields, deny_unknown_fields) + .wrap_err_with(|| format!("while parsing InitialObjectsConfig at {}", path.display()))?; + Ok(config) } /// Return a list of all configuration files that were merged to create the @@ -103,8 +216,9 @@ pub fn parse_carbide_config( site_config_path: Option<&Path>, ) -> eyre::Result> { let merged_config = merged_carbide_config_figment(config_path, site_config_path); - let mut config: CarbideConfig = merged_config - .extract() + let (mut config, unknown_fields) = extract_with_unknown_fields::(&merged_config) + .wrap_err("failed to load configuration files")?; + apply_unknown_field_policy(&unknown_fields, config.deny_unknown_fields) .wrap_err("failed to load configuration files")?; config.config_ctx = Some(merged_config); @@ -222,7 +336,7 @@ mod tests { #[test] #[allow(clippy::result_large_err)] - fn unknown_site_override_field_reports_key_and_source_file() { + fn unknown_site_override_field_is_collected_with_source() { figment::Jail::expect_with(|jail| { jail.create_file( "base.toml", @@ -237,31 +351,71 @@ mod tests { "[site_explorer]\nunknown_site_override_field = true", )?; - let report = parse_carbide_config(Path::new("base.toml"), Some(Path::new("site.toml"))) - .unwrap_err(); - let error = report - .downcast_ref::() - .expect("configuration error retains the Figment source"); - - assert!(matches!( - &error.kind, - figment::error::Kind::UnknownField(field, _) - if field == "unknown_site_override_field" - )); + let figment = + merged_carbide_config_figment(Path::new("base.toml"), Some(Path::new("site.toml"))); + let (config, unknown_fields) = extract_with_unknown_fields::(&figment)?; + + assert!(!config.deny_unknown_fields); assert_eq!( - error.path, - vec![ - "site_explorer".to_string(), - "unknown_site_override_field".to_string() - ] + unknown_fields, + vec![UnknownConfigurationField { + path: "site_explorer.unknown_site_override_field".to_string(), + source: "site.toml".to_string(), + }] ); - let source_path = error - .metadata - .as_ref() - .and_then(|metadata| metadata.source.as_ref()) - .and_then(|source| source.file_path()) - .expect("unknown site key is attributed to its source file"); - assert!(source_path.ends_with("site.toml")); + apply_unknown_field_policy(&unknown_fields, config.deny_unknown_fields) + .expect("unknown fields warn by default"); + Ok(()) + }) + } + + #[test] + #[allow(clippy::result_large_err)] + fn strict_mode_rejects_all_unknown_fields() { + figment::Jail::expect_with(|jail| { + jail.create_file( + "base.toml", + r#" + database_url = "postgres://test" + listen = "[::]:1081" + asn = 1 + deny_unknown_fields = true + unknown_root_field = true + [site_explorer] + unknown_nested_field = true + "#, + )?; + + let figment = merged_carbide_config_figment(Path::new("base.toml"), None); + let (config, unknown_fields) = extract_with_unknown_fields::(&figment)?; + let error = apply_unknown_field_policy(&unknown_fields, config.deny_unknown_fields) + .expect_err("strict mode rejects unknown fields"); + let message = error.to_string(); + assert!(message.contains("unknown_root_field (base.toml)")); + assert!(message.contains("site_explorer.unknown_nested_field (base.toml)")); + Ok(()) + }) + } + + #[test] + #[allow(clippy::result_large_err)] + fn invalid_known_field_still_fails_in_warning_mode() { + figment::Jail::expect_with(|jail| { + jail.create_file( + "base.toml", + r#" + database_url = "postgres://test" + listen = "[::]:1081" + asn = 1 + max_database_connections = "many" + "#, + )?; + + let figment = merged_carbide_config_figment(Path::new("base.toml"), None); + let error = extract_with_unknown_fields::(&figment) + .expect_err("invalid known values remain fatal"); + assert!(matches!(error.kind, figment::error::Kind::InvalidType(..))); + assert_eq!(error.path, vec!["max_database_connections".to_string()]); Ok(()) }) } diff --git a/crates/api-core/src/cfg/provenance.rs b/crates/api-core/src/cfg/provenance.rs index 948c5460c4..a2f86da736 100644 --- a/crates/api-core/src/cfg/provenance.rs +++ b/crates/api-core/src/cfg/provenance.rs @@ -68,7 +68,7 @@ fn collect_explicit_paths( } } -fn source_label(metadata: &figment::Metadata) -> String { +pub(super) fn source_label(metadata: &figment::Metadata) -> String { match metadata.source.as_ref() { Some(figment::Source::File(path)) => path .file_name() diff --git a/crates/api-core/src/test_support/default_config.rs b/crates/api-core/src/test_support/default_config.rs index 00d3257a6b..9183b17df5 100644 --- a/crates/api-core/src/test_support/default_config.rs +++ b/crates/api-core/src/test_support/default_config.rs @@ -156,6 +156,7 @@ pub fn get() -> CarbideConfig { alt_metric_prefix: None, database_url: "pgsql:://localhost".to_string(), max_database_connections: 1000, + deny_unknown_fields: false, database_pool_acquire_timeout: default_database_pool_acquire_timeout(), database_pool_idle_timeout: default_database_pool_idle_timeout(), database_pool_max_lifetime: default_database_pool_max_lifetime(), diff --git a/crates/api/src/run.rs b/crates/api/src/run.rs index 9d1ebf035a..122b050eed 100644 --- a/crates/api/src/run.rs +++ b/crates/api/src/run.rs @@ -69,6 +69,7 @@ pub async fn run( let initial_objects = if let Some(path) = carbide_config.initial_objects_file.as_deref() { Some(carbide_api_core::cfg::load::parse_initial_objects_config( path, + carbide_config.deny_unknown_fields, )?) } else { None From f406c60290300321466c8fee1feba15ce4ce07ee Mon Sep 17 00:00:00 2001 From: Hitesh Wadekar Date: Fri, 7 Aug 2026 09:46:02 -0700 Subject: [PATCH 4/6] Rebase with main --- crates/api-core/src/cfg/file.rs | 2 +- deploy/README.md | 6 +++--- .../{nico-api-config.toml => carbide-api-config.toml} | 0 deploy/nico-base/api/deployment.yaml | 4 ++-- deploy/nico-base/api/kustomization.yaml | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) rename deploy/nico-base/api/config-files/{nico-api-config.toml => carbide-api-config.toml} (100%) diff --git a/crates/api-core/src/cfg/file.rs b/crates/api-core/src/cfg/file.rs index 7b70fcfcf8..d97709e0bd 100644 --- a/crates/api-core/src/cfg/file.rs +++ b/crates/api-core/src/cfg/file.rs @@ -5148,7 +5148,7 @@ mod tests { fn deserialize_shipped_api_configurations() { let repository_root = concat!(env!("CARGO_MANIFEST_DIR"), "/../.."); let deploy_path = - format!("{repository_root}/deploy/nico-base/api/config-files/nico-api-config.toml"); + format!("{repository_root}/deploy/nico-base/api/config-files/carbide-api-config.toml"); let docker_path = format!("{repository_root}/dev/docker-env/carbide-api-config.toml"); let webdev_path = format!("{repository_root}/dev/webdev-env/carbide-api-config.toml"); let site_config = rendered_deployment_site_config(); diff --git a/deploy/README.md b/deploy/README.md index d36492de33..6afe41d09e 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -79,7 +79,7 @@ Path: `deploy/nico-base/api/` - `nico-api-metrics` – metrics, port **1080** - `nico-api-profiler` – profiler, port **1081** - ConfigMaps - - `nico-api-config-files` – base config (`nico-api-config.toml`, `casbin-policy.csv`) + - `nico-api-config-files` – base config (`carbide-api-config.toml`, `casbin-policy.csv`) - `nico-api-site-config-files` – overlay for site‑specific TOML (empty in base) - TLS - `Certificate/nico-api-certificate` → `Secret/nico-api-certificate` (SPIFFE‑style mTLS) @@ -99,11 +99,11 @@ Path: `deploy/nico-base/api/` - `NICO_VAULT_MOUNT` - `NICO_VAULT_PKI_MOUNT` - **Root CA bundle** - - Secret `` mounted where `nico-api-config.toml` expects it. + - Secret `` mounted where `carbide-api-config.toml` expects it. ### Configuration notes -- Runtime config lives in `nico-api-config.toml` and is overlaid by a site‑specific TOML in `nico-api-site-config-files`. +- Runtime config lives in `carbide-api-config.toml` and is overlaid by a site‑specific TOML in `nico-api-site-config-files`. - Important knobs include: - listen/metrics/profiler ports - firmware/DPU settings diff --git a/deploy/nico-base/api/config-files/nico-api-config.toml b/deploy/nico-base/api/config-files/carbide-api-config.toml similarity index 100% rename from deploy/nico-base/api/config-files/nico-api-config.toml rename to deploy/nico-base/api/config-files/carbide-api-config.toml diff --git a/deploy/nico-base/api/deployment.yaml b/deploy/nico-base/api/deployment.yaml index 63df2187f5..022eaab352 100644 --- a/deploy/nico-base/api/deployment.yaml +++ b/deploy/nico-base/api/deployment.yaml @@ -50,7 +50,7 @@ spec: - /bin/sh - -c # if you change this command, keep it in sync (as appropriate) with the overlay in local-dev for the local workflow - - exec /opt/carbide/carbide-api run --config-path /etc/forge/carbide-api/nico-api-config.toml --site-config-path /etc/forge/carbide-api/site/nico-api-site-config.toml + - exec /opt/carbide/carbide-api run --config-path /etc/forge/carbide-api/carbide-api-config.toml --site-config-path /etc/forge/carbide-api/site/nico-api-site-config.toml env: - name: VAULT_ROLE_ID valueFrom: @@ -111,7 +111,7 @@ spec: configMapKeyRef: name: nico-system-nico-database-config key: DB_NAME - - name: CARBIDE_API_DATABASE_URL # Overwrites `database_url` in nico-api-config.toml + - name: CARBIDE_API_DATABASE_URL # Overwrites `database_url` in carbide-api-config.toml value: postgres://$(DATASTORE_USER):$(DATASTORE_PASSWORD)@$(DATASTORE_HOST):$(DATASTORE_PORT)/$(DATASTORE_NAME) - name: CARBIDE_WEB_PRIVATE_COOKIEJAR_KEY value: $(DATASTORE_PASSWORD) diff --git a/deploy/nico-base/api/kustomization.yaml b/deploy/nico-base/api/kustomization.yaml index f2928a637b..93391760ff 100644 --- a/deploy/nico-base/api/kustomization.yaml +++ b/deploy/nico-base/api/kustomization.yaml @@ -38,7 +38,7 @@ resources: configMapGenerator: - name: nico-api-config-files files: - - config-files/nico-api-config.toml + - config-files/carbide-api-config.toml - config-files/casbin-policy.csv - name: nico-api-site-config-files # Site specific list of files is added in `envs` From e345d3a0535fd7a2ce2696a91a2a0bfd45d17ddc Mon Sep 17 00:00:00 2001 From: Hitesh Wadekar Date: Mon, 10 Aug 2026 11:29:34 -0700 Subject: [PATCH 5/6] Fixed suggested by cr --- crates/api-core/src/cfg/file.rs | 64 +++++++++++++++++-- .../src/network_security_group/mod.rs | 28 +++++++- 2 files changed, 85 insertions(+), 7 deletions(-) diff --git a/crates/api-core/src/cfg/file.rs b/crates/api-core/src/cfg/file.rs index d97709e0bd..6abf702abe 100644 --- a/crates/api-core/src/cfg/file.rs +++ b/crates/api-core/src/cfg/file.rs @@ -60,7 +60,10 @@ use model::firmware::{ AgentUpgradePolicyChoice, Firmware, FirmwareComponent, FirmwareComponentType, FirmwareEntry, }; use model::machine::HostHealthConfig; -use model::network_security_group::NetworkSecurityGroupRule; +use model::network_security_group::{ + NetworkSecurityGroupRule, NetworkSecurityGroupRuleAction, NetworkSecurityGroupRuleDirection, + NetworkSecurityGroupRuleNet, NetworkSecurityGroupRuleProtocol, +}; use model::network_segment::NetworkDefinition; use model::resource_pool::define::ResourcePoolDef; use model::tenant::identity_config::SigningAlgorithm; @@ -3275,6 +3278,52 @@ impl Default for DpuConfig { } } +#[derive(Debug, Deserialize)] +#[serde(deny_unknown_fields)] +struct NetworkSecurityGroupRuleConfig { + id: Option, + src_net: NetworkSecurityGroupRuleNet, + dst_net: NetworkSecurityGroupRuleNet, + direction: NetworkSecurityGroupRuleDirection, + ipv6: bool, + src_port_start: Option, + src_port_end: Option, + dst_port_start: Option, + dst_port_end: Option, + protocol: NetworkSecurityGroupRuleProtocol, + action: NetworkSecurityGroupRuleAction, + priority: u32, +} + +impl From for NetworkSecurityGroupRule { + fn from(rule: NetworkSecurityGroupRuleConfig) -> Self { + Self { + id: rule.id, + src_net: rule.src_net, + dst_net: rule.dst_net, + direction: rule.direction, + ipv6: rule.ipv6, + src_port_start: rule.src_port_start, + src_port_end: rule.src_port_end, + dst_port_start: rule.dst_port_start, + dst_port_end: rule.dst_port_end, + protocol: rule.protocol, + action: rule.action, + priority: rule.priority, + } + } +} + +fn deserialize_network_security_group_policy_overrides<'de, D>( + deserializer: D, +) -> Result, D::Error> +where + D: Deserializer<'de>, +{ + Vec::::deserialize(deserializer) + .map(|rules| rules.into_iter().map(Into::into).collect()) +} + #[derive(Clone, Debug, Deserialize, Serialize)] #[serde(deny_unknown_fields)] pub struct NetworkSecurityGroupConfig { @@ -3291,7 +3340,10 @@ pub struct NetworkSecurityGroupConfig { pub stateful_acls_enabled: bool, /// A set of NSG rules that will be inserted before any user-defined rules. - #[serde(default)] + #[serde( + default, + deserialize_with = "deserialize_network_security_group_policy_overrides" + )] pub policy_overrides: Vec, } @@ -5149,8 +5201,8 @@ mod tests { let repository_root = concat!(env!("CARGO_MANIFEST_DIR"), "/../.."); let deploy_path = format!("{repository_root}/deploy/nico-base/api/config-files/carbide-api-config.toml"); - let docker_path = format!("{repository_root}/dev/docker-env/carbide-api-config.toml"); - let webdev_path = format!("{repository_root}/dev/webdev-env/carbide-api-config.toml"); + let docker_config = include_str!("../../../../dev/docker-env/carbide-api-config.toml"); + let webdev_config = include_str!("../../../../dev/webdev-env/carbide-api-config.toml"); let site_config = rendered_deployment_site_config(); let helm_config = rendered_helm_api_config(); @@ -5173,14 +5225,14 @@ mod tests { ( "Docker development", Figment::new() - .merge(Toml::file(&docker_path)) + .merge(Toml::string(docker_config)) .merge(Toml::string( r#"database_url = "postgres://test:test@localhost/test""#, )), ), ( "web development", - Figment::new().merge(Toml::file(&webdev_path)), + Figment::new().merge(Toml::string(webdev_config)), ), ( "rendered Helm", diff --git a/crates/api-model/src/network_security_group/mod.rs b/crates/api-model/src/network_security_group/mod.rs index 0dc9accf43..56ae8d2bd1 100644 --- a/crates/api-model/src/network_security_group/mod.rs +++ b/crates/api-model/src/network_security_group/mod.rs @@ -194,7 +194,6 @@ pub enum NetworkSecurityGroupRuleNet { /// single rule that will be applied on a DPU to restrict /// traffic. #[derive(Debug, Clone, Deserialize, Serialize, PartialEq)] -#[serde(deny_unknown_fields)] pub struct NetworkSecurityGroupRule { pub id: Option, pub src_net: NetworkSecurityGroupRuleNet, @@ -349,3 +348,30 @@ impl<'r> sqlx::FromRow<'r, PgRow> for NetworkSecurityGroup { }) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn persisted_rule_ignores_unknown_legacy_fields() { + let rule: NetworkSecurityGroupRule = serde_json::from_value(serde_json::json!({ + "id": null, + "src_net": { "Prefix": "0.0.0.0/0" }, + "dst_net": { "Prefix": "0.0.0.0/0" }, + "direction": "Ingress", + "ipv6": false, + "src_port_start": null, + "src_port_end": null, + "dst_port_start": null, + "dst_port_end": null, + "protocol": "Any", + "action": "Deny", + "priority": 1, + "legacy_field": "ignored" + })) + .expect("persisted NSG rules remain backward-compatible"); + + assert_eq!(rule.priority, 1); + } +} From 133b2b0261b9c0dd441ced88cd6d964fbd30d650 Mon Sep 17 00:00:00 2001 From: Hitesh Wadekar Date: Thu, 13 Aug 2026 09:29:12 -0700 Subject: [PATCH 6/6] Added boolean configuration for deny unknown config --- crates/api-core/src/cfg/load.rs | 43 +++++++++++++++++++---- crates/api/src/run.rs | 10 +++--- crates/libmlx/src/firmware/credentials.rs | 11 ++++++ 3 files changed, 54 insertions(+), 10 deletions(-) diff --git a/crates/api-core/src/cfg/load.rs b/crates/api-core/src/cfg/load.rs index acf0c799da..a728d00bd4 100644 --- a/crates/api-core/src/cfg/load.rs +++ b/crates/api-core/src/cfg/load.rs @@ -134,8 +134,13 @@ fn apply_unknown_field_policy( } /// Parse the `InitialObjectsConfig` file referenced by -/// [`CarbideConfig::initial_objects_file`]. -pub fn parse_initial_objects_config( +/// [`CarbideConfig::initial_objects_file`], warning about unknown fields. +pub fn parse_initial_objects_config(path: &Path) -> eyre::Result { + parse_initial_objects_config_with_policy(path, false) +} + +/// Parse an `InitialObjectsConfig` using the caller's unknown-field policy. +pub fn parse_initial_objects_config_with_policy( path: &Path, deny_unknown_fields: bool, ) -> eyre::Result { @@ -218,16 +223,42 @@ pub fn parse_carbide_config( let merged_config = merged_carbide_config_figment(config_path, site_config_path); let (mut config, unknown_fields) = extract_with_unknown_fields::(&merged_config) .wrap_err("failed to load configuration files")?; + tracing::info!( + deny_unknown_fields = config.deny_unknown_fields, + unknown_field_policy = if config.deny_unknown_fields { + "deny" + } else { + "warn" + }, + "Using configuration unknown-field policy" + ); apply_unknown_field_policy(&unknown_fields, config.deny_unknown_fields) .wrap_err("failed to load configuration files")?; config.config_ctx = Some(merged_config); - if config.deprecated_force_dpu_nic_mode.is_some() - || config.site_explorer.deprecated_force_dpu_nic_mode.is_some() - { + for (path, is_set) in [ + ( + "force_dpu_nic_mode", + config.deprecated_force_dpu_nic_mode.is_some(), + ), + ( + "site_explorer.force_dpu_nic_mode", + config.site_explorer.deprecated_force_dpu_nic_mode.is_some(), + ), + ] { + if !is_set { + continue; + } + let source = config + .config_ctx + .as_ref() + .and_then(|figment| figment.find_metadata(path)) + .map(super::provenance::source_label) + .unwrap_or_else(|| "configuration".to_string()); tracing::warn!( - config_key = "force_dpu_nic_mode", + config_key = path, + config_source = %source, replacement = "site_explorer.dpu_policy", "Ignoring deprecated configuration key" ); diff --git a/crates/api/src/run.rs b/crates/api/src/run.rs index 122b050eed..2c87dc4f2b 100644 --- a/crates/api/src/run.rs +++ b/crates/api/src/run.rs @@ -67,10 +67,12 @@ pub async fn run( // `InitialObjectsConfig` so that the core runtime can reconcile its contents // against the database on first startup. let initial_objects = if let Some(path) = carbide_config.initial_objects_file.as_deref() { - Some(carbide_api_core::cfg::load::parse_initial_objects_config( - path, - carbide_config.deny_unknown_fields, - )?) + Some( + carbide_api_core::cfg::load::parse_initial_objects_config_with_policy( + path, + carbide_config.deny_unknown_fields, + )?, + ) } else { None }; diff --git a/crates/libmlx/src/firmware/credentials.rs b/crates/libmlx/src/firmware/credentials.rs index e84cdb2267..9d4e011e3f 100644 --- a/crates/libmlx/src/firmware/credentials.rs +++ b/crates/libmlx/src/firmware/credentials.rs @@ -312,4 +312,15 @@ mod tests { } ); } + + #[test] + fn ssh_agent_json_round_trip_preserves_legacy_shape() { + let encoded = serde_json::to_value(Credentials::ssh_agent()) + .expect("SSH-agent credential serializes"); + assert_eq!(encoded, serde_json::json!({ "type": "ssh_agent" })); + + let decoded: Credentials = + serde_json::from_value(encoded).expect("legacy SSH-agent JSON deserializes"); + assert!(matches!(decoded, Credentials::SshAgent {})); + } }