Support configuration overrides for troubleshooting - #2810
Open
francoisferrand wants to merge 2 commits into
Open
Support configuration overrides for troubleshooting#2810francoisferrand wants to merge 2 commits into
francoisferrand wants to merge 2 commits into
Conversation
Any configuration field should be changeable per process, without a new image or release, so that support can adjust a setting on a running platform. The named settings only cover the knobs a schema field exists for, and cannot reach objects with unconstrained keys, such as the librdkafka producer parameters. BACKBEAT_CONFIG_OVERRIDES now holds a JSON document applied to the configuration as a JSON Merge Patch, before validation: the merged result is validated as a whole, so a typo or a wrong type fails at startup rather than leaving the setting silently ignored. Each schema is given the fraction of the patch covering its own fields, applied after the environment variables derived from it, so that nothing silently overrides the escape hatch someone reached for precisely because the usual path did not work. It is applied over the configuration file and any other setting, so that nothing silently overrides the escape hatch someone reached for precisely because the usual path did not work. This stays an escape hatch: the named settings remain the supported way to configure backbeat. Issue: BB-809
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
JSON.parse sets a `__proto__` key as a plain member, so a `BACKBEAT_CONFIG_OVERRIDES` document naming one had it merged into Object.prototype: the configuration field the operator meant to set stayed untouched, and the override silently corrupted every object in the process instead. No configuration field is named that, and a JS object cannot hold such a member anyway, so the key is now dropped when the document is parsed, and ignored by the merge whatever its caller passes. Reaching the escape hatch takes operator access, but neither the parsing nor the merge should depend on that to stay harmless. Issue: BB-809
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
... and 1 file with indirect coverage changes
@@ Coverage Diff @@
## improvement/BB-808 #2810 +/- ##
======================================================
+ Coverage 75.73% 75.80% +0.06%
======================================================
Files 202 204 +2
Lines 14023 14054 +31
======================================================
+ Hits 10621 10653 +32
+ Misses 3392 3391 -1
Partials 10 10
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
francoisferrand
requested review from
a team,
SylvainSenechal and
benzekrimaha
August 18, 2026 16:48
|
|
||
| ``` | ||
| BACKBEAT_CONFIG_OVERRIDES='{"extensions":{"lifecycle":{"conductor":{"concurrency":20}}}}' | ||
| BACKBEAT_CONFIG_OVERRIDES='{"queuePopulator":{"batchMaxRead":null}}' |
There was a problem hiding this comment.
These two lines assign the same env var — only the second one takes effect, silently dropping the lifecycle override. An operator following this example would lose the concurrency change.
Either combine them into one JSON document (like the example below), or split into two separate code blocks each labeled as a standalone example.
Suggested change
| BACKBEAT_CONFIG_OVERRIDES='{"queuePopulator":{"batchMaxRead":null}}' | |
| BACKBEAT_CONFIG_OVERRIDES='{"extensions":{"lifecycle":{"conductor":{"concurrency":20}}},"queuePopulator":{"batchMaxRead":null}}' |
Comment on lines
+128
to
+134
| it('should restore the schema default when a field is deleted', () => { | ||
| assert.strictEqual(backbeatConfig.kafka.backlogMetrics.intervalS, 60); | ||
| process.env[CONFIG_OVERRIDES] = '{"kafka":{"backlogMetrics":{"intervalS":null}}}'; | ||
| config._parseConfig(testConfig); | ||
| // the joi default of the field, not the value of the config file | ||
| assert.strictEqual(config.kafka.backlogMetrics.intervalS, 60); | ||
| }); |
Contributor
There was a problem hiding this comment.
this test passes even with the null delete removed from mergePatch , I checked. The fixture value and the joi default are both 60, so the assertion cannot tell "field deleted, default re-applied" from "nothing happened"...
The delete itself is covered by the required-field test above, it's the restore the default part that isn't. Setting the field to something else first makes the assertion meaningful:
Suggested change
| it('should restore the schema default when a field is deleted', () => { | |
| assert.strictEqual(backbeatConfig.kafka.backlogMetrics.intervalS, 60); | |
| process.env[CONFIG_OVERRIDES] = '{"kafka":{"backlogMetrics":{"intervalS":null}}}'; | |
| config._parseConfig(testConfig); | |
| // the joi default of the field, not the value of the config file | |
| assert.strictEqual(config.kafka.backlogMetrics.intervalS, 60); | |
| }); | |
| it('should restore the schema default when a field is deleted', () => { | |
| testConfig.kafka.backlogMetrics.intervalS = 120; | |
| process.env[CONFIG_OVERRIDES] = '{"kafka":{"backlogMetrics":{"intervalS":null}}}'; | |
| config._parseConfig(testConfig); | |
| assert.strictEqual(config.kafka.backlogMetrics.intervalS, 60); | |
| }); |
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.
Any configuration field should be changeable per process, without a new image or release, so that support can adjust a setting on a running platform. The named settings only cover the knobs a schema field exists for, and cannot reach objects with unconstrained keys, such as the librdkafka producer parameters.
BACKBEAT_CONFIG_OVERRIDES now holds a JSON document applied to the configuration as a JSON Merge Patch, before validation: the merged result is validated as a whole, so a typo or a wrong type fails at startup rather than leaving the setting silently ignored. Each schema is given the fraction of the patch covering its own fields, applied after the environment variables derived from it, so that nothing silently overrides the escape hatch someone reached for precisely because the usual path did not work.
It is applied over the configuration file and any other setting, so that nothing silently overrides the escape hatch someone reached for precisely because the usual path did not work.
This stays an escape hatch: the named settings remain the supported way to configure backbeat.
Issue: BB-809