Skip to content

Commit fcfc718

Browse files
committed
Implement hot-reload safety checks for boot-time configuration fields; reject changes to listeners, admin, shutdown, telemetry metrics, and access log during reload
1 parent 835ff62 commit fcfc718

14 files changed

Lines changed: 333 additions & 81 deletions

File tree

ARCHITECTURE.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
## 1. Axioms / Invariants
44
- **Frozen Data Plane** — Every semantic decision (routing, retries, TLS, RBAC, health, observability) **MUST** be resolved before serialization. No runtime component may construct or reinterpret policy.
55
- **No Runtime Interpretation** — The runtime **MUST NOT** parse text configs, infer defaults, evaluate scripts, or execute dynamic code. It only deserializes trusted `.pvs` artifacts produced by the compiler pipeline.
6-
- **Atomic Reload Only**Configuration transitions **MUST** occur via an all-or-nothing swap of the entire artifact. Partial updates, incremental edits, and mutable in-place structures are forbidden.
6+
- **Atomic Runtime Swap Only**Runtime-safe configuration transitions **MUST** occur via an all-or-nothing swap of the live `RuntimeState`. Fields that require listener, admin, metrics, access-log, or other boot-time service wiring are not hot-reloadable in the current architecture and **MUST** be rejected during reload.
77
- **Fail-Closed Semantics** — Any validation error, environment violation, or artifact incompatibility **MUST** leave the runtime serving the last-known-good state. There is no graceful degradation, no fallback heuristics, and no best-effort mode.
88

99
## 2. Semantic Boundary
@@ -20,7 +20,7 @@
2020
- Corruption, mismatched architecture, or unsupported version **MUST** cause immediate rejection and a return to LKG. The relay is responsible for monotonic versioning; the runtime never mutates artifacts and never attempts repair.
2121

2222
## 4. Failure Semantics
23-
- Reload success is binary. Either the artifact validates and atomically replaces the live state, or it is rejected with no partial side effects.
23+
- Reload success is binary. Either the runtime-safe portion validates and atomically replaces the live state, or the artifact is rejected with no partial side effects.
2424
- Rollback is not a special path; refusal to load a new artifact simply keeps the current state. Manual rollback requires resealing or redelivering a prior artifact.
2525
- LKG is authoritative. The runtime **MUST** keep serving the last applied artifact until a new artifact is proven valid. There is no shadow config, preview mode, or best-effort merging.
2626
- There is no graceful degradation. If upstreams fail or artifacts are invalid, the runtime fails-closed and surfaces explicit errors instead of improvising behavior.
@@ -37,6 +37,7 @@
3737
- `build()` wires telemetry, runtime state, resolver/health monitors, listeners, agents, and admin endpoints **before** any service is started; `run()` is the only place that calls `Server::run_forever`.
3838
- `listener::tls::TlsRuntime` owns TLS materialization. Pingora never sees raw paths—TLS certificates, private keys, and client-auth CA bundles are loaded and validated upfront with explicit `Optional`/`Required` semantics.
3939
- All listener, telemetry, and agent wiring happens before `run()` is invoked, guaranteeing that a failed dependency (cert unreadable, metrics bind failure, etc.) aborts the boot rather than partially starting services.
40+
- Because those services are boot-wired, reload currently rejects changes to `listeners`, `admin`, `shutdown`, `telemetry.metrics`, and `telemetry.access_log` instead of trying to mutate the service graph in place.
4041

4142
## 7. Request Telemetry & Identity Handling
4243
- **RequestTelemetry** — Every inbound request now owns a `RequestTelemetry` struct that captures the immutable `RequestId` plus the active tracing span. `RouterContext` delegates all span/state mutations to this struct so Pingora phases cannot forget to emit span updates or accidentally swap request ids mid-flight.
@@ -51,7 +52,7 @@
5152

5253
## 9. Materialized Runtime Config
5354
- Reload now produces a `MaterializedRuntimeConfig` that owns the pre-built router and upstream manager so request threads never see partially-constructed state.
54-
- `RuntimeState` deref-coerces to the materialized struct and only exposes a single `ConfigVersion` newtype (non-zero) so metrics/admin surfaces no longer juggle `Option<u64>` defaults.
55+
- `RuntimeState` deref-coerces to the materialized struct, while boot-time resources remain owned by the bootstrap layer. Config versioning is still surfaced through `Option<ConfigVersion>` during pre-LKG/bootstrap phases and becomes concrete after a versioned artifact is applied.
5556
- Upstream clusters are segmented into `cluster/state.rs`, `cluster/health.rs`, `cluster/pool.rs`, and `cluster/tls.rs` modules, which keeps load balancing, health tracking, pooling, and TLS materialization isolated behind explicit APIs.
5657

