Repository navigation
Conversation
config.yaml values are not overridable via NB_ environment variables on the combined server, and config.yaml store fields overwrite the matching store environment variables at startup. Fix the Variable Precedence section and note the DSN precedence where the env-var alternative is shown.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
|
No actionable comments were generated in the recent review. Walkthrough
Merge Risk:
|
SunsetDrifter
left a comment
There was a problem hiding this comment.
Checked against v0.83.0 source and the netbirdio/netbird-server:0.83.0 image.
- Merge main: "Combined Server Variables" (#1079) documents
NB_AUTH_SECRETandNB_STORE_ENCRYPTION_KEYoverridingconfig.yamlfrom v0.83.0 (netbirdio/netbird#8405). Lines 286 and 288 contradict it. - MySQL was removed in v0.82.0 (netbirdio/netbird#7914); only the Postgres DSN is copied (
combined/cmd/root.go:131-138). server.accessLogStore.*is copied the same way (root.go:159-169) and is missing from the table.- Line 298 is false for the activity DSN:
activityStore.engine: postgreswith an emptydsnfails to start, and adsnwithout anengineis ignored. - Scope to v0.83.0: netbirdio/netbird#8515 reverses this.
netbird-server --helpalready claimsNB_SERVER_LOGLEVELworks (root.go:60-61).
| 3. Environment variables | ||
| 4. Command-line flags | ||
|
|
||
| The combined `netbird-server` reads `config.yaml` values only from the file and its built-in defaults. `NB_` environment variables named after `config.yaml` keys, such as `NB_SERVER_LOGLEVEL`, are ignored. |
There was a problem hiding this comment.
| The combined `netbird-server` reads `config.yaml` values only from the file and its built-in defaults. `NB_` environment variables named after `config.yaml` keys, such as `NB_SERVER_LOGLEVEL`, are ignored. | |
| In v0.83.0 and earlier, the combined `netbird-server` reads `config.yaml` values only from the file and its built-in defaults. `NB_` environment variables named after `config.yaml` keys, such as `NB_SERVER_LOGLEVEL`, are ignored. The exceptions, from v0.83.0, are `NB_AUTH_SECRET` and `NB_STORE_ENCRYPTION_KEY`, which override `config.yaml`. See [Combined Server Variables](#combined-server-variables). |
|
|
||
| The combined `netbird-server` reads `config.yaml` values only from the file and its built-in defaults. `NB_` environment variables named after `config.yaml` keys, such as `NB_SERVER_LOGLEVEL`, are ignored. | ||
|
|
||
| The store variables are the exception, and there `config.yaml` wins. At startup, `netbird-server` copies these `config.yaml` fields into the matching environment variables, replacing any value set on the container: |
There was a problem hiding this comment.
| The store variables are the exception, and there `config.yaml` wins. At startup, `netbird-server` copies these `config.yaml` fields into the matching environment variables, replacing any value set on the container: | |
| For the store fields below, `config.yaml` wins. At startup, `netbird-server` copies them into the matching environment variables, replacing any value set on the container: |
| | `server.store.dsn` (when `server.store.engine` is `postgres` or `mysql`) | `NB_STORE_ENGINE_POSTGRES_DSN` or `NB_STORE_ENGINE_MYSQL_DSN` | | ||
| | `server.store.file` | `NB_STORE_ENGINE_SQLITE_FILE` | | ||
| | `server.activityStore.engine` | `NB_ACTIVITY_EVENT_STORE_ENGINE` | | ||
| | `server.activityStore.dsn` (when `server.activityStore.engine` is set) | `NB_ACTIVITY_EVENT_POSTGRES_DSN` | | ||
| | `server.activityStore.file` | `NB_ACTIVITY_EVENT_SQLITE_FILE` | |
There was a problem hiding this comment.
| | `server.store.dsn` (when `server.store.engine` is `postgres` or `mysql`) | `NB_STORE_ENGINE_POSTGRES_DSN` or `NB_STORE_ENGINE_MYSQL_DSN` | | |
| | `server.store.file` | `NB_STORE_ENGINE_SQLITE_FILE` | | |
| | `server.activityStore.engine` | `NB_ACTIVITY_EVENT_STORE_ENGINE` | | |
| | `server.activityStore.dsn` (when `server.activityStore.engine` is set) | `NB_ACTIVITY_EVENT_POSTGRES_DSN` | | |
| | `server.activityStore.file` | `NB_ACTIVITY_EVENT_SQLITE_FILE` | | |
| | `server.store.dsn` (when `server.store.engine` is `postgres`) | `NB_STORE_ENGINE_POSTGRES_DSN` | | |
| | `server.store.file` | `NB_STORE_ENGINE_SQLITE_FILE` | | |
| | `server.activityStore.engine` | `NB_ACTIVITY_EVENT_STORE_ENGINE` | | |
| | `server.activityStore.dsn` (when `server.activityStore.engine` is set) | `NB_ACTIVITY_EVENT_POSTGRES_DSN` | | |
| | `server.activityStore.file` | `NB_ACTIVITY_EVENT_SQLITE_FILE` | | |
| | `server.accessLogStore.engine` | `NB_ACCESS_LOG_STORE_ENGINE` | | |
| | `server.accessLogStore.dsn` | `NB_ACCESS_LOG_POSTGRES_DSN` | | |
| | `server.accessLogStore.file` | `NB_ACCESS_LOG_SQLITE_FILE` | |
| | `server.activityStore.dsn` (when `server.activityStore.engine` is set) | `NB_ACTIVITY_EVENT_POSTGRES_DSN` | | ||
| | `server.activityStore.file` | `NB_ACTIVITY_EVENT_SQLITE_FILE` | | ||
|
|
||
| A variable set on the container takes effect only when its `config.yaml` field is empty. |
There was a problem hiding this comment.
| A variable set on the container takes effect only when its `config.yaml` field is empty. | |
| A variable set on the container takes effect only when its `config.yaml` field is empty. The activity DSN is the exception: with `server.activityStore.engine: postgres`, the server does not start without `server.activityStore.dsn`. To take it from the environment, leave both fields empty and set `NB_ACTIVITY_EVENT_STORE_ENGINE=postgres` and `NB_ACTIVITY_EVENT_POSTGRES_DSN`. |
Summary
Companion to netbirdio/netbird#8108, which moves the combined
netbird-serveronto a Viper-backed config loader. That PR doesn't change behavior:CombinedConfigopts no fields into environment or flag overrides, soconfig.yamlvalues still come only from the file and defaults. Its tests check thatNB_-prefixed variables such asNB_SERVER_LOGLEVELdon't override the file.The Variable Precedence section of Environment Variables said the reverse, listing "Configuration file → Environment variables → Command-line flags" for the combined setup too. This PR:
management.json).config.yamlkeys are read only from the file and defaults, andNB_variables named after those keys are ignored.netbird-servercopiesserver.store.*andserver.activityStore.*fields fromconfig.yamlinto the matchingNB_STORE_*/NB_ACTIVITY_EVENT_*variables, soconfig.yamlwins. A table lists each field and its variable (fromcombined/cmd/root.go).Testing
combined/cmd/config.go,config_test.go) and the store env handling incombined/cmd/root.goonfeature/combined-config-loader.npm run buildandnpm run lint:mdxpass.Summary by CodeRabbit