From 0fe41da688d4dd3757989bf0b61395ca20a50e87 Mon Sep 17 00:00:00 2001 From: Zhengchao An Date: Sat, 29 Aug 2026 01:34:04 +0800 Subject: [PATCH] fix(ecstore): document audit/notify KVS divergence and fix auth_token redaction (#6816) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Triages the three divergences backlog#2054 found between the audit and notify default KVS tables, cross-checked against MinIO upstream (internal/logger/config.go, internal/config/notify/parse.go): - webhook: audit's extra batch_size/max_retry/retry_interval/http_timeout keys match MinIO's DefaultAuditWebhookKVS byte-for-byte, while notify's table matches MinIO's notify DefaultWebhookKVS (which lacks them). Intentional, not a copy/paste gap — documented with a doc comment on each table instead of changed. - mqtt: audit's stronger QoS/keep-alive/reconnect defaults have no MinIO precedent (MinIO's audit logging has no MQTT target at all), while notify's 0/0s/0s defaults match MinIO's DefaultMQTTKVS exactly. Documented as an intentional RustFS-original choice, not changed. - auth_token hidden_if_empty: audit had false, notify had true, with no MinIO precedent either way (this KVS version has no per-key hidden flag upstream). Fixed audit to true, matching notify and every other sensitive key in both files (MQTT_PASSWORD, *_TLS_*). Non-empty tokens were already redacted identically on both sides via ends_with("_token") pattern matching in config_admin.rs — this only changes how an *unset* audit webhook auth_token renders in admin config output (omitted instead of shown as an empty value). Refs rustfs/backlog#2054 --- crates/ecstore/src/config/audit.rs | 19 ++++++++++++++++++- crates/ecstore/src/config/notify.rs | 12 ++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/crates/ecstore/src/config/audit.rs b/crates/ecstore/src/config/audit.rs index 06831c478..59d5a3d4a 100644 --- a/crates/ecstore/src/config/audit.rs +++ b/crates/ecstore/src/config/audit.rs @@ -26,6 +26,14 @@ use std::sync::LazyLock; #[allow(clippy::declare_interior_mutable_const)] /// Default KVS for audit webhook settings. +/// +/// `WEBHOOK_BATCH_SIZE`/`WEBHOOK_MAX_RETRY`/`WEBHOOK_RETRY_INTERVAL`/`WEBHOOK_HTTP_TIMEOUT` +/// exist here but not in [`crate::config::notify::DEFAULT_NOTIFY_WEBHOOK_KVS`]. This mirrors +/// MinIO upstream: `internal/logger/config.go`'s `DefaultAuditWebhookKVS` carries the same +/// four keys with the same defaults (`"1"`/`"0"`/`"3s"`/`"5s"`), while +/// `internal/config/notify/parse.go`'s `DefaultWebhookKVS` (bucket event notifications) does +/// not — the notify webhook delivery path never supported them. Not a copy/paste gap +/// (backlog#2054). pub static DEFAULT_AUDIT_WEBHOOK_KVS: LazyLock = LazyLock::new(|| { KVS(vec![ KV { @@ -41,7 +49,7 @@ pub static DEFAULT_AUDIT_WEBHOOK_KVS: LazyLock = LazyLock::new(|| { KV { key: WEBHOOK_AUTH_TOKEN.to_owned(), value: "".to_owned(), - hidden_if_empty: false, + hidden_if_empty: true, // Sensitive field; matches notify's webhook auth_token (backlog#2054) }, KV { key: WEBHOOK_CLIENT_CERT.to_owned(), @@ -103,6 +111,15 @@ pub static DEFAULT_AUDIT_WEBHOOK_KVS: LazyLock = LazyLock::new(|| { #[allow(clippy::declare_interior_mutable_const)] /// Default KVS for audit MQTT settings. +/// +/// `MQTT_QOS`/`MQTT_KEEP_ALIVE_INTERVAL`/`MQTT_RECONNECT_INTERVAL` default to a stronger +/// delivery posture here (`"1"`/`"60s"`/`"5s"`) than +/// [`crate::config::notify::DEFAULT_NOTIFY_MQTT_KVS`] (`"0"`/`"0s"`/`"0s"`, which matches +/// MinIO's own `DefaultMQTTKVS` in `internal/config/notify/parse.go` byte-for-byte). MinIO has +/// no MQTT audit target to compare against — audit-over-MQTT is a RustFS-original addition — +/// so this divergence cannot be checked against upstream; it is intentional (audit favors +/// at-least-once delivery and faster reconnect over notify's opt-in defaults), not a +/// copy/paste gap (backlog#2054). pub static DEFAULT_AUDIT_MQTT_KVS: LazyLock = LazyLock::new(|| { KVS(vec![ KV { diff --git a/crates/ecstore/src/config/notify.rs b/crates/ecstore/src/config/notify.rs index 65c2c018e..96cd97f27 100644 --- a/crates/ecstore/src/config/notify.rs +++ b/crates/ecstore/src/config/notify.rs @@ -26,6 +26,12 @@ use std::sync::LazyLock; /// The default configuration collection of webhooks， /// Initialized only once during the program life cycle, enabling high-performance lazy loading. +/// +/// This table has no `batch_size`/`max_retry`/`retry_interval`/`http_timeout` keys, unlike +/// [`crate::config::audit::DEFAULT_AUDIT_WEBHOOK_KVS`] — matching MinIO upstream, whose +/// `internal/config/notify/parse.go` `DefaultWebhookKVS` (bucket event notifications) also +/// omits them while `internal/logger/config.go`'s `DefaultAuditWebhookKVS` carries them. +/// Intentional, not a copy/paste gap (backlog#2054). pub static DEFAULT_NOTIFY_WEBHOOK_KVS: LazyLock = LazyLock::new(|| { KVS(vec![ KV { @@ -83,6 +89,12 @@ pub static DEFAULT_NOTIFY_WEBHOOK_KVS: LazyLock = LazyLock::new(|| { }); /// MQTT's default configuration collection +/// +/// `MQTT_QOS`/`MQTT_KEEP_ALIVE_INTERVAL`/`MQTT_RECONNECT_INTERVAL` default to `"0"`/`"0s"`/`"0s"` +/// here, matching MinIO's `DefaultMQTTKVS` in `internal/config/notify/parse.go` +/// byte-for-byte — this table is a faithful port. [`crate::config::audit::DEFAULT_AUDIT_MQTT_KVS`] +/// uses stronger, RustFS-original defaults instead (MinIO has no MQTT audit target to compare +/// against); that divergence is intentional, not a copy/paste gap (backlog#2054). pub static DEFAULT_NOTIFY_MQTT_KVS: LazyLock = LazyLock::new(|| { KVS(vec![ KV {