-
Notifications
You must be signed in to change notification settings - Fork 896
feat: add tailnet and agent API definitions #10324
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
Conversation
Current dependencies on/for this PR: This comment was auto-generated by Graphite. |
10fc9ca
to
ef6cead
Compare
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.
Customers use the HTTP endpoint of this to manually send logs.
How do we imagine the implementation logic that forks the gRPC and HTTP handler in coderd?
I was thinking an Agent API service, with an HTTP wrapper for the "legacy" methods. If logging isn't going to be deprecated we could leave the existing HTTP handler for that on the main API. I haven't looked at the code in detail, so it may or may not make sense for the dRPC to share code with the HTTP; either is fine. |
ef6cead
to
5934176
Compare
One thing I've just added, for those watching, are messages to the Coordinator RPC for adding and removing subscriptions (now called Tunnels). I didn't realize when I originally created the API that there is a second wire protocol that Workspace Proxies use to communicate with Coderd, and I'd like to cover that use case as well such that there is a single wire protocol for tailnet. Please look at the RFC for more details: https://www.notion.so/coderhq/Agent-Tailnet-APIs-386764332c134e0981946cefe657fd83?pvs=4#9f55e1837ecc4c9ea93cf5760f20c007 |
I think it'd be possible to share a lot of the code paths by accepting JSON requests and unmarshaling them into protobuf messages, e.g. https://pkg.go.dev/google.golang.org/protobuf/encoding/protojson This way we don't even need to duplicate the representation, generating the API documentation is a question-mark, though. |
I don't think protojson encoding/decoding is compatible with standard encoding/json. So, getting our existing message formats to decode into protobuf is a tall order, and would likely gum up the new API with weird legacy stuff |
ec609ce
to
c18c14d
Compare
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.
One last thing caught my eye, but all in all this looks good to me!
|
||
service Agent { | ||
rpc GetManifest(GetManifestRequest) returns (Manifest); | ||
rpc GetServiceBanner(GetServiceBannerRequest) returns (ServiceBanner); |
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 would be in favor of dropping the Get here (as typical in Go). Possibly also Batch in the updates. Batch doesn't feel like it adds any value if it's the only method. And pluralization can do the same if we want to convey it.
I'm not sure I'd consider the HTTP routes legacy... many of our customers (whether we like it or not) have cURL'd our agent HTTP endpoints to make some kinda magic happen, and we do it for things like This obviously doesn't prevent that though, so I'm good with it! |
c18c14d
to
dda5c85
Compare
Adds API definitions and packages for Tailnet and Agent APIs (API version 2.0)