From 98143e1b7077fe517f67a87831bac8ea8085add0 Mon Sep 17 00:00:00 2001 From: Sas Swart Date: Wed, 25 Mar 2026 09:43:06 +0200 Subject: [PATCH] fix(coderd): allow template deletion when only prebuild workspaces remain (#23417) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem Template administrators cannot delete templates that have running prebuilds. The `deleteTemplate` handler fetches all non-deleted workspaces and blocks deletion if any exist, making no distinction between human-owned workspaces and prebuild workspaces (owned by the system `PrebuildsSystemUserID`). This forces admins into a manual multi-step workflow: set `desired_instances` to 0 on every preset, wait for the reconciler to drain prebuilds, then retry deletion. Prebuilds are an internal system concern that admins should not need to manage manually. ## Fix Replace the blanket `len(workspaces) > 0` guard in `deleteTemplate` with a loop that only blocks deletion when a non-prebuild (human-owned) workspace exists. Prebuild workspaces — owned by `database.PrebuildsSystemUserID` — are now ignored during the check. Once the template is soft-deleted (`deleted=true`), the existing prebuilds reconciler detects `isActive()=false` and cleans up remaining prebuilds asynchronously. No changes to the reconciler are needed. The error message and HTTP status for human workspaces remain unchanged. ## Testing Added two new subtests to `TestDeleteTemplate`: - **`OnlyPrebuilds`**: deletion succeeds when only prebuild workspaces exist. - **`PrebuildsAndHumanWorkspaces`**: deletion is blocked when both prebuild and human workspaces exist. Existing reconciler test ("soft-deleted templates MAY have prebuilds") already covers post-deletion prebuild cleanup. --- coderd/templates.go | 16 +++++++---- coderd/templates_test.go | 61 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 5 deletions(-) diff --git a/coderd/templates.go b/coderd/templates.go index 2bcaf2099f..191615b635 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -90,11 +90,17 @@ func (api *API) deleteTemplate(rw http.ResponseWriter, r *http.Request) { }) return } - if len(workspaces) > 0 { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "All workspaces must be deleted before a template can be removed.", - }) - return + // Allow deletion when only prebuild workspaces remain. Prebuilds + // are owned by the system user and will be cleaned up + // asynchronously by the prebuilds reconciler once the template's + // deleted flag is set. + for _, ws := range workspaces { + if ws.OwnerID != database.PrebuildsSystemUserID { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "All workspaces must be deleted before a template can be removed.", + }) + return + } } err = api.Database.UpdateTemplateDeletedByID(ctx, database.UpdateTemplateDeletedByIDParams{ ID: template.ID, diff --git a/coderd/templates_test.go b/coderd/templates_test.go index d53ecf80d2..08e198f79e 100644 --- a/coderd/templates_test.go +++ b/coderd/templates_test.go @@ -1802,6 +1802,67 @@ func TestDeleteTemplate(t *testing.T) { require.Equal(t, http.StatusForbidden, apiErr.StatusCode()) }) + t.Run("OnlyPrebuilds", func(t *testing.T) { + t.Parallel() + client, db := coderdtest.NewWithDatabase(t, nil) + owner := coderdtest.CreateFirstUser(t, client) + tpl := dbfake.TemplateVersion(t, db). + Seed(database.TemplateVersion{ + CreatedBy: owner.UserID, + OrganizationID: owner.OrganizationID, + }).Do() + + // Create a workspace owned by the prebuilds system user. + dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OwnerID: database.PrebuildsSystemUserID, + OrganizationID: owner.OrganizationID, + TemplateID: tpl.Template.ID, + }).Seed(database.WorkspaceBuild{ + TemplateVersionID: tpl.TemplateVersion.ID, + }).Do() + + ctx := testutil.Context(t, testutil.WaitLong) + + err := client.DeleteTemplate(ctx, tpl.Template.ID) + require.NoError(t, err) + }) + + t.Run("PrebuildsAndHumanWorkspaces", func(t *testing.T) { + t.Parallel() + client, db := coderdtest.NewWithDatabase(t, nil) + owner := coderdtest.CreateFirstUser(t, client) + tpl := dbfake.TemplateVersion(t, db). + Seed(database.TemplateVersion{ + CreatedBy: owner.UserID, + OrganizationID: owner.OrganizationID, + }).Do() + + // Create a prebuild workspace. + dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OwnerID: database.PrebuildsSystemUserID, + OrganizationID: owner.OrganizationID, + TemplateID: tpl.Template.ID, + }).Seed(database.WorkspaceBuild{ + TemplateVersionID: tpl.TemplateVersion.ID, + }).Do() + + // Create a human-owned workspace. + dbfake.WorkspaceBuild(t, db, database.WorkspaceTable{ + OwnerID: owner.UserID, + OrganizationID: owner.OrganizationID, + TemplateID: tpl.Template.ID, + }).Seed(database.WorkspaceBuild{ + TemplateVersionID: tpl.TemplateVersion.ID, + }).Do() + + ctx := testutil.Context(t, testutil.WaitLong) + + err := client.DeleteTemplate(ctx, tpl.Template.ID) + var apiErr *codersdk.Error + require.ErrorAs(t, err, &apiErr) + require.Equal(t, http.StatusBadRequest, apiErr.StatusCode()) + }) + t.Run("DeletedIsSet", func(t *testing.T) { t.Parallel() client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})