fix: code-server path based forwarding, defer to code-server (#11759)

Emyrk · web-flow · commit 081fbef09773 · 2024-01-23T11:36:44.000-06:00
Do not attempt to construct a path based port forward url.
Always defer to code server, as it has it's own proxy method.
diff --git a/agent/agentssh/agentssh.go b/agent/agentssh/agentssh.go
@@ -681,7 +681,11 @@ func (s *Server) CreateCommand(ctx context.Context, script string, env []string)
 
 	// 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", manifest.VSCodePortProxyURI))
+	// If this is empty string, do not set anything. Code-server auto defaults
+	// using its basepath to construct a path based port proxy.
+	if manifest.VSCodePortProxyURI != "" {
+		cmd.Env = append(cmd.Env, fmt.Sprintf("VSCODE_PROXY_URI=%s", manifest.VSCodePortProxyURI))
+	}
 
 	// Hide Coder message on code-server's "Getting Started" page
 	cmd.Env = append(cmd.Env, "CS_DISABLE_GETTING_STARTED_OVERRIDE=true")
diff --git a/coderd/agentapi/manifest.go b/coderd/agentapi/manifest.go
@@ -150,12 +150,14 @@ func (a *ManifestAPI) GetManifest(ctx context.Context, _ *agentproto.GetManifest
 }
 
 func vscodeProxyURI(app appurl.ApplicationURL, accessURL *url.URL, appHost string) string {
-	// This will handle the ports from the accessURL or appHost.
-	appHost = appurl.SubdomainAppHost(appHost, accessURL)
-	// If there is no appHost, then we want to use the access url as the proxy uri.
+	// Proxying by port only works for subdomains. If subdomain support is not
+	// available, return an empty string.
 	if appHost == "" {
-		appHost = accessURL.Host
+		return ""
 	}
+
+	// This will handle the ports from the accessURL or appHost.
+	appHost = appurl.SubdomainAppHost(appHost, accessURL)
 	// Return the url with a scheme and any wildcards replaced with the app slug.
 	return accessURL.Scheme + "://" + strings.ReplaceAll(appHost, "*", app.String())
 }
diff --git a/coderd/agentapi/manifest_internal_test.go b/coderd/agentapi/manifest_internal_test.go
@@ -35,19 +35,18 @@ func Test_vscodeProxyURI(t *testing.T) {
 		Expected    string
 	}{
 		{
-			// No hostname proxies through the access url.
 			Name:        "NoHostname",
 			AccessURL:   coderAccessURL,
 			AppHostname: "",
 			App:         basicApp,
-			Expected:    coderAccessURL.String(),
+			Expected:    "",
 		},
 		{
 			Name:        "NoHostnameAccessURLPort",
 			AccessURL:   accessURLWithPort,
 			AppHostname: "",
 			App:         basicApp,
-			Expected:    accessURLWithPort.String(),
+			Expected:    "",
 		},
 		{
 			Name:        "Hostname",

Original file line number	Diff line number	Diff line change
`@@ -150,12 +150,14 @@ func (a ManifestAPI) GetManifest(ctx context.Context, _ agentproto.GetManifest`
`150`	`150`	`}`
`151`	`151`
`152`	`152`	`func vscodeProxyURI(app appurl.ApplicationURL, accessURL *url.URL, appHost string) string {`
`153`		`- // This will handle the ports from the accessURL or appHost.`
`154`		`- appHost = appurl.SubdomainAppHost(appHost, accessURL)`
`155`		`- // If there is no appHost, then we want to use the access url as the proxy uri.`
	`153`	`+ // Proxying by port only works for subdomains. If subdomain support is not`
	`154`	`+ // available, return an empty string.`
`156`	`155`	`if appHost == "" {`
`157`		`- appHost = accessURL.Host`
	`156`	`+ return ""`
`158`	`157`	`}`
	`158`	`+`
	`159`	`+ // This will handle the ports from the accessURL or appHost.`
	`160`	`+ appHost = appurl.SubdomainAppHost(appHost, accessURL)`
`159`	`161`	`// Return the url with a scheme and any wildcards replaced with the app slug.`
`160`	`162`	`return accessURL.Scheme + "://" + strings.ReplaceAll(appHost, "*", app.String())`
`161`	`163`	`}`