mirror of
https://github.com/coder/coder.git
synced 2026-09-21 12:44:32 +08:00
fix(enterprise/coderd): allow deleting external-agent workspaces (#26501)
I stumbled on this while manually testing another workspace change: if a license expires after an external-agent workspace exists, deleting that workspace is rejected because `CheckBuildUsage` enforces the external-agent entitlement for every workspace transition. I assume this is unintentional from a product POV. This PR narrows the external-agent entitlement check to start builds. Creating or rebuilding external-agent workspaces still requires the feature entitlement, but stop and delete transitions stay available as cleanup paths. That matches the managed-agent entitlement precedent (i.e. the usage billing check) in the same `CheckBuildUsage` path, where license enforcement is scoped to start transitions instead of trapping users with resources they can no longer remove.
This commit is contained in:
@@ -1113,9 +1113,9 @@ func (api *API) CheckBuildUsage(
|
||||
task *database.Task,
|
||||
transition database.WorkspaceTransition,
|
||||
) (wsbuilder.UsageCheckResponse, error) {
|
||||
// If the template version has an external agent, we need to check that the
|
||||
// license is entitled to this feature.
|
||||
if templateVersion.HasExternalAgent.Valid && templateVersion.HasExternalAgent.Bool {
|
||||
// External-agent templates require an entitlement for start builds.
|
||||
if transition == database.WorkspaceTransitionStart &&
|
||||
templateVersion.HasExternalAgent.Valid && templateVersion.HasExternalAgent.Bool {
|
||||
feature, ok := api.Entitlements.Feature(codersdk.FeatureWorkspaceExternalAgent)
|
||||
if !ok || !feature.Enabled {
|
||||
return wsbuilder.UsageCheckResponse{
|
||||
|
||||
@@ -942,37 +942,68 @@ func TestCheckBuildUsage_NeverBlocksOnManagedAgentLimit(t *testing.T) {
|
||||
require.True(t, deleteResp.Permitted)
|
||||
}
|
||||
|
||||
func TestCheckBuildUsage_BlocksWithoutManagedAgentEntitlement(t *testing.T) {
|
||||
func TestCheckBuildUsage_BlocksStartWithoutEntitlement(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tv := &database.TemplateVersion{
|
||||
managedAgentVersion := &database.TemplateVersion{
|
||||
HasAITask: sql.NullBool{Valid: true, Bool: true},
|
||||
HasExternalAgent: sql.NullBool{Valid: true, Bool: false},
|
||||
}
|
||||
task := &database.Task{
|
||||
TemplateVersionID: tv.ID,
|
||||
externalAgentVersion := &database.TemplateVersion{
|
||||
HasExternalAgent: sql.NullBool{Valid: true, Bool: true},
|
||||
}
|
||||
|
||||
// Both "feature absent" and "feature explicitly disabled" should
|
||||
// block AI task builds on licensed deployments.
|
||||
tests := []struct {
|
||||
name string
|
||||
setupEnts func(e *codersdk.Entitlements)
|
||||
name string
|
||||
templateVersion *database.TemplateVersion
|
||||
task *database.Task
|
||||
setupEnts func(e *codersdk.Entitlements)
|
||||
message string
|
||||
checkNoTaskStart bool
|
||||
}{
|
||||
{
|
||||
name: "FeatureAbsent",
|
||||
name: "ManagedAgentFeatureAbsent",
|
||||
templateVersion: managedAgentVersion,
|
||||
task: &database.Task{TemplateVersionID: managedAgentVersion.ID},
|
||||
setupEnts: func(e *codersdk.Entitlements) {
|
||||
e.HasLicense = true
|
||||
delete(e.Features, codersdk.FeatureManagedAgentLimit)
|
||||
},
|
||||
message: "not entitled to managed agents",
|
||||
checkNoTaskStart: true,
|
||||
},
|
||||
{
|
||||
name: "FeatureDisabled",
|
||||
name: "ManagedAgentFeatureDisabled",
|
||||
templateVersion: managedAgentVersion,
|
||||
task: &database.Task{TemplateVersionID: managedAgentVersion.ID},
|
||||
setupEnts: func(e *codersdk.Entitlements) {
|
||||
e.HasLicense = true
|
||||
e.Features[codersdk.FeatureManagedAgentLimit] = codersdk.Feature{
|
||||
Enabled: false,
|
||||
}
|
||||
},
|
||||
message: "not entitled to managed agents",
|
||||
checkNoTaskStart: true,
|
||||
},
|
||||
{
|
||||
name: "ExternalAgentFeatureAbsent",
|
||||
templateVersion: externalAgentVersion,
|
||||
setupEnts: func(e *codersdk.Entitlements) {
|
||||
e.HasLicense = true
|
||||
delete(e.Features, codersdk.FeatureWorkspaceExternalAgent)
|
||||
},
|
||||
message: "uses external agents",
|
||||
},
|
||||
{
|
||||
name: "ExternalAgentFeatureDisabled",
|
||||
templateVersion: externalAgentVersion,
|
||||
setupEnts: func(e *codersdk.Entitlements) {
|
||||
e.HasLicense = true
|
||||
e.Features[codersdk.FeatureWorkspaceExternalAgent] = codersdk.Feature{
|
||||
Enabled: false,
|
||||
}
|
||||
},
|
||||
message: "uses external agents",
|
||||
},
|
||||
}
|
||||
|
||||
@@ -998,28 +1029,24 @@ func TestCheckBuildUsage_BlocksWithoutManagedAgentEntitlement(t *testing.T) {
|
||||
mDB := dbmock.NewMockStore(ctrl)
|
||||
ctx := context.Background()
|
||||
|
||||
// Start transition with a task: should be blocked because the
|
||||
// license doesn't include the managed agent entitlement.
|
||||
resp, err := eapi.CheckBuildUsage(ctx, mDB, tv, task, database.WorkspaceTransitionStart)
|
||||
resp, err := eapi.CheckBuildUsage(ctx, mDB, tc.templateVersion, tc.task, database.WorkspaceTransitionStart)
|
||||
require.NoError(t, err)
|
||||
require.False(t, resp.Permitted)
|
||||
require.Contains(t, resp.Message, "not entitled to managed agents")
|
||||
require.Contains(t, resp.Message, tc.message)
|
||||
|
||||
// Stop and delete transitions should still be permitted so
|
||||
// that existing workspaces can be stopped/cleaned up.
|
||||
stopResp, err := eapi.CheckBuildUsage(ctx, mDB, tv, task, database.WorkspaceTransitionStop)
|
||||
stopResp, err := eapi.CheckBuildUsage(ctx, mDB, tc.templateVersion, tc.task, database.WorkspaceTransitionStop)
|
||||
require.NoError(t, err)
|
||||
require.True(t, stopResp.Permitted)
|
||||
|
||||
deleteResp, err := eapi.CheckBuildUsage(ctx, mDB, tv, task, database.WorkspaceTransitionDelete)
|
||||
deleteResp, err := eapi.CheckBuildUsage(ctx, mDB, tc.templateVersion, tc.task, database.WorkspaceTransitionDelete)
|
||||
require.NoError(t, err)
|
||||
require.True(t, deleteResp.Permitted)
|
||||
|
||||
// Start transition without a task: should be permitted (not
|
||||
// an AI task build, so the entitlement check doesn't apply).
|
||||
noTaskResp, err := eapi.CheckBuildUsage(ctx, mDB, tv, nil, database.WorkspaceTransitionStart)
|
||||
require.NoError(t, err)
|
||||
require.True(t, noTaskResp.Permitted)
|
||||
if tc.checkNoTaskStart {
|
||||
noTaskResp, err := eapi.CheckBuildUsage(ctx, mDB, tc.templateVersion, nil, database.WorkspaceTransitionStart)
|
||||
require.NoError(t, err)
|
||||
require.True(t, noTaskResp.Permitted)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user