From 9cf3c582cbe1a83cea2dab553f9b9aed3c74e7d4 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Tue, 13 Jun 2023 10:42:18 -0500 Subject: [PATCH] test: add unit test that deletes abandoned workspace (#7990) * test: add unit test that deletes abandoned workspace This is to ensure we do not break this functionality in future. This is important incase this edge case happens, an admin can clean up the abandoned resources. --- cli/delete_test.go | 49 ++++++++++++++++++++++++++++++++ coderd/database/dbfake/dbfake.go | 10 ++++++- 2 files changed, 58 insertions(+), 1 deletion(-) diff --git a/cli/delete_test.go b/cli/delete_test.go index c79d07b075..40f5b9ac22 100644 --- a/cli/delete_test.go +++ b/cli/delete_test.go @@ -2,6 +2,7 @@ package cli_test import ( "context" + "fmt" "io" "testing" @@ -10,8 +11,11 @@ import ( "github.com/coder/coder/cli/clitest" "github.com/coder/coder/coderd/coderdtest" + "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/database/dbauthz" "github.com/coder/coder/codersdk" "github.com/coder/coder/pty/ptytest" + "github.com/coder/coder/testutil" ) func TestDelete(t *testing.T) { @@ -68,6 +72,51 @@ func TestDelete(t *testing.T) { <-doneChan }) + // Super orphaned, as the workspace doesn't even have a user. + // This is not a scenario we should ever get into, as we do not allow users + // to be deleted if they have workspaces. However issue #7872 shows that + // it is possible to get into this state. An admin should be able to still + // force a delete action on the workspace. + t.Run("OrphanDeletedUser", func(t *testing.T) { + t.Parallel() + client, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + user := coderdtest.CreateFirstUser(t, client) + deleteMeClient, deleteMeUser := coderdtest.CreateAnotherUser(t, client, user.OrganizationID) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + + workspace := coderdtest.CreateWorkspace(t, deleteMeClient, user.OrganizationID, template.ID) + coderdtest.AwaitWorkspaceBuildJob(t, deleteMeClient, workspace.LatestBuild.ID) + + // The API checks if the user has any workspaces, so we cannot delete a user + // this way. + ctx := testutil.Context(t, testutil.WaitShort) + // nolint:gocritic // Unit test + err := api.Database.UpdateUserDeletedByID(dbauthz.AsSystemRestricted(ctx), database.UpdateUserDeletedByIDParams{ + ID: deleteMeUser.ID, + Deleted: true, + }) + require.NoError(t, err) + + inv, root := clitest.New(t, "delete", fmt.Sprintf("%s/%s", deleteMeUser.ID, workspace.Name), "-y", "--orphan") + + clitest.SetupConfig(t, client, root) + doneChan := make(chan struct{}) + pty := ptytest.New(t).Attach(inv) + inv.Stderr = pty.Output() + go func() { + defer close(doneChan) + err := inv.Run() + // When running with the race detector on, we sometimes get an EOF. + if err != nil { + assert.ErrorIs(t, err, io.EOF) + } + }() + pty.ExpectMatch("workspace has been deleted") + <-doneChan + }) + t.Run("DifferentUser", func(t *testing.T) { t.Parallel() adminClient := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index e3a3aecebb..dad08f081a 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -4646,11 +4646,19 @@ func (q *fakeQuerier) UpdateUserDeletedByID(_ context.Context, params database.U u.Deleted = params.Deleted q.users[i] = u // NOTE: In the real world, this is done by a trigger. - for i, k := range q.apiKeys { + i := 0 + for { + if i >= len(q.apiKeys) { + break + } + k := q.apiKeys[i] if k.UserID == u.ID { q.apiKeys[i] = q.apiKeys[len(q.apiKeys)-1] q.apiKeys = q.apiKeys[:len(q.apiKeys)-1] + // We removed an element, so decrement + i-- } + i++ } return nil }