5758
## 10. Health Monitoring & TLS Reuse
@@ -63,4 +64,3 @@
6364
- Metrics recording flows through `MetricsRegistry`, a thin wrapper around the Prometheus handle. All request/cluster/telemetry helpers share this registry so instrumentation stays centralized.
6465
- The Prometheus endpoint is implemented by `telemetry::metrics::PrometheusEndpoint`, which depends on a pluggable `MetricsTransport`. The default transport binds a `tokio::net::TcpListener`, but tests can now inject custom transports without touching network sockets.
6566
- Metrics exporting remains best-effort: if recorder installation fails or the listener cannot bind, the endpoint logs the error once and metrics stay disabled rather than partially-initialized.
66-

crates/pavis-relay/README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@ The central configuration distribution hub for the Pavis ecosystem.
44

55
## Purpose
66

7-
`pavis-relay` implements the control plane for configuration management, responsible for ingesting validated `.pvs` artifacts, assigning monotonic version numbers, and distributing updates to runtime instances via HTTP long-polling.
7+
`pavis-relay` implements the control plane for configuration management, responsible for ingesting `.pvs` artifacts that have already passed compiler-side validation, assigning monotonic version numbers, and distributing updates to runtime instances via HTTP long-polling.
88

99
## Responsibilities
1010

11-
- Accepting and validating published configuration artifacts
11+
- Accepting published artifacts and verifying binary integrity
1212
- Assigning strictly monotonic version numbers
1313
- Maintaining Last Known Good (LKG) configuration on disk
1414
- Serving configurations via long-polling HTTP API

