mirror of
https://github.com/coder/coder.git
synced 2026-09-22 05:05:20 +08:00
chore(codersdk/workspacesdk): make dialer fail fast for authnz errors (#19173)
Closes #18599. The linked issue was created due to me assuming the dialer didn't fail fast at all. In reality, it does fail fast, but only for a select few status codes. Auth[n|z] errors aren't any of those status codes, despite being 'permanent' in the same way a `400` is. This PR makes 401* and 403 'permanent' errors, meaning the dialer will give up immediately after receiving them from coderd. *One reason to receive a 401 is when the supplied resume_token is invalid. These are not permanent errors, and when we encounter those the dialer will retain the existing behaviour of unsetting the resume token and retrying.
This commit is contained in:
@@ -24,6 +24,10 @@ var permanentErrorStatuses = []int{
|
||||
http.StatusBadRequest, // returned if API mismatch
|
||||
http.StatusNotFound, // returned if user doesn't have permission or agent doesn't exist
|
||||
http.StatusInternalServerError, // returned if database is not reachable,
|
||||
http.StatusForbidden, // returned if user is not authorized
|
||||
// StatusUnauthorized is only a permanent error if the error is not due to
|
||||
// an invalid resume token. See `checkResumeTokenFailure`.
|
||||
http.StatusUnauthorized,
|
||||
}
|
||||
|
||||
type WebsocketDialer struct {
|
||||
@@ -39,6 +43,24 @@ type WebsocketDialer struct {
|
||||
isFirst bool
|
||||
}
|
||||
|
||||
// checkResumeTokenFailure checks if the parsed error indicates a resume token failure
|
||||
// and updates the resumeTokenFailed flag accordingly. Returns true if a resume token
|
||||
// failure was detected.
|
||||
func (w *WebsocketDialer) checkResumeTokenFailure(ctx context.Context, sdkErr *codersdk.Error) bool {
|
||||
if sdkErr == nil {
|
||||
return false
|
||||
}
|
||||
|
||||
for _, v := range sdkErr.Validations {
|
||||
if v.Field == "resume_token" {
|
||||
w.logger.Warn(ctx, "failed to dial tailnet v2+ API: server replied invalid resume token; unsetting for next connection attempt")
|
||||
w.resumeTokenFailed = true
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
type WebsocketDialerOption func(*WebsocketDialer)
|
||||
|
||||
func WithWorkspaceUpdates(req *proto.WorkspaceUpdatesRequest) WebsocketDialerOption {
|
||||
@@ -82,9 +104,14 @@ func (w *WebsocketDialer) Dial(ctx context.Context, r tailnet.ResumeTokenControl
|
||||
if w.isFirst {
|
||||
if res != nil && slices.Contains(permanentErrorStatuses, res.StatusCode) {
|
||||
err = codersdk.ReadBodyAsError(res)
|
||||
// A bit more human-readable help in the case the API version was rejected
|
||||
var sdkErr *codersdk.Error
|
||||
if xerrors.As(err, &sdkErr) {
|
||||
// Check for resume token failure first
|
||||
if w.checkResumeTokenFailure(ctx, sdkErr) {
|
||||
return tailnet.ControlProtocolClients{}, err
|
||||
}
|
||||
|
||||
// A bit more human-readable help in the case the API version was rejected
|
||||
if sdkErr.Message == AgentAPIMismatchMessage &&
|
||||
sdkErr.StatusCode() == http.StatusBadRequest {
|
||||
sdkErr.Helper = fmt.Sprintf(
|
||||
@@ -107,13 +134,8 @@ func (w *WebsocketDialer) Dial(ctx context.Context, r tailnet.ResumeTokenControl
|
||||
bodyErr := codersdk.ReadBodyAsError(res)
|
||||
var sdkErr *codersdk.Error
|
||||
if xerrors.As(bodyErr, &sdkErr) {
|
||||
for _, v := range sdkErr.Validations {
|
||||
if v.Field == "resume_token" {
|
||||
// Unset the resume token for the next attempt
|
||||
w.logger.Warn(ctx, "failed to dial tailnet v2+ API: server replied invalid resume token; unsetting for next connection attempt")
|
||||
w.resumeTokenFailed = true
|
||||
return tailnet.ControlProtocolClients{}, err
|
||||
}
|
||||
if w.checkResumeTokenFailure(ctx, sdkErr) {
|
||||
return tailnet.ControlProtocolClients{}, err
|
||||
}
|
||||
}
|
||||
if !errors.Is(err, context.Canceled) {
|
||||
|
||||
@@ -270,6 +270,46 @@ func TestWebsocketDialer_ResumeTokenFailure(t *testing.T) {
|
||||
require.Error(t, err)
|
||||
}
|
||||
|
||||
func TestWebsocketDialer_UnauthenticatedFailFast(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitShort)
|
||||
logger := slogtest.Make(t, &slogtest.Options{
|
||||
IgnoreErrors: true,
|
||||
}).Leveled(slog.LevelDebug)
|
||||
|
||||
svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
httpapi.Write(ctx, w, http.StatusUnauthorized, codersdk.Response{})
|
||||
}))
|
||||
defer svr.Close()
|
||||
svrURL, err := url.Parse(svr.URL)
|
||||
require.NoError(t, err)
|
||||
|
||||
uut := workspacesdk.NewWebsocketDialer(logger, svrURL, &websocket.DialOptions{})
|
||||
|
||||
_, err = uut.Dial(ctx, nil)
|
||||
require.Error(t, err)
|
||||
}
|
||||
|
||||
func TestWebsocketDialer_UnauthorizedFailFast(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitShort)
|
||||
logger := slogtest.Make(t, &slogtest.Options{
|
||||
IgnoreErrors: true,
|
||||
}).Leveled(slog.LevelDebug)
|
||||
|
||||
svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
httpapi.Write(ctx, w, http.StatusUnauthorized, codersdk.Response{})
|
||||
}))
|
||||
defer svr.Close()
|
||||
svrURL, err := url.Parse(svr.URL)
|
||||
require.NoError(t, err)
|
||||
|
||||
uut := workspacesdk.NewWebsocketDialer(logger, svrURL, &websocket.DialOptions{})
|
||||
|
||||
_, err = uut.Dial(ctx, nil)
|
||||
require.Error(t, err)
|
||||
}
|
||||
|
||||
func TestWebsocketDialer_UplevelVersion(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := testutil.Context(t, testutil.WaitShort)
|
||||
|
||||
Reference in New Issue
Block a user