mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix(coderd): allow template deletion when only prebuild workspaces remain (#23417)
## 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.
This commit is contained in:
+11
-5
@@ -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,
|
||||
|
||||
@@ -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})
|
||||
|
||||
Reference in New Issue
Block a user