Repository navigation
fix: config env overrides - lenient env source, seeded defaults, CLI flag precedence (#92) - #103
Merged
Merged
Conversation
…flag precedence (#92) Three related fixes for the config problems in #92: 1. Documented non-config env vars (INPUTLAYER_BOOTSTRAP_API_KEY, INPUTLAYER_ADMIN_PASSWORD, INPUTLAYER_API_KEY) crashed --config startup because figment rejected unknown keys from the env source. The env source now passes through only the known top-level sections (storage/optimization/logging/http); TOML parsing stays strict. 2. The no-config-file path used Config::default() directly, so INPUTLAYER_* overrides did nothing. load() and from_file() now seed Serialized::defaults first, so env overrides always apply and a partial config file only needs the fields it changes. 3. The server's clap defaults for --host/--port clobbered file and env values unconditionally. They are now Options applied only when explicitly passed: CLI > env > file > defaults. Verified live: INPUTLAYER_HTTP__PORT=8082 with the bootstrap env vars set boots and binds 8082. Three new figment::Jail tests; full lib suite 2630 passed.
jsam
changed the title
With seeded defaults, a missing file silently produced a default config; tests/config_tests.rs rightly asserts an explicit --config path that does not exist is an error. Now checked explicitly. Full test suite run this time (33 binaries), not just --lib.
…loud load errors Adversarial review of this PR found the defaults seed broke configs using the documented storage aliases and silently flipped defaults: 1. The seed emits canonical field names, so a config using default_database / auto_create_databases (both documented, both in the shipped template) failed as a serde duplicate field. A provider wrapper now renames aliases to canonical names before figment merges - aliases work in files AND env vars (they never worked via env before). 2. The seed was Config::default(), whose hand-written values diverge from the field-level serde defaults that governed omitted keys (enable_join_planning false->true against its own 'disabled by default' comment, max_result_rows 0->100000 silently truncating results, gui.enabled). The seed is now built through serde with explicit empty sections, so field-level defaults are authoritative everywhere. 3. server.rs swallowed real load() errors as 'Using default configuration', running on pure defaults after e.g. one bad env value. Configuration errors now print and exit non-zero. 4. The shipped config.toml template referenced the removed [http.auth] enabled field; a new test asserts the template parses. New tests: aliases via file and env, seed-matches-field-defaults, shipped-template-parses. Full suite: 33 binaries green.
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
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.
What
Fixes the config problems from #92, plus a third layer found while reproducing it:
Tested
Remaining from #92 (not in this PR)
The shipped config.toml template still references removed fields ([http.auth] enabled, etc.). Seeded defaults make partial/stale files much less likely to hard-fail, but the template itself still deserves a refresh pass.
Merge order
Independent - no conflicts with #98/#100/#101/#102; mergeable any time.
Refs #92