From 8ba05a90521fc59965e2c21d4a134ad638a6dbc0 Mon Sep 17 00:00:00 2001 From: Garrett Delfosse Date: Fri, 26 Apr 2024 11:52:53 -0400 Subject: [PATCH] feat: add switch http(s) button to error page (#12942) --- Makefile | 3 +- coderd/tailnet.go | 51 +++++++++++++++++++--- coderd/tailnet_test.go | 17 ++++---- coderd/workspaceapps/appurl/appurl.go | 50 +++++++++++++++++++++ coderd/workspaceapps/appurl/appurl_test.go | 10 +++++ coderd/workspaceapps/proxy.go | 14 +++--- site/site.go | 15 ++++--- site/static/error.html | 11 ++++- 8 files changed, 142 insertions(+), 29 deletions(-) diff --git a/Makefile b/Makefile index 5eeba4bb06..dd31c10fff 100644 --- a/Makefile +++ b/Makefile @@ -200,7 +200,8 @@ endef # calling this manually. $(CODER_ALL_BINARIES): go.mod go.sum \ $(GO_SRC_FILES) \ - $(shell find ./examples/templates) + $(shell find ./examples/templates) \ + site/static/error.html $(get-mode-os-arch-ext) if [[ "$$os" != "windows" ]] && [[ "$$ext" != "" ]]; then diff --git a/coderd/tailnet.go b/coderd/tailnet.go index 0bcf21bb9d..e995f92fe6 100644 --- a/coderd/tailnet.go +++ b/coderd/tailnet.go @@ -4,11 +4,14 @@ import ( "bufio" "context" "crypto/tls" + "errors" + "fmt" "net" "net/http" "net/http/httputil" "net/netip" "net/url" + "strings" "sync" "sync/atomic" "time" @@ -23,6 +26,7 @@ import ( "cdr.dev/slog" "github.com/coder/coder/v2/coderd/tracing" "github.com/coder/coder/v2/coderd/workspaceapps" + "github.com/coder/coder/v2/coderd/workspaceapps/appurl" "github.com/coder/coder/v2/codersdk/workspacesdk" "github.com/coder/coder/v2/site" "github.com/coder/coder/v2/tailnet" @@ -341,7 +345,7 @@ type ServerTailnet struct { totalConns *prometheus.CounterVec } -func (s *ServerTailnet) ReverseProxy(targetURL, dashboardURL *url.URL, agentID uuid.UUID) *httputil.ReverseProxy { +func (s *ServerTailnet) ReverseProxy(targetURL, dashboardURL *url.URL, agentID uuid.UUID, app appurl.ApplicationURL, wildcardHostname string) *httputil.ReverseProxy { // Rewrite the targetURL's Host to point to the agent's IP. This is // necessary because due to TCP connection caching, each agent needs to be // addressed invidivually. Otherwise, all connections get dialed as @@ -351,13 +355,46 @@ func (s *ServerTailnet) ReverseProxy(targetURL, dashboardURL *url.URL, agentID u tgt.Host = net.JoinHostPort(tailnet.IPFromUUID(agentID).String(), port) proxy := httputil.NewSingleHostReverseProxy(&tgt) - proxy.ErrorHandler = func(w http.ResponseWriter, r *http.Request, err error) { + proxy.ErrorHandler = func(w http.ResponseWriter, r *http.Request, theErr error) { + var ( + desc = "Failed to proxy request to application: " + theErr.Error() + additionalInfo = "" + additionalButtonLink = "" + additionalButtonText = "" + ) + + var tlsError tls.RecordHeaderError + if (errors.As(theErr, &tlsError) && tlsError.Msg == "first record does not look like a TLS handshake") || + errors.Is(theErr, http.ErrSchemeMismatch) { + // If the error is due to an HTTP/HTTPS mismatch, we can provide a + // more helpful error message with redirect buttons. + switchURL := url.URL{ + Scheme: dashboardURL.Scheme, + } + _, protocol, isPort := app.PortInfo() + if isPort { + targetProtocol := "https" + if protocol == "https" { + targetProtocol = "http" + } + app = app.ChangePortProtocol(targetProtocol) + + switchURL.Host = fmt.Sprintf("%s%s", app.String(), strings.TrimPrefix(wildcardHostname, "*")) + additionalButtonLink = switchURL.String() + additionalButtonText = fmt.Sprintf("Switch to %s", strings.ToUpper(targetProtocol)) + additionalInfo += fmt.Sprintf("This error seems to be due to an app protocol mismatch, try switching to %s.", strings.ToUpper(targetProtocol)) + } + } + site.RenderStaticErrorPage(w, r, site.ErrorPageData{ - Status: http.StatusBadGateway, - Title: "Bad Gateway", - Description: "Failed to proxy request to application: " + err.Error(), - RetryEnabled: true, - DashboardURL: dashboardURL.String(), + Status: http.StatusBadGateway, + Title: "Bad Gateway", + Description: desc, + RetryEnabled: true, + DashboardURL: dashboardURL.String(), + AdditionalInfo: additionalInfo, + AdditionalButtonLink: additionalButtonLink, + AdditionalButtonText: additionalButtonText, }) } proxy.Director = s.director(agentID, proxy.Director) diff --git a/coderd/tailnet_test.go b/coderd/tailnet_test.go index 0a78a8349c..d4dac9b94c 100644 --- a/coderd/tailnet_test.go +++ b/coderd/tailnet_test.go @@ -26,6 +26,7 @@ import ( "github.com/coder/coder/v2/agent/agenttest" "github.com/coder/coder/v2/agent/proto" "github.com/coder/coder/v2/coderd" + "github.com/coder/coder/v2/coderd/workspaceapps/appurl" "github.com/coder/coder/v2/codersdk/agentsdk" "github.com/coder/coder/v2/codersdk/workspacesdk" "github.com/coder/coder/v2/tailnet" @@ -81,7 +82,7 @@ func TestServerTailnet_ReverseProxy_ProxyEnv(t *testing.T) { u, err := url.Parse(fmt.Sprintf("http://127.0.0.1:%d", workspacesdk.AgentHTTPAPIServerPort)) require.NoError(t, err) - rp := serverTailnet.ReverseProxy(u, u, a.id) + rp := serverTailnet.ReverseProxy(u, u, a.id, appurl.ApplicationURL{}, "") rw := httptest.NewRecorder() req := httptest.NewRequest( @@ -112,7 +113,7 @@ func TestServerTailnet_ReverseProxy(t *testing.T) { u, err := url.Parse(fmt.Sprintf("http://127.0.0.1:%d", workspacesdk.AgentHTTPAPIServerPort)) require.NoError(t, err) - rp := serverTailnet.ReverseProxy(u, u, a.id) + rp := serverTailnet.ReverseProxy(u, u, a.id, appurl.ApplicationURL{}, "") rw := httptest.NewRecorder() req := httptest.NewRequest( @@ -143,7 +144,7 @@ func TestServerTailnet_ReverseProxy(t *testing.T) { u, err := url.Parse(fmt.Sprintf("http://127.0.0.1:%d", workspacesdk.AgentHTTPAPIServerPort)) require.NoError(t, err) - rp := serverTailnet.ReverseProxy(u, u, a.id) + rp := serverTailnet.ReverseProxy(u, u, a.id, appurl.ApplicationURL{}, "") rw := httptest.NewRecorder() req := httptest.NewRequest( @@ -177,7 +178,7 @@ func TestServerTailnet_ReverseProxy(t *testing.T) { u, err := url.Parse(fmt.Sprintf("http://127.0.0.1:%d", workspacesdk.AgentHTTPAPIServerPort)) require.NoError(t, err) - rp := serverTailnet.ReverseProxy(u, u, a.id) + rp := serverTailnet.ReverseProxy(u, u, a.id, appurl.ApplicationURL{}, "") req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil) require.NoError(t, err) @@ -222,7 +223,7 @@ func TestServerTailnet_ReverseProxy(t *testing.T) { u, err := url.Parse("http://127.0.0.1" + port) require.NoError(t, err) - rp := serverTailnet.ReverseProxy(u, u, a.id) + rp := serverTailnet.ReverseProxy(u, u, a.id, appurl.ApplicationURL{}, "") for i := 0; i < 5; i++ { rw := httptest.NewRecorder() @@ -279,7 +280,7 @@ func TestServerTailnet_ReverseProxy(t *testing.T) { require.NoError(t, err) for i, ag := range agents { - rp := serverTailnet.ReverseProxy(u, u, ag.id) + rp := serverTailnet.ReverseProxy(u, u, ag.id, appurl.ApplicationURL{}, "") rw := httptest.NewRecorder() req := httptest.NewRequest( @@ -317,7 +318,7 @@ func TestServerTailnet_ReverseProxy(t *testing.T) { uri, err := url.Parse(s.URL) require.NoError(t, err) - rp := serverTailnet.ReverseProxy(uri, uri, a.id) + rp := serverTailnet.ReverseProxy(uri, uri, a.id, appurl.ApplicationURL{}, "") rw := httptest.NewRecorder() req := httptest.NewRequest( @@ -347,7 +348,7 @@ func TestServerTailnet_ReverseProxy(t *testing.T) { u, err := url.Parse(fmt.Sprintf("http://127.0.0.1:%d", workspacesdk.AgentHTTPAPIServerPort)) require.NoError(t, err) - rp := serverTailnet.ReverseProxy(u, u, a.id) + rp := serverTailnet.ReverseProxy(u, u, a.id, appurl.ApplicationURL{}, "") rw := httptest.NewRecorder() req := httptest.NewRequest( diff --git a/coderd/workspaceapps/appurl/appurl.go b/coderd/workspaceapps/appurl/appurl.go index 8b8cfd74d3..31ec677354 100644 --- a/coderd/workspaceapps/appurl/appurl.go +++ b/coderd/workspaceapps/appurl/appurl.go @@ -5,6 +5,7 @@ import ( "net" "net/url" "regexp" + "strconv" "strings" "golang.org/x/xerrors" @@ -83,6 +84,55 @@ func (a ApplicationURL) Path() string { return fmt.Sprintf("/@%s/%s.%s/apps/%s", a.Username, a.WorkspaceName, a.AgentName, a.AppSlugOrPort) } +// PortInfo returns the port, protocol, and whether the AppSlugOrPort is a port or not. +func (a ApplicationURL) PortInfo() (uint, string, bool) { + var ( + port uint64 + protocol string + isPort bool + err error + ) + + if strings.HasSuffix(a.AppSlugOrPort, "s") { + trimmed := strings.TrimSuffix(a.AppSlugOrPort, "s") + port, err = strconv.ParseUint(trimmed, 10, 16) + if err == nil { + protocol = "https" + isPort = true + } + } else { + port, err = strconv.ParseUint(a.AppSlugOrPort, 10, 16) + if err == nil { + protocol = "http" + isPort = true + } + } + + return uint(port), protocol, isPort +} + +func (a *ApplicationURL) ChangePortProtocol(target string) ApplicationURL { + newAppURL := *a + port, protocol, isPort := a.PortInfo() + if !isPort { + return newAppURL + } + + if target == protocol { + return newAppURL + } + + if target == "https" { + newAppURL.AppSlugOrPort = fmt.Sprintf("%ds", port) + } + + if target == "http" { + newAppURL.AppSlugOrPort = fmt.Sprintf("%d", port) + } + + return newAppURL +} + // ParseSubdomainAppURL parses an ApplicationURL from the given subdomain. If // the subdomain is not a valid application URL hostname, returns a non-nil // error. If the hostname is not a subdomain of the given base hostname, returns diff --git a/coderd/workspaceapps/appurl/appurl_test.go b/coderd/workspaceapps/appurl/appurl_test.go index 98a34c6003..8353768de1 100644 --- a/coderd/workspaceapps/appurl/appurl_test.go +++ b/coderd/workspaceapps/appurl/appurl_test.go @@ -124,6 +124,16 @@ func TestParseSubdomainAppURL(t *testing.T) { Username: "user", }, }, + { + Name: "Port--Agent--Workspace--User", + Subdomain: "8080s--agent--workspace--user", + Expected: appurl.ApplicationURL{ + AppSlugOrPort: "8080s", + AgentName: "agent", + WorkspaceName: "workspace", + Username: "user", + }, + }, { Name: "HyphenatedNames", Subdomain: "app-slug--agent-name--workspace-name--user-name", diff --git a/coderd/workspaceapps/proxy.go b/coderd/workspaceapps/proxy.go index 5b14cbf340..7bf470a3cc 100644 --- a/coderd/workspaceapps/proxy.go +++ b/coderd/workspaceapps/proxy.go @@ -66,7 +66,7 @@ var nonCanonicalHeaders = map[string]string{ type AgentProvider interface { // ReverseProxy returns an httputil.ReverseProxy for proxying HTTP requests // to the specified agent. - ReverseProxy(targetURL, dashboardURL *url.URL, agentID uuid.UUID) *httputil.ReverseProxy + ReverseProxy(targetURL, dashboardURL *url.URL, agentID uuid.UUID, app appurl.ApplicationURL, wildcardHost string) *httputil.ReverseProxy // AgentConn returns a new connection to the specified agent. AgentConn(ctx context.Context, agentID uuid.UUID) (_ *workspacesdk.AgentConn, release func(), _ error) @@ -314,7 +314,7 @@ func (s *Server) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request) return } - s.proxyWorkspaceApp(rw, r, *token, chiPath) + s.proxyWorkspaceApp(rw, r, *token, chiPath, appurl.ApplicationURL{}) } // HandleSubdomain handles subdomain-based application proxy requests (aka. @@ -417,7 +417,7 @@ func (s *Server) HandleSubdomain(middlewares ...func(http.Handler) http.Handler) if !ok { return } - s.proxyWorkspaceApp(rw, r, *token, r.URL.Path) + s.proxyWorkspaceApp(rw, r, *token, r.URL.Path, app) })).ServeHTTP(rw, r.WithContext(ctx)) }) } @@ -476,7 +476,7 @@ func (s *Server) parseHostname(rw http.ResponseWriter, r *http.Request, next htt return app, true } -func (s *Server) proxyWorkspaceApp(rw http.ResponseWriter, r *http.Request, appToken SignedToken, path string) { +func (s *Server) proxyWorkspaceApp(rw http.ResponseWriter, r *http.Request, appToken SignedToken, path string, app appurl.ApplicationURL) { ctx := r.Context() // Filter IP headers from untrusted origins. @@ -545,8 +545,12 @@ func (s *Server) proxyWorkspaceApp(rw http.ResponseWriter, r *http.Request, appT r.URL.Path = path appURL.RawQuery = "" + _, protocol, isPort := app.PortInfo() + if isPort { + appURL.Scheme = protocol + } - proxy := s.AgentProvider.ReverseProxy(appURL, s.DashboardURL, appToken.AgentID) + proxy := s.AgentProvider.ReverseProxy(appURL, s.DashboardURL, appToken.AgentID, app, s.Hostname) proxy.ModifyResponse = func(r *http.Response) error { r.Header.Del(httpmw.AccessControlAllowOriginHeader) diff --git a/site/site.go b/site/site.go index 1f7a1120f1..1d9d31d5fc 100644 --- a/site/site.go +++ b/site/site.go @@ -786,12 +786,15 @@ func extractBin(dest string, r io.Reader) (numExtracted int, err error) { type ErrorPageData struct { Status int // HideStatus will remove the status code from the page. - HideStatus bool - Title string - Description string - RetryEnabled bool - DashboardURL string - Warnings []string + HideStatus bool + Title string + Description string + RetryEnabled bool + DashboardURL string + Warnings []string + AdditionalInfo string + AdditionalButtonLink string + AdditionalButtonText string RenderDescriptionMarkdown bool } diff --git a/site/static/error.html b/site/static/error.html index a07c96b8ec..d0eee1bb4a 100644 --- a/site/static/error.html +++ b/site/static/error.html @@ -33,7 +33,7 @@ running). */}} .container { --side-padding: 24px; width: 100%; - max-width: calc(320px + var(--side-padding) * 2); + max-width: calc(500px + var(--side-padding) * 2); padding: 0 var(--side-padding); text-align: center; } @@ -170,6 +170,9 @@ running). */}} {{- if .Error.RenderDescriptionMarkdown }} {{ .ErrorDescriptionHTML }} {{ else }}

{{ .Error.Description }}

+ {{ end }} {{- if .Error.AdditionalInfo }} +
+

{{ .Error.AdditionalInfo }}

{{ end }} {{- if .Error.Warnings }}
@@ -195,7 +198,11 @@ running). */}}
{{ end }}
- {{- if .Error.RetryEnabled }} + {{- if and .Error.AdditionalButtonText .Error.AdditionalButtonLink }} + {{ .Error.AdditionalButtonText }} + {{ end }} {{- if .Error.RetryEnabled }} {{ end }} Back to site