Skip to content

Commit 7b9fc5b

Browse files
mafredrimafredri
committed
in-workspace detction, explicit api token generation (for printed URIs)
1 parent 42c6b03 commit 7b9fc5b

File tree

6 files changed

+181
-64
lines changed

6 files changed

+181
-64
lines changed

‎agent/agent_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -924,7 +924,7 @@ func TestAgent_EnvironmentVariableExpansion(t *testing.T) {
924924
func TestAgent_CoderEnvVars(t *testing.T) {
925925
t.Parallel()
926926

927-
for _, key := range []string{"CODER"} {
927+
for _, key := range []string{"CODER", "CODER_WORKSPACE_NAME", "CODER_WORKSPACE_AGENT_NAME"} {
928928
key := key
929929
t.Run(key, func(t *testing.T) {
930930
t.Parallel()

‎agent/agentssh/agentssh.go

+2
Original file line numberDiff line numberDiff line change
@@ -659,6 +659,8 @@ func (s *Server) CreateCommand(ctx context.Context, script string, env []string)
659659
// Set environment variables reliable detection of being inside a
660660
// Coder workspace.
661661
cmd.Env = append(cmd.Env, "CODER=true")
662+
cmd.Env = append(cmd.Env, "CODER_WORKSPACE_NAME="+manifest.WorkspaceName)
663+
cmd.Env = append(cmd.Env, "CODER_WORKSPACE_AGENT_NAME="+manifest.AgentName)
662664
cmd.Env = append(cmd.Env, fmt.Sprintf("USER=%s", username))
663665
// Git on Windows resolves with UNIX-style paths.
664666
// If using backslashes, it's unable to find the executable.

‎cli/open.go

+113-49
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"fmt"
66
"net/url"
7+
"path/filepath"
78
"strings"
89

910
"github.com/skratchdot/open-golang/open"
@@ -28,111 +29,174 @@ func (r *RootCmd) open() *clibase.Cmd {
2829
return cmd
2930
}
3031

32+
const vscodeDesktopName = "VS Code Desktop"
33+
3134
func (r *RootCmd) openVSCode() *clibase.Cmd {
32-
var testNoOpen bool
35+
var (
36+
generateToken bool
37+
testNoOpen bool
38+
)
3339

3440
client := new(codersdk.Client)
3541
cmd := &clibase.Cmd{
3642
Annotations: workspaceCommand,
3743
Use: "vscode <workspace> [<directory in workspace>]",
38-
Short: "Open a workspace in Visual Studio Code",
44+
Short: "Open a workspace in Visual Studio Code.",
3945
Middleware: clibase.Chain(
40-
clibase.RequireRangeArgs(1, -1),
46+
clibase.RequireRangeArgs(1, 2),
4147
r.InitClient(client),
4248
),
4349
Handler: func(inv *clibase.Invocation) error {
4450
ctx, cancel := context.WithCancel(inv.Context())
4551
defer cancel()
4652

47-
// Prepare an API key. This is for automagical configuration of
48-
// VS Code, however, we could try to probe VS Code settings to see
49-
// if the current configuration is valid. Future improvement idea.
50-
apiKey, err := client.CreateAPIKey(ctx, codersdk.Me)
51-
if err != nil {
52-
return xerrors.Errorf("create API key: %w", err)
53-
}
53+
// Check if we're inside a workspace, and especially inside _this_
54+
// workspace so we can perform path resolution/expansion. Generally,
55+
// we know that if we're inside a workspace, `open` can't be used.
56+
insideAWorkspace := inv.Environ.Get("CODER") == "true"
57+
inWorkspaceName := inv.Environ.Get("CODER_WORKSPACE_NAME") + "." + inv.Environ.Get("CODER_WORKSPACE_AGENT_NAME")
5458

5559
// We need a started workspace to figure out e.g. expanded directory.
5660
// Pehraps the vscode-coder extension could handle this by accepting
5761
// default_directory=true, then probing the agent. Then we wouldn't
5862
// need to wait for the agent to start.
59-
workspaceName := inv.Args[0]
63+
workspaceQuery := inv.Args[0]
6064
autostart := true
61-
workspace, workspaceAgent, err := getWorkspaceAndAgent(ctx, inv, client, autostart, codersdk.Me, workspaceName)
65+
workspace, workspaceAgent, err := getWorkspaceAndAgent(ctx, inv, client, autostart, codersdk.Me, workspaceQuery)
6266
if err != nil {
6367
return xerrors.Errorf("get workspace and agent: %w", err)
6468
}
6569

66-
// We could optionally add a flag to skip wait, like with SSH.
67-
wait := false
68-
for _, script := range workspaceAgent.Scripts {
69-
if script.StartBlocksLogin {
70-
wait = true
71-
break
70+
workspaceName := workspace.Name + "." + workspaceAgent.Name
71+
insideThisWorkspace := insideAWorkspace && inWorkspaceName == workspaceName
72+
73+
if !insideThisWorkspace {
74+
// We could optionally add a flag to skip wait, like with SSH.
75+
wait := false
76+
for _, script := range workspaceAgent.Scripts {
77+
if script.StartBlocksLogin {
78+
wait = true
79+
break
80+
}
7281
}
73-
}
74-
err = cliui.Agent(ctx, inv.Stderr, workspaceAgent.ID, cliui.AgentOptions{
75-
Fetch: client.WorkspaceAgent,
76-
FetchLogs: client.WorkspaceAgentLogsAfter,
77-
Wait: wait,
78-
})
79-
if err != nil {
80-
if xerrors.Is(err, context.Canceled) {
81-
return cliui.Canceled
82+
err = cliui.Agent(ctx, inv.Stderr, workspaceAgent.ID, cliui.AgentOptions{
83+
Fetch: client.WorkspaceAgent,
84+
FetchLogs: client.WorkspaceAgentLogsAfter,
85+
Wait: wait,
86+
})
87+
if err != nil {
88+
if xerrors.Is(err, context.Canceled) {
89+
return cliui.Canceled
90+
}
91+
return xerrors.Errorf("agent: %w", err)
8292
}
83-
return xerrors.Errorf("agent: %w", err)
84-
}
8593

86-
// If the ExpandedDirectory was initially missing, it could mean
87-
// that the agent hadn't reported it in yet. Retry once.
88-
if workspaceAgent.ExpandedDirectory == "" {
89-
autostart = false // Don't retry autostart.
90-
workspace, workspaceAgent, err = getWorkspaceAndAgent(ctx, inv, client, autostart, codersdk.Me, workspaceName)
91-
if err != nil {
92-
return xerrors.Errorf("get workspace and agent retry: %w", err)
94+
// If the ExpandedDirectory was initially missing, it could mean
95+
// that the agent hadn't reported it in yet. Retry once.
96+
if workspaceAgent.ExpandedDirectory == "" {
97+
autostart = false // Don't retry autostart.
98+
workspace, workspaceAgent, err = getWorkspaceAndAgent(ctx, inv, client, autostart, codersdk.Me, workspaceName)
99+
if err != nil {
100+
return xerrors.Errorf("get workspace and agent retry: %w", err)
101+
}
93102
}
94103
}
95104

96-
var folder string
105+
var directory string
97106
switch {
98107
case len(inv.Args) > 1:
99-
folder = inv.Args[1]
108+
directory = inv.Args[1]
100109
// Perhaps we could SSH in to expand the directory?
101-
if strings.HasPrefix(folder, "~") {
102-
return xerrors.Errorf("folder path %q not supported, use an absolute path instead", folder)
110+
if !insideThisWorkspace && strings.HasPrefix(directory, "~") {
111+
return xerrors.Errorf("directory path %q not supported, use an absolute path instead", directory)
112+
}
113+
if insideThisWorkspace {
114+
directory, err = filepath.Abs(directory)
115+
if err != nil {
116+
return xerrors.Errorf("expand directory: %w", err)
117+
}
103118
}
104119
case workspaceAgent.ExpandedDirectory != "":
105-
folder = workspaceAgent.ExpandedDirectory
120+
directory = workspaceAgent.ExpandedDirectory
121+
}
122+
123+
u, err := url.Parse("vscode://coder.coder-remote/open")
124+
if err != nil {
125+
return xerrors.Errorf("parse vscode URI: %w", err)
106126
}
107127

108128
qp := url.Values{}
109129

110130
qp.Add("url", client.URL.String())
111-
qp.Add("token", apiKey.Key)
112131
qp.Add("owner", workspace.OwnerName)
113132
qp.Add("workspace", workspace.Name)
114133
qp.Add("agent", workspaceAgent.Name)
115-
if folder != "" {
116-
qp.Add("folder", folder)
134+
if directory != "" {
135+
qp.Add("folder", directory)
117136
}
118137

119-
uri := fmt.Sprintf("vscode://coder.coder-remote/open?%s", qp.Encode())
120-
_, _ = fmt.Fprintf(inv.Stdout, "Opening %s\n", strings.ReplaceAll(uri, apiKey.Key, "<REDACTED>"))
138+
// We always set the token if we believe we can open without
139+
// printing the URI, otherwise the token must be explicitly
140+
// requested as it will be printed in plain text.
141+
if !insideAWorkspace || generateToken {
142+
// Prepare an API key. This is for automagical configuration of
143+
// VS Code, however, if running on a local machine we could try
144+
// to probe VS Code settings to see if the current configuration
145+
// is valid. Future improvement idea.
146+
apiKey, err := client.CreateAPIKey(ctx, codersdk.Me)
147+
if err != nil {
148+
return xerrors.Errorf("create API key: %w", err)
149+
}
150+
qp.Add("token", apiKey.Key)
151+
}
152+
153+
u.RawQuery = qp.Encode()
154+
155+
openingPath := workspaceName
156+
if directory != "" {
157+
openingPath += ":" + directory
158+
}
121159

122-
if testNoOpen {
160+
if insideAWorkspace {
161+
_, _ = fmt.Fprintf(inv.Stderr, "Opening %s in %s is not supported inside a workspace, please open the following URI on your local machine instead:\n\n", openingPath, vscodeDesktopName)
162+
_, _ = fmt.Fprintf(inv.Stdout, "%s\n", u.String())
123163
return nil
164+
} else {
165+
_, _ = fmt.Fprintf(inv.Stderr, "Opening %s in %s\n", openingPath, vscodeDesktopName)
124166
}
125167

126-
err = open.Run(uri)
168+
if !testNoOpen {
169+
err = open.Run(u.String())
170+
} else {
171+
err = xerrors.New("test.no-open")
172+
}
127173
if err != nil {
128-
return xerrors.Errorf("open: %w", err)
174+
if !generateToken {
175+
qp.Del("token")
176+
u.RawQuery = qp.Encode()
177+
}
178+
179+
_, _ = fmt.Fprintf(inv.Stderr, "Could not automatically open %s in %s: %s\n", openingPath, vscodeDesktopName, err)
180+
_, _ = fmt.Fprintf(inv.Stderr, "Please open the following URI instead:\n\n")
181+
_, _ = fmt.Fprintf(inv.Stdout, "%s\n", u.String())
182+
return nil
129183
}
130184

131185
return nil
132186
},
133187
}
134188

135189
cmd.Options = clibase.OptionSet{
190+
{
191+
Flag: "generate-token",
192+
Env: "CODER_OPEN_VSCODE_GENERATE_TOKEN",
193+
Description: fmt.Sprintf(
194+
"Generate an auth token and include it in the vscode:// URI. This is for automagical configuration of %s and not needed if already configured. "+
195+
"This flag does not need to be specified when running this command on a local machine unless automatic open fails.",
196+
vscodeDesktopName,
197+
),
198+
Value: clibase.BoolOf(&generateToken),
199+
},
136200
{
137201
Flag: "test.no-open",
138202
Description: "Don't run the open command.",

‎cli/open_test.go

+58-11
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package cli_test
33
import (
44
"context"
55
"net/url"
6-
"strings"
76
"testing"
87

98
"github.com/stretchr/testify/assert"
@@ -21,7 +20,7 @@ import (
2120
func TestOpen(t *testing.T) {
2221
t.Parallel()
2322

24-
t.Run("VSCode", func(t *testing.T) {
23+
t.Run("VS Code Local", func(t *testing.T) {
2524
t.Parallel()
2625

2726
client, workspace, agentToken := setupWorkspaceForAgent(t, func(agents []*proto.Agent) []*proto.Agent {
@@ -30,16 +29,69 @@ func TestOpen(t *testing.T) {
3029
return agents
3130
})
3231

32+
_ = agenttest.New(t, client.URL, agentToken)
33+
_ = coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID)
34+
3335
inv, root := clitest.New(t, "open", "vscode", "--test.no-open", workspace.Name)
3436
clitest.SetupConfig(t, client, root)
3537
pty := ptytest.New(t)
3638
inv.Stdin = pty.Input()
37-
inv.Stderr = pty.Output()
3839
inv.Stdout = pty.Output()
3940

41+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
42+
defer cancel()
43+
44+
cmdDone := tGo(t, func() {
45+
err := inv.WithContext(ctx).Run()
46+
assert.NoError(t, err)
47+
})
48+
49+
me, err := client.User(ctx, codersdk.Me)
50+
require.NoError(t, err)
51+
52+
// --test.no-open forces the command to print the URI.
53+
line := pty.ReadLine(ctx)
54+
u, err := url.ParseRequestURI(line)
55+
require.NoError(t, err, "line: %q", line)
56+
57+
qp := u.Query()
58+
assert.Equal(t, client.URL.String(), qp.Get("url"))
59+
assert.Equal(t, me.Username, qp.Get("owner"))
60+
assert.Equal(t, workspace.Name, qp.Get("workspace"))
61+
assert.Equal(t, "agent1", qp.Get("agent"))
62+
assert.Equal(t, "/tmp", qp.Get("folder"))
63+
assert.Equal(t, "", qp.Get("token"))
64+
65+
<-cmdDone
66+
})
67+
t.Run("VS Code Inside Workspace Prints URI", func(t *testing.T) {
68+
t.Parallel()
69+
70+
agentName := "agent1"
71+
client, workspace, agentToken := setupWorkspaceForAgent(t, func(agents []*proto.Agent) []*proto.Agent {
72+
agents[0].Directory = "/tmp"
73+
agents[0].Name = agentName
74+
return agents
75+
})
76+
4077
_ = agenttest.New(t, client.URL, agentToken)
4178
_ = coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID)
4279

80+
t.Log(client.SessionToken())
81+
82+
inv, root := clitest.New(t, "open", "vscode", "--generate-token", workspace.Name)
83+
clitest.SetupConfig(t, client, root)
84+
85+
t.Log(root.Session().Read())
86+
87+
pty := ptytest.New(t)
88+
inv.Stdin = pty.Input()
89+
inv.Stdout = pty.Output()
90+
91+
inv.Environ.Set("CODER", "true")
92+
inv.Environ.Set("CODER_WORKSPACE_NAME", workspace.Name)
93+
inv.Environ.Set("CODER_WORKSPACE_AGENT_NAME", agentName)
94+
4395
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
4496
defer cancel()
4597

@@ -52,22 +104,17 @@ func TestOpen(t *testing.T) {
52104
require.NoError(t, err)
53105

54106
line := pty.ReadLine(ctx)
55-
56-
// Opening vscode://coder.coder-remote/open?...
57-
parts := strings.Split(line, " ")
58-
require.Len(t, parts, 2)
59-
require.Contains(t, parts[1], "vscode://")
60-
u, err := url.ParseRequestURI(parts[1])
61-
require.NoError(t, err)
107+
u, err := url.ParseRequestURI(line)
108+
require.NoError(t, err, "line: %q", line)
62109

63110
qp := u.Query()
64111
assert.Equal(t, client.URL.String(), qp.Get("url"))
65112
assert.Equal(t, me.Username, qp.Get("owner"))
66113
assert.Equal(t, workspace.Name, qp.Get("workspace"))
67114
assert.Equal(t, "agent1", qp.Get("agent"))
68115
assert.Equal(t, "/tmp", qp.Get("folder"))
116+
assert.NotEmpty(t, qp.Get("token"))
69117

70-
cancel()
71118
<-cmdDone
72119
})
73120
}

‎coderd/workspaceagents.go

+2
Original file line numberDiff line numberDiff line change
@@ -241,8 +241,10 @@ func (api *API) workspaceAgentManifest(rw http.ResponseWriter, r *http.Request)
241241

242242
httpapi.Write(ctx, rw, http.StatusOK, agentsdk.Manifest{
243243
AgentID: apiAgent.ID,
244+
AgentName: apiAgent.Name,
244245
OwnerName: owner.Username,
245246
WorkspaceID: workspace.ID,
247+
WorkspaceName: workspace.Name,
246248
Apps: convertApps(dbApps, workspaceAgent, owner.Username, workspace),
247249
Scripts: convertScripts(scripts),
248250
DERPMap: api.DERPMap(),

‎codersdk/agentsdk/agentsdk.go

+5-3
Original file line numberDiff line numberDiff line change
@@ -97,12 +97,14 @@ func (c *Client) PostMetadata(ctx context.Context, req PostMetadataRequest) erro
9797
}
9898

9999
type Manifest struct {
100-
AgentID uuid.UUID `json:"agent_id"`
100+
AgentID uuid.UUID `json:"agent_id"`
101+
AgentName string `json:"agent_name"`
101102
// OwnerName and WorkspaceID are used by an open-source user to identify the workspace.
102103
// We do not provide insurance that this will not be removed in the future,
103104
// but if it's easy to persist lets keep it around.
104-
OwnerName string `json:"owner_name"`
105-
WorkspaceID uuid.UUID `json:"workspace_id"`
105+
OwnerName string `json:"owner_name"`
106+
WorkspaceID uuid.UUID `json:"workspace_id"`
107+
WorkspaceName string `json:"workspace_name"`
106108
// GitAuthConfigs stores the number of Git configurations
107109
// the Coder deployment has. If this number is >0, we
108110
// set up special configuration in the workspace.

0 commit comments

Comments
 (0)