[PR #6047] [CLOSED] Phase 1: connection-mode enum + backwards-compat (issue #5989) #28955

Open
opened 2026-08-05 08:07:12 -04:00 by saavagebueno · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/netbirdio/netbird/pull/6047
Author: @MichaelUray
Created: 5/1/2026
Status: Closed

Base: mainHead: feat/connection-mode-phase1


📝 Commits (10+)

  • d5d416e proto: add ConnectionMode enum and p2p/relay timeout fields to PeerConfig
  • b341bc0 client: add connectionmode package with Mode type and proto bridge
  • 9cad8d6 client/peer: ResolveModeFromEnv with NB_CONNECTION_MODE and deprecation warns
  • a4b0d04 client: add --connection-mode, --relay-timeout, --p2p-timeout CLI flags
  • 62f29c5 client/conn_mgr: replace asymmetric Lazy/ForceRelay precedence with Mode
  • 0d48124 client/peer: connection mode drives skip-ICE branch in Open()
  • 6d40418 client/engine: forward resolved Mode to per-peer ConnConfig
  • be96444 mgmt/types: add ConnectionMode + p2p/relay timeout to Settings
  • 92885d4 openapi: add connection_mode + p2p/relay timeout fields to AccountSettings
  • b00fee3 mgmt/handlers/accounts: accept connection_mode + timeout settings on PUT

📊 Changes

30 files changed (+2027 additions, -521 deletions)

View changed files

📝 client/cmd/root.go (+13 -0)
📝 client/cmd/up.go (+30 -0)
📝 client/internal/conn_mgr.go (+127 -35)
client/internal/conn_mgr_test.go (+101 -0)
📝 client/internal/connect.go (+28 -0)
📝 client/internal/engine.go (+19 -2)
📝 client/internal/lazyconn/env.go (+5 -0)
📝 client/internal/peer/conn.go (+14 -4)
📝 client/internal/peer/env.go (+84 -0)
client/internal/peer/env_test.go (+61 -0)
📝 client/internal/profilemanager/config.go (+33 -0)
📝 client/internal/profilemanager/config_test.go (+49 -0)
📝 client/proto/daemon.pb.go (+79 -11)
📝 client/proto/daemon.proto (+16 -0)
📝 client/proto/daemon_grpc.pb.go (+180 -222)
📝 management/internals/shared/grpc/conversion.go (+30 -1)
📝 management/internals/shared/grpc/conversion_test.go (+118 -0)
📝 management/server/account.go (+65 -1)
📝 management/server/account_test.go (+96 -0)
📝 management/server/activity/codes.go (+14 -0)

...and 10 more files

📄 Description

Summary

This PR is Phase 1 of three that together implement issues #5989 and #5990 (the connection-mode consolidation RFC and its server-side per-peer/per-group companion).

In Scope:

  • New ConnectionMode proto enum with four reserved values (relay-forced, p2p, p2p-lazy, p2p-dynamic); three are functional in this PR. p2p-dynamic is reserved on the wire and in the DB but the daemon currently treats it like p2p (pass-through). The decoupled worker_relay/worker_ice lifecycle that makes p2p-dynamic distinct comes in Phase 2.
  • Two new account-level settings on the management server: relay_timeout_seconds, p2p_timeout_seconds (both nullable, NULL = built-in default).
  • Replaces the asymmetric Lazy/ForceRelay precedence in client/internal/conn_mgr.go with a single resolver: client env > client config > server-pushed.
  • Full backwards-compatibility:
    • Old clients still see only the legacy lazy_connection_enabled boolean (mapped from the resolved mode by toPeerConfig).
    • Old servers send connection_mode = UNSPECIFIED (proto default 0) and the new client falls back to the legacy boolean -- verified via tests.
    • NB_FORCE_RELAY, NB_ENABLE_EXPERIMENTAL_LAZY_CONN, --enable-lazy-connection, and NB_LAZY_CONN_INACTIVITY_THRESHOLD continue to work and emit one-shot deprecation warnings.

The companion Dashboard PR is at netbirdio/dashboard#627 (will be added once it exists).

Implementation map

Layer Files
proto shared/management/proto/management.proto, client/proto/daemon.proto
Mode type shared/connectionmode/ (Mode, ParseString, FromProto, ToProto, ResolveLegacyLazyBool, ToLazyConnectionEnabled)
client env / CLI client/internal/peer/env.go (ResolveModeFromEnv), client/cmd/{root,up}.go (--connection-mode, --relay-timeout, --p2p-timeout)
client engine / conn-mgr client/internal/{engine,conn_mgr,connect}.go (resolveConnectionMode + UpdatedRemotePeerConfig + EngineConfig fields)
client per-peer client/internal/peer/conn.go (Mode-driven skip-ICE branch)
mgmt server management/server/types/settings.go (new nullable columns), management/server/http/handlers/accounts/accounts_handler.go (PUT validation + response), management/server/account.go (audit emission), management/server/activity/codes.go (3 new event codes), management/internals/shared/grpc/conversion.go (toPeerConfig writes both old and new wire fields)
OpenAPI shared/management/http/api/openapi.yml + regenerated types.gen.go
tests shared/connectionmode/mode_test.go, client/internal/peer/env_test.go, client/internal/conn_mgr_test.go, management/internals/shared/grpc/conversion_test.go

Hardware-tested

Verified on a real production NetBird instance with 32 connected peers across 12 OpenWrt-router versions (22.03 to 25.12), Windows 10/11, Debian, Android 12/14, and iOS 26.3.1:

  • Pre-deploy baseline: 32 connected.
  • After deploying the Phase-1 management image: 32 connected, no disconnect, all old clients continued to function unchanged (they read only the legacy lazy_connection_enabled boolean which toPeerConfig keeps writing via the back-compat mapping).
  • Switched the account-wide connection_mode between p2p and p2p-lazy via the API: 32 connected throughout, no disconnect.
  • 8-minute monitoring window after deploy: zero peer-count drift.

The daemon-side change is exercised end-to-end by switching the account-wide setting and observing the daemon pick up the new mode on the next NetworkMap update (handled by UpdatedRemotePeerConfig).

Known limitations (called out in spec section 8)

  1. relay-forced cannot be pushed to old clients because the legacy lazy_connection_enabled boolean cannot express it. The wire field falls back to false and old clients run in normal p2p. Workaround: upgrade the client, or set local NB_FORCE_RELAY=true.
  2. p2p-dynamic is not visible in the Dashboard dropdown in Phase 1 even though the API accepts it; setting it via API gives p2p behaviour until the Phase-2 daemon implementation lands.
  3. Audit events are account-scoped only; per-peer / per-group events follow in Phase 3 (#5990).

Phase 2 / 3 follow-ups

  • Phase 2 (separate PR): p2p-dynamic daemon implementation -- decouple worker_relay / worker_ice OnNewOffer registration, two-tier inactivity manager, plus the DeactivatePeer no-op fix in conn_mgr.go that #5989 calls out for the lazy/eager mismatch.
  • Phase 3 (separate PR, addresses #5990): per-peer / per-group resolution for connection_mode, p2p_timeout, relay_timeout with most-restrictive group-conflict resolution and per-scope audit events.

Test plan

  • All new + existing unit tests in client/internal/... and management/... green
  • TestAccount_GetPeerNetworkMap (existing) remains green -- old peers without connection_mode resolve via ResolveLegacyLazyBool exactly as before
  • TestToPeerConfig_ConnectionModeResolution (new, 9 sub-cases) covers the resolution matrix
  • TestResolveConnectionMode (new, 10 sub-cases) covers the env > config > server precedence chain
  • TestResolveModeFromEnv (new, 9 sub-cases) covers the env-var matrix including the legacy-conflict case
  • Hardware-verified against 32 mixed-version peers in production (no disconnects)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Configurable peer connection modes (relay-forced, p2p, p2p-lazy, p2p-dynamic) via CLI flags and account settings.
    • Relay and P2P timeout overrides to tune idle behavior.
    • Activity/audit events emitted when connection-mode or timeout settings change.
  • Tests

    • Added unit tests covering mode/timeout resolution, env-var parsing, serialization round-trips, and API validation.

🔄 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/6047 **Author:** [@MichaelUray](https://github.com/MichaelUray) **Created:** 5/1/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `feat/connection-mode-phase1` --- ### 📝 Commits (10+) - [`d5d416e`](https://github.com/netbirdio/netbird/commit/d5d416e3f0cf4da0c134f8cec4bd0c85c31b53e2) proto: add ConnectionMode enum and p2p/relay timeout fields to PeerConfig - [`b341bc0`](https://github.com/netbirdio/netbird/commit/b341bc0876ba20489e6380eacdbbeff95c8433d6) client: add connectionmode package with Mode type and proto bridge - [`9cad8d6`](https://github.com/netbirdio/netbird/commit/9cad8d63c53bdfedf476c03b915dd581e0e26f30) client/peer: ResolveModeFromEnv with NB_CONNECTION_MODE and deprecation warns - [`a4b0d04`](https://github.com/netbirdio/netbird/commit/a4b0d043d136972b537af807dfca44f00cd898a7) client: add --connection-mode, --relay-timeout, --p2p-timeout CLI flags - [`62f29c5`](https://github.com/netbirdio/netbird/commit/62f29c56e31664eff793c0384e798d56e2e1f455) client/conn_mgr: replace asymmetric Lazy/ForceRelay precedence with Mode - [`0d48124`](https://github.com/netbirdio/netbird/commit/0d4812436cb83d146e83f6805b5959569a843098) client/peer: connection mode drives skip-ICE branch in Open() - [`6d40418`](https://github.com/netbirdio/netbird/commit/6d4041826a7e5d3f774da6d25b441ae09f06b97d) client/engine: forward resolved Mode to per-peer ConnConfig - [`be96444`](https://github.com/netbirdio/netbird/commit/be964443348822d04abecc69c383d0a7b766cc37) mgmt/types: add ConnectionMode + p2p/relay timeout to Settings - [`92885d4`](https://github.com/netbirdio/netbird/commit/92885d486fac429b41b102ae33a6a0d16b3c81a2) openapi: add connection_mode + p2p/relay timeout fields to AccountSettings - [`b00fee3`](https://github.com/netbirdio/netbird/commit/b00fee3dd05a0afaa0145376c2f8ac319e661979) mgmt/handlers/accounts: accept connection_mode + timeout settings on PUT ### 📊 Changes **30 files changed** (+2027 additions, -521 deletions) <details> <summary>View changed files</summary> 📝 `client/cmd/root.go` (+13 -0) 📝 `client/cmd/up.go` (+30 -0) 📝 `client/internal/conn_mgr.go` (+127 -35) ➕ `client/internal/conn_mgr_test.go` (+101 -0) 📝 `client/internal/connect.go` (+28 -0) 📝 `client/internal/engine.go` (+19 -2) 📝 `client/internal/lazyconn/env.go` (+5 -0) 📝 `client/internal/peer/conn.go` (+14 -4) 📝 `client/internal/peer/env.go` (+84 -0) ➕ `client/internal/peer/env_test.go` (+61 -0) 📝 `client/internal/profilemanager/config.go` (+33 -0) 📝 `client/internal/profilemanager/config_test.go` (+49 -0) 📝 `client/proto/daemon.pb.go` (+79 -11) 📝 `client/proto/daemon.proto` (+16 -0) 📝 `client/proto/daemon_grpc.pb.go` (+180 -222) 📝 `management/internals/shared/grpc/conversion.go` (+30 -1) 📝 `management/internals/shared/grpc/conversion_test.go` (+118 -0) 📝 `management/server/account.go` (+65 -1) 📝 `management/server/account_test.go` (+96 -0) 📝 `management/server/activity/codes.go` (+14 -0) _...and 10 more files_ </details> ### 📄 Description ## Summary This PR is **Phase 1 of three** that together implement issues #5989 and #5990 (the connection-mode consolidation RFC and its server-side per-peer/per-group companion). **In Scope:** - New `ConnectionMode` proto enum with four reserved values (`relay-forced`, `p2p`, `p2p-lazy`, `p2p-dynamic`); three are functional in this PR. `p2p-dynamic` is reserved on the wire and in the DB but the daemon currently treats it like `p2p` (pass-through). The decoupled `worker_relay`/`worker_ice` lifecycle that makes `p2p-dynamic` distinct comes in Phase 2. - Two new account-level settings on the management server: `relay_timeout_seconds`, `p2p_timeout_seconds` (both nullable, NULL = built-in default). - Replaces the asymmetric Lazy/ForceRelay precedence in `client/internal/conn_mgr.go` with a single resolver: client env > client config > server-pushed. - Full backwards-compatibility: - Old clients still see only the legacy `lazy_connection_enabled` boolean (mapped from the resolved mode by `toPeerConfig`). - Old servers send `connection_mode = UNSPECIFIED` (proto default 0) and the new client falls back to the legacy boolean -- verified via tests. - `NB_FORCE_RELAY`, `NB_ENABLE_EXPERIMENTAL_LAZY_CONN`, `--enable-lazy-connection`, and `NB_LAZY_CONN_INACTIVITY_THRESHOLD` continue to work and emit one-shot deprecation warnings. The companion Dashboard PR is at netbirdio/dashboard#627 (will be added once it exists). ## Implementation map | Layer | Files | |---|---| | proto | `shared/management/proto/management.proto`, `client/proto/daemon.proto` | | Mode type | `shared/connectionmode/` (Mode, ParseString, FromProto, ToProto, ResolveLegacyLazyBool, ToLazyConnectionEnabled) | | client env / CLI | `client/internal/peer/env.go` (ResolveModeFromEnv), `client/cmd/{root,up}.go` (`--connection-mode`, `--relay-timeout`, `--p2p-timeout`) | | client engine / conn-mgr | `client/internal/{engine,conn_mgr,connect}.go` (resolveConnectionMode + UpdatedRemotePeerConfig + EngineConfig fields) | | client per-peer | `client/internal/peer/conn.go` (Mode-driven skip-ICE branch) | | mgmt server | `management/server/types/settings.go` (new nullable columns), `management/server/http/handlers/accounts/accounts_handler.go` (PUT validation + response), `management/server/account.go` (audit emission), `management/server/activity/codes.go` (3 new event codes), `management/internals/shared/grpc/conversion.go` (`toPeerConfig` writes both old and new wire fields) | | OpenAPI | `shared/management/http/api/openapi.yml` + regenerated `types.gen.go` | | tests | `shared/connectionmode/mode_test.go`, `client/internal/peer/env_test.go`, `client/internal/conn_mgr_test.go`, `management/internals/shared/grpc/conversion_test.go` | ## Hardware-tested Verified on a real production NetBird instance with **32 connected peers across 12 OpenWrt-router versions (22.03 to 25.12), Windows 10/11, Debian, Android 12/14, and iOS 26.3.1**: - Pre-deploy baseline: 32 connected. - After deploying the Phase-1 management image: 32 connected, no disconnect, **all old clients continued to function unchanged** (they read only the legacy `lazy_connection_enabled` boolean which `toPeerConfig` keeps writing via the back-compat mapping). - Switched the account-wide `connection_mode` between `p2p` and `p2p-lazy` via the API: 32 connected throughout, no disconnect. - 8-minute monitoring window after deploy: zero peer-count drift. The daemon-side change is exercised end-to-end by switching the account-wide setting and observing the daemon pick up the new mode on the next NetworkMap update (handled by `UpdatedRemotePeerConfig`). ## Known limitations (called out in spec section 8) 1. **`relay-forced` cannot be pushed to old clients** because the legacy `lazy_connection_enabled` boolean cannot express it. The wire field falls back to `false` and old clients run in normal `p2p`. Workaround: upgrade the client, or set local `NB_FORCE_RELAY=true`. 2. **`p2p-dynamic` is not visible in the Dashboard dropdown in Phase 1** even though the API accepts it; setting it via API gives `p2p` behaviour until the Phase-2 daemon implementation lands. 3. **Audit events are account-scoped only**; per-peer / per-group events follow in Phase 3 (#5990). ## Phase 2 / 3 follow-ups - **Phase 2** (separate PR): `p2p-dynamic` daemon implementation -- decouple `worker_relay` / `worker_ice` `OnNewOffer` registration, two-tier inactivity manager, plus the `DeactivatePeer` no-op fix in `conn_mgr.go` that #5989 calls out for the lazy/eager mismatch. - **Phase 3** (separate PR, addresses #5990): per-peer / per-group resolution for `connection_mode`, `p2p_timeout`, `relay_timeout` with most-restrictive group-conflict resolution and per-scope audit events. ## Test plan - [x] All new + existing unit tests in `client/internal/...` and `management/...` green - [x] `TestAccount_GetPeerNetworkMap` (existing) remains green -- old peers without `connection_mode` resolve via `ResolveLegacyLazyBool` exactly as before - [x] `TestToPeerConfig_ConnectionModeResolution` (new, 9 sub-cases) covers the resolution matrix - [x] `TestResolveConnectionMode` (new, 10 sub-cases) covers the env > config > server precedence chain - [x] `TestResolveModeFromEnv` (new, 9 sub-cases) covers the env-var matrix including the legacy-conflict case - [x] Hardware-verified against 32 mixed-version peers in production (no disconnects) 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Configurable peer connection modes (relay-forced, p2p, p2p-lazy, p2p-dynamic) via CLI flags and account settings. * Relay and P2P timeout overrides to tune idle behavior. * Activity/audit events emitted when connection-mode or timeout settings change. * **Tests** * Added unit tests covering mode/timeout resolution, env-var parsing, serialization round-trips, and API validation. <!-- 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:07:12 -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#28955