From 0cfd0a9f721310f63f179fe08c9485b16323915a Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Mon, 23 Jun 2025 18:10:08 +0200 Subject: [PATCH 1/2] chore: InsertWorkspaceApp -> UpsertWorkspaceApp Signed-off-by: Danny Kopping --- coderd/agentapi/subagent.go | 7 +- coderd/database/dbauthz/dbauthz.go | 34 ++-- coderd/database/dbauthz/dbauthz_test.go | 4 +- coderd/database/dbgen/dbgen.go | 2 +- coderd/database/dbmem/dbmem.go | 94 ++++++----- coderd/database/dbmetrics/querymetrics.go | 14 +- coderd/database/dbmock/dbmock.go | 30 ++-- coderd/database/querier.go | 2 +- coderd/database/queries.sql.go | 155 ++++++++++-------- coderd/database/queries/workspaceapps.sql | 23 ++- .../provisionerdserver/provisionerdserver.go | 6 +- coderd/workspaces_test.go | 87 +++++++++- 12 files changed, 297 insertions(+), 161 deletions(-) diff --git a/coderd/agentapi/subagent.go b/coderd/agentapi/subagent.go index c00bfecc5ff17..6e7082c02415b 100644 --- a/coderd/agentapi/subagent.go +++ b/coderd/agentapi/subagent.go @@ -12,12 +12,13 @@ import ( "golang.org/x/xerrors" "cdr.dev/slog" + "github.com/coder/quartz" + agentproto "github.com/coder/coder/v2/agent/proto" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/provisioner" - "github.com/coder/quartz" ) type SubAgentAPI struct { @@ -164,8 +165,8 @@ func (a *SubAgentAPI) CreateSubAgent(ctx context.Context, req *agentproto.Create } } - _, err := a.Database.InsertWorkspaceApp(ctx, database.InsertWorkspaceAppParams{ - ID: uuid.New(), + _, err := a.Database.UpsertWorkspaceApp(ctx, database.UpsertWorkspaceAppParams{ + ID: uuid.New(), // TODO: we may need to maintain the app's ID here for stability, but for now we'll leave this as-is. CreatedAt: createdAt, AgentID: subAgent.ID, Slug: app.Slug, diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index adb2007918f8d..3c6f691616d52 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -3938,23 +3938,6 @@ func (q *querier) InsertWorkspaceAgentStats(ctx context.Context, arg database.In return q.db.InsertWorkspaceAgentStats(ctx, arg) } -func (q *querier) InsertWorkspaceApp(ctx context.Context, arg database.InsertWorkspaceAppParams) (database.WorkspaceApp, error) { - // NOTE(DanielleMaywood): - // It is possible for there to exist an agent without a workspace. - // This means that we want to allow execution to continue if - // there isn't a workspace found to allow this behavior to continue. - workspace, err := q.db.GetWorkspaceByAgentID(ctx, arg.AgentID) - if err != nil && !errors.Is(err, sql.ErrNoRows) { - return database.WorkspaceApp{}, err - } - - if err := q.authorizeContext(ctx, policy.ActionUpdate, workspace); err != nil { - return database.WorkspaceApp{}, err - } - - return q.db.InsertWorkspaceApp(ctx, arg) -} - func (q *querier) InsertWorkspaceAppStats(ctx context.Context, arg database.InsertWorkspaceAppStatsParams) error { if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceSystem); err != nil { return err @@ -5181,6 +5164,23 @@ func (q *querier) UpsertWorkspaceAgentPortShare(ctx context.Context, arg databas return q.db.UpsertWorkspaceAgentPortShare(ctx, arg) } +func (q *querier) UpsertWorkspaceApp(ctx context.Context, arg database.UpsertWorkspaceAppParams) (database.WorkspaceApp, error) { + // NOTE(DanielleMaywood): + // It is possible for there to exist an agent without a workspace. + // This means that we want to allow execution to continue if + // there isn't a workspace found to allow this behavior to continue. + workspace, err := q.db.GetWorkspaceByAgentID(ctx, arg.AgentID) + if err != nil && !errors.Is(err, sql.ErrNoRows) { + return database.WorkspaceApp{}, err + } + + if err := q.authorizeContext(ctx, policy.ActionUpdate, workspace); err != nil { + return database.WorkspaceApp{}, err + } + + return q.db.UpsertWorkspaceApp(ctx, arg) +} + func (q *querier) UpsertWorkspaceAppAuditSession(ctx context.Context, arg database.UpsertWorkspaceAppAuditSessionParams) (bool, error) { if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceSystem); err != nil { return false, err diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 0ccd867040116..f5077b7bdcda9 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -4114,7 +4114,7 @@ func (s *MethodTestSuite) TestSystemFunctions() { APIKeyScope: database.AgentKeyScopeEnumAll, }).Asserts(ws, policy.ActionCreateAgent) })) - s.Run("InsertWorkspaceApp", s.Subtest(func(db database.Store, check *expects) { + s.Run("UpsertWorkspaceApp", s.Subtest(func(db database.Store, check *expects) { _ = dbgen.User(s.T(), db, database.User{}) u := dbgen.User(s.T(), db, database.User{}) o := dbgen.Organization(s.T(), db, database.Organization{}) @@ -4130,7 +4130,7 @@ func (s *MethodTestSuite) TestSystemFunctions() { _ = dbgen.WorkspaceBuild(s.T(), db, database.WorkspaceBuild{WorkspaceID: ws.ID, JobID: j.ID, TemplateVersionID: tv.ID}) res := dbgen.WorkspaceResource(s.T(), db, database.WorkspaceResource{JobID: j.ID}) agent := dbgen.WorkspaceAgent(s.T(), db, database.WorkspaceAgent{ResourceID: res.ID}) - check.Args(database.InsertWorkspaceAppParams{ + check.Args(database.UpsertWorkspaceAppParams{ ID: uuid.New(), AgentID: agent.ID, Health: database.WorkspaceAppHealthDisabled, diff --git a/coderd/database/dbgen/dbgen.go b/coderd/database/dbgen/dbgen.go index 5b03fd0eb1396..5b94b2e789c5f 100644 --- a/coderd/database/dbgen/dbgen.go +++ b/coderd/database/dbgen/dbgen.go @@ -778,7 +778,7 @@ func ProvisionerKey(t testing.TB, db database.Store, orig database.ProvisionerKe } func WorkspaceApp(t testing.TB, db database.Store, orig database.WorkspaceApp) database.WorkspaceApp { - resource, err := db.InsertWorkspaceApp(genCtx, database.InsertWorkspaceAppParams{ + resource, err := db.UpsertWorkspaceApp(genCtx, database.UpsertWorkspaceAppParams{ ID: takeFirst(orig.ID, uuid.New()), CreatedAt: takeFirst(orig.CreatedAt, dbtime.Now()), AgentID: takeFirst(orig.AgentID, uuid.New()), diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index f4403d2f50c75..117148a908979 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -10018,48 +10018,6 @@ func (q *FakeQuerier) InsertWorkspaceAgentStats(_ context.Context, arg database. return nil } -func (q *FakeQuerier) InsertWorkspaceApp(_ context.Context, arg database.InsertWorkspaceAppParams) (database.WorkspaceApp, error) { - if err := validateDatabaseType(arg); err != nil { - return database.WorkspaceApp{}, err - } - - q.mutex.Lock() - defer q.mutex.Unlock() - - if arg.SharingLevel == "" { - arg.SharingLevel = database.AppSharingLevelOwner - } - - if arg.OpenIn == "" { - arg.OpenIn = database.WorkspaceAppOpenInSlimWindow - } - - // nolint:gosimple - workspaceApp := database.WorkspaceApp{ - ID: arg.ID, - AgentID: arg.AgentID, - CreatedAt: arg.CreatedAt, - Slug: arg.Slug, - DisplayName: arg.DisplayName, - Icon: arg.Icon, - Command: arg.Command, - Url: arg.Url, - External: arg.External, - Subdomain: arg.Subdomain, - SharingLevel: arg.SharingLevel, - HealthcheckUrl: arg.HealthcheckUrl, - HealthcheckInterval: arg.HealthcheckInterval, - HealthcheckThreshold: arg.HealthcheckThreshold, - Health: arg.Health, - Hidden: arg.Hidden, - DisplayOrder: arg.DisplayOrder, - OpenIn: arg.OpenIn, - DisplayGroup: arg.DisplayGroup, - } - q.workspaceApps = append(q.workspaceApps, workspaceApp) - return workspaceApp, nil -} - func (q *FakeQuerier) InsertWorkspaceAppStats(_ context.Context, arg database.InsertWorkspaceAppStatsParams) error { err := validateDatabaseType(arg) if err != nil { @@ -13192,6 +13150,58 @@ func (q *FakeQuerier) UpsertWorkspaceAgentPortShare(_ context.Context, arg datab return psl, nil } +func (q *FakeQuerier) UpsertWorkspaceApp(ctx context.Context, arg database.UpsertWorkspaceAppParams) (database.WorkspaceApp, error) { + err := validateDatabaseType(arg) + if err != nil { + return database.WorkspaceApp{}, err + } + + q.mutex.Lock() + defer q.mutex.Unlock() + + if arg.SharingLevel == "" { + arg.SharingLevel = database.AppSharingLevelOwner + } + if arg.OpenIn == "" { + arg.OpenIn = database.WorkspaceAppOpenInSlimWindow + } + + buildApp := func(id uuid.UUID, createdAt time.Time) database.WorkspaceApp { + return database.WorkspaceApp{ + ID: id, + CreatedAt: createdAt, + AgentID: arg.AgentID, + Slug: arg.Slug, + DisplayName: arg.DisplayName, + Icon: arg.Icon, + Command: arg.Command, + Url: arg.Url, + External: arg.External, + Subdomain: arg.Subdomain, + SharingLevel: arg.SharingLevel, + HealthcheckUrl: arg.HealthcheckUrl, + HealthcheckInterval: arg.HealthcheckInterval, + HealthcheckThreshold: arg.HealthcheckThreshold, + Health: arg.Health, + Hidden: arg.Hidden, + DisplayOrder: arg.DisplayOrder, + OpenIn: arg.OpenIn, + DisplayGroup: arg.DisplayGroup, + } + } + + for i, app := range q.workspaceApps { + if app.ID == arg.ID { + q.workspaceApps[i] = buildApp(app.ID, app.CreatedAt) + return q.workspaceApps[i], nil + } + } + + workspaceApp := buildApp(arg.ID, arg.CreatedAt) + q.workspaceApps = append(q.workspaceApps, workspaceApp) + return workspaceApp, nil +} + func (q *FakeQuerier) UpsertWorkspaceAppAuditSession(_ context.Context, arg database.UpsertWorkspaceAppAuditSessionParams) (bool, error) { err := validateDatabaseType(arg) if err != nil { diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index 0450776785d42..3a5bfac1ae57b 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -2440,13 +2440,6 @@ func (m queryMetricsStore) InsertWorkspaceAgentStats(ctx context.Context, arg da return r0 } -func (m queryMetricsStore) InsertWorkspaceApp(ctx context.Context, arg database.InsertWorkspaceAppParams) (database.WorkspaceApp, error) { - start := time.Now() - app, err := m.s.InsertWorkspaceApp(ctx, arg) - m.queryLatencies.WithLabelValues("InsertWorkspaceApp").Observe(time.Since(start).Seconds()) - return app, err -} - func (m queryMetricsStore) InsertWorkspaceAppStats(ctx context.Context, arg database.InsertWorkspaceAppStatsParams) error { start := time.Now() r0 := m.s.InsertWorkspaceAppStats(ctx, arg) @@ -3287,6 +3280,13 @@ func (m queryMetricsStore) UpsertWorkspaceAgentPortShare(ctx context.Context, ar return r0, r1 } +func (m queryMetricsStore) UpsertWorkspaceApp(ctx context.Context, arg database.UpsertWorkspaceAppParams) (database.WorkspaceApp, error) { + start := time.Now() + r0, r1 := m.s.UpsertWorkspaceApp(ctx, arg) + m.queryLatencies.WithLabelValues("UpsertWorkspaceApp").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m queryMetricsStore) UpsertWorkspaceAppAuditSession(ctx context.Context, arg database.UpsertWorkspaceAppAuditSessionParams) (bool, error) { start := time.Now() r0, r1 := m.s.UpsertWorkspaceAppAuditSession(ctx, arg) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index dbd8f5ca0753c..067ad062e1dd8 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -5150,21 +5150,6 @@ func (mr *MockStoreMockRecorder) InsertWorkspaceAgentStats(ctx, arg any) *gomock return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InsertWorkspaceAgentStats", reflect.TypeOf((*MockStore)(nil).InsertWorkspaceAgentStats), ctx, arg) } -// InsertWorkspaceApp mocks base method. -func (m *MockStore) InsertWorkspaceApp(ctx context.Context, arg database.InsertWorkspaceAppParams) (database.WorkspaceApp, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "InsertWorkspaceApp", ctx, arg) - ret0, _ := ret[0].(database.WorkspaceApp) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// InsertWorkspaceApp indicates an expected call of InsertWorkspaceApp. -func (mr *MockStoreMockRecorder) InsertWorkspaceApp(ctx, arg any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InsertWorkspaceApp", reflect.TypeOf((*MockStore)(nil).InsertWorkspaceApp), ctx, arg) -} - // InsertWorkspaceAppStats mocks base method. func (m *MockStore) InsertWorkspaceAppStats(ctx context.Context, arg database.InsertWorkspaceAppStatsParams) error { m.ctrl.T.Helper() @@ -6924,6 +6909,21 @@ func (mr *MockStoreMockRecorder) UpsertWorkspaceAgentPortShare(ctx, arg any) *go return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpsertWorkspaceAgentPortShare", reflect.TypeOf((*MockStore)(nil).UpsertWorkspaceAgentPortShare), ctx, arg) } +// UpsertWorkspaceApp mocks base method. +func (m *MockStore) UpsertWorkspaceApp(ctx context.Context, arg database.UpsertWorkspaceAppParams) (database.WorkspaceApp, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpsertWorkspaceApp", ctx, arg) + ret0, _ := ret[0].(database.WorkspaceApp) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// UpsertWorkspaceApp indicates an expected call of UpsertWorkspaceApp. +func (mr *MockStoreMockRecorder) UpsertWorkspaceApp(ctx, arg any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpsertWorkspaceApp", reflect.TypeOf((*MockStore)(nil).UpsertWorkspaceApp), ctx, arg) +} + // UpsertWorkspaceAppAuditSession mocks base method. func (m *MockStore) UpsertWorkspaceAppAuditSession(ctx context.Context, arg database.UpsertWorkspaceAppAuditSessionParams) (bool, error) { m.ctrl.T.Helper() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 4cfd0d1c4da5f..06f51785aa1af 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -530,7 +530,6 @@ type sqlcQuerier interface { InsertWorkspaceAgentScriptTimings(ctx context.Context, arg InsertWorkspaceAgentScriptTimingsParams) (WorkspaceAgentScriptTiming, error) InsertWorkspaceAgentScripts(ctx context.Context, arg InsertWorkspaceAgentScriptsParams) ([]WorkspaceAgentScript, error) InsertWorkspaceAgentStats(ctx context.Context, arg InsertWorkspaceAgentStatsParams) error - InsertWorkspaceApp(ctx context.Context, arg InsertWorkspaceAppParams) (WorkspaceApp, error) InsertWorkspaceAppStats(ctx context.Context, arg InsertWorkspaceAppStatsParams) error InsertWorkspaceAppStatus(ctx context.Context, arg InsertWorkspaceAppStatusParams) (WorkspaceAppStatus, error) InsertWorkspaceBuild(ctx context.Context, arg InsertWorkspaceBuildParams) error @@ -671,6 +670,7 @@ type sqlcQuerier interface { UpsertTemplateUsageStats(ctx context.Context) error UpsertWebpushVAPIDKeys(ctx context.Context, arg UpsertWebpushVAPIDKeysParams) error UpsertWorkspaceAgentPortShare(ctx context.Context, arg UpsertWorkspaceAgentPortShareParams) (WorkspaceAgentPortShare, error) + UpsertWorkspaceApp(ctx context.Context, arg UpsertWorkspaceAppParams) (WorkspaceApp, error) // // The returned boolean, new_or_stale, can be used to deduce if a new session // was started. This means that a new row was inserted (no previous session) or diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index fe32851f0e002..3291e85b7576d 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -16673,7 +16673,68 @@ func (q *sqlQuerier) GetWorkspaceAppsCreatedAfter(ctx context.Context, createdAt return items, nil } -const insertWorkspaceApp = `-- name: InsertWorkspaceApp :one +const insertWorkspaceAppStatus = `-- name: InsertWorkspaceAppStatus :one +INSERT INTO workspace_app_statuses (id, created_at, workspace_id, agent_id, app_id, state, message, uri) +VALUES ($1, $2, $3, $4, $5, $6, $7, $8) +RETURNING id, created_at, agent_id, app_id, workspace_id, state, message, uri +` + +type InsertWorkspaceAppStatusParams struct { + ID uuid.UUID `db:"id" json:"id"` + CreatedAt time.Time `db:"created_at" json:"created_at"` + WorkspaceID uuid.UUID `db:"workspace_id" json:"workspace_id"` + AgentID uuid.UUID `db:"agent_id" json:"agent_id"` + AppID uuid.UUID `db:"app_id" json:"app_id"` + State WorkspaceAppStatusState `db:"state" json:"state"` + Message string `db:"message" json:"message"` + Uri sql.NullString `db:"uri" json:"uri"` +} + +func (q *sqlQuerier) InsertWorkspaceAppStatus(ctx context.Context, arg InsertWorkspaceAppStatusParams) (WorkspaceAppStatus, error) { + row := q.db.QueryRowContext(ctx, insertWorkspaceAppStatus, + arg.ID, + arg.CreatedAt, + arg.WorkspaceID, + arg.AgentID, + arg.AppID, + arg.State, + arg.Message, + arg.Uri, + ) + var i WorkspaceAppStatus + err := row.Scan( + &i.ID, + &i.CreatedAt, + &i.AgentID, + &i.AppID, + &i.WorkspaceID, + &i.State, + &i.Message, + &i.Uri, + ) + return i, err +} + +const updateWorkspaceAppHealthByID = `-- name: UpdateWorkspaceAppHealthByID :exec +UPDATE + workspace_apps +SET + health = $2 +WHERE + id = $1 +` + +type UpdateWorkspaceAppHealthByIDParams struct { + ID uuid.UUID `db:"id" json:"id"` + Health WorkspaceAppHealth `db:"health" json:"health"` +} + +func (q *sqlQuerier) UpdateWorkspaceAppHealthByID(ctx context.Context, arg UpdateWorkspaceAppHealthByIDParams) error { + _, err := q.db.ExecContext(ctx, updateWorkspaceAppHealthByID, arg.ID, arg.Health) + return err +} + +const upsertWorkspaceApp = `-- name: UpsertWorkspaceApp :one INSERT INTO workspace_apps ( id, @@ -16697,10 +16758,29 @@ INSERT INTO display_group ) VALUES - ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19) RETURNING id, created_at, agent_id, display_name, icon, command, url, healthcheck_url, healthcheck_interval, healthcheck_threshold, health, subdomain, sharing_level, slug, external, display_order, hidden, open_in, display_group -` - -type InsertWorkspaceAppParams struct { + ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19) +ON CONFLICT (id) DO UPDATE SET + display_name = EXCLUDED.display_name, + icon = EXCLUDED.icon, + command = EXCLUDED.command, + url = EXCLUDED.url, + external = EXCLUDED.external, + subdomain = EXCLUDED.subdomain, + sharing_level = EXCLUDED.sharing_level, + healthcheck_url = EXCLUDED.healthcheck_url, + healthcheck_interval = EXCLUDED.healthcheck_interval, + healthcheck_threshold = EXCLUDED.healthcheck_threshold, + health = EXCLUDED.health, + display_order = EXCLUDED.display_order, + hidden = EXCLUDED.hidden, + open_in = EXCLUDED.open_in, + display_group = EXCLUDED.display_group, + agent_id = EXCLUDED.agent_id, + slug = EXCLUDED.slug +RETURNING id, created_at, agent_id, display_name, icon, command, url, healthcheck_url, healthcheck_interval, healthcheck_threshold, health, subdomain, sharing_level, slug, external, display_order, hidden, open_in, display_group +` + +type UpsertWorkspaceAppParams struct { ID uuid.UUID `db:"id" json:"id"` CreatedAt time.Time `db:"created_at" json:"created_at"` AgentID uuid.UUID `db:"agent_id" json:"agent_id"` @@ -16722,8 +16802,8 @@ type InsertWorkspaceAppParams struct { DisplayGroup sql.NullString `db:"display_group" json:"display_group"` } -func (q *sqlQuerier) InsertWorkspaceApp(ctx context.Context, arg InsertWorkspaceAppParams) (WorkspaceApp, error) { - row := q.db.QueryRowContext(ctx, insertWorkspaceApp, +func (q *sqlQuerier) UpsertWorkspaceApp(ctx context.Context, arg UpsertWorkspaceAppParams) (WorkspaceApp, error) { + row := q.db.QueryRowContext(ctx, upsertWorkspaceApp, arg.ID, arg.CreatedAt, arg.AgentID, @@ -16769,67 +16849,6 @@ func (q *sqlQuerier) InsertWorkspaceApp(ctx context.Context, arg InsertWorkspace return i, err } -const insertWorkspaceAppStatus = `-- name: InsertWorkspaceAppStatus :one -INSERT INTO workspace_app_statuses (id, created_at, workspace_id, agent_id, app_id, state, message, uri) -VALUES ($1, $2, $3, $4, $5, $6, $7, $8) -RETURNING id, created_at, agent_id, app_id, workspace_id, state, message, uri -` - -type InsertWorkspaceAppStatusParams struct { - ID uuid.UUID `db:"id" json:"id"` - CreatedAt time.Time `db:"created_at" json:"created_at"` - WorkspaceID uuid.UUID `db:"workspace_id" json:"workspace_id"` - AgentID uuid.UUID `db:"agent_id" json:"agent_id"` - AppID uuid.UUID `db:"app_id" json:"app_id"` - State WorkspaceAppStatusState `db:"state" json:"state"` - Message string `db:"message" json:"message"` - Uri sql.NullString `db:"uri" json:"uri"` -} - -func (q *sqlQuerier) InsertWorkspaceAppStatus(ctx context.Context, arg InsertWorkspaceAppStatusParams) (WorkspaceAppStatus, error) { - row := q.db.QueryRowContext(ctx, insertWorkspaceAppStatus, - arg.ID, - arg.CreatedAt, - arg.WorkspaceID, - arg.AgentID, - arg.AppID, - arg.State, - arg.Message, - arg.Uri, - ) - var i WorkspaceAppStatus - err := row.Scan( - &i.ID, - &i.CreatedAt, - &i.AgentID, - &i.AppID, - &i.WorkspaceID, - &i.State, - &i.Message, - &i.Uri, - ) - return i, err -} - -const updateWorkspaceAppHealthByID = `-- name: UpdateWorkspaceAppHealthByID :exec -UPDATE - workspace_apps -SET - health = $2 -WHERE - id = $1 -` - -type UpdateWorkspaceAppHealthByIDParams struct { - ID uuid.UUID `db:"id" json:"id"` - Health WorkspaceAppHealth `db:"health" json:"health"` -} - -func (q *sqlQuerier) UpdateWorkspaceAppHealthByID(ctx context.Context, arg UpdateWorkspaceAppHealthByIDParams) error { - _, err := q.db.ExecContext(ctx, updateWorkspaceAppHealthByID, arg.ID, arg.Health) - return err -} - const insertWorkspaceAppStats = `-- name: InsertWorkspaceAppStats :exec INSERT INTO workspace_app_stats ( diff --git a/coderd/database/queries/workspaceapps.sql b/coderd/database/queries/workspaceapps.sql index f3f8e4066970b..9a6fbf9251788 100644 --- a/coderd/database/queries/workspaceapps.sql +++ b/coderd/database/queries/workspaceapps.sql @@ -10,7 +10,7 @@ SELECT * FROM workspace_apps WHERE agent_id = $1 AND slug = $2; -- name: GetWorkspaceAppsCreatedAfter :many SELECT * FROM workspace_apps WHERE created_at > $1 ORDER BY slug ASC; --- name: InsertWorkspaceApp :one +-- name: UpsertWorkspaceApp :one INSERT INTO workspace_apps ( id, @@ -34,7 +34,26 @@ INSERT INTO display_group ) VALUES - ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19) RETURNING *; + ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19) +ON CONFLICT (id) DO UPDATE SET + display_name = EXCLUDED.display_name, + icon = EXCLUDED.icon, + command = EXCLUDED.command, + url = EXCLUDED.url, + external = EXCLUDED.external, + subdomain = EXCLUDED.subdomain, + sharing_level = EXCLUDED.sharing_level, + healthcheck_url = EXCLUDED.healthcheck_url, + healthcheck_interval = EXCLUDED.healthcheck_interval, + healthcheck_threshold = EXCLUDED.healthcheck_threshold, + health = EXCLUDED.health, + display_order = EXCLUDED.display_order, + hidden = EXCLUDED.hidden, + open_in = EXCLUDED.open_in, + display_group = EXCLUDED.display_group, + agent_id = EXCLUDED.agent_id, + slug = EXCLUDED.slug +RETURNING *; -- name: UpdateWorkspaceAppHealthByID :exec UPDATE diff --git a/coderd/provisionerdserver/provisionerdserver.go b/coderd/provisionerdserver/provisionerdserver.go index a8f7c63a586fe..580520e6e4b4b 100644 --- a/coderd/provisionerdserver/provisionerdserver.go +++ b/coderd/provisionerdserver/provisionerdserver.go @@ -28,6 +28,7 @@ import ( protobuf "google.golang.org/protobuf/proto" "cdr.dev/slog" + "github.com/coder/coder/v2/coderd/util/slice" "github.com/coder/coder/v2/codersdk/drpcsdk" @@ -2606,7 +2607,8 @@ func InsertWorkspaceResource(ctx context.Context, db database.Store, jobID uuid. return xerrors.Errorf("parse app uuid: %w", err) } - dbApp, err := db.InsertWorkspaceApp(ctx, database.InsertWorkspaceAppParams{ + // If workspace apps are "persistent", the ID will not be regenerated across workspace builds, so we have to upsert. + dbApp, err := db.UpsertWorkspaceApp(ctx, database.UpsertWorkspaceAppParams{ ID: id, CreatedAt: dbtime.Now(), AgentID: dbAgent.ID, @@ -2635,7 +2637,7 @@ func InsertWorkspaceResource(ctx context.Context, db database.Store, jobID uuid. OpenIn: openIn, }) if err != nil { - return xerrors.Errorf("insert app: %w", err) + return xerrors.Errorf("upsert app: %w", err) } snapshot.WorkspaceApps = append(snapshot.WorkspaceApps, telemetry.ConvertWorkspaceApp(dbApp)) } diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 4f48136abb3ee..74554962fbd1e 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -19,6 +19,8 @@ import ( "github.com/stretchr/testify/require" "cdr.dev/slog" + "github.com/coder/terraform-provider-coder/v2/provider" + "github.com/coder/coder/v2/agent/agenttest" "github.com/coder/coder/v2/coderd/audit" "github.com/coder/coder/v2/coderd/coderdtest" @@ -42,7 +44,6 @@ import ( "github.com/coder/coder/v2/provisioner/echo" "github.com/coder/coder/v2/provisionersdk/proto" "github.com/coder/coder/v2/testutil" - "github.com/coder/terraform-provider-coder/v2/provider" ) func TestWorkspace(t *testing.T) { @@ -4614,3 +4615,87 @@ func TestWorkspaceFilterHasAITask(t *testing.T) { require.NoError(t, err) require.Len(t, res.Workspaces, 5) } + +func TestWorkspaceAppUpsertRestart(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, &coderdtest.Options{ + IncludeProvisionerDaemon: true, + }) + user := coderdtest.CreateFirstUser(t, client) + + // Define an app to be created with the workspace + apps := []*proto.App{ + { + Id: uuid.NewString(), + Slug: "test-app", + DisplayName: "Test App", + Command: "test-command", + Url: "http://localhost:8080", + Icon: "/test.svg", + }, + } + + // Create template version with workspace app + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ + Parse: echo.ParseComplete, + ProvisionApply: []*proto.Response{{ + Type: &proto.Response_Apply{ + Apply: &proto.ApplyComplete{ + Resources: []*proto.Resource{{ + Name: "test-resource", + Type: "example", + Agents: []*proto.Agent{{ + Id: uuid.NewString(), + Name: "dev", + Auth: &proto.Agent_Token{}, + Apps: apps, + }}, + }}, + }, + }, + }}, + }) + coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) + + // Create template and workspace + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + workspace := coderdtest.CreateWorkspace(t, client, template.ID) + coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + // Verify initial workspace has the app + workspace, err := client.Workspace(ctx, workspace.ID) + require.NoError(t, err) + require.Len(t, workspace.LatestBuild.Resources[0].Agents, 1) + agent := workspace.LatestBuild.Resources[0].Agents[0] + require.Len(t, agent.Apps, 1) + require.Equal(t, "test-app", agent.Apps[0].Slug) + require.Equal(t, "Test App", agent.Apps[0].DisplayName) + + // Stop the workspace + stopBuild := coderdtest.CreateWorkspaceBuild(t, client, workspace, database.WorkspaceTransitionStop) + coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, stopBuild.ID) + + // Restart the workspace (this will trigger upsert for the app) + startBuild := coderdtest.CreateWorkspaceBuild(t, client, workspace, database.WorkspaceTransitionStart) + coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, startBuild.ID) + + // Verify the workspace restarted successfully + workspace, err = client.Workspace(ctx, workspace.ID) + require.NoError(t, err) + require.Equal(t, codersdk.WorkspaceStatusRunning, workspace.LatestBuild.Status) + + // Verify the app is still present after restart (upsert worked) + require.Len(t, workspace.LatestBuild.Resources[0].Agents, 1) + agent = workspace.LatestBuild.Resources[0].Agents[0] + require.Len(t, agent.Apps, 1) + require.Equal(t, "test-app", agent.Apps[0].Slug) + require.Equal(t, "Test App", agent.Apps[0].DisplayName) + + // Verify the provisioner job completed successfully (no error) + require.Equal(t, codersdk.ProvisionerJobSucceeded, workspace.LatestBuild.Job.Status) + require.Empty(t, workspace.LatestBuild.Job.Error) +} From 542cf36acf3ab1f7a3f38dd744320c6d947523c7 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Mon, 23 Jun 2025 20:39:05 +0200 Subject: [PATCH 2/2] chore: review comment Signed-off-by: Danny Kopping --- coderd/agentapi/subagent.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/agentapi/subagent.go b/coderd/agentapi/subagent.go index 6e7082c02415b..1868ad39bd362 100644 --- a/coderd/agentapi/subagent.go +++ b/coderd/agentapi/subagent.go @@ -166,7 +166,7 @@ func (a *SubAgentAPI) CreateSubAgent(ctx context.Context, req *agentproto.Create } _, err := a.Database.UpsertWorkspaceApp(ctx, database.UpsertWorkspaceAppParams{ - ID: uuid.New(), // TODO: we may need to maintain the app's ID here for stability, but for now we'll leave this as-is. + ID: uuid.New(), // NOTE: we may need to maintain the app's ID here for stability, but for now we'll leave this as-is. CreatedAt: createdAt, AgentID: subAgent.ID, Slug: app.Slug,