From 95e95edc94d70c0614c7a6c1ff01c22b052d3371 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 3 Aug 2022 20:31:21 +0300 Subject: [PATCH 01/16] feat: Implement workspace renaming --- cli/root.go | 1 + coderd/coderd.go | 1 + coderd/database/databasefake/databasefake.go | 27 +++++++ coderd/database/querier.go | 1 + coderd/database/queries.sql.go | 20 +++++ coderd/database/queries/workspaces.sql | 9 +++ coderd/workspaces.go | 81 +++++++++++++++++--- coderd/workspaces_test.go | 31 ++++++++ codersdk/workspaces.go | 18 ++++- site/src/api/typesGenerated.ts | 5 ++ 10 files changed, 181 insertions(+), 13 deletions(-) diff --git a/cli/root.go b/cli/root.go index cbece56fcf4cc..7551e09c03c63 100644 --- a/cli/root.go +++ b/cli/root.go @@ -79,6 +79,7 @@ func Core() []*cobra.Command { start(), state(), stop(), + rename(), templates(), update(), users(), diff --git a/coderd/coderd.go b/coderd/coderd.go index a2062b736227a..832f4c4f12988 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -372,6 +372,7 @@ func New(options *Options) *API { httpmw.ExtractWorkspaceParam(options.Database), ) r.Get("/", api.workspace) + r.Patch("/", api.patchWorkspace) r.Route("/builds", func(r chi.Router) { r.Get("/", api.workspaceBuilds) r.Post("/", api.postWorkspaceBuilds) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 7ba138c820312..781f493e148fa 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -9,6 +9,7 @@ import ( "time" "github.com/google/uuid" + "github.com/lib/pq" "golang.org/x/exp/slices" "github.com/coder/coder/coderd/database" @@ -2090,6 +2091,32 @@ func (q *fakeQuerier) UpdateProvisionerJobWithCompleteByID(_ context.Context, ar return sql.ErrNoRows } +func (q *fakeQuerier) UpdateWorkspace(_ context.Context, arg database.UpdateWorkspaceParams) error { + q.mutex.Lock() + defer q.mutex.Unlock() + + for i, workspace := range q.workspaces { + if workspace.Deleted || workspace.ID != arg.ID { + continue + } + for _, other := range q.workspaces { + if other.Deleted || other.ID == workspace.ID || workspace.OwnerID != other.OwnerID { + continue + } + if other.Name == arg.Name { + return &pq.Error{Code: "23505", Message: "duplicate key value violates unique constraint"} + } + } + + workspace.Name = arg.Name + q.workspaces[i] = workspace + + return nil + } + + return sql.ErrNoRows +} + func (q *fakeQuerier) UpdateWorkspaceAutostart(_ context.Context, arg database.UpdateWorkspaceAutostartParams) error { q.mutex.Lock() defer q.mutex.Unlock() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index cddf33e33d427..359491bc9e4a5 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -137,6 +137,7 @@ type querier interface { UpdateUserProfile(ctx context.Context, arg UpdateUserProfileParams) (User, error) UpdateUserRoles(ctx context.Context, arg UpdateUserRolesParams) (User, error) UpdateUserStatus(ctx context.Context, arg UpdateUserStatusParams) (User, error) + UpdateWorkspace(ctx context.Context, arg UpdateWorkspaceParams) error UpdateWorkspaceAgentConnectionByID(ctx context.Context, arg UpdateWorkspaceAgentConnectionByIDParams) error UpdateWorkspaceAgentKeysByID(ctx context.Context, arg UpdateWorkspaceAgentKeysByIDParams) error UpdateWorkspaceAutostart(ctx context.Context, arg UpdateWorkspaceAutostartParams) error diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 3e6643781d30a..fa9be4e3a46a9 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -4685,6 +4685,26 @@ func (q *sqlQuerier) InsertWorkspace(ctx context.Context, arg InsertWorkspacePar return i, err } +const updateWorkspace = `-- name: UpdateWorkspace :exec +UPDATE + workspaces +SET + name = $2 +WHERE + id = $1 + AND deleted = false +` + +type UpdateWorkspaceParams struct { + ID uuid.UUID `db:"id" json:"id"` + Name string `db:"name" json:"name"` +} + +func (q *sqlQuerier) UpdateWorkspace(ctx context.Context, arg UpdateWorkspaceParams) error { + _, err := q.db.ExecContext(ctx, updateWorkspace, arg.ID, arg.Name) + return err +} + const updateWorkspaceAutostart = `-- name: UpdateWorkspaceAutostart :exec UPDATE workspaces diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 5cb6508a12111..9fb63e5ae44ae 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -112,6 +112,15 @@ SET WHERE id = $1; +-- name: UpdateWorkspace :exec +UPDATE + workspaces +SET + name = $2 +WHERE + id = $1 + AND deleted = false; + -- name: UpdateWorkspaceAutostart :exec UPDATE workspaces diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 3b0e7a486a482..011293cf78fa0 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -14,6 +14,7 @@ import ( "github.com/go-chi/chi/v5" "github.com/google/uuid" + "github.com/lib/pq" "github.com/moby/moby/pkg/namesgenerator" "golang.org/x/sync/errgroup" "golang.org/x/xerrors" @@ -323,17 +324,8 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req }) if err == nil { // If the workspace already exists, don't allow creation. - template, err := api.Database.GetTemplateByID(r.Context(), workspace.TemplateID) - if err != nil { - httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{ - Message: fmt.Sprintf("Find template for conflicting workspace name %q.", createWorkspace.Name), - Detail: err.Error(), - }) - return - } - // The template is fetched for clarity to the user on where the conflicting name may be. httpapi.Write(rw, http.StatusConflict, codersdk.Response{ - Message: fmt.Sprintf("Workspace %q already exists in the %q template.", createWorkspace.Name, template.Name), + Message: fmt.Sprintf("Workspace %q already exists.", createWorkspace.Name), Validations: []codersdk.ValidationError{{ Field: "name", Detail: "This value is already in use and should be unique.", @@ -486,6 +478,72 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req findUser(apiKey.UserID, users), findUser(workspaceBuild.InitiatorID, users))) } +func (api *API) patchWorkspace(rw http.ResponseWriter, r *http.Request) { + workspace := httpmw.WorkspaceParam(r) + if !api.Authorize(r, rbac.ActionUpdate, workspace) { + httpapi.ResourceNotFound(rw) + return + } + + var req codersdk.UpdateWorkspaceRequest + if !httpapi.Read(rw, r, &req) { + return + } + + if req.Name == "" || req.Name == workspace.Name { + // Nothing changed, optionally this could be an error. + rw.WriteHeader(http.StatusNoContent) + return + } + // The reason we double check here is in case more fields can be + // patched in the future, it's enough if one changes. + name := workspace.Name + if req.Name != "" || req.Name != workspace.Name { + name = req.Name + } + + err := api.Database.UpdateWorkspace(r.Context(), database.UpdateWorkspaceParams{ + ID: workspace.ID, + Name: name, + }) + if err != nil { + // The query protects against updating deleted workspaces and + // the existence of the workspace is checked in the request, + // the only conclusion we can make is that we're trying to + // update a deleted workspace. + // + // We could do this check earlier but since we're not in a + // transaction, it's pointless. + if errors.Is(err, sql.ErrNoRows) { + httpapi.Write(rw, http.StatusMethodNotAllowed, codersdk.Response{ + Message: fmt.Sprintf("Workspace %q is deleted and cannot be updated.", workspace.Name), + }) + return + } + // Check if we triggered the one-unique-name-per-owner + // constraint. + var pqErr *pq.Error + if errors.As(err, &pqErr) && pqErr.Code.Name() == "unique_violation" { + httpapi.Write(rw, http.StatusConflict, codersdk.Response{ + Message: fmt.Sprintf("Workspace %q already exists.", req.Name), + Validations: []codersdk.ValidationError{{ + Field: "name", + Detail: "This value is already in use and should be unique.", + }}, + }) + return + } + + httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error updating workspace.", + Detail: err.Error(), + }) + return + } + + rw.WriteHeader(http.StatusNoContent) +} + func (api *API) putWorkspaceAutostart(rw http.ResponseWriter, r *http.Request) { workspace := httpmw.WorkspaceParam(r) if !api.Authorize(r, rbac.ActionUpdate, workspace) { @@ -563,7 +621,6 @@ func (api *API) putWorkspaceTTL(rw http.ResponseWriter, r *http.Request) { return nil }) - if err != nil { resp := codersdk.Response{ Message: "Error updating workspace time until shutdown.", @@ -663,7 +720,6 @@ func (api *API) putExtendWorkspace(rw http.ResponseWriter, r *http.Request) { return nil }) - if err != nil { api.Logger.Info(r.Context(), "extending workspace", slog.Error(err)) } @@ -868,6 +924,7 @@ func convertWorkspaces(ctx context.Context, db database.Store, workspaces []data } return apiWorkspaces, nil } + func convertWorkspace( workspace database.Workspace, workspaceBuild database.WorkspaceBuild, diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index db47eb796bcd5..c431565cb4e33 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -77,6 +77,37 @@ func TestWorkspace(t *testing.T) { require.Error(t, err) require.ErrorContains(t, err, "410") // gone }) + + t.Run("Rename", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) + user := coderdtest.CreateFirstUser(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + ws1 := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + ws2 := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + coderdtest.AwaitWorkspaceBuildJob(t, client, ws1.LatestBuild.ID) + coderdtest.AwaitWorkspaceBuildJob(t, client, ws2.LatestBuild.ID) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitMedium) + defer cancel() + + want := ws1.Name + "_2" + err := client.UpdateWorkspace(ctx, ws1.ID, codersdk.UpdateWorkspaceRequest{ + Name: want, + }) + require.NoError(t, err, "workspace rename failed") + + ws, err := client.Workspace(ctx, ws1.ID) + require.NoError(t, err) + require.Equal(t, want, ws.Name, "workspace name not updated") + + err = client.UpdateWorkspace(ctx, ws1.ID, codersdk.UpdateWorkspaceRequest{ + Name: ws2.Name, + }) + require.Error(t, err, "workspace rename should have failed") + }) } func TestAdminViewAllWorkspaces(t *testing.T) { diff --git a/codersdk/workspaces.go b/codersdk/workspaces.go index fb2b56e4dfbf1..07da2e394dabc 100644 --- a/codersdk/workspaces.go +++ b/codersdk/workspaces.go @@ -162,6 +162,23 @@ func (c *Client) WatchWorkspace(ctx context.Context, id uuid.UUID) (<-chan Works return wc, nil } +type UpdateWorkspaceRequest struct { + Name string `json:"name,omitempty" validate:"username"` +} + +func (c *Client) UpdateWorkspace(ctx context.Context, id uuid.UUID, req UpdateWorkspaceRequest) error { + path := fmt.Sprintf("/api/v2/workspaces/%s", id.String()) + res, err := c.Request(ctx, http.MethodPatch, path, req) + if err != nil { + return xerrors.Errorf("update workspace: %w", err) + } + defer res.Body.Close() + if res.StatusCode != http.StatusNoContent { + return readBodyAsError(res) + } + return nil +} + // UpdateWorkspaceAutostartRequest is a request to update a workspace's autostart schedule. type UpdateWorkspaceAutostartRequest struct { Schedule *string `json:"schedule"` @@ -263,7 +280,6 @@ func (f WorkspaceFilter) asRequestOption() requestOption { // Workspaces returns all workspaces the authenticated user has access to. func (c *Client) Workspaces(ctx context.Context, filter WorkspaceFilter) ([]Workspace, error) { res, err := c.Request(ctx, http.MethodGet, "/api/v2/workspaces", nil, filter.asRequestOption()) - if err != nil { return nil, err } diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 38ee495dbf103..999e0095c5540 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -370,6 +370,11 @@ export interface UpdateWorkspaceAutostartRequest { readonly schedule?: string } +// From codersdk/workspaces.go +export interface UpdateWorkspaceRequest { + readonly name?: string +} + // From codersdk/workspaces.go export interface UpdateWorkspaceTTLRequest { readonly ttl_ms?: number From c8adca1ad8b5dd94c98384477ddbd1129d7adbb9 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 4 Aug 2022 16:59:43 +0300 Subject: [PATCH 02/16] wip: Add rename command --- cli/rename.go | 34 ++++++++++++++++++++++++++++++++++ cli/rename_test.go | 44 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) create mode 100644 cli/rename.go create mode 100644 cli/rename_test.go diff --git a/cli/rename.go b/cli/rename.go new file mode 100644 index 0000000000000..41410929d8742 --- /dev/null +++ b/cli/rename.go @@ -0,0 +1,34 @@ +package cli + +import ( + "github.com/spf13/cobra" + "golang.org/x/xerrors" + + "github.com/coder/coder/codersdk" +) + +func rename() *cobra.Command { + return &cobra.Command{ + Annotations: workspaceCommand, + Use: "rename ", + Short: "Rename a workspace", + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + client, err := createClient(cmd) + if err != nil { + return err + } + workspace, err := namedWorkspace(cmd, client, args[0]) + if err != nil { + return xerrors.Errorf("get workspace: %w", err) + } + err = client.UpdateWorkspace(cmd.Context(), workspace.ID, codersdk.UpdateWorkspaceRequest{ + Name: args[1], + }) + if err != nil { + return xerrors.Errorf("rename workspace: %w", err) + } + return nil + }, + } +} diff --git a/cli/rename_test.go b/cli/rename_test.go new file mode 100644 index 0000000000000..ada98646a835b --- /dev/null +++ b/cli/rename_test.go @@ -0,0 +1,44 @@ +package cli_test + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/coder/coder/cli/clitest" + "github.com/coder/coder/coderd/coderdtest" + "github.com/coder/coder/pty/ptytest" + "github.com/coder/coder/testutil" +) + +func TestRename(t *testing.T) { + t.Parallel() + + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) + user := coderdtest.CreateFirstUser(t, client) + version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) + coderdtest.AwaitTemplateVersionJob(t, client, version.ID) + template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) + workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) + coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + want := workspace.Name + "-test" + cmd, root := clitest.New(t, "rename", workspace.Name, want) + clitest.SetupConfig(t, client, root) + pty := ptytest.New(t) + cmd.SetIn(pty.Input()) + cmd.SetOut(pty.Output()) + + err := cmd.ExecuteContext(ctx) + assert.NoError(t, err) + + ws, err := client.Workspace(ctx, workspace.ID) + assert.NoError(t, err) + + got := ws.Name + assert.Equal(t, want, got, "workspace name did not change") +} From 3f8e7800ca805d3c7ceeb0c780a1f17544d887b6 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Mon, 8 Aug 2022 18:54:40 +0300 Subject: [PATCH 03/16] fix: Test --- coderd/workspaces_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go index c431565cb4e33..5139034bf2638 100644 --- a/coderd/workspaces_test.go +++ b/coderd/workspaces_test.go @@ -93,7 +93,7 @@ func TestWorkspace(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitMedium) defer cancel() - want := ws1.Name + "_2" + want := ws1.Name + "-test" err := client.UpdateWorkspace(ctx, ws1.ID, codersdk.UpdateWorkspaceRequest{ Name: want, }) From a1da1fe7c9e4a54b1bc1ede04c70a173e66b1357 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 9 Aug 2022 16:11:01 +0300 Subject: [PATCH 04/16] feat: Implement database.IsUniqueViolation --- coderd/database/errors.go | 30 ++++++++++++++++++++++++++++++ coderd/workspaces.go | 8 ++------ 2 files changed, 32 insertions(+), 6 deletions(-) create mode 100644 coderd/database/errors.go diff --git a/coderd/database/errors.go b/coderd/database/errors.go new file mode 100644 index 0000000000000..1446006ff77f3 --- /dev/null +++ b/coderd/database/errors.go @@ -0,0 +1,30 @@ +package database + +import ( + "errors" + + "github.com/lib/pq" +) + +// UniqueConstraint represents a named unique constraint on a table. +type UniqueConstraint string + +// UniqueConstrain enums. +// TODO(mafredri): Generate these from the database schema. +const ( + UniqueConstraintAny UniqueConstraint = "" + UniqueConstraintWorkspacesOwnerIDLowerIdx UniqueConstraint = "workspaces_owner_id_lower_idx" +) + +func IsUniqueViolation(err error, uniqueConstraint UniqueConstraint) bool { + var pqErr *pq.Error + if errors.As(err, &pqErr) { + if pqErr.Code.Name() == "unique_violation" { + if pqErr.Constraint == string(uniqueConstraint) || uniqueConstraint == UniqueConstraintAny { + return true + } + } + } + + return false +} diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 011293cf78fa0..4db4fa6968f5b 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -14,7 +14,6 @@ import ( "github.com/go-chi/chi/v5" "github.com/google/uuid" - "github.com/lib/pq" "github.com/moby/moby/pkg/namesgenerator" "golang.org/x/sync/errgroup" "golang.org/x/xerrors" @@ -520,10 +519,8 @@ func (api *API) patchWorkspace(rw http.ResponseWriter, r *http.Request) { }) return } - // Check if we triggered the one-unique-name-per-owner - // constraint. - var pqErr *pq.Error - if errors.As(err, &pqErr) && pqErr.Code.Name() == "unique_violation" { + // Check if the name was already in use. + if database.IsUniqueViolation(err, database.UniqueConstraintWorkspacesOwnerIDLowerIdx) { httpapi.Write(rw, http.StatusConflict, codersdk.Response{ Message: fmt.Sprintf("Workspace %q already exists.", req.Name), Validations: []codersdk.ValidationError{{ @@ -533,7 +530,6 @@ func (api *API) patchWorkspace(rw http.ResponseWriter, r *http.Request) { }) return } - httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error updating workspace.", Detail: err.Error(), From 0b23095abd92f97d2cb51a7551aa8f9013c788b7 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 9 Aug 2022 16:12:50 +0300 Subject: [PATCH 05/16] fix: Typo --- coderd/database/errors.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/database/errors.go b/coderd/database/errors.go index 1446006ff77f3..de6960fc6bd6f 100644 --- a/coderd/database/errors.go +++ b/coderd/database/errors.go @@ -9,7 +9,7 @@ import ( // UniqueConstraint represents a named unique constraint on a table. type UniqueConstraint string -// UniqueConstrain enums. +// UniqueConstraint enums. // TODO(mafredri): Generate these from the database schema. const ( UniqueConstraintAny UniqueConstraint = "" From 055d05afaa4b5d94303e92405a93d233ca74a137 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 9 Aug 2022 16:21:39 +0300 Subject: [PATCH 06/16] chore: Refactor to allow matching on multiple constraints --- coderd/database/errors.go | 16 ++++++++++++---- coderd/workspaces.go | 2 +- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/coderd/database/errors.go b/coderd/database/errors.go index de6960fc6bd6f..244b9d73f417d 100644 --- a/coderd/database/errors.go +++ b/coderd/database/errors.go @@ -12,17 +12,25 @@ type UniqueConstraint string // UniqueConstraint enums. // TODO(mafredri): Generate these from the database schema. const ( - UniqueConstraintAny UniqueConstraint = "" - UniqueConstraintWorkspacesOwnerIDLowerIdx UniqueConstraint = "workspaces_owner_id_lower_idx" + UniqueWorkspacesOwnerIDLowerIdx UniqueConstraint = "workspaces_owner_id_lower_idx" ) -func IsUniqueViolation(err error, uniqueConstraint UniqueConstraint) bool { +// IsUniqueViolation checks if the error is due to a unique violation. +// If specific cunique constraints are given as arguments, the error +// must be caused by one of them. If no constraints are given, this +// function returns true on any unique violation. +func IsUniqueViolation(err error, uniqueConstraints ...UniqueConstraint) bool { var pqErr *pq.Error if errors.As(err, &pqErr) { if pqErr.Code.Name() == "unique_violation" { - if pqErr.Constraint == string(uniqueConstraint) || uniqueConstraint == UniqueConstraintAny { + if len(uniqueConstraints) == 0 { return true } + for _, uc := range uniqueConstraints { + if pqErr.Constraint == string(uc) { + return true + } + } } } diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 4db4fa6968f5b..18287e6c82227 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -520,7 +520,7 @@ func (api *API) patchWorkspace(rw http.ResponseWriter, r *http.Request) { return } // Check if the name was already in use. - if database.IsUniqueViolation(err, database.UniqueConstraintWorkspacesOwnerIDLowerIdx) { + if database.IsUniqueViolation(err, database.UniqueWorkspacesOwnerIDLowerIdx) { httpapi.Write(rw, http.StatusConflict, codersdk.Response{ Message: fmt.Sprintf("Workspace %q already exists.", req.Name), Validations: []codersdk.ValidationError{{ From 721eb876b18ef233f8c6dc3f5b9fac1ce9b4a712 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 9 Aug 2022 16:23:13 +0300 Subject: [PATCH 07/16] chore: Typos --- coderd/database/errors.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/coderd/database/errors.go b/coderd/database/errors.go index 244b9d73f417d..e7333e4c6635b 100644 --- a/coderd/database/errors.go +++ b/coderd/database/errors.go @@ -16,9 +16,9 @@ const ( ) // IsUniqueViolation checks if the error is due to a unique violation. -// If specific cunique constraints are given as arguments, the error -// must be caused by one of them. If no constraints are given, this -// function returns true on any unique violation. +// If one or more specific unique constraints are given as arguments, +// the error must be caused by one of them. If no constraints are given, +// this function returns true for any unique violation. func IsUniqueViolation(err error, uniqueConstraints ...UniqueConstraint) bool { var pqErr *pq.Error if errors.As(err, &pqErr) { From 9320850009552754bdae7f047bef64dc0428bc1a Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 9 Aug 2022 19:05:53 +0300 Subject: [PATCH 08/16] fix: Properly handle deletion / affected rows --- coderd/database/querier.go | 2 +- coderd/database/queries.sql.go | 13 +++++++++---- coderd/database/queries/workspaces.sql | 4 +++- coderd/workspaces.go | 27 +++++++++++++------------- 4 files changed, 26 insertions(+), 20 deletions(-) diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 359491bc9e4a5..560355bf9c08f 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -137,7 +137,7 @@ type querier interface { UpdateUserProfile(ctx context.Context, arg UpdateUserProfileParams) (User, error) UpdateUserRoles(ctx context.Context, arg UpdateUserRolesParams) (User, error) UpdateUserStatus(ctx context.Context, arg UpdateUserStatusParams) (User, error) - UpdateWorkspace(ctx context.Context, arg UpdateWorkspaceParams) error + UpdateWorkspace(ctx context.Context, arg UpdateWorkspaceParams) (int64, error) UpdateWorkspaceAgentConnectionByID(ctx context.Context, arg UpdateWorkspaceAgentConnectionByIDParams) error UpdateWorkspaceAgentKeysByID(ctx context.Context, arg UpdateWorkspaceAgentKeysByIDParams) error UpdateWorkspaceAutostart(ctx context.Context, arg UpdateWorkspaceAutostartParams) error diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index fa9be4e3a46a9..e2531b8a790f3 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -4685,13 +4685,15 @@ func (q *sqlQuerier) InsertWorkspace(ctx context.Context, arg InsertWorkspacePar return i, err } -const updateWorkspace = `-- name: UpdateWorkspace :exec +const updateWorkspace = `-- name: UpdateWorkspace :execrows UPDATE workspaces SET name = $2 WHERE id = $1 + -- This can result in rows affected being zero, the caller should + -- handle this case. AND deleted = false ` @@ -4700,9 +4702,12 @@ type UpdateWorkspaceParams struct { Name string `db:"name" json:"name"` } -func (q *sqlQuerier) UpdateWorkspace(ctx context.Context, arg UpdateWorkspaceParams) error { - _, err := q.db.ExecContext(ctx, updateWorkspace, arg.ID, arg.Name) - return err +func (q *sqlQuerier) UpdateWorkspace(ctx context.Context, arg UpdateWorkspaceParams) (int64, error) { + result, err := q.db.ExecContext(ctx, updateWorkspace, arg.ID, arg.Name) + if err != nil { + return 0, err + } + return result.RowsAffected() } const updateWorkspaceAutostart = `-- name: UpdateWorkspaceAutostart :exec diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 9fb63e5ae44ae..81efbc7d24909 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -112,13 +112,15 @@ SET WHERE id = $1; --- name: UpdateWorkspace :exec +-- name: UpdateWorkspace :execrows UPDATE workspaces SET name = $2 WHERE id = $1 + -- This can result in rows affected being zero, the caller should + -- handle this case. AND deleted = false; -- name: UpdateWorkspaceAutostart :exec diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 18287e6c82227..9c9a66b44f712 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -501,24 +501,11 @@ func (api *API) patchWorkspace(rw http.ResponseWriter, r *http.Request) { name = req.Name } - err := api.Database.UpdateWorkspace(r.Context(), database.UpdateWorkspaceParams{ + n, err := api.Database.UpdateWorkspace(r.Context(), database.UpdateWorkspaceParams{ ID: workspace.ID, Name: name, }) if err != nil { - // The query protects against updating deleted workspaces and - // the existence of the workspace is checked in the request, - // the only conclusion we can make is that we're trying to - // update a deleted workspace. - // - // We could do this check earlier but since we're not in a - // transaction, it's pointless. - if errors.Is(err, sql.ErrNoRows) { - httpapi.Write(rw, http.StatusMethodNotAllowed, codersdk.Response{ - Message: fmt.Sprintf("Workspace %q is deleted and cannot be updated.", workspace.Name), - }) - return - } // Check if the name was already in use. if database.IsUniqueViolation(err, database.UniqueWorkspacesOwnerIDLowerIdx) { httpapi.Write(rw, http.StatusConflict, codersdk.Response{ @@ -536,6 +523,18 @@ func (api *API) patchWorkspace(rw http.ResponseWriter, r *http.Request) { }) return } + // If no rows were affected, the workspace must have been deleted + // between the start of the request and the query updating the + // workspace name. + // + // We could do this check earlier but we'd have to start a + // transaction to ensure consistency. + if n == 0 { + httpapi.Write(rw, http.StatusMethodNotAllowed, codersdk.Response{ + Message: fmt.Sprintf("Workspace %q is deleted and cannot be updated.", workspace.Name), + }) + return + } rw.WriteHeader(http.StatusNoContent) } From 256b7ef4b92888b4ae00f4a50fa0b3e274fc2e4d Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 9 Aug 2022 19:24:39 +0300 Subject: [PATCH 09/16] Revert "fix: Properly handle deletion / affected rows" This reverts commit 5ccd7a7afb264c6b97b232fe03cfc0ca9d67703d. --- coderd/database/querier.go | 2 +- coderd/database/queries.sql.go | 13 ++++--------- coderd/database/queries/workspaces.sql | 4 +--- coderd/workspaces.go | 27 +++++++++++++------------- 4 files changed, 20 insertions(+), 26 deletions(-) diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 560355bf9c08f..359491bc9e4a5 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -137,7 +137,7 @@ type querier interface { UpdateUserProfile(ctx context.Context, arg UpdateUserProfileParams) (User, error) UpdateUserRoles(ctx context.Context, arg UpdateUserRolesParams) (User, error) UpdateUserStatus(ctx context.Context, arg UpdateUserStatusParams) (User, error) - UpdateWorkspace(ctx context.Context, arg UpdateWorkspaceParams) (int64, error) + UpdateWorkspace(ctx context.Context, arg UpdateWorkspaceParams) error UpdateWorkspaceAgentConnectionByID(ctx context.Context, arg UpdateWorkspaceAgentConnectionByIDParams) error UpdateWorkspaceAgentKeysByID(ctx context.Context, arg UpdateWorkspaceAgentKeysByIDParams) error UpdateWorkspaceAutostart(ctx context.Context, arg UpdateWorkspaceAutostartParams) error diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index e2531b8a790f3..fa9be4e3a46a9 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -4685,15 +4685,13 @@ func (q *sqlQuerier) InsertWorkspace(ctx context.Context, arg InsertWorkspacePar return i, err } -const updateWorkspace = `-- name: UpdateWorkspace :execrows +const updateWorkspace = `-- name: UpdateWorkspace :exec UPDATE workspaces SET name = $2 WHERE id = $1 - -- This can result in rows affected being zero, the caller should - -- handle this case. AND deleted = false ` @@ -4702,12 +4700,9 @@ type UpdateWorkspaceParams struct { Name string `db:"name" json:"name"` } -func (q *sqlQuerier) UpdateWorkspace(ctx context.Context, arg UpdateWorkspaceParams) (int64, error) { - result, err := q.db.ExecContext(ctx, updateWorkspace, arg.ID, arg.Name) - if err != nil { - return 0, err - } - return result.RowsAffected() +func (q *sqlQuerier) UpdateWorkspace(ctx context.Context, arg UpdateWorkspaceParams) error { + _, err := q.db.ExecContext(ctx, updateWorkspace, arg.ID, arg.Name) + return err } const updateWorkspaceAutostart = `-- name: UpdateWorkspaceAutostart :exec diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 81efbc7d24909..9fb63e5ae44ae 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -112,15 +112,13 @@ SET WHERE id = $1; --- name: UpdateWorkspace :execrows +-- name: UpdateWorkspace :exec UPDATE workspaces SET name = $2 WHERE id = $1 - -- This can result in rows affected being zero, the caller should - -- handle this case. AND deleted = false; -- name: UpdateWorkspaceAutostart :exec diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 9c9a66b44f712..18287e6c82227 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -501,11 +501,24 @@ func (api *API) patchWorkspace(rw http.ResponseWriter, r *http.Request) { name = req.Name } - n, err := api.Database.UpdateWorkspace(r.Context(), database.UpdateWorkspaceParams{ + err := api.Database.UpdateWorkspace(r.Context(), database.UpdateWorkspaceParams{ ID: workspace.ID, Name: name, }) if err != nil { + // The query protects against updating deleted workspaces and + // the existence of the workspace is checked in the request, + // the only conclusion we can make is that we're trying to + // update a deleted workspace. + // + // We could do this check earlier but since we're not in a + // transaction, it's pointless. + if errors.Is(err, sql.ErrNoRows) { + httpapi.Write(rw, http.StatusMethodNotAllowed, codersdk.Response{ + Message: fmt.Sprintf("Workspace %q is deleted and cannot be updated.", workspace.Name), + }) + return + } // Check if the name was already in use. if database.IsUniqueViolation(err, database.UniqueWorkspacesOwnerIDLowerIdx) { httpapi.Write(rw, http.StatusConflict, codersdk.Response{ @@ -523,18 +536,6 @@ func (api *API) patchWorkspace(rw http.ResponseWriter, r *http.Request) { }) return } - // If no rows were affected, the workspace must have been deleted - // between the start of the request and the query updating the - // workspace name. - // - // We could do this check earlier but we'd have to start a - // transaction to ensure consistency. - if n == 0 { - httpapi.Write(rw, http.StatusMethodNotAllowed, codersdk.Response{ - Message: fmt.Sprintf("Workspace %q is deleted and cannot be updated.", workspace.Name), - }) - return - } rw.WriteHeader(http.StatusNoContent) } From 0455b67b8d46569d201c0208abfab102e54ac95a Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 9 Aug 2022 19:27:52 +0300 Subject: [PATCH 10/16] fix: Return the updated row and handle sql.ErrNoRows --- coderd/database/querier.go | 2 +- coderd/database/queries.sql.go | 22 ++++++++++++++++++---- coderd/database/queries/workspaces.sql | 5 +++-- coderd/workspaces.go | 9 ++++----- 4 files changed, 26 insertions(+), 12 deletions(-) diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 359491bc9e4a5..a10b108b0352c 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -137,7 +137,7 @@ type querier interface { UpdateUserProfile(ctx context.Context, arg UpdateUserProfileParams) (User, error) UpdateUserRoles(ctx context.Context, arg UpdateUserRolesParams) (User, error) UpdateUserStatus(ctx context.Context, arg UpdateUserStatusParams) (User, error) - UpdateWorkspace(ctx context.Context, arg UpdateWorkspaceParams) error + UpdateWorkspace(ctx context.Context, arg UpdateWorkspaceParams) (Workspace, error) UpdateWorkspaceAgentConnectionByID(ctx context.Context, arg UpdateWorkspaceAgentConnectionByIDParams) error UpdateWorkspaceAgentKeysByID(ctx context.Context, arg UpdateWorkspaceAgentKeysByIDParams) error UpdateWorkspaceAutostart(ctx context.Context, arg UpdateWorkspaceAutostartParams) error diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index fa9be4e3a46a9..880f550813e65 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -4685,7 +4685,7 @@ func (q *sqlQuerier) InsertWorkspace(ctx context.Context, arg InsertWorkspacePar return i, err } -const updateWorkspace = `-- name: UpdateWorkspace :exec +const updateWorkspace = `-- name: UpdateWorkspace :one UPDATE workspaces SET @@ -4693,6 +4693,7 @@ SET WHERE id = $1 AND deleted = false +RETURNING id, created_at, updated_at, owner_id, organization_id, template_id, deleted, name, autostart_schedule, ttl ` type UpdateWorkspaceParams struct { @@ -4700,9 +4701,22 @@ type UpdateWorkspaceParams struct { Name string `db:"name" json:"name"` } -func (q *sqlQuerier) UpdateWorkspace(ctx context.Context, arg UpdateWorkspaceParams) error { - _, err := q.db.ExecContext(ctx, updateWorkspace, arg.ID, arg.Name) - return err +func (q *sqlQuerier) UpdateWorkspace(ctx context.Context, arg UpdateWorkspaceParams) (Workspace, error) { + row := q.db.QueryRowContext(ctx, updateWorkspace, arg.ID, arg.Name) + var i Workspace + err := row.Scan( + &i.ID, + &i.CreatedAt, + &i.UpdatedAt, + &i.OwnerID, + &i.OrganizationID, + &i.TemplateID, + &i.Deleted, + &i.Name, + &i.AutostartSchedule, + &i.Ttl, + ) + return i, err } const updateWorkspaceAutostart = `-- name: UpdateWorkspaceAutostart :exec diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 9fb63e5ae44ae..8de76e258415e 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -112,14 +112,15 @@ SET WHERE id = $1; --- name: UpdateWorkspace :exec +-- name: UpdateWorkspace :one UPDATE workspaces SET name = $2 WHERE id = $1 - AND deleted = false; + AND deleted = false +RETURNING *; -- name: UpdateWorkspaceAutostart :exec UPDATE diff --git a/coderd/workspaces.go b/coderd/workspaces.go index 18287e6c82227..20652f919edfd 100644 --- a/coderd/workspaces.go +++ b/coderd/workspaces.go @@ -501,18 +501,17 @@ func (api *API) patchWorkspace(rw http.ResponseWriter, r *http.Request) { name = req.Name } - err := api.Database.UpdateWorkspace(r.Context(), database.UpdateWorkspaceParams{ + _, err := api.Database.UpdateWorkspace(r.Context(), database.UpdateWorkspaceParams{ ID: workspace.ID, Name: name, }) if err != nil { // The query protects against updating deleted workspaces and // the existence of the workspace is checked in the request, - // the only conclusion we can make is that we're trying to - // update a deleted workspace. + // if we get ErrNoRows it means the workspace was deleted. // - // We could do this check earlier but since we're not in a - // transaction, it's pointless. + // We could do this check earlier but we'd need to start a + // transaction. if errors.Is(err, sql.ErrNoRows) { httpapi.Write(rw, http.StatusMethodNotAllowed, codersdk.Response{ Message: fmt.Sprintf("Workspace %q is deleted and cannot be updated.", workspace.Name), From 7ce1b3bff5871cd481c5eaa77f3ca85f74c11bf3 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 9 Aug 2022 19:33:32 +0300 Subject: [PATCH 11/16] fix: Database fake --- coderd/database/databasefake/databasefake.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 781f493e148fa..48860c2a90654 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -2091,7 +2091,7 @@ func (q *fakeQuerier) UpdateProvisionerJobWithCompleteByID(_ context.Context, ar return sql.ErrNoRows } -func (q *fakeQuerier) UpdateWorkspace(_ context.Context, arg database.UpdateWorkspaceParams) error { +func (q *fakeQuerier) UpdateWorkspace(_ context.Context, arg database.UpdateWorkspaceParams) (database.Workspace, error) { q.mutex.Lock() defer q.mutex.Unlock() @@ -2104,17 +2104,17 @@ func (q *fakeQuerier) UpdateWorkspace(_ context.Context, arg database.UpdateWork continue } if other.Name == arg.Name { - return &pq.Error{Code: "23505", Message: "duplicate key value violates unique constraint"} + return database.Workspace{}, &pq.Error{Code: "23505", Message: "duplicate key value violates unique constraint"} } } workspace.Name = arg.Name q.workspaces[i] = workspace - return nil + return workspace, nil } - return sql.ErrNoRows + return database.Workspace{}, sql.ErrNoRows } func (q *fakeQuerier) UpdateWorkspaceAutostart(_ context.Context, arg database.UpdateWorkspaceAutostartParams) error { From f8dcf51ce26356f3e239ca7cd17085e7aa0fe295 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 24 Aug 2022 15:47:09 +0300 Subject: [PATCH 12/16] fix: Add data loss warning --- cli/rename.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/cli/rename.go b/cli/rename.go index 41410929d8742..a188f1e54c22d 100644 --- a/cli/rename.go +++ b/cli/rename.go @@ -4,6 +4,7 @@ import ( "github.com/spf13/cobra" "golang.org/x/xerrors" + "github.com/coder/coder/cli/cliui" "github.com/coder/coder/codersdk" ) @@ -14,7 +15,7 @@ func rename() *cobra.Command { Short: "Rename a workspace", Args: cobra.ExactArgs(2), RunE: func(cmd *cobra.Command, args []string) error { - client, err := createClient(cmd) + client, err := CreateClient(cmd) if err != nil { return err } @@ -22,6 +23,15 @@ func rename() *cobra.Command { if err != nil { return xerrors.Errorf("get workspace: %w", err) } + + _, err = cliui.Prompt(cmd, cliui.PromptOptions{ + Text: "WARNING: A rename can result in loss of home volume if the template references the workspace name. Continue?", + IsConfirm: true, + }) + if err != nil { + return err + } + err = client.UpdateWorkspace(cmd.Context(), workspace.ID, codersdk.UpdateWorkspaceRequest{ Name: args[1], }) From 846333bb58e6c6e71d429091a7bca0f1c42eaf4a Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 24 Aug 2022 15:53:52 +0300 Subject: [PATCH 13/16] fix: Skip prompt in rename test --- cli/rename_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/rename_test.go b/cli/rename_test.go index ada98646a835b..92f1f4fd2b264 100644 --- a/cli/rename_test.go +++ b/cli/rename_test.go @@ -27,7 +27,7 @@ func TestRename(t *testing.T) { defer cancel() want := workspace.Name + "-test" - cmd, root := clitest.New(t, "rename", workspace.Name, want) + cmd, root := clitest.New(t, "rename", workspace.Name, want, "--yes") clitest.SetupConfig(t, client, root) pty := ptytest.New(t) cmd.SetIn(pty.Input()) From f8dad7184bde363a56989d62f6161ec7e126e6ef Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 24 Aug 2022 16:29:34 +0300 Subject: [PATCH 14/16] fix: Allow skip prompt --- cli/rename.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cli/rename.go b/cli/rename.go index a188f1e54c22d..25b8a4574ba97 100644 --- a/cli/rename.go +++ b/cli/rename.go @@ -9,7 +9,7 @@ import ( ) func rename() *cobra.Command { - return &cobra.Command{ + cmd := &cobra.Command{ Annotations: workspaceCommand, Use: "rename ", Short: "Rename a workspace", @@ -41,4 +41,8 @@ func rename() *cobra.Command { return nil }, } + + cliui.AllowSkipPrompt(cmd) + + return cmd } From 54771b81fd305b43c18f1aec678798f39176ef24 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 24 Aug 2022 17:11:21 +0300 Subject: [PATCH 15/16] fix: Amend PR comments --- cli/rename.go | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/cli/rename.go b/cli/rename.go index 25b8a4574ba97..168dcffb3e553 100644 --- a/cli/rename.go +++ b/cli/rename.go @@ -1,6 +1,8 @@ package cli import ( + "fmt" + "github.com/spf13/cobra" "golang.org/x/xerrors" @@ -14,6 +16,10 @@ func rename() *cobra.Command { Use: "rename ", Short: "Rename a workspace", Args: cobra.ExactArgs(2), + // Keep hidden until renaming is safe, see: + // * https://github.com/coder/coder/issues/3000 + // * https://github.com/coder/coder/issues/3386 + Hidden: true, RunE: func(cmd *cobra.Command, args []string) error { client, err := CreateClient(cmd) if err != nil { @@ -24,9 +30,17 @@ func rename() *cobra.Command { return xerrors.Errorf("get workspace: %w", err) } + _, _ = fmt.Fprintf(cmd.OutOrStdout(), "%s\n\n", + cliui.Styles.Wrap.Render("WARNING: A rename can result in data loss if a resource references the workspace name in the template (e.g volumes)."), + ) _, err = cliui.Prompt(cmd, cliui.PromptOptions{ - Text: "WARNING: A rename can result in loss of home volume if the template references the workspace name. Continue?", - IsConfirm: true, + Text: fmt.Sprintf("Type %q to confirm rename:", workspace.Name), + Validate: func(s string) error { + if s == workspace.Name { + return nil + } + return xerrors.Errorf("Input %q does not match %q", s, workspace.Name) + }, }) if err != nil { return err From 6b221ffd61f602706f1bca066a8b270f8bb95380 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 24 Aug 2022 17:20:36 +0300 Subject: [PATCH 16/16] fix: Rename test --- cli/rename_test.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/cli/rename_test.go b/cli/rename_test.go index 92f1f4fd2b264..fd0b019ae6395 100644 --- a/cli/rename_test.go +++ b/cli/rename_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/coder/coder/cli/clitest" "github.com/coder/coder/coderd/coderdtest" @@ -33,8 +34,15 @@ func TestRename(t *testing.T) { cmd.SetIn(pty.Input()) cmd.SetOut(pty.Output()) - err := cmd.ExecuteContext(ctx) - assert.NoError(t, err) + errC := make(chan error, 1) + go func() { + errC <- cmd.ExecuteContext(ctx) + }() + + pty.ExpectMatch("confirm rename:") + pty.WriteLine(workspace.Name) + + require.NoError(t, <-errC) ws, err := client.Workspace(ctx, workspace.ID) assert.NoError(t, err)