Skip to content

feat: add flag to disable all direct connections instance-wide #7936

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
Jun 21, 2023
Merged

feat: add flag to disable all direct connections instance-wide #7936

merged 11 commits into from
Jun 21, 2023

Conversation

deansheather
Copy link
Member

@deansheather deansheather commented Jun 9, 2023 •
edited
Loading

Adds coder server flag --disable-direct which does the following:

  • Removes all STUN ports from the DERP map by setting them to -1 and fully removes any STUNOnly nodes
  • Tells agents (on startup) to disable P2P
  • Tells clients to disable P2P when connecting to workspaces
  • coderd will disable P2P when initiating connections to workspaces i.e. for workspace apps

TODO:

  • Also block for coderd -> workspace direct connections
  • Add tests to ensure P2P isn't possible with this flag enabled

Closes #7422

@deansheather deansheather marked this pull request as ready for review June 10, 2023 03:43
kylecarbs
kylecarbs reviewed Jun 13, 2023
View reviewed changes
codersdk/deployment.go Outdated
{
Name: "Disable Direct Connections",
Description: "Disable peer-to-peer (aka. direct) workspace connections. All workspace connections from the CLI will be proxied through Coder (or custom configured DERP servers) and will never be peer-to-peer when enabled. Workspaces may still reach out to STUN servers to get their address until they are restarted after this change has been made, but new connections will still be proxied regardless.",
Flag: "disable-direct",
Copy link
Member

Choose a reason for hiding this comment

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

I think disable-direct-connections would be a bit easier to understand in isolation.

Copy link
Member Author

Choose a reason for hiding this comment

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

disable-direct-connections is going to be used as a global flag in the CLI in another PR, so I'm changing this to block-direct-connections.

coadler
coadler approved these changes Jun 14, 2023
View reviewed changes
@deansheather deansheather enabled auto-merge (squash) June 21, 2023 20:03
@deansheather deansheather requested a review from coadler June 21, 2023 21:51
@deansheather deansheather merged commit a28d422 into main Jun 21, 2023
@deansheather deansheather deleted the dean/disable-direct branch June 21, 2023 22:02
@github-actions github-actions bot locked and limited conversation to collaborators Jun 21, 2023
@deansheather deansheather changed the title feat: add flag to disable all direct connections feat: add flag to disable all direct connections instance-wide Jun 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kylecarbs kylecarbs kylecarbs left review comments

@coadler coadler coadler approved these changes

Assignees

@deansheather deansheather

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

No way to reliably disable direct connections
3 participants
@deansheather @coadler @kylecarbs