web: reuse the parsed web configuration while the file is unchanged

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:
 

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)
+		}
+	}
+}

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) {