diff --git a/cli/autoupdate.go b/cli/autoupdate.go index 52ed0ffd64..1aaac86908 100644 --- a/cli/autoupdate.go +++ b/cli/autoupdate.go @@ -31,7 +31,7 @@ func (r *RootCmd) autoupdate() *serpent.Command { return xerrors.Errorf("validate policy: %w", err) } - workspace, err := namedWorkspace(inv.Context(), client, inv.Args[0]) + workspace, err := client.ResolveWorkspace(inv.Context(), inv.Args[0]) if err != nil { return xerrors.Errorf("get workspace: %w", err) } diff --git a/cli/create.go b/cli/create.go index f48bb4bfae..09a1d2c9c4 100644 --- a/cli/create.go +++ b/cli/create.go @@ -68,7 +68,7 @@ func (r *RootCmd) Create(opts CreateOptions) *serpent.Command { workspaceOwner := codersdk.Me if len(inv.Args) >= 1 { - workspaceOwner, workspaceName, err = splitNamedWorkspace(inv.Args[0]) + workspaceOwner, workspaceName, err = codersdk.SplitWorkspaceIdentifier(inv.Args[0]) if err != nil { return err } @@ -104,7 +104,7 @@ func (r *RootCmd) Create(opts CreateOptions) *serpent.Command { var sourceWorkspace codersdk.Workspace if copyParametersFrom != "" { - sourceWorkspaceOwner, sourceWorkspaceName, err := splitNamedWorkspace(copyParametersFrom) + sourceWorkspaceOwner, sourceWorkspaceName, err := codersdk.SplitWorkspaceIdentifier(copyParametersFrom) if err != nil { return err } diff --git a/cli/delete.go b/cli/delete.go index 88e56405d6..c26864719f 100644 --- a/cli/delete.go +++ b/cli/delete.go @@ -35,7 +35,7 @@ func (r *RootCmd) deleteWorkspace() *serpent.Command { return err } - workspace, err := namedWorkspace(inv.Context(), client, inv.Args[0]) + workspace, err := client.ResolveWorkspace(inv.Context(), inv.Args[0]) if err != nil { return err } diff --git a/cli/delete_test.go b/cli/delete_test.go index 2701241dcd..909166876d 100644 --- a/cli/delete_test.go +++ b/cli/delete_test.go @@ -176,7 +176,7 @@ func TestDelete(t *testing.T) { go func() { defer close(doneChan) err := inv.Run() - assert.ErrorContains(t, err, "invalid workspace name: \"a/b/c\"") + assert.ErrorContains(t, err, "invalid workspace identifier: \"a/b/c\"") }() <-doneChan }) diff --git a/cli/exp_agents.go b/cli/exp_agents.go index 7181911015..89c6b74999 100644 --- a/cli/exp_agents.go +++ b/cli/exp_agents.go @@ -131,7 +131,7 @@ func (r *RootCmd) agentsCommand() *serpent.Command { var workspaceID *uuid.UUID if workspaceFlag != "" { - workspace, err := namedWorkspace(inv.Context(), client, workspaceFlag) + workspace, err := client.ResolveWorkspace(inv.Context(), workspaceFlag) if err != nil { return xerrors.Errorf("resolve workspace %q: %w", workspaceFlag, err) } diff --git a/cli/favorite.go b/cli/favorite.go index 7fdf47270e..75738a3061 100644 --- a/cli/favorite.go +++ b/cli/favorite.go @@ -23,7 +23,7 @@ func (r *RootCmd) favorite() *serpent.Command { return err } - ws, err := namedWorkspace(inv.Context(), client, inv.Args[0]) + ws, err := client.ResolveWorkspace(inv.Context(), inv.Args[0]) if err != nil { return xerrors.Errorf("get workspace: %w", err) } @@ -53,7 +53,7 @@ func (r *RootCmd) unfavorite() *serpent.Command { return err } - ws, err := namedWorkspace(inv.Context(), client, inv.Args[0]) + ws, err := client.ResolveWorkspace(inv.Context(), inv.Args[0]) if err != nil { return xerrors.Errorf("get workspace: %w", err) } diff --git a/cli/logs.go b/cli/logs.go index 11ddd7ba6e..9f1249c332 100644 --- a/cli/logs.go +++ b/cli/logs.go @@ -52,7 +52,7 @@ func (r *RootCmd) logs() *serpent.Command { if err != nil { return err } - ws, err := namedWorkspace(inv.Context(), client, inv.Args[0]) + ws, err := client.ResolveWorkspace(inv.Context(), inv.Args[0]) if err != nil { return xerrors.Errorf("failed to get workspace: %w", err) } diff --git a/cli/rename.go b/cli/rename.go index 402124b753..4dbed8de1b 100644 --- a/cli/rename.go +++ b/cli/rename.go @@ -26,7 +26,7 @@ func (r *RootCmd) rename() *serpent.Command { } appearanceConfig := initAppearance(inv.Context(), client) - workspace, err := namedWorkspace(inv.Context(), client, inv.Args[0]) + workspace, err := client.ResolveWorkspace(inv.Context(), inv.Args[0]) if err != nil { return xerrors.Errorf("get workspace: %w", err) } diff --git a/cli/restart.go b/cli/restart.go index dff3897221..51b7d5204d 100644 --- a/cli/restart.go +++ b/cli/restart.go @@ -36,7 +36,7 @@ func (r *RootCmd) restart() *serpent.Command { ctx := inv.Context() out := inv.Stdout - workspace, err := namedWorkspace(inv.Context(), client, inv.Args[0]) + workspace, err := client.ResolveWorkspace(inv.Context(), inv.Args[0]) if err != nil { return err } diff --git a/cli/root.go b/cli/root.go index 34a3fdcb3b..8232db4a30 100644 --- a/cli/root.go +++ b/cli/root.go @@ -27,7 +27,6 @@ import ( "text/tabwriter" "time" - "github.com/google/uuid" "github.com/mattn/go-isatty" "github.com/mitchellh/go-wordwrap" "golang.org/x/mod/semver" @@ -1071,36 +1070,6 @@ func (o *OrganizationContext) Selected(inv *serpent.Invocation, client *codersdk return codersdk.Organization{}, xerrors.Errorf("Must select an organization with --org=. Choose from: %s", strings.Join(validOrgs, ", ")) } -func splitNamedWorkspace(identifier string) (owner string, workspaceName string, err error) { - parts := strings.Split(identifier, "/") - - switch len(parts) { - case 1: - owner = codersdk.Me - workspaceName = parts[0] - case 2: - owner = parts[0] - workspaceName = parts[1] - default: - return "", "", xerrors.Errorf("invalid workspace name: %q", identifier) - } - return owner, workspaceName, nil -} - -// namedWorkspace fetches and returns a workspace by an identifier, which may be either -// a bare name (for a workspace owned by the current user) or a "user/workspace" combination, -// where user is either a username or UUID. -func namedWorkspace(ctx context.Context, client *codersdk.Client, identifier string) (codersdk.Workspace, error) { - if uid, err := uuid.Parse(identifier); err == nil { - return client.Workspace(ctx, uid) - } - owner, name, err := splitNamedWorkspace(identifier) - if err != nil { - return codersdk.Workspace{}, err - } - return client.WorkspaceByOwnerAndName(ctx, owner, name, codersdk.WorkspaceOptions{}) -} - func initAppearance(ctx context.Context, client *codersdk.Client) codersdk.AppearanceConfig { // best effort cfg, _ := client.Appearance(ctx) diff --git a/cli/schedule.go b/cli/schedule.go index cf292b7f48..5c31c711a6 100644 --- a/cli/schedule.go +++ b/cli/schedule.go @@ -109,7 +109,7 @@ func (r *RootCmd) scheduleShow() *serpent.Command { if len(inv.Args) == 1 { // If the argument contains a slash, we assume it's a full owner/name reference if strings.Contains(inv.Args[0], "/") { - _, workspaceName, err := splitNamedWorkspace(inv.Args[0]) + _, workspaceName, err := codersdk.SplitWorkspaceIdentifier(inv.Args[0]) if err != nil { return err } @@ -161,7 +161,7 @@ func (r *RootCmd) scheduleStart() *serpent.Command { if err != nil { return err } - workspace, err := namedWorkspace(inv.Context(), client, inv.Args[0]) + workspace, err := client.ResolveWorkspace(inv.Context(), inv.Args[0]) if err != nil { return err } @@ -206,7 +206,7 @@ func (r *RootCmd) scheduleStart() *serpent.Command { return err } - updated, err := namedWorkspace(inv.Context(), client, inv.Args[0]) + updated, err := client.ResolveWorkspace(inv.Context(), inv.Args[0]) if err != nil { return err } @@ -234,7 +234,7 @@ func (r *RootCmd) scheduleStop() *serpent.Command { if err != nil { return err } - workspace, err := namedWorkspace(inv.Context(), client, inv.Args[0]) + workspace, err := client.ResolveWorkspace(inv.Context(), inv.Args[0]) if err != nil { return err } @@ -261,7 +261,7 @@ func (r *RootCmd) scheduleStop() *serpent.Command { return err } - updated, err := namedWorkspace(inv.Context(), client, inv.Args[0]) + updated, err := client.ResolveWorkspace(inv.Context(), inv.Args[0]) if err != nil { return err } @@ -293,7 +293,7 @@ func (r *RootCmd) scheduleExtend() *serpent.Command { return err } - workspace, err := namedWorkspace(inv.Context(), client, inv.Args[0]) + workspace, err := client.ResolveWorkspace(inv.Context(), inv.Args[0]) if err != nil { return xerrors.Errorf("get workspace: %w", err) } @@ -325,7 +325,7 @@ func (r *RootCmd) scheduleExtend() *serpent.Command { return err } - updated, err := namedWorkspace(inv.Context(), client, inv.Args[0]) + updated, err := client.ResolveWorkspace(inv.Context(), inv.Args[0]) if err != nil { return err } diff --git a/cli/sharing.go b/cli/sharing.go index c1d9519850..61428d3b37 100644 --- a/cli/sharing.go +++ b/cli/sharing.go @@ -48,7 +48,7 @@ func (r *RootCmd) statusWorkspaceSharing() *serpent.Command { return err } - workspace, err := namedWorkspace(inv.Context(), client, inv.Args[0]) + workspace, err := client.ResolveWorkspace(inv.Context(), inv.Args[0]) if err != nil { return xerrors.Errorf("unable to fetch Workspace %s: %w", inv.Args[0], err) } @@ -110,7 +110,7 @@ func (r *RootCmd) shareWorkspace() *serpent.Command { return xerrors.New("at least one user or group must be provided") } - workspace, err := namedWorkspace(inv.Context(), client, inv.Args[0]) + workspace, err := client.ResolveWorkspace(inv.Context(), inv.Args[0]) if err != nil { return xerrors.Errorf("could not fetch the workspace %s: %w", inv.Args[0], err) } @@ -208,7 +208,7 @@ func (r *RootCmd) unshareWorkspace() *serpent.Command { return err } - workspace, err := namedWorkspace(inv.Context(), client, inv.Args[0]) + workspace, err := client.ResolveWorkspace(inv.Context(), inv.Args[0]) if err != nil { return xerrors.Errorf("could not fetch the workspace %s: %w", inv.Args[0], err) } diff --git a/cli/show.go b/cli/show.go index 0ef3d4e90f..2123993398 100644 --- a/cli/show.go +++ b/cli/show.go @@ -41,7 +41,7 @@ func (r *RootCmd) show() *serpent.Command { if err != nil { return xerrors.Errorf("get server version: %w", err) } - workspace, err := namedWorkspace(inv.Context(), client, inv.Args[0]) + workspace, err := client.ResolveWorkspace(inv.Context(), inv.Args[0]) if err != nil { return xerrors.Errorf("get workspace: %w", err) } diff --git a/cli/show_test.go b/cli/show_test.go index 46213194f9..f078273403 100644 --- a/cli/show_test.go +++ b/cli/show_test.go @@ -65,6 +65,59 @@ func TestShow(t *testing.T) { } _ = testutil.TryReceive(ctx, t, doneChan) }) + + // Regression test: workspace names that are valid dashless UUIDs + // (32 hex chars) should be looked up by name, not parsed as a + // UUID and fetched by ID (which 404s). + t.Run("WorkspaceWithUUIDLikeName", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true}) + owner := coderdtest.CreateFirstUser(t, client) + member, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID) + version := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, completeWithAgent()) + coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID) + template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version.ID) + + // This name is a valid 32-char hex string (dashless UUID). + const wsName = "a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6" + workspace := coderdtest.CreateWorkspace(t, member, template.ID, func(cwr *codersdk.CreateWorkspaceRequest) { + cwr.Name = wsName + }) + build := coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID) + + args := []string{ + "show", + wsName, + } + inv, root := clitest.New(t, args...) + clitest.SetupConfig(t, member, root) + doneChan := make(chan struct{}) + pty := ptytest.New(t).Attach(inv) + ctx := testutil.Context(t, testutil.WaitShort) + go func() { + defer close(doneChan) + err := inv.WithContext(ctx).Run() + assert.NoError(t, err) + }() + matches := []struct { + match string + write string + }{ + {match: fmt.Sprintf("%s/%s", workspace.OwnerName, workspace.Name)}, + {match: fmt.Sprintf("(%s since ", build.Status)}, + {match: fmt.Sprintf("%s:%s", workspace.TemplateName, workspace.LatestBuild.TemplateVersionName)}, + {match: "compute.main"}, + {match: "smith (linux, i386)"}, + {match: "coder ssh " + workspace.Name}, + } + for _, m := range matches { + pty.ExpectMatchContext(ctx, m.match) + if len(m.write) > 0 { + pty.WriteLine(m.write) + } + } + _ = testutil.TryReceive(ctx, t, doneChan) + }) } func TestShowDevcontainers_Golden(t *testing.T) { diff --git a/cli/ssh.go b/cli/ssh.go index 64f4a48ba7..d638aefb36 100644 --- a/cli/ssh.go +++ b/cli/ssh.go @@ -984,7 +984,7 @@ func GetWorkspaceAndAgent(ctx context.Context, inv *serpent.Invocation, client * err error ) - workspace, err = namedWorkspace(ctx, client, workspaceParts[0]) + workspace, err = client.ResolveWorkspace(ctx, workspaceParts[0]) if err != nil { return codersdk.Workspace{}, codersdk.WorkspaceAgent{}, nil, err } @@ -1044,7 +1044,7 @@ func GetWorkspaceAndAgent(ctx context.Context, inv *serpent.Invocation, client * } // Refresh workspace state so that `outdated`, `build`,`template_*` fields are up-to-date. - workspace, err = namedWorkspace(ctx, client, workspaceParts[0]) + workspace, err = client.ResolveWorkspace(ctx, workspaceParts[0]) if err != nil { return codersdk.Workspace{}, codersdk.WorkspaceAgent{}, nil, err } diff --git a/cli/start.go b/cli/start.go index 4d12eeb2dd..b63f357a5f 100644 --- a/cli/start.go +++ b/cli/start.go @@ -43,7 +43,7 @@ func (r *RootCmd) start() *serpent.Command { return err } - workspace, err := namedWorkspace(inv.Context(), client, inv.Args[0]) + workspace, err := client.ResolveWorkspace(inv.Context(), inv.Args[0]) if err != nil { return err } @@ -97,7 +97,7 @@ func (r *RootCmd) start() *serpent.Command { } // Re-fetch workspace after stop completes so // startWorkspace sees the latest state. - workspace, err = namedWorkspace(inv.Context(), client, inv.Args[0]) + workspace, err = client.ResolveWorkspace(inv.Context(), inv.Args[0]) if err != nil { return err } diff --git a/cli/state.go b/cli/state.go index 4dac6a3d17..623295da9b 100644 --- a/cli/state.go +++ b/cli/state.go @@ -41,13 +41,13 @@ func (r *RootCmd) statePull() *serpent.Command { } var build codersdk.WorkspaceBuild if buildNumber == 0 { - workspace, err := namedWorkspace(inv.Context(), client, inv.Args[0]) + workspace, err := client.ResolveWorkspace(inv.Context(), inv.Args[0]) if err != nil { return err } build = workspace.LatestBuild } else { - owner, workspace, err := splitNamedWorkspace(inv.Args[0]) + owner, workspace, err := codersdk.SplitWorkspaceIdentifier(inv.Args[0]) if err != nil { return err } @@ -99,7 +99,7 @@ func (r *RootCmd) statePush() *serpent.Command { if err != nil { return err } - workspace, err := namedWorkspace(inv.Context(), client, inv.Args[0]) + workspace, err := client.ResolveWorkspace(inv.Context(), inv.Args[0]) if err != nil { return err } @@ -107,7 +107,7 @@ func (r *RootCmd) statePush() *serpent.Command { if buildNumber == 0 { build = workspace.LatestBuild } else { - owner, workspace, err := splitNamedWorkspace(inv.Args[0]) + owner, workspace, err := codersdk.SplitWorkspaceIdentifier(inv.Args[0]) if err != nil { return err } diff --git a/cli/stop.go b/cli/stop.go index fb35e4a5e0..6a93371ecc 100644 --- a/cli/stop.go +++ b/cli/stop.go @@ -36,7 +36,7 @@ func (r *RootCmd) stop() *serpent.Command { return err } - workspace, err := namedWorkspace(inv.Context(), client, inv.Args[0]) + workspace, err := client.ResolveWorkspace(inv.Context(), inv.Args[0]) if err != nil { return err } diff --git a/cli/support.go b/cli/support.go index 8501e0e8e9..3269b524ee 100644 --- a/cli/support.go +++ b/cli/support.go @@ -185,7 +185,7 @@ func (r *RootCmd) supportBundle() *serpent.Command { cliLog.Warn(inv.Context(), "no workspace specified") cliui.Warn(inv.Stderr, "No workspace specified. This will result in incomplete information.") } else { - ws, err := namedWorkspace(inv.Context(), client, inv.Args[0]) + ws, err := client.ResolveWorkspace(inv.Context(), inv.Args[0]) if err != nil { return xerrors.Errorf("invalid workspace: %w", err) } diff --git a/cli/update.go b/cli/update.go index 5eda1b5598..816a6fc9f8 100644 --- a/cli/update.go +++ b/cli/update.go @@ -29,7 +29,7 @@ func (r *RootCmd) update() *serpent.Command { return err } - workspace, err := namedWorkspace(inv.Context(), client, inv.Args[0]) + workspace, err := client.ResolveWorkspace(inv.Context(), inv.Args[0]) if err != nil { return err } diff --git a/codersdk/toolsdk/bash.go b/codersdk/toolsdk/bash.go index 03f0905b61..36bf7dbf6b 100644 --- a/codersdk/toolsdk/bash.go +++ b/codersdk/toolsdk/bash.go @@ -190,7 +190,7 @@ func findWorkspaceAndAgent(ctx context.Context, client *codersdk.Client, workspa } // Get workspace - workspace, err := namedWorkspace(ctx, client, workspaceName) + workspace, err := client.ResolveWorkspace(ctx, workspaceName) if err != nil { return codersdk.Workspace{}, codersdk.WorkspaceAgent{}, err } @@ -274,37 +274,6 @@ func getWorkspaceAgent(workspace codersdk.Workspace, agentName string) (codersdk return codersdk.WorkspaceAgent{}, xerrors.Errorf("multiple agents found, please specify the agent name, available agents: %v", availableNames) } -func splitNameAndOwner(identifier string) (name string, owner string) { - // Parse owner and name (workspace, task). - parts := strings.SplitN(identifier, "/", 2) - - if len(parts) == 2 { - owner = parts[0] - name = parts[1] - } else { - owner = "me" - name = identifier - } - - return name, owner -} - -// namedWorkspace gets a workspace by owner/name or just name -func namedWorkspace(ctx context.Context, client *codersdk.Client, identifier string) (codersdk.Workspace, error) { - workspaceName, owner := splitNameAndOwner(identifier) - - // Handle -- separator format (convert to / format) - if strings.Contains(identifier, "--") && !strings.Contains(identifier, "/") { - dashParts := strings.SplitN(identifier, "--", 2) - if len(dashParts) == 2 { - owner = dashParts[0] - workspaceName = dashParts[1] - } - } - - return client.WorkspaceByOwnerAndName(ctx, owner, workspaceName, codersdk.WorkspaceOptions{}) -} - // executeCommandWithTimeout executes a command with timeout support func executeCommandWithTimeout(ctx context.Context, session *gossh.Session, command string) ([]byte, error) { // Set up pipes to capture output diff --git a/codersdk/toolsdk/toolsdk.go b/codersdk/toolsdk/toolsdk.go index b6d1141a14..a2dbda6ba4 100644 --- a/codersdk/toolsdk/toolsdk.go +++ b/codersdk/toolsdk/toolsdk.go @@ -432,11 +432,7 @@ This returns more data than list_workspaces to reduce token usage.`, }, MCPAnnotations: mcpReadOnlyAnnotations, Handler: func(ctx context.Context, deps Deps, args GetWorkspaceArgs) (codersdk.Workspace, error) { - wsID, err := uuid.Parse(args.WorkspaceID) - if err != nil { - return namedWorkspace(ctx, deps.coderClient, NormalizeWorkspaceInput(args.WorkspaceID)) - } - return deps.coderClient.Workspace(ctx, wsID) + return deps.coderClient.ResolveWorkspace(ctx, NormalizeWorkspaceInput(args.WorkspaceID)) }, } diff --git a/codersdk/toolsdk/toolsdk_test.go b/codersdk/toolsdk/toolsdk_test.go index 2e5e005883..0ce93e210c 100644 --- a/codersdk/toolsdk/toolsdk_test.go +++ b/codersdk/toolsdk/toolsdk_test.go @@ -45,6 +45,14 @@ import ( // nolint:gocritic // This is in a test package and does not end up in the build func setupWorkspaceForAgent(t *testing.T, opts *coderdtest.Options) (*codersdk.Client, database.WorkspaceTable, string) { t.Helper() + return setupWorkspaceForAgentWithName(t, opts, "myworkspace") +} + +// setupWorkspaceForAgentWithName creates a workspace setup exactly like main +// SSH tests, but with a caller-provided workspace name. +// nolint:gocritic // This is in a test package and does not end up in the build +func setupWorkspaceForAgentWithName(t *testing.T, opts *coderdtest.Options, workspaceName string) (*codersdk.Client, database.WorkspaceTable, string) { + t.Helper() client, store := coderdtest.NewWithDatabase(t, opts) client.SetLogger(testutil.Logger(t).Named("client")) @@ -54,7 +62,7 @@ func setupWorkspaceForAgent(t *testing.T, opts *coderdtest.Options) (*codersdk.C }) // nolint:gocritic // This is in a test package and does not end up in the build r := dbfake.WorkspaceBuild(t, store, database.WorkspaceTable{ - Name: "myworkspace", + Name: workspaceName, OrganizationID: first.OrganizationID, OwnerID: user.ID, }).WithAgent().Do() @@ -241,6 +249,31 @@ func TestTools(t *testing.T) { } }) + t.Run("GetWorkspace_ByUUIDLikeName", func(t *testing.T) { + t.Parallel() + + // Regression test: a workspace whose name is a valid dashless + // UUID should resolve correctly. Previously, the handler would + // parse the name as a UUID, get a 404 from the ID-based lookup, + // and never fall back to name-based lookup. + const uuidLikeName = "a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6" + // nolint:gocritic // This is in a test package and does not end up in the build + uuidWorkspace := dbfake.WorkspaceBuild(t, store, database.WorkspaceTable{ + OrganizationID: owner.OrganizationID, + OwnerID: member.ID, + Name: uuidLikeName, + }).Do() + + tb, err := toolsdk.NewDeps(memberClient) + require.NoError(t, err) + + result, err := testTool(t, toolsdk.GetWorkspace, tb, toolsdk.GetWorkspaceArgs{ + WorkspaceID: uuidLikeName, + }) + require.NoError(t, err) + require.Equal(t, uuidWorkspace.Workspace.ID, result.ID) + }) + t.Run("ListTemplates", func(t *testing.T) { tb, err := toolsdk.NewDeps(memberClient) require.NoError(t, err) @@ -566,6 +599,24 @@ func TestTools(t *testing.T) { require.NoError(t, err) require.Equal(t, 0, result.ExitCode) require.Equal(t, "owner format works", result.Output) + + // Regression test: agent-backed tools should also work when the + // workspace name is a valid dashless UUID. + const uuidLikeName = "a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6" + uuidClient, uuidWorkspace, uuidAgentToken := setupWorkspaceForAgentWithName(t, nil, uuidLikeName) + _ = agenttest.New(t, uuidClient.URL, uuidAgentToken) + coderdtest.NewWorkspaceAgentWaiter(t, uuidClient, uuidWorkspace.ID).Wait() + + uuidTB, err := toolsdk.NewDeps(uuidClient) + require.NoError(t, err) + + result, err = testTool(t, toolsdk.WorkspaceBash, uuidTB, toolsdk.WorkspaceBashArgs{ + Workspace: uuidWorkspace.Name, + Command: "echo 'uuid-like name works'", + }) + require.NoError(t, err) + require.Equal(t, 0, result.ExitCode) + require.Equal(t, "uuid-like name works", result.Output) }) t.Run("WorkspaceLS", func(t *testing.T) { diff --git a/codersdk/workspaces.go b/codersdk/workspaces.go index 75e5e1ace8..b520f27e4f 100644 --- a/codersdk/workspaces.go +++ b/codersdk/workspaces.go @@ -3,6 +3,7 @@ package codersdk import ( "context" "encoding/json" + "errors" "fmt" "net/http" "net/http/cookiejar" @@ -612,6 +613,53 @@ func (c *Client) WorkspaceByOwnerAndName(ctx context.Context, owner string, name return workspace, json.NewDecoder(res.Body).Decode(&workspace) } +// SplitWorkspaceIdentifier splits an identifier into owner and +// workspace name. A bare name defaults the owner to Me ("me"). An +// "owner/name" pair is accepted, and identifiers with more than one +// "/" are rejected. +func SplitWorkspaceIdentifier(identifier string) (owner, name string, err error) { + owner, name, ok := strings.Cut(identifier, "/") + if !ok { + return Me, identifier, nil + } + if strings.Contains(name, "/") { + return "", "", xerrors.Errorf("invalid workspace identifier: %q", identifier) + } + return owner, name, nil +} + +// ResolveWorkspace fetches a workspace by identifier, which may be a +// UUID, a bare name (owned by the current user), or an "owner/name" +// pair. When the identifier parses as a valid UUID but no workspace +// exists with that ID, the function falls back to a name-based +// lookup because workspace names can be valid UUID strings. +func (c *Client) ResolveWorkspace(ctx context.Context, identifier string) (Workspace, error) { + if uid, err := uuid.Parse(identifier); err == nil { + ws, err := c.Workspace(ctx, uid) + if err == nil { + return ws, nil + } + // A workspace name might be a valid UUID string. If the + // ID-based lookup returned 404, fall through to name-based + // lookup below. + var sdkErr *Error + if !errors.As(err, &sdkErr) || sdkErr.StatusCode() != http.StatusNotFound { + return Workspace{}, err + } + // A standard dashed UUID (36 chars) cannot be a valid + // workspace name (max 32 chars). Skip the wasted + // name-based round-trip. + if err := NameValid(identifier); err != nil { + return Workspace{}, sdkErr + } + } + owner, name, err := SplitWorkspaceIdentifier(identifier) + if err != nil { + return Workspace{}, err + } + return c.WorkspaceByOwnerAndName(ctx, owner, name, WorkspaceOptions{}) +} + type WorkspaceQuota struct { CreditsConsumed int `json:"credits_consumed"` Budget int `json:"budget"` diff --git a/codersdk/workspaces_test.go b/codersdk/workspaces_test.go new file mode 100644 index 0000000000..ee03c88643 --- /dev/null +++ b/codersdk/workspaces_test.go @@ -0,0 +1,310 @@ +package codersdk_test + +import ( + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "net/url" + "sync/atomic" + "testing" + + "github.com/go-chi/chi/v5" + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/codersdk" +) + +func TestResolveWorkspace(t *testing.T) { + t.Parallel() + + // writeJSON is a small helper that writes a JSON-encoded value + // with the given status code. + writeJSON := func(w http.ResponseWriter, status int, v any) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _ = json.NewEncoder(w).Encode(v) + } + + // errResponse builds a codersdk.Response suitable for error + // replies. + errResponse := func(msg string) codersdk.Response { + return codersdk.Response{Message: msg} + } + + // newWorkspace returns a Workspace with the given ID and name. + newWorkspace := func(id uuid.UUID, name string) codersdk.Workspace { + return codersdk.Workspace{ID: id, Name: name} + } + + // Each table case configures a mock server with separate UUID + // and name endpoint behaviors, then calls ResolveWorkspace with + // the given identifier. + type endpointResponse struct { + status int + workspace codersdk.Workspace + errMsg string + } + tests := []struct { + name string + identifier string + // uuidEndpoint configures GET /api/v2/workspaces/{workspace}. + // nil means the endpoint is not registered (404 from chi). + uuidEndpoint *endpointResponse + // nameEndpoint configures GET /api/v2/users/{user}/workspace/{workspace}. + // nil means the endpoint is not registered. + nameEndpoint *endpointResponse + // expectedOwner and expectedName are checked via assert inside + // the name endpoint handler (when non-empty). + expectedOwner string + expectedName string + // Expected outcomes. + wantErr bool + wantStatusCode int + wantUUIDHits int64 + wantNameHits int64 + }{ + { + name: "ByUUID", + identifier: "", // filled dynamically below + uuidEndpoint: &endpointResponse{ + status: http.StatusOK, + }, + wantUUIDHits: 1, + wantNameHits: 0, + }, + { + name: "ByName", + identifier: "my-workspace", + nameEndpoint: &endpointResponse{ + status: http.StatusOK, + }, + expectedOwner: "me", + expectedName: "my-workspace", + wantUUIDHits: 0, + wantNameHits: 1, + }, + { + name: "ByOwnerAndName", + identifier: "alice/my-workspace", + nameEndpoint: &endpointResponse{ + status: http.StatusOK, + }, + expectedOwner: "alice", + expectedName: "my-workspace", + wantUUIDHits: 0, + wantNameHits: 1, + }, + { + name: "OwnerWithUUIDLikeName", + identifier: "alice/a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6", + nameEndpoint: &endpointResponse{ + status: http.StatusOK, + }, + expectedOwner: "alice", + expectedName: "a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6", + wantUUIDHits: 0, + wantNameHits: 1, + }, + { + name: "UUIDLikeNameFallback", + identifier: "a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6", + uuidEndpoint: &endpointResponse{ + status: http.StatusNotFound, + errMsg: "Resource not found.", + }, + nameEndpoint: &endpointResponse{ + status: http.StatusOK, + }, + expectedOwner: "me", + expectedName: "a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6", + wantUUIDHits: 1, + wantNameHits: 1, + }, + { + name: "DashedUUIDNotFound", + identifier: "", // filled dynamically (standard dashed UUID) + uuidEndpoint: &endpointResponse{ + status: http.StatusNotFound, + errMsg: "Resource not found.", + }, + nameEndpoint: &endpointResponse{ + status: http.StatusNotFound, + errMsg: "Resource not found.", + }, + wantErr: true, + wantStatusCode: http.StatusNotFound, + // NameValid rejects dashed UUIDs (36 chars), so the + // name endpoint should not be called. + wantUUIDHits: 1, + wantNameHits: 0, + }, + { + name: "NonNotFoundError", + identifier: "", // filled dynamically + uuidEndpoint: &endpointResponse{ + status: http.StatusInternalServerError, + errMsg: "Internal server error.", + }, + nameEndpoint: &endpointResponse{ + status: http.StatusOK, + }, + wantErr: true, + wantStatusCode: http.StatusInternalServerError, + wantUUIDHits: 1, + wantNameHits: 0, + }, + { + name: "NameNotFound", + identifier: "nonexistent", + nameEndpoint: &endpointResponse{ + status: http.StatusNotFound, + errMsg: "Resource not found.", + }, + expectedOwner: "me", + expectedName: "nonexistent", + wantErr: true, + wantStatusCode: http.StatusNotFound, + wantUUIDHits: 0, + wantNameHits: 1, + }, + { + name: "Forbidden", + identifier: "", // filled dynamically + uuidEndpoint: &endpointResponse{ + status: http.StatusForbidden, + errMsg: "Forbidden.", + }, + nameEndpoint: &endpointResponse{ + status: http.StatusOK, + }, + wantErr: true, + wantStatusCode: http.StatusForbidden, + wantUUIDHits: 1, + wantNameHits: 0, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + wsID := uuid.New() + expected := newWorkspace(wsID, "test-workspace") + + // When identifier is empty, use the workspace UUID + // (standard dashed format). + identifier := tt.identifier + if identifier == "" { + identifier = wsID.String() + } + + var uuidHits, nameHits atomic.Int64 + r := chi.NewRouter() + + if tt.uuidEndpoint != nil { + ep := tt.uuidEndpoint + // Use the expected workspace in OK responses + // unless the test overrides it. + if ep.status == http.StatusOK && ep.workspace.ID == uuid.Nil { + ep.workspace = expected + } + r.Get("/api/v2/workspaces/{workspace}", func(w http.ResponseWriter, req *http.Request) { + uuidHits.Add(1) + if ep.errMsg != "" { + writeJSON(w, ep.status, errResponse(ep.errMsg)) + return + } + writeJSON(w, ep.status, ep.workspace) + }) + } + + if tt.nameEndpoint != nil { + ep := tt.nameEndpoint + if ep.status == http.StatusOK && ep.workspace.ID == uuid.Nil { + ep.workspace = expected + } + r.Get("/api/v2/users/{user}/workspace/{workspace}", func(w http.ResponseWriter, req *http.Request) { + nameHits.Add(1) + if tt.expectedOwner != "" { + assert.Equal(t, tt.expectedOwner, chi.URLParam(req, "user")) + } + if tt.expectedName != "" { + assert.Equal(t, tt.expectedName, chi.URLParam(req, "workspace")) + } + if ep.errMsg != "" { + writeJSON(w, ep.status, errResponse(ep.errMsg)) + return + } + writeJSON(w, ep.status, ep.workspace) + }) + } + + srv := httptest.NewServer(r) + defer srv.Close() + + u, err := url.Parse(srv.URL) + require.NoError(t, err) + client := codersdk.New(u) + + ws, err := client.ResolveWorkspace(t.Context(), identifier) + if tt.wantErr { + require.Error(t, err) + if tt.wantStatusCode != 0 { + var sdkErr *codersdk.Error + require.ErrorAs(t, err, &sdkErr) + require.Equal(t, tt.wantStatusCode, sdkErr.StatusCode()) + } + } else { + require.NoError(t, err) + require.Equal(t, expected.ID, ws.ID) + } + + require.EqualValues(t, tt.wantUUIDHits, uuidHits.Load()) + require.EqualValues(t, tt.wantNameHits, nameHits.Load()) + }) + } + + // Cases that need a structurally different server setup. + + t.Run("TransportError", func(t *testing.T) { + t.Parallel() + + // Close the server immediately so the transport layer fails. + srv := httptest.NewServer(http.NotFoundHandler()) + srvURL, err := url.Parse(srv.URL) + require.NoError(t, err) + srv.Close() + + client := codersdk.New(srvURL) + + _, err = client.ResolveWorkspace(t.Context(), uuid.NewString()) + require.Error(t, err) + + // Transport errors must not be swallowed by the 404 + // fallback path. The error should NOT be a *codersdk.Error. + var sdkErr *codersdk.Error + require.False(t, errors.As(err, &sdkErr), "transport error should not be a codersdk.Error") + }) + + t.Run("InvalidIdentifier", func(t *testing.T) { + t.Parallel() + + var hits atomic.Int64 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + hits.Add(1) + t.Errorf("unexpected HTTP request for invalid identifier: %s", req.URL.Path) + })) + defer srv.Close() + + u, err := url.Parse(srv.URL) + require.NoError(t, err) + client := codersdk.New(u) + + _, err = client.ResolveWorkspace(t.Context(), "a/b/c") + require.Error(t, err) + require.ErrorContains(t, err, "invalid workspace identifier: \"a/b/c\"") + require.EqualValues(t, 0, hits.Load(), "invalid identifiers should fail before any HTTP request") + }) +}