feat: add WorkspaceUpdates tailnet RPC #14847
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @ethanndickson and the rest of your teammates on |
22985f7 to
db04bcf
Compare
ddc493e to
d2d165e
Compare
db04bcf to
50f874c
Compare
d57f3e6 to
8102c71
Compare
50f874c to
fe1e8b5
Compare
8102c71 to
4f57562
Compare
fe1e8b5 to
aaf8e86
Compare
ab7f678 to
fb84465
Compare
be7fa9e to
cb5adf4
Compare
fb84465 to
1a8392d
Compare
cb5adf4 to
2bc6e69
Compare
1a8392d to
51cd615
Compare
2bc6e69 to
54cbfaf
Compare
51cd615 to
de1435d
Compare
54cbfaf to
aad36cf
Compare
de1435d to
398fa2d
Compare
aad36cf to
b5f7529
Compare
398fa2d to
077608b
Compare
b5f7529 to
fabe2d1
Compare
077608b to
c762ae8
Compare
fabe2d1 to
d89d373
Compare
84730b8 to
f3b74b7
Compare
spikecurtis
reviewed
Oct 29, 2024
| auth rbac.Authorizer | ||
|
|
||
| ctx context.Context | ||
| cancelFn func() |
There was a problem hiding this comment.
ctx isn't used anywhere, so it and cancelFn are superfluous, which I think makes Close() on this superfluous.
There was a problem hiding this comment.
The intention was for sub to store a context who's parent is updatesProvider's context, so that calling Close on the updates provider closes all the existing subscriptions - will fix.
enterprise/tailnet/connio.go
Outdated
| @@ -133,7 +133,7 @@ var errDisconnect = xerrors.New("graceful disconnect") | |||
|
|
|||
| func (c *connIO) handleRequest(req *proto.CoordinateRequest) error { | |||
| c.logger.Debug(c.peerCtx, "got request") | |||
| err := c.auth.Authorize(req) | |||
| err := c.auth.Authorize(c.coordCtx, req) | |||
There was a problem hiding this comment.
This needs to be the c.peerCtx --- coordCtx is the context of the Coordinator's lifetime.
You should add a test that adds a subject to a context you use to call Coordinate() and verify the subject is there on the Authorize call. Same test for the AGPL implementation.
e05b286 to
a45a43c
Compare
f3b74b7 to
d1fac01
Compare
ethanndickson
commented
Oct 29, 2024
| _ = testutil.RequireRecvCtx(ctx, t, ch) | ||
| // If we don't cancel the context, the coordinator close will wait until the | ||
| // peer request loop finishes, which will be after the timeout | ||
| peerCtxCancel() |
There was a problem hiding this comment.
This was actually passing without, but it would take the entire 10 seconds. Interestingly, the PGCoord doesn't have this behaviour
There was a problem hiding this comment.
Yeah, PGCoord is more aggressive with closing things down, forcibly booting anything still connected because it has contexts plumbed thru for background tasks like heartbeats. The AGPL coordinator is considerably simpler and has no coordinator-scoped context.
964bfe9 to
494afa9
Compare
spikecurtis
approved these changes
Oct 29, 2024
| _ = testutil.RequireRecvCtx(ctx, t, ch) | ||
| // If we don't cancel the context, the coordinator close will wait until the | ||
| // peer request loop finishes, which will be after the timeout | ||
| peerCtxCancel() |
There was a problem hiding this comment.
Yeah, PGCoord is more aggressive with closing things down, forcibly booting anything still connected because it has contexts plumbed thru for background tasks like heartbeats. The AGPL coordinator is considerably simpler and has no coordinator-scoped context.
494afa9 to
cdb4235
Compare
a45a43c to
744633f
Compare
cdb4235 to
7d0a2b6
Compare
54ceea2 to
8be52ed
Compare
7d0a2b6 to
33d6628
Compare
8be52ed to
f941e78
Compare
33d6628 to
7be63f5
Compare
7be63f5 to
b509099
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #14716
Closes #14717
Adds a new user-scoped tailnet API endpoint (
api/v2/tailnet) with a new RPC stream for receiving updates on workspaces owned by a specific user, as defined in #14716.When a stream is started, the
WorkspaceUpdatesProviderwill begin listening on the user-scoped pubsub events implemented in #14964. When a relevant event type is seen (such as a workspace state transition), the provider will query the DB for all the workspaces (and agents) owned by the user. This gets compared against the result of the previous query to produce a set of workspace updates.Workspace updates can be requested for any user ID, however only workspaces the authorised user is permitted to
ActionReadwill have their updates streamed.Opening a tunnel to an agent requires that the user can perform
ActionSSHagainst the workspace containing it.