From 633968bc25389aacd04230ce9498a470cfbb2f31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20R=C3=BCger?= Date: Wed, 2 Sep 2026 14:41:37 +0200 Subject: [PATCH] web: reload the rate limiter when its configuration changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The documentation says the configuration file "is read upon every http request", and users, headers and certificates behave that way. The rate limiter did not: it was built once when the server started and captured in the handler, so editing rate_limit had no effect until a restart, with nothing in the logs to say so. Removing the section entirely did not lift the limit either. Build the limiter from the configuration the handler has just read, keeping the existing one while the values are unchanged. Rebuilding on every request would hand out a fresh burst each time and defeat the limit entirely, so the limiter is replaced only when interval or burst actually change, and the change is logged. Replacing the limiter resets its token bucket, so a change to rate_limit starts the interval afresh. That is now stated in the documentation. The limiter is still built once at startup so that an enabled rate limiter is reported in the logs there as it was before. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Manuel Rüger --- docs/web-configuration.md | 4 +- web/handler.go | 31 ++++++++++- web/rate_limit_test.go | 114 ++++++++++++++++++++++++++++++++++++++ web/tls_config.go | 14 ++--- 4 files changed, 152 insertions(+), 11 deletions(-) create mode 100644 web/rate_limit_test.go 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 {