Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

fix: allow access to unhealthy/initializing apps#12086

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
deansheather merged 3 commits intomainfromdean/unhealthy-app-access
Feb 13, 2024
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletionscoderd/workspaceapps/db.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Suggested change
// in case there are bugs with the healthcheckcodethat lock users out of
// in case there are bugs with the healthcheck that lock users out of

could be a lot of things: misconfiguration, app healthcheck endpoint could be misbehaving

// their apps completely.

// As a sanity check, ensure the token we just made is valid for this
// request.
Expand Down
91 changes: 76 additions & 15 deletionscoderd/workspaceapps/db_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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"

Expand All@@ -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
Expand DownExpand Up@@ -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)
Expand DownExpand Up@@ -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,
},
},
},
},
{
Expand DownExpand Up@@ -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 {
Expand DownExpand Up@@ -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)
})
}
7 changes: 0 additions & 7 deletionscoderd/workspaceapps/request.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand DownExpand Up@@ -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 {
Expand DownExpand Up@@ -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
}
}
Expand DownExpand Up@@ -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
}
Expand DownExpand Up@@ -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
}
7 changes: 5 additions & 2 deletionssite/src/modules/resources/AppLink/AppLink.tsx
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -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
Expand All@@ -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";
}
Expand Down
[8]ページ先頭
©2009-2025 Movatter.jp