Load runtime config from config store#685
Conversation
ChristianPavilonis
force-pushed
the
feature/ts-config-in-config-store
branch
from
52963d2 to
b8c0ab5
Compare
prk-Jr
left a comment
prk-Jr
left a comment
There was a problem hiding this comment.
🔧 Request changes: config-store validation currently does not cover the full runtime startup surface.
🔧 crates/trusted-server-core/src/runtime_config.rs:42 validates only Settings::validate() before canonicalizing. Runtime startup still performs additional fallible construction later in crates/trusted-server-adapter-fastly/src/main.rs:97 and :106 via build_orchestrator(&settings) and IntegrationRegistry::new(&settings). I reproduced this by setting [integrations.prebid] server_url = ""; ts-config-canonicalize exited 0 and produced a config hash, even though the existing empty_prebid_server_url_fails_orchestrator_startup test documents that the same config fails startup. Please make load_runtime_config run the same startup validation needed to serve requests, or share a validate_runtime_startup(&Settings) helper between canonicalization and Fastly startup.
🔧 crates/trusted-server-core/src/settings.rs:97 / crates/trusted-server-core/src/integrations/prebid.rs:54: strict parsing still preserves unknown fields under known integrations. IntegrationSettings stores integration payloads as flattened JsonValue maps, and built-in typed configs such as PrebidIntegrationConfig do not deny unknown fields. I reproduced this by adding server_urll = "https://typo.example" under [integrations.prebid]; canonicalization exited 0 and preserved the typo in output. That lets misspelled production config be published while typed runtime code ignores it. Please reject unknown fields for built-in integration configs, either with deny_unknown_fields on typed structs or a registry-level validation pass during load_runtime_config.
🔧 crates/integration-tests/fixtures/configs/trusted-server.integration.toml:14 hardcodes origin_url = "http://127.0.0.1:8888", but scripts/integration-tests.sh and scripts/integration-tests-browser.sh still expose INTEGRATION_ORIGIN_PORT and export old TRUSTED_SERVER__* build-time overrides. Since this change removes env merging from runtime config, setting INTEGRATION_ORIGIN_PORT now changes the harness port without changing the config-store value consumed by Trusted Server. Please template/render the integration TOML with the selected port, or remove the port override and stale env-var build comments.
🔧 docs/guide/configuration.md and docs/guide/error-reference.md still instruct operators to use removed TRUSTED_SERVER__* overrides. For example, configuration.md says env overrides are no longer merged, but later has many Environment Override sections starting around line 104; error-reference.md says env loading was removed around line 130, but later recommends TRUSTED_SERVER__EDGE_COOKIE__SECRET_KEY and TRUSTED_SERVER__INTEGRATIONS__PREBID__DEBUG. Please replace these with TOML/config-store examples or clearly mark them as legacy/non-runtime-only if any tooling still consumes them.
😃 The fail-closed health behavior is a strong part of this change. Loading config before serving /health means missing or invalid config-store state makes the service unhealthy instead of silently serving with defaults.
Local verification on PR head:
- cargo fmt --all -- --check exited 0
- cargo clippy --workspace --all-targets --all-features -- -D warnings exited 0
- cargo test --workspace exited 0
| let settings = Settings::from_toml(toml_str)?; | ||
|
|
||
| settings | ||
| .validate() |
There was a problem hiding this comment.
🔧 This validation step is narrower than runtime startup. load_runtime_config accepts configs that build_orchestrator or IntegrationRegistry::new reject later; I reproduced this with [integrations.prebid] server_url = "" and ts-config-canonicalize still exited 0 with a hash. Please run the same startup validation here, or through a shared helper, before canonicalizing/publishing the config.
| message: "Failed to serialize validated configuration".to_string(), | ||
| })?; | ||
|
|
||
| let canonical_value = retain_declared_fields( |
There was a problem hiding this comment.
🔧 This canonicalization path still preserves unknown fields under known integrations because Settings::from_toml keeps integration payloads as raw JsonValue. Repro: server_urll = "https://typo.example" under [integrations.prebid] canonicalizes and survives output, while typed Prebid config ignores it. Please reject unknown fields for built-in integrations during runtime config loading.
| [publisher] | ||
| domain = "test-publisher.com" | ||
| cookie_domain = ".test-publisher.com" | ||
| origin_url = "http://127.0.0.1:8888" |
There was a problem hiding this comment.
🔧 This hardcodes 8888, but the integration scripts still expose INTEGRATION_ORIGIN_PORT and export old TRUSTED_SERVER__PUBLISHER__ORIGIN_URL build-time overrides. Since env merging is gone, a non-default INTEGRATION_ORIGIN_PORT no longer changes the runtime config-store payload. Please template/render this fixture from the selected port, or remove the port override and stale env-var build comments.
|
|
||
| > **Note:** Older revisions of this guide referred to build-time | ||
| > `TRUSTED_SERVER__*` application-setting overrides. That mechanism has been | ||
| > removed for application configuration. |
There was a problem hiding this comment.
🔧 This says the TRUSTED_SERVER__* application-setting mechanism was removed, but the section immediately below still documents the override format and the rest of the page keeps many Environment Override examples. Operators following this page will set env vars that runtime no longer reads. Please convert those examples to TOML/config-store examples, or mark them legacy/test-only if they are only retained for compatibility tests.
| log::debug!("Loaded runtime config hash={}", loaded_config.config_hash); | ||
| log::debug!("Settings {settings:?}"); | ||
|
|
||
| // `/health` intentionally depends on successful runtime config loading. |
There was a problem hiding this comment.
😃 Good call making /health depend on successful config-store loading. This makes readiness fail closed when ts-config is missing or invalid instead of serving with defaults.
|
Closing in favor of fuller CLI PR (#669) |
2 participants
Summary
ts_config_storekeyts-configinstead of baking TOML into the binary.Changes
crates/trusted-server-adapter-fastly/src/main.rscrates/trusted-server-core/src/runtime_config.rscrates/trusted-server-core/src/settings.rscrates/trusted-server-core/build.rs/src/settings_data.rscrates/trusted-server-core/src/bin/ts-config-canonicalize.rsscripts/fastly-dev.sh/scripts/render-fastly-local-config.pyfastly.local.toml.trusted-server.example.tomlcrates/integration-tests/**docs/guide/**/docs/superpowers/specs/2026-04-20-config-store-runtime-config-design.mdCloses
Closes #166
Test plan
cargo test --workspacecargo clippy --workspace --all-targets --all-features -- -D warningscargo fmt --all -- --checkcd crates/js/lib && npx vitest runcd crates/js/lib && npm run formatcd docs && npm run formatcargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1fastly compute serveChecklist
unwrap()in production code — useexpect("should ...")println!)