Skip to content

Commit 0ecc44e

Browse files
authored
feat(heartbeat): report applied config_hash for cross-plane config verification (#777)
1 parent 6f7dfbc commit 0ecc44e

3 files changed

Lines changed: 182 additions & 0 deletions

File tree

crates/aisix-core/src/config_status.rs

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,15 @@ impl ConfigStatus {
387387
self.inner.lock().unwrap().ever_applied
388388
}
389389

390+
/// The hash of the last applied (served) config snapshot, or `None`
391+
/// when no snapshot has been applied yet this boot. A cheap targeted
392+
/// read for callers that need only the hash — the heartbeat's
393+
/// per-node config-verification field — without building the full
394+
/// [`Self::view`] / [`Self::metrics`] snapshot.
395+
pub fn applied_config_hash(&self) -> Option<String> {
396+
self.inner.lock().unwrap().config_hash.clone()
397+
}
398+
390399
/// Point-in-time JSON view for `GET /status/config`.
391400
pub fn view(&self) -> ConfigStatusView {
392401
self.inner.lock().unwrap().view()
@@ -710,6 +719,47 @@ mod tests {
710719
assert!(v.last_failure.is_none());
711720
}
712721

722+
#[test]
723+
fn applied_config_hash_is_none_until_applied_then_tracks_latest() {
724+
let cs = ConfigStatus::new(SourceKind::Etcd);
725+
assert_eq!(cs.applied_config_hash(), None);
726+
// Distinct source vs applied hashes: a regression reading the
727+
// observed source_hash instead of the served config_hash would
728+
// pass with equal values, so keep them apart.
729+
cs.record_load(LoadObservation {
730+
source_hash: "src1".into(),
731+
observed_revision: Some(1),
732+
applied: Some(applied("applied1", &[("models", 1)])),
733+
rejected: vec![],
734+
is_reload: true,
735+
wholly_rejected: false,
736+
});
737+
// Must be the APPLIED (served) hash, never the observed source_hash.
738+
assert_eq!(cs.applied_config_hash().as_deref(), Some("applied1"));
739+
// A later apply carrying a new hash is reflected.
740+
cs.record_load(LoadObservation {
741+
source_hash: "src2".into(),
742+
observed_revision: Some(2),
743+
applied: Some(applied("applied2", &[("models", 1)])),
744+
rejected: vec![],
745+
is_reload: true,
746+
wholly_rejected: false,
747+
});
748+
assert_eq!(cs.applied_config_hash().as_deref(), Some("applied2"));
749+
// A wholly-rejected reload keeps the last-good applied hash even
750+
// as source_hash advances — we report what we serve, not what we
751+
// observed.
752+
cs.record_load(LoadObservation {
753+
source_hash: "src3".into(),
754+
observed_revision: Some(3),
755+
applied: None,
756+
rejected: vec![],
757+
is_reload: true,
758+
wholly_rejected: true,
759+
});
760+
assert_eq!(cs.applied_config_hash().as_deref(), Some("applied2"));
761+
}
762+
713763
#[test]
714764
fn empty_when_clean_load_with_zero_resources() {
715765
let cs = ConfigStatus::new(SourceKind::Etcd);

crates/aisix-server/src/heartbeat.rs

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,17 @@ pub type AppliedRevisionFetcher = Arc<dyn Fn() -> i64 + Send + Sync>;
8383
/// them as resettable, not lifetime totals.
8484
pub type ExporterHealthFetcher = Arc<dyn Fn() -> HashMap<String, SinkStatsSnapshot> + Send + Sync>;
8585

86+
/// Per-tick source of the applied configuration hash — the sha256 the
87+
/// supervisor computed over the accepted (served) resource set
88+
/// (`ConfigStatus::applied_config_hash`, added in #774). Reported as
89+
/// `config_hash` so cp-api can diff the hash a DP node actually applied
90+
/// against the hash it expects that node to be serving (per-node config
91+
/// verification). Returning `None` (config not applied yet) or leaving
92+
/// the fetcher unwired (tests / managed configs) omits the field, so the
93+
/// legacy body shape is preserved and cp-api's tolerance of its absence
94+
/// is honoured.
95+
pub type ConfigHashFetcher = Arc<dyn Fn() -> Option<String> + Send + Sync>;
96+
8697
/// File paths to the on-disk mTLS bundle the heartbeat client presents
8798
/// to cp-api. Same three files written by cert-bundle provisioning and
8899
/// re-used on every subsequent boot when the bundle is already on disk.
@@ -134,6 +145,11 @@ pub struct HeartbeatConfig {
134145
/// Optional source of per-exporter delivery counters. `None`
135146
/// reports an empty `exporter_health` array. See #519 D.2.
136147
pub exporter_health_fetcher: Option<ExporterHealthFetcher>,
148+
/// Optional source of the supervisor's applied config hash. `None`
149+
/// (tests / managed-mode configs without a supervisor wired in) omits
150+
/// `config_hash` from the body — cp-api tolerates its absence. See
151+
/// #774.
152+
pub config_hash_fetcher: Option<ConfigHashFetcher>,
137153
}
138154

139155
impl std::fmt::Debug for HeartbeatConfig {
@@ -157,6 +173,10 @@ impl std::fmt::Debug for HeartbeatConfig {
157173
"exporter_health_fetcher",
158174
&self.exporter_health_fetcher.as_ref().map(|_| "<fn>"),
159175
)
176+
.field(
177+
"config_hash_fetcher",
178+
&self.config_hash_fetcher.as_ref().map(|_| "<fn>"),
179+
)
160180
.finish()
161181
}
162182
}
@@ -183,6 +203,7 @@ impl HeartbeatConfig {
183203
rejection_fetcher: None,
184204
applied_revision_fetcher: None,
185205
exporter_health_fetcher: None,
206+
config_hash_fetcher: None,
186207
}
187208
}
188209

@@ -204,6 +225,12 @@ impl HeartbeatConfig {
204225
self.exporter_health_fetcher = Some(fetcher);
205226
self
206227
}
228+
229+
/// Wire the supervisor's applied config-hash source (#774).
230+
pub fn with_config_hash_fetcher(mut self, fetcher: ConfigHashFetcher) -> Self {
231+
self.config_hash_fetcher = Some(fetcher);
232+
self
233+
}
207234
}
208235

