Fix env var config roundtrip through build.rs TOML serialization#362
Merged
Fix env var config roundtrip through build.rs TOML serialization#362
Conversation
aram356
force-pushed
the
fix/env-var-config-roundtrip
branch
6 times, most recently
from
3784e50 to
ac2b267
Compare
Three bugs caused env-var-sourced integration settings to lose their
types when build.rs serialized Settings to TOML:
1. collect_env_vars in build.rs skipped empty objects, arrays, and null
values, so new env vars for those fields never triggered a rebuild.
2. vec_from_seq_or_map hard-failed on bracket-wrapped strings that
aren't valid JSON (e.g. glob patterns like [/.static/prebid/{*rest}]).
Now falls back to bracket-stripping + comma-splitting.
3. Env vars arrive as strings ("true", "1000") in the config crate's
HashMap<String, JsonValue>. Without eager normalization, toml's
serializer wrote them as TOML strings, so the runtime re-parse
produced String("true") instead of Bool(true). Added
IntegrationSettings::normalize() called from a new from_toml_and_env
path used only by build.rs, keeping the runtime from_toml path clean.
Also replaced the publisher.origin_url trailing-slash normalization
with a validator that rejects trailing slashes at validation time.
aram356
force-pushed
the
fix/env-var-config-roundtrip
branch
from
ac2b267 to
543a89b
Compare
5 tasks
- Remove Settings::new() which read raw source TOML instead of the
build-output TOML, making it broken after the from_toml/from_toml_and_env
split. The canonical runtime entry point is get_settings() in
settings_data.rs.
- Move InsecureSecretKey and certificate_check guards from the removed
Settings::new() into get_settings() where they belong.
- Add .validate() call in from_toml_and_env() so trailing-slash and
other validation errors are caught at build time, not just at runtime.
- Add ends_with(']') guard to vec_from_seq_or_map bracket fallback to
prevent incorrect slicing on malformed input like "[foo".
- Update docs and comments to reflect that env var overrides are
build-time only (baked by build.rs), not runtime.
- Remove unused `use core::str` import and test_settings_new test.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix env var → build.rs → embedded TOML → runtime config pipeline so that
TRUSTED_SERVER__INTEGRATIONS__*env vars preserve correct types through the roundtrip.Changes
build.rscollect_env_vars+rerun-if-env-changedwith always-rerun sentinel; conditional output write; callfrom_toml_and_envsettings.rsfrom_toml(runtime,toml::from_str) /from_toml_and_env(build-time, config crate + normalize); addIntegrationSettings::normalize();vec_from_seq_or_mapbracket fallback;validate_no_trailing_slashreplacesPublisher::normalize(); 3 new testsCargo.tomltomlmoved from[dev-dependencies]to[dependencies]Closes
Closes #363
Closes #346
Test plan
cargo test --workspace— 367 tests passtemp_envtests updated to usefrom_toml_and_envtest_env_var_roundtrip_normalizes_integration_types— full build.rs pipeline: env vars →from_toml_and_env→ TOML serialize →from_toml→ typed configtest_from_toml_ignores_env_vars— runtime path does not read env varsvalidate_rejects_trailing_slash_in_origin_url— validation rejects trailing/Checklist
unwrap()in production code — useexpect("should ...")