Override most config values with env var - #2850
Open
dwwoelfel wants to merge 1 commit into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughConfiguration loading now derives environment variable names from declared string-valued keys, reads matching ChangesConfiguration environment overrides
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant ConfigLoader
participant env-overrides
participant Environment
participant DecryptedConfig
ConfigLoader->>DecryptedConfig: decrypt and transform EDN
ConfigLoader->>env-overrides: request environment overrides
env-overrides->>Environment: read INSTANT_CONFIG_* variables
Environment-->>env-overrides: return matching values
env-overrides-->>ConfigLoader: return obfuscated key-value map
ConfigLoader->>DecryptedConfig: merge overrides
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
server/test/instant/config_edn_test.clj (1)
50-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest populated environment overrides.
This test checks only an empty environment and
overridable-key-type. It does not verify thatenv-overridesreads a value, keeps:s3-endpointplain, and obfuscates:postmark-token.Inject an environment lookup function, or use an equivalent fixture. Add assertions for both value types.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/test/instant/config_edn_test.clj` around lines 50 - 58, Extend env-overrides-obfuscate-secrets-only to supply a populated environment lookup fixture to config-edn/env-overrides, then assert that :s3-endpoint returns its plain configured value while :postmark-token returns an obfuscated value. Keep the existing empty-environment and overridable-key-type assertions, and use the existing crypt-util/obfuscate behavior rather than bypassing it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/src/instant/config_edn.clj`:
- Around line 216-234: Update decrypted-config so raw environment overrides are
merged into the source config-edn map before required-key validation and
s/conform; then apply the existing obfuscation/decryption postwalk and
env-overrides handling to the validated combined config without using
already-obfuscated values for validation. Add a regression test covering a
required production string key absent from EDN but supplied through
INSTANT_CONFIG_DATABASE_CLUSTER_ID.
---
Nitpick comments:
In `@server/test/instant/config_edn_test.clj`:
- Around line 50-58: Extend env-overrides-obfuscate-secrets-only to supply a
populated environment lookup fixture to config-edn/env-overrides, then assert
that :s3-endpoint returns its plain configured value while :postmark-token
returns an obfuscated value. Keep the existing empty-environment and
overridable-key-type assertions, and use the existing crypt-util/obfuscate
behavior rather than bypassing it.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c10b81ef-8db0-4b94-8e72-a3fbeac1b5f4
📒 Files selected for processing (2)
server/src/instant/config_edn.cljserver/test/instant/config_edn_test.clj
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.
Allows most config values from the config edn file to be supplied by an env var.
We don't support