mirror of https://github.com/coder/coder.git
fix: allow access to unhealthy/initializing apps (#12086)
This commit is contained in:
parent
ec25fb8bbc
commit
fead57f304
|
@ -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.
|
||||
|
|
|
@ -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)
|
||||
})
|
||||
}
|
||||
|
|
|
@ -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
|
||||
}
|
||||
|
|
|
@ -61,12 +61,16 @@ export const AppLink: FC<AppLinkProps> = ({ 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 = <BaseIcon app={app} />;
|
||||
|
||||
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<AppLinkProps> = ({ app, workspace, agent }) => {
|
|||
primaryTooltip = "Initializing...";
|
||||
}
|
||||
if (app.health === "unhealthy") {
|
||||
canClick = false;
|
||||
icon = <ErrorOutlineIcon css={{ color: theme.palette.warning.light }} />;
|
||||
primaryTooltip = "Unhealthy";
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue