mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-13 16:46:55 +00:00
feat(kms): observe unknown fields in the last three silent persisted formats (#6003)
This commit is contained in:
@@ -35,6 +35,10 @@ tokio = { workspace = true, features = ["fs", "io-util", "macros", "rt-multi-thr
|
||||
uuid = { workspace = true, features = ["serde", "v4", "fast-rng", "macro-diagnostics"] }
|
||||
jiff = { workspace = true, features = ["serde"] }
|
||||
serde = { workspace = true, features = ["derive"] }
|
||||
# Observes fields a persisted-format deserialization ignored, per the
|
||||
# repository rule that formats too compatibility-bound for
|
||||
# deny_unknown_fields must at least warn (AGENTS.md).
|
||||
serde_ignored = { workspace = true }
|
||||
serde_json = { workspace = true, features = ["raw_value"] }
|
||||
tracing = { workspace = true }
|
||||
thiserror = { workspace = true }
|
||||
|
||||
@@ -26,6 +26,7 @@ use crate::backends::{
|
||||
use crate::config::{KmsConfig, VaultConfig};
|
||||
use crate::encryption::{AesDekCrypto, DataKeyEnvelope, DekCrypto, generate_key_material};
|
||||
use crate::error::{KmsError, Result};
|
||||
use crate::persisted_observability::{BoundedUnknownFieldName, UnknownFieldSummary};
|
||||
use crate::policy::{self, AttemptError, OpClass, RetryPolicy};
|
||||
use crate::types::*;
|
||||
use async_trait::async_trait;
|
||||
@@ -60,7 +61,12 @@ pub struct VaultKmsClient {
|
||||
}
|
||||
|
||||
/// Key data stored in Vault
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
///
|
||||
/// `Deserialize` is hand-written so fields the current build does not know
|
||||
/// are counted and warned about instead of vanishing silently — this record
|
||||
/// is compatibility-bound in both directions (older and newer builds read
|
||||
/// each other's writes), so `deny_unknown_fields` is not an option.
|
||||
#[derive(Debug, Clone, Serialize)]
|
||||
struct VaultKeyData {
|
||||
/// Key algorithm
|
||||
algorithm: String,
|
||||
@@ -108,6 +114,187 @@ struct VaultKeyData {
|
||||
baseline_version: Option<u32>,
|
||||
}
|
||||
|
||||
impl UnknownFieldSummary {
|
||||
fn record_for_vault_kv2_key(&self) {
|
||||
let Some((field, field_name_truncated, field_count)) = self.record("vault-kv2-key") else {
|
||||
return;
|
||||
};
|
||||
|
||||
static RECORDS_WITH_UNKNOWN_FIELDS: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0);
|
||||
let observed_records = RECORDS_WITH_UNKNOWN_FIELDS
|
||||
.fetch_add(1, std::sync::atomic::Ordering::Relaxed)
|
||||
.saturating_add(1);
|
||||
if observed_records.is_power_of_two() {
|
||||
tracing::warn!(
|
||||
field = ?field,
|
||||
field_name_truncated,
|
||||
field_count,
|
||||
observed_records,
|
||||
"Vault KV2 key record contains unknown fields"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl<'de> Deserialize<'de> for VaultKeyData {
|
||||
fn deserialize<D>(deserializer: D) -> std::result::Result<Self, D::Error>
|
||||
where
|
||||
D: serde::Deserializer<'de>,
|
||||
{
|
||||
use serde::de::{self, IgnoredAny, MapAccess, Visitor};
|
||||
use std::fmt;
|
||||
|
||||
enum Field {
|
||||
Algorithm,
|
||||
Usage,
|
||||
CreatedAt,
|
||||
Status,
|
||||
Version,
|
||||
Description,
|
||||
Metadata,
|
||||
Tags,
|
||||
DeletionDate,
|
||||
RotatedAt,
|
||||
EncryptedKeyMaterial,
|
||||
BaselineVersion,
|
||||
Unknown(BoundedUnknownFieldName),
|
||||
}
|
||||
|
||||
impl<'de> Deserialize<'de> for Field {
|
||||
fn deserialize<D>(deserializer: D) -> std::result::Result<Self, D::Error>
|
||||
where
|
||||
D: serde::Deserializer<'de>,
|
||||
{
|
||||
struct FieldVisitor;
|
||||
|
||||
impl Visitor<'_> for FieldVisitor {
|
||||
type Value = Field;
|
||||
|
||||
fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
formatter.write_str("a Vault KV2 key record field name")
|
||||
}
|
||||
|
||||
fn visit_str<E>(self, value: &str) -> std::result::Result<Self::Value, E>
|
||||
where
|
||||
E: de::Error,
|
||||
{
|
||||
Ok(match value {
|
||||
"algorithm" => Field::Algorithm,
|
||||
"usage" => Field::Usage,
|
||||
"created_at" => Field::CreatedAt,
|
||||
"status" => Field::Status,
|
||||
"version" => Field::Version,
|
||||
"description" => Field::Description,
|
||||
"metadata" => Field::Metadata,
|
||||
"tags" => Field::Tags,
|
||||
"deletion_date" => Field::DeletionDate,
|
||||
"rotated_at" => Field::RotatedAt,
|
||||
"encrypted_key_material" => Field::EncryptedKeyMaterial,
|
||||
"baseline_version" => Field::BaselineVersion,
|
||||
_ => Field::Unknown(BoundedUnknownFieldName::new(value)),
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
deserializer.deserialize_identifier(FieldVisitor)
|
||||
}
|
||||
}
|
||||
|
||||
struct VaultKeyDataVisitor;
|
||||
|
||||
impl<'de> Visitor<'de> for VaultKeyDataVisitor {
|
||||
type Value = VaultKeyData;
|
||||
|
||||
fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
formatter.write_str("a Vault KV2 key record")
|
||||
}
|
||||
|
||||
fn visit_map<A>(self, mut map: A) -> std::result::Result<Self::Value, A::Error>
|
||||
where
|
||||
A: MapAccess<'de>,
|
||||
{
|
||||
macro_rules! read_field {
|
||||
($slot:ident, $name:literal) => {{
|
||||
if $slot.is_some() {
|
||||
return Err(de::Error::duplicate_field($name));
|
||||
}
|
||||
$slot = Some(map.next_value()?);
|
||||
}};
|
||||
}
|
||||
|
||||
let mut algorithm = None;
|
||||
let mut usage = None;
|
||||
let mut created_at = None;
|
||||
let mut status = None;
|
||||
let mut version = None;
|
||||
let mut description = None;
|
||||
let mut metadata = None;
|
||||
let mut tags = None;
|
||||
let mut deletion_date = None;
|
||||
let mut rotated_at = None;
|
||||
let mut encrypted_key_material = None;
|
||||
let mut baseline_version = None;
|
||||
let mut unknown_fields = UnknownFieldSummary::default();
|
||||
|
||||
while let Some(field) = map.next_key()? {
|
||||
match field {
|
||||
Field::Algorithm => read_field!(algorithm, "algorithm"),
|
||||
Field::Usage => read_field!(usage, "usage"),
|
||||
Field::CreatedAt => read_field!(created_at, "created_at"),
|
||||
Field::Status => read_field!(status, "status"),
|
||||
Field::Version => read_field!(version, "version"),
|
||||
Field::Description => read_field!(description, "description"),
|
||||
Field::Metadata => read_field!(metadata, "metadata"),
|
||||
Field::Tags => read_field!(tags, "tags"),
|
||||
Field::DeletionDate => read_field!(deletion_date, "deletion_date"),
|
||||
Field::RotatedAt => read_field!(rotated_at, "rotated_at"),
|
||||
Field::EncryptedKeyMaterial => read_field!(encrypted_key_material, "encrypted_key_material"),
|
||||
Field::BaselineVersion => read_field!(baseline_version, "baseline_version"),
|
||||
Field::Unknown(field) => {
|
||||
let _: IgnoredAny = map.next_value()?;
|
||||
unknown_fields.observe(field);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let key_data = VaultKeyData {
|
||||
algorithm: algorithm.ok_or_else(|| de::Error::missing_field("algorithm"))?,
|
||||
usage: usage.ok_or_else(|| de::Error::missing_field("usage"))?,
|
||||
created_at: created_at.ok_or_else(|| de::Error::missing_field("created_at"))?,
|
||||
status: status.ok_or_else(|| de::Error::missing_field("status"))?,
|
||||
version: version.ok_or_else(|| de::Error::missing_field("version"))?,
|
||||
description: description.unwrap_or(None),
|
||||
metadata: metadata.ok_or_else(|| de::Error::missing_field("metadata"))?,
|
||||
tags: tags.ok_or_else(|| de::Error::missing_field("tags"))?,
|
||||
deletion_date: deletion_date.unwrap_or(None),
|
||||
rotated_at: rotated_at.unwrap_or(None),
|
||||
encrypted_key_material: encrypted_key_material
|
||||
.ok_or_else(|| de::Error::missing_field("encrypted_key_material"))?,
|
||||
baseline_version: baseline_version.unwrap_or(None),
|
||||
};
|
||||
unknown_fields.record_for_vault_kv2_key();
|
||||
Ok(key_data)
|
||||
}
|
||||
}
|
||||
|
||||
const FIELDS: &[&str] = &[
|
||||
"algorithm",
|
||||
"usage",
|
||||
"created_at",
|
||||
"status",
|
||||
"version",
|
||||
"description",
|
||||
"metadata",
|
||||
"tags",
|
||||
"deletion_date",
|
||||
"rotated_at",
|
||||
"encrypted_key_material",
|
||||
"baseline_version",
|
||||
];
|
||||
deserializer.deserialize_struct("VaultKeyData", FIELDS, VaultKeyDataVisitor)
|
||||
}
|
||||
}
|
||||
|
||||
/// Immutable per-version master key material record stored under
|
||||
/// `{prefix}/{key_id}/versions/{N}`.
|
||||
///
|
||||
@@ -2465,6 +2652,38 @@ mod tests {
|
||||
assert_eq!(legacy.version, 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn vault_key_data_unknown_fields_remain_readable_and_are_observed() {
|
||||
// A record written by a newer build carries fields this build does not
|
||||
// know. It must stay readable — and the drop must be visible, not
|
||||
// silent (rustfs/backlog#1641). Only the field name may be logged; the
|
||||
// value can sit next to key material.
|
||||
let mut value = serde_json::to_value(healthy_key_data()).expect("serialize key data");
|
||||
let object = value.as_object_mut().expect("key data serializes to an object");
|
||||
object.insert("field_from_the_future".to_string(), serde_json::json!("field value must not be logged"));
|
||||
|
||||
let logs = crate::test_support::CapturedLogs::default();
|
||||
let subscriber = tracing_subscriber::fmt()
|
||||
.with_ansi(false)
|
||||
.with_max_level(tracing::Level::WARN)
|
||||
.with_writer(logs.clone())
|
||||
.finish();
|
||||
let dispatch = tracing::Dispatch::new(subscriber);
|
||||
let recorder = metrics_util::debugging::DebuggingRecorder::new();
|
||||
let parsed: VaultKeyData = metrics::with_local_recorder(&recorder, || {
|
||||
tracing::dispatcher::with_default(&dispatch, || {
|
||||
serde_json::from_value(value).expect("unknown fields must remain readable")
|
||||
})
|
||||
});
|
||||
assert_eq!(parsed.algorithm, healthy_key_data().algorithm);
|
||||
assert_eq!(crate::test_support::unknown_field_metric(&recorder, "vault-kv2-key"), 1);
|
||||
|
||||
let output = logs.output();
|
||||
assert!(output.contains("Vault KV2 key record contains unknown fields"), "got: {output}");
|
||||
assert!(output.contains("field_from_the_future"));
|
||||
assert!(!output.contains("field value must not be logged"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_cas_conflict_only_matches_cas_failures() {
|
||||
let cas = ClientError::APIError {
|
||||
|
||||
@@ -27,6 +27,7 @@ use crate::backends::{
|
||||
use crate::config::{KmsConfig, VaultTransitConfig};
|
||||
use crate::encryption::{DataKeyEnvelope, generate_key_material};
|
||||
use crate::error::{KmsError, Result};
|
||||
use crate::persisted_observability::{BoundedUnknownFieldName, UnknownFieldSummary};
|
||||
use crate::policy::{self, AttemptError, OpClass, RetryPolicy};
|
||||
use crate::types::*;
|
||||
use async_trait::async_trait;
|
||||
@@ -114,7 +115,12 @@ struct TransitKeyMetadata {
|
||||
}
|
||||
|
||||
/// Serializable version of TransitKeyMetadata for KV v2 persistence.
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
///
|
||||
/// `Deserialize` is hand-written so fields the current build does not know
|
||||
/// are counted and warned about instead of vanishing silently — this record
|
||||
/// is compatibility-bound in both directions (older and newer builds read
|
||||
/// each other's writes), so `deny_unknown_fields` is not an option.
|
||||
#[derive(Debug, Clone, Serialize)]
|
||||
struct TransitKeyMetadataPersisted {
|
||||
key_usage: KeyUsage,
|
||||
description: Option<String>,
|
||||
@@ -127,6 +133,168 @@ struct TransitKeyMetadataPersisted {
|
||||
current_version: u32,
|
||||
}
|
||||
|
||||
impl UnknownFieldSummary {
|
||||
fn record_for_transit_key_metadata(&self) {
|
||||
let Some((field, field_name_truncated, field_count)) = self.record("vault-transit-key-metadata") else {
|
||||
return;
|
||||
};
|
||||
|
||||
static RECORDS_WITH_UNKNOWN_FIELDS: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0);
|
||||
let observed_records = RECORDS_WITH_UNKNOWN_FIELDS
|
||||
.fetch_add(1, std::sync::atomic::Ordering::Relaxed)
|
||||
.saturating_add(1);
|
||||
if observed_records.is_power_of_two() {
|
||||
tracing::warn!(
|
||||
field = ?field,
|
||||
field_name_truncated,
|
||||
field_count,
|
||||
observed_records,
|
||||
"Vault Transit key metadata record contains unknown fields"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl<'de> Deserialize<'de> for TransitKeyMetadataPersisted {
|
||||
fn deserialize<D>(deserializer: D) -> std::result::Result<Self, D::Error>
|
||||
where
|
||||
D: serde::Deserializer<'de>,
|
||||
{
|
||||
use serde::de::{self, IgnoredAny, MapAccess, Visitor};
|
||||
use std::fmt;
|
||||
|
||||
enum Field {
|
||||
KeyUsage,
|
||||
Description,
|
||||
Tags,
|
||||
KeyState,
|
||||
CreatedAt,
|
||||
DeletionDate,
|
||||
Origin,
|
||||
CreatedBy,
|
||||
CurrentVersion,
|
||||
Unknown(BoundedUnknownFieldName),
|
||||
}
|
||||
|
||||
impl<'de> Deserialize<'de> for Field {
|
||||
fn deserialize<D>(deserializer: D) -> std::result::Result<Self, D::Error>
|
||||
where
|
||||
D: serde::Deserializer<'de>,
|
||||
{
|
||||
struct FieldVisitor;
|
||||
|
||||
impl Visitor<'_> for FieldVisitor {
|
||||
type Value = Field;
|
||||
|
||||
fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
formatter.write_str("a Vault Transit key metadata field name")
|
||||
}
|
||||
|
||||
fn visit_str<E>(self, value: &str) -> std::result::Result<Self::Value, E>
|
||||
where
|
||||
E: de::Error,
|
||||
{
|
||||
Ok(match value {
|
||||
"key_usage" => Field::KeyUsage,
|
||||
"description" => Field::Description,
|
||||
"tags" => Field::Tags,
|
||||
"key_state" => Field::KeyState,
|
||||
"created_at" => Field::CreatedAt,
|
||||
"deletion_date" => Field::DeletionDate,
|
||||
"origin" => Field::Origin,
|
||||
"created_by" => Field::CreatedBy,
|
||||
"current_version" => Field::CurrentVersion,
|
||||
_ => Field::Unknown(BoundedUnknownFieldName::new(value)),
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
deserializer.deserialize_identifier(FieldVisitor)
|
||||
}
|
||||
}
|
||||
|
||||
struct TransitKeyMetadataPersistedVisitor;
|
||||
|
||||
impl<'de> Visitor<'de> for TransitKeyMetadataPersistedVisitor {
|
||||
type Value = TransitKeyMetadataPersisted;
|
||||
|
||||
fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
formatter.write_str("a Vault Transit key metadata record")
|
||||
}
|
||||
|
||||
fn visit_map<A>(self, mut map: A) -> std::result::Result<Self::Value, A::Error>
|
||||
where
|
||||
A: MapAccess<'de>,
|
||||
{
|
||||
macro_rules! read_field {
|
||||
($slot:ident, $name:literal) => {{
|
||||
if $slot.is_some() {
|
||||
return Err(de::Error::duplicate_field($name));
|
||||
}
|
||||
$slot = Some(map.next_value()?);
|
||||
}};
|
||||
}
|
||||
|
||||
let mut key_usage = None;
|
||||
let mut description = None;
|
||||
let mut tags = None;
|
||||
let mut key_state = None;
|
||||
let mut created_at = None;
|
||||
let mut deletion_date = None;
|
||||
let mut origin = None;
|
||||
let mut created_by = None;
|
||||
let mut current_version = None;
|
||||
let mut unknown_fields = UnknownFieldSummary::default();
|
||||
|
||||
while let Some(field) = map.next_key()? {
|
||||
match field {
|
||||
Field::KeyUsage => read_field!(key_usage, "key_usage"),
|
||||
Field::Description => read_field!(description, "description"),
|
||||
Field::Tags => read_field!(tags, "tags"),
|
||||
Field::KeyState => read_field!(key_state, "key_state"),
|
||||
Field::CreatedAt => read_field!(created_at, "created_at"),
|
||||
Field::DeletionDate => read_field!(deletion_date, "deletion_date"),
|
||||
Field::Origin => read_field!(origin, "origin"),
|
||||
Field::CreatedBy => read_field!(created_by, "created_by"),
|
||||
Field::CurrentVersion => read_field!(current_version, "current_version"),
|
||||
Field::Unknown(field) => {
|
||||
let _: IgnoredAny = map.next_value()?;
|
||||
unknown_fields.observe(field);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let metadata = TransitKeyMetadataPersisted {
|
||||
key_usage: key_usage.ok_or_else(|| de::Error::missing_field("key_usage"))?,
|
||||
description: description.unwrap_or(None),
|
||||
tags: tags.ok_or_else(|| de::Error::missing_field("tags"))?,
|
||||
key_state: key_state.ok_or_else(|| de::Error::missing_field("key_state"))?,
|
||||
created_at: created_at.ok_or_else(|| de::Error::missing_field("created_at"))?,
|
||||
deletion_date: deletion_date.unwrap_or(None),
|
||||
origin: origin.ok_or_else(|| de::Error::missing_field("origin"))?,
|
||||
created_by: created_by.unwrap_or(None),
|
||||
current_version: current_version.ok_or_else(|| de::Error::missing_field("current_version"))?,
|
||||
};
|
||||
unknown_fields.record_for_transit_key_metadata();
|
||||
Ok(metadata)
|
||||
}
|
||||
}
|
||||
|
||||
const FIELDS: &[&str] = &[
|
||||
"key_usage",
|
||||
"description",
|
||||
"tags",
|
||||
"key_state",
|
||||
"created_at",
|
||||
"deletion_date",
|
||||
"origin",
|
||||
"created_by",
|
||||
"current_version",
|
||||
];
|
||||
deserializer.deserialize_struct("TransitKeyMetadataPersisted", FIELDS, TransitKeyMetadataPersistedVisitor)
|
||||
}
|
||||
}
|
||||
|
||||
impl TransitKeyMetadata {
|
||||
fn from_create_request(request: &CreateKeyRequest) -> Self {
|
||||
Self {
|
||||
@@ -2133,6 +2301,41 @@ mod tests {
|
||||
assert!(metadata.deletion_date.is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn transit_key_metadata_unknown_fields_remain_readable_and_are_observed() {
|
||||
// A record written by a newer build carries fields this build does not
|
||||
// know. It must stay readable — and the drop must be visible, not
|
||||
// silent (rustfs/backlog#1641). Only the field name may be logged.
|
||||
let persisted: TransitKeyMetadataPersisted = TransitKeyMetadata::synthesized().into();
|
||||
let mut value = serde_json::to_value(&persisted).expect("serialize metadata record");
|
||||
let object = value.as_object_mut().expect("metadata record serializes to an object");
|
||||
object.insert("field_from_the_future".to_string(), serde_json::json!("field value must not be logged"));
|
||||
|
||||
let logs = crate::test_support::CapturedLogs::default();
|
||||
let subscriber = tracing_subscriber::fmt()
|
||||
.with_ansi(false)
|
||||
.with_max_level(tracing::Level::WARN)
|
||||
.with_writer(logs.clone())
|
||||
.finish();
|
||||
let dispatch = tracing::Dispatch::new(subscriber);
|
||||
let recorder = metrics_util::debugging::DebuggingRecorder::new();
|
||||
let parsed: TransitKeyMetadataPersisted = metrics::with_local_recorder(&recorder, || {
|
||||
tracing::dispatcher::with_default(&dispatch, || {
|
||||
serde_json::from_value(value).expect("unknown fields must remain readable")
|
||||
})
|
||||
});
|
||||
assert_eq!(parsed.key_state, KeyState::Enabled);
|
||||
assert_eq!(crate::test_support::unknown_field_metric(&recorder, "vault-transit-key-metadata"), 1);
|
||||
|
||||
let output = logs.output();
|
||||
assert!(
|
||||
output.contains("Vault Transit key metadata record contains unknown fields"),
|
||||
"got: {output}"
|
||||
);
|
||||
assert!(output.contains("field_from_the_future"));
|
||||
assert!(!output.contains("field value must not be logged"));
|
||||
}
|
||||
|
||||
/// KV2 write acknowledgement (`SecretVersionMetadata`) for `kv2::set`.
|
||||
fn kv2_write_ack() -> serde_json::Value {
|
||||
serde_json::json!({
|
||||
|
||||
@@ -1129,6 +1129,53 @@ pub fn allow_immediate_deletion_from_env() -> bool {
|
||||
get_env_bool(ENV_KMS_ALLOW_IMMEDIATE_DELETION, false)
|
||||
}
|
||||
|
||||
impl crate::persisted_observability::UnknownFieldSummary {
|
||||
fn record_for_kms_config(&self) {
|
||||
let Some((field, field_name_truncated, field_count)) = self.record("kms-config") else {
|
||||
return;
|
||||
};
|
||||
|
||||
static RECORDS_WITH_UNKNOWN_FIELDS: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0);
|
||||
let observed_records = RECORDS_WITH_UNKNOWN_FIELDS
|
||||
.fetch_add(1, std::sync::atomic::Ordering::Relaxed)
|
||||
.saturating_add(1);
|
||||
if observed_records.is_power_of_two() {
|
||||
tracing::warn!(
|
||||
field = ?field,
|
||||
field_name_truncated,
|
||||
field_count,
|
||||
observed_records,
|
||||
"persisted KMS configuration contains unknown fields"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Deserialize a persisted KMS configuration, observing ignored fields.
|
||||
///
|
||||
/// The persisted configuration deliberately tolerates unknown fields — a
|
||||
/// rolling upgrade writes fields the previous build does not know, and
|
||||
/// rejecting them would turn every upgrade into a hard stop (see the
|
||||
/// regression test pinning that tolerance). Tolerated must not mean
|
||||
/// invisible: this loader wraps the deserializer with `serde_ignored`, so
|
||||
/// every field the configuration silently dropped is counted and sampled
|
||||
/// into a warning, per the repository rule that formats too
|
||||
/// compatibility-bound for `deny_unknown_fields` must at least log unknown
|
||||
/// fields. Only field paths are recorded, never values — a mistyped field
|
||||
/// name can sit next to a secret.
|
||||
pub fn kms_config_from_persisted_json(data: &[u8]) -> serde_json::Result<KmsConfig> {
|
||||
use crate::persisted_observability::{BoundedUnknownFieldName, UnknownFieldSummary};
|
||||
|
||||
let mut deserializer = serde_json::Deserializer::from_slice(data);
|
||||
let mut unknown_fields = UnknownFieldSummary::default();
|
||||
let config: KmsConfig = serde_ignored::deserialize(&mut deserializer, |path| {
|
||||
unknown_fields.observe(BoundedUnknownFieldName::new(&path.to_string()));
|
||||
})?;
|
||||
deserializer.end()?;
|
||||
unknown_fields.record_for_kms_config();
|
||||
Ok(config)
|
||||
}
|
||||
|
||||
fn vault_tls_config(skip_tls_verify: bool) -> Option<TlsConfig> {
|
||||
skip_tls_verify.then_some(TlsConfig {
|
||||
ca_cert_path: None,
|
||||
@@ -1979,6 +2026,58 @@ mod tests {
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn persisted_config_unknown_fields_remain_readable_and_are_observed() {
|
||||
// Unknown fields in a persisted config are deliberately tolerated (a
|
||||
// rolling upgrade writes fields the previous build does not know), but
|
||||
// tolerated must not mean invisible (rustfs/backlog#1641): the
|
||||
// observing loader counts and warns, naming only the field path —
|
||||
// never the value, which can sit next to a secret. Coverage includes a
|
||||
// field nested inside the backend variant, which the externally tagged
|
||||
// enum exposes to the observer.
|
||||
let mut value = serde_json::to_value(KmsConfig::default()).expect("serialize config");
|
||||
value.as_object_mut().expect("config serializes to an object").insert(
|
||||
"top_level_field_from_the_future".to_string(),
|
||||
serde_json::json!("top-level value must not be logged"),
|
||||
);
|
||||
value
|
||||
.pointer_mut("/backend_config/Local")
|
||||
.expect("default config has a Local backend section")
|
||||
.as_object_mut()
|
||||
.expect("Local backend section is an object")
|
||||
.insert(
|
||||
"nested_field_from_the_future".to_string(),
|
||||
serde_json::json!("nested value must not be logged"),
|
||||
);
|
||||
let data = serde_json::to_vec(&value).expect("encode config");
|
||||
|
||||
let logs = crate::test_support::CapturedLogs::default();
|
||||
let subscriber = tracing_subscriber::fmt()
|
||||
.with_ansi(false)
|
||||
.with_max_level(tracing::Level::WARN)
|
||||
.with_writer(logs.clone())
|
||||
.finish();
|
||||
let dispatch = tracing::Dispatch::new(subscriber);
|
||||
let recorder = metrics_util::debugging::DebuggingRecorder::new();
|
||||
let config = metrics::with_local_recorder(&recorder, || {
|
||||
tracing::dispatcher::with_default(&dispatch, || {
|
||||
kms_config_from_persisted_json(&data).expect("unknown fields must remain readable")
|
||||
})
|
||||
});
|
||||
assert!(matches!(config.backend_config, BackendConfig::Local(_)));
|
||||
assert_eq!(crate::test_support::unknown_field_metric(&recorder, "kms-config"), 2);
|
||||
|
||||
let output = logs.output();
|
||||
assert!(output.contains("persisted KMS configuration contains unknown fields"), "got: {output}");
|
||||
assert!(!output.contains("must not be logged"));
|
||||
|
||||
// A clean config observes nothing and logs nothing.
|
||||
let clean = serde_json::to_vec(&KmsConfig::default()).expect("encode clean config");
|
||||
let recorder = metrics_util::debugging::DebuggingRecorder::new();
|
||||
metrics::with_local_recorder(&recorder, || kms_config_from_persisted_json(&clean).expect("clean config must parse"));
|
||||
assert_eq!(crate::test_support::unknown_field_metric(&recorder, "kms-config"), 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_validate_rejects_incomplete_approle() {
|
||||
let mut config = KmsConfig::vault_approle(
|
||||
|
||||
Reference in New Issue
Block a user