[PR #5263] [CLOSED] [client] reuse logic in both up and login - login now respects env + persist config #25408

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

📋 Pull Request Information

Original PR: https://github.com/netbirdio/netbird/pull/5263
Author: @eyJhb
Created: 2/5/2026
Status: Closed

Base: mainHead: client-login-management


📝 Commits (10+)

  • 48ac60b move some logic from up to login
  • 0e1b4d3 moved up flags into up file instead of root
  • 1c3797f moved input config variables back to up handler
  • 541a8d5 moved logic from up into login for reusing it
  • ea66881 fixed comments from ai
  • 276ab7a minor fix on not propagating error
  • bcc9f6e minor nitpicks
  • 0a6521b more nitpicking
  • adb0544 fixes typo
  • 287ce0a fixes the already connected case - just do like is currently done, bug for bug

📊 Changes

3 files changed (+329 additions, -513 deletions)

View changed files

📝 client/cmd/login.go (+237 -51)
📝 client/cmd/root.go (+6 -38)
📝 client/cmd/up.go (+86 -424)

📄 Description

Describe your changes

I've moved a bunch of logic from up, into login, and then reused it for both commands.

  • Moved logic from up.go into login.go (reason for using login.go, was that up.go seemed to rely heavily on it anyways)
  • Moved flags from root.go into up.go, unsure why they were placed there
  • login will now respect env
  • login will now save the configuration

This is completely untested thus far, but should be a pretty solid base.

Discussed it on Slack https://netbirdio.slack.com/archives/C02KHAE8VLZ/p1770216154715019

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change

Not needed, as it just matches what should currently be happening, ie. bugfix.

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

Summary by CodeRabbit

  • Refactor

    • Centralized startup and "up" flows around a single config-input path and simplified CLI surface to a focused set of options (preshared-key, interface, wireguard-port, network-monitor, dns-router-interval, mtu).
    • Consolidated daemon integration so CLI inputs translate consistently into service config/login requests.
  • Bug Fixes / Reliability

    • Improved foreground and daemon login orchestration, safer profile switching, better error handling, and clearer success feedback ("Logged in successfully").
  • Enhancements

    • Broader config coverage (DNS/NAT, MTU, WireGuard port, SSH TTL, feature flags) exposed to CLI flows.

🔄 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/5263 **Author:** [@eyJhb](https://github.com/eyJhb) **Created:** 2/5/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `client-login-management` --- ### 📝 Commits (10+) - [`48ac60b`](https://github.com/netbirdio/netbird/commit/48ac60b26775c29708c96b5df4213587434c97cd) move some logic from up to login - [`0e1b4d3`](https://github.com/netbirdio/netbird/commit/0e1b4d36f0cdac7cb3e4712cf81f795642a4aaf6) moved up flags into up file instead of root - [`1c3797f`](https://github.com/netbirdio/netbird/commit/1c3797feb629b896c6911d9a73fccf582b96c183) moved input config variables back to up handler - [`541a8d5`](https://github.com/netbirdio/netbird/commit/541a8d5b8c06e7218006af4a34398a3a37c29fb4) moved logic from up into login for reusing it - [`ea66881`](https://github.com/netbirdio/netbird/commit/ea66881fe00fa7e5bae9e04cb4fb3da734e590b8) fixed comments from ai - [`276ab7a`](https://github.com/netbirdio/netbird/commit/276ab7adcf8637c2338765a7f29596b78d81b636) minor fix on not propagating error - [`bcc9f6e`](https://github.com/netbirdio/netbird/commit/bcc9f6e68a3f391cf7c95546a49cc3d3196c4384) minor nitpicks - [`0a6521b`](https://github.com/netbirdio/netbird/commit/0a6521bf72e035d47f3b187c353d112e40309d69) more nitpicking - [`adb0544`](https://github.com/netbirdio/netbird/commit/adb0544db566e5aa449840d36f17ac81eb735564) fixes typo - [`287ce0a`](https://github.com/netbirdio/netbird/commit/287ce0a57038cc1e654fe595b987e115f6d6be3f) fixes the already connected case - just do like is currently done, bug for bug ### 📊 Changes **3 files changed** (+329 additions, -513 deletions) <details> <summary>View changed files</summary> 📝 `client/cmd/login.go` (+237 -51) 📝 `client/cmd/root.go` (+6 -38) 📝 `client/cmd/up.go` (+86 -424) </details> ### 📄 Description ## Describe your changes I've moved a bunch of logic from up, into login, and then reused it for both commands. - Moved logic from `up.go` into `login.go` (reason for using `login.go`, was that `up.go` seemed to rely heavily on it anyways) - Moved flags from `root.go` into `up.go`, unsure why they were placed there - `login` will now respect env - `login` will now save the configuration This is completely untested thus far, but should be a pretty solid base. ## Issue ticket number and link Discussed it on Slack https://netbirdio.slack.com/archives/C02KHAE8VLZ/p1770216154715019 ## Stack <!-- branch-stack --> ### Checklist - [x] Is it a bug fix - [ ] Is a typo/documentation fix - [ ] Is a feature enhancement - [ ] It is a refactor - [ ] Created tests that fail without the change (if possible) > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is **not needed** for this change Not needed, as it just matches what should currently be happening, ie. bugfix. ### Docs PR URL (required if "docs added" is checked) Paste the PR link from https://github.com/netbirdio/docs here: https://github.com/netbirdio/docs/pull/__ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Centralized startup and "up" flows around a single config-input path and simplified CLI surface to a focused set of options (preshared-key, interface, wireguard-port, network-monitor, dns-router-interval, mtu). * Consolidated daemon integration so CLI inputs translate consistently into service config/login requests. * **Bug Fixes / Reliability** * Improved foreground and daemon login orchestration, safer profile switching, better error handling, and clearer success feedback ("Logged in successfully"). * **Enhancements** * Broader config coverage (DNS/NAT, MTU, WireGuard port, SSH TTL, feature flags) exposed to CLI flows. <!-- 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 07:05:58 -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#25408