From fc715fdc25fb9199f8727e975d0338a1bd6d98ce Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 5 Oct 2023 08:51:58 -0500 Subject: [PATCH 01/37] prune template versions --- coderd/database/dbauthz/dbauthz.go | 11 + coderd/database/dbfake/dbfake.go | 9 + coderd/database/dbmetrics/dbmetrics.go | 7 + coderd/database/dbmock/dbmock.go | 14 + coderd/database/dump.sql | 4 +- .../000160_delete_template_versions.down.sql | 0 .../000160_delete_template_versions.up.sql | 28 ++ coderd/database/models.go | 2 + coderd/database/querier.go | 6 + coderd/database/queries.sql.go | 304 +++++++++++------- coderd/database/queries/templateversions.sql | 38 +++ docs/admin/audit-logs.md | 2 +- enterprise/audit/table.go | 3 +- 13 files changed, 301 insertions(+), 127 deletions(-) create mode 100644 coderd/database/migrations/000160_delete_template_versions.down.sql create mode 100644 coderd/database/migrations/000160_delete_template_versions.up.sql diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index c0a5cfcecf3cf..c5cf8f63755ce 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -2242,6 +2242,17 @@ func (q *querier) InsertWorkspaceResourceMetadata(ctx context.Context, arg datab return q.db.InsertWorkspaceResourceMetadata(ctx, arg) } +func (q *querier) PruneUnusedTemplateVersions(ctx context.Context, arg database.PruneUnusedTemplateVersionsParams) error { + tpl, err := q.db.GetTemplateByID(ctx, arg.TemplateID) + if err != nil { + return err + } + if err := q.authorizeContext(ctx, rbac.ActionUpdate, tpl); err != nil { + return err + } + return q.db.PruneUnusedTemplateVersions(ctx, arg) +} + func (q *querier) RegisterWorkspaceProxy(ctx context.Context, arg database.RegisterWorkspaceProxyParams) (database.WorkspaceProxy, error) { fetch := func(ctx context.Context, arg database.RegisterWorkspaceProxyParams) (database.WorkspaceProxy, error) { return q.db.GetWorkspaceProxyByID(ctx, arg.ID) diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index 1e6e4d9d410e7..ed63a6d4d281e 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -5175,6 +5175,15 @@ func (q *FakeQuerier) InsertWorkspaceResourceMetadata(_ context.Context, arg dat return metadata, nil } +func (q *FakeQuerier) PruneUnusedTemplateVersions(ctx context.Context, arg database.PruneUnusedTemplateVersionsParams) error { + err := validateDatabaseType(arg) + if err != nil { + return err + } + + panic("not implemented") +} + func (q *FakeQuerier) RegisterWorkspaceProxy(_ context.Context, arg database.RegisterWorkspaceProxyParams) (database.WorkspaceProxy, error) { q.mutex.Lock() defer q.mutex.Unlock() diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go index 3811cb9561801..e69afbab71546 100644 --- a/coderd/database/dbmetrics/dbmetrics.go +++ b/coderd/database/dbmetrics/dbmetrics.go @@ -1411,6 +1411,13 @@ func (m metricsStore) InsertWorkspaceResourceMetadata(ctx context.Context, arg d return metadata, err } +func (m metricsStore) PruneUnusedTemplateVersions(ctx context.Context, arg database.PruneUnusedTemplateVersionsParams) error { + start := time.Now() + r0 := m.s.PruneUnusedTemplateVersions(ctx, arg) + m.queryLatencies.WithLabelValues("PruneUnusedTemplateVersions").Observe(time.Since(start).Seconds()) + return r0 +} + func (m metricsStore) RegisterWorkspaceProxy(ctx context.Context, arg database.RegisterWorkspaceProxyParams) (database.WorkspaceProxy, error) { start := time.Now() proxy, err := m.s.RegisterWorkspaceProxy(ctx, arg) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index b2001b8b19640..2c6b95723f215 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -2980,6 +2980,20 @@ func (mr *MockStoreMockRecorder) Ping(arg0 interface{}) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Ping", reflect.TypeOf((*MockStore)(nil).Ping), arg0) } +// PruneUnusedTemplateVersions mocks base method. +func (m *MockStore) PruneUnusedTemplateVersions(arg0 context.Context, arg1 database.PruneUnusedTemplateVersionsParams) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "PruneUnusedTemplateVersions", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// PruneUnusedTemplateVersions indicates an expected call of PruneUnusedTemplateVersions. +func (mr *MockStoreMockRecorder) PruneUnusedTemplateVersions(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PruneUnusedTemplateVersions", reflect.TypeOf((*MockStore)(nil).PruneUnusedTemplateVersions), arg0, arg1) +} + // RegisterWorkspaceProxy mocks base method. func (m *MockStore) RegisterWorkspaceProxy(arg0 context.Context, arg1 database.RegisterWorkspaceProxyParams) (database.WorkspaceProxy, error) { m.ctrl.T.Helper() diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 17bb9c539e741..d3a57d17e5321 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -640,7 +640,8 @@ CREATE TABLE template_versions ( job_id uuid NOT NULL, created_by uuid NOT NULL, external_auth_providers text[], - message character varying(1048576) DEFAULT ''::character varying NOT NULL + message character varying(1048576) DEFAULT ''::character varying NOT NULL, + deleted boolean DEFAULT false NOT NULL ); COMMENT ON COLUMN template_versions.external_auth_providers IS 'IDs of External auth providers for a specific template version'; @@ -685,6 +686,7 @@ CREATE VIEW template_version_with_user AS template_versions.created_by, template_versions.external_auth_providers, template_versions.message, + template_versions.deleted, COALESCE(visible_users.avatar_url, ''::text) AS created_by_avatar_url, COALESCE(visible_users.username, ''::text) AS created_by_username FROM (public.template_versions diff --git a/coderd/database/migrations/000160_delete_template_versions.down.sql b/coderd/database/migrations/000160_delete_template_versions.down.sql new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/coderd/database/migrations/000160_delete_template_versions.up.sql b/coderd/database/migrations/000160_delete_template_versions.up.sql new file mode 100644 index 0000000000000..39a68c7975ba0 --- /dev/null +++ b/coderd/database/migrations/000160_delete_template_versions.up.sql @@ -0,0 +1,28 @@ +BEGIN; + +-- The view will be rebuilt with the new column +DROP VIEW template_version_with_user; + +-- Hard deleting can cause reference issues with audit logs, +-- so we only support soft deleting this resource at this time. +ALTER TABLE template_versions + ADD COLUMN deleted BOOLEAN NOT NULL DEFAULT FALSE; + +-- Restore the old version of the template_version_with_user view. +CREATE VIEW + template_version_with_user +AS +SELECT + template_versions.*, + coalesce(visible_users.avatar_url, '') AS created_by_avatar_url, + coalesce(visible_users.username, '') AS created_by_username +FROM + template_versions + LEFT JOIN + visible_users + ON + template_versions.created_by = visible_users.id; + +COMMENT ON VIEW template_version_with_user IS 'Joins in the username + avatar url of the created by user.'; + +COMMIT; diff --git a/coderd/database/models.go b/coderd/database/models.go index ab6d8861ee434..943aae9329595 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -1807,6 +1807,7 @@ type TemplateVersion struct { CreatedBy uuid.UUID `db:"created_by" json:"created_by"` ExternalAuthProviders []string `db:"external_auth_providers" json:"external_auth_providers"` Message string `db:"message" json:"message"` + Deleted bool `db:"deleted" json:"deleted"` CreatedByAvatarURL sql.NullString `db:"created_by_avatar_url" json:"created_by_avatar_url"` CreatedByUsername string `db:"created_by_username" json:"created_by_username"` } @@ -1861,6 +1862,7 @@ type TemplateVersionTable struct { ExternalAuthProviders []string `db:"external_auth_providers" json:"external_auth_providers"` // Message describing the changes in this version of the template, similar to a Git commit message. Like a commit message, this should be a short, high-level description of the changes in this version of the template. This message is immutable and should not be updated after the fact. Message string `db:"message" json:"message"` + Deleted bool `db:"deleted" json:"deleted"` } type TemplateVersionVariable struct { diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 04b7a4a4eedad..36bc71dfb2956 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -274,6 +274,12 @@ type sqlcQuerier interface { InsertWorkspaceProxy(ctx context.Context, arg InsertWorkspaceProxyParams) (WorkspaceProxy, error) InsertWorkspaceResource(ctx context.Context, arg InsertWorkspaceResourceParams) (WorkspaceResource, error) InsertWorkspaceResourceMetadata(ctx context.Context, arg InsertWorkspaceResourceMetadataParams) ([]WorkspaceResourceMetadatum, error) + // Pruning templates is a soft delete action, so is technically reversible. + // Soft deleting prevents the version from being used and discovered + // by listing. + // Only unused template versions will be pruned, which are any versions not + // referenced by the latest build of a workspace. + PruneUnusedTemplateVersions(ctx context.Context, arg PruneUnusedTemplateVersionsParams) error RegisterWorkspaceProxy(ctx context.Context, arg RegisterWorkspaceProxyParams) (WorkspaceProxy, error) RevokeDBCryptKey(ctx context.Context, activeKeyDigest string) error // Non blocking lock. Returns true if the lock was acquired, false otherwise. diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 40b7ee7e72715..e5a8303f0b4e3 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5303,7 +5303,7 @@ func (q *sqlQuerier) InsertTemplateVersionParameter(ctx context.Context, arg Ins const getPreviousTemplateVersion = `-- name: GetPreviousTemplateVersion :one SELECT - id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, created_by_avatar_url, created_by_username + id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, deleted, created_by_avatar_url, created_by_username FROM template_version_with_user AS template_versions WHERE @@ -5339,6 +5339,7 @@ func (q *sqlQuerier) GetPreviousTemplateVersion(ctx context.Context, arg GetPrev &i.CreatedBy, pq.Array(&i.ExternalAuthProviders), &i.Message, + &i.Deleted, &i.CreatedByAvatarURL, &i.CreatedByUsername, ) @@ -5347,7 +5348,7 @@ func (q *sqlQuerier) GetPreviousTemplateVersion(ctx context.Context, arg GetPrev const getTemplateVersionByID = `-- name: GetTemplateVersionByID :one SELECT - id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, created_by_avatar_url, created_by_username + id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, deleted, created_by_avatar_url, created_by_username FROM template_version_with_user AS template_versions WHERE @@ -5369,6 +5370,7 @@ func (q *sqlQuerier) GetTemplateVersionByID(ctx context.Context, id uuid.UUID) ( &i.CreatedBy, pq.Array(&i.ExternalAuthProviders), &i.Message, + &i.Deleted, &i.CreatedByAvatarURL, &i.CreatedByUsername, ) @@ -5377,7 +5379,7 @@ func (q *sqlQuerier) GetTemplateVersionByID(ctx context.Context, id uuid.UUID) ( const getTemplateVersionByJobID = `-- name: GetTemplateVersionByJobID :one SELECT - id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, created_by_avatar_url, created_by_username + id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, deleted, created_by_avatar_url, created_by_username FROM template_version_with_user AS template_versions WHERE @@ -5399,6 +5401,7 @@ func (q *sqlQuerier) GetTemplateVersionByJobID(ctx context.Context, jobID uuid.U &i.CreatedBy, pq.Array(&i.ExternalAuthProviders), &i.Message, + &i.Deleted, &i.CreatedByAvatarURL, &i.CreatedByUsername, ) @@ -5407,7 +5410,7 @@ func (q *sqlQuerier) GetTemplateVersionByJobID(ctx context.Context, jobID uuid.U const getTemplateVersionByTemplateIDAndName = `-- name: GetTemplateVersionByTemplateIDAndName :one SELECT - id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, created_by_avatar_url, created_by_username + id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, deleted, created_by_avatar_url, created_by_username FROM template_version_with_user AS template_versions WHERE @@ -5435,6 +5438,7 @@ func (q *sqlQuerier) GetTemplateVersionByTemplateIDAndName(ctx context.Context, &i.CreatedBy, pq.Array(&i.ExternalAuthProviders), &i.Message, + &i.Deleted, &i.CreatedByAvatarURL, &i.CreatedByUsername, ) @@ -5443,7 +5447,7 @@ func (q *sqlQuerier) GetTemplateVersionByTemplateIDAndName(ctx context.Context, const getTemplateVersionsByIDs = `-- name: GetTemplateVersionsByIDs :many SELECT - id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, created_by_avatar_url, created_by_username + id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, deleted, created_by_avatar_url, created_by_username FROM template_version_with_user AS template_versions WHERE @@ -5471,6 +5475,7 @@ func (q *sqlQuerier) GetTemplateVersionsByIDs(ctx context.Context, ids []uuid.UU &i.CreatedBy, pq.Array(&i.ExternalAuthProviders), &i.Message, + &i.Deleted, &i.CreatedByAvatarURL, &i.CreatedByUsername, ); err != nil { @@ -5489,16 +5494,23 @@ func (q *sqlQuerier) GetTemplateVersionsByIDs(ctx context.Context, ids []uuid.UU const getTemplateVersionsByTemplateID = `-- name: GetTemplateVersionsByTemplateID :many SELECT - id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, created_by_avatar_url, created_by_username + id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, deleted, created_by_avatar_url, created_by_username FROM template_version_with_user AS template_versions WHERE template_id = $1 :: uuid + AND CASE + -- If no filter is provided, default to returning ALL template versions. + -- The called should always provide a filter if they want to omit + -- deleted versions. + WHEN $2 :: boolean IS NULL THEN true + ELSE template_versions.deleted = $2 :: boolean + END AND CASE -- This allows using the last element on a page as effectively a cursor. -- This is an important option for scripts that need to paginate without -- duplicating or missing data. - WHEN $2 :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN ( + WHEN $3 :: uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN ( -- The pagination cursor is the last ID of the previous page. -- The query is ordered by the created_at field, so select all -- rows after the cursor. @@ -5508,7 +5520,7 @@ WHERE FROM template_versions WHERE - id = $2 + id = $3 ) ) ELSE true @@ -5516,14 +5528,15 @@ WHERE ORDER BY -- Deterministic and consistent ordering of all rows, even if they share -- a timestamp. This is to ensure consistent pagination. - (created_at, id) ASC OFFSET $3 + (created_at, id) ASC OFFSET $4 LIMIT -- A null limit means "no limit", so 0 means return all - NULLIF($4 :: int, 0) + NULLIF($5 :: int, 0) ` type GetTemplateVersionsByTemplateIDParams struct { TemplateID uuid.UUID `db:"template_id" json:"template_id"` + Deleted bool `db:"deleted" json:"deleted"` AfterID uuid.UUID `db:"after_id" json:"after_id"` OffsetOpt int32 `db:"offset_opt" json:"offset_opt"` LimitOpt int32 `db:"limit_opt" json:"limit_opt"` @@ -5532,6 +5545,7 @@ type GetTemplateVersionsByTemplateIDParams struct { func (q *sqlQuerier) GetTemplateVersionsByTemplateID(ctx context.Context, arg GetTemplateVersionsByTemplateIDParams) ([]TemplateVersion, error) { rows, err := q.db.QueryContext(ctx, getTemplateVersionsByTemplateID, arg.TemplateID, + arg.Deleted, arg.AfterID, arg.OffsetOpt, arg.LimitOpt, @@ -5555,6 +5569,7 @@ func (q *sqlQuerier) GetTemplateVersionsByTemplateID(ctx context.Context, arg Ge &i.CreatedBy, pq.Array(&i.ExternalAuthProviders), &i.Message, + &i.Deleted, &i.CreatedByAvatarURL, &i.CreatedByUsername, ); err != nil { @@ -5572,7 +5587,7 @@ func (q *sqlQuerier) GetTemplateVersionsByTemplateID(ctx context.Context, arg Ge } const getTemplateVersionsCreatedAfter = `-- name: GetTemplateVersionsCreatedAfter :many -SELECT id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, created_by_avatar_url, created_by_username FROM template_version_with_user AS template_versions WHERE created_at > $1 +SELECT id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, deleted, created_by_avatar_url, created_by_username FROM template_version_with_user AS template_versions WHERE created_at > $1 ` func (q *sqlQuerier) GetTemplateVersionsCreatedAfter(ctx context.Context, createdAt time.Time) ([]TemplateVersion, error) { @@ -5596,6 +5611,7 @@ func (q *sqlQuerier) GetTemplateVersionsCreatedAfter(ctx context.Context, create &i.CreatedBy, pq.Array(&i.ExternalAuthProviders), &i.Message, + &i.Deleted, &i.CreatedByAvatarURL, &i.CreatedByUsername, ); err != nil { @@ -5659,6 +5675,46 @@ func (q *sqlQuerier) InsertTemplateVersion(ctx context.Context, arg InsertTempla return err } +const pruneUnusedTemplateVersions = `-- name: PruneUnusedTemplateVersions :exec +UPDATE + template_versions +SET + deleted = true, + updated_at = $1 +WHERE + -- Actively used template versions (meaning the latest build is using + -- the version) are never pruned. A "restart" command on the workspace, + -- even if failed, would use the version. So it cannot be pruned until + -- the build is outdated. + -- TODO: This is an issue for "deleted workspaces", since a deleted workspace + -- has a build with the transition "delete". This will prevent that template + -- version from ever being pruned. We need a method to prune deleted workspaces. + template_versions.id != ANY( + SELECT DISTINCT ON(workspace_id) + template_version_id + FROM + workspace_builds + ORDER BY build_number DESC + ) + -- Scope a prune to a single template. + AND template_id = $2 :: uuid +` + +type PruneUnusedTemplateVersionsParams struct { + UpdatedAt time.Time `db:"updated_at" json:"updated_at"` + TemplateID uuid.UUID `db:"template_id" json:"template_id"` +} + +// Pruning templates is a soft delete action, so is technically reversible. +// Soft deleting prevents the version from being used and discovered +// by listing. +// Only unused template versions will be pruned, which are any versions not +// referenced by the latest build of a workspace. +func (q *sqlQuerier) PruneUnusedTemplateVersions(ctx context.Context, arg PruneUnusedTemplateVersionsParams) error { + _, err := q.db.ExecContext(ctx, pruneUnusedTemplateVersions, arg.UpdatedAt, arg.TemplateID) + return err +} + const updateTemplateVersionByID = `-- name: UpdateTemplateVersionByID :exec UPDATE template_versions @@ -9554,6 +9610,119 @@ func (q *sqlQuerier) InsertWorkspaceResourceMetadata(ctx context.Context, arg In return items, nil } +const getWorkspaceAgentScriptsByAgentIDs = `-- name: GetWorkspaceAgentScriptsByAgentIDs :many +SELECT workspace_agent_id, log_source_id, log_path, created_at, script, cron, start_blocks_login, run_on_start, run_on_stop, timeout_seconds FROM workspace_agent_scripts WHERE workspace_agent_id = ANY($1 :: uuid [ ]) +` + +func (q *sqlQuerier) GetWorkspaceAgentScriptsByAgentIDs(ctx context.Context, ids []uuid.UUID) ([]WorkspaceAgentScript, error) { + rows, err := q.db.QueryContext(ctx, getWorkspaceAgentScriptsByAgentIDs, pq.Array(ids)) + if err != nil { + return nil, err + } + defer rows.Close() + var items []WorkspaceAgentScript + for rows.Next() { + var i WorkspaceAgentScript + if err := rows.Scan( + &i.WorkspaceAgentID, + &i.LogSourceID, + &i.LogPath, + &i.CreatedAt, + &i.Script, + &i.Cron, + &i.StartBlocksLogin, + &i.RunOnStart, + &i.RunOnStop, + &i.TimeoutSeconds, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const insertWorkspaceAgentScripts = `-- name: InsertWorkspaceAgentScripts :many +INSERT INTO + workspace_agent_scripts (workspace_agent_id, created_at, log_source_id, log_path, script, cron, start_blocks_login, run_on_start, run_on_stop, timeout_seconds) +SELECT + $1 :: uuid AS workspace_agent_id, + $2 :: timestamptz AS created_at, + unnest($3 :: uuid [ ]) AS log_source_id, + unnest($4 :: text [ ]) AS log_path, + unnest($5 :: text [ ]) AS script, + unnest($6 :: text [ ]) AS cron, + unnest($7 :: boolean [ ]) AS start_blocks_login, + unnest($8 :: boolean [ ]) AS run_on_start, + unnest($9 :: boolean [ ]) AS run_on_stop, + unnest($10 :: integer [ ]) AS timeout_seconds +RETURNING workspace_agent_scripts.workspace_agent_id, workspace_agent_scripts.log_source_id, workspace_agent_scripts.log_path, workspace_agent_scripts.created_at, workspace_agent_scripts.script, workspace_agent_scripts.cron, workspace_agent_scripts.start_blocks_login, workspace_agent_scripts.run_on_start, workspace_agent_scripts.run_on_stop, workspace_agent_scripts.timeout_seconds +` + +type InsertWorkspaceAgentScriptsParams struct { + WorkspaceAgentID uuid.UUID `db:"workspace_agent_id" json:"workspace_agent_id"` + CreatedAt time.Time `db:"created_at" json:"created_at"` + LogSourceID []uuid.UUID `db:"log_source_id" json:"log_source_id"` + LogPath []string `db:"log_path" json:"log_path"` + Script []string `db:"script" json:"script"` + Cron []string `db:"cron" json:"cron"` + StartBlocksLogin []bool `db:"start_blocks_login" json:"start_blocks_login"` + RunOnStart []bool `db:"run_on_start" json:"run_on_start"` + RunOnStop []bool `db:"run_on_stop" json:"run_on_stop"` + TimeoutSeconds []int32 `db:"timeout_seconds" json:"timeout_seconds"` +} + +func (q *sqlQuerier) InsertWorkspaceAgentScripts(ctx context.Context, arg InsertWorkspaceAgentScriptsParams) ([]WorkspaceAgentScript, error) { + rows, err := q.db.QueryContext(ctx, insertWorkspaceAgentScripts, + arg.WorkspaceAgentID, + arg.CreatedAt, + pq.Array(arg.LogSourceID), + pq.Array(arg.LogPath), + pq.Array(arg.Script), + pq.Array(arg.Cron), + pq.Array(arg.StartBlocksLogin), + pq.Array(arg.RunOnStart), + pq.Array(arg.RunOnStop), + pq.Array(arg.TimeoutSeconds), + ) + if err != nil { + return nil, err + } + defer rows.Close() + var items []WorkspaceAgentScript + for rows.Next() { + var i WorkspaceAgentScript + if err := rows.Scan( + &i.WorkspaceAgentID, + &i.LogSourceID, + &i.LogPath, + &i.CreatedAt, + &i.Script, + &i.Cron, + &i.StartBlocksLogin, + &i.RunOnStart, + &i.RunOnStop, + &i.TimeoutSeconds, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const getDeploymentWorkspaceStats = `-- name: GetDeploymentWorkspaceStats :one WITH workspaces_with_jobs AS ( SELECT @@ -10502,116 +10671,3 @@ func (q *sqlQuerier) UpdateWorkspacesDormantDeletingAtByTemplateID(ctx context.C _, err := q.db.ExecContext(ctx, updateWorkspacesDormantDeletingAtByTemplateID, arg.TimeTilDormantAutodeleteMs, arg.DormantAt, arg.TemplateID) return err } - -const getWorkspaceAgentScriptsByAgentIDs = `-- name: GetWorkspaceAgentScriptsByAgentIDs :many -SELECT workspace_agent_id, log_source_id, log_path, created_at, script, cron, start_blocks_login, run_on_start, run_on_stop, timeout_seconds FROM workspace_agent_scripts WHERE workspace_agent_id = ANY($1 :: uuid [ ]) -` - -func (q *sqlQuerier) GetWorkspaceAgentScriptsByAgentIDs(ctx context.Context, ids []uuid.UUID) ([]WorkspaceAgentScript, error) { - rows, err := q.db.QueryContext(ctx, getWorkspaceAgentScriptsByAgentIDs, pq.Array(ids)) - if err != nil { - return nil, err - } - defer rows.Close() - var items []WorkspaceAgentScript - for rows.Next() { - var i WorkspaceAgentScript - if err := rows.Scan( - &i.WorkspaceAgentID, - &i.LogSourceID, - &i.LogPath, - &i.CreatedAt, - &i.Script, - &i.Cron, - &i.StartBlocksLogin, - &i.RunOnStart, - &i.RunOnStop, - &i.TimeoutSeconds, - ); err != nil { - return nil, err - } - items = append(items, i) - } - if err := rows.Close(); err != nil { - return nil, err - } - if err := rows.Err(); err != nil { - return nil, err - } - return items, nil -} - -const insertWorkspaceAgentScripts = `-- name: InsertWorkspaceAgentScripts :many -INSERT INTO - workspace_agent_scripts (workspace_agent_id, created_at, log_source_id, log_path, script, cron, start_blocks_login, run_on_start, run_on_stop, timeout_seconds) -SELECT - $1 :: uuid AS workspace_agent_id, - $2 :: timestamptz AS created_at, - unnest($3 :: uuid [ ]) AS log_source_id, - unnest($4 :: text [ ]) AS log_path, - unnest($5 :: text [ ]) AS script, - unnest($6 :: text [ ]) AS cron, - unnest($7 :: boolean [ ]) AS start_blocks_login, - unnest($8 :: boolean [ ]) AS run_on_start, - unnest($9 :: boolean [ ]) AS run_on_stop, - unnest($10 :: integer [ ]) AS timeout_seconds -RETURNING workspace_agent_scripts.workspace_agent_id, workspace_agent_scripts.log_source_id, workspace_agent_scripts.log_path, workspace_agent_scripts.created_at, workspace_agent_scripts.script, workspace_agent_scripts.cron, workspace_agent_scripts.start_blocks_login, workspace_agent_scripts.run_on_start, workspace_agent_scripts.run_on_stop, workspace_agent_scripts.timeout_seconds -` - -type InsertWorkspaceAgentScriptsParams struct { - WorkspaceAgentID uuid.UUID `db:"workspace_agent_id" json:"workspace_agent_id"` - CreatedAt time.Time `db:"created_at" json:"created_at"` - LogSourceID []uuid.UUID `db:"log_source_id" json:"log_source_id"` - LogPath []string `db:"log_path" json:"log_path"` - Script []string `db:"script" json:"script"` - Cron []string `db:"cron" json:"cron"` - StartBlocksLogin []bool `db:"start_blocks_login" json:"start_blocks_login"` - RunOnStart []bool `db:"run_on_start" json:"run_on_start"` - RunOnStop []bool `db:"run_on_stop" json:"run_on_stop"` - TimeoutSeconds []int32 `db:"timeout_seconds" json:"timeout_seconds"` -} - -func (q *sqlQuerier) InsertWorkspaceAgentScripts(ctx context.Context, arg InsertWorkspaceAgentScriptsParams) ([]WorkspaceAgentScript, error) { - rows, err := q.db.QueryContext(ctx, insertWorkspaceAgentScripts, - arg.WorkspaceAgentID, - arg.CreatedAt, - pq.Array(arg.LogSourceID), - pq.Array(arg.LogPath), - pq.Array(arg.Script), - pq.Array(arg.Cron), - pq.Array(arg.StartBlocksLogin), - pq.Array(arg.RunOnStart), - pq.Array(arg.RunOnStop), - pq.Array(arg.TimeoutSeconds), - ) - if err != nil { - return nil, err - } - defer rows.Close() - var items []WorkspaceAgentScript - for rows.Next() { - var i WorkspaceAgentScript - if err := rows.Scan( - &i.WorkspaceAgentID, - &i.LogSourceID, - &i.LogPath, - &i.CreatedAt, - &i.Script, - &i.Cron, - &i.StartBlocksLogin, - &i.RunOnStart, - &i.RunOnStop, - &i.TimeoutSeconds, - ); err != nil { - return nil, err - } - items = append(items, i) - } - if err := rows.Close(); err != nil { - return nil, err - } - if err := rows.Err(); err != nil { - return nil, err - } - return items, nil -} diff --git a/coderd/database/queries/templateversions.sql b/coderd/database/queries/templateversions.sql index db30b8cb17e84..8c1bc3c9cff27 100644 --- a/coderd/database/queries/templateversions.sql +++ b/coderd/database/queries/templateversions.sql @@ -1,10 +1,18 @@ -- name: GetTemplateVersionsByTemplateID :many +-- name: GetTemplateVersionsByTemplateID :many SELECT * FROM template_version_with_user AS template_versions WHERE template_id = @template_id :: uuid + AND CASE + -- If no filter is provided, default to returning ALL template versions. + -- The called should always provide a filter if they want to omit + -- deleted versions. + WHEN @deleted :: boolean IS NULL THEN true + ELSE template_versions.deleted = @deleted :: boolean + END AND CASE -- This allows using the last element on a page as effectively a cursor. -- This is an important option for scripts that need to paginate without @@ -129,3 +137,33 @@ WHERE AND template_id = $3 ORDER BY created_at DESC LIMIT 1; + + +-- name: PruneUnusedTemplateVersions :exec +-- Pruning templates is a soft delete action, so is technically reversible. +-- Soft deleting prevents the version from being used and discovered +-- by listing. +-- Only unused template versions will be pruned, which are any versions not +-- referenced by the latest build of a workspace. +UPDATE + template_versions +SET + deleted = true, + updated_at = @updated_at +WHERE + -- Actively used template versions (meaning the latest build is using + -- the version) are never pruned. A "restart" command on the workspace, + -- even if failed, would use the version. So it cannot be pruned until + -- the build is outdated. + -- TODO: This is an issue for "deleted workspaces", since a deleted workspace + -- has a build with the transition "delete". This will prevent that template + -- version from ever being pruned. We need a method to prune deleted workspaces. + template_versions.id != ANY( + SELECT DISTINCT ON(workspace_id) + template_version_id + FROM + workspace_builds + ORDER BY build_number DESC + ) + -- Scope a prune to a single template. + AND template_id = @template_id :: uuid; diff --git a/docs/admin/audit-logs.md b/docs/admin/audit-logs.md index 864d20cc756be..69198124f9792 100644 --- a/docs/admin/audit-logs.md +++ b/docs/admin/audit-logs.md @@ -16,7 +16,7 @@ We track the following resources: | GitSSHKey
create |
FieldTracked
created_atfalse
private_keytrue
public_keytrue
updated_atfalse
user_idtrue
| | License
create, delete |
FieldTracked
exptrue
idfalse
jwtfalse
uploaded_attrue
uuidtrue
| | Template
write, delete |
FieldTracked
active_version_idtrue
allow_user_autostarttrue
allow_user_autostoptrue
allow_user_cancel_workspace_jobstrue
autostop_requirement_days_of_weektrue
autostop_requirement_weekstrue
created_atfalse
created_bytrue
created_by_avatar_urlfalse
created_by_usernamefalse
default_ttltrue
deletedfalse
descriptiontrue
display_nametrue
failure_ttltrue
group_acltrue
icontrue
idtrue
max_ttltrue
nametrue
organization_idfalse
provisionertrue
time_til_dormanttrue
time_til_dormant_autodeletetrue
updated_atfalse
user_acltrue
| -| TemplateVersion
create, write |
FieldTracked
created_atfalse
created_bytrue
created_by_avatar_urlfalse
created_by_usernamefalse
external_auth_providersfalse
idtrue
job_idfalse
messagefalse
nametrue
organization_idfalse
readmetrue
template_idtrue
updated_atfalse
| +| TemplateVersion
create, write, delete |
FieldTracked
created_atfalse
created_bytrue
created_by_avatar_urlfalse
created_by_usernamefalse
deletedtrue
external_auth_providersfalse
idtrue
job_idfalse
messagefalse
nametrue
organization_idfalse
readmetrue
template_idtrue
updated_atfalse
| | User
create, write, delete |
FieldTracked
avatar_urlfalse
created_atfalse
deletedtrue
emailtrue
hashed_passwordtrue
idtrue
last_seen_atfalse
login_typetrue
quiet_hours_scheduletrue
rbac_rolestrue
statustrue
updated_atfalse
usernametrue
| | Workspace
create, write, delete |
FieldTracked
autostart_scheduletrue
created_atfalse
deletedfalse
deleting_attrue
dormant_attrue
idtrue
last_used_atfalse
nametrue
organization_idfalse
owner_idtrue
template_idtrue
ttltrue
updated_atfalse
| | WorkspaceBuild
start, stop |
FieldTracked
build_numberfalse
created_atfalse
daily_costfalse
deadlinefalse
idfalse
initiator_by_avatar_urlfalse
initiator_by_usernamefalse
initiator_idfalse
job_idfalse
max_deadlinefalse
provisioner_statefalse
reasonfalse
template_version_idtrue
transitionfalse
updated_atfalse
workspace_idfalse
| diff --git a/enterprise/audit/table.go b/enterprise/audit/table.go index 611c35db1456b..806f04f8cbcd9 100644 --- a/enterprise/audit/table.go +++ b/enterprise/audit/table.go @@ -18,7 +18,7 @@ import ( var AuditActionMap = map[string][]codersdk.AuditAction{ "GitSSHKey": {codersdk.AuditActionCreate}, "Template": {codersdk.AuditActionWrite, codersdk.AuditActionDelete}, - "TemplateVersion": {codersdk.AuditActionCreate, codersdk.AuditActionWrite}, + "TemplateVersion": {codersdk.AuditActionCreate, codersdk.AuditActionWrite, codersdk.AuditActionDelete}, "User": {codersdk.AuditActionCreate, codersdk.AuditActionWrite, codersdk.AuditActionDelete}, "Workspace": {codersdk.AuditActionCreate, codersdk.AuditActionWrite, codersdk.AuditActionDelete}, "WorkspaceBuild": {codersdk.AuditActionStart, codersdk.AuditActionStop}, @@ -99,6 +99,7 @@ var auditableResourcesTypes = map[any]map[string]Action{ "external_auth_providers": ActionIgnore, // Not helpful because this can only change when new versions are added. "created_by_avatar_url": ActionIgnore, "created_by_username": ActionIgnore, + "deleted": ActionTrack, }, &database.User{}: { "id": ActionTrack, From 611dac1f00d360dd03dc806c545a523050902617 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 5 Oct 2023 13:16:09 -0500 Subject: [PATCH 02/37] Implement fakes and mocks --- coderd/database/dbauthz/dbauthz.go | 6 +-- coderd/database/dbfake/dbfake.go | 41 ++++++++++++++++++-- coderd/database/dbmetrics/dbmetrics.go | 6 +-- coderd/database/dbmock/dbmock.go | 7 ++-- coderd/database/querier.go | 2 +- coderd/database/queries.sql.go | 29 ++++++++++++-- coderd/database/queries/templateversions.sql | 7 +++- 7 files changed, 79 insertions(+), 19 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index c5cf8f63755ce..48d02d6ae237a 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -2242,13 +2242,13 @@ func (q *querier) InsertWorkspaceResourceMetadata(ctx context.Context, arg datab return q.db.InsertWorkspaceResourceMetadata(ctx, arg) } -func (q *querier) PruneUnusedTemplateVersions(ctx context.Context, arg database.PruneUnusedTemplateVersionsParams) error { +func (q *querier) PruneUnusedTemplateVersions(ctx context.Context, arg database.PruneUnusedTemplateVersionsParams) ([]uuid.UUID, error) { tpl, err := q.db.GetTemplateByID(ctx, arg.TemplateID) if err != nil { - return err + return nil, err } if err := q.authorizeContext(ctx, rbac.ActionUpdate, tpl); err != nil { - return err + return nil, err } return q.db.PruneUnusedTemplateVersions(ctx, arg) } diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index ed63a6d4d281e..9eeb712b4324c 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -5175,13 +5175,48 @@ func (q *FakeQuerier) InsertWorkspaceResourceMetadata(_ context.Context, arg dat return metadata, nil } -func (q *FakeQuerier) PruneUnusedTemplateVersions(ctx context.Context, arg database.PruneUnusedTemplateVersionsParams) error { +func (q *FakeQuerier) PruneUnusedTemplateVersions(ctx context.Context, arg database.PruneUnusedTemplateVersionsParams) ([]uuid.UUID, error) { err := validateDatabaseType(arg) if err != nil { - return err + return nil, err + } + q.mutex.Lock() + defer q.mutex.Unlock() + type latestBuild struct { + Number int32 + Version uuid.UUID + } + latest := make(map[uuid.UUID]latestBuild) + + for _, b := range q.workspaceBuilds { + v, ok := latest[b.WorkspaceID] + if ok || b.BuildNumber < v.Number { + // Not the latest + continue + } + latest[b.WorkspaceID] = latestBuild{ + Number: b.BuildNumber, + Version: b.TemplateVersionID, + } + } + usedVersions := make(map[uuid.UUID]bool) + for _, l := range latest { + usedVersions[l.Version] = true + } + + var deleted []uuid.UUID + for i, v := range q.templateVersions { + if v.Deleted { + continue + } + if _, ok := usedVersions[v.ID]; !ok { + v.Deleted = true + q.templateVersions[i] = v + deleted = append(deleted, v.ID) + } } - panic("not implemented") + return deleted, nil } func (q *FakeQuerier) RegisterWorkspaceProxy(_ context.Context, arg database.RegisterWorkspaceProxyParams) (database.WorkspaceProxy, error) { diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go index e69afbab71546..84c037de533d9 100644 --- a/coderd/database/dbmetrics/dbmetrics.go +++ b/coderd/database/dbmetrics/dbmetrics.go @@ -1411,11 +1411,11 @@ func (m metricsStore) InsertWorkspaceResourceMetadata(ctx context.Context, arg d return metadata, err } -func (m metricsStore) PruneUnusedTemplateVersions(ctx context.Context, arg database.PruneUnusedTemplateVersionsParams) error { +func (m metricsStore) PruneUnusedTemplateVersions(ctx context.Context, arg database.PruneUnusedTemplateVersionsParams) ([]uuid.UUID, error) { start := time.Now() - r0 := m.s.PruneUnusedTemplateVersions(ctx, arg) + r0, r1 := m.s.PruneUnusedTemplateVersions(ctx, arg) m.queryLatencies.WithLabelValues("PruneUnusedTemplateVersions").Observe(time.Since(start).Seconds()) - return r0 + return r0, r1 } func (m metricsStore) RegisterWorkspaceProxy(ctx context.Context, arg database.RegisterWorkspaceProxyParams) (database.WorkspaceProxy, error) { diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 2c6b95723f215..f3411b3500a00 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -2981,11 +2981,12 @@ func (mr *MockStoreMockRecorder) Ping(arg0 interface{}) *gomock.Call { } // PruneUnusedTemplateVersions mocks base method. -func (m *MockStore) PruneUnusedTemplateVersions(arg0 context.Context, arg1 database.PruneUnusedTemplateVersionsParams) error { +func (m *MockStore) PruneUnusedTemplateVersions(arg0 context.Context, arg1 database.PruneUnusedTemplateVersionsParams) ([]uuid.UUID, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "PruneUnusedTemplateVersions", arg0, arg1) - ret0, _ := ret[0].(error) - return ret0 + ret0, _ := ret[0].([]uuid.UUID) + ret1, _ := ret[1].(error) + return ret0, ret1 } // PruneUnusedTemplateVersions indicates an expected call of PruneUnusedTemplateVersions. diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 36bc71dfb2956..15afae33a02c6 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -279,7 +279,7 @@ type sqlcQuerier interface { // by listing. // Only unused template versions will be pruned, which are any versions not // referenced by the latest build of a workspace. - PruneUnusedTemplateVersions(ctx context.Context, arg PruneUnusedTemplateVersionsParams) error + PruneUnusedTemplateVersions(ctx context.Context, arg PruneUnusedTemplateVersionsParams) ([]uuid.UUID, error) RegisterWorkspaceProxy(ctx context.Context, arg RegisterWorkspaceProxyParams) (WorkspaceProxy, error) RevokeDBCryptKey(ctx context.Context, activeKeyDigest string) error // Non blocking lock. Returns true if the lock was acquired, false otherwise. diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index e5a8303f0b4e3..8308a65935c94 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5675,7 +5675,7 @@ func (q *sqlQuerier) InsertTemplateVersion(ctx context.Context, arg InsertTempla return err } -const pruneUnusedTemplateVersions = `-- name: PruneUnusedTemplateVersions :exec +const pruneUnusedTemplateVersions = `-- name: PruneUnusedTemplateVersions :many UPDATE template_versions SET @@ -5696,8 +5696,11 @@ WHERE workspace_builds ORDER BY build_number DESC ) + -- Ignore already deleted versions. + AND template_versions.deleted = false -- Scope a prune to a single template. AND template_id = $2 :: uuid +RETURNING id ` type PruneUnusedTemplateVersionsParams struct { @@ -5710,9 +5713,27 @@ type PruneUnusedTemplateVersionsParams struct { // by listing. // Only unused template versions will be pruned, which are any versions not // referenced by the latest build of a workspace. -func (q *sqlQuerier) PruneUnusedTemplateVersions(ctx context.Context, arg PruneUnusedTemplateVersionsParams) error { - _, err := q.db.ExecContext(ctx, pruneUnusedTemplateVersions, arg.UpdatedAt, arg.TemplateID) - return err +func (q *sqlQuerier) PruneUnusedTemplateVersions(ctx context.Context, arg PruneUnusedTemplateVersionsParams) ([]uuid.UUID, error) { + rows, err := q.db.QueryContext(ctx, pruneUnusedTemplateVersions, arg.UpdatedAt, arg.TemplateID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []uuid.UUID + for rows.Next() { + var id uuid.UUID + if err := rows.Scan(&id); err != nil { + return nil, err + } + items = append(items, id) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil } const updateTemplateVersionByID = `-- name: UpdateTemplateVersionByID :exec diff --git a/coderd/database/queries/templateversions.sql b/coderd/database/queries/templateversions.sql index 8c1bc3c9cff27..faca946176681 100644 --- a/coderd/database/queries/templateversions.sql +++ b/coderd/database/queries/templateversions.sql @@ -139,7 +139,7 @@ ORDER BY created_at DESC LIMIT 1; --- name: PruneUnusedTemplateVersions :exec +-- name: PruneUnusedTemplateVersions :many -- Pruning templates is a soft delete action, so is technically reversible. -- Soft deleting prevents the version from being used and discovered -- by listing. @@ -165,5 +165,8 @@ WHERE workspace_builds ORDER BY build_number DESC ) + -- Ignore already deleted versions. + AND template_versions.deleted = false -- Scope a prune to a single template. - AND template_id = @template_id :: uuid; + AND template_id = @template_id :: uuid +RETURNING id; From 1de0a5dfb41b133a97618af3f1f6289694e71952 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 5 Oct 2023 15:46:17 -0500 Subject: [PATCH 03/37] Exclude template active versions --- coderd/database/dbfake/dbfake.go | 4 ++++ coderd/database/queries.sql.go | 4 ++++ coderd/database/queries/templateversions.sql | 4 ++++ 3 files changed, 12 insertions(+) diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index 9eeb712b4324c..6459b31e98c59 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -5199,10 +5199,14 @@ func (q *FakeQuerier) PruneUnusedTemplateVersions(ctx context.Context, arg datab Version: b.TemplateVersionID, } } + usedVersions := make(map[uuid.UUID]bool) for _, l := range latest { usedVersions[l.Version] = true } + for _, tpl := range q.templates { + usedVersions[tpl.ActiveVersionID] = true + } var deleted []uuid.UUID for i, v := range q.templateVersions { diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 8308a65935c94..fca15163e8332 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5681,6 +5681,8 @@ UPDATE SET deleted = true, updated_at = $1 +FROM + (SELECT active_version_id FROM templates WHERE id = $2) AS active_version WHERE -- Actively used template versions (meaning the latest build is using -- the version) are never pruned. A "restart" command on the workspace, @@ -5696,6 +5698,8 @@ WHERE workspace_builds ORDER BY build_number DESC ) + -- Also never delete the active template version + AND template_versions.id != ANY(active_version) -- Ignore already deleted versions. AND template_versions.deleted = false -- Scope a prune to a single template. diff --git a/coderd/database/queries/templateversions.sql b/coderd/database/queries/templateversions.sql index faca946176681..1c48dfba90abd 100644 --- a/coderd/database/queries/templateversions.sql +++ b/coderd/database/queries/templateversions.sql @@ -150,6 +150,8 @@ UPDATE SET deleted = true, updated_at = @updated_at +FROM + (SELECT active_version_id FROM templates WHERE id = @template_id) AS active_version WHERE -- Actively used template versions (meaning the latest build is using -- the version) are never pruned. A "restart" command on the workspace, @@ -165,6 +167,8 @@ WHERE workspace_builds ORDER BY build_number DESC ) + -- Also never delete the active template version + AND template_versions.id != ANY(active_version) -- Ignore already deleted versions. AND template_versions.deleted = false -- Scope a prune to a single template. From 3e5f91db99bd874603e1c0c951aa2a087d578fcd Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 5 Oct 2023 15:54:46 -0500 Subject: [PATCH 04/37] Delete versions are unusable --- coderd/database/queries/templateversions.sql | 59 ++++++++++++-------- coderd/templates.go | 9 +++ coderd/templateversions.go | 7 +++ coderd/workspaces.go | 12 ++++ codersdk/templateversions.go | 1 + 5 files changed, 66 insertions(+), 22 deletions(-) diff --git a/coderd/database/queries/templateversions.sql b/coderd/database/queries/templateversions.sql index 1c48dfba90abd..06d12e402445e 100644 --- a/coderd/database/queries/templateversions.sql +++ b/coderd/database/queries/templateversions.sql @@ -151,26 +151,41 @@ SET deleted = true, updated_at = @updated_at FROM - (SELECT active_version_id FROM templates WHERE id = @template_id) AS active_version + -- Delete all versions that are returned from this query. + ( + SELECT + id + FROM + -- Scope a prune to a single template and ignore already deleted template versions + (SELECT * FROM template_versions WHERE template_id = @template_id AND deleted = false) AS template_versions + LEFT JOIN + provisioner_jobs ON template_versions.job_id = provisioner_jobs.id + LEFT JOIN + templates ON template_versions.template_id = templates.id + WHERE + -- Actively used template versions (meaning the latest build is using + -- the version) are never pruned. A "restart" command on the workspace, + -- even if failed, would use the version. So it cannot be pruned until + -- the build is outdated. + -- TODO: This is an issue for "deleted workspaces", since a deleted workspace + -- has a build with the transition "delete". This will prevent that template + -- version from ever being pruned. We need a method to prune deleted workspaces. + template_versions.id != ANY( + SELECT DISTINCT ON(workspace_id) + template_version_id + FROM + workspace_builds + ORDER BY build_number DESC + ) + -- Also never delete the active template version + AND active_version_id != template_versions.id + AND CASE + WHEN @job_status != '' THEN + provisioner_jobs.job_status = @job_status + ELSE + true + END + + ) AS deleted_versions WHERE - -- Actively used template versions (meaning the latest build is using - -- the version) are never pruned. A "restart" command on the workspace, - -- even if failed, would use the version. So it cannot be pruned until - -- the build is outdated. - -- TODO: This is an issue for "deleted workspaces", since a deleted workspace - -- has a build with the transition "delete". This will prevent that template - -- version from ever being pruned. We need a method to prune deleted workspaces. - template_versions.id != ANY( - SELECT DISTINCT ON(workspace_id) - template_version_id - FROM - workspace_builds - ORDER BY build_number DESC - ) - -- Also never delete the active template version - AND template_versions.id != ANY(active_version) - -- Ignore already deleted versions. - AND template_versions.deleted = false - -- Scope a prune to a single template. - AND template_id = @template_id :: uuid -RETURNING id; + id = ANY(deleted_versions); diff --git a/coderd/templates.go b/coderd/templates.go index 9adc77bfe37e5..89089853facd3 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -193,6 +193,15 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque }) return } + if templateVersion.Deleted { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: fmt.Sprintf("Template version %s is deleted.", createTemplate.VersionID), + Validations: []codersdk.ValidationError{ + {Field: "template_version_id", Detail: "Template version is deleted"}, + }, + }) + return + } templateVersionAudit.Old = templateVersion if templateVersion.TemplateID.Valid { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ diff --git a/coderd/templateversions.go b/coderd/templateversions.go index fbde7e28e8651..083632be58fa2 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -1040,6 +1040,12 @@ func (api *API) patchActiveTemplateVersion(rw http.ResponseWriter, r *http.Reque }) return } + if version.Deleted { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "The provided template version is deleted.", + }) + return + } err = api.Database.InTx(func(store database.Store) error { err = store.UpdateTemplateActiveVersionByID(ctx, database.UpdateTemplateActiveVersionByIDParams{ @@ -1384,6 +1390,7 @@ func convertTemplateVersion(version database.TemplateVersion, job codersdk.Provi Message: version.Message, Job: job, Readme: version.Readme, + Deleted: version.Deleted, CreatedBy: codersdk.MinimalUser{ ID: version.CreatedBy, Username: version.CreatedByUsername, diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 6aa08b991705d..cdab1b7091398 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -356,6 +356,18 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req }) return } + if templateVersion.Deleted { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Template version has been deleted.", + Validations: []codersdk.ValidationError{ + { + Field: "template_version_id", + Detail: "template version deleted and unusable", + }, + }, + }) + return + } templateID = templateVersion.TemplateID.UUID } diff --git a/codersdk/templateversions.go b/codersdk/templateversions.go index 773c256e05d08..77384b85d5b26 100644 --- a/codersdk/templateversions.go +++ b/codersdk/templateversions.go @@ -29,6 +29,7 @@ type TemplateVersion struct { Job ProvisionerJob `json:"job"` Readme string `json:"readme"` CreatedBy MinimalUser `json:"created_by"` + Deleted bool `json:"deleted"` Warnings []TemplateVersionWarning `json:"warnings,omitempty" enums:"DEPRECATED_PARAMETERS"` } From 6e6fbc323061dfe63d97a4a945d7518da00baf4c Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 5 Oct 2023 16:37:10 -0500 Subject: [PATCH 05/37] migration bump --- .../000160_delete_template_versions.down.sql | 0 .../000162_delete_template_versions.down.sql | 26 +++++++++++++++++++ ...=> 000162_delete_template_versions.up.sql} | 0 coderd/database/queries/templateversions.sql | 2 ++ 4 files changed, 28 insertions(+) delete mode 100644 coderd/database/migrations/000160_delete_template_versions.down.sql create mode 100644 coderd/database/migrations/000162_delete_template_versions.down.sql rename coderd/database/migrations/{000160_delete_template_versions.up.sql => 000162_delete_template_versions.up.sql} (100%) diff --git a/coderd/database/migrations/000160_delete_template_versions.down.sql b/coderd/database/migrations/000160_delete_template_versions.down.sql deleted file mode 100644 index e69de29bb2d1d..0000000000000 diff --git a/coderd/database/migrations/000162_delete_template_versions.down.sql b/coderd/database/migrations/000162_delete_template_versions.down.sql new file mode 100644 index 0000000000000..4dfa26d7117d4 --- /dev/null +++ b/coderd/database/migrations/000162_delete_template_versions.down.sql @@ -0,0 +1,26 @@ +BEGIN; + +-- The view will be rebuilt with the new column +DROP VIEW template_version_with_user; + +ALTER TABLE template_versions + DROP COLUMN deleted; + +-- Restore the old version of the template_version_with_user view. +CREATE VIEW + template_version_with_user +AS +SELECT + template_versions.*, + coalesce(visible_users.avatar_url, '') AS created_by_avatar_url, + coalesce(visible_users.username, '') AS created_by_username +FROM + template_versions + LEFT JOIN + visible_users + ON + template_versions.created_by = visible_users.id; + +COMMENT ON VIEW template_version_with_user IS 'Joins in the username + avatar url of the created by user.'; + +COMMIT; diff --git a/coderd/database/migrations/000160_delete_template_versions.up.sql b/coderd/database/migrations/000162_delete_template_versions.up.sql similarity index 100% rename from coderd/database/migrations/000160_delete_template_versions.up.sql rename to coderd/database/migrations/000162_delete_template_versions.up.sql diff --git a/coderd/database/queries/templateversions.sql b/coderd/database/queries/templateversions.sql index 06d12e402445e..2f58fcf90dd5b 100644 --- a/coderd/database/queries/templateversions.sql +++ b/coderd/database/queries/templateversions.sql @@ -180,6 +180,8 @@ FROM -- Also never delete the active template version AND active_version_id != template_versions.id AND CASE + -- Optionally, only prune versions that match a given + -- job status like 'failed'. WHEN @job_status != '' THEN provisioner_jobs.job_status = @job_status ELSE From 2e2c09246120d012a729ab4a8b4c2de9ac6b2749 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 5 Oct 2023 17:32:36 -0500 Subject: [PATCH 06/37] Spaces to tabs --- coderd/database/dbfake/dbfake.go | 2 +- coderd/database/queries.sql.go | 71 +++++++++++++------- coderd/database/queries/templateversions.sql | 60 ++++++++--------- 3 files changed, 76 insertions(+), 57 deletions(-) diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index af8d926fa8057..c73738c056902 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -5196,7 +5196,7 @@ func (q *FakeQuerier) InsertWorkspaceResourceMetadata(_ context.Context, arg dat return metadata, nil } -func (q *FakeQuerier) PruneUnusedTemplateVersions(ctx context.Context, arg database.PruneUnusedTemplateVersionsParams) ([]uuid.UUID, error) { +func (q *FakeQuerier) PruneUnusedTemplateVersions(_ context.Context, arg database.PruneUnusedTemplateVersionsParams) ([]uuid.UUID, error) { err := validateDatabaseType(arg) if err != nil { return nil, err diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index dce8ab7662df2..79366e04b8629 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5689,34 +5689,53 @@ SET deleted = true, updated_at = $1 FROM - (SELECT active_version_id FROM templates WHERE id = $2) AS active_version -WHERE - -- Actively used template versions (meaning the latest build is using - -- the version) are never pruned. A "restart" command on the workspace, - -- even if failed, would use the version. So it cannot be pruned until - -- the build is outdated. - -- TODO: This is an issue for "deleted workspaces", since a deleted workspace - -- has a build with the transition "delete". This will prevent that template - -- version from ever being pruned. We need a method to prune deleted workspaces. - template_versions.id != ANY( - SELECT DISTINCT ON(workspace_id) - template_version_id - FROM - workspace_builds - ORDER BY build_number DESC - ) - -- Also never delete the active template version - AND template_versions.id != ANY(active_version) - -- Ignore already deleted versions. - AND template_versions.deleted = false - -- Scope a prune to a single template. - AND template_id = $2 :: uuid -RETURNING id + -- Delete all versions that are returned from this query. + ( + SELECT + scoped_template_versions.id + FROM + -- Scope a prune to a single template and ignore already deleted template versions + (SELECT id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, deleted FROM template_versions WHERE template_versions.template_id = $2 :: uuid AND deleted = false) AS scoped_template_versions + LEFT JOIN + provisioner_jobs ON scoped_template_versions.job_id = provisioner_jobs.id + LEFT JOIN + templates ON scoped_template_versions.template_id = templates.id + WHERE + -- Actively used template versions (meaning the latest build is using + -- the version) are never pruned. A "restart" command on the workspace, + -- even if failed, would use the version. So it cannot be pruned until + -- the build is outdated. + -- TODO: This is an issue for "deleted workspaces", since a deleted workspace + -- has a build with the transition "delete". This will prevent that template + -- version from ever being pruned. We need a method to prune deleted workspaces. + scoped_template_versions.id != ANY( + SELECT DISTINCT ON(workspace_id) + scoped_template_versions + FROM + workspace_builds + ORDER BY build_number DESC + ) + -- Also never delete the active template version + AND active_version_id != scoped_template_versions.id + AND CASE + -- Optionally, only prune versions that match a given + -- job status like 'failed'. + WHEN $3 :: provisioner_job_status IS NOT NULL THEN + provisioner_jobs.job_status = $3 :: provisioner_job_status + ELSE + true + END + + ) AS deleted_versions +WHERE + template_versions.id = ANY(deleted_versions) +RETURNING template_versions.id ` type PruneUnusedTemplateVersionsParams struct { - UpdatedAt time.Time `db:"updated_at" json:"updated_at"` - TemplateID uuid.UUID `db:"template_id" json:"template_id"` + UpdatedAt time.Time `db:"updated_at" json:"updated_at"` + TemplateID uuid.UUID `db:"template_id" json:"template_id"` + JobStatus NullProvisionerJobStatus `db:"job_status" json:"job_status"` } // Pruning templates is a soft delete action, so is technically reversible. @@ -5725,7 +5744,7 @@ type PruneUnusedTemplateVersionsParams struct { // Only unused template versions will be pruned, which are any versions not // referenced by the latest build of a workspace. func (q *sqlQuerier) PruneUnusedTemplateVersions(ctx context.Context, arg PruneUnusedTemplateVersionsParams) ([]uuid.UUID, error) { - rows, err := q.db.QueryContext(ctx, pruneUnusedTemplateVersions, arg.UpdatedAt, arg.TemplateID) + rows, err := q.db.QueryContext(ctx, pruneUnusedTemplateVersions, arg.UpdatedAt, arg.TemplateID, arg.JobStatus) if err != nil { return nil, err } diff --git a/coderd/database/queries/templateversions.sql b/coderd/database/queries/templateversions.sql index 2f58fcf90dd5b..eb8a5964f5e06 100644 --- a/coderd/database/queries/templateversions.sql +++ b/coderd/database/queries/templateversions.sql @@ -7,9 +7,9 @@ FROM WHERE template_id = @template_id :: uuid AND CASE - -- If no filter is provided, default to returning ALL template versions. - -- The called should always provide a filter if they want to omit - -- deleted versions. + -- If no filter is provided, default to returning ALL template versions. + -- The called should always provide a filter if they want to omit + -- deleted versions. WHEN @deleted :: boolean IS NULL THEN true ELSE template_versions.deleted = @deleted :: boolean END @@ -33,8 +33,8 @@ WHERE ELSE true END ORDER BY - -- Deterministic and consistent ordering of all rows, even if they share - -- a timestamp. This is to ensure consistent pagination. + -- Deterministic and consistent ordering of all rows, even if they share + -- a timestamp. This is to ensure consistent pagination. (created_at, id) ASC OFFSET @offset_opt LIMIT -- A null limit means "no limit", so 0 means return all @@ -149,20 +149,20 @@ UPDATE template_versions SET deleted = true, - updated_at = @updated_at + updated_at = sqlc.arg('updated_at') FROM - -- Delete all versions that are returned from this query. - ( - SELECT - id - FROM + -- Delete all versions that are returned from this query. + ( + SELECT + scoped_template_versions.id + FROM -- Scope a prune to a single template and ignore already deleted template versions - (SELECT * FROM template_versions WHERE template_id = @template_id AND deleted = false) AS template_versions - LEFT JOIN - provisioner_jobs ON template_versions.job_id = provisioner_jobs.id - LEFT JOIN - templates ON template_versions.template_id = templates.id - WHERE + (SELECT * FROM template_versions WHERE template_versions.template_id = sqlc.arg('template_id') :: uuid AND deleted = false) AS scoped_template_versions + LEFT JOIN + provisioner_jobs ON scoped_template_versions.job_id = provisioner_jobs.id + LEFT JOIN + templates ON scoped_template_versions.template_id = templates.id + WHERE -- Actively used template versions (meaning the latest build is using -- the version) are never pruned. A "restart" command on the workspace, -- even if failed, would use the version. So it cannot be pruned until @@ -170,24 +170,24 @@ FROM -- TODO: This is an issue for "deleted workspaces", since a deleted workspace -- has a build with the transition "delete". This will prevent that template -- version from ever being pruned. We need a method to prune deleted workspaces. - template_versions.id != ANY( + scoped_template_versions.id != ANY( SELECT DISTINCT ON(workspace_id) - template_version_id + scoped_template_versions FROM workspace_builds ORDER BY build_number DESC ) -- Also never delete the active template version - AND active_version_id != template_versions.id - AND CASE - -- Optionally, only prune versions that match a given - -- job status like 'failed'. - WHEN @job_status != '' THEN - provisioner_jobs.job_status = @job_status - ELSE - true - END - + AND active_version_id != scoped_template_versions.id + AND CASE + -- Optionally, only prune versions that match a given + -- job status like 'failed'. + WHEN sqlc.narg('job_status') :: provisioner_job_status IS NOT NULL THEN + provisioner_jobs.job_status = sqlc.narg('job_status') :: provisioner_job_status + ELSE + true + END ) AS deleted_versions WHERE - id = ANY(deleted_versions); + template_versions.id = ANY(deleted_versions) +RETURNING template_versions.id; From 7f3d80537272054e6e9c55da1f5ce99309156db1 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 9 Oct 2023 11:03:16 -0500 Subject: [PATCH 07/37] add template version prune command --- cli/templatedelete.go | 28 +---- cli/templateprune.go | 106 +++++++++++++++++++ cli/templates.go | 37 ++++++- coderd/coderd.go | 1 + coderd/database/dbfake/dbfake.go | 22 ++++ coderd/database/querier.go | 1 + coderd/database/queries.sql.go | 98 +++++++++-------- coderd/database/queries/templateversions.sql | 43 +++++--- coderd/templateversions.go | 68 ++++++++++++ coderd/templateversions_test.go | 97 +++++++++++++++++ codersdk/templates.go | 29 +++++ site/src/api/typesGenerated.ts | 12 +++ site/src/testHelpers/entities.ts | 3 + 13 files changed, 458 insertions(+), 87 deletions(-) create mode 100644 cli/templateprune.go diff --git a/cli/templatedelete.go b/cli/templatedelete.go index 6cb4213a93895..e15fe4bd48722 100644 --- a/cli/templatedelete.go +++ b/cli/templatedelete.go @@ -48,33 +48,13 @@ func (r *RootCmd) templateDelete() *clibase.Cmd { templates = append(templates, template) } } else { - allTemplates, err := client.TemplatesByOrganization(ctx, organization.ID) + template, err := selectTemplate(inv, client, organization) if err != nil { - return xerrors.Errorf("get templates by organization: %w", err) + return err } - if len(allTemplates) == 0 { - return xerrors.Errorf("no templates exist in the current organization %q", organization.Name) - } - - opts := make([]string, 0, len(allTemplates)) - for _, template := range allTemplates { - opts = append(opts, template.Name) - } - - selection, err := cliui.Select(inv, cliui.SelectOptions{ - Options: opts, - }) - if err != nil { - return xerrors.Errorf("select template: %w", err) - } - - for _, template := range allTemplates { - if template.Name == selection { - templates = append(templates, template) - templateNames = append(templateNames, template.Name) - } - } + templates = append(templates, template) + templateNames = append(templateNames, template.Name) } // Confirm deletion of the template. diff --git a/cli/templateprune.go b/cli/templateprune.go new file mode 100644 index 0000000000000..15f7e2584b5b3 --- /dev/null +++ b/cli/templateprune.go @@ -0,0 +1,106 @@ +package cli + +import ( + "encoding/json" + "fmt" + "strings" + "time" + + "golang.org/x/xerrors" + + "github.com/coder/pretty" + + "github.com/coder/coder/v2/cli/clibase" + "github.com/coder/coder/v2/cli/cliui" + "github.com/coder/coder/v2/codersdk" +) + +func (r *RootCmd) templatePrune() *clibase.Cmd { + var ( + all clibase.Bool + ) + client := new(codersdk.Client) + cmd := &clibase.Cmd{ + Use: "prune [name...]", + Short: "Prune unused failed template versions from a given template(s)", + Middleware: clibase.Chain( + r.InitClient(client), + ), + Options: clibase.OptionSet{ + cliui.SkipPromptOption(), + clibase.Option{ + Name: "all", + Description: "Include all unused template versions. By default, only failed template versions are pruned.", + Flag: "all", + Value: &all, + }, + }, + Handler: func(inv *clibase.Invocation) error { + var ( + ctx = inv.Context() + templateNames = []string{} + templates = []codersdk.Template{} + ) + + organization, err := CurrentOrganization(inv, client) + if err != nil { + return err + } + + if len(inv.Args) > 0 { + templateNames = inv.Args + + for _, templateName := range templateNames { + template, err := client.TemplateByName(ctx, organization.ID, templateName) + if err != nil { + return xerrors.Errorf("get template by name: %w", err) + } + templates = append(templates, template) + } + } else { + template, err := selectTemplate(inv, client, organization) + if err != nil { + return err + } + + templates = append(templates, template) + templateNames = append(templateNames, template.Name) + } + + // Confirm prune of the template. + _, err = cliui.Prompt(inv, cliui.PromptOptions{ + Text: fmt.Sprintf("Prune template versions of these templates: %s?", pretty.Sprint(cliui.DefaultStyles.Code, strings.Join(templateNames, ", "))), + IsConfirm: true, + Default: cliui.ConfirmNo, + }) + if err != nil { + return err + } + + for _, template := range templates { + resp, err := client.PruneTemplateVersions(ctx, template.ID, all.Value()) + if err != nil { + return xerrors.Errorf("delete template %q: %w", template.Name, err) + } + + _, _ = fmt.Fprintln( + inv.Stdout, fmt.Sprintf("Deleted %s versions from "+pretty.Sprint(cliui.DefaultStyles.Keyword, template.Name)+" at "+cliui.Timestamp(time.Now()), len(resp.DeletedIDs)), + ) + + if ok, _ := inv.ParsedFlags().GetBool("verbose"); err == nil && ok { + data, err := json.Marshal(resp) + if err != nil { + return xerrors.Errorf("marshal verbose response: %w", err) + } + _, _ = fmt.Fprintln( + inv.Stdout, string(data), + ) + } + } + + return nil + }, + } + + return cmd +} diff --git a/cli/templates.go b/cli/templates.go index 3d24ec14b5ccc..5a47577dab82c 100644 --- a/cli/templates.go +++ b/cli/templates.go @@ -3,9 +3,9 @@ package cli import ( "time" - "github.com/google/uuid" - "github.com/coder/pretty" + "github.com/google/uuid" + "golang.org/x/xerrors" "github.com/coder/coder/v2/cli/clibase" "github.com/coder/coder/v2/cli/cliui" @@ -43,12 +43,45 @@ func (r *RootCmd) templates() *clibase.Cmd { r.templateVersions(), r.templateDelete(), r.templatePull(), + r.templatePrune(), }, } return cmd } +func selectTemplate(inv *clibase.Invocation, client *codersdk.Client, organization codersdk.Organization) (codersdk.Template, error) { + var empty codersdk.Template + ctx := inv.Context() + allTemplates, err := client.TemplatesByOrganization(ctx, organization.ID) + if err != nil { + return empty, xerrors.Errorf("get templates by organization: %w", err) + } + + if len(allTemplates) == 0 { + return empty, xerrors.Errorf("no templates exist in the current organization %q", organization.Name) + } + + opts := make([]string, 0, len(allTemplates)) + for _, template := range allTemplates { + opts = append(opts, template.Name) + } + + selection, err := cliui.Select(inv, cliui.SelectOptions{ + Options: opts, + }) + if err != nil { + return empty, xerrors.Errorf("select template: %w", err) + } + + for _, template := range allTemplates { + if template.Name == selection { + return template, nil + } + } + return empty, xerrors.Errorf("no template selected") +} + type templateTableRow struct { // Used by json format: Template codersdk.Template diff --git a/coderd/coderd.go b/coderd/coderd.go index b8cf0957773c4..9c23aa2e87df6 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -671,6 +671,7 @@ func New(options *Options) *API { r.Delete("/", api.deleteTemplate) r.Patch("/", api.patchTemplateMeta) r.Route("/versions", func(r chi.Router) { + r.Delete("/prune", api.deletePruneTemplateVersions) r.Get("/", api.templateVersionsByTemplate) r.Patch("/", api.patchActiveTemplateVersion) r.Get("/{templateversionname}", api.templateVersionByName) diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index c73738c056902..bde4b2239a494 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -2758,6 +2758,9 @@ func (q *FakeQuerier) GetTemplateVersionsByTemplateID(_ context.Context, arg dat if templateVersion.TemplateID.UUID != arg.TemplateID { continue } + if arg.Deleted.Valid && arg.Deleted.Bool != templateVersion.Deleted { + continue + } version = append(version, q.templateVersionWithUserNoLock(templateVersion)) } @@ -5234,7 +5237,26 @@ func (q *FakeQuerier) PruneUnusedTemplateVersions(_ context.Context, arg databas if v.Deleted { continue } + if _, ok := usedVersions[v.ID]; !ok { + var job *database.ProvisionerJob + for _, j := range q.provisionerJobs { + if v.JobID == j.ID { + job = &j + break + } + } + + if arg.JobStatus.Valid { + if job.JobStatus != arg.JobStatus.ProvisionerJobStatus { + continue + } + } + + if job.JobStatus == database.ProvisionerJobStatusRunning || job.JobStatus == database.ProvisionerJobStatusPending { + continue + } + v.Deleted = true q.templateVersions[i] = v deleted = append(deleted, v.ID) diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 15afae33a02c6..98c4c483978b7 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -279,6 +279,7 @@ type sqlcQuerier interface { // by listing. // Only unused template versions will be pruned, which are any versions not // referenced by the latest build of a workspace. + // used_versions.transition != 'delete', PruneUnusedTemplateVersions(ctx context.Context, arg PruneUnusedTemplateVersionsParams) ([]uuid.UUID, error) RegisterWorkspaceProxy(ctx context.Context, arg RegisterWorkspaceProxyParams) (WorkspaceProxy, error) RevokeDBCryptKey(ctx context.Context, activeKeyDigest string) error diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 79366e04b8629..8fd27be2d2094 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5507,9 +5507,9 @@ FROM WHERE template_id = $1 :: uuid AND CASE - -- If no filter is provided, default to returning ALL template versions. - -- The called should always provide a filter if they want to omit - -- deleted versions. + -- If no filter is provided, default to returning ALL template versions. + -- The called should always provide a filter if they want to omit + -- deleted versions. WHEN $2 :: boolean IS NULL THEN true ELSE template_versions.deleted = $2 :: boolean END @@ -5533,8 +5533,8 @@ WHERE ELSE true END ORDER BY - -- Deterministic and consistent ordering of all rows, even if they share - -- a timestamp. This is to ensure consistent pagination. + -- Deterministic and consistent ordering of all rows, even if they share + -- a timestamp. This is to ensure consistent pagination. (created_at, id) ASC OFFSET $4 LIMIT -- A null limit means "no limit", so 0 means return all @@ -5542,11 +5542,11 @@ LIMIT ` type GetTemplateVersionsByTemplateIDParams struct { - TemplateID uuid.UUID `db:"template_id" json:"template_id"` - Deleted bool `db:"deleted" json:"deleted"` - AfterID uuid.UUID `db:"after_id" json:"after_id"` - OffsetOpt int32 `db:"offset_opt" json:"offset_opt"` - LimitOpt int32 `db:"limit_opt" json:"limit_opt"` + TemplateID uuid.UUID `db:"template_id" json:"template_id"` + Deleted sql.NullBool `db:"deleted" json:"deleted"` + AfterID uuid.UUID `db:"after_id" json:"after_id"` + OffsetOpt int32 `db:"offset_opt" json:"offset_opt"` + LimitOpt int32 `db:"limit_opt" json:"limit_opt"` } func (q *sqlQuerier) GetTemplateVersionsByTemplateID(ctx context.Context, arg GetTemplateVersionsByTemplateIDParams) ([]TemplateVersion, error) { @@ -5689,46 +5689,54 @@ SET deleted = true, updated_at = $1 FROM - -- Delete all versions that are returned from this query. - ( - SELECT + -- Delete all versions that are returned from this query. + ( + SELECT scoped_template_versions.id - FROM + FROM -- Scope a prune to a single template and ignore already deleted template versions - (SELECT id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, deleted FROM template_versions WHERE template_versions.template_id = $2 :: uuid AND deleted = false) AS scoped_template_versions - LEFT JOIN - provisioner_jobs ON scoped_template_versions.job_id = provisioner_jobs.id - LEFT JOIN - templates ON scoped_template_versions.template_id = templates.id - WHERE - -- Actively used template versions (meaning the latest build is using - -- the version) are never pruned. A "restart" command on the workspace, - -- even if failed, would use the version. So it cannot be pruned until - -- the build is outdated. - -- TODO: This is an issue for "deleted workspaces", since a deleted workspace - -- has a build with the transition "delete". This will prevent that template - -- version from ever being pruned. We need a method to prune deleted workspaces. - scoped_template_versions.id != ANY( - SELECT DISTINCT ON(workspace_id) - scoped_template_versions + (SELECT id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, deleted FROM template_versions WHERE template_versions.template_id = $2 :: uuid AND deleted = false) AS scoped_template_versions + LEFT JOIN + provisioner_jobs ON scoped_template_versions.job_id = provisioner_jobs.id + LEFT JOIN + templates ON scoped_template_versions.template_id = templates.id + WHERE + -- Actively used template versions (meaning the latest build is using + -- the version) are never pruned. A "restart" command on the workspace, + -- even if failed, would use the version. So it cannot be pruned until + -- the build is outdated. + NOT EXISTS ( + -- Return all "used" versions, where "used" is defined as being + -- used by a latest workspace build. + SELECT template_version_id FROM ( + SELECT + DISTINCT ON (workspace_id) template_version_id, transition FROM - workspace_builds - ORDER BY build_number DESC - ) + workspace_builds + ORDER BY workspace_id, build_number DESC + ) AS used_versions + WHERE + -- TODO: This is an issue for "deleted workspaces", since a deleted workspace + -- has a build with the transition "delete". This will prevent that template + -- version from ever being pruned. We need a method to prune deleted workspaces. + scoped_template_versions.id = used_versions.template_version_id + ) -- Also never delete the active template version - AND active_version_id != scoped_template_versions.id - AND CASE - -- Optionally, only prune versions that match a given - -- job status like 'failed'. - WHEN $3 :: provisioner_job_status IS NOT NULL THEN - provisioner_jobs.job_status = $3 :: provisioner_job_status - ELSE - true - END - + AND active_version_id != scoped_template_versions.id + AND CASE + -- Optionally, only prune versions that match a given + -- job status like 'failed'. + WHEN $3 :: provisioner_job_status IS NOT NULL THEN + provisioner_jobs.job_status = $3 :: provisioner_job_status + ELSE + true + END + -- Pending or running jobs should not be pruned, as they are "in progress" + AND provisioner_jobs.job_status != 'running' + AND provisioner_jobs.job_status != 'pending' ) AS deleted_versions WHERE - template_versions.id = ANY(deleted_versions) + template_versions.id IN (deleted_versions.id) RETURNING template_versions.id ` @@ -5743,6 +5751,8 @@ type PruneUnusedTemplateVersionsParams struct { // by listing. // Only unused template versions will be pruned, which are any versions not // referenced by the latest build of a workspace. +// +// used_versions.transition != 'delete', func (q *sqlQuerier) PruneUnusedTemplateVersions(ctx context.Context, arg PruneUnusedTemplateVersionsParams) ([]uuid.UUID, error) { rows, err := q.db.QueryContext(ctx, pruneUnusedTemplateVersions, arg.UpdatedAt, arg.TemplateID, arg.JobStatus) if err != nil { diff --git a/coderd/database/queries/templateversions.sql b/coderd/database/queries/templateversions.sql index eb8a5964f5e06..e1c78bc4b7236 100644 --- a/coderd/database/queries/templateversions.sql +++ b/coderd/database/queries/templateversions.sql @@ -1,5 +1,4 @@ -- name: GetTemplateVersionsByTemplateID :many --- name: GetTemplateVersionsByTemplateID :many SELECT * FROM @@ -10,8 +9,8 @@ WHERE -- If no filter is provided, default to returning ALL template versions. -- The called should always provide a filter if they want to omit -- deleted versions. - WHEN @deleted :: boolean IS NULL THEN true - ELSE template_versions.deleted = @deleted :: boolean + WHEN sqlc.narg('deleted') :: boolean IS NULL THEN true + ELSE template_versions.deleted = sqlc.narg('deleted') :: boolean END AND CASE -- This allows using the last element on a page as effectively a cursor. @@ -163,20 +162,27 @@ FROM LEFT JOIN templates ON scoped_template_versions.template_id = templates.id WHERE - -- Actively used template versions (meaning the latest build is using - -- the version) are never pruned. A "restart" command on the workspace, - -- even if failed, would use the version. So it cannot be pruned until - -- the build is outdated. - -- TODO: This is an issue for "deleted workspaces", since a deleted workspace - -- has a build with the transition "delete". This will prevent that template - -- version from ever being pruned. We need a method to prune deleted workspaces. - scoped_template_versions.id != ANY( - SELECT DISTINCT ON(workspace_id) - scoped_template_versions + -- Actively used template versions (meaning the latest build is using + -- the version) are never pruned. A "restart" command on the workspace, + -- even if failed, would use the version. So it cannot be pruned until + -- the build is outdated. + NOT EXISTS ( + -- Return all "used" versions, where "used" is defined as being + -- used by a latest workspace build. + SELECT template_version_id FROM ( + SELECT + DISTINCT ON (workspace_id) template_version_id, transition FROM - workspace_builds - ORDER BY build_number DESC - ) + workspace_builds + ORDER BY workspace_id, build_number DESC + ) AS used_versions + WHERE + -- TODO: This is an issue for "deleted workspaces", since a deleted workspace + -- has a build with the transition "delete". This will prevent that template + -- version from ever being pruned. We need a method to prune deleted workspaces. +-- used_versions.transition != 'delete', + scoped_template_versions.id = used_versions.template_version_id + ) -- Also never delete the active template version AND active_version_id != scoped_template_versions.id AND CASE @@ -187,7 +193,10 @@ FROM ELSE true END + -- Pending or running jobs should not be pruned, as they are "in progress" + AND provisioner_jobs.job_status != 'running' + AND provisioner_jobs.job_status != 'pending' ) AS deleted_versions WHERE - template_versions.id = ANY(deleted_versions) + template_versions.id IN (deleted_versions.id) RETURNING template_versions.id; diff --git a/coderd/templateversions.go b/coderd/templateversions.go index 2645f3b233d99..f8ee896950e98 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -743,6 +743,11 @@ func (api *API) templateVersionsByTemplate(rw http.ResponseWriter, r *http.Reque AfterID: paginationParams.AfterID, LimitOpt: int32(paginationParams.Limit), OffsetOpt: int32(paginationParams.Offset), + // Exclude deleted templates versions + Deleted: sql.NullBool{ + Bool: false, + Valid: true, + }, }) if errors.Is(err, sql.ErrNoRows) { httpapi.Write(ctx, rw, http.StatusOK, apiVersions) @@ -991,6 +996,69 @@ func (api *API) previousTemplateVersionByOrganizationTemplateAndName(rw http.Res httpapi.Write(ctx, rw, http.StatusOK, convertTemplateVersion(previousTemplateVersion, convertProvisionerJob(jobs[0]), nil)) } +// @Summary Prune template unused versions by template id +// @ID prune-template-versions-by-template-id +// @Security CoderSessionToken +// @Accept json +// @Produce json +// @Tags Templates +// @Param template path string true "Template ID" format(uuid) +// @Success 200 {object} codersdk.Response +// @Router /templates/{template}/versions/prune [delete] +func (api *API) deletePruneTemplateVersions(rw http.ResponseWriter, r *http.Request) { + var ( + ctx = r.Context() + template = httpmw.TemplateParam(r) + auditor = *api.Auditor.Load() + aReq, commitAudit = audit.InitRequest[database.Template](rw, &audit.RequestParams{ + Audit: auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionWrite, + }) + ) + defer commitAudit() + aReq.Old = template + + var req codersdk.PruneTemplateVersionsRequest + if !httpapi.Read(ctx, rw, r, &req) { + return + } + + status := database.NullProvisionerJobStatus{ + ProvisionerJobStatus: database.ProvisionerJobStatusFailed, + Valid: true, + } + if req.All { + status = database.NullProvisionerJobStatus{} + } + + deleted, err := api.Database.PruneUnusedTemplateVersions(ctx, database.PruneUnusedTemplateVersionsParams{ + UpdatedAt: dbtime.Now(), + TemplateID: template.ID, + JobStatus: status, + }) + + if httpapi.Is404Error(err) { + httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ + Message: "Template or template versions not found.", + }) + return + } + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error fetching template version.", + Detail: err.Error(), + }) + return + } + + httpapi.Write(ctx, rw, http.StatusOK, codersdk.PruneTemplateVersionsResponse{ + TemplateID: template.ID, + DeletedIDs: deleted, + }) +} + // @Summary Update active template version by template ID // @ID update-active-template-version-by-template-id // @Security CoderSessionToken diff --git a/coderd/templateversions_test.go b/coderd/templateversions_test.go index 898c24a805519..12d0581ba8d4b 100644 --- a/coderd/templateversions_test.go +++ b/coderd/templateversions_test.go @@ -1515,3 +1515,100 @@ func TestTemplateVersionParameters_Order(t *testing.T) { require.Equal(t, secondParameterName, templateRichParameters[3].Name) require.Equal(t, thirdParameterName, templateRichParameters[4].Name) } + +func TestTemplatePruneVersions(t *testing.T) { + t.Parallel() + + client, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + user := coderdtest.CreateFirstUser(t, client) + var _ = api + + var totalVersions int + // Create a template to prune + initialVersion := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + totalVersions++ + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, initialVersion.ID) + + allFailed := make([]uuid.UUID, 0) + expDeleted := make([]uuid.UUID, 0) + // create some failed versions + for i := 0; i < 2; i++ { + failed := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ + Parse: echo.ParseComplete, + ProvisionPlan: echo.PlanFailed, + ProvisionApply: echo.ApplyFailed, + }, func(req *codersdk.CreateTemplateVersionRequest) { + req.TemplateID = template.ID + }) + allFailed = append(allFailed, failed.ID) + totalVersions++ + } + + // Create some unused versions + for i := 0; i < 2; i++ { + unused := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ + Parse: echo.ParseComplete, + ProvisionPlan: echo.PlanComplete, + ProvisionApply: echo.ApplyComplete, + }, func(req *codersdk.CreateTemplateVersionRequest) { + req.TemplateID = template.ID + }) + expDeleted = append(expDeleted, unused.ID) + totalVersions++ + } + + // Create some used template versions + for i := 0; i < 2; i++ { + used := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ + Parse: echo.ParseComplete, + ProvisionPlan: echo.PlanComplete, //echo.PlanFailed, + ProvisionApply: echo.ApplyComplete, + }, func(req *codersdk.CreateTemplateVersionRequest) { + req.TemplateID = template.ID + }) + coderdtest.AwaitTemplateVersionJobCompleted(t, client, used.ID) + workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, uuid.Nil, func(request *codersdk.CreateWorkspaceRequest) { + request.TemplateVersionID = used.ID + }) + coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID) + totalVersions++ + } + + ctx := testutil.Context(t, testutil.WaitMedium) + versions, err := client.TemplateVersionsByTemplate(ctx, codersdk.TemplateVersionsByTemplateRequest{ + TemplateID: template.ID, + Pagination: codersdk.Pagination{ + Limit: 100, + }, + }) + require.NoError(t, err, "fetch all versions") + require.Len(t, versions, totalVersions, "total versions") + + // Prune failed versions + deleteFailed, err := client.PruneTemplateVersions(ctx, template.ID, false) + require.NoError(t, err, "prune failed versions") + require.ElementsMatch(t, deleteFailed.DeletedIDs, allFailed, "all failed versions deleted") + + remaining, err := client.TemplateVersionsByTemplate(ctx, codersdk.TemplateVersionsByTemplateRequest{ + TemplateID: template.ID, + Pagination: codersdk.Pagination{ + Limit: 100, + }, + }) + require.NoError(t, err, "fetch all non-failed versions") + require.Len(t, remaining, totalVersions-len(allFailed), "remaining non-failed versions") + + // Try pruning "All" unused templates + deleted, err := client.PruneTemplateVersions(ctx, template.ID, true) + require.NoError(t, err, "prune versions") + require.ElementsMatch(t, deleted.DeletedIDs, expDeleted, "all expected versions deleted") + + remaining, err = client.TemplateVersionsByTemplate(ctx, codersdk.TemplateVersionsByTemplateRequest{ + TemplateID: template.ID, + Pagination: codersdk.Pagination{ + Limit: 100, + }, + }) + require.NoError(t, err, "fetch all versions") + require.Len(t, remaining, totalVersions-len(expDeleted)-len(allFailed), "remaining versions") +} diff --git a/codersdk/templates.go b/codersdk/templates.go index 7fc441bda5bd3..04ad9998d6565 100644 --- a/codersdk/templates.go +++ b/codersdk/templates.go @@ -136,6 +136,17 @@ type ( } ) +type PruneTemplateVersionsRequest struct { + // By default, only failed versions are pruned. Set this to true + // to prune all unused versions regardless of job status. + All bool `json:"all"` +} + +type PruneTemplateVersionsResponse struct { + TemplateID uuid.UUID `json:"template_id" format:"uuid"` + DeletedIDs []uuid.UUID `json:"deleted_ids"` +} + type TemplateRole string const ( @@ -227,6 +238,24 @@ func (c *Client) Template(ctx context.Context, template uuid.UUID) (Template, er return resp, json.NewDecoder(res.Body).Decode(&resp) } +func (c *Client) PruneTemplateVersions(ctx context.Context, template uuid.UUID, all bool) (PruneTemplateVersionsResponse, error) { + res, err := c.Request(ctx, http.MethodDelete, + fmt.Sprintf("/api/v2/templates/%s/versions/prune", template), + PruneTemplateVersionsRequest{ + All: all, + }, + ) + if err != nil { + return PruneTemplateVersionsResponse{}, err + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + return PruneTemplateVersionsResponse{}, ReadBodyAsError(res) + } + var resp PruneTemplateVersionsResponse + return resp, json.NewDecoder(res.Body).Decode(&resp) +} + func (c *Client) DeleteTemplate(ctx context.Context, template uuid.UUID) error { res, err := c.Request(ctx, http.MethodDelete, fmt.Sprintf("/api/v2/templates/%s", template), nil) if err != nil { diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 401485d55078a..c59407355cbe1 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -753,6 +753,17 @@ export interface ProxyHealthReport { readonly warnings: string[]; } +// From codersdk/templates.go +export interface PruneTemplateVersionsRequest { + readonly all: boolean; +} + +// From codersdk/templates.go +export interface PruneTemplateVersionsResponse { + readonly template_id: string; + readonly deleted_ids: string[]; +} + // From codersdk/workspaces.go export interface PutExtendWorkspaceRequest { readonly deadline: string; @@ -1009,6 +1020,7 @@ export interface TemplateVersion { readonly job: ProvisionerJob; readonly readme: string; readonly created_by: MinimalUser; + readonly deleted: boolean; readonly warnings?: TemplateVersionWarning[]; } diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index f9341889a9c54..5f1b6539f8564 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -379,6 +379,7 @@ You can add instructions here [Some link info](https://coder.com)`, created_by: MockUser, + deleted: false, }; export const MockTemplateVersion2: TypesGen.TemplateVersion = { @@ -397,6 +398,7 @@ You can add instructions here [Some link info](https://coder.com)`, created_by: MockUser, + deleted: false, }; export const MockTemplateVersion3: TypesGen.TemplateVersion = { @@ -410,6 +412,7 @@ export const MockTemplateVersion3: TypesGen.TemplateVersion = { readme: "README", created_by: MockUser, warnings: ["UNSUPPORTED_WORKSPACES"], + deleted: false, }; export const MockTemplate: TypesGen.Template = { From 02aa08524f632976fc54d405658d9f9747a565ad Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 9 Oct 2023 13:19:53 -0500 Subject: [PATCH 08/37] Rename to archive --- cli/templates.go | 2 +- ...lateprune.go => templateversionarchive.go} | 21 +- coderd/coderd.go | 2 +- coderd/database/dbauthz/dbauthz.go | 22 +- coderd/database/dbfake/dbfake.go | 134 +++++------ coderd/database/dbmetrics/dbmetrics.go | 14 +- coderd/database/dbmock/dbmock.go | 12 +- coderd/database/dump.sql | 4 +- .../000162_delete_template_versions.down.sql | 2 +- .../000162_delete_template_versions.up.sql | 9 +- coderd/database/models.go | 6 +- coderd/database/querier.go | 14 +- coderd/database/queries.sql.go | 224 +++++++++--------- coderd/database/queries/templateversions.sql | 36 +-- coderd/templateversions.go | 22 +- coderd/templateversions_test.go | 18 +- codersdk/templates.go | 26 +- 17 files changed, 283 insertions(+), 285 deletions(-) rename cli/{templateprune.go => templateversionarchive.go} (74%) diff --git a/cli/templates.go b/cli/templates.go index 5a47577dab82c..d7cd02a467ef2 100644 --- a/cli/templates.go +++ b/cli/templates.go @@ -43,7 +43,7 @@ func (r *RootCmd) templates() *clibase.Cmd { r.templateVersions(), r.templateDelete(), r.templatePull(), - r.templatePrune(), + r.archiveTemplateVersions(), }, } diff --git a/cli/templateprune.go b/cli/templateversionarchive.go similarity index 74% rename from cli/templateprune.go rename to cli/templateversionarchive.go index 15f7e2584b5b3..334e731b56524 100644 --- a/cli/templateprune.go +++ b/cli/templateversionarchive.go @@ -6,23 +6,22 @@ import ( "strings" "time" - "golang.org/x/xerrors" - "github.com/coder/pretty" + "golang.org/x/xerrors" "github.com/coder/coder/v2/cli/clibase" "github.com/coder/coder/v2/cli/cliui" "github.com/coder/coder/v2/codersdk" ) -func (r *RootCmd) templatePrune() *clibase.Cmd { +func (r *RootCmd) archiveTemplateVersions() *clibase.Cmd { var ( all clibase.Bool ) client := new(codersdk.Client) cmd := &clibase.Cmd{ - Use: "prune [name...]", - Short: "Prune unused failed template versions from a given template(s)", + Use: "archive [name...] ", + Short: "Archive unused failed template versions from a given template(s)", Middleware: clibase.Chain( r.InitClient(client), ), @@ -30,7 +29,7 @@ func (r *RootCmd) templatePrune() *clibase.Cmd { cliui.SkipPromptOption(), clibase.Option{ Name: "all", - Description: "Include all unused template versions. By default, only failed template versions are pruned.", + Description: "Include all unused template versions. By default, only failed template versions are archived.", Flag: "all", Value: &all, }, @@ -67,9 +66,9 @@ func (r *RootCmd) templatePrune() *clibase.Cmd { templateNames = append(templateNames, template.Name) } - // Confirm prune of the template. + // Confirm archive of the template. _, err = cliui.Prompt(inv, cliui.PromptOptions{ - Text: fmt.Sprintf("Prune template versions of these templates: %s?", pretty.Sprint(cliui.DefaultStyles.Code, strings.Join(templateNames, ", "))), + Text: fmt.Sprintf("Archive template versions of these templates: %s?", pretty.Sprint(cliui.DefaultStyles.Code, strings.Join(templateNames, ", "))), IsConfirm: true, Default: cliui.ConfirmNo, }) @@ -78,13 +77,13 @@ func (r *RootCmd) templatePrune() *clibase.Cmd { } for _, template := range templates { - resp, err := client.PruneTemplateVersions(ctx, template.ID, all.Value()) + resp, err := client.ArchiveTemplateVersions(ctx, template.ID, all.Value()) if err != nil { - return xerrors.Errorf("delete template %q: %w", template.Name, err) + return xerrors.Errorf("archive template versions for %q: %w", template.Name, err) } _, _ = fmt.Fprintln( - inv.Stdout, fmt.Sprintf("Deleted %s versions from "+pretty.Sprint(cliui.DefaultStyles.Keyword, template.Name)+" at "+cliui.Timestamp(time.Now()), len(resp.DeletedIDs)), + inv.Stdout, fmt.Sprintf("Archive %s versions from "+pretty.Sprint(cliui.DefaultStyles.Keyword, template.Name)+" at "+cliui.Timestamp(time.Now()), len(resp.ArchivedIDs)), ) if ok, _ := inv.ParsedFlags().GetBool("verbose"); err == nil && ok { diff --git a/coderd/coderd.go b/coderd/coderd.go index 9c23aa2e87df6..791a3e0a33557 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -671,7 +671,7 @@ func New(options *Options) *API { r.Delete("/", api.deleteTemplate) r.Patch("/", api.patchTemplateMeta) r.Route("/versions", func(r chi.Router) { - r.Delete("/prune", api.deletePruneTemplateVersions) + r.Post("/archive", api.postArchiveTemplateVersions) r.Get("/", api.templateVersionsByTemplate) r.Patch("/", api.patchActiveTemplateVersion) r.Get("/{templateversionname}", api.templateVersionByName) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 48d02d6ae237a..7d30eb72b7597 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -673,6 +673,17 @@ func (q *querier) AllUserIDs(ctx context.Context) ([]uuid.UUID, error) { return q.db.AllUserIDs(ctx) } +func (q *querier) ArchiveUnusedTemplateVersions(ctx context.Context, arg database.ArchiveUnusedTemplateVersionsParams) ([]uuid.UUID, error) { + tpl, err := q.db.GetTemplateByID(ctx, arg.TemplateID) + if err != nil { + return nil, err + } + if err := q.authorizeContext(ctx, rbac.ActionUpdate, tpl); err != nil { + return nil, err + } + return q.db.ArchiveUnusedTemplateVersions(ctx, arg) +} + func (q *querier) CleanTailnetCoordinators(ctx context.Context) error { if err := q.authorizeContext(ctx, rbac.ActionDelete, rbac.ResourceTailnetCoordinator); err != nil { return err @@ -2242,17 +2253,6 @@ func (q *querier) InsertWorkspaceResourceMetadata(ctx context.Context, arg datab return q.db.InsertWorkspaceResourceMetadata(ctx, arg) } -func (q *querier) PruneUnusedTemplateVersions(ctx context.Context, arg database.PruneUnusedTemplateVersionsParams) ([]uuid.UUID, error) { - tpl, err := q.db.GetTemplateByID(ctx, arg.TemplateID) - if err != nil { - return nil, err - } - if err := q.authorizeContext(ctx, rbac.ActionUpdate, tpl); err != nil { - return nil, err - } - return q.db.PruneUnusedTemplateVersions(ctx, arg) -} - func (q *querier) RegisterWorkspaceProxy(ctx context.Context, arg database.RegisterWorkspaceProxyParams) (database.WorkspaceProxy, error) { fetch := func(ctx context.Context, arg database.RegisterWorkspaceProxyParams) (database.WorkspaceProxy, error) { return q.db.GetWorkspaceProxyByID(ctx, arg.ID) diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index bde4b2239a494..da91b75ed2715 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -845,6 +845,73 @@ func (q *FakeQuerier) AllUserIDs(_ context.Context) ([]uuid.UUID, error) { return userIDs, nil } +func (q *FakeQuerier) ArchiveUnusedTemplateVersions(_ context.Context, arg database.ArchiveUnusedTemplateVersionsParams) ([]uuid.UUID, error) { + err := validateDatabaseType(arg) + if err != nil { + return nil, err + } + q.mutex.Lock() + defer q.mutex.Unlock() + type latestBuild struct { + Number int32 + Version uuid.UUID + } + latest := make(map[uuid.UUID]latestBuild) + + for _, b := range q.workspaceBuilds { + v, ok := latest[b.WorkspaceID] + if ok || b.BuildNumber < v.Number { + // Not the latest + continue + } + latest[b.WorkspaceID] = latestBuild{ + Number: b.BuildNumber, + Version: b.TemplateVersionID, + } + } + + usedVersions := make(map[uuid.UUID]bool) + for _, l := range latest { + usedVersions[l.Version] = true + } + for _, tpl := range q.templates { + usedVersions[tpl.ActiveVersionID] = true + } + + var deleted []uuid.UUID + for i, v := range q.templateVersions { + if v.Archived { + continue + } + + if _, ok := usedVersions[v.ID]; !ok { + var job *database.ProvisionerJob + for i, j := range q.provisionerJobs { + if v.JobID == j.ID { + job = &q.provisionerJobs[i] + break + } + } + + if arg.JobStatus.Valid { + if job.JobStatus != arg.JobStatus.ProvisionerJobStatus { + continue + } + } + + if job.JobStatus == database.ProvisionerJobStatusRunning || job.JobStatus == database.ProvisionerJobStatusPending { + continue + } + + v.Archived = true + q.templateVersions[i] = v + deleted = append(deleted, v.ID) + } + } + + return deleted, nil +} + func (*FakeQuerier) CleanTailnetCoordinators(_ context.Context) error { return ErrUnimplemented } @@ -5199,73 +5266,6 @@ func (q *FakeQuerier) InsertWorkspaceResourceMetadata(_ context.Context, arg dat return metadata, nil } -func (q *FakeQuerier) PruneUnusedTemplateVersions(_ context.Context, arg database.PruneUnusedTemplateVersionsParams) ([]uuid.UUID, error) { - err := validateDatabaseType(arg) - if err != nil { - return nil, err - } - q.mutex.Lock() - defer q.mutex.Unlock() - type latestBuild struct { - Number int32 - Version uuid.UUID - } - latest := make(map[uuid.UUID]latestBuild) - - for _, b := range q.workspaceBuilds { - v, ok := latest[b.WorkspaceID] - if ok || b.BuildNumber < v.Number { - // Not the latest - continue - } - latest[b.WorkspaceID] = latestBuild{ - Number: b.BuildNumber, - Version: b.TemplateVersionID, - } - } - - usedVersions := make(map[uuid.UUID]bool) - for _, l := range latest { - usedVersions[l.Version] = true - } - for _, tpl := range q.templates { - usedVersions[tpl.ActiveVersionID] = true - } - - var deleted []uuid.UUID - for i, v := range q.templateVersions { - if v.Deleted { - continue - } - - if _, ok := usedVersions[v.ID]; !ok { - var job *database.ProvisionerJob - for _, j := range q.provisionerJobs { - if v.JobID == j.ID { - job = &j - break - } - } - - if arg.JobStatus.Valid { - if job.JobStatus != arg.JobStatus.ProvisionerJobStatus { - continue - } - } - - if job.JobStatus == database.ProvisionerJobStatusRunning || job.JobStatus == database.ProvisionerJobStatusPending { - continue - } - - v.Deleted = true - q.templateVersions[i] = v - deleted = append(deleted, v.ID) - } - } - - return deleted, nil -} - func (q *FakeQuerier) RegisterWorkspaceProxy(_ context.Context, arg database.RegisterWorkspaceProxyParams) (database.WorkspaceProxy, error) { q.mutex.Lock() defer q.mutex.Unlock() diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go index 84c037de533d9..d44c49724380e 100644 --- a/coderd/database/dbmetrics/dbmetrics.go +++ b/coderd/database/dbmetrics/dbmetrics.go @@ -107,6 +107,13 @@ func (m metricsStore) AllUserIDs(ctx context.Context) ([]uuid.UUID, error) { return r0, r1 } +func (m metricsStore) ArchiveUnusedTemplateVersions(ctx context.Context, arg database.ArchiveUnusedTemplateVersionsParams) ([]uuid.UUID, error) { + start := time.Now() + r0, r1 := m.s.ArchiveUnusedTemplateVersions(ctx, arg) + m.queryLatencies.WithLabelValues("ArchiveUnusedTemplateVersions").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m metricsStore) CleanTailnetCoordinators(ctx context.Context) error { start := time.Now() err := m.s.CleanTailnetCoordinators(ctx) @@ -1411,13 +1418,6 @@ func (m metricsStore) InsertWorkspaceResourceMetadata(ctx context.Context, arg d return metadata, err } -func (m metricsStore) PruneUnusedTemplateVersions(ctx context.Context, arg database.PruneUnusedTemplateVersionsParams) ([]uuid.UUID, error) { - start := time.Now() - r0, r1 := m.s.PruneUnusedTemplateVersions(ctx, arg) - m.queryLatencies.WithLabelValues("PruneUnusedTemplateVersions").Observe(time.Since(start).Seconds()) - return r0, r1 -} - func (m metricsStore) RegisterWorkspaceProxy(ctx context.Context, arg database.RegisterWorkspaceProxyParams) (database.WorkspaceProxy, error) { start := time.Now() proxy, err := m.s.RegisterWorkspaceProxy(ctx, arg) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index f3411b3500a00..389d0fff22dd2 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -2980,19 +2980,19 @@ func (mr *MockStoreMockRecorder) Ping(arg0 interface{}) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Ping", reflect.TypeOf((*MockStore)(nil).Ping), arg0) } -// PruneUnusedTemplateVersions mocks base method. -func (m *MockStore) PruneUnusedTemplateVersions(arg0 context.Context, arg1 database.PruneUnusedTemplateVersionsParams) ([]uuid.UUID, error) { +// ArchiveUnusedTemplateVersions mocks base method. +func (m *MockStore) ArchiveUnusedTemplateVersions(arg0 context.Context, arg1 database.ArchiveUnusedTemplateVersionsParams) ([]uuid.UUID, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "PruneUnusedTemplateVersions", arg0, arg1) + ret := m.ctrl.Call(m, "ArchiveUnusedTemplateVersions", arg0, arg1) ret0, _ := ret[0].([]uuid.UUID) ret1, _ := ret[1].(error) return ret0, ret1 } -// PruneUnusedTemplateVersions indicates an expected call of PruneUnusedTemplateVersions. -func (mr *MockStoreMockRecorder) PruneUnusedTemplateVersions(arg0, arg1 interface{}) *gomock.Call { +// ArchiveUnusedTemplateVersions indicates an expected call of ArchiveUnusedTemplateVersions. +func (mr *MockStoreMockRecorder) ArchiveUnusedTemplateVersions(arg0, arg1 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PruneUnusedTemplateVersions", reflect.TypeOf((*MockStore)(nil).PruneUnusedTemplateVersions), arg0, arg1) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ArchiveUnusedTemplateVersions", reflect.TypeOf((*MockStore)(nil).ArchiveUnusedTemplateVersions), arg0, arg1) } // RegisterWorkspaceProxy mocks base method. diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 0b7f27d5174da..8b130d698cde4 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -671,7 +671,7 @@ CREATE TABLE template_versions ( created_by uuid NOT NULL, external_auth_providers text[], message character varying(1048576) DEFAULT ''::character varying NOT NULL, - deleted boolean DEFAULT false NOT NULL + archived boolean DEFAULT false NOT NULL ); COMMENT ON COLUMN template_versions.external_auth_providers IS 'IDs of External auth providers for a specific template version'; @@ -716,7 +716,7 @@ CREATE VIEW template_version_with_user AS template_versions.created_by, template_versions.external_auth_providers, template_versions.message, - template_versions.deleted, + template_versions.archived, COALESCE(visible_users.avatar_url, ''::text) AS created_by_avatar_url, COALESCE(visible_users.username, ''::text) AS created_by_username FROM (public.template_versions diff --git a/coderd/database/migrations/000162_delete_template_versions.down.sql b/coderd/database/migrations/000162_delete_template_versions.down.sql index 4dfa26d7117d4..25d413df1a159 100644 --- a/coderd/database/migrations/000162_delete_template_versions.down.sql +++ b/coderd/database/migrations/000162_delete_template_versions.down.sql @@ -4,7 +4,7 @@ BEGIN; DROP VIEW template_version_with_user; ALTER TABLE template_versions - DROP COLUMN deleted; + DROP COLUMN archived; -- Restore the old version of the template_version_with_user view. CREATE VIEW diff --git a/coderd/database/migrations/000162_delete_template_versions.up.sql b/coderd/database/migrations/000162_delete_template_versions.up.sql index 39a68c7975ba0..301d6312448e2 100644 --- a/coderd/database/migrations/000162_delete_template_versions.up.sql +++ b/coderd/database/migrations/000162_delete_template_versions.up.sql @@ -3,10 +3,9 @@ BEGIN; -- The view will be rebuilt with the new column DROP VIEW template_version_with_user; --- Hard deleting can cause reference issues with audit logs, --- so we only support soft deleting this resource at this time. +-- Archived template versions are not visible or usable by default. ALTER TABLE template_versions - ADD COLUMN deleted BOOLEAN NOT NULL DEFAULT FALSE; + ADD COLUMN archived BOOLEAN NOT NULL DEFAULT FALSE; -- Restore the old version of the template_version_with_user view. CREATE VIEW @@ -18,8 +17,8 @@ SELECT coalesce(visible_users.username, '') AS created_by_username FROM template_versions - LEFT JOIN - visible_users + LEFT JOIN + visible_users ON template_versions.created_by = visible_users.id; diff --git a/coderd/database/models.go b/coderd/database/models.go index e58cdcbf9debf..1709e16fc81f9 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -1883,7 +1883,7 @@ type TemplateVersion struct { CreatedBy uuid.UUID `db:"created_by" json:"created_by"` ExternalAuthProviders []string `db:"external_auth_providers" json:"external_auth_providers"` Message string `db:"message" json:"message"` - Deleted bool `db:"deleted" json:"deleted"` + Archived bool `db:"archived" json:"archived"` CreatedByAvatarURL sql.NullString `db:"created_by_avatar_url" json:"created_by_avatar_url"` CreatedByUsername string `db:"created_by_username" json:"created_by_username"` } @@ -1937,8 +1937,8 @@ type TemplateVersionTable struct { // IDs of External auth providers for a specific template version ExternalAuthProviders []string `db:"external_auth_providers" json:"external_auth_providers"` // Message describing the changes in this version of the template, similar to a Git commit message. Like a commit message, this should be a short, high-level description of the changes in this version of the template. This message is immutable and should not be updated after the fact. - Message string `db:"message" json:"message"` - Deleted bool `db:"deleted" json:"deleted"` + Message string `db:"message" json:"message"` + Archived bool `db:"archived" json:"archived"` } type TemplateVersionVariable struct { diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 98c4c483978b7..7b106ee0c5b29 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -33,6 +33,13 @@ type sqlcQuerier interface { ActivityBumpWorkspace(ctx context.Context, workspaceID uuid.UUID) error // AllUserIDs returns all UserIDs regardless of user status or deletion. AllUserIDs(ctx context.Context) ([]uuid.UUID, error) + // Archiving templates is a soft delete action, so is reversible. + // Archiving prevents the version from being used and discovered + // by listing. + // Only unused template versions will be archived, which are any versions not + // referenced by the latest build of a workspace. + // used_versions.transition != 'delete', + ArchiveUnusedTemplateVersions(ctx context.Context, arg ArchiveUnusedTemplateVersionsParams) ([]uuid.UUID, error) CleanTailnetCoordinators(ctx context.Context) error DeleteAPIKeyByID(ctx context.Context, id string) error DeleteAPIKeysByUserID(ctx context.Context, userID uuid.UUID) error @@ -274,13 +281,6 @@ type sqlcQuerier interface { InsertWorkspaceProxy(ctx context.Context, arg InsertWorkspaceProxyParams) (WorkspaceProxy, error) InsertWorkspaceResource(ctx context.Context, arg InsertWorkspaceResourceParams) (WorkspaceResource, error) InsertWorkspaceResourceMetadata(ctx context.Context, arg InsertWorkspaceResourceMetadataParams) ([]WorkspaceResourceMetadatum, error) - // Pruning templates is a soft delete action, so is technically reversible. - // Soft deleting prevents the version from being used and discovered - // by listing. - // Only unused template versions will be pruned, which are any versions not - // referenced by the latest build of a workspace. - // used_versions.transition != 'delete', - PruneUnusedTemplateVersions(ctx context.Context, arg PruneUnusedTemplateVersionsParams) ([]uuid.UUID, error) RegisterWorkspaceProxy(ctx context.Context, arg RegisterWorkspaceProxyParams) (WorkspaceProxy, error) RevokeDBCryptKey(ctx context.Context, activeKeyDigest string) error // Non blocking lock. Returns true if the lock was acquired, false otherwise. diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 8fd27be2d2094..a2d0f231d5114 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5308,9 +5308,103 @@ func (q *sqlQuerier) InsertTemplateVersionParameter(ctx context.Context, arg Ins return i, err } +const archiveUnusedTemplateVersions = `-- name: ArchiveUnusedTemplateVersions :many +UPDATE + template_versions +SET + archived = true, + updated_at = $1 +FROM + -- Delete all versions that are returned from this query. + ( + SELECT + scoped_template_versions.id + FROM + -- Scope an archive to a single template and ignore already archived template versions + (SELECT id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, archived FROM template_versions WHERE template_versions.template_id = $2 :: uuid AND archived = false) AS scoped_template_versions + LEFT JOIN + provisioner_jobs ON scoped_template_versions.job_id = provisioner_jobs.id + LEFT JOIN + templates ON scoped_template_versions.template_id = templates.id + WHERE + -- Actively used template versions (meaning the latest build is using + -- the version) are never archived. A "restart" command on the workspace, + -- even if failed, would use the version. So it cannot be archived until + -- the build is outdated. + NOT EXISTS ( + -- Return all "used" versions, where "used" is defined as being + -- used by a latest workspace build. + SELECT template_version_id FROM ( + SELECT + DISTINCT ON (workspace_id) template_version_id, transition + FROM + workspace_builds + ORDER BY workspace_id, build_number DESC + ) AS used_versions + WHERE + -- TODO: This is an issue for "deleted workspaces", since a deleted workspace + -- has a build with the transition "delete". This will prevent that template + -- version from ever being archived. We need a method to archive deleted workspaces. + scoped_template_versions.id = used_versions.template_version_id + ) + -- Also never archive the active template version + AND active_version_id != scoped_template_versions.id + AND CASE + -- Optionally, only archive versions that match a given + -- job status like 'failed'. + WHEN $3 :: provisioner_job_status IS NOT NULL THEN + provisioner_jobs.job_status = $3 :: provisioner_job_status + ELSE + true + END + -- Pending or running jobs should not be archived, as they are "in progress" + AND provisioner_jobs.job_status != 'running' + AND provisioner_jobs.job_status != 'pending' + ) AS archived_versions +WHERE + template_versions.id IN (archived_versions.id) +RETURNING template_versions.id +` + +type ArchiveUnusedTemplateVersionsParams struct { + UpdatedAt time.Time `db:"updated_at" json:"updated_at"` + TemplateID uuid.UUID `db:"template_id" json:"template_id"` + JobStatus NullProvisionerJobStatus `db:"job_status" json:"job_status"` +} + +// Archiving templates is a soft delete action, so is reversible. +// Archiving prevents the version from being used and discovered +// by listing. +// Only unused template versions will be archived, which are any versions not +// referenced by the latest build of a workspace. +// +// used_versions.transition != 'delete', +func (q *sqlQuerier) ArchiveUnusedTemplateVersions(ctx context.Context, arg ArchiveUnusedTemplateVersionsParams) ([]uuid.UUID, error) { + rows, err := q.db.QueryContext(ctx, archiveUnusedTemplateVersions, arg.UpdatedAt, arg.TemplateID, arg.JobStatus) + if err != nil { + return nil, err + } + defer rows.Close() + var items []uuid.UUID + for rows.Next() { + var id uuid.UUID + if err := rows.Scan(&id); err != nil { + return nil, err + } + items = append(items, id) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const getPreviousTemplateVersion = `-- name: GetPreviousTemplateVersion :one SELECT - id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, deleted, created_by_avatar_url, created_by_username + id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, archived, created_by_avatar_url, created_by_username FROM template_version_with_user AS template_versions WHERE @@ -5346,7 +5440,7 @@ func (q *sqlQuerier) GetPreviousTemplateVersion(ctx context.Context, arg GetPrev &i.CreatedBy, pq.Array(&i.ExternalAuthProviders), &i.Message, - &i.Deleted, + &i.Archived, &i.CreatedByAvatarURL, &i.CreatedByUsername, ) @@ -5355,7 +5449,7 @@ func (q *sqlQuerier) GetPreviousTemplateVersion(ctx context.Context, arg GetPrev const getTemplateVersionByID = `-- name: GetTemplateVersionByID :one SELECT - id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, deleted, created_by_avatar_url, created_by_username + id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, archived, created_by_avatar_url, created_by_username FROM template_version_with_user AS template_versions WHERE @@ -5377,7 +5471,7 @@ func (q *sqlQuerier) GetTemplateVersionByID(ctx context.Context, id uuid.UUID) ( &i.CreatedBy, pq.Array(&i.ExternalAuthProviders), &i.Message, - &i.Deleted, + &i.Archived, &i.CreatedByAvatarURL, &i.CreatedByUsername, ) @@ -5386,7 +5480,7 @@ func (q *sqlQuerier) GetTemplateVersionByID(ctx context.Context, id uuid.UUID) ( const getTemplateVersionByJobID = `-- name: GetTemplateVersionByJobID :one SELECT - id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, deleted, created_by_avatar_url, created_by_username + id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, archived, created_by_avatar_url, created_by_username FROM template_version_with_user AS template_versions WHERE @@ -5408,7 +5502,7 @@ func (q *sqlQuerier) GetTemplateVersionByJobID(ctx context.Context, jobID uuid.U &i.CreatedBy, pq.Array(&i.ExternalAuthProviders), &i.Message, - &i.Deleted, + &i.Archived, &i.CreatedByAvatarURL, &i.CreatedByUsername, ) @@ -5417,7 +5511,7 @@ func (q *sqlQuerier) GetTemplateVersionByJobID(ctx context.Context, jobID uuid.U const getTemplateVersionByTemplateIDAndName = `-- name: GetTemplateVersionByTemplateIDAndName :one SELECT - id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, deleted, created_by_avatar_url, created_by_username + id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, archived, created_by_avatar_url, created_by_username FROM template_version_with_user AS template_versions WHERE @@ -5445,7 +5539,7 @@ func (q *sqlQuerier) GetTemplateVersionByTemplateIDAndName(ctx context.Context, &i.CreatedBy, pq.Array(&i.ExternalAuthProviders), &i.Message, - &i.Deleted, + &i.Archived, &i.CreatedByAvatarURL, &i.CreatedByUsername, ) @@ -5454,7 +5548,7 @@ func (q *sqlQuerier) GetTemplateVersionByTemplateIDAndName(ctx context.Context, const getTemplateVersionsByIDs = `-- name: GetTemplateVersionsByIDs :many SELECT - id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, deleted, created_by_avatar_url, created_by_username + id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, archived, created_by_avatar_url, created_by_username FROM template_version_with_user AS template_versions WHERE @@ -5482,7 +5576,7 @@ func (q *sqlQuerier) GetTemplateVersionsByIDs(ctx context.Context, ids []uuid.UU &i.CreatedBy, pq.Array(&i.ExternalAuthProviders), &i.Message, - &i.Deleted, + &i.Archived, &i.CreatedByAvatarURL, &i.CreatedByUsername, ); err != nil { @@ -5501,7 +5595,7 @@ func (q *sqlQuerier) GetTemplateVersionsByIDs(ctx context.Context, ids []uuid.UU const getTemplateVersionsByTemplateID = `-- name: GetTemplateVersionsByTemplateID :many SELECT - id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, deleted, created_by_avatar_url, created_by_username + id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, archived, created_by_avatar_url, created_by_username FROM template_version_with_user AS template_versions WHERE @@ -5509,9 +5603,9 @@ WHERE AND CASE -- If no filter is provided, default to returning ALL template versions. -- The called should always provide a filter if they want to omit - -- deleted versions. + -- archived versions. WHEN $2 :: boolean IS NULL THEN true - ELSE template_versions.deleted = $2 :: boolean + ELSE template_versions.archived = $2 :: boolean END AND CASE -- This allows using the last element on a page as effectively a cursor. @@ -5543,7 +5637,7 @@ LIMIT type GetTemplateVersionsByTemplateIDParams struct { TemplateID uuid.UUID `db:"template_id" json:"template_id"` - Deleted sql.NullBool `db:"deleted" json:"deleted"` + Archived sql.NullBool `db:"archived" json:"archived"` AfterID uuid.UUID `db:"after_id" json:"after_id"` OffsetOpt int32 `db:"offset_opt" json:"offset_opt"` LimitOpt int32 `db:"limit_opt" json:"limit_opt"` @@ -5552,7 +5646,7 @@ type GetTemplateVersionsByTemplateIDParams struct { func (q *sqlQuerier) GetTemplateVersionsByTemplateID(ctx context.Context, arg GetTemplateVersionsByTemplateIDParams) ([]TemplateVersion, error) { rows, err := q.db.QueryContext(ctx, getTemplateVersionsByTemplateID, arg.TemplateID, - arg.Deleted, + arg.Archived, arg.AfterID, arg.OffsetOpt, arg.LimitOpt, @@ -5576,7 +5670,7 @@ func (q *sqlQuerier) GetTemplateVersionsByTemplateID(ctx context.Context, arg Ge &i.CreatedBy, pq.Array(&i.ExternalAuthProviders), &i.Message, - &i.Deleted, + &i.Archived, &i.CreatedByAvatarURL, &i.CreatedByUsername, ); err != nil { @@ -5594,7 +5688,7 @@ func (q *sqlQuerier) GetTemplateVersionsByTemplateID(ctx context.Context, arg Ge } const getTemplateVersionsCreatedAfter = `-- name: GetTemplateVersionsCreatedAfter :many -SELECT id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, deleted, created_by_avatar_url, created_by_username FROM template_version_with_user AS template_versions WHERE created_at > $1 +SELECT id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, archived, created_by_avatar_url, created_by_username FROM template_version_with_user AS template_versions WHERE created_at > $1 ` func (q *sqlQuerier) GetTemplateVersionsCreatedAfter(ctx context.Context, createdAt time.Time) ([]TemplateVersion, error) { @@ -5618,7 +5712,7 @@ func (q *sqlQuerier) GetTemplateVersionsCreatedAfter(ctx context.Context, create &i.CreatedBy, pq.Array(&i.ExternalAuthProviders), &i.Message, - &i.Deleted, + &i.Archived, &i.CreatedByAvatarURL, &i.CreatedByUsername, ); err != nil { @@ -5682,100 +5776,6 @@ func (q *sqlQuerier) InsertTemplateVersion(ctx context.Context, arg InsertTempla return err } -const pruneUnusedTemplateVersions = `-- name: PruneUnusedTemplateVersions :many -UPDATE - template_versions -SET - deleted = true, - updated_at = $1 -FROM - -- Delete all versions that are returned from this query. - ( - SELECT - scoped_template_versions.id - FROM - -- Scope a prune to a single template and ignore already deleted template versions - (SELECT id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, deleted FROM template_versions WHERE template_versions.template_id = $2 :: uuid AND deleted = false) AS scoped_template_versions - LEFT JOIN - provisioner_jobs ON scoped_template_versions.job_id = provisioner_jobs.id - LEFT JOIN - templates ON scoped_template_versions.template_id = templates.id - WHERE - -- Actively used template versions (meaning the latest build is using - -- the version) are never pruned. A "restart" command on the workspace, - -- even if failed, would use the version. So it cannot be pruned until - -- the build is outdated. - NOT EXISTS ( - -- Return all "used" versions, where "used" is defined as being - -- used by a latest workspace build. - SELECT template_version_id FROM ( - SELECT - DISTINCT ON (workspace_id) template_version_id, transition - FROM - workspace_builds - ORDER BY workspace_id, build_number DESC - ) AS used_versions - WHERE - -- TODO: This is an issue for "deleted workspaces", since a deleted workspace - -- has a build with the transition "delete". This will prevent that template - -- version from ever being pruned. We need a method to prune deleted workspaces. - scoped_template_versions.id = used_versions.template_version_id - ) - -- Also never delete the active template version - AND active_version_id != scoped_template_versions.id - AND CASE - -- Optionally, only prune versions that match a given - -- job status like 'failed'. - WHEN $3 :: provisioner_job_status IS NOT NULL THEN - provisioner_jobs.job_status = $3 :: provisioner_job_status - ELSE - true - END - -- Pending or running jobs should not be pruned, as they are "in progress" - AND provisioner_jobs.job_status != 'running' - AND provisioner_jobs.job_status != 'pending' - ) AS deleted_versions -WHERE - template_versions.id IN (deleted_versions.id) -RETURNING template_versions.id -` - -type PruneUnusedTemplateVersionsParams struct { - UpdatedAt time.Time `db:"updated_at" json:"updated_at"` - TemplateID uuid.UUID `db:"template_id" json:"template_id"` - JobStatus NullProvisionerJobStatus `db:"job_status" json:"job_status"` -} - -// Pruning templates is a soft delete action, so is technically reversible. -// Soft deleting prevents the version from being used and discovered -// by listing. -// Only unused template versions will be pruned, which are any versions not -// referenced by the latest build of a workspace. -// -// used_versions.transition != 'delete', -func (q *sqlQuerier) PruneUnusedTemplateVersions(ctx context.Context, arg PruneUnusedTemplateVersionsParams) ([]uuid.UUID, error) { - rows, err := q.db.QueryContext(ctx, pruneUnusedTemplateVersions, arg.UpdatedAt, arg.TemplateID, arg.JobStatus) - if err != nil { - return nil, err - } - defer rows.Close() - var items []uuid.UUID - for rows.Next() { - var id uuid.UUID - if err := rows.Scan(&id); err != nil { - return nil, err - } - items = append(items, id) - } - if err := rows.Close(); err != nil { - return nil, err - } - if err := rows.Err(); err != nil { - return nil, err - } - return items, nil -} - const updateTemplateVersionByID = `-- name: UpdateTemplateVersionByID :exec UPDATE template_versions diff --git a/coderd/database/queries/templateversions.sql b/coderd/database/queries/templateversions.sql index e1c78bc4b7236..b5b51d1af22db 100644 --- a/coderd/database/queries/templateversions.sql +++ b/coderd/database/queries/templateversions.sql @@ -8,9 +8,9 @@ WHERE AND CASE -- If no filter is provided, default to returning ALL template versions. -- The called should always provide a filter if they want to omit - -- deleted versions. - WHEN sqlc.narg('deleted') :: boolean IS NULL THEN true - ELSE template_versions.deleted = sqlc.narg('deleted') :: boolean + -- archived versions. + WHEN sqlc.narg('archived') :: boolean IS NULL THEN true + ELSE template_versions.archived = sqlc.narg('archived') :: boolean END AND CASE -- This allows using the last element on a page as effectively a cursor. @@ -138,16 +138,16 @@ ORDER BY created_at DESC LIMIT 1; --- name: PruneUnusedTemplateVersions :many --- Pruning templates is a soft delete action, so is technically reversible. --- Soft deleting prevents the version from being used and discovered +-- name: ArchiveUnusedTemplateVersions :many +-- Archiving templates is a soft delete action, so is reversible. +-- Archiving prevents the version from being used and discovered -- by listing. --- Only unused template versions will be pruned, which are any versions not +-- Only unused template versions will be archived, which are any versions not -- referenced by the latest build of a workspace. UPDATE template_versions SET - deleted = true, + archived = true, updated_at = sqlc.arg('updated_at') FROM -- Delete all versions that are returned from this query. @@ -155,16 +155,16 @@ FROM SELECT scoped_template_versions.id FROM - -- Scope a prune to a single template and ignore already deleted template versions - (SELECT * FROM template_versions WHERE template_versions.template_id = sqlc.arg('template_id') :: uuid AND deleted = false) AS scoped_template_versions + -- Scope an archive to a single template and ignore already archived template versions + (SELECT * FROM template_versions WHERE template_versions.template_id = sqlc.arg('template_id') :: uuid AND archived = false) AS scoped_template_versions LEFT JOIN provisioner_jobs ON scoped_template_versions.job_id = provisioner_jobs.id LEFT JOIN templates ON scoped_template_versions.template_id = templates.id WHERE -- Actively used template versions (meaning the latest build is using - -- the version) are never pruned. A "restart" command on the workspace, - -- even if failed, would use the version. So it cannot be pruned until + -- the version) are never archived. A "restart" command on the workspace, + -- even if failed, would use the version. So it cannot be archived until -- the build is outdated. NOT EXISTS ( -- Return all "used" versions, where "used" is defined as being @@ -179,24 +179,24 @@ FROM WHERE -- TODO: This is an issue for "deleted workspaces", since a deleted workspace -- has a build with the transition "delete". This will prevent that template - -- version from ever being pruned. We need a method to prune deleted workspaces. + -- version from ever being archived. We need a method to archive deleted workspaces. -- used_versions.transition != 'delete', scoped_template_versions.id = used_versions.template_version_id ) - -- Also never delete the active template version + -- Also never archive the active template version AND active_version_id != scoped_template_versions.id AND CASE - -- Optionally, only prune versions that match a given + -- Optionally, only archive versions that match a given -- job status like 'failed'. WHEN sqlc.narg('job_status') :: provisioner_job_status IS NOT NULL THEN provisioner_jobs.job_status = sqlc.narg('job_status') :: provisioner_job_status ELSE true END - -- Pending or running jobs should not be pruned, as they are "in progress" + -- Pending or running jobs should not be archived, as they are "in progress" AND provisioner_jobs.job_status != 'running' AND provisioner_jobs.job_status != 'pending' - ) AS deleted_versions + ) AS archived_versions WHERE - template_versions.id IN (deleted_versions.id) + template_versions.id IN (archived_versions.id) RETURNING template_versions.id; diff --git a/coderd/templateversions.go b/coderd/templateversions.go index f8ee896950e98..0aa641d9d9d39 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -996,16 +996,16 @@ func (api *API) previousTemplateVersionByOrganizationTemplateAndName(rw http.Res httpapi.Write(ctx, rw, http.StatusOK, convertTemplateVersion(previousTemplateVersion, convertProvisionerJob(jobs[0]), nil)) } -// @Summary Prune template unused versions by template id -// @ID prune-template-versions-by-template-id +// @Summary Archive template unused versions by template id +// @ID archive-template-versions-by-template-id // @Security CoderSessionToken // @Accept json // @Produce json // @Tags Templates // @Param template path string true "Template ID" format(uuid) // @Success 200 {object} codersdk.Response -// @Router /templates/{template}/versions/prune [delete] -func (api *API) deletePruneTemplateVersions(rw http.ResponseWriter, r *http.Request) { +// @Router /templates/{template}/versions/archive [post] +func (api *API) postArchiveTemplateVersions(rw http.ResponseWriter, r *http.Request) { var ( ctx = r.Context() template = httpmw.TemplateParam(r) @@ -1020,7 +1020,7 @@ func (api *API) deletePruneTemplateVersions(rw http.ResponseWriter, r *http.Requ defer commitAudit() aReq.Old = template - var req codersdk.PruneTemplateVersionsRequest + var req codersdk.ArchiveTemplateVersionsRequest if !httpapi.Read(ctx, rw, r, &req) { return } @@ -1033,7 +1033,7 @@ func (api *API) deletePruneTemplateVersions(rw http.ResponseWriter, r *http.Requ status = database.NullProvisionerJobStatus{} } - deleted, err := api.Database.PruneUnusedTemplateVersions(ctx, database.PruneUnusedTemplateVersionsParams{ + deleted, err := api.Database.ArchiveUnusedTemplateVersions(ctx, database.ArchiveUnusedTemplateVersionsParams{ UpdatedAt: dbtime.Now(), TemplateID: template.ID, JobStatus: status, @@ -1053,9 +1053,9 @@ func (api *API) deletePruneTemplateVersions(rw http.ResponseWriter, r *http.Requ return } - httpapi.Write(ctx, rw, http.StatusOK, codersdk.PruneTemplateVersionsResponse{ - TemplateID: template.ID, - DeletedIDs: deleted, + httpapi.Write(ctx, rw, http.StatusOK, codersdk.ArchiveTemplateVersionsResponse{ + TemplateID: template.ID, + ArchivedIDs: deleted, }) } @@ -1123,9 +1123,9 @@ func (api *API) patchActiveTemplateVersion(rw http.ResponseWriter, r *http.Reque }) return } - if version.Deleted { + if version.Archived { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "The provided template version is deleted.", + Message: "The provided template version is archived.", }) return } diff --git a/coderd/templateversions_test.go b/coderd/templateversions_test.go index 12d0581ba8d4b..c41d1a78d293f 100644 --- a/coderd/templateversions_test.go +++ b/coderd/templateversions_test.go @@ -1516,7 +1516,7 @@ func TestTemplateVersionParameters_Order(t *testing.T) { require.Equal(t, thirdParameterName, templateRichParameters[4].Name) } -func TestTemplatePruneVersions(t *testing.T) { +func TestTemplateArchiveVersions(t *testing.T) { t.Parallel() client, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) @@ -1524,7 +1524,7 @@ func TestTemplatePruneVersions(t *testing.T) { var _ = api var totalVersions int - // Create a template to prune + // Create a template to archive initialVersion := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) totalVersions++ template := coderdtest.CreateTemplate(t, client, user.OrganizationID, initialVersion.ID) @@ -1584,10 +1584,10 @@ func TestTemplatePruneVersions(t *testing.T) { require.NoError(t, err, "fetch all versions") require.Len(t, versions, totalVersions, "total versions") - // Prune failed versions - deleteFailed, err := client.PruneTemplateVersions(ctx, template.ID, false) - require.NoError(t, err, "prune failed versions") - require.ElementsMatch(t, deleteFailed.DeletedIDs, allFailed, "all failed versions deleted") + // Archive failed versions + deleteFailed, err := client.ArchiveTemplateVersions(ctx, template.ID, false) + require.NoError(t, err, "archive failed versions") + require.ElementsMatch(t, deleteFailed.ArchivedIDs, allFailed, "all failed versions deleted") remaining, err := client.TemplateVersionsByTemplate(ctx, codersdk.TemplateVersionsByTemplateRequest{ TemplateID: template.ID, @@ -1599,9 +1599,9 @@ func TestTemplatePruneVersions(t *testing.T) { require.Len(t, remaining, totalVersions-len(allFailed), "remaining non-failed versions") // Try pruning "All" unused templates - deleted, err := client.PruneTemplateVersions(ctx, template.ID, true) - require.NoError(t, err, "prune versions") - require.ElementsMatch(t, deleted.DeletedIDs, expDeleted, "all expected versions deleted") + deleted, err := client.ArchiveTemplateVersions(ctx, template.ID, true) + require.NoError(t, err, "archive versions") + require.ElementsMatch(t, deleted.ArchivedIDs, expDeleted, "all expected versions deleted") remaining, err = client.TemplateVersionsByTemplate(ctx, codersdk.TemplateVersionsByTemplateRequest{ TemplateID: template.ID, diff --git a/codersdk/templates.go b/codersdk/templates.go index 04ad9998d6565..61e88bcade56b 100644 --- a/codersdk/templates.go +++ b/codersdk/templates.go @@ -136,15 +136,15 @@ type ( } ) -type PruneTemplateVersionsRequest struct { - // By default, only failed versions are pruned. Set this to true - // to prune all unused versions regardless of job status. +type ArchiveTemplateVersionsRequest struct { + // By default, only failed versions are archived. Set this to true + // to archive all unused versions regardless of job status. All bool `json:"all"` } -type PruneTemplateVersionsResponse struct { - TemplateID uuid.UUID `json:"template_id" format:"uuid"` - DeletedIDs []uuid.UUID `json:"deleted_ids"` +type ArchiveTemplateVersionsResponse struct { + TemplateID uuid.UUID `json:"template_id" format:"uuid"` + ArchivedIDs []uuid.UUID `json:"archived_ids"` } type TemplateRole string @@ -238,21 +238,21 @@ func (c *Client) Template(ctx context.Context, template uuid.UUID) (Template, er return resp, json.NewDecoder(res.Body).Decode(&resp) } -func (c *Client) PruneTemplateVersions(ctx context.Context, template uuid.UUID, all bool) (PruneTemplateVersionsResponse, error) { - res, err := c.Request(ctx, http.MethodDelete, - fmt.Sprintf("/api/v2/templates/%s/versions/prune", template), - PruneTemplateVersionsRequest{ +func (c *Client) ArchiveTemplateVersions(ctx context.Context, template uuid.UUID, all bool) (ArchiveTemplateVersionsResponse, error) { + res, err := c.Request(ctx, http.MethodPost, + fmt.Sprintf("/api/v2/templates/%s/versions/archive", template), + ArchiveTemplateVersionsRequest{ All: all, }, ) if err != nil { - return PruneTemplateVersionsResponse{}, err + return ArchiveTemplateVersionsResponse{}, err } defer res.Body.Close() if res.StatusCode != http.StatusOK { - return PruneTemplateVersionsResponse{}, ReadBodyAsError(res) + return ArchiveTemplateVersionsResponse{}, ReadBodyAsError(res) } - var resp PruneTemplateVersionsResponse + var resp ArchiveTemplateVersionsResponse return resp, json.NewDecoder(res.Body).Decode(&resp) } From 9a416c3318d36447bbe4f70099467f1aa8a8f756 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 9 Oct 2023 13:39:34 -0500 Subject: [PATCH 09/37] Rename to "archive" --- coderd/apidoc/docs.go | 41 ++++++++++++++ coderd/apidoc/swagger.json | 35 ++++++++++++ coderd/database/dbfake/dbfake.go | 8 +-- coderd/database/dbmock/dbmock.go | 30 +++++------ ...000162_archive_template_versions.down.sql} | 0 ...> 000162_archive_template_versions.up.sql} | 0 coderd/database/queries.sql.go | 2 +- coderd/database/queries/templateversions.sql | 2 +- coderd/httpapi/queryparams.go | 11 ++++ coderd/httpapi/queryparams_test.go | 42 +++++++++++++++ coderd/templates.go | 6 +-- coderd/templateversions.go | 31 ++++++++--- coderd/templateversions_test.go | 16 +++--- coderd/workspaces.go | 6 +-- codersdk/templates.go | 9 +++- codersdk/templateversions.go | 2 +- docs/admin/audit-logs.md | 2 +- docs/api/schemas.md | 2 + docs/api/templates.md | 53 +++++++++++++++++++ docs/cli/templates.md | 1 + docs/cli/templates_archive.md | 29 ++++++++++ docs/manifest.json | 5 ++ enterprise/audit/table.go | 2 +- site/src/api/typesGenerated.ts | 25 ++++----- site/src/testHelpers/entities.ts | 6 +-- 25 files changed, 304 insertions(+), 62 deletions(-) rename coderd/database/migrations/{000162_delete_template_versions.down.sql => 000162_archive_template_versions.down.sql} (100%) rename coderd/database/migrations/{000162_delete_template_versions.up.sql => 000162_archive_template_versions.up.sql} (100%) create mode 100644 docs/cli/templates_archive.md diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 5913dd9eab3da..5906f0d6392d2 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -2365,6 +2365,44 @@ const docTemplate = `{ } } }, + "/templates/{template}/versions/archive": { + "post": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "Templates" + ], + "summary": "Archive template unused versions by template id", + "operationId": "archive-template-versions-by-template-id", + "parameters": [ + { + "type": "string", + "format": "uuid", + "description": "Template ID", + "name": "template", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.Response" + } + } + } + } + }, "/templates/{template}/versions/{templateversionname}": { "get": { "security": [ @@ -9957,6 +9995,9 @@ const docTemplate = `{ "codersdk.TemplateVersion": { "type": "object", "properties": { + "archived": { + "type": "boolean" + }, "created_at": { "type": "string", "format": "date-time" diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 37b3c5ea66b57..bde85cc0423d5 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -2067,6 +2067,38 @@ } } }, + "/templates/{template}/versions/archive": { + "post": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "consumes": ["application/json"], + "produces": ["application/json"], + "tags": ["Templates"], + "summary": "Archive template unused versions by template id", + "operationId": "archive-template-versions-by-template-id", + "parameters": [ + { + "type": "string", + "format": "uuid", + "description": "Template ID", + "name": "template", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.Response" + } + } + } + } + }, "/templates/{template}/versions/{templateversionname}": { "get": { "security": [ @@ -9006,6 +9038,9 @@ "codersdk.TemplateVersion": { "type": "object", "properties": { + "archived": { + "type": "boolean" + }, "created_at": { "type": "string", "format": "date-time" diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index da91b75ed2715..2aae16e868637 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -878,7 +878,7 @@ func (q *FakeQuerier) ArchiveUnusedTemplateVersions(_ context.Context, arg datab usedVersions[tpl.ActiveVersionID] = true } - var deleted []uuid.UUID + var archived []uuid.UUID for i, v := range q.templateVersions { if v.Archived { continue @@ -905,11 +905,11 @@ func (q *FakeQuerier) ArchiveUnusedTemplateVersions(_ context.Context, arg datab v.Archived = true q.templateVersions[i] = v - deleted = append(deleted, v.ID) + archived = append(archived, v.ID) } } - return deleted, nil + return archived, nil } func (*FakeQuerier) CleanTailnetCoordinators(_ context.Context) error { @@ -2825,7 +2825,7 @@ func (q *FakeQuerier) GetTemplateVersionsByTemplateID(_ context.Context, arg dat if templateVersion.TemplateID.UUID != arg.TemplateID { continue } - if arg.Deleted.Valid && arg.Deleted.Bool != templateVersion.Deleted { + if arg.Archived.Valid && arg.Archived.Bool != templateVersion.Archived { continue } version = append(version, q.templateVersionWithUserNoLock(templateVersion)) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 389d0fff22dd2..60380a1b8404d 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -97,6 +97,21 @@ func (mr *MockStoreMockRecorder) AllUserIDs(arg0 interface{}) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AllUserIDs", reflect.TypeOf((*MockStore)(nil).AllUserIDs), arg0) } +// ArchiveUnusedTemplateVersions mocks base method. +func (m *MockStore) ArchiveUnusedTemplateVersions(arg0 context.Context, arg1 database.ArchiveUnusedTemplateVersionsParams) ([]uuid.UUID, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ArchiveUnusedTemplateVersions", arg0, arg1) + ret0, _ := ret[0].([]uuid.UUID) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ArchiveUnusedTemplateVersions indicates an expected call of ArchiveUnusedTemplateVersions. +func (mr *MockStoreMockRecorder) ArchiveUnusedTemplateVersions(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ArchiveUnusedTemplateVersions", reflect.TypeOf((*MockStore)(nil).ArchiveUnusedTemplateVersions), arg0, arg1) +} + // CleanTailnetCoordinators mocks base method. func (m *MockStore) CleanTailnetCoordinators(arg0 context.Context) error { m.ctrl.T.Helper() @@ -2980,21 +2995,6 @@ func (mr *MockStoreMockRecorder) Ping(arg0 interface{}) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Ping", reflect.TypeOf((*MockStore)(nil).Ping), arg0) } -// ArchiveUnusedTemplateVersions mocks base method. -func (m *MockStore) ArchiveUnusedTemplateVersions(arg0 context.Context, arg1 database.ArchiveUnusedTemplateVersionsParams) ([]uuid.UUID, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ArchiveUnusedTemplateVersions", arg0, arg1) - ret0, _ := ret[0].([]uuid.UUID) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// ArchiveUnusedTemplateVersions indicates an expected call of ArchiveUnusedTemplateVersions. -func (mr *MockStoreMockRecorder) ArchiveUnusedTemplateVersions(arg0, arg1 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ArchiveUnusedTemplateVersions", reflect.TypeOf((*MockStore)(nil).ArchiveUnusedTemplateVersions), arg0, arg1) -} - // RegisterWorkspaceProxy mocks base method. func (m *MockStore) RegisterWorkspaceProxy(arg0 context.Context, arg1 database.RegisterWorkspaceProxyParams) (database.WorkspaceProxy, error) { m.ctrl.T.Helper() diff --git a/coderd/database/migrations/000162_delete_template_versions.down.sql b/coderd/database/migrations/000162_archive_template_versions.down.sql similarity index 100% rename from coderd/database/migrations/000162_delete_template_versions.down.sql rename to coderd/database/migrations/000162_archive_template_versions.down.sql diff --git a/coderd/database/migrations/000162_delete_template_versions.up.sql b/coderd/database/migrations/000162_archive_template_versions.up.sql similarity index 100% rename from coderd/database/migrations/000162_delete_template_versions.up.sql rename to coderd/database/migrations/000162_archive_template_versions.up.sql diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index a2d0f231d5114..deb2b85c209dd 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5315,7 +5315,7 @@ SET archived = true, updated_at = $1 FROM - -- Delete all versions that are returned from this query. + -- Archive all versions that are returned from this query. ( SELECT scoped_template_versions.id diff --git a/coderd/database/queries/templateversions.sql b/coderd/database/queries/templateversions.sql index b5b51d1af22db..cf1e21d095eee 100644 --- a/coderd/database/queries/templateversions.sql +++ b/coderd/database/queries/templateversions.sql @@ -150,7 +150,7 @@ SET archived = true, updated_at = sqlc.arg('updated_at') FROM - -- Delete all versions that are returned from this query. + -- Archive all versions that are returned from this query. ( SELECT scoped_template_versions.id diff --git a/coderd/httpapi/queryparams.go b/coderd/httpapi/queryparams.go index 77991ac019075..9b7daf2310900 100644 --- a/coderd/httpapi/queryparams.go +++ b/coderd/httpapi/queryparams.go @@ -79,6 +79,17 @@ func (p *QueryParamParser) Int(vals url.Values, def int, queryParam string) int return v } +func (p *QueryParamParser) Boolean(vals url.Values, def bool, queryParam string) bool { + v, err := parseQueryParam(p, vals, strconv.ParseBool, def, queryParam) + if err != nil { + p.Errors = append(p.Errors, codersdk.ValidationError{ + Field: queryParam, + Detail: fmt.Sprintf("Query param %q must be a valid boolean (%s)", queryParam, err.Error()), + }) + } + return v +} + func (p *QueryParamParser) Required(queryParam string) *QueryParamParser { p.RequiredParams[queryParam] = true return p diff --git a/coderd/httpapi/queryparams_test.go b/coderd/httpapi/queryparams_test.go index da0dac4ad0aa0..f919b478dfd78 100644 --- a/coderd/httpapi/queryparams_test.go +++ b/coderd/httpapi/queryparams_test.go @@ -157,6 +157,48 @@ func TestParseQueryParams(t *testing.T) { testQueryParams(t, expParams, parser, parser.String) }) + t.Run("Boolean", func(t *testing.T) { + t.Parallel() + expParams := []queryParamTestCase[bool]{ + { + QueryParam: "valid_true", + Value: "true", + Expected: true, + }, + { + QueryParam: "casing", + Value: "True", + Expected: true, + }, + { + QueryParam: "all_caps", + Value: "TRUE", + Expected: true, + }, + { + QueryParam: "no_value_true_def", + NoSet: true, + Default: true, + Expected: true, + }, + { + QueryParam: "no_value", + NoSet: true, + Expected: false, + }, + + { + QueryParam: "invalid_boolean", + Value: "yes", + Expected: false, + ExpectedErrorContains: "must be a valid boolean", + }, + } + + parser := httpapi.NewQueryParamParser() + testQueryParams(t, expParams, parser, parser.Boolean) + }) + t.Run("Int", func(t *testing.T) { t.Parallel() expParams := []queryParamTestCase[int]{ diff --git a/coderd/templates.go b/coderd/templates.go index d63524a4f3bfb..a05017298d740 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -193,11 +193,11 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque }) return } - if templateVersion.Deleted { + if templateVersion.Archived { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: fmt.Sprintf("Template version %s is deleted.", createTemplate.VersionID), + Message: fmt.Sprintf("Template version %s is archived.", createTemplate.VersionID), Validations: []codersdk.ValidationError{ - {Field: "template_version_id", Detail: "Template version is deleted"}, + {Field: "template_version_id", Detail: "Template version is archived"}, }, }) return diff --git a/coderd/templateversions.go b/coderd/templateversions.go index 0aa641d9d9d39..a90e4d8390c9a 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -717,6 +717,17 @@ func (api *API) templateVersionsByTemplate(rw http.ResponseWriter, r *http.Reque return } + // If this throws an error, the boolean is false. Which is the default we want. + parser := httpapi.NewQueryParamParser() + includeArchived := parser.Boolean(r.URL.Query(), false, "include_archived") + if len(parser.Errors) > 0 { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Invalid query parameters.", + Validations: parser.Errors, + }) + return + } + var err error apiVersions := []codersdk.TemplateVersion{} err = api.Database.InTx(func(store database.Store) error { @@ -738,16 +749,21 @@ func (api *API) templateVersionsByTemplate(rw http.ResponseWriter, r *http.Reque } } + // Exclude archived templates versions + archiveFilter := sql.NullBool{ + Bool: false, + Valid: true, + } + if includeArchived { + archiveFilter = sql.NullBool{Valid: false} + } + versions, err := store.GetTemplateVersionsByTemplateID(ctx, database.GetTemplateVersionsByTemplateIDParams{ TemplateID: template.ID, AfterID: paginationParams.AfterID, LimitOpt: int32(paginationParams.Limit), OffsetOpt: int32(paginationParams.Offset), - // Exclude deleted templates versions - Deleted: sql.NullBool{ - Bool: false, - Valid: true, - }, + Archived: archiveFilter, }) if errors.Is(err, sql.ErrNoRows) { httpapi.Write(ctx, rw, http.StatusOK, apiVersions) @@ -1033,7 +1049,7 @@ func (api *API) postArchiveTemplateVersions(rw http.ResponseWriter, r *http.Requ status = database.NullProvisionerJobStatus{} } - deleted, err := api.Database.ArchiveUnusedTemplateVersions(ctx, database.ArchiveUnusedTemplateVersionsParams{ + archived, err := api.Database.ArchiveUnusedTemplateVersions(ctx, database.ArchiveUnusedTemplateVersionsParams{ UpdatedAt: dbtime.Now(), TemplateID: template.ID, JobStatus: status, @@ -1055,7 +1071,7 @@ func (api *API) postArchiveTemplateVersions(rw http.ResponseWriter, r *http.Requ httpapi.Write(ctx, rw, http.StatusOK, codersdk.ArchiveTemplateVersionsResponse{ TemplateID: template.ID, - ArchivedIDs: deleted, + ArchivedIDs: archived, }) } @@ -1478,6 +1494,7 @@ func convertTemplateVersion(version database.TemplateVersion, job codersdk.Provi Username: version.CreatedByUsername, AvatarURL: version.CreatedByAvatarURL.String, }, + Archived: version.Archived, Warnings: warnings, } } diff --git a/coderd/templateversions_test.go b/coderd/templateversions_test.go index c41d1a78d293f..ce93adcabc7c1 100644 --- a/coderd/templateversions_test.go +++ b/coderd/templateversions_test.go @@ -1530,7 +1530,7 @@ func TestTemplateArchiveVersions(t *testing.T) { template := coderdtest.CreateTemplate(t, client, user.OrganizationID, initialVersion.ID) allFailed := make([]uuid.UUID, 0) - expDeleted := make([]uuid.UUID, 0) + expArchived := make([]uuid.UUID, 0) // create some failed versions for i := 0; i < 2; i++ { failed := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ @@ -1553,7 +1553,7 @@ func TestTemplateArchiveVersions(t *testing.T) { }, func(req *codersdk.CreateTemplateVersionRequest) { req.TemplateID = template.ID }) - expDeleted = append(expDeleted, unused.ID) + expArchived = append(expArchived, unused.ID) totalVersions++ } @@ -1585,9 +1585,9 @@ func TestTemplateArchiveVersions(t *testing.T) { require.Len(t, versions, totalVersions, "total versions") // Archive failed versions - deleteFailed, err := client.ArchiveTemplateVersions(ctx, template.ID, false) + archiveFailed, err := client.ArchiveTemplateVersions(ctx, template.ID, false) require.NoError(t, err, "archive failed versions") - require.ElementsMatch(t, deleteFailed.ArchivedIDs, allFailed, "all failed versions deleted") + require.ElementsMatch(t, archiveFailed.ArchivedIDs, allFailed, "all failed versions archived") remaining, err := client.TemplateVersionsByTemplate(ctx, codersdk.TemplateVersionsByTemplateRequest{ TemplateID: template.ID, @@ -1598,10 +1598,10 @@ func TestTemplateArchiveVersions(t *testing.T) { require.NoError(t, err, "fetch all non-failed versions") require.Len(t, remaining, totalVersions-len(allFailed), "remaining non-failed versions") - // Try pruning "All" unused templates - deleted, err := client.ArchiveTemplateVersions(ctx, template.ID, true) + // Try archiving "All" unused templates + archived, err := client.ArchiveTemplateVersions(ctx, template.ID, true) require.NoError(t, err, "archive versions") - require.ElementsMatch(t, deleted.ArchivedIDs, expDeleted, "all expected versions deleted") + require.ElementsMatch(t, archived.ArchivedIDs, expArchived, "all expected versions archived") remaining, err = client.TemplateVersionsByTemplate(ctx, codersdk.TemplateVersionsByTemplateRequest{ TemplateID: template.ID, @@ -1610,5 +1610,5 @@ func TestTemplateArchiveVersions(t *testing.T) { }, }) require.NoError(t, err, "fetch all versions") - require.Len(t, remaining, totalVersions-len(expDeleted)-len(allFailed), "remaining versions") + require.Len(t, remaining, totalVersions-len(expArchived)-len(allFailed), "remaining versions") } diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 16cfb4d777e84..be539b0d600b2 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -356,13 +356,13 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req }) return } - if templateVersion.Deleted { + if templateVersion.Archived { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Template version has been deleted.", + Message: "Archived template versions cannot be used to make a workspace.", Validations: []codersdk.ValidationError{ { Field: "template_version_id", - Detail: "template version deleted and unusable", + Detail: "template version archived", }, }, }) diff --git a/codersdk/templates.go b/codersdk/templates.go index 61e88bcade56b..38793b9f5fdf1 100644 --- a/codersdk/templates.go +++ b/codersdk/templates.go @@ -340,13 +340,18 @@ func (c *Client) UpdateActiveTemplateVersion(ctx context.Context, template uuid. // TemplateVersionsByTemplateRequest defines the request parameters for // TemplateVersionsByTemplate. type TemplateVersionsByTemplateRequest struct { - TemplateID uuid.UUID `json:"template_id" validate:"required" format:"uuid"` + TemplateID uuid.UUID `json:"template_id" validate:"required" format:"uuid"` + IncludeArchived bool `json:"include_archived"` Pagination } // TemplateVersionsByTemplate lists versions associated with a template. func (c *Client) TemplateVersionsByTemplate(ctx context.Context, req TemplateVersionsByTemplateRequest) ([]TemplateVersion, error) { - res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/templates/%s/versions", req.TemplateID), nil, req.Pagination.asRequestOption()) + u := fmt.Sprintf("/api/v2/templates/%s/versions", req.TemplateID) + if req.IncludeArchived { + u += "?include_archived=true" + } + res, err := c.Request(ctx, http.MethodGet, u, nil, req.Pagination.asRequestOption()) if err != nil { return nil, err } diff --git a/codersdk/templateversions.go b/codersdk/templateversions.go index 77384b85d5b26..4263fbaf06975 100644 --- a/codersdk/templateversions.go +++ b/codersdk/templateversions.go @@ -29,7 +29,7 @@ type TemplateVersion struct { Job ProvisionerJob `json:"job"` Readme string `json:"readme"` CreatedBy MinimalUser `json:"created_by"` - Deleted bool `json:"deleted"` + Archived bool `json:"archived"` Warnings []TemplateVersionWarning `json:"warnings,omitempty" enums:"DEPRECATED_PARAMETERS"` } diff --git a/docs/admin/audit-logs.md b/docs/admin/audit-logs.md index 69198124f9792..45fc94367e7a9 100644 --- a/docs/admin/audit-logs.md +++ b/docs/admin/audit-logs.md @@ -16,7 +16,7 @@ We track the following resources: | GitSSHKey
create |
FieldTracked
created_atfalse
private_keytrue
public_keytrue
updated_atfalse
user_idtrue
| | License
create, delete |
FieldTracked
exptrue
idfalse
jwtfalse
uploaded_attrue
uuidtrue
| | Template
write, delete |
FieldTracked
active_version_idtrue
allow_user_autostarttrue
allow_user_autostoptrue
allow_user_cancel_workspace_jobstrue
autostop_requirement_days_of_weektrue
autostop_requirement_weekstrue
created_atfalse
created_bytrue
created_by_avatar_urlfalse
created_by_usernamefalse
default_ttltrue
deletedfalse
descriptiontrue
display_nametrue
failure_ttltrue
group_acltrue
icontrue
idtrue
max_ttltrue
nametrue
organization_idfalse
provisionertrue
time_til_dormanttrue
time_til_dormant_autodeletetrue
updated_atfalse
user_acltrue
| -| TemplateVersion
create, write, delete |
FieldTracked
created_atfalse
created_bytrue
created_by_avatar_urlfalse
created_by_usernamefalse
deletedtrue
external_auth_providersfalse
idtrue
job_idfalse
messagefalse
nametrue
organization_idfalse
readmetrue
template_idtrue
updated_atfalse
| +| TemplateVersion
create, write, delete |
FieldTracked
archivedtrue
created_atfalse
created_bytrue
created_by_avatar_urlfalse
created_by_usernamefalse
external_auth_providersfalse
idtrue
job_idfalse
messagefalse
nametrue
organization_idfalse
readmetrue
template_idtrue
updated_atfalse
| | User
create, write, delete |
FieldTracked
avatar_urlfalse
created_atfalse
deletedtrue
emailtrue
hashed_passwordtrue
idtrue
last_seen_atfalse
login_typetrue
quiet_hours_scheduletrue
rbac_rolestrue
statustrue
updated_atfalse
usernametrue
| | Workspace
create, write, delete |
FieldTracked
autostart_scheduletrue
created_atfalse
deletedfalse
deleting_attrue
dormant_attrue
idtrue
last_used_atfalse
nametrue
organization_idfalse
owner_idtrue
template_idtrue
ttltrue
updated_atfalse
| | WorkspaceBuild
start, stop |
FieldTracked
build_numberfalse
created_atfalse
daily_costfalse
deadlinefalse
idfalse
initiator_by_avatar_urlfalse
initiator_by_usernamefalse
initiator_idfalse
job_idfalse
max_deadlinefalse
provisioner_statefalse
reasonfalse
template_version_idtrue
transitionfalse
updated_atfalse
workspace_idfalse
| diff --git a/docs/api/schemas.md b/docs/api/schemas.md index 93b14beab75c2..c845704f78c81 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -4677,6 +4677,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in ```json { + "archived": true, "created_at": "2019-08-24T14:15:22Z", "created_by": { "avatar_url": "http://example.com", @@ -4716,6 +4717,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in | Name | Type | Required | Restrictions | Description | | ----------------- | --------------------------------------------------------------------------- | -------- | ------------ | ----------- | +| `archived` | boolean | false | | | | `created_at` | string | false | | | | `created_by` | [codersdk.MinimalUser](#codersdkminimaluser) | false | | | | `id` | string | false | | | diff --git a/docs/api/templates.md b/docs/api/templates.md index 4f0dead4959fb..5b6f0a2bcbce7 100644 --- a/docs/api/templates.md +++ b/docs/api/templates.md @@ -373,6 +373,7 @@ curl -X GET http://coder-server:8080/api/v2/organizations/{organization}/templat ```json { + "archived": true, "created_at": "2019-08-24T14:15:22Z", "created_by": { "avatar_url": "http://example.com", @@ -443,6 +444,7 @@ curl -X GET http://coder-server:8080/api/v2/organizations/{organization}/templat ```json { + "archived": true, "created_at": "2019-08-24T14:15:22Z", "created_by": { "avatar_url": "http://example.com", @@ -537,6 +539,7 @@ curl -X POST http://coder-server:8080/api/v2/organizations/{organization}/templa ```json { + "archived": true, "created_at": "2019-08-24T14:15:22Z", "created_by": { "avatar_url": "http://example.com", @@ -838,6 +841,7 @@ curl -X GET http://coder-server:8080/api/v2/templates/{template}/versions \ ```json [ { + "archived": true, "created_at": "2019-08-24T14:15:22Z", "created_by": { "avatar_url": "http://example.com", @@ -887,6 +891,7 @@ Status Code **200** | Name | Type | Required | Restrictions | Description | | -------------------- | ------------------------------------------------------------------------ | -------- | ------------ | ----------- | | `[array item]` | array | false | | | +| `» archived` | boolean | false | | | | `» created_at` | string(date-time) | false | | | | `» created_by` | [codersdk.MinimalUser](schemas.md#codersdkminimaluser) | false | | | | `»» avatar_url` | string(uri) | false | | | @@ -984,6 +989,50 @@ curl -X PATCH http://coder-server:8080/api/v2/templates/{template}/versions \ To perform this operation, you must be authenticated. [Learn more](authentication.md). +## Archive template unused versions by template id + +### Code samples + +```shell +# Example request using curl +curl -X POST http://coder-server:8080/api/v2/templates/{template}/versions/archive \ + -H 'Accept: application/json' \ + -H 'Coder-Session-Token: API_KEY' +``` + +`POST /templates/{template}/versions/archive` + +### Parameters + +| Name | In | Type | Required | Description | +| ---------- | ---- | ------------ | -------- | ----------- | +| `template` | path | string(uuid) | true | Template ID | + +### Example responses + +> 200 Response + +```json +{ + "detail": "string", + "message": "string", + "validations": [ + { + "detail": "string", + "field": "string" + } + ] +} +``` + +### Responses + +| Status | Meaning | Description | Schema | +| ------ | ------------------------------------------------------- | ----------- | ------------------------------------------------ | +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [codersdk.Response](schemas.md#codersdkresponse) | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). + ## Get template version by template ID and name ### Code samples @@ -1011,6 +1060,7 @@ curl -X GET http://coder-server:8080/api/v2/templates/{template}/versions/{templ ```json [ { + "archived": true, "created_at": "2019-08-24T14:15:22Z", "created_by": { "avatar_url": "http://example.com", @@ -1060,6 +1110,7 @@ Status Code **200** | Name | Type | Required | Restrictions | Description | | -------------------- | ------------------------------------------------------------------------ | -------- | ------------ | ----------- | | `[array item]` | array | false | | | +| `» archived` | boolean | false | | | | `» created_at` | string(date-time) | false | | | | `» created_by` | [codersdk.MinimalUser](schemas.md#codersdkminimaluser) | false | | | | `»» avatar_url` | string(uri) | false | | | @@ -1128,6 +1179,7 @@ curl -X GET http://coder-server:8080/api/v2/templateversions/{templateversion} \ ```json { + "archived": true, "created_at": "2019-08-24T14:15:22Z", "created_by": { "avatar_url": "http://example.com", @@ -1207,6 +1259,7 @@ curl -X PATCH http://coder-server:8080/api/v2/templateversions/{templateversion} ```json { + "archived": true, "created_at": "2019-08-24T14:15:22Z", "created_by": { "avatar_url": "http://example.com", diff --git a/docs/cli/templates.md b/docs/cli/templates.md index 602b4c4fb6ef5..f1551e535829a 100644 --- a/docs/cli/templates.md +++ b/docs/cli/templates.md @@ -35,6 +35,7 @@ Templates are written in standard Terraform and describe the infrastructure for | Name | Purpose | | ------------------------------------------------ | ------------------------------------------------------------------------------ | +| [archive](./templates_archive.md) | Archive unused failed template versions from a given template(s) | | [create](./templates_create.md) | Create a template from the current directory or as specified by flag | | [delete](./templates_delete.md) | Delete templates | | [edit](./templates_edit.md) | Edit the metadata of a template by name. | diff --git a/docs/cli/templates_archive.md b/docs/cli/templates_archive.md new file mode 100644 index 0000000000000..42db079fefa61 --- /dev/null +++ b/docs/cli/templates_archive.md @@ -0,0 +1,29 @@ + + +# templates archive + +Archive unused failed template versions from a given template(s) + +## Usage + +```console +coder templates archive [flags] [name...] +``` + +## Options + +### --all + +| | | +| ---- | ----------------- | +| Type | bool | + +Include all unused template versions. By default, only failed template versions are archived. + +### -y, --yes + +| | | +| ---- | ----------------- | +| Type | bool | + +Bypass prompts. diff --git a/docs/manifest.json b/docs/manifest.json index e6541f5634250..4c1aee6bd02ea 100644 --- a/docs/manifest.json +++ b/docs/manifest.json @@ -801,6 +801,11 @@ "description": "Manage templates", "path": "cli/templates.md" }, + { + "title": "templates archive", + "description": "Archive unused failed template versions from a given template(s)", + "path": "cli/templates_archive.md" + }, { "title": "templates create", "description": "Create a template from the current directory or as specified by flag", diff --git a/enterprise/audit/table.go b/enterprise/audit/table.go index 806f04f8cbcd9..5bfe44557cb3c 100644 --- a/enterprise/audit/table.go +++ b/enterprise/audit/table.go @@ -99,7 +99,7 @@ var auditableResourcesTypes = map[any]map[string]Action{ "external_auth_providers": ActionIgnore, // Not helpful because this can only change when new versions are added. "created_by_avatar_url": ActionIgnore, "created_by_username": ActionIgnore, - "deleted": ActionTrack, + "archived": ActionTrack, }, &database.User{}: { "id": ActionTrack, diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index c59407355cbe1..72820a3645855 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -52,6 +52,17 @@ export interface AppearanceConfig { readonly support_links?: LinkConfig[]; } +// From codersdk/templates.go +export interface ArchiveTemplateVersionsRequest { + readonly all: boolean; +} + +// From codersdk/templates.go +export interface ArchiveTemplateVersionsResponse { + readonly template_id: string; + readonly archived_ids: string[]; +} + // From codersdk/roles.go export interface AssignableRoles extends Role { readonly assignable: boolean; @@ -753,17 +764,6 @@ export interface ProxyHealthReport { readonly warnings: string[]; } -// From codersdk/templates.go -export interface PruneTemplateVersionsRequest { - readonly all: boolean; -} - -// From codersdk/templates.go -export interface PruneTemplateVersionsResponse { - readonly template_id: string; - readonly deleted_ids: string[]; -} - // From codersdk/workspaces.go export interface PutExtendWorkspaceRequest { readonly deadline: string; @@ -1020,7 +1020,7 @@ export interface TemplateVersion { readonly job: ProvisionerJob; readonly readme: string; readonly created_by: MinimalUser; - readonly deleted: boolean; + readonly archived: boolean; readonly warnings?: TemplateVersionWarning[]; } @@ -1076,6 +1076,7 @@ export interface TemplateVersionVariable { // From codersdk/templates.go export interface TemplateVersionsByTemplateRequest extends Pagination { readonly template_id: string; + readonly include_archived: boolean; } // From codersdk/apikey.go diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 5f1b6539f8564..bf32cb70cb773 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -379,7 +379,7 @@ You can add instructions here [Some link info](https://coder.com)`, created_by: MockUser, - deleted: false, + archived: false, }; export const MockTemplateVersion2: TypesGen.TemplateVersion = { @@ -398,7 +398,7 @@ You can add instructions here [Some link info](https://coder.com)`, created_by: MockUser, - deleted: false, + archived: false, }; export const MockTemplateVersion3: TypesGen.TemplateVersion = { @@ -412,7 +412,7 @@ export const MockTemplateVersion3: TypesGen.TemplateVersion = { readme: "README", created_by: MockUser, warnings: ["UNSUPPORTED_WORKSPACES"], - deleted: false, + archived: false, }; export const MockTemplate: TypesGen.Template = { From c06d63ad08e3361a662a30530971cf449572a77a Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 9 Oct 2023 14:06:49 -0500 Subject: [PATCH 10/37] Bump migration file --- ...ersions.down.sql => 000163_archive_template_versions.down.sql} | 0 ...te_versions.up.sql => 000163_archive_template_versions.up.sql} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename coderd/database/migrations/{000162_archive_template_versions.down.sql => 000163_archive_template_versions.down.sql} (100%) rename coderd/database/migrations/{000162_archive_template_versions.up.sql => 000163_archive_template_versions.up.sql} (100%) diff --git a/coderd/database/migrations/000162_archive_template_versions.down.sql b/coderd/database/migrations/000163_archive_template_versions.down.sql similarity index 100% rename from coderd/database/migrations/000162_archive_template_versions.down.sql rename to coderd/database/migrations/000163_archive_template_versions.down.sql diff --git a/coderd/database/migrations/000162_archive_template_versions.up.sql b/coderd/database/migrations/000163_archive_template_versions.up.sql similarity index 100% rename from coderd/database/migrations/000162_archive_template_versions.up.sql rename to coderd/database/migrations/000163_archive_template_versions.up.sql From 460b0d2c7463132524d66629c1f56ab35d283895 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 9 Oct 2023 14:16:05 -0500 Subject: [PATCH 11/37] Make update golden files --- cli/templateversionarchive.go | 4 +--- cli/testdata/coder_templates_--help.golden | 1 + .../coder_templates_archive_--help.golden | 17 +++++++++++++++++ coderd/templateversions_test.go | 5 ++--- 4 files changed, 21 insertions(+), 6 deletions(-) create mode 100644 cli/testdata/coder_templates_archive_--help.golden diff --git a/cli/templateversionarchive.go b/cli/templateversionarchive.go index 334e731b56524..d1a8d153dbe8f 100644 --- a/cli/templateversionarchive.go +++ b/cli/templateversionarchive.go @@ -15,9 +15,7 @@ import ( ) func (r *RootCmd) archiveTemplateVersions() *clibase.Cmd { - var ( - all clibase.Bool - ) + var all clibase.Bool client := new(codersdk.Client) cmd := &clibase.Cmd{ Use: "archive [name...] ", diff --git a/cli/testdata/coder_templates_--help.golden b/cli/testdata/coder_templates_--help.golden index ee1a55893825e..4b8fbe5a7a369 100644 --- a/cli/testdata/coder_templates_--help.golden +++ b/cli/testdata/coder_templates_--help.golden @@ -23,6 +23,7 @@ USAGE: $ coder templates push my-template SUBCOMMANDS: + archive Archive unused failed template versions from a given template(s) create Create a template from the current directory or as specified by flag delete Delete templates diff --git a/cli/testdata/coder_templates_archive_--help.golden b/cli/testdata/coder_templates_archive_--help.golden new file mode 100644 index 0000000000000..5c3ef2a04af05 --- /dev/null +++ b/cli/testdata/coder_templates_archive_--help.golden @@ -0,0 +1,17 @@ +coder v0.0.0-devel + +USAGE: + coder templates archive [flags] [name...] + + Archive unused failed template versions from a given template(s) + +OPTIONS: + --all bool + Include all unused template versions. By default, only failed template + versions are archived. + + -y, --yes bool + Bypass prompts. + +——— +Run `coder --help` for a list of global options. diff --git a/coderd/templateversions_test.go b/coderd/templateversions_test.go index ce93adcabc7c1..df56a3c6d1d97 100644 --- a/coderd/templateversions_test.go +++ b/coderd/templateversions_test.go @@ -1519,9 +1519,8 @@ func TestTemplateVersionParameters_Order(t *testing.T) { func TestTemplateArchiveVersions(t *testing.T) { t.Parallel() - client, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) user := coderdtest.CreateFirstUser(t, client) - var _ = api var totalVersions int // Create a template to archive @@ -1561,7 +1560,7 @@ func TestTemplateArchiveVersions(t *testing.T) { for i := 0; i < 2; i++ { used := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{ Parse: echo.ParseComplete, - ProvisionPlan: echo.PlanComplete, //echo.PlanFailed, + ProvisionPlan: echo.PlanComplete, ProvisionApply: echo.ApplyComplete, }, func(req *codersdk.CreateTemplateVersionRequest) { req.TemplateID = template.ID From d9bfea5066ab6b14cffa41ff5130a115d83b143b Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 9 Oct 2023 14:31:09 -0500 Subject: [PATCH 12/37] Fix swagger id --- coderd/apidoc/docs.go | 2 +- coderd/apidoc/swagger.json | 2 +- coderd/templateversions.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index bbae048069640..ab467eedf5884 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -2382,7 +2382,7 @@ const docTemplate = `{ "Templates" ], "summary": "Archive template unused versions by template id", - "operationId": "archive-template-versions-by-template-id", + "operationId": "archive-template-unused-versions-by-template-id", "parameters": [ { "type": "string", diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 1ca1d81411037..ce62cbccd8fd2 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -2078,7 +2078,7 @@ "produces": ["application/json"], "tags": ["Templates"], "summary": "Archive template unused versions by template id", - "operationId": "archive-template-versions-by-template-id", + "operationId": "archive-template-unused-versions-by-template-id", "parameters": [ { "type": "string", diff --git a/coderd/templateversions.go b/coderd/templateversions.go index a90e4d8390c9a..fd7da564a1409 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -1013,7 +1013,7 @@ func (api *API) previousTemplateVersionByOrganizationTemplateAndName(rw http.Res } // @Summary Archive template unused versions by template id -// @ID archive-template-versions-by-template-id +// @ID archive-template-unused-versions-by-template-id // @Security CoderSessionToken // @Accept json // @Produce json From 655c906670f135f1ae090d47704b54733ad8766a Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 9 Oct 2023 14:47:52 -0500 Subject: [PATCH 13/37] Fix swagger accept json --- coderd/templateversions.go | 1 + 1 file changed, 1 insertion(+) diff --git a/coderd/templateversions.go b/coderd/templateversions.go index fd7da564a1409..41a80bf12f6b7 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -1019,6 +1019,7 @@ func (api *API) previousTemplateVersionByOrganizationTemplateAndName(rw http.Res // @Produce json // @Tags Templates // @Param template path string true "Template ID" format(uuid) +// @Param request body codersdk.ArchiveTemplateVersionsRequest true "Archive request" // @Success 200 {object} codersdk.Response // @Router /templates/{template}/versions/archive [post] func (api *API) postArchiveTemplateVersions(rw http.ResponseWriter, r *http.Request) { From d4e54b22ac8be00e5fa21aae81372ff488311a26 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 9 Oct 2023 19:55:06 +0000 Subject: [PATCH 14/37] Make gen --- coderd/apidoc/docs.go | 18 ++++++++++++++++++ coderd/apidoc/swagger.json | 18 ++++++++++++++++++ docs/api/schemas.md | 14 ++++++++++++++ docs/api/templates.md | 16 +++++++++++++--- 4 files changed, 63 insertions(+), 3 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index ab467eedf5884..662e9448d6048 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -2391,6 +2391,15 @@ const docTemplate = `{ "name": "template", "in": "path", "required": true + }, + { + "description": "Archive request", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.ArchiveTemplateVersionsRequest" + } } ], "responses": { @@ -7119,6 +7128,15 @@ const docTemplate = `{ } } }, + "codersdk.ArchiveTemplateVersionsRequest": { + "type": "object", + "properties": { + "all": { + "description": "By default, only failed versions are archived. Set this to true\nto archive all unused versions regardless of job status.", + "type": "boolean" + } + } + }, "codersdk.AssignableRoles": { "type": "object", "properties": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index ce62cbccd8fd2..366c8371cd8b9 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -2087,6 +2087,15 @@ "name": "template", "in": "path", "required": true + }, + { + "description": "Archive request", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.ArchiveTemplateVersionsRequest" + } } ], "responses": { @@ -6319,6 +6328,15 @@ } } }, + "codersdk.ArchiveTemplateVersionsRequest": { + "type": "object", + "properties": { + "all": { + "description": "By default, only failed versions are archived. Set this to true\nto archive all unused versions regardless of job status.", + "type": "boolean" + } + } + }, "codersdk.AssignableRoles": { "type": "object", "properties": { diff --git a/docs/api/schemas.md b/docs/api/schemas.md index 7aa47ad30a927..32946ccd8b104 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -980,6 +980,20 @@ _None_ | `service_banner` | [codersdk.ServiceBannerConfig](#codersdkservicebannerconfig) | false | | | | `support_links` | array of [codersdk.LinkConfig](#codersdklinkconfig) | false | | | +## codersdk.ArchiveTemplateVersionsRequest + +```json +{ + "all": true +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +| ----- | ------- | -------- | ------------ | ------------------------------------------------------------------------------------------------------------------------ | +| `all` | boolean | false | | By default, only failed versions are archived. Set this to true to archive all unused versions regardless of job status. | + ## codersdk.AssignableRoles ```json diff --git a/docs/api/templates.md b/docs/api/templates.md index 5b6f0a2bcbce7..410faf7ad8ad4 100644 --- a/docs/api/templates.md +++ b/docs/api/templates.md @@ -996,17 +996,27 @@ To perform this operation, you must be authenticated. [Learn more](authenticatio ```shell # Example request using curl curl -X POST http://coder-server:8080/api/v2/templates/{template}/versions/archive \ + -H 'Content-Type: application/json' \ -H 'Accept: application/json' \ -H 'Coder-Session-Token: API_KEY' ``` `POST /templates/{template}/versions/archive` +> Body parameter + +```json +{ + "all": true +} +``` + ### Parameters -| Name | In | Type | Required | Description | -| ---------- | ---- | ------------ | -------- | ----------- | -| `template` | path | string(uuid) | true | Template ID | +| Name | In | Type | Required | Description | +| ---------- | ---- | -------------------------------------------------------------------------------------------- | -------- | --------------- | +| `template` | path | string(uuid) | true | Template ID | +| `body` | body | [codersdk.ArchiveTemplateVersionsRequest](schemas.md#codersdkarchivetemplateversionsrequest) | true | Archive request | ### Example responses From 938f2569f8371dff851407d70088e1909760b927 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 9 Oct 2023 15:44:14 -0500 Subject: [PATCH 15/37] Add unarchive --- coderd/coderd.go | 2 + coderd/templateversions.go | 85 +++++++++++++++++++++++++++++++++ coderd/templateversions_test.go | 18 +++++++ codersdk/templates.go | 19 ++++++++ 4 files changed, 124 insertions(+) diff --git a/coderd/coderd.go b/coderd/coderd.go index aedb2867869c6..1932fa87b997f 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -685,6 +685,8 @@ func New(options *Options) *API { r.Get("/", api.templateVersion) r.Patch("/", api.patchTemplateVersion) r.Patch("/cancel", api.patchCancelTemplateVersion) + r.Post("/archive", api.postArchiveTemplateVersion()) + r.Post("/unarchive", api.postUnarchiveTemplateVersion()) // Old agents may expect a non-error response from /schema and /parameters endpoints. // The idea is to return an empty [], so that the coder CLI won't get blocked accidentally. r.Get("/schema", templateVersionSchemaDeprecated) diff --git a/coderd/templateversions.go b/coderd/templateversions.go index 41a80bf12f6b7..b96f22bf44251 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -1076,6 +1076,91 @@ func (api *API) postArchiveTemplateVersions(rw http.ResponseWriter, r *http.Requ }) } +// @Summary Archive template version +// @ID archive-template-version +// @Security CoderSessionToken +// @Accept json +// @Tags Templates +// @Param templateversion path string true "Template version ID" format(uuid) +// @Success 200 {object} codersdk.Response +// @Router /templateversions/{templateversion}/archive [post] +func (api *API) postArchiveTemplateVersion() func(rw http.ResponseWriter, r *http.Request) { + return api.setArchiveTemplateVersion(true) +} + +// @Summary Unarchive template version +// @ID unarchive-template-version +// @Security CoderSessionToken +// @Accept json +// @Tags Templates +// @Param templateversion path string true "Template version ID" format(uuid) +// @Success 200 {object} codersdk.Response +// @Router /templateversions/{templateversion}/unarchive [post] +func (api *API) postUnarchiveTemplateVersion() func(rw http.ResponseWriter, r *http.Request) { + return api.setArchiveTemplateVersion(false) +} + +func (api *API) setArchiveTemplateVersion(archive bool) func(rw http.ResponseWriter, r *http.Request) { + return func(rw http.ResponseWriter, r *http.Request) { + var ( + ctx = r.Context() + templateVersion = httpmw.TemplateVersionParam(r) + auditor = *api.Auditor.Load() + aReq, commitAudit = audit.InitRequest[database.TemplateVersion](rw, &audit.RequestParams{ + Audit: auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionWrite, + }) + ) + defer commitAudit() + aReq.Old = templateVersion + + verb := "archived" + if !archive { + verb = "unarchived" + } + if templateVersion.Archived == archive { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: fmt.Sprintf("Template version already %s", verb), + }) + return + } + + if !templateVersion.TemplateID.Valid { + // Maybe we should allow this? + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Cannot archive template versions not associate with a template.", + }) + return + } + + archived, err := api.Database.ArchiveUnusedTemplateVersions(ctx, database.ArchiveUnusedTemplateVersionsParams{ + UpdatedAt: dbtime.Now(), + TemplateID: templateVersion.TemplateID.UUID, + TemplateVersionID: templateVersion.ID, + JobStatus: database.NullProvisionerJobStatus{}, + Unarchive: !archive, + }) + + if httpapi.Is404Error(err) || (len(archived) == 0 && err == nil) { + httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ + Message: "Template or template versions not found.", + }) + return + } + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error fetching template version.", + Detail: err.Error(), + }) + return + } + + httpapi.Write(ctx, rw, http.StatusOK, fmt.Sprintf("template version %q %s", templateVersion.ID.String(), verb)) + } +} + // @Summary Update active template version by template ID // @ID update-active-template-version-by-template-id // @Security CoderSessionToken diff --git a/coderd/templateversions_test.go b/coderd/templateversions_test.go index df56a3c6d1d97..737a511c67f5e 100644 --- a/coderd/templateversions_test.go +++ b/coderd/templateversions_test.go @@ -1610,4 +1610,22 @@ func TestTemplateArchiveVersions(t *testing.T) { }) require.NoError(t, err, "fetch all versions") require.Len(t, remaining, totalVersions-len(expArchived)-len(allFailed), "remaining versions") + + // Unarchive a version + err = client.SetArchiveTemplateVersion(ctx, expArchived[0], false) + require.NoError(t, err, "unarchive a version") + + tv, err := client.TemplateVersion(ctx, expArchived[0]) + require.NoError(t, err, "fetch version") + require.False(t, tv.Archived, "expect unarchived") + + // Check the remaining again + remaining, err = client.TemplateVersionsByTemplate(ctx, codersdk.TemplateVersionsByTemplateRequest{ + TemplateID: template.ID, + Pagination: codersdk.Pagination{ + Limit: 100, + }, + }) + require.NoError(t, err, "fetch all versions") + require.Len(t, remaining, totalVersions-len(expArchived)-len(allFailed)+1, "remaining versions") } diff --git a/codersdk/templates.go b/codersdk/templates.go index 38793b9f5fdf1..d31f9927e075a 100644 --- a/codersdk/templates.go +++ b/codersdk/templates.go @@ -256,6 +256,25 @@ func (c *Client) ArchiveTemplateVersions(ctx context.Context, template uuid.UUID return resp, json.NewDecoder(res.Body).Decode(&resp) } +func (c *Client) SetArchiveTemplateVersion(ctx context.Context, templateVersion uuid.UUID, archive bool) error { + u := fmt.Sprintf("/api/v2/templateversions/%s", templateVersion.String()) + if archive { + u += "/archive" + } else { + u += "/unarchive" + } + res, err := c.Request(ctx, http.MethodPost, u, nil) + if err != nil { + return err + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + return ReadBodyAsError(res) + } + + return nil +} + func (c *Client) DeleteTemplate(ctx context.Context, template uuid.UUID) error { res, err := c.Request(ctx, http.MethodDelete, fmt.Sprintf("/api/v2/templates/%s", template), nil) if err != nil { From 7498c7309c5e399b11debac721bca1c8f3cf6f57 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 9 Oct 2023 16:13:27 -0500 Subject: [PATCH 16/37] Add unarchive api --- coderd/database/dbauthz/dbauthz.go | 16 ++++++ coderd/database/dbfake/dbfake.go | 9 +++ coderd/database/dbmetrics/dbmetrics.go | 7 +++ coderd/database/dbmock/dbmock.go | 14 +++++ coderd/database/querier.go | 2 + coderd/database/queries.sql.go | 58 +++++++++++++++++--- coderd/database/queries/templateversions.sql | 29 +++++++++- coderd/templateversions.go | 31 ++++++++--- 8 files changed, 150 insertions(+), 16 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 978647184143c..d71dccb20baaf 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -2271,6 +2271,22 @@ func (q *querier) TryAcquireLock(ctx context.Context, id int64) (bool, error) { return q.db.TryAcquireLock(ctx, id) } +func (q *querier) UnarchiveTemplateVersion(ctx context.Context, arg database.UnarchiveTemplateVersionParams) error { + v, err := q.db.GetTemplateVersionByID(ctx, arg.TemplateVersionID) + if err != nil { + return err + } + + tpl, err := q.db.GetTemplateByID(ctx, v.TemplateID.UUID) + if err != nil { + return err + } + if err := q.authorizeContext(ctx, rbac.ActionUpdate, tpl); err != nil { + return err + } + return q.db.UnarchiveTemplateVersion(ctx, arg) +} + func (q *querier) UpdateAPIKeyByID(ctx context.Context, arg database.UpdateAPIKeyByIDParams) error { fetch := func(ctx context.Context, arg database.UpdateAPIKeyByIDParams) (database.APIKey, error) { return q.db.GetAPIKeyByID(ctx, arg.ID) diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index 00e32f2111b96..070b1c5e7096f 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -5330,6 +5330,15 @@ func (*FakeQuerier) TryAcquireLock(_ context.Context, _ int64) (bool, error) { return false, xerrors.New("TryAcquireLock must only be called within a transaction") } +func (q *FakeQuerier) UnarchiveTemplateVersion(ctx context.Context, arg database.UnarchiveTemplateVersionParams) error { + err := validateDatabaseType(arg) + if err != nil { + return err + } + + panic("not implemented") +} + func (q *FakeQuerier) UpdateAPIKeyByID(_ context.Context, arg database.UpdateAPIKeyByIDParams) error { if err := validateDatabaseType(arg); err != nil { return err diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go index d5d4322f24293..1702b95513490 100644 --- a/coderd/database/dbmetrics/dbmetrics.go +++ b/coderd/database/dbmetrics/dbmetrics.go @@ -1439,6 +1439,13 @@ func (m metricsStore) TryAcquireLock(ctx context.Context, pgTryAdvisoryXactLock return ok, err } +func (m metricsStore) UnarchiveTemplateVersion(ctx context.Context, arg database.UnarchiveTemplateVersionParams) error { + start := time.Now() + r0 := m.s.UnarchiveTemplateVersion(ctx, arg) + m.queryLatencies.WithLabelValues("UnarchiveTemplateVersion").Observe(time.Since(start).Seconds()) + return r0 +} + func (m metricsStore) UpdateAPIKeyByID(ctx context.Context, arg database.UpdateAPIKeyByIDParams) error { start := time.Now() err := m.s.UpdateAPIKeyByID(ctx, arg) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index ad9ac90352ae8..8a4c3a298efb5 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -3039,6 +3039,20 @@ func (mr *MockStoreMockRecorder) TryAcquireLock(arg0, arg1 interface{}) *gomock. return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TryAcquireLock", reflect.TypeOf((*MockStore)(nil).TryAcquireLock), arg0, arg1) } +// UnarchiveTemplateVersion mocks base method. +func (m *MockStore) UnarchiveTemplateVersion(arg0 context.Context, arg1 database.UnarchiveTemplateVersionParams) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UnarchiveTemplateVersion", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// UnarchiveTemplateVersion indicates an expected call of UnarchiveTemplateVersion. +func (mr *MockStoreMockRecorder) UnarchiveTemplateVersion(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UnarchiveTemplateVersion", reflect.TypeOf((*MockStore)(nil).UnarchiveTemplateVersion), arg0, arg1) +} + // UpdateAPIKeyByID mocks base method. func (m *MockStore) UpdateAPIKeyByID(arg0 context.Context, arg1 database.UpdateAPIKeyByIDParams) error { m.ctrl.T.Helper() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index f727e213d4321..68ada693ee794 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -288,6 +288,8 @@ type sqlcQuerier interface { // This must be called from within a transaction. The lock will be automatically // released when the transaction ends. TryAcquireLock(ctx context.Context, pgTryAdvisoryXactLock int64) (bool, error) + // This will always work regardless of the current state of the template version. + UnarchiveTemplateVersion(ctx context.Context, arg UnarchiveTemplateVersionParams) error UpdateAPIKeyByID(ctx context.Context, arg UpdateAPIKeyByIDParams) error UpdateExternalAuthLink(ctx context.Context, arg UpdateExternalAuthLinkParams) (ExternalAuthLink, error) UpdateGitSSHKey(ctx context.Context, arg UpdateGitSSHKeyParams) (GitSSHKey, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 6ce9894df7030..3ed293a945243 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5321,7 +5321,24 @@ FROM scoped_template_versions.id FROM -- Scope an archive to a single template and ignore already archived template versions - (SELECT id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, archived FROM template_versions WHERE template_versions.template_id = $2 :: uuid AND archived = false) AS scoped_template_versions + ( + SELECT + id, template_id, organization_id, created_at, updated_at, name, readme, job_id, created_by, external_auth_providers, message, archived + FROM + template_versions + WHERE + template_versions.template_id = $2 :: uuid + AND + archived = false + AND + -- This allows archiving a specfic template version. + CASE + WHEN $3::uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN + template_versions.id = $3 :: uuid + ELSE + true + END + ) AS scoped_template_versions LEFT JOIN provisioner_jobs ON scoped_template_versions.job_id = provisioner_jobs.id LEFT JOIN @@ -5352,8 +5369,8 @@ FROM AND CASE -- Optionally, only archive versions that match a given -- job status like 'failed'. - WHEN $3 :: provisioner_job_status IS NOT NULL THEN - provisioner_jobs.job_status = $3 :: provisioner_job_status + WHEN $4 :: provisioner_job_status IS NOT NULL THEN + provisioner_jobs.job_status = $4 :: provisioner_job_status ELSE true END @@ -5367,9 +5384,10 @@ RETURNING template_versions.id ` type ArchiveUnusedTemplateVersionsParams struct { - UpdatedAt time.Time `db:"updated_at" json:"updated_at"` - TemplateID uuid.UUID `db:"template_id" json:"template_id"` - JobStatus NullProvisionerJobStatus `db:"job_status" json:"job_status"` + UpdatedAt time.Time `db:"updated_at" json:"updated_at"` + TemplateID uuid.UUID `db:"template_id" json:"template_id"` + TemplateVersionID uuid.UUID `db:"template_version_id" json:"template_version_id"` + JobStatus NullProvisionerJobStatus `db:"job_status" json:"job_status"` } // Archiving templates is a soft delete action, so is reversible. @@ -5380,7 +5398,12 @@ type ArchiveUnusedTemplateVersionsParams struct { // // used_versions.transition != 'delete', func (q *sqlQuerier) ArchiveUnusedTemplateVersions(ctx context.Context, arg ArchiveUnusedTemplateVersionsParams) ([]uuid.UUID, error) { - rows, err := q.db.QueryContext(ctx, archiveUnusedTemplateVersions, arg.UpdatedAt, arg.TemplateID, arg.JobStatus) + rows, err := q.db.QueryContext(ctx, archiveUnusedTemplateVersions, + arg.UpdatedAt, + arg.TemplateID, + arg.TemplateVersionID, + arg.JobStatus, + ) if err != nil { return nil, err } @@ -5776,6 +5799,27 @@ func (q *sqlQuerier) InsertTemplateVersion(ctx context.Context, arg InsertTempla return err } +const unarchiveTemplateVersion = `-- name: UnarchiveTemplateVersion :exec +UPDATE + template_versions +SET + archived = false, + updated_at = $1 +WHERE + id = $2 +` + +type UnarchiveTemplateVersionParams struct { + UpdatedAt time.Time `db:"updated_at" json:"updated_at"` + TemplateVersionID uuid.UUID `db:"template_version_id" json:"template_version_id"` +} + +// This will always work regardless of the current state of the template version. +func (q *sqlQuerier) UnarchiveTemplateVersion(ctx context.Context, arg UnarchiveTemplateVersionParams) error { + _, err := q.db.ExecContext(ctx, unarchiveTemplateVersion, arg.UpdatedAt, arg.TemplateVersionID) + return err +} + const updateTemplateVersionByID = `-- name: UpdateTemplateVersionByID :exec UPDATE template_versions diff --git a/coderd/database/queries/templateversions.sql b/coderd/database/queries/templateversions.sql index cf1e21d095eee..4da49611c37d1 100644 --- a/coderd/database/queries/templateversions.sql +++ b/coderd/database/queries/templateversions.sql @@ -138,6 +138,16 @@ ORDER BY created_at DESC LIMIT 1; +-- name: UnarchiveTemplateVersion :exec +-- This will always work regardless of the current state of the template version. +UPDATE + template_versions +SET + archived = false, + updated_at = sqlc.arg('updated_at') +WHERE + id = sqlc.arg('template_version_id'); + -- name: ArchiveUnusedTemplateVersions :many -- Archiving templates is a soft delete action, so is reversible. -- Archiving prevents the version from being used and discovered @@ -156,7 +166,24 @@ FROM scoped_template_versions.id FROM -- Scope an archive to a single template and ignore already archived template versions - (SELECT * FROM template_versions WHERE template_versions.template_id = sqlc.arg('template_id') :: uuid AND archived = false) AS scoped_template_versions + ( + SELECT + * + FROM + template_versions + WHERE + template_versions.template_id = sqlc.arg('template_id') :: uuid + AND + archived = false + AND + -- This allows archiving a specfic template version. + CASE + WHEN sqlc.arg('template_version_id')::uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN + template_versions.id = sqlc.arg('template_version_id') :: uuid + ELSE + true + END + ) AS scoped_template_versions LEFT JOIN provisioner_jobs ON scoped_template_versions.job_id = provisioner_jobs.id LEFT JOIN diff --git a/coderd/templateversions.go b/coderd/templateversions.go index b96f22bf44251..a9ff4c7c8e642 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -1135,15 +1135,30 @@ func (api *API) setArchiveTemplateVersion(archive bool) func(rw http.ResponseWri return } - archived, err := api.Database.ArchiveUnusedTemplateVersions(ctx, database.ArchiveUnusedTemplateVersionsParams{ - UpdatedAt: dbtime.Now(), - TemplateID: templateVersion.TemplateID.UUID, - TemplateVersionID: templateVersion.ID, - JobStatus: database.NullProvisionerJobStatus{}, - Unarchive: !archive, - }) + var err error + if archive { + archived, archiveError := api.Database.ArchiveUnusedTemplateVersions(ctx, database.ArchiveUnusedTemplateVersionsParams{ + UpdatedAt: dbtime.Now(), + TemplateID: templateVersion.TemplateID.UUID, + TemplateVersionID: templateVersion.ID, + JobStatus: database.NullProvisionerJobStatus{}, + }) + + if archiveError != nil { + err = archiveError + } else { + if len(archived) == 0 { + err = sql.ErrNoRows + } + } + } else { + err = api.Database.UnarchiveTemplateVersion(ctx, database.UnarchiveTemplateVersionParams{ + UpdatedAt: dbtime.Now(), + TemplateVersionID: templateVersion.ID, + }) + } - if httpapi.Is404Error(err) || (len(archived) == 0 && err == nil) { + if httpapi.Is404Error(err) { httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ Message: "Template or template versions not found.", }) From 685512a871957429efbddecb910c0192827b266a Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 9 Oct 2023 16:30:38 -0500 Subject: [PATCH 17/37] Add cli command to unarchive a version --- cli/templates.go | 1 + cli/templateversionarchive.go | 74 ++++++++++++++++++++++++++++++++++- 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/cli/templates.go b/cli/templates.go index d7cd02a467ef2..021adda101ab0 100644 --- a/cli/templates.go +++ b/cli/templates.go @@ -44,6 +44,7 @@ func (r *RootCmd) templates() *clibase.Cmd { r.templateDelete(), r.templatePull(), r.archiveTemplateVersions(), + r.archiveTemplateVersion(), }, } diff --git a/cli/templateversionarchive.go b/cli/templateversionarchive.go index d1a8d153dbe8f..052b341b9f3f9 100644 --- a/cli/templateversionarchive.go +++ b/cli/templateversionarchive.go @@ -14,11 +14,83 @@ import ( "github.com/coder/coder/v2/codersdk" ) +func (r *RootCmd) archiveTemplateVersion() *clibase.Cmd { + var unarchive clibase.Bool + + client := new(codersdk.Client) + cmd := &clibase.Cmd{ + Use: "archive-version [template-version-names...] ", + Short: "Archive or unarchive a template version(s)", + Middleware: clibase.Chain( + r.InitClient(client), + ), + Options: clibase.OptionSet{ + cliui.SkipPromptOption(), + clibase.Option{ + Name: "unarchive", + Description: "Unarchive the selected template version", + Flag: "unarchive", + Value: &unarchive, + }, + }, + Handler: func(inv *clibase.Invocation) error { + var ( + ctx = inv.Context() + versions []codersdk.TemplateVersion + ) + + organization, err := CurrentOrganization(inv, client) + if err != nil { + return err + } + + if len(inv.Args) == 0 { + return xerrors.Errorf("missing template name") + } + if len(inv.Args) < 2 { + return xerrors.Errorf("missing template version name(s)") + } + + templateName := inv.Args[0] + template, err := client.TemplateByName(ctx, organization.ID, templateName) + if err != nil { + return xerrors.Errorf("get template by name: %w", err) + } + for _, versionName := range inv.Args[1:] { + version, err := client.TemplateVersionByOrganizationAndName(ctx, organization.ID, template.Name, versionName) + if err != nil { + return xerrors.Errorf("get template version by name %q: %w", versionName, err) + } + versions = append(versions, version) + } + + verb := "archived" + if unarchive { + verb = "unarchived" + } + for _, version := range versions { + err := client.SetArchiveTemplateVersion(ctx, version.ID, !unarchive.Value()) + if err != nil { + return xerrors.Errorf("set archive to %t template versions for %q: %w", !unarchive, template.Name, err) + } + + _, _ = fmt.Fprintln( + inv.Stdout, fmt.Sprintf("Version "+pretty.Sprint(cliui.DefaultStyles.Keyword, version.Name)+" "+verb+" at "+cliui.Timestamp(time.Now())), + ) + } + + return nil + }, + } + + return cmd +} + func (r *RootCmd) archiveTemplateVersions() *clibase.Cmd { var all clibase.Bool client := new(codersdk.Client) cmd := &clibase.Cmd{ - Use: "archive [name...] ", + Use: "archive-template [template-name...] ", Short: "Archive unused failed template versions from a given template(s)", Middleware: clibase.Chain( r.InitClient(client), From f446ae5c2b50c7b3c61e1ad0fb6f05bf337d4a8a Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 9 Oct 2023 17:25:02 -0500 Subject: [PATCH 18/37] Linting --- cli/templateversionarchive.go | 2 +- coderd/database/queries.sql.go | 2 +- coderd/database/queries/templateversions.sql | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cli/templateversionarchive.go b/cli/templateversionarchive.go index 052b341b9f3f9..6611629205ed5 100644 --- a/cli/templateversionarchive.go +++ b/cli/templateversionarchive.go @@ -20,7 +20,7 @@ func (r *RootCmd) archiveTemplateVersion() *clibase.Cmd { client := new(codersdk.Client) cmd := &clibase.Cmd{ Use: "archive-version [template-version-names...] ", - Short: "Archive or unarchive a template version(s)", + Short: "Archive or unarchive a template version(s).", Middleware: clibase.Chain( r.InitClient(client), ), diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 3ed293a945243..3da08c1e3148d 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5331,7 +5331,7 @@ FROM AND archived = false AND - -- This allows archiving a specfic template version. + -- This allows archiving a specific template version. CASE WHEN $3::uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN template_versions.id = $3 :: uuid diff --git a/coderd/database/queries/templateversions.sql b/coderd/database/queries/templateversions.sql index 4da49611c37d1..ac816085800b6 100644 --- a/coderd/database/queries/templateversions.sql +++ b/coderd/database/queries/templateversions.sql @@ -176,7 +176,7 @@ FROM AND archived = false AND - -- This allows archiving a specfic template version. + -- This allows archiving a specific template version. CASE WHEN sqlc.arg('template_version_id')::uuid != '00000000-0000-0000-0000-000000000000'::uuid THEN template_versions.id = sqlc.arg('template_version_id') :: uuid From a9ff9d65a7d5752e9d4a12285dcb6e82271c202c Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 9 Oct 2023 17:27:44 -0500 Subject: [PATCH 19/37] Update golden files --- cli/templateversionarchive.go | 2 +- cli/testdata/coder_templates_--help.golden | 24 ++++++++++++---------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/cli/templateversionarchive.go b/cli/templateversionarchive.go index 6611629205ed5..7f320b3c3fc04 100644 --- a/cli/templateversionarchive.go +++ b/cli/templateversionarchive.go @@ -28,7 +28,7 @@ func (r *RootCmd) archiveTemplateVersion() *clibase.Cmd { cliui.SkipPromptOption(), clibase.Option{ Name: "unarchive", - Description: "Unarchive the selected template version", + Description: "Unarchive the selected template version.", Flag: "unarchive", Value: &unarchive, }, diff --git a/cli/testdata/coder_templates_--help.golden b/cli/testdata/coder_templates_--help.golden index 4b8fbe5a7a369..aae90e5fa13c3 100644 --- a/cli/testdata/coder_templates_--help.golden +++ b/cli/testdata/coder_templates_--help.golden @@ -23,17 +23,19 @@ USAGE: $ coder templates push my-template SUBCOMMANDS: - archive Archive unused failed template versions from a given template(s) - create Create a template from the current directory or as specified by - flag - delete Delete templates - edit Edit the metadata of a template by name. - init Get started with a templated template. - list List all the templates available for the organization - pull Download the latest version of a template to a path. - push Push a new template version from the current directory or as - specified by flag - versions Manage different versions of the specified template + archive-template Archive unused failed template versions from a given + template(s) + archive-version Archive or unarchive a template version(s). + create Create a template from the current directory or as + specified by flag + delete Delete templates + edit Edit the metadata of a template by name. + init Get started with a templated template. + list List all the templates available for the organization + pull Download the latest version of a template to a path. + push Push a new template version from the current directory + or as specified by flag + versions Manage different versions of the specified template ——— Run `coder --help` for a list of global options. From b83a15e1a73458d5b92949190e3d5c76dcc05936 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 9 Oct 2023 18:06:44 -0500 Subject: [PATCH 20/37] Move cmd commands, allow archiuve deleted --- cli/templates.go | 1 - cli/templateversionarchive.go | 31 ++++++++++++++++---- cli/templateversions.go | 23 ++++++++++++++- coderd/database/querier.go | 1 - coderd/database/queries.sql.go | 7 ++--- coderd/database/queries/templateversions.sql | 6 ++-- coderd/templateversions.go | 2 +- 7 files changed, 53 insertions(+), 18 deletions(-) diff --git a/cli/templates.go b/cli/templates.go index 021adda101ab0..d7cd02a467ef2 100644 --- a/cli/templates.go +++ b/cli/templates.go @@ -44,7 +44,6 @@ func (r *RootCmd) templates() *clibase.Cmd { r.templateDelete(), r.templatePull(), r.archiveTemplateVersions(), - r.archiveTemplateVersion(), }, } diff --git a/cli/templateversionarchive.go b/cli/templateversionarchive.go index 7f320b3c3fc04..89e5147d730b4 100644 --- a/cli/templateversionarchive.go +++ b/cli/templateversionarchive.go @@ -19,7 +19,7 @@ func (r *RootCmd) archiveTemplateVersion() *clibase.Cmd { client := new(codersdk.Client) cmd := &clibase.Cmd{ - Use: "archive-version [template-version-names...] ", + Use: "archive [template-version-names...] ", Short: "Archive or unarchive a template version(s).", Middleware: clibase.Chain( r.InitClient(client), @@ -68,10 +68,21 @@ func (r *RootCmd) archiveTemplateVersion() *clibase.Cmd { if unarchive { verb = "unarchived" } + failed := 0 for _, version := range versions { + if version.Archived == !unarchive.Value() { + _, _ = fmt.Fprintln( + inv.Stdout, fmt.Sprintf("Version "+pretty.Sprint(cliui.DefaultStyles.Keyword, version.Name)+" already "+verb), + ) + continue + } + err := client.SetArchiveTemplateVersion(ctx, version.ID, !unarchive.Value()) if err != nil { - return xerrors.Errorf("set archive to %t template versions for %q: %w", !unarchive, template.Name, err) + failed++ + _, _ = fmt.Fprintln(inv.Stderr, fmt.Sprintf("Failed to archive template version %q: %s", version.Name, + pretty.Sprint(cliui.DefaultStyles.Error, err.Error()))) + continue } _, _ = fmt.Fprintln( @@ -79,6 +90,9 @@ func (r *RootCmd) archiveTemplateVersion() *clibase.Cmd { ) } + if failed > 0 { + return xerrors.Errorf("failed on %d template versions", failed) + } return nil }, } @@ -90,7 +104,7 @@ func (r *RootCmd) archiveTemplateVersions() *clibase.Cmd { var all clibase.Bool client := new(codersdk.Client) cmd := &clibase.Cmd{ - Use: "archive-template [template-name...] ", + Use: "archive [template-name...] ", Short: "Archive unused failed template versions from a given template(s)", Middleware: clibase.Chain( r.InitClient(client), @@ -146,14 +160,18 @@ func (r *RootCmd) archiveTemplateVersions() *clibase.Cmd { return err } + failed := 0 for _, template := range templates { resp, err := client.ArchiveTemplateVersions(ctx, template.ID, all.Value()) if err != nil { - return xerrors.Errorf("archive template versions for %q: %w", template.Name, err) + _, _ = fmt.Fprintln(inv.Stderr, fmt.Sprintf("Failed to archive template versions for %q: %s", template.Name, + pretty.Sprint(cliui.DefaultStyles.Error, err.Error()))) + failed++ + continue } _, _ = fmt.Fprintln( - inv.Stdout, fmt.Sprintf("Archive %s versions from "+pretty.Sprint(cliui.DefaultStyles.Keyword, template.Name)+" at "+cliui.Timestamp(time.Now()), len(resp.ArchivedIDs)), + inv.Stdout, fmt.Sprintf("Archived %d versions from "+pretty.Sprint(cliui.DefaultStyles.Keyword, template.Name)+" at "+cliui.Timestamp(time.Now()), len(resp.ArchivedIDs)), ) if ok, _ := inv.ParsedFlags().GetBool("verbose"); err == nil && ok { @@ -167,6 +185,9 @@ func (r *RootCmd) archiveTemplateVersions() *clibase.Cmd { } } + if failed > 0 { + return xerrors.Errorf("failed on %d templates", failed) + } return nil }, } diff --git a/cli/templateversions.go b/cli/templateversions.go index 299ae98e96b23..1a17564c21b23 100644 --- a/cli/templateversions.go +++ b/cli/templateversions.go @@ -5,6 +5,8 @@ import ( "strings" "time" + "github.com/coder/pretty" + "github.com/google/uuid" "golang.org/x/xerrors" @@ -29,6 +31,7 @@ func (r *RootCmd) templateVersions() *clibase.Cmd { }, Children: []*clibase.Cmd{ r.templateVersionsList(), + r.archiveTemplateVersion(), }, } @@ -42,6 +45,8 @@ func (r *RootCmd) templateVersionsList() *clibase.Cmd { ) client := new(codersdk.Client) + var includeArchived clibase.Bool + cmd := &clibase.Cmd{ Use: "list