diff --git a/coderd/workspaceapps/db.go b/coderd/workspaceapps/db.go index b17c4a4a05c69..c514b1d3f9ed5 100644 --- a/coderd/workspaceapps/db.go +++ b/coderd/workspaceapps/db.go @@ -198,11 +198,9 @@ func (p *DBTokenProvider) Issue(ctx context.Context, rw http.ResponseWriter, r * return nil, "", false } - // Check that the app is healthy. - if dbReq.AppHealth != "" && dbReq.AppHealth != database.WorkspaceAppHealthDisabled && dbReq.AppHealth != database.WorkspaceAppHealthHealthy { - WriteWorkspaceAppOffline(p.Logger, p.DashboardURL, rw, r, &appReq, fmt.Sprintf("App health is %q, not %q", dbReq.AppHealth, database.WorkspaceAppHealthHealthy)) - return nil, "", false - } + // This is where we used to check app health, but we don't do that anymore + // in case there are bugs with the healthcheck code that lock users out of + // their apps completely. // As a sanity check, ensure the token we just made is valid for this // request. diff --git a/coderd/workspaceapps/db_test.go b/coderd/workspaceapps/db_test.go index b2b9f4e50e356..eccc96d0080b4 100644 --- a/coderd/workspaceapps/db_test.go +++ b/coderd/workspaceapps/db_test.go @@ -37,9 +37,12 @@ func Test_ResolveRequest(t *testing.T) { appNameAuthed = "app-authed" appNamePublic = "app-public" appNameInvalidURL = "app-invalid-url" - appNameUnhealthy = "app-unhealthy" + // Users can access unhealthy and initializing apps (as of 2024-02). + appNameUnhealthy = "app-unhealthy" + appNameInitializing = "app-initializing" // This agent will never connect, so it will never become "connected". + // Users cannot access unhealthy agents. agentNameUnhealthy = "agent-unhealthy" appNameAgentUnhealthy = "app-agent-unhealthy" @@ -55,6 +58,15 @@ func Test_ResolveRequest(t *testing.T) { w.WriteHeader(http.StatusInternalServerError) _, _ = w.Write([]byte("unhealthy")) })) + t.Cleanup(unhealthySrv.Close) + + // Start a listener for a server that never responds. + initializingServer, err := net.Listen("tcp", "localhost:0") + require.NoError(t, err) + t.Cleanup(func() { + _ = initializingServer.Close() + }) + initializingURL := fmt.Sprintf("http://%s", initializingServer.Addr().String()) deploymentValues := coderdtest.DeploymentValues(t) deploymentValues.DisablePathApps = false @@ -82,7 +94,7 @@ func Test_ResolveRequest(t *testing.T) { }) ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitMedium) - defer cancel() + t.Cleanup(cancel) firstUser := coderdtest.CreateFirstUser(t, client) me, err := client.User(ctx, codersdk.Me) @@ -143,6 +155,17 @@ func Test_ResolveRequest(t *testing.T) { Threshold: 1, }, }, + { + Slug: appNameInitializing, + DisplayName: appNameInitializing, + SharingLevel: proto.AppSharingLevel_PUBLIC, + Url: appURL, + Healthcheck: &proto.Healthcheck{ + Url: initializingURL, + Interval: 30, + Threshold: 1000, + }, + }, }, }, { @@ -805,7 +828,55 @@ func Test_ResolveRequest(t *testing.T) { require.Contains(t, bodyStr, `Agent state is "`) }) - t.Run("UnhealthyApp", func(t *testing.T) { + // Initializing apps are now permitted to connect anyways. This wasn't + // always the case, but we're testing the behavior to ensure it doesn't + // change back accidentally. + t.Run("InitializingAppPermitted", func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + agent, err := client.WorkspaceAgent(ctx, agentID) + require.NoError(t, err) + + for _, app := range agent.Apps { + if app.Slug == appNameInitializing { + t.Log("app is", app.Health) + require.Equal(t, codersdk.WorkspaceAppHealthInitializing, app.Health) + break + } + } + + req := (workspaceapps.Request{ + AccessMethod: workspaceapps.AccessMethodPath, + BasePath: "/app", + UsernameOrID: me.Username, + WorkspaceNameOrID: workspace.Name, + AgentNameOrID: agentName, + AppSlugOrPort: appNameInitializing, + }).Normalize() + + rw := httptest.NewRecorder() + r := httptest.NewRequest("GET", "/app", nil) + r.Header.Set(codersdk.SessionTokenHeader, client.SessionToken()) + + token, ok := workspaceapps.ResolveRequest(rw, r, workspaceapps.ResolveRequestOptions{ + Logger: api.Logger, + SignedTokenProvider: api.WorkspaceAppsProvider, + DashboardURL: api.AccessURL, + PathAppBaseURL: api.AccessURL, + AppHostname: api.AppHostname, + AppRequest: req, + }) + require.True(t, ok, "ResolveRequest failed, should pass even though app is initializing") + require.NotNil(t, token) + }) + + // Unhealthy apps are now permitted to connect anyways. This wasn't always + // the case, but we're testing the behavior to ensure it doesn't change back + // accidentally. + t.Run("UnhealthyAppPermitted", func(t *testing.T) { t.Parallel() require.Eventually(t, func() bool { @@ -850,17 +921,7 @@ func Test_ResolveRequest(t *testing.T) { AppHostname: api.AppHostname, AppRequest: req, }) - require.False(t, ok, "request succeeded even though app is unhealthy") - require.Nil(t, token) - - w := rw.Result() - defer w.Body.Close() - require.Equal(t, http.StatusBadGateway, w.StatusCode) - - body, err := io.ReadAll(w.Body) - require.NoError(t, err) - bodyStr := string(body) - bodyStr = strings.ReplaceAll(bodyStr, """, `"`) - require.Contains(t, bodyStr, `App health is "unhealthy"`) + require.True(t, ok, "ResolveRequest failed, should pass even though app is unhealthy") + require.NotNil(t, token) }) } diff --git a/coderd/workspaceapps/request.go b/coderd/workspaceapps/request.go index 427ce343fddc2..a6eb12c145f9c 100644 --- a/coderd/workspaceapps/request.go +++ b/coderd/workspaceapps/request.go @@ -198,9 +198,6 @@ type databaseRequest struct { // AppURL is the resolved URL to the workspace app. This is only set for non // terminal requests. AppURL *url.URL - // AppHealth is the health of the app. For terminal requests, this is always - // database.WorkspaceAppHealthHealthy. - AppHealth database.WorkspaceAppHealth // AppSharingLevel is the sharing level of the app. This is forced to be set // to AppSharingLevelOwner if the access method is terminal. AppSharingLevel database.AppSharingLevel @@ -292,7 +289,6 @@ func (r Request) getDatabase(ctx context.Context, db database.Store) (*databaseR agentNameOrID = r.AgentNameOrID appURL string appSharingLevel database.AppSharingLevel - appHealth = database.WorkspaceAppHealthDisabled portUint, portUintErr = strconv.ParseUint(r.AppSlugOrPort, 10, 16) ) if portUintErr == nil { @@ -331,7 +327,6 @@ func (r Request) getDatabase(ctx context.Context, db database.Store) (*databaseR appSharingLevel = database.AppSharingLevelOwner } appURL = app.Url.String - appHealth = app.Health break } } @@ -377,7 +372,6 @@ func (r Request) getDatabase(ctx context.Context, db database.Store) (*databaseR Workspace: workspace, Agent: agent, AppURL: appURLParsed, - AppHealth: appHealth, AppSharingLevel: appSharingLevel, }, nil } @@ -430,7 +424,6 @@ func (r Request) getDatabaseTerminal(ctx context.Context, db database.Store) (*d Workspace: workspace, Agent: agent, AppURL: nil, - AppHealth: database.WorkspaceAppHealthHealthy, AppSharingLevel: database.AppSharingLevelOwner, }, nil } diff --git a/site/src/modules/resources/AppLink/AppLink.tsx b/site/src/modules/resources/AppLink/AppLink.tsx index 4971946f683cf..86cf0da77696b 100644 --- a/site/src/modules/resources/AppLink/AppLink.tsx +++ b/site/src/modules/resources/AppLink/AppLink.tsx @@ -61,12 +61,16 @@ export const AppLink: FC = ({ app, workspace, agent }) => { app, ); + // canClick is ONLY false when it's a subdomain app and the admin hasn't + // enabled wildcard access URL or the session token is being fetched. + // + // To avoid bugs in the healthcheck code locking users out of apps, we no + // longer block access to apps if they are unhealthy/initializing. let canClick = true; let icon = ; let primaryTooltip = ""; if (app.health === "initializing") { - canClick = false; icon = ( // This is a hack to make the spinner appear in the center of the start // icon space @@ -85,7 +89,6 @@ export const AppLink: FC = ({ app, workspace, agent }) => { primaryTooltip = "Initializing..."; } if (app.health === "unhealthy") { - canClick = false; icon = ; primaryTooltip = "Unhealthy"; }