diff --git a/docs/web-configuration.md b/docs/web-configuration.md index c3b1cf12..575c5db1 100644 --- a/docs/web-configuration.md +++ b/docs/web-configuration.md @@ -11,8 +11,10 @@ defined by the scheme described below. Brackets indicate that a parameter is optional. For non-list parameters the value is set to the specified default. -The file is read upon every http request, such as any change in the -configuration, so the certificates are picked up immediately. +The file is consulted upon every http request, so any change in the +configuration, such as the certificates, is picked up immediately. It is only +re-read and re-parsed when its modification time or size has changed since the +previous request. Generic placeholders are defined as follows: diff --git a/web/config_cache_test.go b/web/config_cache_test.go new file mode 100644 index 00000000..4bbe50f7 --- /dev/null +++ b/web/config_cache_test.go @@ -0,0 +1,137 @@ +// 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 ( + "os" + "path/filepath" + "testing" +) + +func writeConfigFile(t *testing.T, path, content string) { + t.Helper() + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + t.Fatalf("Unable to write config: %v", err) + } +} + +func getConfigOrFail(t *testing.T, path string) *Config { + t.Helper() + c, err := getConfig(path) + if err != nil { + t.Fatalf("getConfig: %v", err) + } + return c +} + +// TestGetConfigReloadsOnChange checks that an edited configuration file is +// picked up, which is what the per-request read exists for. +func TestGetConfigReloadsOnChange(t *testing.T) { + path := filepath.Join(t.TempDir(), "web-config.yml") + + writeConfigFile(t, path, "http_server_config:\n http2: true\n") + if c := getConfigOrFail(t, path); !c.HTTPConfig.HTTP2 { + t.Error("http2 = false, expected true") + } + + writeConfigFile(t, path, "http_server_config:\n http2: false\n headers:\n X-Frame-Options: deny\n") + c := getConfigOrFail(t, path) + if c.HTTPConfig.HTTP2 { + t.Error("http2 = true after the file changed, expected false") + } + if got := c.HTTPConfig.Header["X-Frame-Options"]; got != "deny" { + t.Errorf("X-Frame-Options = %q after the file changed, expected %q", got, "deny") + } +} + +// TestGetConfigReusesParsedConfig checks that an unchanged file is not parsed +// again. The cache keys on modification time and size, so the test rewrites the +// file with different content of the same length and restores the timestamps: +// the previously parsed configuration is expected to survive. This is also the +// documented limitation of the approach. +func TestGetConfigReusesParsedConfig(t *testing.T) { + path := filepath.Join(t.TempDir(), "web-config.yml") + + writeConfigFile(t, path, "http_server_config:\n http2: true\n") + if c := getConfigOrFail(t, path); !c.HTTPConfig.HTTP2 { + t.Fatal("http2 = false, expected true") + } + + info, err := os.Stat(path) + if err != nil { + t.Fatalf("Stat: %v", err) + } + // Same length as the content above. + writeConfigFile(t, path, "http_server_config:\n http2: nope\n") + if err := os.Chtimes(path, info.ModTime(), info.ModTime()); err != nil { + t.Fatalf("Chtimes: %v", err) + } + + c, err := getConfig(path) + if err != nil { + t.Fatalf("getConfig re-parsed a file it should have taken from the cache: %v", err) + } + if !c.HTTPConfig.HTTP2 { + t.Error("http2 = false, expected the cached configuration") + } +} + +// TestGetConfigReturnsACopy checks that a caller cannot change what the next +// caller sees. +func TestGetConfigReturnsACopy(t *testing.T) { + path := filepath.Join(t.TempDir(), "web-config.yml") + writeConfigFile(t, path, "http_server_config:\n http2: true\n") + + first := getConfigOrFail(t, path) + first.HTTPConfig.HTTP2 = false + + if second := getConfigOrFail(t, path); !second.HTTPConfig.HTTP2 { + t.Error("a change to one caller's configuration was visible to the next") + } +} + +// TestGetConfigDoesNotCacheFailures checks that a file which does not parse is +// reported every time, rather than once. +func TestGetConfigDoesNotCacheFailures(t *testing.T) { + path := filepath.Join(t.TempDir(), "web-config.yml") + writeConfigFile(t, path, "this is not a valid configuration\n") + + for i := range 2 { + if _, err := getConfig(path); err == nil { + t.Fatalf("getConfig call %d = nil, expected an error", i+1) + } + } +} + +// TestGetConfigMissingFile checks that a missing file is still an error. +func TestGetConfigMissingFile(t *testing.T) { + if _, err := getConfig(filepath.Join(t.TempDir(), "does-not-exist.yml")); err == nil { + t.Error("getConfig() = nil, expected an error") + } +} + +func BenchmarkGetConfig(b *testing.B) { + path := filepath.Join(b.TempDir(), "web-config.yml") + content := "http_server_config:\n headers:\n X-Frame-Options: deny\n" + if err := os.WriteFile(path, []byte(content), 0o600); err != nil { + b.Fatalf("Unable to write config: %v", err) + } + + b.ReportAllocs() + for b.Loop() { + if _, err := getConfig(path); err != nil { + b.Fatal(err) + } + } +} diff --git a/web/tls_config.go b/web/tls_config.go index b40be6bb..69d755c5 100644 --- a/web/tls_config.go +++ b/web/tls_config.go @@ -27,6 +27,7 @@ import ( "slices" "strconv" "strings" + "sync" "time" "github.com/coreos/go-systemd/v22/activation" @@ -147,7 +148,64 @@ type RateLimiterConfig struct { Interval time.Duration `yaml:"interval"` } +// configCache memoizes parsed configuration files. The file is consulted on +// every request so that changes are picked up without a restart, and parsing it +// each time is by far the most expensive part of serving one. +// +// An entry is reused only while the file's modification time and size are both +// unchanged, so an edit is picked up on the next request as before. A write +// that leaves both identical is not noticed; on filesystems whose modification +// times have sub-second resolution that needs a rewrite within the same +// nanosecond, and elsewhere within the same second, at exactly the same length. +type configCache struct { + mtx sync.Mutex + entries map[string]configCacheEntry +} + +type configCacheEntry struct { + modTime time.Time + size int64 + config *Config +} + +var parsedConfigs = &configCache{entries: make(map[string]configCacheEntry)} + +// get returns the cached configuration for path if it was parsed from a file +// with the same modification time and size. +func (c *configCache) get(path string, info os.FileInfo) *Config { + c.mtx.Lock() + defer c.mtx.Unlock() + + entry, ok := c.entries[path] + if !ok || entry.size != info.Size() || !entry.modTime.Equal(info.ModTime()) { + return nil + } + return entry.config +} + +func (c *configCache) set(path string, info os.FileInfo, config *Config) { + c.mtx.Lock() + defer c.mtx.Unlock() + + c.entries[path] = configCacheEntry{ + modTime: info.ModTime(), + size: info.Size(), + config: config, + } +} + func getConfig(configPath string) (*Config, error) { + info, err := os.Stat(configPath) + if err != nil { + return nil, err + } + if c := parsedConfigs.get(configPath, info); c != nil { + // Hand out a copy so that a caller cannot change what the next one + // sees. The maps and slices inside are shared, and are only ever read. + cached := *c + return &cached, nil + } + content, err := os.ReadFile(configPath) if err != nil { return nil, err @@ -165,7 +223,15 @@ func getConfig(configPath string) (*Config, error) { err = validateHeaderConfig(c.HTTPConfig.Header) } c.TLSConfig.SetDirectory(filepath.Dir(configPath)) - return c, err + if err != nil { + // A file that does not parse is not cached, so that the error is + // reported again for every request until it is fixed. + return c, err + } + + parsedConfigs.set(configPath, info, c) + cached := *c + return &cached, nil } func getTLSConfig(configPath string) (*tls.Config, error) {