From e35a0dde6b425eed36b03184ae23360bbfc83b21 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Fri, 28 Oct 2022 17:58:16 +0000 Subject: [PATCH 1/5] feat: Add `VSCODE_PROXY_URI` to surface code-server ports Fixes #4776. --- agent/agent.go | 5 ++++- coderd/activitybump_test.go | 2 +- coderd/workspaceagents.go | 41 ++++++++++++++++++++++++++++++++++++ coderd/workspaceapps_test.go | 13 ++++++++++-- codersdk/workspaceagents.go | 1 + 5 files changed, 58 insertions(+), 4 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index e9ba7672cd98f..7f5012609a087 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -569,7 +569,6 @@ func (a *agent) createCommand(ctx context.Context, rawCommand string, env []stri // Set environment variables reliable detection of being inside a // Coder workspace. cmd.Env = append(cmd.Env, "CODER=true") - cmd.Env = append(cmd.Env, fmt.Sprintf("USER=%s", username)) // Git on Windows resolves with UNIX-style paths. // If using backslashes, it's unable to find the executable. @@ -585,6 +584,10 @@ func (a *agent) createCommand(ctx context.Context, rawCommand string, env []stri cmd.Env = append(cmd.Env, fmt.Sprintf("SSH_CLIENT=%s %s %s", srcAddr, srcPort, dstPort)) cmd.Env = append(cmd.Env, fmt.Sprintf("SSH_CONNECTION=%s %s %s %s", srcAddr, srcPort, dstAddr, dstPort)) + // This adds the ports dialog to code-server that enables + // proxying a port dynamically. + cmd.Env = append(cmd.Env, fmt.Sprintf("VSCODE_PROXY_URI=%s", metadata.VSCodePortProxyURI)) + // Load environment variables passed via the agent. // These should override all variables we manually specify. for envKey, value := range metadata.EnvironmentVariables { diff --git a/coderd/activitybump_test.go b/coderd/activitybump_test.go index e498b98fa0c80..a67dc66e1c80d 100644 --- a/coderd/activitybump_test.go +++ b/coderd/activitybump_test.go @@ -31,7 +31,7 @@ func TestWorkspaceActivityBump(t *testing.T) { }) user := coderdtest.CreateFirstUser(t, client) - workspace = createWorkspaceWithApps(t, client, user.OrganizationID, 1234, func(cwr *codersdk.CreateWorkspaceRequest) { + workspace = createWorkspaceWithApps(t, client, user.OrganizationID, "", 1234, func(cwr *codersdk.CreateWorkspaceRequest) { cwr.TTLMillis = &ttlMillis }) diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go index edae77f37777e..e5694e618919f 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -83,6 +83,46 @@ func (api *API) workspaceAgentMetadata(rw http.ResponseWriter, r *http.Request) }) return } + resource, err := api.Database.GetWorkspaceResourceByID(r.Context(), workspaceAgent.ResourceID) + if err != nil { + httpapi.Write(r.Context(), rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error fetching workspace resource.", + Detail: err.Error(), + }) + return + } + build, err := api.Database.GetWorkspaceBuildByJobID(r.Context(), resource.JobID) + if err != nil { + httpapi.Write(r.Context(), rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error fetching workspace build.", + Detail: err.Error(), + }) + return + } + workspace, err := api.Database.GetWorkspaceByID(r.Context(), build.WorkspaceID) + if err != nil { + httpapi.Write(r.Context(), rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error fetching workspace.", + Detail: err.Error(), + }) + return + } + owner, err := api.Database.GetUserByID(r.Context(), workspace.OwnerID) + if err != nil { + httpapi.Write(r.Context(), rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error fetching workspace owner.", + Detail: err.Error(), + }) + return + } + + vscodeProxyURI := strings.ReplaceAll(api.AppHostname, "*", + fmt.Sprintf("%s://{{port}}--%s--%s--%s", + api.AccessURL.Scheme, + workspaceAgent.Name, + workspace.Name, + owner.Username, + )) httpapi.Write(ctx, rw, http.StatusOK, codersdk.WorkspaceAgentMetadata{ Apps: convertApps(dbApps), @@ -91,6 +131,7 @@ func (api *API) workspaceAgentMetadata(rw http.ResponseWriter, r *http.Request) EnvironmentVariables: apiAgent.EnvironmentVariables, StartupScript: apiAgent.StartupScript, Directory: apiAgent.Directory, + VSCodePortProxyURI: vscodeProxyURI, }) } diff --git a/coderd/workspaceapps_test.go b/coderd/workspaceapps_test.go index fde0f54e07d05..3b54a83cd8831 100644 --- a/coderd/workspaceapps_test.go +++ b/coderd/workspaceapps_test.go @@ -121,7 +121,7 @@ func setupProxyTest(t *testing.T, customAppHost ...string) (*codersdk.Client, co }) user := coderdtest.CreateFirstUser(t, client) - workspace := createWorkspaceWithApps(t, client, user.OrganizationID, uint16(tcpAddr.Port)) + workspace := createWorkspaceWithApps(t, client, user.OrganizationID, appHost, uint16(tcpAddr.Port)) // Configure the HTTP client to not follow redirects and to route all // requests regardless of hostname to the coderd test server. @@ -139,7 +139,7 @@ func setupProxyTest(t *testing.T, customAppHost ...string) (*codersdk.Client, co return client, user, workspace, uint16(tcpAddr.Port) } -func createWorkspaceWithApps(t *testing.T, client *codersdk.Client, orgID uuid.UUID, port uint16, workspaceMutators ...func(*codersdk.CreateWorkspaceRequest)) codersdk.Workspace { +func createWorkspaceWithApps(t *testing.T, client *codersdk.Client, orgID uuid.UUID, appHost string, port uint16, workspaceMutators ...func(*codersdk.CreateWorkspaceRequest)) codersdk.Workspace { authToken := uuid.NewString() appURL := fmt.Sprintf("http://127.0.0.1:%d?%s", port, proxyTestAppQuery) @@ -194,6 +194,15 @@ func createWorkspaceWithApps(t *testing.T, client *codersdk.Client, orgID uuid.U agentClient := codersdk.New(client.URL) agentClient.SessionToken = authToken + metadata, err := agentClient.WorkspaceAgentMetadata(context.Background()) + require.NoError(t, err) + require.Equal(t, fmt.Sprintf( + "http://{{port}}--%s--%s--%s%s", + proxyTestAgentName, + workspace.Name, + "testuser", + strings.ReplaceAll(appHost, "*", ""), + ), metadata.VSCodePortProxyURI) agentCloser := agent.New(agent.Options{ Client: agentClient, Logger: slogtest.Make(t, nil).Named("agent"), diff --git a/codersdk/workspaceagents.go b/codersdk/workspaceagents.go index b3c82772c2917..bf15820f57cd3 100644 --- a/codersdk/workspaceagents.go +++ b/codersdk/workspaceagents.go @@ -123,6 +123,7 @@ type WorkspaceAgentMetadata struct { // the Coder deployment has. If this number is >0, we // set up special configuration in the workspace. GitAuthConfigs int `json:"git_auth_configs"` + VSCodePortProxyURI string `json:"vscode_port_proxy_uri"` Apps []WorkspaceApp `json:"apps"` DERPMap *tailcfg.DERPMap `json:"derpmap"` EnvironmentVariables map[string]string `json:"environment_variables"` From 9ee60d67e86dfa1c75d35e8f39cf758f9c744a16 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Tue, 1 Nov 2022 21:11:26 +0000 Subject: [PATCH 2/5] Check if app host is provided --- coderd/workspaceapps_test.go | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/coderd/workspaceapps_test.go b/coderd/workspaceapps_test.go index 3b54a83cd8831..a854be144f4d8 100644 --- a/coderd/workspaceapps_test.go +++ b/coderd/workspaceapps_test.go @@ -194,15 +194,17 @@ func createWorkspaceWithApps(t *testing.T, client *codersdk.Client, orgID uuid.U agentClient := codersdk.New(client.URL) agentClient.SessionToken = authToken - metadata, err := agentClient.WorkspaceAgentMetadata(context.Background()) - require.NoError(t, err) - require.Equal(t, fmt.Sprintf( - "http://{{port}}--%s--%s--%s%s", - proxyTestAgentName, - workspace.Name, - "testuser", - strings.ReplaceAll(appHost, "*", ""), - ), metadata.VSCodePortProxyURI) + if appHost != "" { + metadata, err := agentClient.WorkspaceAgentMetadata(context.Background()) + require.NoError(t, err) + require.Equal(t, fmt.Sprintf( + "http://{{port}}--%s--%s--%s%s", + proxyTestAgentName, + workspace.Name, + "testuser", + strings.ReplaceAll(appHost, "*", ""), + ), metadata.VSCodePortProxyURI) + } agentCloser := agent.New(agent.Options{ Client: agentClient, Logger: slogtest.Make(t, nil).Named("agent"), From 87b5113774061981dcbb341a468bdb52a1560693 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Tue, 1 Nov 2022 21:28:36 +0000 Subject: [PATCH 3/5] Fix conn cache --- coderd/wsconncache/wsconncache.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/coderd/wsconncache/wsconncache.go b/coderd/wsconncache/wsconncache.go index 252ef5897f195..47853ec4ca150 100644 --- a/coderd/wsconncache/wsconncache.go +++ b/coderd/wsconncache/wsconncache.go @@ -86,8 +86,12 @@ func (c *Cache) Acquire(r *http.Request, id uuid.UUID) (*Conn, func(), error) { // A singleflight group is used to allow for concurrent requests to the // same identifier to resolve. rawConn, err, _ = c.connGroup.Do(id.String(), func() (interface{}, error) { + c.closeMutex.Lock() + c.closeGroup.Add(1) + c.closeMutex.Unlock() agentConn, err := c.dialer(r, id) if err != nil { + c.closeGroup.Done() return nil, xerrors.Errorf("dial: %w", err) } timeoutCtx, timeoutCancelFunc := context.WithCancel(context.Background()) @@ -102,9 +106,6 @@ func (c *Cache) Acquire(r *http.Request, id uuid.UUID) (*Conn, func(), error) { timeoutCancel: timeoutCancelFunc, transport: transport, } - c.closeMutex.Lock() - c.closeGroup.Add(1) - c.closeMutex.Unlock() go func() { defer c.closeGroup.Done() var err error From c9ec3ce682b94c080a88e11eba5a7b9d0b967b38 Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Thu, 3 Nov 2022 21:43:44 +0000 Subject: [PATCH 4/5] Fix race condition on close --- agent/agent.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index 7f5012609a087..e0d9549149c4f 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -221,6 +221,13 @@ func (a *agent) run(ctx context.Context) error { } func (a *agent) createTailnet(ctx context.Context, derpMap *tailcfg.DERPMap) (*tailnet.Conn, error) { + a.closeMutex.Lock() + if a.isClosed() { + a.closeMutex.Unlock() + return nil, xerrors.New("closed") + } + a.connCloseWait.Add(1) + a.closeMutex.Unlock() network, err := tailnet.NewConn(&tailnet.Options{ Addresses: []netip.Prefix{netip.PrefixFrom(codersdk.TailnetIP, 128)}, DERPMap: derpMap, @@ -242,9 +249,6 @@ func (a *agent) createTailnet(ctx context.Context, derpMap *tailcfg.DERPMap) (*t if err != nil { return nil, xerrors.Errorf("listen on the ssh port: %w", err) } - a.closeMutex.Lock() - a.connCloseWait.Add(1) - a.closeMutex.Unlock() go func() { defer a.connCloseWait.Done() for { From 8482346c210b42f339d81e695217fb1604e2f8cc Mon Sep 17 00:00:00 2001 From: Kyle Carberry Date: Fri, 4 Nov 2022 02:40:40 +0000 Subject: [PATCH 5/5] Fix vscode configs test --- agent/agent.go | 14 +++----------- agent/agent_test.go | 2 ++ 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index a9c8a19d0076f..013439788e7ac 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -226,14 +226,13 @@ func (a *agent) createTailnet(ctx context.Context, derpMap *tailcfg.DERPMap) (*t a.closeMutex.Unlock() return nil, xerrors.New("closed") } - a.connCloseWait.Add(1) - a.closeMutex.Unlock() network, err := tailnet.NewConn(&tailnet.Options{ Addresses: []netip.Prefix{netip.PrefixFrom(codersdk.TailnetIP, 128)}, DERPMap: derpMap, Logger: a.logger.Named("tailnet"), }) if err != nil { + a.closeMutex.Unlock() return nil, xerrors.Errorf("create tailnet: %w", err) } a.network = network @@ -244,6 +243,8 @@ func (a *agent) createTailnet(ctx context.Context, derpMap *tailcfg.DERPMap) (*t } return a.stats.wrapConn(conn) }) + a.connCloseWait.Add(4) + a.closeMutex.Unlock() sshListener, err := network.Listen("tcp", ":"+strconv.Itoa(codersdk.TailnetSSHPort)) if err != nil { @@ -264,9 +265,6 @@ func (a *agent) createTailnet(ctx context.Context, derpMap *tailcfg.DERPMap) (*t if err != nil { return nil, xerrors.Errorf("listen for reconnecting pty: %w", err) } - a.closeMutex.Lock() - a.connCloseWait.Add(1) - a.closeMutex.Unlock() go func() { defer a.connCloseWait.Done() for { @@ -302,9 +300,6 @@ func (a *agent) createTailnet(ctx context.Context, derpMap *tailcfg.DERPMap) (*t if err != nil { return nil, xerrors.Errorf("listen for speedtest: %w", err) } - a.closeMutex.Lock() - a.connCloseWait.Add(1) - a.closeMutex.Unlock() go func() { defer a.connCloseWait.Done() for { @@ -327,9 +322,6 @@ func (a *agent) createTailnet(ctx context.Context, derpMap *tailcfg.DERPMap) (*t if err != nil { return nil, xerrors.Errorf("listen for statistics: %w", err) } - a.closeMutex.Lock() - a.connCloseWait.Add(1) - a.closeMutex.Unlock() go func() { defer a.connCloseWait.Done() defer statisticsListener.Close() diff --git a/agent/agent_test.go b/agent/agent_test.go index d04843fd1f4b2..3117f2c13850b 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -23,6 +23,7 @@ import ( "golang.org/x/xerrors" "tailscale.com/net/speedtest" + "tailscale.com/tailcfg" scp "github.com/bramvdbogaerde/go-scp" "github.com/google/uuid" @@ -559,6 +560,7 @@ func TestAgent(t *testing.T) { agentID: uuid.New(), metadata: codersdk.WorkspaceAgentMetadata{ GitAuthConfigs: 1, + DERPMap: &tailcfg.DERPMap{}, }, statsChan: make(chan *codersdk.AgentStats), coordinator: tailnet.NewCoordinator(),