Configure backbeat from the environment, without rewriting the config file - #2809
Open
francoisferrand wants to merge 3 commits into
Open
Configure backbeat from the environment, without rewriting the config file#2809francoisferrand wants to merge 3 commits into
francoisferrand wants to merge 3 commits into
Conversation
The log configuration only defaulted as a whole object: a section setting just one level left the other undefined, and werelogs was then configured with an undefined dump level. The per-extension log section turned that gap into a hard failure by requiring both levels, which leaves no way to set only one — and an override from the environment always produces such a partial section, since a variable names a single field. Each level now carries its own default, shared by the global and the per-extension schemas, so setting one leaves the other at its default instead of unset or rejected. Issue: BB-808
docker-entrypoint.sh applied every environment variable by rewriting conf/config.json in place with jq. That fails outright when the file is mounted read-only, as zenko does: setting any mapped variable crashed the container at startup. The mapping was also a hand-maintained shell table duplicated from the joi schema, and drifting from it. Overrides are now applied in memory before validation, and their names are derived from the schema itself, so a new configuration field becomes settable from the environment as soon as it is declared. The names that do not follow from the config path are annotated next to the field they name, and the few variables setting several fields at once are kept in an explicit table. Values go through the schema like any other configuration: an invalid one fails the startup instead of being silently ignored. Every variable the entrypoint used to apply keeps working, pinned by a test: Zenko and Federation set them, and Federation forwards arbitrary ones from the field. Two exceptions, both already already broken: * the `EXTENSIONS_LIFECYCLE_RULES_*` and `REDIS_LOCALCACHE_*` variables wrote config sections the schema no longer knows, so setting one made validation reject the whole file and the process fail to start - they are now simply ignored; * `MONGODB_HOSTS` no longer selects the "mongo" log source -not supported anymore- and only sets the replica set hosts of the shared MongoDB connection. Issue: BB-808
Setting a per-extension log level silently reset the dump level to a constant, dropping whatever a deployment had configured globally. An extension now inherits the levels it does not configure from the global log config, so it overrides only what it names — which is also what an override from the environment produces, since a variable names a single field. Extension schemas reach the global configuration through the validation context. The extension validators were already given it, but as the configuration object still being built, so nothing could be read from it. The extension configurations are kept out of it: they are validated one after the other, so referencing one would resolve differently depending on the order of the configuration file, and an extension must not depend on another one. Issue: BB-808
Contributor
Hello francoisferrand,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Contributor
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
francoisferrand
changed the base branch from
development/9.0
to
development/9.5
August 18, 2026 16:24
Contributor
DarkIsDude
requested changes
Aug 19, 2026
DarkIsDude
left a comment
DarkIsDude
left a comment
There was a problem hiding this comment.
Sorry for the review. I thought it'll be a fast one but finally not really.
With what we have, if we have for example SERVER_PORT as an env var and a extension.Replication.Server.Port, the env var can have an impact. Those very short env var name and generic, can be very dangerous. Setting a short env var car have a wide impact ? What do you think ? Maybe I'm not very clear, let me know by slack if it's the case.
| ENV AWS_SDK_JS_SUPPRESS_MAINTENANCE_MODE_MESSAGE=1 | ||
|
|
||
| ENTRYPOINT ["tini", "-g", "--", "/usr/src/app/docker-entrypoint.sh"] | ||
| ENTRYPOINT ["tini", "-g", "--"] |
|
|
||
| const { MAX_QUEUED_DEFAULT } = require('../../lib/constants').backbeatConsumer; | ||
|
|
||
| // the historic env var names put the backend before `RETRY`, e.g. |
| const logLevelJoi = joi.alternatives() | ||
| .try('error', 'warn', 'info', 'debug', 'trace'); | ||
|
|
||
| // the levels default individually, so that setting one of them, from the |
There was a problem hiding this comment.
It was a decision made by Sylvain to required both or none. Why did you change that ?
| @@ -0,0 +1,368 @@ | |||
| 'use strict'; | |||
|
|
|||
| /* eslint-disable no-param-reassign */ | |||
There was a problem hiding this comment.
That's a bit aggresive ? Why do you want to disable it globally ?
| The fields of an object with unconstrained keys, such as `kafka.producerParams`, | ||
| have no derived name, and neither have the fields the schema forbids. | ||
|
|
||
| ## Values |
There was a problem hiding this comment.
I'm not sure we need that, it's pretty standard ?
| } | ||
|
|
||
| module.exports = configValidator; | ||
| module.exports = extensionConfigValidator('gc', joiSchema); |
There was a problem hiding this comment.
This is never tested and can be a huge source of regression (same for other extentions)
| }); | ||
|
|
||
| it('should not derive a name for the params BackbeatProducer sets itself', () => { | ||
| const names = [...envVarMappings(backbeatConfigJoi).keys()]; |
There was a problem hiding this comment.
Not really easy to read this test. Better to use your own config and don't reuse production one
| try { | ||
| return JSON.parse(value); | ||
| } catch (err) { | ||
| throw new Error(`invalid JSON value for ${name}: ${err.message}`); |
There was a problem hiding this comment.
I'll let you decide but, ready the test file, we could split this one with:
- One for generic function (getPath, setPath, ...)
- One for managing composite env var
- One for managing replacement
- One for the extention logic
- Of course an index to expose what is needed to validation
I'm not sure it's useful, so I'll let you decide
|
Should land to the next release ? |
Codecov Report❌ Patch coverage is
Additional details and impacted files
... and 3 files with indirect coverage changes
@@ Coverage Diff @@
## development/9.5 #2809 +/- ##
===================================================
- Coverage 75.76% 75.73% -0.03%
===================================================
Files 200 202 +2
Lines 13922 14023 +101
===================================================
+ Hits 10548 10621 +73
- Misses 3364 3392 +28
Partials 10 10
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
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
3 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.
The docker entrypoint applied env-var configuration by rewriting conf/config.json in place with jq. That breaks when the config is mounted read-only, which is what zenko-operator does: setting any mapped variable made the container exit at startup. The mapping was also a hand-maintained shell table duplicated from the joi schema, and had drifted from it — LOG_LEVEL rejected the warn and error levels the schema accepts, and a few variables whose config section had since been removed were written back in, so setting one stopped the process from starting.
Overrides are now applied in memory before validation, with their names derived from the schemas themselves: a new configuration field is settable from the environment as soon as it is declared. Names that don't follow from the config path are annotated next to the field they name, which is how the historic ones keep working, and the handful of variables setting several fields at once stay in an explicit table. Values go through the schema like any other configuration, so an invalid one fails the startup instead of being silently ignored. docs/configuration.md documents the whole thing.
Every variable the entrypoint applied is pinned by a test, since zenko-operator and Federation set them and Federation forwards arbitrary ones from the field. Two deliberate exceptions, both already dead: MONGODB_HOSTS no longer selects the mongo log source dropped in BB-836, and the lifecycle rules and local cache variables are ignored rather than fatal.
Issue: BB-808