diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 8066ebd0479a1..129053b3269b4 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -217,7 +217,7 @@ var ( rbac.ResourceTemplate.Type: {policy.ActionRead, policy.ActionUpdate}, // Unsure why provisionerd needs update and read personal rbac.ResourceUser.Type: {policy.ActionRead, policy.ActionReadPersonal, policy.ActionUpdatePersonal}, - rbac.ResourceWorkspaceDormant.Type: {policy.ActionDelete, policy.ActionRead, policy.ActionUpdate, policy.ActionWorkspaceStop}, + rbac.ResourceWorkspaceDormant.Type: {policy.ActionDelete, policy.ActionRead, policy.ActionUpdate, policy.ActionWorkspaceStop, policy.ActionCreateAgent}, rbac.ResourceWorkspace.Type: {policy.ActionDelete, policy.ActionRead, policy.ActionUpdate, policy.ActionWorkspaceStart, policy.ActionWorkspaceStop, policy.ActionCreateAgent}, // Provisionerd needs to read, update, and delete tasks associated with workspaces. rbac.ResourceTask.Type: {policy.ActionRead, policy.ActionUpdate, policy.ActionDelete}, diff --git a/coderd/database/migrations/000371_api_key_scopes_array_allow_list.up.sql b/coderd/database/migrations/000371_api_key_scopes_array_allow_list.up.sql index b38bf89880bed..12fb99f89f83f 100644 --- a/coderd/database/migrations/000371_api_key_scopes_array_allow_list.up.sql +++ b/coderd/database/migrations/000371_api_key_scopes_array_allow_list.up.sql @@ -141,13 +141,19 @@ ALTER TYPE api_key_scope ADD VALUE IF NOT EXISTS 'workspace_proxy:read'; ALTER TYPE api_key_scope ADD VALUE IF NOT EXISTS 'workspace_proxy:update'; -- End enum extensions +-- Purge old API keys to speed up the migration for large deployments. +-- Note: that problem should be solved in coderd once PR 20863 is released: +-- https://github.com/coder/coder/blob/main/coderd/database/dbpurge/dbpurge.go#L85 +DELETE FROM api_keys WHERE expires_at < NOW() - INTERVAL '7 days'; + -- Add new columns without defaults; backfill; then enforce NOT NULL ALTER TABLE api_keys ADD COLUMN scopes api_key_scope[]; ALTER TABLE api_keys ADD COLUMN allow_list text[]; -- Backfill existing rows for compatibility -UPDATE api_keys SET scopes = ARRAY[scope::api_key_scope]; -UPDATE api_keys SET allow_list = ARRAY['*:*']; +UPDATE api_keys SET + scopes = ARRAY[scope::api_key_scope], + allow_list = ARRAY['*:*']; -- Enforce NOT NULL ALTER TABLE api_keys ALTER COLUMN scopes SET NOT NULL; diff --git a/coderd/database/migrations/testdata/fixtures/000371_add_api_key_and_oauth2_provider_app_token.up.sql b/coderd/database/migrations/testdata/fixtures/000371_add_api_key_and_oauth2_provider_app_token.up.sql new file mode 100644 index 0000000000000..cd597539971f1 --- /dev/null +++ b/coderd/database/migrations/testdata/fixtures/000371_add_api_key_and_oauth2_provider_app_token.up.sql @@ -0,0 +1,57 @@ +-- Ensure api_keys and oauth2_provider_app_tokens have live data after +-- migration 000371 deletes expired rows. +INSERT INTO api_keys ( + id, + hashed_secret, + user_id, + last_used, + expires_at, + created_at, + updated_at, + login_type, + lifetime_seconds, + ip_address, + token_name, + scopes, + allow_list +) +VALUES ( + 'fixture-api-key', + '\xaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa', + '30095c71-380b-457a-8995-97b8ee6e5307', + NOW() - INTERVAL '1 hour', + NOW() + INTERVAL '30 days', + NOW() - INTERVAL '1 day', + NOW() - INTERVAL '1 day', + 'password', + 86400, + '0.0.0.0', + 'fixture-api-key', + ARRAY['workspace:read']::api_key_scope[], + ARRAY['*:*'] +) +ON CONFLICT (id) DO NOTHING; + +INSERT INTO oauth2_provider_app_tokens ( + id, + created_at, + expires_at, + hash_prefix, + refresh_hash, + app_secret_id, + api_key_id, + audience, + user_id +) +VALUES ( + '9f92f3c9-811f-4f6f-9a1c-3f2eed1f9f15', + NOW() - INTERVAL '30 minutes', + NOW() + INTERVAL '30 days', + CAST('fixture-hash-prefix' AS bytea), + CAST('fixture-refresh-hash' AS bytea), + 'b0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11', + 'fixture-api-key', + 'https://coder.example.com', + '30095c71-380b-457a-8995-97b8ee6e5307' +) +ON CONFLICT (id) DO NOTHING; diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index ff32a1126792d..bbcb657ea2093 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -23531,6 +23531,7 @@ SET WHERE template_id = $3 AND dormant_at IS NOT NULL + AND deleted = false -- Prebuilt workspaces (identified by having the prebuilds system user as owner_id) -- should not have their dormant or deleting at set, as these are handled by the -- prebuilds reconciliation loop. diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index d48285bb7de9c..2b6e78f71b423 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -854,6 +854,7 @@ SET WHERE template_id = @template_id AND dormant_at IS NOT NULL + AND deleted = false -- Prebuilt workspaces (identified by having the prebuilds system user as owner_id) -- should not have their dormant or deleting at set, as these are handled by the -- prebuilds reconciliation loop. diff --git a/enterprise/coderd/schedule/template_test.go b/enterprise/coderd/schedule/template_test.go index e764826f76922..e9d30cdb5df79 100644 --- a/enterprise/coderd/schedule/template_test.go +++ b/enterprise/coderd/schedule/template_test.go @@ -737,6 +737,105 @@ func TestNotifications(t *testing.T) { require.Contains(t, sent[i].Targets, dormantWs.OwnerID) } }) + + // Regression test for https://github.com/coder/coder/issues/20913 + // Deleted workspaces should not receive dormancy notifications. + t.Run("DeletedWorkspacesNotNotified", func(t *testing.T) { + t.Parallel() + + var ( + db, _ = dbtestutil.NewDB(t) + ctx = testutil.Context(t, testutil.WaitLong) + user = dbgen.User(t, db, database.User{}) + file = dbgen.File(t, db, database.File{ + CreatedBy: user.ID, + }) + templateJob = dbgen.ProvisionerJob(t, db, nil, database.ProvisionerJob{ + FileID: file.ID, + InitiatorID: user.ID, + Tags: database.StringMap{ + "foo": "bar", + }, + }) + timeTilDormant = time.Minute * 2 + templateVersion = dbgen.TemplateVersion(t, db, database.TemplateVersion{ + CreatedBy: user.ID, + JobID: templateJob.ID, + OrganizationID: templateJob.OrganizationID, + }) + template = dbgen.Template(t, db, database.Template{ + ActiveVersionID: templateVersion.ID, + CreatedBy: user.ID, + OrganizationID: templateJob.OrganizationID, + TimeTilDormant: int64(timeTilDormant), + TimeTilDormantAutoDelete: int64(timeTilDormant), + }) + ) + + // Create a dormant workspace that is NOT deleted. + activeDormantWorkspace := dbgen.Workspace(t, db, database.WorkspaceTable{ + OwnerID: user.ID, + TemplateID: template.ID, + OrganizationID: templateJob.OrganizationID, + LastUsedAt: time.Now().Add(-time.Hour), + }) + _, err := db.UpdateWorkspaceDormantDeletingAt(ctx, database.UpdateWorkspaceDormantDeletingAtParams{ + ID: activeDormantWorkspace.ID, + DormantAt: sql.NullTime{ + Time: activeDormantWorkspace.LastUsedAt.Add(timeTilDormant), + Valid: true, + }, + }) + require.NoError(t, err) + + // Create a dormant workspace that IS deleted. + deletedDormantWorkspace := dbgen.Workspace(t, db, database.WorkspaceTable{ + OwnerID: user.ID, + TemplateID: template.ID, + OrganizationID: templateJob.OrganizationID, + LastUsedAt: time.Now().Add(-time.Hour), + Deleted: true, // Mark as deleted + }) + _, err = db.UpdateWorkspaceDormantDeletingAt(ctx, database.UpdateWorkspaceDormantDeletingAtParams{ + ID: deletedDormantWorkspace.ID, + DormantAt: sql.NullTime{ + Time: deletedDormantWorkspace.LastUsedAt.Add(timeTilDormant), + Valid: true, + }, + }) + require.NoError(t, err) + + // Setup dependencies + notifyEnq := notificationstest.NewFakeEnqueuer() + logger := slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) + const userQuietHoursSchedule = "CRON_TZ=UTC 0 0 * * *" // midnight UTC + userQuietHoursStore, err := schedule.NewEnterpriseUserQuietHoursScheduleStore(userQuietHoursSchedule, true) + require.NoError(t, err) + userQuietHoursStorePtr := &atomic.Pointer[agplschedule.UserQuietHoursScheduleStore]{} + userQuietHoursStorePtr.Store(&userQuietHoursStore) + templateScheduleStore := schedule.NewEnterpriseTemplateScheduleStore(userQuietHoursStorePtr, notifyEnq, logger, nil) + + // Lower the dormancy TTL to ensure the schedule recalculates deadlines and + // triggers notifications. + _, err = templateScheduleStore.Set(dbauthz.AsNotifier(ctx), db, template, agplschedule.TemplateScheduleOptions{ + TimeTilDormant: timeTilDormant / 2, + TimeTilDormantAutoDelete: timeTilDormant / 2, + }) + require.NoError(t, err) + + // We should only receive a notification for the non-deleted dormant workspace. + sent := notifyEnq.Sent() + require.Len(t, sent, 1, "expected exactly 1 notification for the non-deleted workspace") + require.Equal(t, sent[0].UserID, activeDormantWorkspace.OwnerID) + require.Equal(t, sent[0].TemplateID, notifications.TemplateWorkspaceMarkedForDeletion) + require.Contains(t, sent[0].Targets, activeDormantWorkspace.ID) + + // Ensure the deleted workspace was NOT notified + for _, notification := range sent { + require.NotContains(t, notification.Targets, deletedDormantWorkspace.ID, + "deleted workspace should not receive notifications") + } + }) } func TestTemplateTTL(t *testing.T) { diff --git a/enterprise/coderd/workspaces_test.go b/enterprise/coderd/workspaces_test.go index 5201e613f7a1d..ce8054f9bac2a 100644 --- a/enterprise/coderd/workspaces_test.go +++ b/enterprise/coderd/workspaces_test.go @@ -833,6 +833,73 @@ func TestWorkspaceAutobuild(t *testing.T) { require.True(t, ws.LastUsedAt.After(dormantLastUsedAt)) }) + // This test has been added to ensure we don't introduce a regression + // to this issue https://github.com/coder/coder/issues/20711. + t.Run("DormantAutostop", func(t *testing.T) { + t.Parallel() + + var ( + ticker = make(chan time.Time) + statCh = make(chan autobuild.Stats) + inactiveTTL = time.Minute + logger = slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) + ) + + client, db, user := coderdenttest.NewWithDatabase(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + AutobuildTicker: ticker, + AutobuildStats: statCh, + IncludeProvisionerDaemon: true, + TemplateScheduleStore: schedule.NewEnterpriseTemplateScheduleStore(agplUserQuietHoursScheduleStore(), notifications.NewNoopEnqueuer(), logger, nil), + }, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{codersdk.FeatureAdvancedTemplateScheduling: 1}, + }, + }) + + // Create a template version that includes agents on both start AND stop builds. + // This simulates a template without `count = data.coder_workspace.me.start_count`. + authToken := uuid.NewString() + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ + Parse: echo.ParseComplete, + ProvisionPlan: echo.PlanComplete, + ProvisionApply: echo.ProvisionApplyWithAgent(authToken), + }) + + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) { + ctr.TimeTilDormantMillis = ptr.Ref[int64](inactiveTTL.Milliseconds()) + }) + + coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) + ws := coderdtest.CreateWorkspace(t, client, template.ID) + build := coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, ws.LatestBuild.ID) + require.Equal(t, codersdk.WorkspaceStatusRunning, build.Status) + + // Simulate the workspace becoming inactive and transitioning to dormant. + tickTime := ws.LastUsedAt.Add(inactiveTTL * 2) + + p, err := coderdtest.GetProvisionerForTags(db, time.Now(), ws.OrganizationID, nil) + require.NoError(t, err) + coderdtest.UpdateProvisionerLastSeenAt(t, db, p.ID, tickTime) + ticker <- tickTime + stats := <-statCh + + // Expect workspace to transition to stopped state. + require.Len(t, stats.Transitions, 1) + require.Equal(t, stats.Transitions[ws.ID], database.WorkspaceTransitionStop) + + // The autostop build should succeed even though the template includes + // agents without `count = data.coder_workspace.me.start_count`. + // This verifies that provisionerd has permission to create agents on + // dormant workspaces during stop builds. + ws = coderdtest.MustWorkspace(t, client, ws.ID) + require.NotNil(t, ws.DormantAt, "workspace should be marked as dormant") + require.Equal(t, codersdk.WorkspaceTransitionStop, ws.LatestBuild.Transition) + + latestBuild := coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, ws.LatestBuild.ID) + require.Equal(t, codersdk.WorkspaceStatusStopped, latestBuild.Status) + }) + // This test serves as a regression prevention for generating // audit logs in the same transaction the transition workspaces to // the dormant state. The auditor that is passed to autobuild does