209236
/// Spawn the heartbeat worker. Returns the JoinHandle so `main` can
@@ -282,6 +309,15 @@ struct HeartbeatBody<'a> {
282309
/// against the revision returned by its own kine writes: a DP with
283310
/// `applied_revision >= write_revision` has seen that write.
284311
applied_revision: i64,
312+
/// The sha256 the DP computed over its applied (served) config set
313+
/// (#774). Omitted from the wire when the supervisor has not applied a
314+
/// snapshot yet, or the fetcher isn't wired (tests / managed-mode
315+
/// configs), so cp-api still sees the historical body shape — cp-api
316+
/// records it on telemetry-bearing beats and tolerates its absence.
317+
/// Present, it lets cp-api diff the hash a node applied against the
318+
/// hash it expects that node to serve.
319+
#[serde(skip_serializing_if = "Option::is_none")]
320+
config_hash: Option<String>,
285321
/// Per-exporter delivery counters (#519 D.2), sorted by exporter
286322
/// name. Always present; empty when no exporter pipeline has
287323
/// started. Counters reset when an exporter's pipeline is rebuilt
@@ -300,6 +336,12 @@ struct HeartbeatBody<'a> {
300336
/// guarantee so a future verbose sink can't bloat the heartbeat.
301337
const EXPORTER_LAST_ERROR_MAX_CHARS: usize = 256;
302338

