mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix(coderd): add role param to agent RPC to prevent false connectivity (#22052)
## Summary coder-logstream-kube and other tools that use the agent token to connect to the RPC endpoint were incorrectly triggering connection monitoring, causing false connected/disconnected timestamps on the agent. This led to VSCode/JetBrains disconnections and incorrect dashboard status. ## Changes Add a `role` query parameter to `/api/v2/workspaceagents/me/rpc`: - `role=agent`: triggers connection monitoring (default for the agent SDK) - any other value (e.g. `logstream-kube`): skips connection monitoring - omitted: triggers monitoring for backward compatibility with older agents The agent SDK now sends `role=agent` by default. A new `Role` field on the `agentsdk.Client` allows non-agent callers to specify a different role. ## Required follow-up coder-logstream-kube needs to set `client.Role = "logstream-kube"` before calling `ConnectRPC20()`. Without that change, it will still send `role=agent` and trigger monitoring. Fixes #21625
This commit is contained in:
@@ -59,6 +59,17 @@ func (api *API) workspaceAgentRPC(rw http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
// The role parameter distinguishes the real workspace agent from
|
||||
// other clients using the same agent token (e.g. coder-logstream-kube).
|
||||
// Only connections with the "agent" role trigger connection monitoring
|
||||
// that updates first_connected_at/last_connected_at/disconnected_at.
|
||||
// For backward compatibility, we default to monitoring when the role
|
||||
// is omitted, since older agents don't send this parameter. In a
|
||||
// future release, once all agents include role=agent, we can change
|
||||
// this default to skip monitoring for unspecified roles.
|
||||
role := r.URL.Query().Get("role")
|
||||
monitorConnection := role == "" || role == "agent"
|
||||
|
||||
api.WebsocketWaitMutex.Lock()
|
||||
api.WebsocketWaitGroup.Add(1)
|
||||
api.WebsocketWaitMutex.Unlock()
|
||||
@@ -121,10 +132,15 @@ func (api *API) workspaceAgentRPC(rw http.ResponseWriter, r *http.Request) {
|
||||
slog.F("agent_api_version", workspaceAgent.APIVersion),
|
||||
slog.F("agent_resource_id", workspaceAgent.ResourceID))
|
||||
|
||||
closeCtx, closeCtxCancel := context.WithCancel(ctx)
|
||||
defer closeCtxCancel()
|
||||
monitor := api.startAgentYamuxMonitor(closeCtx, workspace, workspaceAgent, build, mux)
|
||||
defer monitor.close()
|
||||
if monitorConnection {
|
||||
closeCtx, closeCtxCancel := context.WithCancel(ctx)
|
||||
defer closeCtxCancel()
|
||||
monitor := api.startAgentYamuxMonitor(closeCtx, workspace, workspaceAgent, build, mux)
|
||||
defer monitor.close()
|
||||
} else {
|
||||
logger.Debug(ctx, "skipping agent connection monitoring",
|
||||
slog.F("role", role))
|
||||
}
|
||||
|
||||
agentAPI := agentapi.New(agentapi.Options{
|
||||
AgentID: workspaceAgent.ID,
|
||||
|
||||
@@ -11,6 +11,7 @@ import (
|
||||
agentproto "github.com/coder/coder/v2/agent/proto"
|
||||
"github.com/coder/coder/v2/coderd/coderdtest"
|
||||
"github.com/coder/coder/v2/coderd/database"
|
||||
"github.com/coder/coder/v2/coderd/database/dbauthz"
|
||||
"github.com/coder/coder/v2/coderd/database/dbfake"
|
||||
"github.com/coder/coder/v2/coderd/database/dbtime"
|
||||
"github.com/coder/coder/v2/coderd/rbac"
|
||||
@@ -168,3 +169,85 @@ func TestAgentAPI_LargeManifest(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestWorkspaceAgentRPCRole(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
t.Run("AgentRoleMonitorsConnection", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
client, db := coderdtest.NewWithDatabase(t, nil)
|
||||
user := coderdtest.CreateFirstUser(t, client)
|
||||
r := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{
|
||||
OrganizationID: user.OrganizationID,
|
||||
OwnerID: user.UserID,
|
||||
}).WithAgent().Do()
|
||||
|
||||
// Connect with role=agent using ConnectRPCWithRole. This is
|
||||
// how the real workspace agent connects.
|
||||
ac := agentsdk.New(client.URL, agentsdk.WithFixedToken(r.AgentToken))
|
||||
conn, err := ac.ConnectRPCWithRole(ctx, "agent")
|
||||
require.NoError(t, err)
|
||||
defer func() {
|
||||
_ = conn.Close()
|
||||
}()
|
||||
|
||||
// The connection monitor updates the database asynchronously,
|
||||
// so we need to wait for first_connected_at to be set.
|
||||
var agent database.WorkspaceAgent
|
||||
require.Eventually(t, func() bool {
|
||||
agent, err = db.GetWorkspaceAgentByID(dbauthz.AsSystemRestricted(ctx), r.Agents[0].ID)
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
return agent.FirstConnectedAt.Valid
|
||||
}, testutil.WaitShort, testutil.IntervalFast)
|
||||
assert.True(t, agent.LastConnectedAt.Valid,
|
||||
"last_connected_at should be set for agent role")
|
||||
})
|
||||
|
||||
t.Run("NonAgentRoleSkipsMonitoring", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
client, db := coderdtest.NewWithDatabase(t, nil)
|
||||
user := coderdtest.CreateFirstUser(t, client)
|
||||
r := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{
|
||||
OrganizationID: user.OrganizationID,
|
||||
OwnerID: user.UserID,
|
||||
}).WithAgent().Do()
|
||||
|
||||
// Connect with a non-agent role using ConnectRPCWithRole.
|
||||
// This is how coder-logstream-kube should connect.
|
||||
ac := agentsdk.New(client.URL, agentsdk.WithFixedToken(r.AgentToken))
|
||||
conn, err := ac.ConnectRPCWithRole(ctx, "logstream-kube")
|
||||
require.NoError(t, err)
|
||||
|
||||
// Send a log to confirm the RPC connection is functional.
|
||||
agentAPI := agentproto.NewDRPCAgentClient(conn)
|
||||
_, err = agentAPI.BatchCreateLogs(ctx, &agentproto.BatchCreateLogsRequest{
|
||||
LogSourceId: []byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0},
|
||||
})
|
||||
// We don't care about the log source error, just that the
|
||||
// RPC is functional.
|
||||
_ = err
|
||||
|
||||
// Close the connection and give the server time to process.
|
||||
_ = conn.Close()
|
||||
time.Sleep(100 * time.Millisecond)
|
||||
|
||||
// Verify that connectivity timestamps were never set.
|
||||
agent, err := db.GetWorkspaceAgentByID(dbauthz.AsSystemRestricted(ctx), r.Agents[0].ID)
|
||||
require.NoError(t, err)
|
||||
assert.False(t, agent.FirstConnectedAt.Valid,
|
||||
"first_connected_at should NOT be set for non-agent role")
|
||||
assert.False(t, agent.LastConnectedAt.Valid,
|
||||
"last_connected_at should NOT be set for non-agent role")
|
||||
assert.False(t, agent.DisconnectedAt.Valid,
|
||||
"disconnected_at should NOT be set for non-agent role")
|
||||
})
|
||||
|
||||
// NOTE: Backward compatibility (empty role) is implicitly tested by
|
||||
// existing tests like TestWorkspaceAgentReportStats which use
|
||||
// ConnectRPC() (no role). The server defaults to monitoring when
|
||||
// the role query parameter is omitted.
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user