Skip to content

Commit

Permalink
stop logging cloud_init_bytes in sled-agent (#6439)
Browse files Browse the repository at this point in the history
aka "want sled-agent to stop screaming at base64 floppy disks"

Fixes #6387.

1. Create a `NoDebug` wrapper struct for this purpose in omicron-common.
(Note that the OpenAPI schema for sled-agent does not change; schemars
seems to honor `#[serde(transparent)]`.)
2. Apply `NoDebug` inside the `cloud_init_bytes` `Option`. Thus we log
everything else about the instance hardware, and whether or not there is
cidata, but not the cidata itself.
3. Demote an INFO-level log of the Propolis client's
`InstanceEnsureRequest` to DEBUG.
  • Loading branch information
iliana authored Aug 27, 2024
1 parent 7a6f45c commit a24fa8c
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 4 deletions.
24 changes: 24 additions & 0 deletions common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,27 @@ where
async fn never_bail() -> Result<bool, Error> {
Ok(false)
}

/// A wrapper struct that does nothing other than elide the inner value from
/// [`std::fmt::Debug`] output.
///
/// We define this within Omicron instead of using one of the many available
/// crates that do the same thing because it's trivial to do so, and we want the
/// flexibility to add traits to this type without needing to wait on upstream
/// to add an optional dependency.
///
/// If you want to use this for secrets, consider that it might not do
/// everything you expect (it does not zeroize memory on drop, nor get in the
/// way of you removing the inner value from this wrapper struct).
#[derive(
Clone, Copy, serde::Deserialize, serde::Serialize, schemars::JsonSchema,
)]
#[repr(transparent)]
#[serde(transparent)]
pub struct NoDebug<T>(pub T);

impl<T> std::fmt::Debug for NoDebug<T> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "..")
}
}
7 changes: 4 additions & 3 deletions sled-agent/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ use omicron_common::api::internal::shared::{
};
use omicron_common::backoff;
use omicron_common::zpool_name::ZpoolName;
use omicron_common::NoDebug;
use omicron_uuid_kinds::{GenericUuid, InstanceUuid, PropolisUuid};
use propolis_client::Client as PropolisClient;
use rand::prelude::IteratorRandom;
Expand Down Expand Up @@ -335,7 +336,7 @@ struct InstanceRunner {

// Disk related properties
requested_disks: Vec<propolis_client::types::DiskRequest>,
cloud_init_bytes: Option<String>,
cloud_init_bytes: Option<NoDebug<String>>,

// Internal State management
state: InstanceStates,
Expand Down Expand Up @@ -718,10 +719,10 @@ impl InstanceRunner {
.map(Into::into)
.collect(),
migrate,
cloud_init_bytes: self.cloud_init_bytes.clone(),
cloud_init_bytes: self.cloud_init_bytes.clone().map(|x| x.0),
};

info!(self.log, "Sending ensure request to propolis: {:?}", request);
debug!(self.log, "Sending ensure request to propolis: {:?}", request);
let result = client.instance_ensure().body(request).send().await;
info!(self.log, "result of instance_ensure call is {:?}", result);
result?;
Expand Down
3 changes: 2 additions & 1 deletion sled-agent/types/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use omicron_common::api::internal::{
DhcpConfig, NetworkInterface, ResolvedVpcFirewallRule, SourceNatConfig,
},
};
use omicron_common::NoDebug;
use omicron_uuid_kinds::InstanceUuid;
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -60,7 +61,7 @@ pub struct InstanceHardware {
pub dhcp_config: DhcpConfig,
// TODO: replace `propolis_client::*` with locally-modeled request type
pub disks: Vec<propolis_client::types::DiskRequest>,
pub cloud_init_bytes: Option<String>,
pub cloud_init_bytes: Option<NoDebug<String>>,
}

/// Metadata used to track statistics about an instance.
Expand Down

0 comments on commit a24fa8c

Please sign in to comment.