diff --git a/docs/web-configuration.md b/docs/web-configuration.md index c3b1cf12..21f90cc0 100644 --- a/docs/web-configuration.md +++ b/docs/web-configuration.md @@ -128,7 +128,9 @@ basic_auth_users: [ : ... ] -# Rate limiting requests on the endpoint using a token bucket +# Rate limiting requests on the endpoint using a token bucket. +# Changing these values takes effect without a restart, and starts the +# interval afresh. rate_limit: interval: # time interval between two requests, set to 0 to disable rate limiter burst: # and permits a burst of requests. diff --git a/web/handler.go b/web/handler.go index 0a2718d5..a04e1a87 100644 --- a/web/handler.go +++ b/web/handler.go @@ -81,12 +81,41 @@ type webHandler struct { handler http.Handler logger *slog.Logger cache *cache + // limiterMtx guards limiter and limiterConfig, which are replaced when the + // rate limiter configuration changes on disk. + limiterMtx sync.Mutex limiter *rate.Limiter + limiterConfig RateLimiterConfig // bcryptMtx is there to ensure that bcrypt.CompareHashAndPassword is run // only once in parallel as this is CPU intensive. bcryptMtx sync.Mutex } +// rateLimiter returns the limiter to apply for the given configuration, +// building a new one when the configuration has changed since the last +// request. It returns nil when rate limiting is not configured. +// +// Replacing the limiter resets its token bucket, so a change to rate_limit +// starts the interval afresh. +func (u *webHandler) rateLimiter(c RateLimiterConfig) *rate.Limiter { + u.limiterMtx.Lock() + defer u.limiterMtx.Unlock() + + if c == u.limiterConfig { + return u.limiter + } + u.limiterConfig = c + + if c.Interval == 0 { + u.limiter = nil + u.logger.Info("Rate Limiter is disabled.") + return nil + } + u.limiter = rate.NewLimiter(rate.Every(c.Interval), c.Burst) + u.logger.Info("Rate Limiter is enabled.", "burst", c.Burst, "interval", c.Interval) + return u.limiter +} + func (u *webHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { c, err := getConfig(u.tlsConfigPath) if err != nil { @@ -95,7 +124,7 @@ func (u *webHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } - if u.limiter != nil && !u.limiter.Allow() { + if limiter := u.rateLimiter(c.RateLimiterConfig); limiter != nil && !limiter.Allow() { http.Error(w, http.StatusText(http.StatusTooManyRequests), http.StatusTooManyRequests) return } diff --git a/web/rate_limit_test.go b/web/rate_limit_test.go new file mode 100644 index 00000000..e4b4d8c5 --- /dev/null +++ b/web/rate_limit_test.go @@ -0,0 +1,114 @@ +// Copyright The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package web + +import ( + "context" + "net/http" + "os" + "path/filepath" + "testing" +) + +const ( + // rateLimitedConfig allows a single request and then refuses everything + // for an hour, which makes the limiter's state easy to observe. + rateLimitedConfig = "rate_limit:\n interval: 1h\n burst: 1\n" + unlimitedConfig = "http_server_config:\n http2: true\n" +) + +// TestRateLimitIsReloaded checks that a change to rate_limit takes effect +// without a restart. The configuration file is documented as being read on +// every request, and every other key behaves that way. +func TestRateLimitIsReloaded(t *testing.T) { + configPath := filepath.Join(t.TempDir(), "web-config.yml") + writeConfig := func(content string) { + t.Helper() + if err := os.WriteFile(configPath, []byte(content), 0o600); err != nil { + t.Fatalf("Unable to write config: %v", err) + } + } + writeConfig(rateLimitedConfig) + + server := &http.Server{ + Handler: http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Write([]byte("Hello World!")) + }), + } + + done := make(chan struct{}) + t.Cleanup(func() { + if err := server.Shutdown(context.Background()); err != nil { + t.Fatal(err) + } + <-done + }) + + go func() { + flags := FlagConfig{ + WebListenAddresses: &([]string{port}), + WebSystemdSocket: OfBool(false), + WebConfigFile: &configPath, + } + ListenAndServe(server, &flags, testlogger) + close(done) + }() + + waitForPort(t, port) + + requireStatus := func(what string, expected int) { + t.Helper() + r, err := http.Get("http://localhost" + port) + if err != nil { + t.Fatal(err) + } + r.Body.Close() + if r.StatusCode != expected { + t.Fatalf("%s: got status %d, expected %d", what, r.StatusCode, expected) + } + } + + // The burst is spent by the first request. + requireStatus("first request", http.StatusOK) + requireStatus("second request", http.StatusTooManyRequests) + + // Removing rate_limit lifts the limit. + writeConfig(unlimitedConfig) + requireStatus("after removing rate_limit", http.StatusOK) + requireStatus("after removing rate_limit, again", http.StatusOK) + + // Putting it back applies it again, with a fresh burst. + writeConfig(rateLimitedConfig) + requireStatus("after restoring rate_limit", http.StatusOK) + requireStatus("after restoring rate_limit, again", http.StatusTooManyRequests) +} + +// TestRateLimiterUnchangedConfigKeepsItsBucket checks that an unchanged +// configuration does not rebuild the limiter, which would hand out a fresh +// burst on every request and defeat the limit. +func TestRateLimiterUnchangedConfigKeepsItsBucket(t *testing.T) { + h := &webHandler{logger: testlogger} + c := RateLimiterConfig{Interval: 1, Burst: 1} + + first := h.rateLimiter(c) + if first == nil { + t.Fatal("rateLimiter() = nil, expected a limiter") + } + if second := h.rateLimiter(c); second != first { + t.Error("rateLimiter() built a new limiter for an unchanged configuration") + } + if disabled := h.rateLimiter(RateLimiterConfig{}); disabled != nil { + t.Error("rateLimiter() = non-nil for a configuration without an interval") + } +} diff --git a/web/tls_config.go b/web/tls_config.go index b40be6bb..10556166 100644 --- a/web/tls_config.go +++ b/web/tls_config.go @@ -34,7 +34,6 @@ import ( config_util "github.com/prometheus/common/config" "go.yaml.in/yaml/v2" "golang.org/x/sync/errgroup" - "golang.org/x/time/rate" ) var ( @@ -400,19 +399,16 @@ func Serve(l net.Listener, server *http.Server, flags *FlagConfig, logger *slog. return err } - var limiter *rate.Limiter - if c.RateLimiterConfig.Interval != 0 { - limiter = rate.NewLimiter(rate.Every(c.RateLimiterConfig.Interval), c.RateLimiterConfig.Burst) - logger.Info("Rate Limiter is enabled.", "burst", c.RateLimiterConfig.Burst, "interval", c.RateLimiterConfig.Interval) - } - - server.Handler = &webHandler{ + webHandler := &webHandler{ tlsConfigPath: tlsConfigPath, logger: logger, handler: handler, cache: newCache(), - limiter: limiter, } + // Build the limiter up front so that an enabled rate limiter is reported at + // startup as before. The handler rebuilds it whenever the file changes. + webHandler.rateLimiter(c.RateLimiterConfig) + server.Handler = webHandler config, err := ConfigToTLSConfig(&c.TLSConfig) switch err {