From 4dcbf5c98e315fdf67a4a2b391275d296c9c8dcd Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 17 Nov 2022 10:50:31 +0100 Subject: [PATCH 01/17] Add database column allow_user_cancel_workspace_jobs --- coderd/database/dump.sql | 5 +- ..._allow_user_cancel_workspace_jobs.down.sql | 1 + ...te_allow_user_cancel_workspace_jobs.up.sql | 4 + coderd/database/models.go | 2 + coderd/database/queries.sql.go | 73 +++++++++++-------- coderd/database/queries/templates.sql | 8 +- 6 files changed, 59 insertions(+), 34 deletions(-) create mode 100644 coderd/database/migrations/000081_template_allow_user_cancel_workspace_jobs.down.sql create mode 100644 coderd/database/migrations/000081_template_allow_user_cancel_workspace_jobs.up.sql diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 13ad911f3949d..d351cd1b2637b 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -358,13 +358,16 @@ CREATE TABLE templates ( icon character varying(256) DEFAULT ''::character varying NOT NULL, user_acl jsonb DEFAULT '{}'::jsonb NOT NULL, group_acl jsonb DEFAULT '{}'::jsonb NOT NULL, - display_name character varying(64) DEFAULT ''::character varying NOT NULL + display_name character varying(64) DEFAULT ''::character varying NOT NULL, + allow_user_cancel_workspace_jobs boolean DEFAULT true NOT NULL ); COMMENT ON COLUMN templates.default_ttl IS 'The default duration for auto-stop for workspaces created from this template.'; COMMENT ON COLUMN templates.display_name IS 'Display name is a custom, human-friendly template name that user can set.'; +COMMENT ON COLUMN templates.allow_user_cancel_workspace_jobs IS 'Allow users to cancel in-progress workspace jobs.'; + CREATE TABLE user_links ( user_id uuid NOT NULL, login_type login_type NOT NULL, diff --git a/coderd/database/migrations/000081_template_allow_user_cancel_workspace_jobs.down.sql b/coderd/database/migrations/000081_template_allow_user_cancel_workspace_jobs.down.sql new file mode 100644 index 0000000000000..7754b54767b84 --- /dev/null +++ b/coderd/database/migrations/000081_template_allow_user_cancel_workspace_jobs.down.sql @@ -0,0 +1 @@ +ALTER TABLE templates DROP COLUMN allow_user_cancel_workspace_jobs; diff --git a/coderd/database/migrations/000081_template_allow_user_cancel_workspace_jobs.up.sql b/coderd/database/migrations/000081_template_allow_user_cancel_workspace_jobs.up.sql new file mode 100644 index 0000000000000..7ea9a7dd0734b --- /dev/null +++ b/coderd/database/migrations/000081_template_allow_user_cancel_workspace_jobs.up.sql @@ -0,0 +1,4 @@ +ALTER TABLE templates ADD COLUMN allow_user_cancel_workspace_jobs boolean NOT NULL DEFAULT true; + +COMMENT ON COLUMN templates.allow_user_cancel_workspace_jobs +IS 'Allow users to cancel in-progress workspace jobs.'; diff --git a/coderd/database/models.go b/coderd/database/models.go index 209049daab428..dc5b802668129 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -596,6 +596,8 @@ type Template struct { GroupACL TemplateACL `db:"group_acl" json:"group_acl"` // Display name is a custom, human-friendly template name that user can set. DisplayName string `db:"display_name" json:"display_name"` + // Allow users to cancel in-progress workspace jobs. + AllowUserCancelWorkspaceJobs bool `db:"allow_user_cancel_workspace_jobs" json:"allow_user_cancel_workspace_jobs"` } type TemplateVersion struct { diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 207fe622bb423..e05154ad64173 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3116,7 +3116,7 @@ func (q *sqlQuerier) GetTemplateAverageBuildTime(ctx context.Context, arg GetTem const getTemplateByID = `-- name: GetTemplateByID :one SELECT - id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, default_ttl, created_by, icon, user_acl, group_acl, display_name + id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, default_ttl, created_by, icon, user_acl, group_acl, display_name, allow_user_cancel_workspace_jobs FROM templates WHERE @@ -3144,13 +3144,14 @@ func (q *sqlQuerier) GetTemplateByID(ctx context.Context, id uuid.UUID) (Templat &i.UserACL, &i.GroupACL, &i.DisplayName, + &i.AllowUserCancelWorkspaceJobs, ) return i, err } const getTemplateByOrganizationAndName = `-- name: GetTemplateByOrganizationAndName :one SELECT - id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, default_ttl, created_by, icon, user_acl, group_acl, display_name + id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, default_ttl, created_by, icon, user_acl, group_acl, display_name, allow_user_cancel_workspace_jobs FROM templates WHERE @@ -3186,12 +3187,13 @@ func (q *sqlQuerier) GetTemplateByOrganizationAndName(ctx context.Context, arg G &i.UserACL, &i.GroupACL, &i.DisplayName, + &i.AllowUserCancelWorkspaceJobs, ) return i, err } const getTemplates = `-- name: GetTemplates :many -SELECT id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, default_ttl, created_by, icon, user_acl, group_acl, display_name FROM templates +SELECT id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, default_ttl, created_by, icon, user_acl, group_acl, display_name, allow_user_cancel_workspace_jobs FROM templates ORDER BY (name, id) ASC ` @@ -3220,6 +3222,7 @@ func (q *sqlQuerier) GetTemplates(ctx context.Context) ([]Template, error) { &i.UserACL, &i.GroupACL, &i.DisplayName, + &i.AllowUserCancelWorkspaceJobs, ); err != nil { return nil, err } @@ -3236,7 +3239,7 @@ func (q *sqlQuerier) GetTemplates(ctx context.Context) ([]Template, error) { const getTemplatesWithFilter = `-- name: GetTemplatesWithFilter :many SELECT - id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, default_ttl, created_by, icon, user_acl, group_acl, display_name + id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, default_ttl, created_by, icon, user_acl, group_acl, display_name, allow_user_cancel_workspace_jobs FROM templates WHERE @@ -3300,6 +3303,7 @@ func (q *sqlQuerier) GetTemplatesWithFilter(ctx context.Context, arg GetTemplate &i.UserACL, &i.GroupACL, &i.DisplayName, + &i.AllowUserCancelWorkspaceJobs, ); err != nil { return nil, err } @@ -3330,27 +3334,29 @@ INSERT INTO icon, user_acl, group_acl, - display_name + display_name, + allow_user_cancel_workspace_jobs ) VALUES - ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14) RETURNING id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, default_ttl, created_by, icon, user_acl, group_acl, display_name + ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15) RETURNING id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, default_ttl, created_by, icon, user_acl, group_acl, display_name, allow_user_cancel_workspace_jobs ` type InsertTemplateParams struct { - ID uuid.UUID `db:"id" json:"id"` - CreatedAt time.Time `db:"created_at" json:"created_at"` - UpdatedAt time.Time `db:"updated_at" json:"updated_at"` - OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` - Name string `db:"name" json:"name"` - Provisioner ProvisionerType `db:"provisioner" json:"provisioner"` - ActiveVersionID uuid.UUID `db:"active_version_id" json:"active_version_id"` - Description string `db:"description" json:"description"` - DefaultTTL int64 `db:"default_ttl" json:"default_ttl"` - CreatedBy uuid.UUID `db:"created_by" json:"created_by"` - Icon string `db:"icon" json:"icon"` - UserACL TemplateACL `db:"user_acl" json:"user_acl"` - GroupACL TemplateACL `db:"group_acl" json:"group_acl"` - DisplayName string `db:"display_name" json:"display_name"` + ID uuid.UUID `db:"id" json:"id"` + CreatedAt time.Time `db:"created_at" json:"created_at"` + UpdatedAt time.Time `db:"updated_at" json:"updated_at"` + OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` + Name string `db:"name" json:"name"` + Provisioner ProvisionerType `db:"provisioner" json:"provisioner"` + ActiveVersionID uuid.UUID `db:"active_version_id" json:"active_version_id"` + Description string `db:"description" json:"description"` + DefaultTTL int64 `db:"default_ttl" json:"default_ttl"` + CreatedBy uuid.UUID `db:"created_by" json:"created_by"` + Icon string `db:"icon" json:"icon"` + UserACL TemplateACL `db:"user_acl" json:"user_acl"` + GroupACL TemplateACL `db:"group_acl" json:"group_acl"` + DisplayName string `db:"display_name" json:"display_name"` + AllowUserCancelWorkspaceJobs bool `db:"allow_user_cancel_workspace_jobs" json:"allow_user_cancel_workspace_jobs"` } func (q *sqlQuerier) InsertTemplate(ctx context.Context, arg InsertTemplateParams) (Template, error) { @@ -3369,6 +3375,7 @@ func (q *sqlQuerier) InsertTemplate(ctx context.Context, arg InsertTemplateParam arg.UserACL, arg.GroupACL, arg.DisplayName, + arg.AllowUserCancelWorkspaceJobs, ) var i Template err := row.Scan( @@ -3387,6 +3394,7 @@ func (q *sqlQuerier) InsertTemplate(ctx context.Context, arg InsertTemplateParam &i.UserACL, &i.GroupACL, &i.DisplayName, + &i.AllowUserCancelWorkspaceJobs, ) return i, err } @@ -3400,7 +3408,7 @@ SET WHERE id = $3 RETURNING - id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, default_ttl, created_by, icon, user_acl, group_acl, display_name + id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, default_ttl, created_by, icon, user_acl, group_acl, display_name, allow_user_cancel_workspace_jobs ` type UpdateTemplateACLByIDParams struct { @@ -3428,6 +3436,7 @@ func (q *sqlQuerier) UpdateTemplateACLByID(ctx context.Context, arg UpdateTempla &i.UserACL, &i.GroupACL, &i.DisplayName, + &i.AllowUserCancelWorkspaceJobs, ) return i, err } @@ -3483,21 +3492,23 @@ SET default_ttl = $4, name = $5, icon = $6, - display_name = $7 + display_name = $7, + allow_user_cancel_workspace_jobs = $8 WHERE id = $1 RETURNING - id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, default_ttl, created_by, icon, user_acl, group_acl, display_name + id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, default_ttl, created_by, icon, user_acl, group_acl, display_name, allow_user_cancel_workspace_jobs ` type UpdateTemplateMetaByIDParams struct { - ID uuid.UUID `db:"id" json:"id"` - UpdatedAt time.Time `db:"updated_at" json:"updated_at"` - Description string `db:"description" json:"description"` - DefaultTTL int64 `db:"default_ttl" json:"default_ttl"` - Name string `db:"name" json:"name"` - Icon string `db:"icon" json:"icon"` - DisplayName string `db:"display_name" json:"display_name"` + ID uuid.UUID `db:"id" json:"id"` + UpdatedAt time.Time `db:"updated_at" json:"updated_at"` + Description string `db:"description" json:"description"` + DefaultTTL int64 `db:"default_ttl" json:"default_ttl"` + Name string `db:"name" json:"name"` + Icon string `db:"icon" json:"icon"` + DisplayName string `db:"display_name" json:"display_name"` + AllowUserCancelWorkspaceJobs bool `db:"allow_user_cancel_workspace_jobs" json:"allow_user_cancel_workspace_jobs"` } func (q *sqlQuerier) UpdateTemplateMetaByID(ctx context.Context, arg UpdateTemplateMetaByIDParams) (Template, error) { @@ -3509,6 +3520,7 @@ func (q *sqlQuerier) UpdateTemplateMetaByID(ctx context.Context, arg UpdateTempl arg.Name, arg.Icon, arg.DisplayName, + arg.AllowUserCancelWorkspaceJobs, ) var i Template err := row.Scan( @@ -3527,6 +3539,7 @@ func (q *sqlQuerier) UpdateTemplateMetaByID(ctx context.Context, arg UpdateTempl &i.UserACL, &i.GroupACL, &i.DisplayName, + &i.AllowUserCancelWorkspaceJobs, ) return i, err } diff --git a/coderd/database/queries/templates.sql b/coderd/database/queries/templates.sql index 0757f823d6e40..2ef10667a9ba8 100644 --- a/coderd/database/queries/templates.sql +++ b/coderd/database/queries/templates.sql @@ -70,10 +70,11 @@ INSERT INTO icon, user_acl, group_acl, - display_name + display_name, + allow_user_cancel_workspace_jobs ) VALUES - ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14) RETURNING *; + ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15) RETURNING *; -- name: UpdateTemplateActiveVersionByID :exec UPDATE @@ -102,7 +103,8 @@ SET default_ttl = $4, name = $5, icon = $6, - display_name = $7 + display_name = $7, + allow_user_cancel_workspace_jobs = $8 WHERE id = $1 RETURNING From 400413ebb9d5d061403bd15bde5804dcf91380fe Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 17 Nov 2022 11:20:24 +0100 Subject: [PATCH 02/17] Adjust API --- coderd/templates.go | 60 +++++++++++++++++++++++---------------- codersdk/organizations.go | 3 ++ codersdk/templates.go | 13 +++++---- 3 files changed, 46 insertions(+), 30 deletions(-) diff --git a/coderd/templates.go b/coderd/templates.go index 7f27fb7c2f882..80dcac854cec8 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -220,6 +220,11 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque return } + var allowUserCancelWorkspaceJobs bool + if createTemplate.AllowUserCancelWorkspaceJobs != nil { + allowUserCancelWorkspaceJobs = *createTemplate.AllowUserCancelWorkspaceJobs + } + var dbTemplate database.Template var template codersdk.Template err = api.Database.InTx(func(tx database.Store) error { @@ -239,8 +244,9 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque GroupACL: database.TemplateACL{ organization.ID.String(): []rbac.Action{rbac.ActionRead}, }, - DisplayName: createTemplate.DisplayName, - Icon: createTemplate.Icon, + DisplayName: createTemplate.DisplayName, + Icon: createTemplate.Icon, + AllowUserCancelWorkspaceJobs: allowUserCancelWorkspaceJobs, }) if err != nil { return xerrors.Errorf("insert template: %s", err) @@ -476,6 +482,7 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) { req.Description == template.Description && req.DisplayName == template.DisplayName && req.Icon == template.Icon && + req.AllowUserCancelWorkspaceJobs == template.AllowUserCancelWorkspaceJobs && req.DefaultTTLMillis == time.Duration(template.DefaultTTL).Milliseconds() { return nil } @@ -488,6 +495,7 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) { desc := req.Description icon := req.Icon maxTTL := time.Duration(req.DefaultTTLMillis) * time.Millisecond + allowUserCancelWorkspaceJobs := req.AllowUserCancelWorkspaceJobs if name == "" { name = template.Name @@ -497,13 +505,14 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) { } updated, err = tx.UpdateTemplateMetaByID(ctx, database.UpdateTemplateMetaByIDParams{ - ID: template.ID, - UpdatedAt: database.Now(), - Name: name, - DisplayName: displayName, - Description: desc, - Icon: icon, - DefaultTTL: int64(maxTTL), + ID: template.ID, + UpdatedAt: database.Now(), + Name: name, + DisplayName: displayName, + Description: desc, + Icon: icon, + DefaultTTL: int64(maxTTL), + AllowUserCancelWorkspaceJobs: allowUserCancelWorkspaceJobs, }) if err != nil { return err @@ -740,21 +749,22 @@ func (api *API) convertTemplate( buildTimeStats := api.metricsCache.TemplateBuildTimeStats(template.ID) return codersdk.Template{ - ID: template.ID, - CreatedAt: template.CreatedAt, - UpdatedAt: template.UpdatedAt, - OrganizationID: template.OrganizationID, - Name: template.Name, - DisplayName: template.DisplayName, - Provisioner: codersdk.ProvisionerType(template.Provisioner), - ActiveVersionID: template.ActiveVersionID, - WorkspaceOwnerCount: workspaceOwnerCount, - ActiveUserCount: activeCount, - BuildTimeStats: buildTimeStats, - Description: template.Description, - Icon: template.Icon, - DefaultTTLMillis: time.Duration(template.DefaultTTL).Milliseconds(), - CreatedByID: template.CreatedBy, - CreatedByName: createdByName, + ID: template.ID, + CreatedAt: template.CreatedAt, + UpdatedAt: template.UpdatedAt, + OrganizationID: template.OrganizationID, + Name: template.Name, + DisplayName: template.DisplayName, + Provisioner: codersdk.ProvisionerType(template.Provisioner), + ActiveVersionID: template.ActiveVersionID, + WorkspaceOwnerCount: workspaceOwnerCount, + ActiveUserCount: activeCount, + BuildTimeStats: buildTimeStats, + Description: template.Description, + Icon: template.Icon, + DefaultTTLMillis: time.Duration(template.DefaultTTL).Milliseconds(), + CreatedByID: template.CreatedBy, + CreatedByName: createdByName, + AllowUserCancelWorkspaceJobs: template.AllowUserCancelWorkspaceJobs, } } diff --git a/codersdk/organizations.go b/codersdk/organizations.go index 21ede686d96c6..9461f6fdd2b4d 100644 --- a/codersdk/organizations.go +++ b/codersdk/organizations.go @@ -72,6 +72,9 @@ type CreateTemplateRequest struct { // DefaultTTLMillis allows optionally specifying the default TTL // for all workspaces created from this template. DefaultTTLMillis *int64 `json:"default_ttl_ms,omitempty"` + + // Allow users to cancel in-progress workspace jobs. + AllowUserCancelWorkspaceJobs *bool `json:"allow_user_cancel_workspace_jobs"` } // CreateWorkspaceRequest provides options for creating a new workspace. diff --git a/codersdk/templates.go b/codersdk/templates.go index 786984a1cedfc..42b8ae87c6800 100644 --- a/codersdk/templates.go +++ b/codersdk/templates.go @@ -31,6 +31,8 @@ type Template struct { DefaultTTLMillis int64 `json:"default_ttl_ms"` CreatedByID uuid.UUID `json:"created_by_id"` CreatedByName string `json:"created_by_name"` + + AllowUserCancelWorkspaceJobs bool `json:"allow_user_cancel_workspace_jobs"` } type TemplateBuildTimeStats struct { @@ -72,11 +74,12 @@ type UpdateTemplateACL struct { } type UpdateTemplateMeta struct { - Name string `json:"name,omitempty" validate:"omitempty,template_name"` - DisplayName string `json:"display_name,omitempty" validate:"omitempty,template_display_name"` - Description string `json:"description,omitempty"` - Icon string `json:"icon,omitempty"` - DefaultTTLMillis int64 `json:"default_ttl_ms,omitempty"` + Name string `json:"name,omitempty" validate:"omitempty,template_name"` + DisplayName string `json:"display_name,omitempty" validate:"omitempty,template_display_name"` + Description string `json:"description,omitempty"` + Icon string `json:"icon,omitempty"` + DefaultTTLMillis int64 `json:"default_ttl_ms,omitempty"` + AllowUserCancelWorkspaceJobs bool `json:"allow_user_cancel_workspace_jobs"` } // Template returns a single template. From 7f8dcaad571d313467fcec1a25925ed3e8a4b116 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 17 Nov 2022 11:33:42 +0100 Subject: [PATCH 03/17] site: typesGenerated.ts --- site/src/api/typesGenerated.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 75c85198b0626..4d03581996933 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -182,6 +182,7 @@ export interface CreateTemplateRequest { readonly template_version_id: string readonly parameter_values?: CreateParameterRequest[] readonly default_ttl_ms?: number + readonly allow_user_cancel_workspace_jobs?: boolean } // From codersdk/templateversions.go @@ -642,6 +643,7 @@ export interface Template { readonly default_ttl_ms: number readonly created_by_id: string readonly created_by_name: string + readonly allow_user_cancel_workspace_jobs: boolean } // From codersdk/templates.go @@ -720,6 +722,7 @@ export interface UpdateTemplateMeta { readonly description?: string readonly icon?: string readonly default_ttl_ms?: number + readonly allow_user_cancel_workspace_jobs: boolean } // From codersdk/users.go From 3f76534f9d7336fd095cc97a8d1924cd94b16439 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 17 Nov 2022 11:44:41 +0100 Subject: [PATCH 04/17] Expose template.allow_ in Workspaces API --- coderd/workspaces.go | 31 ++++++++++++++++--------------- codersdk/organizations.go | 1 + codersdk/workspaces.go | 31 ++++++++++++++++--------------- site/src/api/typesGenerated.ts | 1 + 4 files changed, 34 insertions(+), 30 deletions(-) diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 3f26dd6367c53..67fdd76981445 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -1010,21 +1010,22 @@ func convertWorkspace( ttlMillis := convertWorkspaceTTLMillis(workspace.Ttl) return codersdk.Workspace{ - ID: workspace.ID, - CreatedAt: workspace.CreatedAt, - UpdatedAt: workspace.UpdatedAt, - OwnerID: workspace.OwnerID, - OwnerName: owner.Username, - TemplateID: workspace.TemplateID, - LatestBuild: workspaceBuild, - TemplateName: template.Name, - TemplateIcon: template.Icon, - TemplateDisplayName: template.DisplayName, - Outdated: workspaceBuild.TemplateVersionID.String() != template.ActiveVersionID.String(), - Name: workspace.Name, - AutostartSchedule: autostartSchedule, - TTLMillis: ttlMillis, - LastUsedAt: workspace.LastUsedAt, + ID: workspace.ID, + CreatedAt: workspace.CreatedAt, + UpdatedAt: workspace.UpdatedAt, + OwnerID: workspace.OwnerID, + OwnerName: owner.Username, + TemplateID: workspace.TemplateID, + LatestBuild: workspaceBuild, + TemplateName: template.Name, + TemplateIcon: template.Icon, + TemplateDisplayName: template.DisplayName, + TemplateAllowUserCancelWorkspaceJobs: template.AllowUserCancelWorkspaceJobs, + Outdated: workspaceBuild.TemplateVersionID.String() != template.ActiveVersionID.String(), + Name: workspace.Name, + AutostartSchedule: autostartSchedule, + TTLMillis: ttlMillis, + LastUsedAt: workspace.LastUsedAt, } } diff --git a/codersdk/organizations.go b/codersdk/organizations.go index 9461f6fdd2b4d..c61161abd7dcf 100644 --- a/codersdk/organizations.go +++ b/codersdk/organizations.go @@ -74,6 +74,7 @@ type CreateTemplateRequest struct { DefaultTTLMillis *int64 `json:"default_ttl_ms,omitempty"` // Allow users to cancel in-progress workspace jobs. + // *bool as the default value is "true". AllowUserCancelWorkspaceJobs *bool `json:"allow_user_cancel_workspace_jobs"` } diff --git a/codersdk/workspaces.go b/codersdk/workspaces.go index 24cfc6b96e072..d03e4d7c8ac9e 100644 --- a/codersdk/workspaces.go +++ b/codersdk/workspaces.go @@ -17,21 +17,22 @@ import ( // Workspace is a deployment of a template. It references a specific // version and can be updated. type Workspace struct { - ID uuid.UUID `json:"id"` - CreatedAt time.Time `json:"created_at"` - UpdatedAt time.Time `json:"updated_at"` - OwnerID uuid.UUID `json:"owner_id"` - OwnerName string `json:"owner_name"` - TemplateID uuid.UUID `json:"template_id"` - TemplateName string `json:"template_name"` - TemplateDisplayName string `json:"template_display_name"` - TemplateIcon string `json:"template_icon"` - LatestBuild WorkspaceBuild `json:"latest_build"` - Outdated bool `json:"outdated"` - Name string `json:"name"` - AutostartSchedule *string `json:"autostart_schedule,omitempty"` - TTLMillis *int64 `json:"ttl_ms,omitempty"` - LastUsedAt time.Time `json:"last_used_at"` + ID uuid.UUID `json:"id"` + CreatedAt time.Time `json:"created_at"` + UpdatedAt time.Time `json:"updated_at"` + OwnerID uuid.UUID `json:"owner_id"` + OwnerName string `json:"owner_name"` + TemplateID uuid.UUID `json:"template_id"` + TemplateName string `json:"template_name"` + TemplateDisplayName string `json:"template_display_name"` + TemplateIcon string `json:"template_icon"` + TemplateAllowUserCancelWorkspaceJobs bool `json:"template_allow_user_cancel_workspace_jobs"` + LatestBuild WorkspaceBuild `json:"latest_build"` + Outdated bool `json:"outdated"` + Name string `json:"name"` + AutostartSchedule *string `json:"autostart_schedule,omitempty"` + TTLMillis *int64 `json:"ttl_ms,omitempty"` + LastUsedAt time.Time `json:"last_used_at"` } type WorkspacesRequest struct { diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 4d03581996933..0e1c86ca90da1 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -797,6 +797,7 @@ export interface Workspace { readonly template_name: string readonly template_display_name: string readonly template_icon: string + readonly template_allow_user_cancel_workspace_jobs: boolean readonly latest_build: WorkspaceBuild readonly outdated: boolean readonly name: string From 64b8bf377f20f28247202e145a9772e1323b2248 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 17 Nov 2022 11:53:37 +0100 Subject: [PATCH 05/17] Fix: site tests --- site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx | 1 + .../pages/TemplateSettingsPage/TemplateSettingsPage.test.tsx | 1 + site/src/testHelpers/entities.ts | 2 ++ 3 files changed, 4 insertions(+) diff --git a/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx b/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx index 14e0aa07594c1..3ab5523aa53e7 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx @@ -86,6 +86,7 @@ export const TemplateSettingsForm: FC = ({ // on display, convert from ms => hours default_ttl_ms: template.default_ttl_ms / MS_HOUR_CONVERSION, icon: template.icon, + allow_user_cancel_workspace_jobs: true, }, validationSchema, onSubmit: (formData) => { diff --git a/site/src/pages/TemplateSettingsPage/TemplateSettingsPage.test.tsx b/site/src/pages/TemplateSettingsPage/TemplateSettingsPage.test.tsx index ea4effe0fbdbb..375f9ae8929b1 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateSettingsPage.test.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateSettingsPage.test.tsx @@ -28,6 +28,7 @@ const validFormValues = { description: "A description", icon: "A string", default_ttl_ms: 1, + allow_user_cancel_workspace_jobs: true, } const fillAndSubmitForm = async ({ diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 7d1e5213518b7..da8fe6bc732c4 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -202,6 +202,7 @@ export const MockTemplate: TypesGen.Template = { created_by_id: "test-creator-id", created_by_name: "test_creator", icon: "/icon/code.svg", + allow_user_cancel_workspace_jobs: true, } export const MockWorkspaceApp: TypesGen.WorkspaceApp = { @@ -437,6 +438,7 @@ export const MockWorkspace: TypesGen.Workspace = { template_name: MockTemplate.name, template_icon: MockTemplate.icon, template_display_name: MockTemplate.display_name, + template_allow_user_cancel_workspace_jobs: MockTemplate.allow_user_cancel_workspace_jobs, outdated: false, owner_id: MockUser.id, owner_name: MockUser.username, From 73431c9d63d1df385be92b5f67434ff04b7411e0 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 17 Nov 2022 11:54:24 +0100 Subject: [PATCH 06/17] Fix: make fmt/prettier --- site/src/testHelpers/entities.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index da8fe6bc732c4..9d9d672f78fa5 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -438,7 +438,8 @@ export const MockWorkspace: TypesGen.Workspace = { template_name: MockTemplate.name, template_icon: MockTemplate.icon, template_display_name: MockTemplate.display_name, - template_allow_user_cancel_workspace_jobs: MockTemplate.allow_user_cancel_workspace_jobs, + template_allow_user_cancel_workspace_jobs: + MockTemplate.allow_user_cancel_workspace_jobs, outdated: false, owner_id: MockUser.id, owner_name: MockUser.username, From ba5c06bf7677b5e0da6affafbf6e5bc25f0a3ee3 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 17 Nov 2022 12:23:19 +0100 Subject: [PATCH 07/17] Fix: enterprise --- enterprise/audit/table.go | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/enterprise/audit/table.go b/enterprise/audit/table.go index 68f51813627c1..e8399b38433d2 100644 --- a/enterprise/audit/table.go +++ b/enterprise/audit/table.go @@ -48,23 +48,24 @@ var AuditableResources = auditMap(map[any]map[string]Action{ "updated_at": ActionIgnore, // Changes, but is implicit and not helpful in a diff. }, &database.Template{}: { - "id": ActionTrack, - "created_at": ActionIgnore, // Never changes, but is implicit and not helpful in a diff. - "updated_at": ActionIgnore, // Changes, but is implicit and not helpful in a diff. - "organization_id": ActionIgnore, /// Never changes. - "deleted": ActionIgnore, // Changes, but is implicit when a delete event is fired. - "name": ActionTrack, - "display_name": ActionTrack, - "provisioner": ActionTrack, - "active_version_id": ActionTrack, - "description": ActionTrack, - "icon": ActionTrack, - "default_ttl": ActionTrack, - "min_autostart_interval": ActionTrack, - "created_by": ActionTrack, - "is_private": ActionTrack, - "group_acl": ActionTrack, - "user_acl": ActionTrack, + "id": ActionTrack, + "created_at": ActionIgnore, // Never changes, but is implicit and not helpful in a diff. + "updated_at": ActionIgnore, // Changes, but is implicit and not helpful in a diff. + "organization_id": ActionIgnore, /// Never changes. + "deleted": ActionIgnore, // Changes, but is implicit when a delete event is fired. + "name": ActionTrack, + "display_name": ActionTrack, + "provisioner": ActionTrack, + "active_version_id": ActionTrack, + "description": ActionTrack, + "icon": ActionTrack, + "default_ttl": ActionTrack, + "min_autostart_interval": ActionTrack, + "created_by": ActionTrack, + "is_private": ActionTrack, + "group_acl": ActionTrack, + "user_acl": ActionTrack, + "allow_user_cancel_workspace_jobs": ActionTrack, }, &database.TemplateVersion{}: { "id": ActionTrack, From 9be6b1f1059ab7402506fb8c6eb076cd4d1b3db7 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 17 Nov 2022 12:46:59 +0100 Subject: [PATCH 08/17] Database tests --- coderd/templates_test.go | 13 ++++++++----- coderd/workspaces_test.go | 4 ++++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/coderd/templates_test.go b/coderd/templates_test.go index 94bc5383bf02a..70059bdfc1153 100644 --- a/coderd/templates_test.go +++ b/coderd/templates_test.go @@ -285,11 +285,12 @@ func TestPatchTemplateMeta(t *testing.T) { version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) req := codersdk.UpdateTemplateMeta{ - Name: "new-template-name", - DisplayName: "Displayed Name 456", - Description: "lorem ipsum dolor sit amet et cetera", - Icon: "/icons/new-icon.png", - DefaultTTLMillis: 12 * time.Hour.Milliseconds(), + Name: "new-template-name", + DisplayName: "Displayed Name 456", + Description: "lorem ipsum dolor sit amet et cetera", + Icon: "/icons/new-icon.png", + DefaultTTLMillis: 12 * time.Hour.Milliseconds(), + AllowUserCancelWorkspaceJobs: false, } // It is unfortunate we need to sleep, but the test can fail if the // updatedAt is too close together. @@ -306,6 +307,7 @@ func TestPatchTemplateMeta(t *testing.T) { assert.Equal(t, req.Description, updated.Description) assert.Equal(t, req.Icon, updated.Icon) assert.Equal(t, req.DefaultTTLMillis, updated.DefaultTTLMillis) + assert.False(t, req.AllowUserCancelWorkspaceJobs) // Extra paranoid: did it _really_ happen? updated, err = client.Template(ctx, template.ID) @@ -316,6 +318,7 @@ func TestPatchTemplateMeta(t *testing.T) { assert.Equal(t, req.Description, updated.Description) assert.Equal(t, req.Icon, updated.Icon) assert.Equal(t, req.DefaultTTLMillis, updated.DefaultTTLMillis) + assert.False(t, req.AllowUserCancelWorkspaceJobs) require.Len(t, auditor.AuditLogs, 4) assert.Equal(t, database.AuditActionWrite, auditor.AuditLogs[3].Action) diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index 71cc655b7e8ec..1711de1ba3413 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -125,13 +125,16 @@ func TestWorkspace(t *testing.T) { const templateIcon = "/img/icon.svg" const templateDisplayName = "This is template" + var templateAllowUserCancelWorkspaceJobs = false template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID, func(ctr *codersdk.CreateTemplateRequest) { ctr.Icon = templateIcon ctr.DisplayName = templateDisplayName + ctr.AllowUserCancelWorkspaceJobs = &templateAllowUserCancelWorkspaceJobs }) require.NotEmpty(t, template.Name) require.NotEmpty(t, template.DisplayName) require.NotEmpty(t, template.Icon) + require.False(t, template.AllowUserCancelWorkspaceJobs) workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) @@ -144,6 +147,7 @@ func TestWorkspace(t *testing.T) { assert.Equal(t, template.Name, ws.TemplateName) assert.Equal(t, templateIcon, ws.TemplateIcon) assert.Equal(t, templateDisplayName, ws.TemplateDisplayName) + assert.Equal(t, templateAllowUserCancelWorkspaceJobs, ws.TemplateAllowUserCancelWorkspaceJobs) }) } From 8579883730268a7dc66bb39ecaea03fff83864f1 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 17 Nov 2022 17:03:46 +0100 Subject: [PATCH 09/17] Add CLI tests --- cli/templateedit.go | 23 +++++++++++++---------- cli/templateedit_test.go | 7 +++++++ 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/cli/templateedit.go b/cli/templateedit.go index 50307ec73c819..5a99634511520 100644 --- a/cli/templateedit.go +++ b/cli/templateedit.go @@ -13,11 +13,12 @@ import ( func templateEdit() *cobra.Command { var ( - name string - displayName string - description string - icon string - defaultTTL time.Duration + name string + displayName string + description string + icon string + defaultTTL time.Duration + allowUserCancelWorkspaceJobs bool ) cmd := &cobra.Command{ @@ -40,11 +41,12 @@ func templateEdit() *cobra.Command { // NOTE: coderd will ignore empty fields. req := codersdk.UpdateTemplateMeta{ - Name: name, - DisplayName: displayName, - Description: description, - Icon: icon, - DefaultTTLMillis: defaultTTL.Milliseconds(), + Name: name, + DisplayName: displayName, + Description: description, + Icon: icon, + DefaultTTLMillis: defaultTTL.Milliseconds(), + AllowUserCancelWorkspaceJobs: allowUserCancelWorkspaceJobs, } _, err = client.UpdateTemplateMeta(cmd.Context(), template.ID, req) @@ -61,6 +63,7 @@ func templateEdit() *cobra.Command { cmd.Flags().StringVarP(&description, "description", "", "", "Edit the template description") cmd.Flags().StringVarP(&icon, "icon", "", "", "Edit the template icon path") cmd.Flags().DurationVarP(&defaultTTL, "default-ttl", "", 0, "Edit the template default time before shutdown - workspaces created from this template to this value.") + cmd.Flags().BoolVarP(&allowUserCancelWorkspaceJobs, "allow-user-cancel-workspace-jobs", "", true, "Allow users to cancel in-progress workspace jobs.") cliui.AllowSkipPrompt(cmd) return cmd diff --git a/cli/templateedit_test.go b/cli/templateedit_test.go index 558839de2c9d6..28fe0edabf8c5 100644 --- a/cli/templateedit_test.go +++ b/cli/templateedit_test.go @@ -2,6 +2,7 @@ package cli_test import ( "context" + "strconv" "testing" "time" @@ -31,6 +32,8 @@ func TestTemplateEdit(t *testing.T) { desc := "lorem ipsum dolor sit amet et cetera" icon := "/icons/new-icon.png" defaultTTL := 12 * time.Hour + allowUserCancelWorkspaceJobs := false + cmdArgs := []string{ "templates", "edit", @@ -40,6 +43,7 @@ func TestTemplateEdit(t *testing.T) { "--description", desc, "--icon", icon, "--default-ttl", defaultTTL.String(), + "--allow-user-cancel-workspace-jobs=" + strconv.FormatBool(allowUserCancelWorkspaceJobs), } cmd, root := clitest.New(t, cmdArgs...) clitest.SetupConfig(t, client, root) @@ -57,6 +61,7 @@ func TestTemplateEdit(t *testing.T) { assert.Equal(t, desc, updated.Description) assert.Equal(t, icon, updated.Icon) assert.Equal(t, defaultTTL.Milliseconds(), updated.DefaultTTLMillis) + assert.Equal(t, allowUserCancelWorkspaceJobs, updated.AllowUserCancelWorkspaceJobs) }) t.Run("FirstEmptyThenNotModified", func(t *testing.T) { t.Parallel() @@ -75,6 +80,7 @@ func TestTemplateEdit(t *testing.T) { "--description", template.Description, "--icon", template.Icon, "--default-ttl", (time.Duration(template.DefaultTTLMillis) * time.Millisecond).String(), + "--allow-user-cancel-workspace-jobs=" + strconv.FormatBool(template.AllowUserCancelWorkspaceJobs), } cmd, root := clitest.New(t, cmdArgs...) clitest.SetupConfig(t, client, root) @@ -91,6 +97,7 @@ func TestTemplateEdit(t *testing.T) { assert.Equal(t, template.Description, updated.Description) assert.Equal(t, template.Icon, updated.Icon) assert.Equal(t, template.DefaultTTLMillis, updated.DefaultTTLMillis) + assert.Equal(t, template.AllowUserCancelWorkspaceJobs, updated.AllowUserCancelWorkspaceJobs) }) t.Run("InvalidDisplayName", func(t *testing.T) { t.Parallel() From 3fe5beff799814ab51a211a543fa44f04df4228f Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 17 Nov 2022 19:51:18 +0100 Subject: [PATCH 10/17] Add checkbox --- .../TemplateSettingsForm.tsx | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx b/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx index 3ab5523aa53e7..9224325a6bcf6 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx @@ -1,3 +1,6 @@ +import Box from "@material-ui/core/Box" +import Checkbox from "@material-ui/core/Checkbox" +import Typography from "@material-ui/core/Typography" import data from "@emoji-mart/data/sets/14/twitter.json" import Picker from "@emoji-mart/react" import Button from "@material-ui/core/Button" @@ -86,7 +89,8 @@ export const TemplateSettingsForm: FC = ({ // on display, convert from ms => hours default_ttl_ms: template.default_ttl_ms / MS_HOUR_CONVERSION, icon: template.icon, - allow_user_cancel_workspace_jobs: true, + allow_user_cancel_workspace_jobs: + template.allow_user_cancel_workspace_jobs, }, validationSchema, onSubmit: (formData) => { @@ -213,6 +217,28 @@ export const TemplateSettingsForm: FC = ({ {form.values.default_ttl_ms && !form.errors.default_ttl_ms && ( {Language.ttlHelperText(form.values.default_ttl_ms)} )} + + +
+ +
+ + + Allow users to cancel in-progress workspace jobs. + + + It is advised to keep the option disabled when canceling a + workspace job may leave the workspace in an unhealthy state, and + extra permissions are required to manually repair its resources. + + + From 9a303b5561cd56a21942565760bdf4786f4eb38e Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 17 Nov 2022 19:56:49 +0100 Subject: [PATCH 11/17] i18n --- site/src/i18n/en/templatePage.json | 4 +++- .../src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx | 6 ++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/site/src/i18n/en/templatePage.json b/site/src/i18n/en/templatePage.json index 1a86e7a815d90..5d1c10e13428f 100644 --- a/site/src/i18n/en/templatePage.json +++ b/site/src/i18n/en/templatePage.json @@ -10,5 +10,7 @@ "deleteCta": "Delete Template" } }, - "displayNameLabel": "Display name" + "displayNameLabel": "Display name", + "allowUserCancelWorkspaceJobsLabel": "Allow users to cancel in-progress workspace jobs.", + "allowUserCancelWorkspaceJobsNotice": "It is advised to keep the option disabled when canceling a workspace job may leave the workspace in an unhealthy state, and extra permissions are required to manually repair its resources." } diff --git a/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx b/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx index 9224325a6bcf6..f1851cbeb0687 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx @@ -230,12 +230,10 @@ export const TemplateSettingsForm: FC = ({ - Allow users to cancel in-progress workspace jobs. + {t("allowUserCancelWorkspaceJobsLabel")} - It is advised to keep the option disabled when canceling a - workspace job may leave the workspace in an unhealthy state, and - extra permissions are required to manually repair its resources. + {t("allowUserCancelWorkspaceJobsNotice")} From ba443ebdb22856c1fb17345fa11c4783515b87be Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Fri, 18 Nov 2022 13:42:25 +0100 Subject: [PATCH 12/17] Logic: block cancelling --- coderd/workspacebuilds.go | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/coderd/workspacebuilds.go b/coderd/workspacebuilds.go index 0d823d0a8d82b..d33810d406d3d 100644 --- a/coderd/workspacebuilds.go +++ b/coderd/workspacebuilds.go @@ -25,6 +25,11 @@ import ( "github.com/coder/coder/codersdk" ) +var ( + errTemplateNotExists = xerrors.New("No template exists for this workspace") + errUserNotExists = xerrors.New("User does not exist") +) + func (api *API) workspaceBuild(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() workspaceBuild := httpmw.WorkspaceBuildParam(r) @@ -599,6 +604,21 @@ func (api *API) patchCancelWorkspaceBuild(rw http.ResponseWriter, r *http.Reques return } + valid, err := api.verifyUserCanCancelWorkspaceBuilds(ctx, httpmw.APIKey(r).UserID, workspace.TemplateID) + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error verifying permission to cancel workspace build.", + Detail: err.Error(), + }) + return + } + if !valid { + httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{ + Message: "User is not allowed cancel workspace builds. Owner role is required.", + }) + return + } + job, err := api.Database.GetProvisionerJobByID(ctx, workspaceBuild.JobID) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ @@ -646,6 +666,23 @@ func (api *API) patchCancelWorkspaceBuild(rw http.ResponseWriter, r *http.Reques }) } +func (api *API) verifyUserCanCancelWorkspaceBuilds(ctx context.Context, userID uuid.UUID, templateID uuid.UUID) (bool, error) { + template, err := api.Database.GetTemplateByID(ctx, templateID) + if err != nil { + return false, errTemplateNotExists + } + + if template.AllowUserCancelWorkspaceJobs { + return true, nil // all users can cancel workspace builds + } + + user, err := api.Database.GetUserByID(ctx, userID) + if err != nil { + return false, errUserNotExists + } + return slices.Contains(user.RBACRoles, rbac.RoleOwner()), nil // only user with "owner" role can cancel workspace builds +} + func (api *API) workspaceBuildResources(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() workspaceBuild := httpmw.WorkspaceBuildParam(r) From 7d36b2d1b2832c12c0259b60445c1a5f1b003287 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Fri, 18 Nov 2022 13:56:54 +0100 Subject: [PATCH 13/17] Unit tests for conditional cancel --- coderd/workspacebuilds_test.go | 102 ++++++++++++++++++++++----------- 1 file changed, 70 insertions(+), 32 deletions(-) diff --git a/coderd/workspacebuilds_test.go b/coderd/workspacebuilds_test.go index 6d3721dccc1fd..43fbe51f0f5a9 100644 --- a/coderd/workspacebuilds_test.go +++ b/coderd/workspacebuilds_test.go @@ -367,41 +367,79 @@ func TestWorkspaceBuildsProvisionerState(t *testing.T) { func TestPatchCancelWorkspaceBuild(t *testing.T) { t.Parallel() - client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) - user := coderdtest.CreateFirstUser(t, client) - version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ - Parse: echo.ParseComplete, - ProvisionApply: []*proto.Provision_Response{{ - Type: &proto.Provision_Response_Log{ - Log: &proto.Log{}, - }, - }}, - ProvisionPlan: echo.ProvisionComplete, + t.Run("User is allowed to cancel", func(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + user := coderdtest.CreateFirstUser(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ + Parse: echo.ParseComplete, + ProvisionApply: []*proto.Provision_Response{{ + Type: &proto.Provision_Response_Log{ + Log: &proto.Log{}, + }, + }}, + ProvisionPlan: echo.ProvisionComplete, + }) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + var build codersdk.WorkspaceBuild + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + require.Eventually(t, func() bool { + var err error + build, err = client.WorkspaceBuild(ctx, workspace.LatestBuild.ID) + return assert.NoError(t, err) && build.Job.Status == codersdk.ProvisionerJobRunning + }, testutil.WaitShort, testutil.IntervalFast) + err := client.CancelWorkspaceBuild(ctx, build.ID) + require.NoError(t, err) + require.Eventually(t, func() bool { + var err error + build, err = client.WorkspaceBuild(ctx, build.ID) + return assert.NoError(t, err) && + // The job will never actually cancel successfully because it will never send a + // provision complete response. + assert.Empty(t, build.Job.Error) && + build.Job.Status == codersdk.ProvisionerJobCanceling + }, testutil.WaitShort, testutil.IntervalFast) }) - coderdtest.AwaitTemplateVersionJob(t, client, version.ID) - template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) - workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) - var build codersdk.WorkspaceBuild + t.Run("User is not allowed to cancel", func(t *testing.T) { + t.Parallel() - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + owner := coderdtest.CreateFirstUser(t, client) + version := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, &echo.Responses{ + Parse: echo.ParseComplete, + ProvisionApply: []*proto.Provision_Response{{ + Type: &proto.Provision_Response_Log{ + Log: &proto.Log{}, + }, + }}, + ProvisionPlan: echo.ProvisionComplete, + }) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version.ID) - require.Eventually(t, func() bool { - var err error - build, err = client.WorkspaceBuild(ctx, workspace.LatestBuild.ID) - return assert.NoError(t, err) && build.Job.Status == codersdk.ProvisionerJobRunning - }, testutil.WaitShort, testutil.IntervalFast) - err := client.CancelWorkspaceBuild(ctx, build.ID) - require.NoError(t, err) - require.Eventually(t, func() bool { - var err error - build, err = client.WorkspaceBuild(ctx, build.ID) - return assert.NoError(t, err) && - // The job will never actually cancel successfully because it will never send a - // provision complete response. - assert.Empty(t, build.Job.Error) && - build.Job.Status == codersdk.ProvisionerJobCanceling - }, testutil.WaitShort, testutil.IntervalFast) + userClient := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID) + workspace := coderdtest.CreateWorkspace(t, userClient, owner.OrganizationID, template.ID) + var build codersdk.WorkspaceBuild + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + require.Eventually(t, func() bool { + var err error + build, err = userClient.WorkspaceBuild(ctx, workspace.LatestBuild.ID) + return assert.NoError(t, err) && build.Job.Status == codersdk.ProvisionerJobRunning + }, testutil.WaitShort, testutil.IntervalFast) + err := userClient.CancelWorkspaceBuild(ctx, build.ID) + var apiErr *codersdk.Error + require.ErrorAs(t, err, &apiErr) + require.Equal(t, http.StatusForbidden, apiErr.StatusCode()) + }) } func TestWorkspaceBuildResources(t *testing.T) { From 472fee8cd15daf9a63351dd722d09169fe75338a Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Fri, 18 Nov 2022 14:08:17 +0100 Subject: [PATCH 14/17] Fix: message --- coderd/workspacebuilds.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/workspacebuilds.go b/coderd/workspacebuilds.go index d33810d406d3d..31e9a900ae665 100644 --- a/coderd/workspacebuilds.go +++ b/coderd/workspacebuilds.go @@ -614,7 +614,7 @@ func (api *API) patchCancelWorkspaceBuild(rw http.ResponseWriter, r *http.Reques } if !valid { httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{ - Message: "User is not allowed cancel workspace builds. Owner role is required.", + Message: "User is not allowed to cancel workspace builds. Owner role is required.", }) return } From 4ec5aabf3ea7113135d291209d16cb419f9886de Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Fri, 18 Nov 2022 19:20:14 +0100 Subject: [PATCH 15/17] Address PR comment --- .../TemplateSettingsPage/TemplateSettingsForm.tsx | 2 ++ .../TemplateSettingsPage.test.tsx | 13 +++++++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx b/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx index f1851cbeb0687..4a861694493b5 100644 --- a/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx +++ b/site/src/pages/TemplateSettingsPage/TemplateSettingsForm.tsx @@ -59,6 +59,7 @@ export const validationSchema = Yup.object({ .integer() .min(0) .max(24 * MAX_TTL_DAYS /* 7 days in hours */, Language.ttlMaxError), + allow_user_cancel_workspace_jobs: Yup.boolean(), }) export interface TemplateSettingsForm { @@ -220,6 +221,7 @@ export const TemplateSettingsForm: FC = ({
+ {/*"getFieldHelpers" can't be used as it requires "helperText" property to be present.*/} ) => { const nameField = await screen.findByLabelText(FormLanguage.nameLabel) await userEvent.clear(nameField) await userEvent.type(nameField, name) const { t } = i18next - const displayNameLabel = t("displayNameLabel", { - ns: "templatePage", - }) + const displayNameLabel = t("displayNameLabel", { ns: "templatePage" }) const displayNameField = await screen.findByLabelText(displayNameLabel) await userEvent.clear(displayNameField) @@ -65,6 +64,12 @@ const fillAndSubmitForm = async ({ await userEvent.clear(maxTtlField) await userEvent.type(maxTtlField, default_ttl_ms.toString()) + const allowCancelJobsField = await screen.getByRole("checkbox") + // checkbox is checked by default, so it must be clicked to get unchecked + if (!allow_user_cancel_workspace_jobs) { + await userEvent.click(allowCancelJobsField) + } + const submitButton = await screen.findByText( FooterFormLanguage.defaultSubmitLabel, ) From c215f5ae123296ba3e5318eb99b49de029197100 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Mon, 21 Nov 2022 11:18:00 +0100 Subject: [PATCH 16/17] Address PR comments --- coderd/workspacebuilds.go | 9 ++------- codersdk/templates.go | 2 +- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/coderd/workspacebuilds.go b/coderd/workspacebuilds.go index 31e9a900ae665..48466dade9017 100644 --- a/coderd/workspacebuilds.go +++ b/coderd/workspacebuilds.go @@ -25,11 +25,6 @@ import ( "github.com/coder/coder/codersdk" ) -var ( - errTemplateNotExists = xerrors.New("No template exists for this workspace") - errUserNotExists = xerrors.New("User does not exist") -) - func (api *API) workspaceBuild(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() workspaceBuild := httpmw.WorkspaceBuildParam(r) @@ -669,7 +664,7 @@ func (api *API) patchCancelWorkspaceBuild(rw http.ResponseWriter, r *http.Reques func (api *API) verifyUserCanCancelWorkspaceBuilds(ctx context.Context, userID uuid.UUID, templateID uuid.UUID) (bool, error) { template, err := api.Database.GetTemplateByID(ctx, templateID) if err != nil { - return false, errTemplateNotExists + return false, xerrors.New("no template exists for this workspace") } if template.AllowUserCancelWorkspaceJobs { @@ -678,7 +673,7 @@ func (api *API) verifyUserCanCancelWorkspaceBuilds(ctx context.Context, userID u user, err := api.Database.GetUserByID(ctx, userID) if err != nil { - return false, errUserNotExists + return false, xerrors.New("user does not exist") } return slices.Contains(user.RBACRoles, rbac.RoleOwner()), nil // only user with "owner" role can cancel workspace builds } diff --git a/codersdk/templates.go b/codersdk/templates.go index bbae4238ec53c..68742538703ff 100644 --- a/codersdk/templates.go +++ b/codersdk/templates.go @@ -79,7 +79,7 @@ type UpdateTemplateMeta struct { Description string `json:"description,omitempty"` Icon string `json:"icon,omitempty"` DefaultTTLMillis int64 `json:"default_ttl_ms,omitempty"` - AllowUserCancelWorkspaceJobs bool `json:"allow_user_cancel_workspace_jobs"` + AllowUserCancelWorkspaceJobs bool `json:"allow_user_cancel_workspace_jobs,omitempty"` } // Template returns a single template. From 4e5ee76b7a712d09f8edcd65ac46d5455114b555 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Mon, 21 Nov 2022 11:25:23 +0100 Subject: [PATCH 17/17] Fix: make --- site/src/api/typesGenerated.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 9cd4aa4746aff..68f9a987a2829 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -727,7 +727,7 @@ export interface UpdateTemplateMeta { readonly description?: string readonly icon?: string readonly default_ttl_ms?: number - readonly allow_user_cancel_workspace_jobs: boolean + readonly allow_user_cancel_workspace_jobs?: boolean } // From codersdk/users.go