fix: enforce agent and app health in ticket code

coder · deansheather · Mar 30, 2023 · Mar 16, 2023 · Mar 17, 2023 · Mar 28, 2023
commit 9398ee4e4b4f103bae6fcc52cb700460fbaf5dfe
diff --git a/coderd/coderd.go b/coderd/coderd.go
@@ -287,6 +287,7 @@ func New(options *Options) *API {
 			options.Database,
 			options.DeploymentValues,
 			oauthConfigs,
+			options.AgentInactiveDisconnectTimeout,
 			options.AppSigningKey,
 		),
 		metricsCache:          metricsCache,

diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go
@@ -3,6 +3,7 @@ package database
 import (
 	"sort"
 	"strconv"
+	"time"
 
 	"golang.org/x/exp/maps"
 
@@ -36,6 +37,26 @@ func (s WorkspaceStatus) Valid() bool {
 	}
 }
 
+type WorkspaceAgentStatus string
+
+// This is also in codersdk/workspaceagents.go and should be kept in sync.
+const (
+	WorkspaceAgentStatusConnecting   WorkspaceAgentStatus = "connecting"
+	WorkspaceAgentStatusConnected    WorkspaceAgentStatus = "connected"
+	WorkspaceAgentStatusDisconnected WorkspaceAgentStatus = "disconnected"
+	WorkspaceAgentStatusTimeout      WorkspaceAgentStatus = "timeout"
+)
+
+func (s WorkspaceAgentStatus) Valid() bool {
+	switch s {
+	case WorkspaceAgentStatusConnecting, WorkspaceAgentStatusConnected,
+		WorkspaceAgentStatusDisconnected, WorkspaceAgentStatusTimeout:
+		return true
+	default:
+		return false
+	}
+}
+
 type AuditableGroup struct {
 	Group
 	Members []GroupMember `json:"members"`
@@ -199,6 +220,61 @@ func (l License) RBACObject() rbac.Object {
 	return rbac.ResourceLicense.WithIDString(strconv.FormatInt(int64(l.ID), 10))
 }
 
+type WorkspaceAgentConnectionStatus struct {
+	Status           WorkspaceAgentStatus `json:"status"`
+	FirstConnectedAt *time.Time           `json:"first_connected_at"`
+	LastConnectedAt  *time.Time           `json:"last_connected_at"`
+	DisconnectedAt   *time.Time           `json:"disconnected_at"`
+}
+
+func (a WorkspaceAgent) Status(inactiveTimeout time.Duration) WorkspaceAgentConnectionStatus {
+	connectionTimeout := time.Duration(a.ConnectionTimeoutSeconds) * time.Second
+
+	status := WorkspaceAgentConnectionStatus{
+		Status: WorkspaceAgentStatusDisconnected,
+	}
+	if a.FirstConnectedAt.Valid {
+		status.FirstConnectedAt = &a.FirstConnectedAt.Time
+	}
+	if a.LastConnectedAt.Valid {
+		status.LastConnectedAt = &a.LastConnectedAt.Time
+	}
+	if a.DisconnectedAt.Valid {
+		status.DisconnectedAt = &a.DisconnectedAt.Time
+	}
+
+	switch {
+	case !a.FirstConnectedAt.Valid:
+		switch {
+		case connectionTimeout > 0 && Now().Sub(a.CreatedAt) > connectionTimeout:
+			// If the agent took too long to connect the first time,
+			// mark it as timed out.
+			status.Status = WorkspaceAgentStatusTimeout
+		default:
+			// If the agent never connected, it's waiting for the compute
+			// to start up.
+			status.Status = WorkspaceAgentStatusConnecting
+		}
+	// We check before instead of after because last connected at and
+	// disconnected at can be equal timestamps in tight-timed tests.
+	case !a.DisconnectedAt.Time.Before(a.LastConnectedAt.Time):
+		// If we've disconnected after our last connection, we know the
+		// agent is no longer connected.
+		status.Status = WorkspaceAgentStatusDisconnected
+	case Now().Sub(a.LastConnectedAt.Time) > inactiveTimeout:
+		// The connection died without updating the last connected.
+		status.Status = WorkspaceAgentStatusDisconnected
+		// Client code needs an accurate disconnected at if the agent has been inactive.
+		status.DisconnectedAt = &a.LastConnectedAt.Time
+	case a.LastConnectedAt.Valid:
+		// The agent should be assumed connected if it's under inactivity timeouts
+		// and last connected at has been properly set.
+		status.Status = WorkspaceAgentStatusConnected
+	}
+
+	return status
+}
+
 func ConvertUserRows(rows []GetUsersRow) []User {
 	users := make([]User, len(rows))
 	for i, r := range rows {

diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go
@@ -559,28 +559,6 @@ func (api *API) workspaceAgentPTY(rw http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	// TODO(@deansheather): bring back agent state check in the upstream ticket
-	// code
-	/*
-		apiAgent, err := convertWorkspaceAgent(
-			api.DERPMap, *api.TailnetCoordinator.Load(), workspaceAgent, nil, api.AgentInactiveDisconnectTimeout,
-			api.DeploymentValues.AgentFallbackTroubleshootingURL.String(),
-		)
-		if err != nil {
-			httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
-				Message: "Internal error reading workspace agent.",
-				Detail:  err.Error(),
-			})
-			return
-		}
-		if apiAgent.Status != codersdk.WorkspaceAgentConnected {
-			httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
-				Message: fmt.Sprintf("Agent state is %q, it must be in the %q state.", apiAgent.Status, codersdk.WorkspaceAgentConnected),
-			})
-			return
-		}
-	*/
-
 	reconnect, err := uuid.Parse(r.URL.Query().Get("reconnect"))
 	if err != nil {
 		httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
@@ -1218,45 +1196,11 @@ func convertWorkspaceAgent(derpMap *tailcfg.DERPMap, coordinator tailnet.Coordin
 		}
 	}
 
-	if dbAgent.FirstConnectedAt.Valid {
-		workspaceAgent.FirstConnectedAt = &dbAgent.FirstConnectedAt.Time
-	}
-	if dbAgent.LastConnectedAt.Valid {
-		workspaceAgent.LastConnectedAt = &dbAgent.LastConnectedAt.Time
-	}
-	if dbAgent.DisconnectedAt.Valid {
-		workspaceAgent.DisconnectedAt = &dbAgent.DisconnectedAt.Time
-	}
-
-	connectionTimeout := time.Duration(dbAgent.ConnectionTimeoutSeconds) * time.Second
-	switch {
-	case !dbAgent.FirstConnectedAt.Valid:
-		switch {
-		case connectionTimeout > 0 && database.Now().Sub(dbAgent.CreatedAt) > connectionTimeout:
-			// If the agent took too long to connect the first time,
-			// mark it as timed out.
-			workspaceAgent.Status = codersdk.WorkspaceAgentTimeout
-		default:
-			// If the agent never connected, it's waiting for the compute
-			// to start up.
-			workspaceAgent.Status = codersdk.WorkspaceAgentConnecting
-		}
-	// We check before instead of after because last connected at and
-	// disconnected at can be equal timestamps in tight-timed tests.
-	case !dbAgent.DisconnectedAt.Time.Before(dbAgent.LastConnectedAt.Time):
-		// If we've disconnected after our last connection, we know the
-		// agent is no longer connected.
-		workspaceAgent.Status = codersdk.WorkspaceAgentDisconnected
-	case database.Now().Sub(dbAgent.LastConnectedAt.Time) > agentInactiveDisconnectTimeout:
-		// The connection died without updating the last connected.
-		workspaceAgent.Status = codersdk.WorkspaceAgentDisconnected
-		// Client code needs an accurate disconnected at if the agent has been inactive.
-		workspaceAgent.DisconnectedAt = &dbAgent.LastConnectedAt.Time
-	case dbAgent.LastConnectedAt.Valid:
-		// The agent should be assumed connected if it's under inactivity timeouts
-		// and last connected at has been properly set.
-		workspaceAgent.Status = codersdk.WorkspaceAgentConnected
-	}
+	status := dbAgent.Status(agentInactiveDisconnectTimeout)
+	workspaceAgent.Status = codersdk.WorkspaceAgentStatus(status.Status)
+	workspaceAgent.FirstConnectedAt = status.FirstConnectedAt
+	workspaceAgent.LastConnectedAt = status.LastConnectedAt
+	workspaceAgent.DisconnectedAt = status.DisconnectedAt
 
 	return workspaceAgent, nil
 }

diff --git a/coderd/workspaceapps/auth.go b/coderd/workspaceapps/auth.go
@@ -3,6 +3,7 @@ package workspaceapps
 import (
 	"context"
 	"database/sql"
+	"fmt"
 	"net/http"
 	"strings"
 	"time"
@@ -158,6 +159,19 @@ func (p *Provider) ResolveRequest(rw http.ResponseWriter, r *http.Request, appRe
 		return nil, false
 	}
 
+	// Check that the agent is online.
+	agentStatus := dbReq.Agent.Status(p.WorkspaceAgentInactiveTimeout)
+	if agentStatus.Status != database.WorkspaceAgentStatusConnected {
+		p.writeWorkspaceAppOffline(rw, r, &appReq, fmt.Sprintf("Agent state is %q, not %q", agentStatus.Status, database.WorkspaceAgentStatusConnected))
+		return nil, false
+	}
+
+	// Check that the app is healthy.
+	if dbReq.AppHealth != "" && dbReq.AppHealth != database.WorkspaceAppHealthDisabled && dbReq.AppHealth != database.WorkspaceAppHealthHealthy {
+		p.writeWorkspaceAppOffline(rw, r, &appReq, fmt.Sprintf("App health is %q, not %q", dbReq.AppHealth, database.WorkspaceAppHealthHealthy))
+		return nil, false
+	}
+
 	// As a sanity check, ensure the ticket we just made is valid for this
 	// request.
 	if !ticket.MatchesRequest(appReq) {
@@ -347,3 +361,27 @@ func (p *Provider) writeWorkspaceApp500(rw http.ResponseWriter, r *http.Request,
 		DashboardURL: p.AccessURL.String(),
 	})
 }
+
+// writeWorkspaceAppOffline writes a HTML 502 error page for a workspace app. If
+// appReq is not nil, it will be used to log the request details at debug level.
+func (p *Provider) writeWorkspaceAppOffline(rw http.ResponseWriter, r *http.Request, appReq *Request, msg string) {
+	if appReq != nil {
+		slog.Helper()
+		p.Logger.Debug(r.Context(),
+			"workspace app offline: "+msg,
+			slog.F("username_or_id", appReq.UsernameOrID),
+			slog.F("workspace_and_agent", appReq.WorkspaceAndAgent),
+			slog.F("workspace_name_or_id", appReq.WorkspaceNameOrID),
+			slog.F("agent_name_or_id", appReq.AgentNameOrID),
+			slog.F("app_slug_or_port", appReq.AppSlugOrPort),
+		)
+	}
+
+	site.RenderStaticErrorPage(rw, r, site.ErrorPageData{
+		Status:       http.StatusBadGateway,
+		Title:        "Application Offline",
+		Description:  msg,
+		RetryEnabled: true,
+		DashboardURL: p.AccessURL.String(),
+	})
+}
diff --git a/coderd/workspaceapps/provider.go b/coderd/workspaceapps/provider.go
@@ -2,6 +2,7 @@ package workspaceapps
 
 import (
 	"net/url"
+	"time"
 
 	"cdr.dev/slog"
 	"github.com/coder/coder/coderd/database"
@@ -16,26 +17,32 @@ import (
 type Provider struct {
 	Logger slog.Logger
 
-	AccessURL        *url.URL
-	Authorizer       rbac.Authorizer
-	Database         database.Store
-	DeploymentValues *codersdk.DeploymentValues
-	OAuth2Configs    *httpmw.OAuth2Configs
-	TicketSigningKey []byte
+	AccessURL                     *url.URL
+	Authorizer                    rbac.Authorizer
+	Database                      database.Store
+	DeploymentValues              *codersdk.DeploymentValues
+	OAuth2Configs                 *httpmw.OAuth2Configs
+	WorkspaceAgentInactiveTimeout time.Duration
+	TicketSigningKey              []byte
 }
 
-func New(log slog.Logger, accessURL *url.URL, authz rbac.Authorizer, db database.Store, cfg *codersdk.DeploymentValues, oauth2Cfgs *httpmw.OAuth2Configs, ticketSigningKey []byte) *Provider {
+func New(log slog.Logger, accessURL *url.URL, authz rbac.Authorizer, db database.Store, cfg *codersdk.DeploymentValues, oauth2Cfgs *httpmw.OAuth2Configs, workspaceAgentInactiveTimeout time.Duration, ticketSigningKey []byte) *Provider {
 	if len(ticketSigningKey) != 64 {
 		panic("ticket signing key must be 64 bytes")
 	}
 
+	if workspaceAgentInactiveTimeout == 0 {
+		workspaceAgentInactiveTimeout = 1 * time.Minute
+	}
+
 	return &Provider{
-		Logger:           log,
-		AccessURL:        accessURL,
-		Authorizer:       authz,
-		Database:         db,
-		DeploymentValues: cfg,
-		OAuth2Configs:    oauth2Cfgs,
-		TicketSigningKey: ticketSigningKey,
+		Logger:                        log,
+		AccessURL:                     accessURL,
+		Authorizer:                    authz,
+		Database:                      db,
+		DeploymentValues:              cfg,
+		OAuth2Configs:                 oauth2Cfgs,
+		WorkspaceAgentInactiveTimeout: workspaceAgentInactiveTimeout,
+		TicketSigningKey:              ticketSigningKey,
 	}
 }