Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add --disable-auto-connectflag to prevent auto connection after daemon service start (fixes #444, fixes #1382) #1161

Merged
merged 11 commits into from
Feb 20, 2024

Conversation

oskardotglobal
Copy link
Contributor

@oskardotglobal oskardotglobal commented Sep 23, 2023

Describe your changes

With these changes, the command up supports the flag --disable-auto-connect that allows users to disable auto connection on the client after a computer restart or when the daemon restarts.

To disable auto-connect:

netbird down
netbird up --disable-auto-connect

To enable auto-connect:

netbird down
netbird up --disable-auto-connect=false

Issue ticket number and link

#444
#1382

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)
  • Extended the README / documentation, if necessary

@oskardotglobal oskardotglobal marked this pull request as ready for review September 23, 2023 15:11
@oskardotglobal
Copy link
Contributor Author

oskardotglobal commented Sep 23, 2023

By the way, I've taken a look at the service package and it seems like there's currently no way to use that to enable / disable the service using it

@oskardotglobal
Copy link
Contributor Author

To be honest, I kind of forgot this PR existed; I did completely revamp how I did this without ever committing it.

With these latest changes, the client will just not connect when the daemon is started unless this is changed via the config. However, it still defaults to connecting by default so this PR is backwards compatible. This functionality can be toggled with the netbird service disable and netbird service enable commands.

@oskardotglobal oskardotglobal changed the title feat: start daemon on demand (fixes #444 partially) feat: start daemon on demand (fixes #444, fixes #1382) Feb 11, 2024
@mlsmaycon
Copy link
Collaborator

Hello @oskardotglobal thanks for the awesome contribution. I have some remarks as we see this feature being released for new and existing users:

Ideally, all clients should autostart from the beginning, the majority of users are fine with that and offer an easy way to always be connected.

With that in mind, we should update the configuration option introduced in the PR because the default value for bool in Go is false, so let's rename the configuration from AutoStart to DisableAutoConnect. Then, when loading from an existing configuration, DisableAutoStart will be false, keeping the current behavior of connecting on boot.

Other note, the netbird service disable might drive people into the wrong conclusion and assume that this disabled the system service all along. It is better for us to work with: netbird service install --disable-auto-connect. We can also add this flag to the netbird up command.

Let us know what do you think.

@oskardotglobal
Copy link
Contributor Author

Sure, I'll make the proposed naming changes.
Maybe the flag should be global?

@oskardotglobal
Copy link
Contributor Author

I've made the changes and moved the flag to the up command as I thought it'd be better since it's that way for the rosenpass flag

client/cmd/up.go Outdated Show resolved Hide resolved
@mlsmaycon
Copy link
Collaborator

Thanks, @oskardotglobal; I've added one more note about where to add the set the flag.

@mlsmaycon
Copy link
Collaborator

Thanks @oskardotglobal we will review it today and if tests goes ok it will be part of the 0.26 release

Copy link
Collaborator

@mlsmaycon mlsmaycon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validated the flag and functionality in the configuration file changes and it is working.
We need to update it so that the change goes as part of the CLI request to the daemon process. If you are not sure about this change, let us know, we can help you out.

client/cmd/up.go Outdated Show resolved Hide resolved
@mlsmaycon mlsmaycon changed the title feat: start daemon on demand (fixes #444, fixes #1382) feat: add --disable-auto-connectflag to prevent auto connection after daemon service start (fixes #444, fixes #1382) Feb 20, 2024
@mlsmaycon mlsmaycon merged commit 8fd4166 into netbirdio:main Feb 20, 2024
16 checks passed
@mlsmaycon
Copy link
Collaborator

Thanks, @oskardotglobal, for the awesome contribution!

@oskardotglobal
Copy link
Contributor Author

To be honest, this has been bothering me ever since I started using netbird ;), but I'm still happy to help

Foosec pushed a commit to Foosec/netbird that referenced this pull request May 8, 2024
… daemon service start (fixes netbirdio#444, fixes netbirdio#1382) (netbirdio#1161)

With these changes, the command up supports the flag --disable-auto-connect that allows users to disable auto connection on the client after a computer restart or when the daemon restarts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants