[PR #6668] [management] Allow activityStore DSN via env when using postgres #28724

Open
opened 2026-08-05 08:06:49 -04:00 by saavagebueno · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/netbirdio/netbird/pull/6668
Author: @Tarasusrus
Created: 7/4/2026
Status: 🔄 Open

Base: mainHead: fix/activitystore-dsn-env


📝 Commits (4)

  • 487f964 [management] Allow activityStore DSN via env when using postgres
  • 7fa2ae6 [management] Address review: empty env DSN and missing-DSN regression test
  • f3a0652 [management] Restore prior env state in unset-DSN subtest
  • 9984e5f [management] Isolate activity-store env mutations in tests

📊 Changes

2 files changed (+96 additions, -1 deletions)

View changed files

📝 combined/cmd/root.go (+3 -1)
combined/cmd/root_test.go (+93 -0)

📄 Description

Summary

  • initializeConfig() rejected server.activityStore.engine: postgres whenever server.activityStore.dsn was empty in config.yaml, even though the store layer (management/server/activity/store/sql_store.go) already reads NB_ACTIVITY_EVENT_POSTGRES_DSN as a fallback.
  • This blocked env-only / Kubernetes-style deployments (secrets via env, not on-disk config) from configuring the activity store on postgres.
  • Fix: the eager check in combined/cmd/root.go now also accepts the DSN via NB_ACTIVITY_EVENT_POSTGRES_DSN before failing.

Fixes #5976.

Test plan

  • Added a red/green unit test (combined/cmd/root_test.go) reproducing the exact reported error, now passing after the fix
  • go test ./combined/cmd/... passes
  • golangci-lint run ./combined/cmd/... clean
  • gofmt clean

Note for maintainers

While investigating this, I found the same class of problem is worse for authStore (embedded IdP storage): combined/cmd/config.go:576 requires authStore.dsn in config.yaml with no environment variable fallback at all anywhere in the codebase (unlike activityStore/main store). Kept out of scope here since it's a separate code path; happy to open a follow-up issue/PR if useful.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Updated PostgreSQL activity storage configuration validation: when server.activityStore.engine is set to postgres and the DSN is missing, the required DSN is now taken from the NB_ACTIVITY_EVENT_POSTGRES_DSN environment variable.
    • Improved the error message when the PostgreSQL DSN is not provided.
  • Tests
    • Added regression tests covering successful DSN loading from the environment variable and failure when the environment variable is unset or empty.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/netbirdio/netbird/pull/6668 **Author:** [@Tarasusrus](https://github.com/Tarasusrus) **Created:** 7/4/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/activitystore-dsn-env` --- ### 📝 Commits (4) - [`487f964`](https://github.com/netbirdio/netbird/commit/487f96474fdd0a5c7890e385f1e92d5476caeb28) [management] Allow activityStore DSN via env when using postgres - [`7fa2ae6`](https://github.com/netbirdio/netbird/commit/7fa2ae600100904040318c6a200ec39f748066a0) [management] Address review: empty env DSN and missing-DSN regression test - [`f3a0652`](https://github.com/netbirdio/netbird/commit/f3a0652ba27625b3b25cd9996cc18236429f6738) [management] Restore prior env state in unset-DSN subtest - [`9984e5f`](https://github.com/netbirdio/netbird/commit/9984e5f803baff7c8c0cc388da39d0a7ac27bcd9) [management] Isolate activity-store env mutations in tests ### 📊 Changes **2 files changed** (+96 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `combined/cmd/root.go` (+3 -1) ➕ `combined/cmd/root_test.go` (+93 -0) </details> ### 📄 Description ## Summary - `initializeConfig()` rejected `server.activityStore.engine: postgres` whenever `server.activityStore.dsn` was empty in `config.yaml`, even though the store layer (`management/server/activity/store/sql_store.go`) already reads `NB_ACTIVITY_EVENT_POSTGRES_DSN` as a fallback. - This blocked env-only / Kubernetes-style deployments (secrets via env, not on-disk config) from configuring the activity store on postgres. - Fix: the eager check in `combined/cmd/root.go` now also accepts the DSN via `NB_ACTIVITY_EVENT_POSTGRES_DSN` before failing. Fixes #5976. ## Test plan - [x] Added a red/green unit test (`combined/cmd/root_test.go`) reproducing the exact reported error, now passing after the fix - [x] `go test ./combined/cmd/...` passes - [x] `golangci-lint run ./combined/cmd/...` clean - [x] `gofmt` clean ## Note for maintainers While investigating this, I found the same class of problem is worse for `authStore` (embedded IdP storage): `combined/cmd/config.go:576` requires `authStore.dsn` in `config.yaml` with **no environment variable fallback at all** anywhere in the codebase (unlike activityStore/main store). Kept out of scope here since it's a separate code path; happy to open a follow-up issue/PR if useful. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Updated PostgreSQL activity storage configuration validation: when `server.activityStore.engine` is set to `postgres` and the DSN is missing, the required DSN is now taken from the `NB_ACTIVITY_EVENT_POSTGRES_DSN` environment variable. * Improved the error message when the PostgreSQL DSN is not provided. * **Tests** * Added regression tests covering successful DSN loading from the environment variable and failure when the environment variable is unset or empty. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
saavagebueno added the pull-request label 2026-08-05 08:06:49 -04:00
Sign in to join this conversation.
No Label pull-request
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DYNR/netbird#28724