Skip to content

feat: Add support for renaming workspaces #3409

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
Aug 26, 2022
Merged
62 changes: 62 additions & 0 deletions cli/rename.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package cli

import (
"fmt"

"github.com/spf13/cobra"
"golang.org/x/xerrors"

"github.com/coder/coder/cli/cliui"
"github.com/coder/coder/codersdk"
)

func rename() *cobra.Command {
cmd := &cobra.Command{
Annotations: workspaceCommand,
Use: "rename <workspace> <new name>",
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 {
return err
}
workspace, err := namedWorkspace(cmd, client, args[0])
if err != nil {
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: 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
}

err = client.UpdateWorkspace(cmd.Context(), workspace.ID, codersdk.UpdateWorkspaceRequest{
Name: args[1],
})
if err != nil {
return xerrors.Errorf("rename workspace: %w", err)
}
return nil
},
}

cliui.AllowSkipPrompt(cmd)

return cmd
}
52 changes: 52 additions & 0 deletions cli/rename_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package cli_test

import (
"context"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"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, "--yes")
clitest.SetupConfig(t, client, root)
pty := ptytest.New(t)
cmd.SetIn(pty.Input())
cmd.SetOut(pty.Output())

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)

got := ws.Name
assert.Equal(t, want, got, "workspace name did not change")
}
1 change: 1 addition & 0 deletions cli/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ func Core() []*cobra.Command {
start(),
state(),
stop(),
rename(),
templates(),
update(),
users(),
Expand Down
1 change: 1 addition & 0 deletions coderd/coderd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
27 changes: 27 additions & 0 deletions coderd/database/databasefake/databasefake.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -2090,6 +2091,32 @@ func (q *fakeQuerier) UpdateProvisionerJobWithCompleteByID(_ context.Context, ar
return sql.ErrNoRows
}

func (q *fakeQuerier) UpdateWorkspace(_ context.Context, arg database.UpdateWorkspaceParams) (database.Workspace, 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 database.Workspace{}, &pq.Error{Code: "23505", Message: "duplicate key value violates unique constraint"}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL you can just return the correct SQL error codes 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit janky 😅.. but at least the codes will never change.

}
}

workspace.Name = arg.Name
q.workspaces[i] = workspace

return workspace, nil
}

return database.Workspace{}, sql.ErrNoRows
}

func (q *fakeQuerier) UpdateWorkspaceAutostart(_ context.Context, arg database.UpdateWorkspaceAutostartParams) error {
q.mutex.Lock()
defer q.mutex.Unlock()
Expand Down
38 changes: 38 additions & 0 deletions coderd/database/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package database

import (
"errors"

"github.com/lib/pq"
)

// UniqueConstraint represents a named unique constraint on a table.
type UniqueConstraint string

// UniqueConstraint enums.
// TODO(mafredri): Generate these from the database schema.
const (
UniqueWorkspacesOwnerIDLowerIdx UniqueConstraint = "workspaces_owner_id_lower_idx"
)

// IsUniqueViolation checks if the error is due to a 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) {
if pqErr.Code.Name() == "unique_violation" {
if len(uniqueConstraints) == 0 {
return true
}
for _, uc := range uniqueConstraints {
if pqErr.Constraint == string(uc) {
return true
}
}
}
}

return false
}
1 change: 1 addition & 0 deletions coderd/database/querier.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

34 changes: 34 additions & 0 deletions coderd/database/queries.sql.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions coderd/database/queries/workspaces.sql
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,16 @@ SET
WHERE
id = $1;

-- name: UpdateWorkspace :one
UPDATE
workspaces
SET
name = $2
WHERE
id = $1
AND deleted = false
RETURNING *;

-- name: UpdateWorkspaceAutostart :exec
UPDATE
workspaces
Expand Down
76 changes: 64 additions & 12 deletions coderd/workspaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,17 +323,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.",
Expand Down Expand Up @@ -486,6 +477,68 @@ 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,
// if we get ErrNoRows it means the workspace was deleted.
//
// 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),
})
return
}
// Check if the name was already in use.
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{{
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) {
Expand Down Expand Up @@ -563,7 +616,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.",
Expand Down Expand Up @@ -663,7 +715,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))
}
Expand Down Expand Up @@ -868,6 +919,7 @@ func convertWorkspaces(ctx context.Context, db database.Store, workspaces []data
}
return apiWorkspaces, nil
}

func convertWorkspace(
workspace database.Workspace,
workspaceBuild database.WorkspaceBuild,
Expand Down
Loading