diff --git a/cli/state_test.go b/cli/state_test.go index 05fa7da077..a84a92367e 100644 --- a/cli/state_test.go +++ b/cli/state_test.go @@ -33,7 +33,7 @@ func TestStatePull(t *testing.T) { OrganizationID: owner.OrganizationID, OwnerID: taUser.ID, }). - Seed(database.WorkspaceBuild{ProvisionerState: wantState}). + Seed(database.WorkspaceBuild{}).ProvisionerState(wantState). Do() statefilePath := filepath.Join(t.TempDir(), "state") inv, root := clitest.New(t, "state", "pull", r.Workspace.Name, statefilePath) @@ -54,7 +54,7 @@ func TestStatePull(t *testing.T) { OrganizationID: owner.OrganizationID, OwnerID: taUser.ID, }). - Seed(database.WorkspaceBuild{ProvisionerState: wantState}). + Seed(database.WorkspaceBuild{}).ProvisionerState(wantState). Do() inv, root := clitest.New(t, "state", "pull", r.Workspace.Name) var gotState bytes.Buffer @@ -74,7 +74,7 @@ func TestStatePull(t *testing.T) { OrganizationID: owner.OrganizationID, OwnerID: taUser.ID, }). - Seed(database.WorkspaceBuild{ProvisionerState: wantState}). + Seed(database.WorkspaceBuild{}).ProvisionerState(wantState). Do() inv, root := clitest.New(t, "state", "pull", taUser.Username+"/"+r.Workspace.Name, "--build", fmt.Sprintf("%d", r.Build.BuildNumber)) @@ -170,7 +170,7 @@ func TestStatePush(t *testing.T) { OrganizationID: owner.OrganizationID, OwnerID: taUser.ID, }). - Seed(database.WorkspaceBuild{ProvisionerState: initialState}). + Seed(database.WorkspaceBuild{}).ProvisionerState(initialState). Do() wantState := []byte("updated state") stateFile, err := os.CreateTemp(t.TempDir(), "") diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 629f249534..e23ea72162 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -668,6 +668,31 @@ var ( }), Scope: rbac.ScopeAll, }.WithCachedASTValue() + + subjectWorkspaceBuilder = rbac.Subject{ + Type: rbac.SubjectTypeWorkspaceBuilder, + FriendlyName: "Workspace Builder", + ID: uuid.Nil.String(), + Roles: rbac.Roles([]rbac.Role{ + { + Identifier: rbac.RoleIdentifier{Name: "workspace-builder"}, + DisplayName: "Workspace Builder", + Site: rbac.Permissions(map[string][]policy.Action{ + // Reading provisioner daemons to check eligibility. + rbac.ResourceProvisionerDaemon.Type: {policy.ActionRead}, + // Updating provisioner jobs (e.g. marking prebuild + // jobs complete). + rbac.ResourceProvisionerJobs.Type: {policy.ActionUpdate}, + // Reading provisioner state requires template update + // permission. + rbac.ResourceTemplate.Type: {policy.ActionUpdate}, + }), + User: []rbac.Permission{}, + ByOrgID: map[string]rbac.OrgPermissions{}, + }, + }), + Scope: rbac.ScopeAll, + }.WithCachedASTValue() ) // AsProvisionerd returns a context with an actor that has permissions required @@ -774,6 +799,14 @@ func AsBoundaryUsageTracker(ctx context.Context) context.Context { return As(ctx, subjectBoundaryUsageTracker) } +// AsWorkspaceBuilder returns a context with an actor that has permissions +// required for the workspace builder to prepare workspace builds. This +// includes reading provisioner daemons, updating provisioner jobs, and +// reading provisioner state (which requires template update permission). +func AsWorkspaceBuilder(ctx context.Context) context.Context { + return As(ctx, subjectWorkspaceBuilder) +} + var AsRemoveActor = rbac.Subject{ ID: "remove-actor", } @@ -2257,7 +2290,7 @@ func (q *querier) GetAuditLogsOffset(ctx context.Context, arg database.GetAuditL } func (q *querier) GetAuthenticatedWorkspaceAgentAndBuildByAuthToken(ctx context.Context, authToken uuid.UUID) (database.GetAuthenticatedWorkspaceAgentAndBuildByAuthTokenRow, error) { - // This is a system function + // This is a system function. if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil { return database.GetAuthenticatedWorkspaceAgentAndBuildByAuthTokenRow{}, err } @@ -3914,6 +3947,11 @@ func (q *querier) GetWorkspaceBuildParametersByBuildIDs(ctx context.Context, wor return q.db.GetAuthorizedWorkspaceBuildParametersByBuildIDs(ctx, workspaceBuildIDs, prep) } +func (q *querier) GetWorkspaceBuildProvisionerStateByID(ctx context.Context, buildID uuid.UUID) (database.GetWorkspaceBuildProvisionerStateByIDRow, error) { + // Fetching the provisioner state requires Update permission on the template. + return fetchWithAction(q.log, q.auth, policy.ActionUpdate, q.db.GetWorkspaceBuildProvisionerStateByID)(ctx, buildID) +} + func (q *querier) GetWorkspaceBuildStatsByTemplates(ctx context.Context, since time.Time) ([]database.GetWorkspaceBuildStatsByTemplatesRow, error) { if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil { return nil, err diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 1fb1c81f7a..6e14d7fd5f 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -1969,6 +1969,15 @@ func (s *MethodTestSuite) TestWorkspace() { dbm.EXPECT().GetWorkspaceByID(gomock.Any(), ws.ID).Return(ws, nil).AnyTimes() check.Args(build.ID).Asserts(ws, policy.ActionRead).Returns(build) })) + s.Run("GetWorkspaceBuildProvisionerStateByID", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { + row := database.GetWorkspaceBuildProvisionerStateByIDRow{ + ProvisionerState: []byte("state"), + TemplateID: uuid.New(), + TemplateOrganizationID: uuid.New(), + } + dbm.EXPECT().GetWorkspaceBuildProvisionerStateByID(gomock.Any(), gomock.Any()).Return(row, nil).AnyTimes() + check.Args(uuid.New()).Asserts(row, policy.ActionUpdate).Returns(row) + })) s.Run("GetWorkspaceBuildByJobID", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { ws := testutil.Fake(s.T(), faker, database.Workspace{}) build := testutil.Fake(s.T(), faker, database.WorkspaceBuild{WorkspaceID: ws.ID}) diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index d0a019b32c..d939141caa 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -67,6 +67,8 @@ type WorkspaceBuildBuilder struct { jobError string // Error message for failed jobs jobErrorCode string // Error code for failed jobs + + provisionerState []byte } // BuilderOption is a functional option for customizing job timestamps @@ -138,6 +140,15 @@ func (b WorkspaceBuildBuilder) Seed(seed database.WorkspaceBuild) WorkspaceBuild return b } +// ProvisionerState sets the provisioner state for the workspace build. +// This is stored separately from the seed because ProvisionerState is +// not part of the WorkspaceBuild view struct. +func (b WorkspaceBuildBuilder) ProvisionerState(state []byte) WorkspaceBuildBuilder { + //nolint: revive // returns modified struct + b.provisionerState = state + return b +} + func (b WorkspaceBuildBuilder) Resource(resource ...*sdkproto.Resource) WorkspaceBuildBuilder { //nolint: revive // returns modified struct b.resources = append(b.resources, resource...) @@ -464,6 +475,14 @@ func (b WorkspaceBuildBuilder) doInTX() WorkspaceResponse { } resp.Build = dbgen.WorkspaceBuild(b.t, b.db, b.seed) + if len(b.provisionerState) > 0 { + err = b.db.UpdateWorkspaceBuildProvisionerStateByID(ownerCtx, database.UpdateWorkspaceBuildProvisionerStateByIDParams{ + ID: resp.Build.ID, + UpdatedAt: dbtime.Now(), + ProvisionerState: b.provisionerState, + }) + require.NoError(b.t, err, "update provisioner state") + } b.logger.Debug(context.Background(), "created workspace build", slog.F("build_id", resp.Build.ID), slog.F("workspace_id", resp.Workspace.ID), diff --git a/coderd/database/dbgen/dbgen.go b/coderd/database/dbgen/dbgen.go index 0fdc1ea522..94606054e3 100644 --- a/coderd/database/dbgen/dbgen.go +++ b/coderd/database/dbgen/dbgen.go @@ -504,7 +504,7 @@ func WorkspaceBuild(t testing.TB, db database.Store, orig database.WorkspaceBuil Transition: takeFirst(orig.Transition, database.WorkspaceTransitionStart), InitiatorID: takeFirst(orig.InitiatorID, uuid.New()), JobID: jobID, - ProvisionerState: takeFirstSlice(orig.ProvisionerState, []byte{}), + ProvisionerState: []byte{}, Deadline: takeFirst(orig.Deadline, dbtime.Now().Add(time.Hour)), MaxDeadline: takeFirst(orig.MaxDeadline, time.Time{}), Reason: takeFirst(orig.Reason, database.BuildReasonInitiator), diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index 00bddf3688..4859892a2c 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -2430,6 +2430,14 @@ func (m queryMetricsStore) GetWorkspaceBuildParametersByBuildIDs(ctx context.Con return r0, r1 } +func (m queryMetricsStore) GetWorkspaceBuildProvisionerStateByID(ctx context.Context, workspaceBuildID uuid.UUID) (database.GetWorkspaceBuildProvisionerStateByIDRow, error) { + start := time.Now() + r0, r1 := m.s.GetWorkspaceBuildProvisionerStateByID(ctx, workspaceBuildID) + m.queryLatencies.WithLabelValues("GetWorkspaceBuildProvisionerStateByID").Observe(time.Since(start).Seconds()) + m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "GetWorkspaceBuildProvisionerStateByID").Inc() + return r0, r1 +} + func (m queryMetricsStore) GetWorkspaceBuildStatsByTemplates(ctx context.Context, since time.Time) ([]database.GetWorkspaceBuildStatsByTemplatesRow, error) { start := time.Now() r0, r1 := m.s.GetWorkspaceBuildStatsByTemplates(ctx, since) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 15d0ac6cea..ce8a253181 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -4544,6 +4544,21 @@ func (mr *MockStoreMockRecorder) GetWorkspaceBuildParametersByBuildIDs(ctx, work return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetWorkspaceBuildParametersByBuildIDs", reflect.TypeOf((*MockStore)(nil).GetWorkspaceBuildParametersByBuildIDs), ctx, workspaceBuildIds) } +// GetWorkspaceBuildProvisionerStateByID mocks base method. +func (m *MockStore) GetWorkspaceBuildProvisionerStateByID(ctx context.Context, workspaceBuildID uuid.UUID) (database.GetWorkspaceBuildProvisionerStateByIDRow, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetWorkspaceBuildProvisionerStateByID", ctx, workspaceBuildID) + ret0, _ := ret[0].(database.GetWorkspaceBuildProvisionerStateByIDRow) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetWorkspaceBuildProvisionerStateByID indicates an expected call of GetWorkspaceBuildProvisionerStateByID. +func (mr *MockStoreMockRecorder) GetWorkspaceBuildProvisionerStateByID(ctx, workspaceBuildID any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetWorkspaceBuildProvisionerStateByID", reflect.TypeOf((*MockStore)(nil).GetWorkspaceBuildProvisionerStateByID), ctx, workspaceBuildID) +} + // GetWorkspaceBuildStatsByTemplates mocks base method. func (m *MockStore) GetWorkspaceBuildStatsByTemplates(ctx context.Context, since time.Time) ([]database.GetWorkspaceBuildStatsByTemplatesRow, error) { m.ctrl.T.Helper() diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 7a6e4ad8fc..2ff7f878c0 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -2708,7 +2708,6 @@ CREATE VIEW workspace_build_with_user AS workspace_builds.build_number, workspace_builds.transition, workspace_builds.initiator_id, - workspace_builds.provisioner_state, workspace_builds.job_id, workspace_builds.deadline, workspace_builds.reason, diff --git a/coderd/database/gentest/models_test.go b/coderd/database/gentest/models_test.go index 7cd54224cf..cf27671a2c 100644 --- a/coderd/database/gentest/models_test.go +++ b/coderd/database/gentest/models_test.go @@ -51,15 +51,34 @@ func TestViewSubsetTemplateVersion(t *testing.T) { } } -// TestViewSubsetWorkspaceBuild ensures WorkspaceBuildTable is a subset of WorkspaceBuild +// TestViewSubsetWorkspaceBuild ensures WorkspaceBuildTable is a subset of +// WorkspaceBuild, with the exception of ProvisionerState which is +// intentionally excluded from the workspace_build_with_user view to avoid +// loading the large Terraform state blob on hot paths. func TestViewSubsetWorkspaceBuild(t *testing.T) { t.Parallel() table := reflect.TypeOf(database.WorkspaceBuildTable{}) joined := reflect.TypeOf(database.WorkspaceBuild{}) - tableFields := allFields(table) - joinedFields := allFields(joined) - if !assert.Subset(t, fieldNames(joinedFields), fieldNames(tableFields), "table is not subset") { + tableFields := fieldNames(allFields(table)) + joinedFields := fieldNames(allFields(joined)) + + // ProvisionerState is intentionally excluded from the + // workspace_build_with_user view to avoid loading multi-MB Terraform + // state blobs on hot paths. Callers that need it use + // GetWorkspaceBuildProvisionerStateByID instead. + excludedFields := map[string]bool{ + "ProvisionerState": true, + } + + var filtered []string + for _, name := range tableFields { + if !excludedFields[name] { + filtered = append(filtered, name) + } + } + + if !assert.Subset(t, joinedFields, filtered, "table is not subset") { t.Log("Some fields were added to the WorkspaceBuild Table without updating the 'workspace_build_with_user' view.") t.Log("See migration 000141_join_users_build_version.up.sql to create the view.") } diff --git a/coderd/database/migrations/000421_workspace_build_view_drop_provisioner_state.down.sql b/coderd/database/migrations/000421_workspace_build_view_drop_provisioner_state.down.sql new file mode 100644 index 0000000000..74b2d4d924 --- /dev/null +++ b/coderd/database/migrations/000421_workspace_build_view_drop_provisioner_state.down.sql @@ -0,0 +1,31 @@ +-- Restore provisioner_state to workspace_build_with_user view. +DROP VIEW workspace_build_with_user; + +CREATE VIEW workspace_build_with_user AS +SELECT + workspace_builds.id, + workspace_builds.created_at, + workspace_builds.updated_at, + workspace_builds.workspace_id, + workspace_builds.template_version_id, + workspace_builds.build_number, + workspace_builds.transition, + workspace_builds.initiator_id, + workspace_builds.provisioner_state, + workspace_builds.job_id, + workspace_builds.deadline, + workspace_builds.reason, + workspace_builds.daily_cost, + workspace_builds.max_deadline, + workspace_builds.template_version_preset_id, + workspace_builds.has_ai_task, + workspace_builds.has_external_agent, + COALESCE(visible_users.avatar_url, ''::text) AS initiator_by_avatar_url, + COALESCE(visible_users.username, ''::text) AS initiator_by_username, + COALESCE(visible_users.name, ''::text) AS initiator_by_name +FROM + workspace_builds +LEFT JOIN + visible_users ON workspace_builds.initiator_id = visible_users.id; + +COMMENT ON VIEW workspace_build_with_user IS 'Joins in the username + avatar url of the initiated by user.'; diff --git a/coderd/database/migrations/000421_workspace_build_view_drop_provisioner_state.up.sql b/coderd/database/migrations/000421_workspace_build_view_drop_provisioner_state.up.sql new file mode 100644 index 0000000000..e3562b6a1d --- /dev/null +++ b/coderd/database/migrations/000421_workspace_build_view_drop_provisioner_state.up.sql @@ -0,0 +1,33 @@ +-- Drop and recreate workspace_build_with_user to exclude provisioner_state. +-- This avoids loading the large Terraform state blob (1-5 MB per workspace) +-- on every query that uses this view. The callers that need provisioner_state +-- now fetch it separately via GetWorkspaceBuildProvisionerStateByID. +DROP VIEW workspace_build_with_user; + +CREATE VIEW workspace_build_with_user AS +SELECT + workspace_builds.id, + workspace_builds.created_at, + workspace_builds.updated_at, + workspace_builds.workspace_id, + workspace_builds.template_version_id, + workspace_builds.build_number, + workspace_builds.transition, + workspace_builds.initiator_id, + workspace_builds.job_id, + workspace_builds.deadline, + workspace_builds.reason, + workspace_builds.daily_cost, + workspace_builds.max_deadline, + workspace_builds.template_version_preset_id, + workspace_builds.has_ai_task, + workspace_builds.has_external_agent, + COALESCE(visible_users.avatar_url, ''::text) AS initiator_by_avatar_url, + COALESCE(visible_users.username, ''::text) AS initiator_by_username, + COALESCE(visible_users.name, ''::text) AS initiator_by_name +FROM + workspace_builds +LEFT JOIN + visible_users ON workspace_builds.initiator_id = visible_users.id; + +COMMENT ON VIEW workspace_build_with_user IS 'Joins in the username + avatar url of the initiated by user.'; diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index 5008de03f3..072737f4a8 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -316,6 +316,14 @@ func (t GetFileTemplatesRow) RBACObject() rbac.Object { WithGroupACL(t.GroupACL) } +// RBACObject for a workspace build's provisioner state requires Update access of the template. +func (t GetWorkspaceBuildProvisionerStateByIDRow) RBACObject() rbac.Object { + return rbac.ResourceTemplate.WithID(t.TemplateID). + InOrg(t.TemplateOrganizationID). + WithACLUserList(t.UserACL). + WithGroupACL(t.GroupACL) +} + func (t Template) DeepCopy() Template { cpy := t cpy.UserACL = maps.Clone(t.UserACL) diff --git a/coderd/database/models.go b/coderd/database/models.go index 24ab3608f9..8a05c655da 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -4987,7 +4987,6 @@ type WorkspaceBuild struct { BuildNumber int32 `db:"build_number" json:"build_number"` Transition WorkspaceTransition `db:"transition" json:"transition"` InitiatorID uuid.UUID `db:"initiator_id" json:"initiator_id"` - ProvisionerState []byte `db:"provisioner_state" json:"provisioner_state"` JobID uuid.UUID `db:"job_id" json:"job_id"` Deadline time.Time `db:"deadline" json:"deadline"` Reason BuildReason `db:"reason" json:"reason"` diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 4ecc0c5017..f5a6cefe45 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -506,6 +506,11 @@ type sqlcQuerier interface { GetWorkspaceBuildMetricsByResourceID(ctx context.Context, id uuid.UUID) (GetWorkspaceBuildMetricsByResourceIDRow, error) GetWorkspaceBuildParameters(ctx context.Context, workspaceBuildID uuid.UUID) ([]WorkspaceBuildParameter, error) GetWorkspaceBuildParametersByBuildIDs(ctx context.Context, workspaceBuildIds []uuid.UUID) ([]WorkspaceBuildParameter, error) + // Fetches the provisioner state of a workspace build, joined through to the + // template so that dbauthz can enforce policy.ActionUpdate on the template. + // Provisioner state contains sensitive Terraform state and should only be + // accessible to template administrators. + GetWorkspaceBuildProvisionerStateByID(ctx context.Context, workspaceBuildID uuid.UUID) (GetWorkspaceBuildProvisionerStateByIDRow, error) GetWorkspaceBuildStatsByTemplates(ctx context.Context, since time.Time) ([]GetWorkspaceBuildStatsByTemplatesRow, error) GetWorkspaceBuildsByWorkspaceID(ctx context.Context, arg GetWorkspaceBuildsByWorkspaceIDParams) ([]WorkspaceBuild, error) GetWorkspaceBuildsCreatedAfter(ctx context.Context, createdAt time.Time) ([]WorkspaceBuild, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 2eb8fdb0a2..33ebea2a80 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -17955,7 +17955,7 @@ const getAuthenticatedWorkspaceAgentAndBuildByAuthToken = `-- name: GetAuthentic SELECT workspaces.id, workspaces.created_at, workspaces.updated_at, workspaces.owner_id, workspaces.organization_id, workspaces.template_id, workspaces.deleted, workspaces.name, workspaces.autostart_schedule, workspaces.ttl, workspaces.last_used_at, workspaces.dormant_at, workspaces.deleting_at, workspaces.automatic_updates, workspaces.favorite, workspaces.next_start_at, workspaces.group_acl, workspaces.user_acl, workspace_agents.id, workspace_agents.created_at, workspace_agents.updated_at, workspace_agents.name, workspace_agents.first_connected_at, workspace_agents.last_connected_at, workspace_agents.disconnected_at, workspace_agents.resource_id, workspace_agents.auth_token, workspace_agents.auth_instance_id, workspace_agents.architecture, workspace_agents.environment_variables, workspace_agents.operating_system, workspace_agents.instance_metadata, workspace_agents.resource_metadata, workspace_agents.directory, workspace_agents.version, workspace_agents.last_connected_replica_id, workspace_agents.connection_timeout_seconds, workspace_agents.troubleshooting_url, workspace_agents.motd_file, workspace_agents.lifecycle_state, workspace_agents.expanded_directory, workspace_agents.logs_length, workspace_agents.logs_overflowed, workspace_agents.started_at, workspace_agents.ready_at, workspace_agents.subsystems, workspace_agents.display_apps, workspace_agents.api_version, workspace_agents.display_order, workspace_agents.parent_id, workspace_agents.api_key_scope, workspace_agents.deleted, - workspace_build_with_user.id, workspace_build_with_user.created_at, workspace_build_with_user.updated_at, workspace_build_with_user.workspace_id, workspace_build_with_user.template_version_id, workspace_build_with_user.build_number, workspace_build_with_user.transition, workspace_build_with_user.initiator_id, workspace_build_with_user.provisioner_state, workspace_build_with_user.job_id, workspace_build_with_user.deadline, workspace_build_with_user.reason, workspace_build_with_user.daily_cost, workspace_build_with_user.max_deadline, workspace_build_with_user.template_version_preset_id, workspace_build_with_user.has_ai_task, workspace_build_with_user.has_external_agent, workspace_build_with_user.initiator_by_avatar_url, workspace_build_with_user.initiator_by_username, workspace_build_with_user.initiator_by_name, + workspace_build_with_user.id, workspace_build_with_user.created_at, workspace_build_with_user.updated_at, workspace_build_with_user.workspace_id, workspace_build_with_user.template_version_id, workspace_build_with_user.build_number, workspace_build_with_user.transition, workspace_build_with_user.initiator_id, workspace_build_with_user.job_id, workspace_build_with_user.deadline, workspace_build_with_user.reason, workspace_build_with_user.daily_cost, workspace_build_with_user.max_deadline, workspace_build_with_user.template_version_preset_id, workspace_build_with_user.has_ai_task, workspace_build_with_user.has_external_agent, workspace_build_with_user.initiator_by_avatar_url, workspace_build_with_user.initiator_by_username, workspace_build_with_user.initiator_by_name, tasks.id AS task_id FROM workspace_agents @@ -18093,7 +18093,6 @@ func (q *sqlQuerier) GetAuthenticatedWorkspaceAgentAndBuildByAuthToken(ctx conte &i.WorkspaceBuild.BuildNumber, &i.WorkspaceBuild.Transition, &i.WorkspaceBuild.InitiatorID, - &i.WorkspaceBuild.ProvisionerState, &i.WorkspaceBuild.JobID, &i.WorkspaceBuild.Deadline, &i.WorkspaceBuild.Reason, @@ -21007,7 +21006,7 @@ func (q *sqlQuerier) InsertWorkspaceBuildParameters(ctx context.Context, arg Ins } const getActiveWorkspaceBuildsByTemplateID = `-- name: GetActiveWorkspaceBuildsByTemplateID :many -SELECT wb.id, wb.created_at, wb.updated_at, wb.workspace_id, wb.template_version_id, wb.build_number, wb.transition, wb.initiator_id, wb.provisioner_state, wb.job_id, wb.deadline, wb.reason, wb.daily_cost, wb.max_deadline, wb.template_version_preset_id, wb.has_ai_task, wb.has_external_agent, wb.initiator_by_avatar_url, wb.initiator_by_username, wb.initiator_by_name +SELECT wb.id, wb.created_at, wb.updated_at, wb.workspace_id, wb.template_version_id, wb.build_number, wb.transition, wb.initiator_id, wb.job_id, wb.deadline, wb.reason, wb.daily_cost, wb.max_deadline, wb.template_version_preset_id, wb.has_ai_task, wb.has_external_agent, wb.initiator_by_avatar_url, wb.initiator_by_username, wb.initiator_by_name FROM ( SELECT workspace_id, MAX(build_number) as max_build_number @@ -21055,7 +21054,6 @@ func (q *sqlQuerier) GetActiveWorkspaceBuildsByTemplateID(ctx context.Context, t &i.BuildNumber, &i.Transition, &i.InitiatorID, - &i.ProvisionerState, &i.JobID, &i.Deadline, &i.Reason, @@ -21163,7 +21161,7 @@ func (q *sqlQuerier) GetFailedWorkspaceBuildsByTemplateID(ctx context.Context, a const getLatestWorkspaceBuildByWorkspaceID = `-- name: GetLatestWorkspaceBuildByWorkspaceID :one SELECT - id, created_at, updated_at, workspace_id, template_version_id, build_number, transition, initiator_id, provisioner_state, job_id, deadline, reason, daily_cost, max_deadline, template_version_preset_id, has_ai_task, has_external_agent, initiator_by_avatar_url, initiator_by_username, initiator_by_name + id, created_at, updated_at, workspace_id, template_version_id, build_number, transition, initiator_id, job_id, deadline, reason, daily_cost, max_deadline, template_version_preset_id, has_ai_task, has_external_agent, initiator_by_avatar_url, initiator_by_username, initiator_by_name FROM workspace_build_with_user AS workspace_builds WHERE @@ -21186,7 +21184,6 @@ func (q *sqlQuerier) GetLatestWorkspaceBuildByWorkspaceID(ctx context.Context, w &i.BuildNumber, &i.Transition, &i.InitiatorID, - &i.ProvisionerState, &i.JobID, &i.Deadline, &i.Reason, @@ -21205,7 +21202,7 @@ func (q *sqlQuerier) GetLatestWorkspaceBuildByWorkspaceID(ctx context.Context, w const getLatestWorkspaceBuildsByWorkspaceIDs = `-- name: GetLatestWorkspaceBuildsByWorkspaceIDs :many SELECT DISTINCT ON (workspace_id) - id, created_at, updated_at, workspace_id, template_version_id, build_number, transition, initiator_id, provisioner_state, job_id, deadline, reason, daily_cost, max_deadline, template_version_preset_id, has_ai_task, has_external_agent, initiator_by_avatar_url, initiator_by_username, initiator_by_name + id, created_at, updated_at, workspace_id, template_version_id, build_number, transition, initiator_id, job_id, deadline, reason, daily_cost, max_deadline, template_version_preset_id, has_ai_task, has_external_agent, initiator_by_avatar_url, initiator_by_username, initiator_by_name FROM workspace_build_with_user AS workspace_builds WHERE @@ -21232,7 +21229,6 @@ func (q *sqlQuerier) GetLatestWorkspaceBuildsByWorkspaceIDs(ctx context.Context, &i.BuildNumber, &i.Transition, &i.InitiatorID, - &i.ProvisionerState, &i.JobID, &i.Deadline, &i.Reason, @@ -21260,7 +21256,7 @@ func (q *sqlQuerier) GetLatestWorkspaceBuildsByWorkspaceIDs(ctx context.Context, const getWorkspaceBuildByID = `-- name: GetWorkspaceBuildByID :one SELECT - id, created_at, updated_at, workspace_id, template_version_id, build_number, transition, initiator_id, provisioner_state, job_id, deadline, reason, daily_cost, max_deadline, template_version_preset_id, has_ai_task, has_external_agent, initiator_by_avatar_url, initiator_by_username, initiator_by_name + id, created_at, updated_at, workspace_id, template_version_id, build_number, transition, initiator_id, job_id, deadline, reason, daily_cost, max_deadline, template_version_preset_id, has_ai_task, has_external_agent, initiator_by_avatar_url, initiator_by_username, initiator_by_name FROM workspace_build_with_user AS workspace_builds WHERE @@ -21281,7 +21277,6 @@ func (q *sqlQuerier) GetWorkspaceBuildByID(ctx context.Context, id uuid.UUID) (W &i.BuildNumber, &i.Transition, &i.InitiatorID, - &i.ProvisionerState, &i.JobID, &i.Deadline, &i.Reason, @@ -21299,7 +21294,7 @@ func (q *sqlQuerier) GetWorkspaceBuildByID(ctx context.Context, id uuid.UUID) (W const getWorkspaceBuildByJobID = `-- name: GetWorkspaceBuildByJobID :one SELECT - id, created_at, updated_at, workspace_id, template_version_id, build_number, transition, initiator_id, provisioner_state, job_id, deadline, reason, daily_cost, max_deadline, template_version_preset_id, has_ai_task, has_external_agent, initiator_by_avatar_url, initiator_by_username, initiator_by_name + id, created_at, updated_at, workspace_id, template_version_id, build_number, transition, initiator_id, job_id, deadline, reason, daily_cost, max_deadline, template_version_preset_id, has_ai_task, has_external_agent, initiator_by_avatar_url, initiator_by_username, initiator_by_name FROM workspace_build_with_user AS workspace_builds WHERE @@ -21320,7 +21315,6 @@ func (q *sqlQuerier) GetWorkspaceBuildByJobID(ctx context.Context, jobID uuid.UU &i.BuildNumber, &i.Transition, &i.InitiatorID, - &i.ProvisionerState, &i.JobID, &i.Deadline, &i.Reason, @@ -21338,7 +21332,7 @@ func (q *sqlQuerier) GetWorkspaceBuildByJobID(ctx context.Context, jobID uuid.UU const getWorkspaceBuildByWorkspaceIDAndBuildNumber = `-- name: GetWorkspaceBuildByWorkspaceIDAndBuildNumber :one SELECT - id, created_at, updated_at, workspace_id, template_version_id, build_number, transition, initiator_id, provisioner_state, job_id, deadline, reason, daily_cost, max_deadline, template_version_preset_id, has_ai_task, has_external_agent, initiator_by_avatar_url, initiator_by_username, initiator_by_name + id, created_at, updated_at, workspace_id, template_version_id, build_number, transition, initiator_id, job_id, deadline, reason, daily_cost, max_deadline, template_version_preset_id, has_ai_task, has_external_agent, initiator_by_avatar_url, initiator_by_username, initiator_by_name FROM workspace_build_with_user AS workspace_builds WHERE @@ -21363,7 +21357,6 @@ func (q *sqlQuerier) GetWorkspaceBuildByWorkspaceIDAndBuildNumber(ctx context.Co &i.BuildNumber, &i.Transition, &i.InitiatorID, - &i.ProvisionerState, &i.JobID, &i.Deadline, &i.Reason, @@ -21435,6 +21428,48 @@ func (q *sqlQuerier) GetWorkspaceBuildMetricsByResourceID(ctx context.Context, i return i, err } +const getWorkspaceBuildProvisionerStateByID = `-- name: GetWorkspaceBuildProvisionerStateByID :one +SELECT + workspace_builds.provisioner_state, + templates.id AS template_id, + templates.organization_id AS template_organization_id, + templates.user_acl, + templates.group_acl +FROM + workspace_builds +INNER JOIN + workspaces ON workspaces.id = workspace_builds.workspace_id +INNER JOIN + templates ON templates.id = workspaces.template_id +WHERE + workspace_builds.id = $1 +` + +type GetWorkspaceBuildProvisionerStateByIDRow struct { + ProvisionerState []byte `db:"provisioner_state" json:"provisioner_state"` + TemplateID uuid.UUID `db:"template_id" json:"template_id"` + TemplateOrganizationID uuid.UUID `db:"template_organization_id" json:"template_organization_id"` + UserACL TemplateACL `db:"user_acl" json:"user_acl"` + GroupACL TemplateACL `db:"group_acl" json:"group_acl"` +} + +// Fetches the provisioner state of a workspace build, joined through to the +// template so that dbauthz can enforce policy.ActionUpdate on the template. +// Provisioner state contains sensitive Terraform state and should only be +// accessible to template administrators. +func (q *sqlQuerier) GetWorkspaceBuildProvisionerStateByID(ctx context.Context, workspaceBuildID uuid.UUID) (GetWorkspaceBuildProvisionerStateByIDRow, error) { + row := q.db.QueryRowContext(ctx, getWorkspaceBuildProvisionerStateByID, workspaceBuildID) + var i GetWorkspaceBuildProvisionerStateByIDRow + err := row.Scan( + &i.ProvisionerState, + &i.TemplateID, + &i.TemplateOrganizationID, + &i.UserACL, + &i.GroupACL, + ) + return i, err +} + const getWorkspaceBuildStatsByTemplates = `-- name: GetWorkspaceBuildStatsByTemplates :many SELECT w.template_id, @@ -21504,7 +21539,7 @@ func (q *sqlQuerier) GetWorkspaceBuildStatsByTemplates(ctx context.Context, sinc const getWorkspaceBuildsByWorkspaceID = `-- name: GetWorkspaceBuildsByWorkspaceID :many SELECT - id, created_at, updated_at, workspace_id, template_version_id, build_number, transition, initiator_id, provisioner_state, job_id, deadline, reason, daily_cost, max_deadline, template_version_preset_id, has_ai_task, has_external_agent, initiator_by_avatar_url, initiator_by_username, initiator_by_name + id, created_at, updated_at, workspace_id, template_version_id, build_number, transition, initiator_id, job_id, deadline, reason, daily_cost, max_deadline, template_version_preset_id, has_ai_task, has_external_agent, initiator_by_avatar_url, initiator_by_username, initiator_by_name FROM workspace_build_with_user AS workspace_builds WHERE @@ -21568,7 +21603,6 @@ func (q *sqlQuerier) GetWorkspaceBuildsByWorkspaceID(ctx context.Context, arg Ge &i.BuildNumber, &i.Transition, &i.InitiatorID, - &i.ProvisionerState, &i.JobID, &i.Deadline, &i.Reason, @@ -21595,7 +21629,7 @@ func (q *sqlQuerier) GetWorkspaceBuildsByWorkspaceID(ctx context.Context, arg Ge } const getWorkspaceBuildsCreatedAfter = `-- name: GetWorkspaceBuildsCreatedAfter :many -SELECT id, created_at, updated_at, workspace_id, template_version_id, build_number, transition, initiator_id, provisioner_state, job_id, deadline, reason, daily_cost, max_deadline, template_version_preset_id, has_ai_task, has_external_agent, initiator_by_avatar_url, initiator_by_username, initiator_by_name FROM workspace_build_with_user WHERE created_at > $1 +SELECT id, created_at, updated_at, workspace_id, template_version_id, build_number, transition, initiator_id, job_id, deadline, reason, daily_cost, max_deadline, template_version_preset_id, has_ai_task, has_external_agent, initiator_by_avatar_url, initiator_by_username, initiator_by_name FROM workspace_build_with_user WHERE created_at > $1 ` func (q *sqlQuerier) GetWorkspaceBuildsCreatedAfter(ctx context.Context, createdAt time.Time) ([]WorkspaceBuild, error) { @@ -21616,7 +21650,6 @@ func (q *sqlQuerier) GetWorkspaceBuildsCreatedAfter(ctx context.Context, created &i.BuildNumber, &i.Transition, &i.InitiatorID, - &i.ProvisionerState, &i.JobID, &i.Deadline, &i.Reason, diff --git a/coderd/database/queries/workspacebuilds.sql b/coderd/database/queries/workspacebuilds.sql index aee50f7edf..d74136deb6 100644 --- a/coderd/database/queries/workspacebuilds.sql +++ b/coderd/database/queries/workspacebuilds.sql @@ -271,3 +271,23 @@ JOIN workspace_resources wr ON wr.job_id = wb.job_id JOIN workspace_agents wa ON wa.resource_id = wr.id WHERE wb.job_id = (SELECT job_id FROM workspace_resources WHERE workspace_resources.id = $1) GROUP BY wb.created_at, wb.transition, t.name, o.name, w.owner_id; + +-- name: GetWorkspaceBuildProvisionerStateByID :one +-- Fetches the provisioner state of a workspace build, joined through to the +-- template so that dbauthz can enforce policy.ActionUpdate on the template. +-- Provisioner state contains sensitive Terraform state and should only be +-- accessible to template administrators. +SELECT + workspace_builds.provisioner_state, + templates.id AS template_id, + templates.organization_id AS template_organization_id, + templates.user_acl, + templates.group_acl +FROM + workspace_builds +INNER JOIN + workspaces ON workspaces.id = workspace_builds.workspace_id +INNER JOIN + templates ON templates.id = workspaces.template_id +WHERE + workspace_builds.id = @workspace_build_id; diff --git a/coderd/jobreaper/detector.go b/coderd/jobreaper/detector.go index a24d18d7e3..b0bcc2d25d 100644 --- a/coderd/jobreaper/detector.go +++ b/coderd/jobreaper/detector.go @@ -348,8 +348,12 @@ func reapJob(ctx context.Context, log slog.Logger, db database.Store, pub pubsub // Only copy the provisioner state if there's no state in // the current build. - if len(build.ProvisionerState) == 0 { - // Get the previous build if it exists. + currentStateRow, err := db.GetWorkspaceBuildProvisionerStateByID(ctx, build.ID) + if err != nil { + return xerrors.Errorf("get workspace build provisioner state: %w", err) + } + if len(currentStateRow.ProvisionerState) == 0 { + // Get the previous build's state if it exists. prevBuild, err := db.GetWorkspaceBuildByWorkspaceIDAndBuildNumber(ctx, database.GetWorkspaceBuildByWorkspaceIDAndBuildNumberParams{ WorkspaceID: build.WorkspaceID, BuildNumber: build.BuildNumber - 1, @@ -358,10 +362,14 @@ func reapJob(ctx context.Context, log slog.Logger, db database.Store, pub pubsub return xerrors.Errorf("get previous workspace build: %w", err) } if err == nil { + prevStateRow, err := db.GetWorkspaceBuildProvisionerStateByID(ctx, prevBuild.ID) + if err != nil { + return xerrors.Errorf("get previous workspace build provisioner state: %w", err) + } err = db.UpdateWorkspaceBuildProvisionerStateByID(ctx, database.UpdateWorkspaceBuildProvisionerStateByIDParams{ ID: build.ID, UpdatedAt: dbtime.Now(), - ProvisionerState: prevBuild.ProvisionerState, + ProvisionerState: prevStateRow.ProvisionerState, }) if err != nil { return xerrors.Errorf("update workspace build by id: %w", err) diff --git a/coderd/jobreaper/detector_test.go b/coderd/jobreaper/detector_test.go index d92070a0e2..1f0df05e4f 100644 --- a/coderd/jobreaper/detector_test.go +++ b/coderd/jobreaper/detector_test.go @@ -126,9 +126,9 @@ func TestDetectorHungWorkspaceBuild(t *testing.T) { previousBuild := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ OrganizationID: org.ID, OwnerID: user.ID, - }).Pubsub(pubsub).Seed(database.WorkspaceBuild{ - ProvisionerState: expectedWorkspaceBuildState, - }).Succeeded(dbfake.WithJobCompletedAt(twentyMinAgo)). + }).Pubsub(pubsub).Seed(database.WorkspaceBuild{}). + ProvisionerState(expectedWorkspaceBuildState). + Succeeded(dbfake.WithJobCompletedAt(twentyMinAgo)). Do() // Current build (hung - running job with UpdatedAt > 5 min ago). @@ -163,7 +163,9 @@ func TestDetectorHungWorkspaceBuild(t *testing.T) { // Check that the provisioner state was copied. build, err := db.GetWorkspaceBuildByID(ctx, currentBuild.Build.ID) require.NoError(t, err) - require.Equal(t, expectedWorkspaceBuildState, build.ProvisionerState) + provisionerStateRow, err := db.GetWorkspaceBuildProvisionerStateByID(ctx, build.ID) + require.NoError(t, err) + require.Equal(t, expectedWorkspaceBuildState, provisionerStateRow.ProvisionerState) detector.Close() detector.Wait() @@ -194,9 +196,9 @@ func TestDetectorHungWorkspaceBuildNoOverrideState(t *testing.T) { previousBuild := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ OrganizationID: org.ID, OwnerID: user.ID, - }).Pubsub(pubsub).Seed(database.WorkspaceBuild{ - ProvisionerState: []byte(`{"dean":"NOT cool","colin":"also NOT cool"}`), - }).Succeeded(dbfake.WithJobCompletedAt(twentyMinAgo)). + }).Pubsub(pubsub).Seed(database.WorkspaceBuild{}). + ProvisionerState([]byte(`{"dean":"NOT cool","colin":"also NOT cool"}`)). + Succeeded(dbfake.WithJobCompletedAt(twentyMinAgo)). Do() // Current build (hung - running job with UpdatedAt > 5 min ago). @@ -204,9 +206,8 @@ func TestDetectorHungWorkspaceBuildNoOverrideState(t *testing.T) { currentBuild := dbfake.WorkspaceBuild(t, db, previousBuild.Workspace). Pubsub(pubsub). Seed(database.WorkspaceBuild{ - BuildNumber: 2, - ProvisionerState: expectedWorkspaceBuildState, - }). + BuildNumber: 2, + }).ProvisionerState(expectedWorkspaceBuildState). Starting(dbfake.WithJobStartedAt(tenMinAgo), dbfake.WithJobUpdatedAt(sixMinAgo)). Do() @@ -235,7 +236,9 @@ func TestDetectorHungWorkspaceBuildNoOverrideState(t *testing.T) { // Check that the provisioner state was NOT copied. build, err := db.GetWorkspaceBuildByID(ctx, currentBuild.Build.ID) require.NoError(t, err) - require.Equal(t, expectedWorkspaceBuildState, build.ProvisionerState) + provisionerStateRow, err := db.GetWorkspaceBuildProvisionerStateByID(ctx, build.ID) + require.NoError(t, err) + require.Equal(t, expectedWorkspaceBuildState, provisionerStateRow.ProvisionerState) detector.Close() detector.Wait() @@ -266,9 +269,9 @@ func TestDetectorHungWorkspaceBuildNoOverrideStateIfNoExistingBuild(t *testing.T currentBuild := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ OrganizationID: org.ID, OwnerID: user.ID, - }).Pubsub(pubsub).Seed(database.WorkspaceBuild{ - ProvisionerState: expectedWorkspaceBuildState, - }).Starting(dbfake.WithJobStartedAt(tenMinAgo), dbfake.WithJobUpdatedAt(sixMinAgo)). + }).Pubsub(pubsub).Seed(database.WorkspaceBuild{}). + ProvisionerState(expectedWorkspaceBuildState). + Starting(dbfake.WithJobStartedAt(tenMinAgo), dbfake.WithJobUpdatedAt(sixMinAgo)). Do() t.Log("current job ID: ", currentBuild.Build.JobID) @@ -295,7 +298,9 @@ func TestDetectorHungWorkspaceBuildNoOverrideStateIfNoExistingBuild(t *testing.T // Check that the provisioner state was NOT updated. build, err := db.GetWorkspaceBuildByID(ctx, currentBuild.Build.ID) require.NoError(t, err) - require.Equal(t, expectedWorkspaceBuildState, build.ProvisionerState) + provisionerStateRow, err := db.GetWorkspaceBuildProvisionerStateByID(ctx, build.ID) + require.NoError(t, err) + require.Equal(t, expectedWorkspaceBuildState, provisionerStateRow.ProvisionerState) detector.Close() detector.Wait() @@ -325,9 +330,9 @@ func TestDetectorPendingWorkspaceBuildNoOverrideStateIfNoExistingBuild(t *testin currentBuild := dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ OrganizationID: org.ID, OwnerID: user.ID, - }).Pubsub(pubsub).Seed(database.WorkspaceBuild{ - ProvisionerState: expectedWorkspaceBuildState, - }).Pending(dbfake.WithJobCreatedAt(thirtyFiveMinAgo), dbfake.WithJobUpdatedAt(thirtyFiveMinAgo)). + }).Pubsub(pubsub).Seed(database.WorkspaceBuild{}). + ProvisionerState(expectedWorkspaceBuildState). + Pending(dbfake.WithJobCreatedAt(thirtyFiveMinAgo), dbfake.WithJobUpdatedAt(thirtyFiveMinAgo)). Do() t.Log("current job ID: ", currentBuild.Build.JobID) @@ -356,7 +361,9 @@ func TestDetectorPendingWorkspaceBuildNoOverrideStateIfNoExistingBuild(t *testin // Check that the provisioner state was NOT updated. build, err := db.GetWorkspaceBuildByID(ctx, currentBuild.Build.ID) require.NoError(t, err) - require.Equal(t, expectedWorkspaceBuildState, build.ProvisionerState) + provisionerStateRow, err := db.GetWorkspaceBuildProvisionerStateByID(ctx, build.ID) + require.NoError(t, err) + require.Equal(t, expectedWorkspaceBuildState, provisionerStateRow.ProvisionerState) detector.Close() detector.Wait() @@ -398,9 +405,9 @@ func TestDetectorWorkspaceBuildForDormantWorkspace(t *testing.T) { Time: now.Add(-time.Hour), Valid: true, }, - }).Pubsub(pubsub).Seed(database.WorkspaceBuild{ - ProvisionerState: expectedWorkspaceBuildState, - }).Starting(dbfake.WithJobStartedAt(tenMinAgo), dbfake.WithJobUpdatedAt(sixMinAgo)). + }).Pubsub(pubsub).Seed(database.WorkspaceBuild{}). + ProvisionerState(expectedWorkspaceBuildState). + Starting(dbfake.WithJobStartedAt(tenMinAgo), dbfake.WithJobUpdatedAt(sixMinAgo)). Do() t.Log("current job ID: ", currentBuild.Build.JobID) diff --git a/coderd/provisionerdserver/provisionerdserver.go b/coderd/provisionerdserver/provisionerdserver.go index 0e4dd20a05..4bce052be4 100644 --- a/coderd/provisionerdserver/provisionerdserver.go +++ b/coderd/provisionerdserver/provisionerdserver.go @@ -725,11 +725,16 @@ func (s *server) acquireProtoJob(ctx context.Context, job database.ProvisionerJo } } + provisionerStateRow, err := s.Database.GetWorkspaceBuildProvisionerStateByID(ctx, workspaceBuild.ID) + if err != nil { + return nil, failJob(fmt.Sprintf("get workspace build provisioner state: %s", err)) + } + protoJob.Type = &proto.AcquiredJob_WorkspaceBuild_{ WorkspaceBuild: &proto.AcquiredJob_WorkspaceBuild{ WorkspaceBuildId: workspaceBuild.ID.String(), WorkspaceName: workspace.Name, - State: workspaceBuild.ProvisionerState, + State: provisionerStateRow.ProvisionerState, RichParameterValues: convertRichParameterValues(workspaceBuildParameters), PreviousParameterValues: convertRichParameterValues(lastWorkspaceBuildParameters), VariableValues: asVariableValues(templateVariables), diff --git a/coderd/provisionerdserver/provisionerdserver_test.go b/coderd/provisionerdserver/provisionerdserver_test.go index 6509591595..75e48fb399 100644 --- a/coderd/provisionerdserver/provisionerdserver_test.go +++ b/coderd/provisionerdserver/provisionerdserver_test.go @@ -1321,7 +1321,9 @@ func TestFailJob(t *testing.T) { <-publishedLogs build, err := db.GetWorkspaceBuildByID(ctx, buildID) require.NoError(t, err) - require.Equal(t, "some state", string(build.ProvisionerState)) + provisionerStateRow, err := db.GetWorkspaceBuildProvisionerStateByID(ctx, build.ID) + require.NoError(t, err) + require.Equal(t, "some state", string(provisionerStateRow.ProvisionerState)) require.Len(t, auditor.AuditLogs(), 1) // Assert that the workspace_id field get populated diff --git a/coderd/rbac/authz.go b/coderd/rbac/authz.go index 8e39734c10..99a75e5a0b 100644 --- a/coderd/rbac/authz.go +++ b/coderd/rbac/authz.go @@ -81,6 +81,7 @@ const ( SubjectAibridged SubjectType = "aibridged" SubjectTypeDBPurge SubjectType = "dbpurge" SubjectTypeBoundaryUsageTracker SubjectType = "boundary_usage_tracker" + SubjectTypeWorkspaceBuilder SubjectType = "workspace_builder" ) const ( diff --git a/coderd/workspacebuilds.go b/coderd/workspacebuilds.go index b9bc5d8d71..80602102c7 100644 --- a/coderd/workspacebuilds.go +++ b/coderd/workspacebuilds.go @@ -856,32 +856,24 @@ func (api *API) workspaceBuildLogs(rw http.ResponseWriter, r *http.Request) { func (api *API) workspaceBuildState(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() workspaceBuild := httpmw.WorkspaceBuildParam(r) - workspace, err := api.Database.GetWorkspaceByID(ctx, workspaceBuild.WorkspaceID) - if err != nil { - httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "No workspace exists for this job.", - }) + + // The dbauthz layer enforces policy.ActionUpdate on the template. + row, err := api.Database.GetWorkspaceBuildProvisionerStateByID(ctx, workspaceBuild.ID) + if httpapi.Is404Error(err) { + httpapi.ResourceNotFound(rw) return } - template, err := api.Database.GetTemplateByID(ctx, workspace.TemplateID) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Failed to get template", + Message: "Internal error fetching provisioner state.", Detail: err.Error(), }) return } - // You must have update permissions on the template to get the state. - // This matches a push! - if !api.Authorize(r, policy.ActionUpdate, template.RBACObject()) { - httpapi.ResourceNotFound(rw) - return - } - rw.Header().Set("Content-Type", "application/json") rw.WriteHeader(http.StatusOK) - _, _ = rw.Write(workspaceBuild.ProvisionerState) + _, _ = rw.Write(row.ProvisionerState) } // @Summary Update workspace build state diff --git a/coderd/wsbuilder/wsbuilder.go b/coderd/wsbuilder/wsbuilder.go index a72127f1f0..ef688a5eeb 100644 --- a/coderd/wsbuilder/wsbuilder.go +++ b/coderd/wsbuilder/wsbuilder.go @@ -452,7 +452,7 @@ func (b *Builder) buildTx(authFunc func(action policy.Action, object rbac.Object // to read all provisioner daemons. We need to retrieve the eligible // provisioner daemons for this job to show in the UI if there is no // matching provisioner daemon. - provisionerDaemons, err := b.store.GetEligibleProvisionerDaemonsByProvisionerJobIDs(dbauthz.AsSystemReadProvisionerDaemons(b.ctx), []uuid.UUID{provisionerJob.ID}) + provisionerDaemons, err := b.store.GetEligibleProvisionerDaemonsByProvisionerJobIDs(dbauthz.AsWorkspaceBuilder(b.ctx), []uuid.UUID{provisionerJob.ID}) if err != nil { // NOTE: we do **not** want to fail a workspace build if we fail to // retrieve provisioner daemons. This is just to show in the UI if there @@ -570,8 +570,8 @@ func (b *Builder) buildTx(authFunc func(action policy.Action, object rbac.Object } } if b.state.orphan && !hasActiveEligibleProvisioner { - // nolint: gocritic // At this moment, we are pretending to be provisionerd. - if err := store.UpdateProvisionerJobWithCompleteWithStartedAtByID(dbauthz.AsProvisionerd(b.ctx), database.UpdateProvisionerJobWithCompleteWithStartedAtByIDParams{ + // nolint: gocritic // User won't necessarily have the permission to do this so we act as a system user. + if err := store.UpdateProvisionerJobWithCompleteWithStartedAtByID(dbauthz.AsWorkspaceBuilder(b.ctx), database.UpdateProvisionerJobWithCompleteWithStartedAtByIDParams{ CompletedAt: sql.NullTime{Valid: true, Time: now}, Error: sql.NullString{Valid: false}, ErrorCode: sql.NullString{Valid: false}, @@ -815,7 +815,12 @@ func (b *Builder) getState() ([]byte, error) { if err != nil { return nil, xerrors.Errorf("get last build to get state: %w", err) } - return bld.ProvisionerState, nil + // nolint: gocritic // Workspace builder needs to read provisioner state for the new build. + state, err := b.store.GetWorkspaceBuildProvisionerStateByID(dbauthz.AsWorkspaceBuilder(b.ctx), bld.ID) + if err != nil { + return nil, xerrors.Errorf("get workspace build provisioner state: %w", err) + } + return state.ProvisionerState, nil } func (b *Builder) getParameters() (names, values []string, err error) { diff --git a/coderd/wsbuilder/wsbuilder_test.go b/coderd/wsbuilder/wsbuilder_test.go index 38f88f7508..1e90a3d4ea 100644 --- a/coderd/wsbuilder/wsbuilder_test.go +++ b/coderd/wsbuilder/wsbuilder_test.go @@ -65,6 +65,7 @@ func TestBuilder_NoOptions(t *testing.T) { withTemplate, withInactiveVersion(nil), withLastBuildFound, + withLastBuildState, withTemplateVersionVariables(inactiveVersionID, nil), withRichParameters(nil), withParameterSchemas(inactiveJobID, nil), @@ -124,6 +125,7 @@ func TestBuilder_Initiator(t *testing.T) { withTemplate, withInactiveVersion(nil), withLastBuildFound, + withLastBuildState, withTemplateVersionVariables(inactiveVersionID, nil), withRichParameters(nil), withParameterSchemas(inactiveJobID, nil), @@ -174,6 +176,7 @@ func TestBuilder_Baggage(t *testing.T) { withTemplate, withInactiveVersion(nil), withLastBuildFound, + withLastBuildState, withTemplateVersionVariables(inactiveVersionID, nil), withRichParameters(nil), withParameterSchemas(inactiveJobID, nil), @@ -216,6 +219,7 @@ func TestBuilder_Reason(t *testing.T) { withTemplate, withInactiveVersion(nil), withLastBuildFound, + withLastBuildState, withTemplateVersionVariables(inactiveVersionID, nil), withRichParameters(nil), withParameterSchemas(inactiveJobID, nil), @@ -365,6 +369,7 @@ func TestWorkspaceBuildWithTags(t *testing.T) { withTemplate, withInactiveVersion(richParameters), withLastBuildFound, + withLastBuildState, withTemplateVersionVariables(inactiveVersionID, templateVersionVariables), withRichParameters(nil), withParameterSchemas(inactiveJobID, nil), @@ -464,6 +469,7 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) { withTemplate, withInactiveVersion(richParameters), withLastBuildFound, + withLastBuildState, withTemplateVersionVariables(inactiveVersionID, nil), withRichParameters(initialBuildParameters), withParameterSchemas(inactiveJobID, nil), @@ -515,6 +521,7 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) { withTemplate, withInactiveVersion(richParameters), withLastBuildFound, + withLastBuildState, withTemplateVersionVariables(inactiveVersionID, nil), withRichParameters(initialBuildParameters), withParameterSchemas(inactiveJobID, nil), @@ -661,6 +668,7 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) { withTemplate, withActiveVersion(version2params), withLastBuildFound, + withLastBuildState, withTemplateVersionVariables(activeVersionID, nil), withRichParameters(initialBuildParameters), withParameterSchemas(activeJobID, nil), @@ -727,6 +735,7 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) { withTemplate, withActiveVersion(version2params), withLastBuildFound, + withLastBuildState, withTemplateVersionVariables(activeVersionID, nil), withRichParameters(initialBuildParameters), withParameterSchemas(activeJobID, nil), @@ -791,6 +800,7 @@ func TestWorkspaceBuildWithRichParameters(t *testing.T) { withTemplate, withActiveVersion(version2params), withLastBuildFound, + withLastBuildState, withTemplateVersionVariables(activeVersionID, nil), withRichParameters(initialBuildParameters), withParameterSchemas(activeJobID, nil), @@ -1062,6 +1072,7 @@ func TestWorkspaceBuildUsageChecker(t *testing.T) { withTemplate, withInactiveVersion(nil), withLastBuildFound, + withLastBuildState, withTemplateVersionVariables(inactiveVersionID, nil), withRichParameters(nil), withParameterSchemas(inactiveJobID, nil), @@ -1175,6 +1186,7 @@ func TestWorkspaceBuildWithTask(t *testing.T) { withTemplate, withInactiveVersion(nil), withLastBuildFound, + withLastBuildState, withTemplateVersionVariables(inactiveVersionID, nil), withRichParameters(nil), withParameterSchemas(inactiveJobID, nil), @@ -1378,7 +1390,6 @@ func withLastBuildFound(mTx *dbmock.MockStore) { Transition: database.WorkspaceTransitionStart, InitiatorID: userID, JobID: lastBuildJobID, - ProvisionerState: []byte("last build state"), Reason: database.BuildReasonInitiator, }, nil) @@ -1398,6 +1409,14 @@ func withLastBuildFound(mTx *dbmock.MockStore) { }, nil) } +func withLastBuildState(mTx *dbmock.MockStore) { + mTx.EXPECT().GetWorkspaceBuildProvisionerStateByID(gomock.Any(), lastBuildID). + Times(1). + Return(database.GetWorkspaceBuildProvisionerStateByIDRow{ + ProvisionerState: []byte("last build state"), + }, nil) +} + func withLastBuildNotFound(mTx *dbmock.MockStore) { mTx.EXPECT().GetLatestWorkspaceBuildByWorkspaceID(gomock.Any(), workspaceID). Times(1). diff --git a/docs/admin/security/audit-logs.md b/docs/admin/security/audit-logs.md index 7e6ec3cae1..9d630765a8 100644 --- a/docs/admin/security/audit-logs.md +++ b/docs/admin/security/audit-logs.md @@ -36,7 +36,7 @@ We track the following resources: | Template
write, delete | |
FieldTracked
active_version_idtrue
activity_bumptrue
allow_user_autostarttrue
allow_user_autostoptrue
allow_user_cancel_workspace_jobstrue
autostart_block_days_of_weektrue
autostop_requirement_days_of_weektrue
autostop_requirement_weekstrue
cors_behaviortrue
created_atfalse
created_bytrue
created_by_avatar_urlfalse
created_by_namefalse
created_by_usernamefalse
default_ttltrue
deletedfalse
deprecatedtrue
descriptiontrue
disable_module_cachetrue
display_nametrue
failure_ttltrue
group_acltrue
icontrue
idtrue
max_port_sharing_leveltrue
nametrue
organization_display_namefalse
organization_iconfalse
organization_idfalse
organization_namefalse
provisionertrue
require_active_versiontrue
time_til_dormanttrue
time_til_dormant_autodeletetrue
updated_atfalse
use_classic_parameter_flowtrue
user_acltrue
| | TemplateVersion
create, write | |
FieldTracked
archivedtrue
created_atfalse
created_bytrue
created_by_avatar_urlfalse
created_by_namefalse
created_by_usernamefalse
external_auth_providersfalse
has_ai_taskfalse
has_external_agentfalse
idtrue
job_idfalse
messagefalse
nametrue
organization_idfalse
readmetrue
source_example_idfalse
template_idtrue
updated_atfalse
| | User
create, write, delete | |
FieldTracked
avatar_urlfalse
created_atfalse
deletedtrue
emailtrue
github_com_user_idfalse
hashed_one_time_passcodefalse
hashed_passwordtrue
idtrue
is_systemtrue
last_seen_atfalse
login_typetrue
nametrue
one_time_passcode_expires_attrue
quiet_hours_scheduletrue
rbac_rolestrue
statustrue
updated_atfalse
usernametrue
| -| WorkspaceBuild
start, stop | |
FieldTracked
build_numberfalse
created_atfalse
daily_costfalse
deadlinefalse
has_ai_taskfalse
has_external_agentfalse
idfalse
initiator_by_avatar_urlfalse
initiator_by_namefalse
initiator_by_usernamefalse
initiator_idfalse
job_idfalse
max_deadlinefalse
provisioner_statefalse
reasonfalse
template_version_idtrue
template_version_preset_idfalse
transitionfalse
updated_atfalse
workspace_idfalse
| +| WorkspaceBuild
start, stop | |
FieldTracked
build_numberfalse
created_atfalse
daily_costfalse
deadlinefalse
has_ai_taskfalse
has_external_agentfalse
idfalse
initiator_by_avatar_urlfalse
initiator_by_namefalse
initiator_by_usernamefalse
initiator_idfalse
job_idfalse
max_deadlinefalse
reasonfalse
template_version_idtrue
template_version_preset_idfalse
transitionfalse
updated_atfalse
workspace_idfalse
| | WorkspaceProxy
| |
FieldTracked
created_attrue
deletedfalse
derp_enabledtrue
derp_onlytrue
display_nametrue
icontrue
idtrue
nametrue
region_idtrue
token_hashed_secrettrue
updated_atfalse
urltrue
versiontrue
wildcard_hostnametrue
| | WorkspaceTable
| |
FieldTracked
automatic_updatestrue
autostart_scheduletrue
created_atfalse
deletedfalse
deleting_attrue
dormant_attrue
favoritetrue
group_acltrue
idtrue
last_used_atfalse
nametrue
next_start_attrue
organization_idfalse
owner_idtrue
template_idtrue
ttltrue
updated_atfalse
user_acltrue
| diff --git a/enterprise/audit/table.go b/enterprise/audit/table.go index cad1dcfb59..913559ed7d 100644 --- a/enterprise/audit/table.go +++ b/enterprise/audit/table.go @@ -190,7 +190,6 @@ var auditableResourcesTypes = map[any]map[string]Action{ "build_number": ActionIgnore, "transition": ActionIgnore, "initiator_id": ActionIgnore, - "provisioner_state": ActionIgnore, "job_id": ActionIgnore, "deadline": ActionIgnore, "reason": ActionIgnore, diff --git a/enterprise/coderd/schedule/template_test.go b/enterprise/coderd/schedule/template_test.go index ab4a58f8cf..e2efd7c6ac 100644 --- a/enterprise/coderd/schedule/template_test.go +++ b/enterprise/coderd/schedule/template_test.go @@ -253,13 +253,16 @@ func TestTemplateUpdateBuildDeadlines(t *testing.T) { TemplateID: template.ID, }).Seed(database.WorkspaceBuild{ TemplateVersionID: templateVersion.ID, - ProvisionerState: []byte(must(cryptorand.String(64))), - }).Succeeded(dbfake.WithJobCompletedAt(buildTime)).Do() + }).ProvisionerState([]byte(must(cryptorand.String(64)))).Succeeded(dbfake.WithJobCompletedAt(buildTime)).Do() // Assert test invariant: workspace build state must not be empty - require.NotEmpty(t, buildResp.Build.ProvisionerState, "provisioner state must not be empty") + var buildProvisionerState []byte + buildProvisionerStateRow, err := db.GetWorkspaceBuildProvisionerStateByID(ctx, buildResp.Build.ID) + require.NoError(t, err) + buildProvisionerState = buildProvisionerStateRow.ProvisionerState + require.NotEmpty(t, buildProvisionerState, "provisioner state must not be empty") - err := db.UpdateWorkspaceBuildDeadlineByID(ctx, database.UpdateWorkspaceBuildDeadlineByIDParams{ + err = db.UpdateWorkspaceBuildDeadlineByID(ctx, database.UpdateWorkspaceBuildDeadlineByIDParams{ ID: buildResp.Build.ID, UpdatedAt: buildTime, Deadline: c.deadline, @@ -310,7 +313,9 @@ func TestTemplateUpdateBuildDeadlines(t *testing.T) { require.WithinDuration(t, c.newMaxDeadline, newBuild.MaxDeadline, time.Second, "max_deadline") // Check that the new build has the same state as before. - require.Equal(t, wsBuild.ProvisionerState, newBuild.ProvisionerState, "provisioner state mismatch") + newBuildProvisionerStateRow, err := db.GetWorkspaceBuildProvisionerStateByID(ctx, newBuild.ID) + require.NoError(t, err) + require.Equal(t, buildProvisionerState, newBuildProvisionerStateRow.ProvisionerState, "provisioner state mismatch") }) } } @@ -388,7 +393,8 @@ func TestTemplateUpdateBuildDeadlinesSkip(t *testing.T) { shouldBeUpdated bool // Set below: - wsBuild database.WorkspaceBuild + wsBuild database.WorkspaceBuild + wsBuildProvisionerState []byte }{ { name: "DifferentTemplate", @@ -483,19 +489,25 @@ func TestTemplateUpdateBuildDeadlinesSkip(t *testing.T) { }, OrganizationID: templateJob.OrganizationID, }) + wsBuildProvisionerState := []byte(must(cryptorand.String(64))) wsBuild := dbgen.WorkspaceBuild(t, db, database.WorkspaceBuild{ WorkspaceID: wsID, BuildNumber: b.buildNumber, JobID: job.ID, InitiatorID: user.ID, TemplateVersionID: templateVersion.ID, - ProvisionerState: []byte(must(cryptorand.String(64))), }) + err = db.UpdateWorkspaceBuildProvisionerStateByID(ctx, database.UpdateWorkspaceBuildProvisionerStateByIDParams{ + ID: wsBuild.ID, + UpdatedAt: wsBuild.UpdatedAt, + ProvisionerState: wsBuildProvisionerState, + }) + require.NoError(t, err) // Assert test invariant: workspace build state must not be empty - require.NotEmpty(t, wsBuild.ProvisionerState, "provisioner state must not be empty") + require.NotEmpty(t, wsBuildProvisionerState, "provisioner state must not be empty") - err := db.UpdateWorkspaceBuildDeadlineByID(ctx, database.UpdateWorkspaceBuildDeadlineByIDParams{ + err = db.UpdateWorkspaceBuildDeadlineByID(ctx, database.UpdateWorkspaceBuildDeadlineByIDParams{ ID: wsBuild.ID, UpdatedAt: buildTime, Deadline: originalMaxDeadline, @@ -507,8 +519,9 @@ func TestTemplateUpdateBuildDeadlinesSkip(t *testing.T) { require.NoError(t, err) // Assert test invariant: workspace build state must not be empty - require.NotEmpty(t, wsBuild.ProvisionerState, "provisioner state must not be empty") + require.NotEmpty(t, wsBuildProvisionerState, "provisioner state must not be empty") + builds[i].wsBuildProvisionerState = wsBuildProvisionerState builds[i].wsBuild = wsBuild if !b.buildStarted { @@ -595,7 +608,9 @@ func TestTemplateUpdateBuildDeadlinesSkip(t *testing.T) { assert.WithinDuration(t, originalMaxDeadline, newBuild.MaxDeadline, time.Second, msg) } - assert.Equal(t, builds[i].wsBuild.ProvisionerState, newBuild.ProvisionerState, "provisioner state mismatch") + newBuildProvisionerStateRow, err := db.GetWorkspaceBuildProvisionerStateByID(ctx, newBuild.ID) + require.NoError(t, err) + assert.Equal(t, builds[i].wsBuildProvisionerState, newBuildProvisionerStateRow.ProvisionerState, "provisioner state mismatch") } }