Skip to content

Commit c8f8c95

Browse files
mafredrimafredri
authored
feat: Add support for renaming workspaces (#3409)
* feat: Implement workspace renaming * feat: Add hidden rename command (and data loss warning) * feat: Implement database.IsUniqueViolation
1 parent 623fc5b commit c8f8c95

File tree

13 files changed

+343
-13
lines changed

13 files changed

+343
-13
lines changed

‎cli/rename.go

+62
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
package cli
2+
3+
import (
4+
"fmt"
5+
6+
"github.com/spf13/cobra"
7+
"golang.org/x/xerrors"
8+
9+
"github.com/coder/coder/cli/cliui"
10+
"github.com/coder/coder/codersdk"
11+
)
12+
13+
func rename() *cobra.Command {
14+
cmd := &cobra.Command{
15+
Annotations: workspaceCommand,
16+
Use: "rename <workspace> <new name>",
17+
Short: "Rename a workspace",
18+
Args: cobra.ExactArgs(2),
19+
// Keep hidden until renaming is safe, see:
20+
// * https://github.com/coder/coder/issues/3000
21+
// * https://github.com/coder/coder/issues/3386
22+
Hidden: true,
23+
RunE: func(cmd *cobra.Command, args []string) error {
24+
client, err := CreateClient(cmd)
25+
if err != nil {
26+
return err
27+
}
28+
workspace, err := namedWorkspace(cmd, client, args[0])
29+
if err != nil {
30+
return xerrors.Errorf("get workspace: %w", err)
31+
}
32+
33+
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "%s\n\n",
34+
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)."),
35+
)
36+
_, err = cliui.Prompt(cmd, cliui.PromptOptions{
37+
Text: fmt.Sprintf("Type %q to confirm rename:", workspace.Name),
38+
Validate: func(s string) error {
39+
if s == workspace.Name {
40+
return nil
41+
}
42+
return xerrors.Errorf("Input %q does not match %q", s, workspace.Name)
43+
},
44+
})
45+
if err != nil {
46+
return err
47+
}
48+
49+
err = client.UpdateWorkspace(cmd.Context(), workspace.ID, codersdk.UpdateWorkspaceRequest{
50+
Name: args[1],
51+
})
52+
if err != nil {
53+
return xerrors.Errorf("rename workspace: %w", err)
54+
}
55+
return nil
56+
},
57+
}
58+
59+
cliui.AllowSkipPrompt(cmd)
60+
61+
return cmd
62+
}

‎cli/rename_test.go

+52
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
package cli_test
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/stretchr/testify/assert"
8+
"github.com/stretchr/testify/require"
9+
10+
"github.com/coder/coder/cli/clitest"
11+
"github.com/coder/coder/coderd/coderdtest"
12+
"github.com/coder/coder/pty/ptytest"
13+
"github.com/coder/coder/testutil"
14+
)
15+
16+
func TestRename(t *testing.T) {
17+
t.Parallel()
18+
19+
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true})
20+
user := coderdtest.CreateFirstUser(t, client)
21+
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
22+
coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
23+
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
24+
workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
25+
coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
26+
27+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
28+
defer cancel()
29+
30+
want := workspace.Name + "-test"
31+
cmd, root := clitest.New(t, "rename", workspace.Name, want, "--yes")
32+
clitest.SetupConfig(t, client, root)
33+
pty := ptytest.New(t)
34+
cmd.SetIn(pty.Input())
35+
cmd.SetOut(pty.Output())
36+
37+
errC := make(chan error, 1)
38+
go func() {
39+
errC <- cmd.ExecuteContext(ctx)
40+
}()
41+
42+
pty.ExpectMatch("confirm rename:")
43+
pty.WriteLine(workspace.Name)
44+
45+
require.NoError(t, <-errC)
46+
47+
ws, err := client.Workspace(ctx, workspace.ID)
48+
assert.NoError(t, err)
49+
50+
got := ws.Name
51+
assert.Equal(t, want, got, "workspace name did not change")
52+
}

‎cli/root.go

