Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

web: add feature flag PropagateCancels #7778

Merged
merged 2 commits into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
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, note 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)
}
}
Loading