-
Notifications
You must be signed in to change notification settings - Fork 149
feat(health): add ClusterEndpointSource for BCM/file fleet inventory #3359
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,7 +17,7 @@ | |
|
|
||
| use std::fmt::Debug; | ||
| use std::net::{IpAddr, SocketAddr}; | ||
| use std::path::Path; | ||
| use std::path::{Path, PathBuf}; | ||
| use std::time::Duration; | ||
|
|
||
| use figment::Figment; | ||
|
|
@@ -84,13 +84,72 @@ pub struct EndpointSourcesConfig { | |
|
|
||
| /// Static BMC endpoints | ||
| pub static_bmc_endpoints: Vec<StaticBmcEndpoint>, | ||
|
|
||
| /// Cluster inventory file source (file or cluster manager JSON RPC) | ||
| pub cluster: Configurable<ClusterEndpointSourceConfig>, | ||
| } | ||
|
|
||
| impl Default for EndpointSourcesConfig { | ||
| fn default() -> Self { | ||
| Self { | ||
| carbide_api: Configurable::Enabled(CarbideApiConnectionConfig::default()), | ||
| static_bmc_endpoints: Vec::new(), | ||
| cluster: Configurable::Disabled, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| #[derive(Debug, Clone, Serialize, Deserialize)] | ||
| #[serde(deny_unknown_fields)] | ||
| pub struct ClusterEndpointSourceConfig { | ||
| /// Path to a JSON inventory file containing BMC endpoints and credentials for the cluster. | ||
| /// Used when `cluster_manager_url` is absent. | ||
| #[serde(default)] | ||
| pub inventory_path: PathBuf, | ||
|
|
||
| /// Cluster manager head-node URL (e.g. https://10.x.x.x:8081). | ||
| /// When set, inventory and credentials are fetched live via cluster manager JSON RPC | ||
| /// instead of reading `inventory_path`. | ||
| #[serde(default)] | ||
| pub cluster_manager_url: Option<url::Url>, | ||
|
|
||
| /// Cluster manager partition to read bmcsettings from (default: "base"). | ||
| /// The cluster manager stores BMC username/password at partition level; this selects which partition. | ||
| #[serde(default = "default_cluster_manager_partition")] | ||
| pub cluster_manager_partition: String, | ||
|
|
||
| /// Fallback BMC username if cluster manager JSON RPC does not return one. | ||
| /// Cluster manager default is "bright" (set during head-node installation). | ||
| #[serde(default = "default_cluster_manager_username")] | ||
| pub default_username: String, | ||
|
|
||
| /// Fallback BMC password if cluster manager JSON RPC does not return one. | ||
| /// Must be set explicitly — no code-level default. | ||
| #[serde(default)] | ||
| pub default_password: Option<String>, | ||
|
|
||
| /// Optional BMC port override. None uses the BmcClient default (443/HTTPS). | ||
| #[serde(default)] | ||
| pub port: Option<u16>, | ||
| } | ||
|
Comment on lines
+104
to
+134
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '\n## files\n'
git ls-files crates/health/src | sed -n '1,120p'
printf '\n## outline config.rs\n'
ast-grep outline crates/health/src/config.rs --view expanded || true
printf '\n## outline cluster.rs\n'
ast-grep outline crates/health/src/cluster.rs --view expanded || true
printf '\n## search validate and endpoint source references\n'
rg -n "ClusterEndpointSourceConfig|validate\\(|inventory_path|cluster_manager_url|read_from_file" crates/health/src -n
printf '\n## targeted read config.rs around relevant lines\n'
wc -l crates/health/src/config.rs crates/health/src/cluster.rs
sed -n '1,260p' crates/health/src/config.rs
printf '\n--- cluster.rs ---\n'
sed -n '1,260p' crates/health/src/cluster.rsRepository: NVIDIA/infra-controller Length of output: 24070 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '\n## config validation section\n'
sed -n '1290,1405p' crates/health/src/config.rs
printf '\n## cluster endpoint loader\n'
sed -n '260,360p' crates/health/src/endpoint/cluster.rs
printf '\n## config validation tests nearby\n'
sed -n '1680,1775p' crates/health/src/config.rsRepository: NVIDIA/infra-controller Length of output: 10538 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '\n## cluster config impls/tests\n'
rg -n "impl ClusterEndpointSourceConfig|cluster_manager_url|inventory_path|endpoint_sources\.cluster|Loaded cluster endpoints|Failed to read cluster inventory" crates/health/src/config.rs crates/health/src/endpoint -n
printf '\n## default endpoint sources config\n'
sed -n '80,160p' crates/health/src/config.rs
printf '\n## endpoint source config tests around cluster/static config\n'
sed -n '1460,1710p' crates/health/src/config.rsRepository: NVIDIA/infra-controller Length of output: 13386 Validate cluster endpoint source configuration before startup 🤖 Prompt for AI Agents |
||
|
|
||
| fn default_cluster_manager_partition() -> String { | ||
| "base".to_string() | ||
| } | ||
|
|
||
| fn default_cluster_manager_username() -> String { | ||
| "bright".to_string() | ||
| } | ||
|
|
||
| impl Default for ClusterEndpointSourceConfig { | ||
| fn default() -> Self { | ||
| Self { | ||
| inventory_path: PathBuf::default(), | ||
| cluster_manager_url: None, | ||
| cluster_manager_partition: default_cluster_manager_partition(), | ||
| default_username: default_cluster_manager_username(), | ||
| default_password: None, | ||
| port: None, | ||
| } | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Secret leakage via derived
Debug/Serialize.ClusterEndpointSourceConfigderivesDebugandSerialize, anddefault_password: Option<String>is a plain field. Anywhere this config is logged with?configortracing::debug!(?cfg, ...)(or serialized for diagnostics), the password will be printed verbatim. Compare withStaticBmcEndpoint, which has a hand-writtenDebugimpl (crates/health/src/config.rs:233-243) that deliberately omitsusername/password. This new struct breaks that established convention.🔒 Suggested fix: redact the password in `Debug`
📝 Committable suggestion
🤖 Prompt for AI Agents