crates/pavis-relay/src/app.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,9 +111,7 @@ fn build_codec(config: &PipelineConfig) -> Result<BoxedCodec> {
111111
config::CodecKind::Serde => {
112112
#[cfg(feature = "codec-serde")]
113113
{
114-
Ok(Box::new(pavis_codec_serde::SerdeCodec {
115-
format: pavis_codec_serde::SerdeFormat::Yaml,
116-
}))
114+
Ok(Box::new(crate::codec::AutoSerdeCodec))
117115
}
118116
#[cfg(not(feature = "codec-serde"))]
119117
{

crates/pavis-relay/src/codec.rs

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,65 @@
1-
use pavis_codec_api::{Codec, CodecError};
1+
use pavis_codec_api::{CheckedArtifact, Codec, CodecError};
2+
use pavis_core::RuntimeConfig;
3+
use pavis_ingest_api::{Artifact, Format};
24

35
pub type BoxedCodec = Box<dyn Codec<Error = CodecError> + Send + Sync>;
6+
7+
#[cfg(feature = "codec-serde")]
8+
#[derive(Debug, Default)]
9+
pub struct AutoSerdeCodec;
10+
11+
#[cfg(feature = "codec-serde")]
12+
impl AutoSerdeCodec {
13+
fn codec_for_format(format: Format) -> Result<pavis_codec_serde::SerdeCodec, CodecError> {
14+
let serde_format = match format {
15+
Format::Yaml => pavis_codec_serde::SerdeFormat::Yaml,
16+
Format::Json => pavis_codec_serde::SerdeFormat::Json,
17+
other => {
18+
return Err(CodecError::Check(anyhow::anyhow!(
19+
"Unsupported format: {:?}",
20+
other
21+
)));
22+
}
23+
};
24+
Ok(pavis_codec_serde::SerdeCodec {
25+
format: serde_format,
26+
})
27+
}
28+
}
29+
30+
#[cfg(feature = "codec-serde")]
31+
impl Codec for AutoSerdeCodec {
32+
type Error = CodecError;
33+
34+
fn check(&self, artifact: Artifact) -> Result<CheckedArtifact, Self::Error> {
35+
Self::codec_for_format(artifact.format)?.check(artifact)
36+
}
37+
38+
fn compile(&self, checked: &CheckedArtifact) -> Result<RuntimeConfig, Self::Error> {
39+
Self::codec_for_format(checked.artifact.format)?.compile(checked)
40+
}
41+
}
42+
43+
#[cfg(all(test, feature = "codec-serde"))]
44+
mod tests {
45+
use super::AutoSerdeCodec;
46+
use pavis_codec_api::{Codec, CompactionLevel};
47+
use pavis_ingest_api::{Artifact, Format, SourceInfo};
48+
use std::path::PathBuf;
49+
50+
fn fixture_path(name: &str) -> PathBuf {
51+
PathBuf::from(env!("CARGO_MANIFEST_DIR"))
52+
.join("../pavis-codec-serde/tests/fixtures")
53+
.join(name)
54+
}
55+
56+
#[test]
57+
fn auto_codec_accepts_json_artifacts() {
58+
let codec = AutoSerdeCodec;
59+
let bytes = std::fs::read(fixture_path("minimal.json")).expect("read fixture");
60+
let artifact = Artifact::new(bytes.into(), Format::Json, SourceInfo::unknown());
61+
62+
let validated = codec.materialize(artifact, CompactionLevel::Off);
63+
assert!(validated.is_ok());
64+
}
65+
}

crates/pavis/src/agent/driver.rs

Lines changed: 44 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ use tokio::sync::watch;
2323

2424
use crate::agent::fsm::{Effect, Event, Fsm, Response, VerifiedUpdate};
2525
use crate::agent::lkg::{load_lkg_config, tmp_path_for, write_atomic};
26+
use crate::reload::ensure_reload_safe;
2627
use crate::state::{RuntimeState, RuntimeStateHandle};
2728
use crate::telemetry::metrics::MetricsRegistry;
2829
use crate::validate_env::{self, RuntimeEnvError};
@@ -674,6 +675,8 @@ impl ConfigAgent {
674675
};
675676
let validated = unsafe { pavis_core::ValidatedRuntimeConfig::from_trusted(config) };
676677
let current = self.state.load();
678+
ensure_reload_safe(&current.config, &validated)
679+
.map_err(|err| self.record_validation_failure(err))?;
677680
if let Err(err) = validate_env::validate_runtime_env(&validated, Some(&current.config)) {
678681
let _ = tokio::fs::remove_file(&tmp_path).await;
679682
return Err(self.record_validation_failure(err.into()));
@@ -838,6 +841,29 @@ mod tests {
838841
ServiceName, Telemetry, TracingPolicy,
839842
};
840843
use pavis_pvs::PvsError;
844+
845+
fn test_validated_config(service_name: &str) -> pavis_core::ValidatedRuntimeConfig {
846+
let listener = ListenerBuilder::new()
847+
.name(ListenerName("test".to_string()))
848+
.address("127.0.0.1:0".parse().unwrap())
849+
.build()
850+
.unwrap();
851+
let config = RuntimeConfigBuilder::new()
852+
.telemetry(Telemetry {
853+
level: LogLevel::Info,
854+
pingora: LogLevel::Info,
855+
service_name: ServiceName(service_name.to_string()),
856+
metrics: Metrics::Disabled,
857+
access_log: AccessLogPolicy::Disabled,
858+
tracing: TracingPolicy::Disabled,
859+
})
860+
.add_listener(listener)
861+
.build()
862+
.unwrap();
863+
864+
pavis_core::validate_runtime(config).unwrap()
865+
}
866+
841867
#[test]
842868
fn test_classify_validation_error() {
843869
let err = anyhow::anyhow!(PvsError::VersionMismatch {
@@ -1031,7 +1057,10 @@ mod tests {
10311057
async fn test_config_agent_verify_update_success() {
10321058
let dir = tempfile::tempdir().unwrap();
10331059
let lkg = dir.path().join("lkg.pvs");
1034-
let state = Arc::new(RuntimeStateHandle::new(RuntimeState::default()));
1060+
let current = test_validated_config("current");
1061+
let state = Arc::new(RuntimeStateHandle::new(
1062+
RuntimeState::from_config(&current).unwrap(),
1063+
));
10351064
let agent = ConfigAgent::new(
10361065
"http://localhost".to_string(),
10371066
lkg,
@@ -1040,25 +1069,9 @@ mod tests {
10401069
)
10411070
.unwrap();
10421071

1043-
let listener = ListenerBuilder::new()
1044-
.name(ListenerName("test".to_string()))
1045-
.address("127.0.0.1:0".parse().unwrap())
1046-
.build()
1047-
.unwrap();
1048-
let config = RuntimeConfigBuilder::new()
1049-
.telemetry(Telemetry {
1050-
level: LogLevel::Info,
1051-
pingora: LogLevel::Info,
1052-
service_name: ServiceName("test".to_string()),
1053-
metrics: Metrics::Disabled,
1054-
access_log: AccessLogPolicy::Disabled,
1055-
tracing: TracingPolicy::Disabled,
1056-
})
1057-
.add_listener(listener)
1058-
.build()
1059-
.unwrap();
1072+
let config = test_validated_config("next");
10601073

1061-
let bytes = pavis_pvs::encode(&config).unwrap();
1074+
let bytes = pavis_pvs::encode(config.as_ref()).unwrap();
10621075
let etag = checksum_for_bytes(&bytes);
10631076

10641077
let update = agent
@@ -1277,25 +1290,10 @@ mod tests {
12771290

12781291
#[tokio::test]
12791292
async fn test_poll_once_updated() {
1280-
let listener = ListenerBuilder::new()
1281-
.name(ListenerName("test".to_string()))
1282-
.address("127.0.0.1:0".parse().unwrap())
1283-
.build()
1284-
.unwrap();
1285-
let config = RuntimeConfigBuilder::new()
1286-
.telemetry(Telemetry {
1287-
level: LogLevel::Info,
1288-
pingora: LogLevel::Info,
1289-
service_name: ServiceName("test".to_string()),
1290-
metrics: Metrics::Disabled,
1291-
access_log: AccessLogPolicy::Disabled,
1292-
tracing: TracingPolicy::Disabled,
1293-
})
1294-
.add_listener(listener)
1295-
.build()
1296-
.unwrap();
1293+
let current = test_validated_config("current");
1294+
let config = test_validated_config("next");
12971295

1298-
let bytes = pavis_pvs::encode(&config).unwrap();
1296+
let bytes = pavis_pvs::encode(config.as_ref()).unwrap();
12991297
let etag = checksum_for_bytes(&bytes);
13001298

13011299
let stub = RelayStub {
@@ -1307,7 +1305,9 @@ mod tests {
13071305

13081306
let dir = tempfile::tempdir().unwrap();
13091307
let lkg = dir.path().join("lkg.pvs");
1310-
let state = Arc::new(RuntimeStateHandle::new(RuntimeState::default()));
1308+
let state = Arc::new(RuntimeStateHandle::new(
1309+
RuntimeState::from_config(&current).unwrap(),
1310+
));
13111311
let agent = ConfigAgent::new(url, lkg, state, Duration::from_secs(1)).unwrap();
13121312

13131313
let outcome = agent.poll_once(100).await.unwrap();
@@ -1341,7 +1341,11 @@ mod tests {
13411341
async fn test_worker_run_loop_verify_and_apply() {
13421342
let dir = tempfile::tempdir().unwrap();
13431343
let lkg = dir.path().join("lkg.pvs");
1344-
let state = Arc::new(RuntimeStateHandle::new(RuntimeState::default()));
1344+
let current = test_validated_config("current");
1345+
let next = test_validated_config("next");
1346+
let state = Arc::new(RuntimeStateHandle::new(
1347+
RuntimeState::from_config(&current).unwrap(),
1348+
));
13451349
let agent = Arc::new(
13461350
ConfigAgent::new(
13471351
"http://localhost".to_string(),
@@ -1352,25 +1356,7 @@ mod tests {
13521356
.unwrap(),
13531357
);
13541358

1355-
let listener = ListenerBuilder::new()
1356-
.name(ListenerName("test".to_string()))
1357-
.address("127.0.0.1:0".parse().unwrap())
1358-
.build()
1359-
.unwrap();
1360-
let config = RuntimeConfigBuilder::new()
1361-
.telemetry(Telemetry {
1362-
level: LogLevel::Info,
1363-
pingora: LogLevel::Info,
1364-
service_name: ServiceName("test".to_string()),
1365-
metrics: Metrics::Disabled,
1366-
access_log: AccessLogPolicy::Disabled,
1367-
tracing: TracingPolicy::Disabled,
1368-
})
1369-
.add_listener(listener)
1370-
.build()
1371-
.unwrap();
1372-
1373-
let bytes = pavis_pvs::encode(&config).unwrap();
1359+
let bytes = pavis_pvs::encode(next.as_ref()).unwrap();
13741360
let etag = checksum_for_bytes(&bytes);
13751361

13761362
let (shutdown_tx, mut shutdown_rx) = watch::channel(false);

crates/pavis/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ pub mod listener;
55
pub mod load;
66
pub mod proxy;
77
pub mod regex_validator;
8+
pub mod reload;
89
pub mod retry;
910
pub mod router;
1011
pub mod shutdown;

0 commit comments

Comments
 (0)