339+
/// Defensive cap on the reported `config_hash` length. The applied hash
340+
/// is a 64-char sha256 hex string, so this never triggers in practice —
341+
/// it matches the cp-api ingestion contract (`config_hash` ≤ 128 chars)
342+
/// so a future hash-scheme change can't overflow the CP's column.
343+
const CONFIG_HASH_MAX_CHARS: usize = 128;
344+
303345
/// On-the-wire shape of one exporter's delivery health (#519 D.2). Kept
304346
/// separate from `aisix_obs::SinkStatsSnapshot` so the obs crate's
305347
/// internal counters can evolve without forcing a wire bump.
@@ -365,6 +407,13 @@ async fn send(client: &reqwest::Client, cfg: &HeartbeatConfig, uptime: i64) -> a
365407
.as_ref()
366408
.map(|fetcher| fetcher())
367409
.unwrap_or(0);
410+
let config_hash = cfg
411+
.config_hash_fetcher
412+
.as_ref()
413+
.and_then(|fetcher| fetcher())
414+
// Defensive clamp — the hash is 64 hex chars, but the CP column
415+
// caps at 128 so never send more.
416+
.map(|h| h.chars().take(CONFIG_HASH_MAX_CHARS).collect::<String>());
368417
let mut exporter_health: Vec<ExporterHealthWire> = cfg
369418
.exporter_health_fetcher
370419
.as_ref()
@@ -390,6 +439,7 @@ async fn send(client: &reqwest::Client, cfg: &HeartbeatConfig, uptime: i64) -> a
390439
version: &BUILD_VERSION,
391440
supported_guardrail_kinds: aisix_guardrails::supported_kinds(),
392441
applied_revision,
442+
config_hash,
393443
exporter_health,
394444
rejected_resources: rejections,
395445
})
@@ -708,6 +758,81 @@ mod tests {
708758
.unwrap()
709759
.iter()
710760
.any(|k| k == "keyword"));
761+
// No config-hash fetcher wired → the field stays off the wire so
762+
// the legacy body shape is preserved (#774).
763+
assert!(
764+
body.get("config_hash").is_none(),
765+
"config_hash must stay off the wire when no fetcher is wired",
766+
);
767+
}
768+
769+
/// #774: a telemetry-bearing beat carries the supervisor's applied
770+
/// config hash so cp-api can diff expected-vs-reported config per
771+
/// node. A normal 64-hex-char hash rides verbatim; an over-long value
772+
/// is clamped to 128 chars (the cp-api ingestion contract).
773+
#[tokio::test]
774+
async fn send_includes_and_clamps_config_hash() {
775+
let server = MockServer::start().await;
776+
Mock::given(method("POST"))
777+
.and(path("/dp/heartbeat"))
778+
.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
779+
"ok": true
780+
})))
781+
.mount(&server)
782+
.await;
783+
784+
let dir = tempfile::tempdir().unwrap();
785+
let mtls = write_test_bundle(dir.path());
786+
787+
// Real-shaped hash (64 hex chars) rides unchanged.
788+
let hash64 = "a".repeat(64);
789+
let cfg = cfg_with_bundle(format!("{}/dp/heartbeat", server.uri()), mtls.clone())
790+
.with_config_hash_fetcher({
791+
let h = hash64.clone();
792+
Arc::new(move || Some(h.clone()))
793+
});
794+
send(&plain_client(), &cfg, 7).await.unwrap();
795+
796+
// A pathologically long value is clamped to 128 chars.
797+
let cfg_long = cfg_with_bundle(format!("{}/dp/heartbeat", server.uri()), mtls)
798+
.with_config_hash_fetcher(Arc::new(|| Some("b".repeat(300))));
799+
send(&plain_client(), &cfg_long, 8).await.unwrap();
800+
801+
let received = server.received_requests().await.unwrap();
802+
let b0: serde_json::Value = serde_json::from_slice(&received[0].body).unwrap();
803+
assert_eq!(b0["config_hash"], hash64);
804+
805+
let b1: serde_json::Value = serde_json::from_slice(&received[1].body).unwrap();
806+
assert_eq!(b1["config_hash"].as_str().unwrap().len(), 128);
807+
assert_eq!(b1["config_hash"], "b".repeat(128));
808+
}
809+
810+
/// A wired fetcher that yields `None` (config not applied yet) still
811+
/// omits `config_hash` — the CP treats absence and "no hash yet" the
812+
/// same, and the legacy body shape is preserved.
813+
#[tokio::test]
814+
async fn send_omits_config_hash_when_fetcher_yields_none() {
815+
let server = MockServer::start().await;
816+
Mock::given(method("POST"))
817+
.and(path("/dp/heartbeat"))
818+
.respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({
819+
"ok": true
820+
})))
821+
.mount(&server)
822+
.await;
823+
824+
let dir = tempfile::tempdir().unwrap();
825+
let mtls = write_test_bundle(dir.path());
826+
let cfg = cfg_with_bundle(format!("{}/dp/heartbeat", server.uri()), mtls)
827+
.with_config_hash_fetcher(Arc::new(|| None));
828+
send(&plain_client(), &cfg, 7).await.unwrap();
829+
830+
let received = server.received_requests().await.unwrap();
831+
let body: serde_json::Value = serde_json::from_slice(&received[0].body).unwrap();
832+
assert!(
833+
body.get("config_hash").is_none(),
834+
"a None fetcher result must omit config_hash from the wire",
835+
);
711836
}
712837

713838
/// #592: every heartbeat carries the per-boot instance identity —

crates/aisix-server/src/main.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -712,6 +712,9 @@ async fn run(mut cfg: Config) -> anyhow::Result<()> {
712712
// - applied_revision: the highest etcd revision the supervisor has
713713
// applied, so cp-api can show "propagating…" until the DP catches
714714
// up with a kine write (#519 B.3)
715+
// - config_hash: the hash of the applied (served) config set, so
716+
// cp-api can diff the hash a node reports against the hash it
717+
// expects that node to be serving (#774)
715718
// - supported_guardrail_kinds + exporter_health (#519 B.6 / D.2)
716719
let heartbeat_task = heartbeat_cfg.map(|mut h| {
717720
// Heartbeat only exists in managed mode, which config
@@ -726,6 +729,10 @@ async fn run(mut cfg: Config) -> anyhow::Result<()> {
726729
}));
727730
let watch_status = supervisor.watch_status();
728731
h = h.with_applied_revision_fetcher(Arc::new(move || watch_status.snapshot().revision));
732+
let config_status_for_heartbeat = supervisor.config_status();
733+
h = h.with_config_hash_fetcher(Arc::new(move || {
734+
config_status_for_heartbeat.applied_config_hash()
735+
}));
729736
let fan_out = proxy_state.otlp_fan_out.clone();
730737
h = h.with_exporter_health_fetcher(Arc::new(move || fan_out.exporter_stats()));
731738
heartbeat::spawn(h, cancel_rx.clone())

0 commit comments

Comments
 (0)