Skip to content

Commit a3c6cb1

Browse files
coadlercoadler
authored
fix: don't query workspace in UpdateWorkspaceAgentConnectionByID (coder#7042)
1 parent 592b849 commit a3c6cb1

File tree

6 files changed

+18
-34
lines changed

6 files changed

+18
-34
lines changed

‎coderd/database/dbauthz/querier.go

-8
Original file line numberDiff line numberDiff line change
@@ -1552,14 +1552,6 @@ func (q *querier) UpdateWorkspace(ctx context.Context, arg database.UpdateWorksp
15521552
return updateWithReturn(q.log, q.auth, fetch, q.db.UpdateWorkspace)(ctx, arg)
15531553
}
15541554

1555-
func (q *querier) UpdateWorkspaceAgentConnectionByID(ctx context.Context, arg database.UpdateWorkspaceAgentConnectionByIDParams) error {
1556-
// TODO: This is a workspace agent operation. Should users be able to query this?
1557-
fetch := func(ctx context.Context, arg database.UpdateWorkspaceAgentConnectionByIDParams) (database.Workspace, error) {
1558-
return q.db.GetWorkspaceByAgentID(ctx, arg.ID)
1559-
}
1560-
return update(q.log, q.auth, fetch, q.db.UpdateWorkspaceAgentConnectionByID)(ctx, arg)
1561-
}
1562-
15631555
func (q *querier) InsertWorkspaceAgentStat(ctx context.Context, arg database.InsertWorkspaceAgentStatParams) (database.WorkspaceAgentStat, error) {
15641556
// TODO: This is a workspace agent operation. Should users be able to query this?
15651557
// Not really sure what this is for.

‎coderd/database/dbauthz/querier_test.go

-9
Original file line numberDiff line numberDiff line change
@@ -1170,15 +1170,6 @@ func (s *MethodTestSuite) TestWorkspace() {
11701170
ID: w.ID,
11711171
}).Asserts(w, rbac.ActionUpdate).Returns(expected)
11721172
}))
1173-
s.Run("UpdateWorkspaceAgentConnectionByID", s.Subtest(func(db database.Store, check *expects) {
1174-
ws := dbgen.Workspace(s.T(), db, database.Workspace{})
1175-
build := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID, JobID: uuid.New()})
1176-
res := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: build.JobID})
1177-
agt := dbgen.WorkspaceAgent(s.T(), db, database.WorkspaceAgent{ResourceID: res.ID})
1178-
check.Args(database.UpdateWorkspaceAgentConnectionByIDParams{
1179-
ID: agt.ID,
1180-
}).Asserts(ws, rbac.ActionUpdate).Returns()
1181-
}))
11821173
s.Run("InsertWorkspaceAgentStat", s.Subtest(func(db database.Store, check *expects) {
11831174
ws := dbgen.Workspace(s.T(), db, database.Workspace{})
11841175
check.Args(database.InsertWorkspaceAgentStatParams{

‎coderd/database/dbauthz/system.go

+7
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,13 @@ func (q *querier) InsertWorkspaceResourceMetadata(ctx context.Context, arg datab
348348
return q.db.InsertWorkspaceResourceMetadata(ctx, arg)
349349
}
350350

351+
func (q *querier) UpdateWorkspaceAgentConnectionByID(ctx context.Context, arg database.UpdateWorkspaceAgentConnectionByIDParams) error {
352+
if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceSystem); err != nil {
353+
return err
354+
}
355+
return q.db.UpdateWorkspaceAgentConnectionByID(ctx, arg)
356+
}
357+
351358
// TODO: We need to create a ProvisionerJob resource type
352359
func (q *querier) AcquireProvisionerJob(ctx context.Context, arg database.AcquireProvisionerJobParams) (database.ProvisionerJob, error) {
353360
// if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceSystem); err != nil {

‎coderd/database/dbauthz/system_test.go

+9
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,15 @@ func (s *MethodTestSuite) TestSystemFunctions() {
230230
WorkspaceResourceID: uuid.New(),
231231
}).Asserts(rbac.ResourceSystem, rbac.ActionCreate)
232232
}))
233+
s.Run("UpdateWorkspaceAgentConnectionByID", s.Subtest(func(db database.Store, check *expects) {
234+
ws := dbgen.Workspace(s.T(), db, database.Workspace{})
235+
build := dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID, JobID: uuid.New()})
236+
res := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: build.JobID})
237+
agt := dbgen.WorkspaceAgent(s.T(), db, database.WorkspaceAgent{ResourceID: res.ID})
238+
check.Args(database.UpdateWorkspaceAgentConnectionByIDParams{
239+
ID: agt.ID,
240+
}).Asserts(rbac.ResourceSystem, rbac.ActionUpdate).Returns()
241+
}))
233242
s.Run("AcquireProvisionerJob", s.Subtest(func(db database.Store, check *expects) {
234243
// TODO: we need to create a ProvisionerJob resource
235244
j := dbgen.ProvisionerJob(s.T(), db, database.ProvisionerJob{

‎coderd/workspaceagents.go

+2-12
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222
"github.com/bep/debounce"
2323
"github.com/go-chi/chi/v5"
2424
"github.com/google/uuid"
25-
"go.opentelemetry.io/otel/trace"
2625
"golang.org/x/exp/slices"
2726
"golang.org/x/mod/semver"
2827
"golang.org/x/xerrors"
@@ -36,7 +35,6 @@ import (
3635
"github.com/coder/coder/coderd/httpapi"
3736
"github.com/coder/coder/coderd/httpmw"
3837
"github.com/coder/coder/coderd/rbac"
39-
"github.com/coder/coder/coderd/tracing"
4038
"github.com/coder/coder/coderd/util/ptr"
4139
"github.com/coder/coder/codersdk"
4240
"github.com/coder/coder/codersdk/agentsdk"
@@ -859,7 +857,8 @@ func (api *API) workspaceAgentCoordinate(rw http.ResponseWriter, r *http.Request
859857
}
860858
disconnectedAt := workspaceAgent.DisconnectedAt
861859
updateConnectionTimes := func(ctx context.Context) error {
862-
err = api.Database.UpdateWorkspaceAgentConnectionByID(ctx, database.UpdateWorkspaceAgentConnectionByIDParams{
860+
//nolint:gocritic // We only update ourself.
861+
err = api.Database.UpdateWorkspaceAgentConnectionByID(dbauthz.AsSystemRestricted(ctx), database.UpdateWorkspaceAgentConnectionByIDParams{
863862
ID: workspaceAgent.ID,
864863
FirstConnectedAt: firstConnectedAt,
865864
LastConnectedAt: lastConnectedAt,
@@ -920,11 +919,6 @@ func (api *API) workspaceAgentCoordinate(rw http.ResponseWriter, r *http.Request
920919
}
921920
api.publishWorkspaceUpdate(ctx, build.WorkspaceID)
922921

923-
// End span so we don't get long lived trace data.
924-
tracing.EndHTTPSpan(r, http.StatusOK, trace.SpanFromContext(ctx))
925-
// Ignore all trace spans after this.
926-
ctx = trace.ContextWithSpan(ctx, tracing.NoopSpan)
927-
928922
api.Logger.Info(ctx, "accepting agent", slog.F("agent", workspaceAgent))
929923

930924
defer conn.Close(websocket.StatusNormalClosure, "")
@@ -1356,10 +1350,6 @@ func (api *API) watchWorkspaceAgentMetadata(rw http.ResponseWriter, r *http.Requ
13561350
<-senderClosed
13571351
}()
13581352

1359-
// We don't want this intentionally long request to skew our tracing
1360-
// reports.
1361-
ctx = trace.ContextWithSpan(ctx, tracing.NoopSpan)
1362-
13631353
const refreshInterval = time.Second * 5
13641354
refreshTicker := time.NewTicker(refreshInterval)
13651355
defer refreshTicker.Stop()

‎coderd/workspaces.go

-5
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313

1414
"github.com/go-chi/chi/v5"
1515
"github.com/google/uuid"
16-
"go.opentelemetry.io/otel/trace"
1716
"golang.org/x/xerrors"
1817

1918
"cdr.dev/slog"
@@ -26,7 +25,6 @@ import (
2625
"github.com/coder/coder/coderd/schedule"
2726
"github.com/coder/coder/coderd/searchquery"
2827
"github.com/coder/coder/coderd/telemetry"
29-
"github.com/coder/coder/coderd/tracing"
3028
"github.com/coder/coder/coderd/util/ptr"
3129
"github.com/coder/coder/codersdk"
3230
)
@@ -970,9 +968,6 @@ func (api *API) watchWorkspace(rw http.ResponseWriter, r *http.Request) {
970968
<-senderClosed
971969
}()
972970

973-
// Ignore all trace spans after this, they're not too useful.
974-
ctx = trace.ContextWithSpan(ctx, tracing.NoopSpan)
975-
976971
sendUpdate := func(_ context.Context, _ []byte) {
977972
workspace, err := api.Database.GetWorkspaceByID(ctx, workspace.ID)
978973
if err != nil {

0 commit comments

Comments
 (0)