feat: Add support for renaming workspaces (#3409)

* feat: Implement workspace renaming

* feat: Add hidden rename command (and data loss warning)

* feat: Implement database.IsUniqueViolation
This commit is contained in:
Mathias Fredriksson 2022-08-26 12:28:38 +03:00 committed by GitHub
parent 623fc5baac
commit c8f8c95f6a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 343 additions and 13 deletions

62
cli/rename.go Normal file
View File

@ -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
cli/rename_test.go Normal file
View File

@ -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")
}

View File

@ -79,6 +79,7 @@ func Core() []*cobra.Command {
start(),
state(),
stop(),
rename(),
templates(),
update(),
users(),

View File

@ -374,6 +374,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)

View File

@ -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"
@ -2086,6 +2087,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"}
}
}
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()

38
coderd/database/errors.go Normal file
View File

@ -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
}

View File

@ -139,6 +139,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) (Workspace, error)
UpdateWorkspaceAgentConnectionByID(ctx context.Context, arg UpdateWorkspaceAgentConnectionByIDParams) error
UpdateWorkspaceAgentKeysByID(ctx context.Context, arg UpdateWorkspaceAgentKeysByIDParams) error
UpdateWorkspaceAutostart(ctx context.Context, arg UpdateWorkspaceAutostartParams) error

View File

@ -4732,6 +4732,40 @@ func (q *sqlQuerier) InsertWorkspace(ctx context.Context, arg InsertWorkspacePar
return i, err
}
const updateWorkspace = `-- name: UpdateWorkspace :one
UPDATE
workspaces
SET
name = $2
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 {
ID uuid.UUID `db:"id" json:"id"`
Name string `db:"name" json:"name"`
}
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
UPDATE
workspaces

View File

@ -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

View File

@ -316,17 +316,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.",
@ -479,6 +470,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) {
@ -556,7 +609,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.",
@ -656,7 +708,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))
}
@ -861,6 +912,7 @@ func convertWorkspaces(ctx context.Context, db database.Store, workspaces []data
}
return apiWorkspaces, nil
}
func convertWorkspace(
workspace database.Workspace,
workspaceBuild database.WorkspaceBuild,

View File

@ -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 + "-test"
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) {

View File

@ -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
}

View File

@ -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