- Notifications
You must be signed in to change notification settings - Fork924
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This stack of pull requests is managed by Graphite.Learn more about stacking. Join@spikecurtis and the rest of your teammates on |
5174c41
to25ec905
Compare"cdr.dev/slog" | ||
) | ||
type Tunnel struct { |
There was a problem hiding this comment.
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 thespeaker
should look, and as an illustrative example for reviewers of how thespeaker
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 thetailnet
package.
25ec905
to39ca9e8
CompareThere was a problem hiding this 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.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
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())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Could probably omit thectx.Err()
here
There was a problem hiding this comment.
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 whatDebug
messages are supposed to be for.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
39ca9e8
toc56c16b
CompareUh oh!
There was an error while loading.Please reload this page.
c56c16b
to48e91e4
Comparef7ddbb7
intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
closes#14731
Defines and implements the CoderVPN control protocol, which will be used to communicate with desktop client applications.