Resolve plugin klangk.config values from klangkd.yaml (features_config:) (#1659)#1706
Merged
Conversation
…onfig Add a config-file source for plugin-declared dynamic keys (the keys the build emits into features.json: container_env_keys + the per-feature config blocks), alongside the server's env. A new features_config: block in klangkd.yaml supplies durable deploy values (OAuth client IDs, RAG endpoints) so they live in the committed config file instead of env. resolve_dynamic_config gains a features_config parameter; precedence per key is env > features_config: > plugin-declared default (env stays the per-invocation override). file:/cmd: prefixes are honored on YAML values too, consistent with the env path; unlike top-level KLANGK_* fields, a bad ref here logs and falls through to the default rather than aborting boot (same as a broken env ref). Plugins.container_env / frontend_config pass settings.features_config through. Builds on #1655 key-set bridge with no change to the bridge itself. KlangkSettings gains a features_config: dict[str, str] | None field; the _resolve_indirections validator only processes top-level str fields, so the dict values keep raw file:/cmd: prefixes for per-key deref at use time. Docs: klangkd-config.md section + table row; environment.md cross-ref under KLANGK_FEATURE_GITHUB_OAUTH_CLIENT_ID. Changelog: Added entry.
Address adversarial review (PR #1706) — logic and security model were sound, but three properties the brief/docs rely on were structurally true yet unverified by tests, plus two minor doc clarifications. Tests added: - A reserved/non-prefixed key (KLANGK_JWT_SECRET) inside a features_config: block is NOT injected into container_env — the KLANGK_FEATURE_ prefix guard runs before resolution, so the block can't bypass it. Locks in #1662's defense-in-depth against a future refactor that iterated features_config directly. - Empty-string env wins over features_config (env is consulted first and authoritative when set, even when empty). Matches pre-#1659 env behavior; pinned so a 'treat empty as unset' change is deliberate. - reload() (SIGHUP path) picks up edits to the features_config: block in the YAML — verifies the changelog's user-facing reloadable claim. Docs: - klangkd-config.md: note that numeric/boolean values in the block must be quoted (the field is str-typed; unquoted ints are rejected at construction, same as every other config-file field). - resolve_dynamic_config docstring: a broken file:/cmd: env ref returns the default, not the features_config value (env is authoritative when set; the block is a fallback for unset keys, not a recovery path for broken env values).
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
1 participant
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
Resolves #1659 — adds a config-file source for plugin-declared dynamic keys alongside the server's env, so long-lived deploy config (OAuth client IDs, RAG endpoints) can live in the committed
klangkd.yamlinstead of env.Builds on #1655's key-set bridge with no change to the bridge itself — only
resolve_dynamic_config's source set widens.Changes
KlangkSettings.features_config: dict[str, str] | None— a new field populated from thefeatures_config:block of the YAML config file. The_resolve_indirectionsvalidator only processes top-levelstrfields, so the dict's values keep their rawfile:/cmd:prefixes for per-key deref at use time.resolve_dynamic_config(key, default, features_config=None)— gains afeatures_configparameter. Precedence is now env >features_config:> plugin-declared default (env stays the per-invocation override).file:/cmd:prefixes are honored on YAML values too, consistent with the env path; unlike top-levelKLANGK_*fields, a badfile:/cmd:ref here does not abort boot (the values can't be resolved at construction) — it logs and falls through to the default, mirroring how a broken env ref already behaves. The parameter defaults toNone, so direct callers (tests, legacy paths) get the pre-Resolve plugin klangk.config values from klangkd.yaml (features_config:), in addition to env #1659 env-only behavior.Plugins.container_env/frontend_config— passsettings.features_configthrough toresolve_dynamic_config.Acceptance criteria
features_config:block supplies plugin-declared values; env still wins (precedence: env >features_config:> plugin default).file:/cmd:prefixes honored in the YAML values too (consistent with env + the rest ofresolve_dynamic_config).klangkd-config.mdsection + table row for the new block;environment.mdcross-references it underKLANGK_FEATURE_GITHUB_OAUTH_CLIENT_ID.klangkd.yaml.Tests
27 new tests, all green; full backend + CLI unit suite (3255 tests) passes.
TestResolveDynamicConfigFeaturesConfig(test_util.py, 11) — env wins overfeatures_config;features_configwins over default;file:/cmd:in YAML values; bad refs fall through to default;Nonepreserves env-only behavior; empty-string value treated as deliberate-empty.TestConfigFile(test_settings.py, 3) — the block loads from YAML verbatim;file:/cmd:prefixes kept raw (not resolved at construction); defaults toNone.TestFeaturesConfigSource(test_plugins.py, 8) —container_envandfrontend_configresolve fromsettings.features_configwhen env unset; env wins on both surfaces;file:prefix resolves;features_configwins over the plugin default; no-block path preserves env-only behavior.ruff check/ruff format,prettier,markdownlint, andtrufflehoghooks all clean.Out of scope (per the issue)
container_env_keys) — landed in Ship feature manifest (features.json) in wheel + KLANGK_PLUGINS_DIR retires from runtime + KLANGK_FEATURES_ENABLE runtime activation (canonical) #1655.KLANGK_FEATURES_ENABLE) — landed in Ship feature manifest (features.json) in wheel + KLANGK_PLUGINS_DIR retires from runtime + KLANGK_FEATURES_ENABLE runtime activation (canonical) #1655.Closes #1659 (the last open sub-issue of the #1607 parent tracker).