Skip to content

feat: add CoderVPN protocol definition & implementation #14855

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

Merged
merged 1 commit into from
Oct 1, 2024

Conversation

spikecurtis
Copy link
Contributor

@spikecurtis spikecurtis commented Sep 27, 2024

closes #14731

Defines and implements the CoderVPN control protocol, which will be used to communicate with desktop client applications.

Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @spikecurtis and the rest of your teammates on Graphite Graphite

@spikecurtis spikecurtis requested a review from sreya September 27, 2024 11:34
@spikecurtis spikecurtis marked this pull request as ready for review September 27, 2024 11:35
@spikecurtis spikecurtis force-pushed the spike/codervpn-protobuf branch from 5174c41 to 25ec905 Compare September 27, 2024 11:36
"cdr.dev/slog"
)

type Tunnel struct {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note to reviewers:

This component is not finished and not unit tested. I built the basic skeleton of it to inform how the API for the speaker should look, and as an illustrative example for reviewers of how the speaker will be used in practice.

It will be completed and tested after we have the other pieces in place to actually start the CoderVPN tunnel using the tailnet package.

@spikecurtis spikecurtis force-pushed the spike/codervpn-protobuf branch from 25ec905 to 39ca9e8 Compare September 27, 2024 12:15
Copy link
Collaborator

@sreya sreya left a comment

Choose a reason for hiding this comment

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

Looks good overall, I like the extensive comments. Some potential suggestions related to naming.

case s.recvCh <- msg:
s.logger.Debug(s.ctx, "passed received message to speaker")
case <-s.ctx.Done():
s.logger.Debug(s.ctx, "recvLoop canceled", slog.Error(s.ctx.Err()))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could probably omit the ctx.Err() here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll admit it's almost always "context canceled" but sometimes it's "deadline exceeded" and that's interesting and occasionally useful in debugging, which is what Debug messages are supposed to be for.

@spikecurtis spikecurtis force-pushed the spike/codervpn-protobuf branch from 39ca9e8 to c56c16b Compare September 30, 2024 05:55
@spikecurtis spikecurtis changed the title feat: add CoderVPN protocol definition & implementaion feat: add CoderVPN protocol definition & implementation Sep 30, 2024
@spikecurtis spikecurtis requested a review from sreya October 1, 2024 04:28
@spikecurtis spikecurtis force-pushed the spike/codervpn-protobuf branch from c56c16b to 48e91e4 Compare October 1, 2024 10:15
@spikecurtis spikecurtis requested a review from sreya October 1, 2024 10:16
@spikecurtis spikecurtis merged commit f7ddbb7 into main Oct 1, 2024
30 checks passed
@spikecurtis spikecurtis deleted the spike/codervpn-protobuf branch October 1, 2024 15:40
@github-actions github-actions bot locked and limited conversation to collaborators Oct 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for CoderVPN Control Protocol
2 participants