Skip to content

Commit

Permalink
feat: don't return 200 for deleted workspaces (#1556)
Browse files Browse the repository at this point in the history
  • Loading branch information
deansheather committed May 19, 2022
1 parent eb8f371 commit 4eb0bb6
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 1 deletion.
4 changes: 4 additions & 0 deletions coderd/autobuild/executor/lifecycle_executor_test.go
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"os"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -470,6 +471,9 @@ func mustTransitionWorkspace(t *testing.T, client *codersdk.Client, workspaceID
func mustWorkspace(t *testing.T, client *codersdk.Client, workspaceID uuid.UUID) codersdk.Workspace {
ctx := context.Background()
ws, err := client.Workspace(ctx, workspaceID)
if err != nil && strings.Contains(err.Error(), "status code 410") {
ws, err = client.DeletedWorkspace(ctx, workspaceID)
}
require.NoError(t, err, "no workspace found with id %s", workspaceID)
return ws
}
Expand Down
35 changes: 35 additions & 0 deletions coderd/workspaces.go
Expand Up @@ -7,6 +7,7 @@ import (
"errors"
"fmt"
"net/http"
"strconv"
"time"

"github.com/go-chi/chi/v5"
Expand Down Expand Up @@ -34,6 +35,40 @@ func (api *api) workspace(rw http.ResponseWriter, r *http.Request) {
return
}

if !api.Authorize(rw, r, rbac.ActionRead,
rbac.ResourceWorkspace.InOrg(workspace.OrganizationID).WithOwner(workspace.OwnerID.String()).WithID(workspace.ID.String())) {
return
}

// The `deleted` query parameter (which defaults to `false`) MUST match the
// `Deleted` field on the workspace otherwise you will get a 410 Gone.
var (
deletedStr = r.URL.Query().Get("deleted")
showDeleted = false
)
if deletedStr != "" {
var err error
showDeleted, err = strconv.ParseBool(deletedStr)
if err != nil {
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
Message: fmt.Sprintf("invalid bool for 'deleted' query param: %s", err),
})
return
}
}
if workspace.Deleted && !showDeleted {
httpapi.Write(rw, http.StatusGone, httpapi.Response{
Message: fmt.Sprintf("workspace %q was deleted, you can view this workspace by specifying '?deleted=true' and trying again", workspace.ID.String()),
})
return
}
if !workspace.Deleted && showDeleted {
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
Message: fmt.Sprintf("workspace %q is not deleted, please remove '?deleted=true' and try again", workspace.ID.String()),
})
return
}

build, err := api.Database.GetLatestWorkspaceBuildByWorkspaceID(r.Context(), workspace.ID)
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Expand Down
52 changes: 52 additions & 0 deletions coderd/workspaces_test.go
Expand Up @@ -20,6 +20,58 @@ import (
"github.com/coder/coder/provisionersdk/proto"
)

func TestWorkspace(t *testing.T) {
t.Parallel()

t.Run("OK", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, nil)
user := coderdtest.CreateFirstUser(t, client)
coderdtest.NewProvisionerDaemon(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)

_, err := client.Workspace(context.Background(), workspace.ID)
require.NoError(t, err)
})

t.Run("Deleted", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, nil)
user := coderdtest.CreateFirstUser(t, client)
coderdtest.NewProvisionerDaemon(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)

// Getting with deleted=true should fail.
_, err := client.DeletedWorkspace(context.Background(), workspace.ID)
require.Error(t, err)
require.ErrorContains(t, err, "400") // bad request

// Delete the workspace
build, err := client.CreateWorkspaceBuild(context.Background(), workspace.ID, codersdk.CreateWorkspaceBuildRequest{
Transition: database.WorkspaceTransitionDelete,
})
require.NoError(t, err, "delete the workspace")
coderdtest.AwaitWorkspaceBuildJob(t, client, build.ID)

// Getting with deleted=true should work.
workspaceNew, err := client.DeletedWorkspace(context.Background(), workspace.ID)
require.NoError(t, err)
require.Equal(t, workspace.ID, workspaceNew.ID)

// Getting with deleted=false should not work.
_, err = client.Workspace(context.Background(), workspace.ID)
require.Error(t, err)
require.ErrorContains(t, err, "410") // gone
})
}

func TestAdminViewAllWorkspaces(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, nil)
Expand Down
8 changes: 8 additions & 0 deletions codersdk/client.go
Expand Up @@ -36,6 +36,14 @@ type Client struct {

type requestOption func(*http.Request)

func queryParam(k, v string) requestOption {
return func(r *http.Request) {
q := r.URL.Query()
q.Set(k, v)
r.URL.RawQuery = q.Encode()
}
}

// Request performs an HTTP request with the body provided.
// The caller is responsible for closing the response body.
func (c *Client) Request(ctx context.Context, method, path string, body interface{}, opts ...requestOption) (*http.Response, error) {
Expand Down
11 changes: 10 additions & 1 deletion codersdk/workspaces.go
Expand Up @@ -42,7 +42,16 @@ type CreateWorkspaceBuildRequest struct {

// Workspace returns a single workspace.
func (c *Client) Workspace(ctx context.Context, id uuid.UUID) (Workspace, error) {
res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/workspaces/%s", id), nil)
return c.getWorkspace(ctx, id)
}

// DeletedWorkspace returns a single workspace that was deleted.
func (c *Client) DeletedWorkspace(ctx context.Context, id uuid.UUID) (Workspace, error) {
return c.getWorkspace(ctx, id, queryParam("deleted", "true"))
}

func (c *Client) getWorkspace(ctx context.Context, id uuid.UUID, opts ...requestOption) (Workspace, error) {
res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/v2/workspaces/%s", id), nil, opts...)
if err != nil {
return Workspace{}, err
}
Expand Down

0 comments on commit 4eb0bb6

Please sign in to comment.