+1
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ func Core() []*cobra.Command {
7979
start(),
8080
state(),
8181
stop(),
82+
rename(),
8283
templates(),
8384
update(),
8485
users(),

‎coderd/coderd.go

+1
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,7 @@ func New(options *Options) *API {
374374
httpmw.ExtractWorkspaceParam(options.Database),
375375
)
376376
r.Get("/", api.workspace)
377+
r.Patch("/", api.patchWorkspace)
377378
r.Route("/builds", func(r chi.Router) {
378379
r.Get("/", api.workspaceBuilds)
379380
r.Post("/", api.postWorkspaceBuilds)

‎coderd/database/databasefake/databasefake.go

+27
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"time"
1010

1111
"github.com/google/uuid"
12+
"github.com/lib/pq"
1213
"golang.org/x/exp/slices"
1314

1415
"github.com/coder/coder/coderd/database"
@@ -2086,6 +2087,32 @@ func (q *fakeQuerier) UpdateProvisionerJobWithCompleteByID(_ context.Context, ar
20862087
return sql.ErrNoRows
20872088
}
20882089

2090+
func (q *fakeQuerier) UpdateWorkspace(_ context.Context, arg database.UpdateWorkspaceParams) (database.Workspace, error) {
2091+
q.mutex.Lock()
2092+
defer q.mutex.Unlock()
2093+
2094+
for i, workspace := range q.workspaces {
2095+
if workspace.Deleted || workspace.ID != arg.ID {
2096+
continue
2097+
}
2098+
for _, other := range q.workspaces {
2099+
if other.Deleted || other.ID == workspace.ID || workspace.OwnerID != other.OwnerID {
2100+
continue
2101+
}
2102+
if other.Name == arg.Name {
2103+
return database.Workspace{}, &pq.Error{Code: "23505", Message: "duplicate key value violates unique constraint"}
2104+
}
2105+
}
2106+
2107+
workspace.Name = arg.Name
2108+
q.workspaces[i] = workspace
2109+
2110+
return workspace, nil
2111+
}
2112+
2113+
return database.Workspace{}, sql.ErrNoRows
2114+
}
2115+
20892116
func (q *fakeQuerier) UpdateWorkspaceAutostart(_ context.Context, arg database.UpdateWorkspaceAutostartParams) error {
20902117
q.mutex.Lock()
20912118
defer q.mutex.Unlock()

‎coderd/database/errors.go

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
package database
2+
3+
import (
4+
"errors"
5+
6+
"github.com/lib/pq"
7+
)
8+
9+
// UniqueConstraint represents a named unique constraint on a table.
10+
type UniqueConstraint string
11+
12+
// UniqueConstraint enums.
13+
// TODO(mafredri): Generate these from the database schema.
14+
const (
15+
UniqueWorkspacesOwnerIDLowerIdx UniqueConstraint = "workspaces_owner_id_lower_idx"
16+
)
17+
18+
// IsUniqueViolation checks if the error is due to a unique violation.
19+
// If one or more specific unique constraints are given as arguments,
20+
// the error must be caused by one of them. If no constraints are given,
21+
// this function returns true for any unique violation.
22+
func IsUniqueViolation(err error, uniqueConstraints ...UniqueConstraint) bool {
23+
var pqErr *pq.Error
24+
if errors.As(err, &pqErr) {
25+
if pqErr.Code.Name() == "unique_violation" {
26+
if len(uniqueConstraints) == 0 {
27+
return true
28+
}
29+
for _, uc := range uniqueConstraints {
30+
if pqErr.Constraint == string(uc) {
31+
return true
32+
}
33+
}
34+
}
35+
}
36+
37+
return false
38+
}

‎coderd/database/querier.go

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go

+34
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/workspaces.sql

+10
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,16 @@ SET
112112
WHERE
113113
id = $1;
114114

115+
-- name: UpdateWorkspace :one
116+
UPDATE
117+
workspaces
118+
SET
119+
name = $2
120+
WHERE
121+
id = $1
122+
AND deleted = false
123+
RETURNING *;
124+
115125
-- name: UpdateWorkspaceAutostart :exec
116126
UPDATE
117127
workspaces

‎coderd/workspaces.go

+64-12
Original file line numberDiff line numberDiff line change
@@ -316,17 +316,8 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req
316316
})
317317
if err == nil {
318318
// If the workspace already exists, don't allow creation.
319-
template, err := api.Database.GetTemplateByID(r.Context(), workspace.TemplateID)
320-
if err != nil {
321-
httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
322-
Message: fmt.Sprintf("Find template for conflicting workspace name %q.", createWorkspace.Name),
323-
Detail: err.Error(),
324-
})
325-
return
326-
}
327-
// The template is fetched for clarity to the user on where the conflicting name may be.
328319
httpapi.Write(rw, http.StatusConflict, codersdk.Response{
329-
Message: fmt.Sprintf("Workspace %q already exists in the %q template.", createWorkspace.Name, template.Name),
320+
Message: fmt.Sprintf("Workspace %q already exists.", createWorkspace.Name),
330321
Validations: []codersdk.ValidationError{{
331322
Field: "name",
332323
Detail: "This value is already in use and should be unique.",
@@ -479,6 +470,68 @@ func (api *API) postWorkspacesByOrganization(rw http.ResponseWriter, r *http.Req
479470
findUser(apiKey.UserID, users), findUser(workspaceBuild.InitiatorID, users)))
480471
}
481472

473+
func (api *API) patchWorkspace(rw http.ResponseWriter, r *http.Request) {
474+
workspace := httpmw.WorkspaceParam(r)
475+
if !api.Authorize(r, rbac.ActionUpdate, workspace) {
476+
httpapi.ResourceNotFound(rw)
477+
return
478+
}
479+
480+
var req codersdk.UpdateWorkspaceRequest
481+
if !httpapi.Read(rw, r, &req) {
482+
return
483+
}
484+
485+
if req.Name == "" || req.Name == workspace.Name {
486+
// Nothing changed, optionally this could be an error.
487+
rw.WriteHeader(http.StatusNoContent)
488+
return
489+
}
490+
// The reason we double check here is in case more fields can be
491+
// patched in the future, it's enough if one changes.
492+
name := workspace.Name
493+
if req.Name != "" || req.Name != workspace.Name {
494+
name = req.Name
495+
}
496+
497+
_, err := api.Database.UpdateWorkspace(r.Context(), database.UpdateWorkspaceParams{
498+
ID: workspace.ID,
499+
Name: name,
500+
})
501+
if err != nil {
502+
// The query protects against updating deleted workspaces and
503+
// the existence of the workspace is checked in the request,
504+
// if we get ErrNoRows it means the workspace was deleted.
505+
//
506+
// We could do this check earlier but we'd need to start a
507+
// transaction.
508+
if errors.Is(err, sql.ErrNoRows) {
509+
httpapi.Write(rw, http.StatusMethodNotAllowed, codersdk.Response{
510+
Message: fmt.Sprintf("Workspace %q is deleted and cannot be updated.", workspace.Name),
511+
})
512+
return
513+
}
514+
// Check if the name was already in use.
515+
if database.IsUniqueViolation(err, database.UniqueWorkspacesOwnerIDLowerIdx) {
516+
httpapi.Write(rw, http.StatusConflict, codersdk.Response{
517+
Message: fmt.Sprintf("Workspace %q already exists.", req.Name),
518+
Validations: []codersdk.ValidationError{{
519+
Field: "name",
520+
Detail: "This value is already in use and should be unique.",
521+
}},
522+
})
523+
return
524+
}
525+
httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
526+
Message: "Internal error updating workspace.",
527+
Detail: err.Error(),
528+
})
529+
return
530+
}
531+
532+
rw.WriteHeader(http.StatusNoContent)
533+
}
534+
482535
func (api *API) putWorkspaceAutostart(rw http.ResponseWriter, r *http.Request) {
483536
workspace := httpmw.WorkspaceParam(r)
484537
if !api.Authorize(r, rbac.ActionUpdate, workspace) {
@@ -556,7 +609,6 @@ func (api *API) putWorkspaceTTL(rw http.ResponseWriter, r *http.Request) {
556609

557610
return nil
558611
})
559-
560612
if err != nil {
561613
resp := codersdk.Response{
562614
Message: "Error updating workspace time until shutdown.",
@@ -656,7 +708,6 @@ func (api *API) putExtendWorkspace(rw http.ResponseWriter, r *http.Request) {
656708

657709
return nil
658710
})
659-
660711
if err != nil {
661712
api.Logger.Info(r.Context(), "extending workspace", slog.Error(err))
662713
}
@@ -861,6 +912,7 @@ func convertWorkspaces(ctx context.Context, db database.Store, workspaces []data
861912
}
862913
return apiWorkspaces, nil
863914
}
915+
864916
func convertWorkspace(
865917
workspace database.Workspace,
866918
workspaceBuild database.WorkspaceBuild,

0 commit comments

Comments
 (0)