Skip to content

Commit

Permalink
web: add feature flag PropagateCancels
Browse files Browse the repository at this point in the history
This allow client-initiated cancels to propagate through gRPC.
  • Loading branch information
jsha committed Oct 31, 2024
1 parent 3377102 commit 7962ea5
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 5 deletions.
9 changes: 9 additions & 0 deletions features/features.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,15 @@ type Config struct {
// This flag should only be used in conjunction with UseKvLimitsForNewOrder.
DisableLegacyLimitWrites bool

// PropagateCancels controls whether the WFE and ocsp-responder allows cancellation
// of an inbound request to cancel downstream gRPC and other queries. In practice,
// cancellation of an inbound request is achieved by Nginx closing the connection
// on which the request was happening. This may help shed load in overcapacity
// situations. However, not that in-progress database queries (for instance, in the
// SA) are not cancelled. Database queries waiting for an available connection may
// be cancelled.
PropagateCancels bool

// InsertAuthzsIndividually causes the SA's NewOrderAndAuthzs method to
// create each new authz one at a time, rather than using MultiInserter.
// Although this is expected to be a performance penalty, it is necessary to
Expand Down
1 change: 1 addition & 0 deletions test/config-next/wfe2.json
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@
"Overrides": "test/config-next/wfe2-ratelimit-overrides.yml"
},
"features": {
"PropagateCancels": true,
"ServeRenewalInfo": true,
"CheckIdentifiersPaused": true,
"UseKvLimitsForNewOrder": true
Expand Down
13 changes: 8 additions & 5 deletions web/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"strings"
"time"

"github.com/letsencrypt/boulder/features"
blog "github.com/letsencrypt/boulder/log"
)

Expand Down Expand Up @@ -127,11 +128,13 @@ func (th *TopHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
Origin: r.Header.Get("Origin"),
Extra: make(map[string]interface{}),
}
// We specifically override the default r.Context() because we would prefer
// for clients to not be able to cancel our operations in arbitrary places.
// Instead we start a new context, and apply timeouts in our various RPCs.
ctx := context.WithoutCancel(r.Context())
r = r.WithContext(ctx)
if !features.Get().PropagateCancels {
// We specifically override the default r.Context() because we would prefer
// for clients to not be able to cancel our operations in arbitrary places.
// Instead we start a new context, and apply timeouts in our various RPCs.
ctx := context.WithoutCancel(r.Context())
r = r.WithContext(ctx)
}

// Some clients will send a HTTP Host header that includes the default port
// for the scheme that they are using. Previously when we were fronted by
Expand Down
36 changes: 36 additions & 0 deletions web/context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,16 @@ package web

import (
"bytes"
"context"
"crypto/tls"
"fmt"
"net/http"
"net/http/httptest"
"strings"
"testing"
"time"

"github.com/letsencrypt/boulder/features"
blog "github.com/letsencrypt/boulder/log"
"github.com/letsencrypt/boulder/test"
)
Expand Down Expand Up @@ -117,3 +120,36 @@ func TestHostHeaderRewrite(t *testing.T) {
req.Host = "localhost:123"
th.ServeHTTP(httptest.NewRecorder(), req)
}

type cancelHandler struct {
res chan string
}

func (ch cancelHandler) ServeHTTP(e *RequestEvent, w http.ResponseWriter, r *http.Request) {
select {
case <-r.Context().Done():
ch.res <- r.Context().Err().Error()
case <-time.After(300 * time.Millisecond):
ch.res <- "300 ms passed"
}
}

func TestPropagateCancel(t *testing.T) {
mockLog := blog.UseMock()
res := make(chan string)
features.Set(features.Config{PropagateCancels: true})
th := NewTopHandler(mockLog, cancelHandler{res})
ctx, cancel := context.WithCancel(context.Background())
go func() {
req, err := http.NewRequestWithContext(ctx, "GET", "/thisisignored", &bytes.Reader{})
if err != nil {
t.Error(err)
}
th.ServeHTTP(httptest.NewRecorder(), req)
}()
cancel()
result := <-res
if result != "context canceled" {
t.Errorf("expected 'context canceled', got %q", result)
}
}

0 comments on commit 7962ea5

Please sign in to comment.