Merge branch 'main' into golangci-v2.12.2

Absorbs the cache size management and LRU eviction work (#55). All four
textual conflicts resolved in favor of main's implementation, with this
branch's mechanical lint conformance re-applied on top:

- internal/config/config.go: took main's cache_max_bytes wiring
  (CacheMaxBytes, cacheMaxBytesExplicit) verbatim and expressed the key
  through this branch's constant convention as keyCacheMaxBytes.
- internal/imgcache/cache.go: took main's disabled-cache guards, LRU
  touch on lookup, and variant_content size accounting verbatim;
  re-applied this branch's signature wrapping for lll.
- internal/imgcache/storage.go: took main's new writeIfAbsent helper and
  its temp-file cleanup defer verbatim. The auto-merge had silently
  dropped that defer in favor of this branch's inline cleanup form;
  restored so the cleanup semantics that arrived from main are intact.
- TODO.md: kept both sides' Completed Steps entries.

.golangci.yml resolves to this branch's canonical version
(sha256 021cc83f4e6fc7c31b95b34b846723dfcf20b66b7baeea1dc40406e643346bcb).

make build and make test are green; #55's code is not yet conformant
with the canonical lint config, which the following commits address.
This commit is contained in:
2026-08-09 13:16:14 +00:00
15 changed files with 2610 additions and 57 deletions

View File

@@ -0,0 +1,271 @@
package config
import (
"errors"
"io"
"log/slog"
"os"
"path/filepath"
"strings"
"testing"
)
// discardLogger returns a logger that swallows all output, for tests
// that exercise code paths which log.
func discardLogger() *slog.Logger {
return slog.New(slog.NewTextHandler(io.Discard, nil))
}
// TestCacheMaxBytesExplicitValueUsedWithoutFloor verifies that an
// explicitly configured cache_max_bytes value is used exactly as
// given: the 500 MiB floor applies only to the computed default, never
// to explicit values.
func TestCacheMaxBytesExplicitValueUsedWithoutFloor(t *testing.T) {
yamlContent := "signing_key: " + validTestSigningKey + "\ncache_max_bytes: 1024\n"
c, err := configFromYAML(t, yamlContent)
if err != nil {
t.Fatalf("explicit cache_max_bytes must be accepted, got error: %v", err)
}
if c.CacheMaxBytes != 1024 {
t.Errorf("CacheMaxBytes = %d, want 1024 (no floor for explicit values)",
c.CacheMaxBytes)
}
}
// TestCacheMaxBytesZeroIsValidAndDisablesCache verifies that an
// explicit zero is a valid value (it disables the disk cache), not an
// error.
func TestCacheMaxBytesZeroIsValidAndDisablesCache(t *testing.T) {
yamlContent := "signing_key: " + validTestSigningKey + "\ncache_max_bytes: 0\n"
c, err := configFromYAML(t, yamlContent)
if err != nil {
t.Fatalf("cache_max_bytes: 0 must be accepted, got error: %v", err)
}
if c.CacheMaxBytes != 0 {
t.Errorf("CacheMaxBytes = %d, want 0", c.CacheMaxBytes)
}
}
// TestCacheMaxBytesLargeExplicitValueParses verifies that values above
// 32-bit range parse correctly (the field is an int64 byte count).
func TestCacheMaxBytesLargeExplicitValueParses(t *testing.T) {
yamlContent := "signing_key: " + validTestSigningKey + "\ncache_max_bytes: 10737418240\n"
c, err := configFromYAML(t, yamlContent)
if err != nil {
t.Fatalf("large cache_max_bytes must be accepted, got error: %v", err)
}
if c.CacheMaxBytes != 10737418240 {
t.Errorf("CacheMaxBytes = %d, want 10737418240", c.CacheMaxBytes)
}
}
// TestCacheMaxBytesInvalidValuesAbortStartup verifies that a SET but
// invalid cache_max_bytes value aborts startup naming the key and the
// offending value, per the no-silent-fallback rule: defaults apply
// only to omitted keys.
func TestCacheMaxBytesInvalidValuesAbortStartup(t *testing.T) {
signingKeyLine := "signing_key: " + validTestSigningKey + "\n"
cases := []struct {
name string
yaml string
// wantErrSubstrings must all appear in the error message.
wantErrSubstrings []string
}{
{
name: "negative",
yaml: signingKeyLine + "cache_max_bytes: -1024\n",
wantErrSubstrings: []string{"cache_max_bytes", "-1024"},
},
{
name: "float",
yaml: signingKeyLine + "cache_max_bytes: 3.5\n",
wantErrSubstrings: []string{"cache_max_bytes", "3.5"},
},
{
name: "non-numeric string",
yaml: signingKeyLine + "cache_max_bytes: banana\n",
wantErrSubstrings: []string{"cache_max_bytes", "banana"},
},
{
name: "explicit null",
yaml: signingKeyLine + "cache_max_bytes: null\n",
wantErrSubstrings: []string{"cache_max_bytes", "null"},
},
{
name: "bare key no value",
yaml: signingKeyLine + "cache_max_bytes:\n",
wantErrSubstrings: []string{"cache_max_bytes", "null"},
},
{
name: "boolean",
yaml: signingKeyLine + "cache_max_bytes: true\n",
wantErrSubstrings: []string{"cache_max_bytes", "true"},
},
{
name: "list",
yaml: signingKeyLine + "cache_max_bytes:\n - 1\n",
wantErrSubstrings: []string{"cache_max_bytes"},
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
c, err := configFromYAML(t, tc.yaml)
if err == nil {
t.Fatalf("config with %s cache_max_bytes must abort startup, got config: %+v",
tc.name, c)
}
t.Logf("got expected error: %v", err)
for _, want := range tc.wantErrSubstrings {
if !strings.Contains(err.Error(), want) {
t.Errorf("error %q does not mention %q", err.Error(), want)
}
}
})
}
}
// TestComputeDefaultCacheMaxBytesUses75PercentOfFreeSpace verifies the
// computed default is 75% of the probed free space when that exceeds
// the floor.
func TestComputeDefaultCacheMaxBytesUses75PercentOfFreeSpace(t *testing.T) {
// 4 GiB free -> 3 GiB default.
probe := func(string) (uint64, error) { return 4294967296, nil }
got, err := ComputeDefaultCacheMaxBytes(t.TempDir(), probe)
if err != nil {
t.Fatalf("ComputeDefaultCacheMaxBytes returned error: %v", err)
}
if got != 3221225472 {
t.Errorf("ComputeDefaultCacheMaxBytes = %d, want 3221225472 (75%% of 4 GiB)", got)
}
}
// TestComputeDefaultCacheMaxBytesAppliesFloorToComputedDefault
// verifies that when 75% of free space is below 500 MiB, the computed
// default is floored at DefaultCacheMaxBytesFloor.
func TestComputeDefaultCacheMaxBytesAppliesFloorToComputedDefault(t *testing.T) {
cases := []struct {
name string
freeBytes uint64
}{
{name: "100 MiB free", freeBytes: 104857600},
{name: "zero free", freeBytes: 0},
{name: "just below floor threshold", freeBytes: 699050665},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
probe := func(string) (uint64, error) { return tc.freeBytes, nil }
got, err := ComputeDefaultCacheMaxBytes(t.TempDir(), probe)
if err != nil {
t.Fatalf("ComputeDefaultCacheMaxBytes returned error: %v", err)
}
if got != DefaultCacheMaxBytesFloor {
t.Errorf("ComputeDefaultCacheMaxBytes = %d, want floor %d",
got, DefaultCacheMaxBytesFloor)
}
})
}
}
// TestComputeDefaultCacheMaxBytesPropagatesProbeError verifies that a
// failing free-space probe produces an error naming the config key,
// instead of a silently wrong default.
func TestComputeDefaultCacheMaxBytesPropagatesProbeError(t *testing.T) {
probe := func(string) (uint64, error) { return 0, errors.New("statfs failed") }
_, err := ComputeDefaultCacheMaxBytes(t.TempDir(), probe)
if err == nil {
t.Fatal("probe failure must produce an error, got nil")
}
t.Logf("got expected error: %v", err)
if !strings.Contains(err.Error(), "cache_max_bytes") {
t.Errorf("error %q does not name the config key cache_max_bytes", err.Error())
}
}
// TestResolveCacheMaxBytesComputesDefaultWhenOmitted verifies that an
// omitted cache_max_bytes key resolves to the computed default, that
// the probe is pointed at <state_dir>/cache/ (which must be created
// first so statfs measures the right filesystem), and that the result
// lands on the Config.
func TestResolveCacheMaxBytesComputesDefaultWhenOmitted(t *testing.T) {
c, err := configFromYAML(t, "signing_key: "+validTestSigningKey+"\n")
if err != nil {
t.Fatalf("minimal config should be valid, got error: %v", err)
}
c.StateDir = t.TempDir()
wantCacheDir := filepath.Join(c.StateDir, "cache")
var probedPath string
// 4 GiB free -> 3 GiB default.
probe := func(path string) (uint64, error) {
probedPath = path
return 4294967296, nil
}
if err := c.resolveCacheMaxBytes(discardLogger(), probe); err != nil {
t.Fatalf("resolveCacheMaxBytes returned error: %v", err)
}
if c.CacheMaxBytes != 3221225472 {
t.Errorf("CacheMaxBytes = %d, want computed default 3221225472", c.CacheMaxBytes)
}
if probedPath != wantCacheDir {
t.Errorf("free space probed at %q, want cache directory %q", probedPath, wantCacheDir)
}
info, err := os.Stat(wantCacheDir)
if err != nil || !info.IsDir() {
t.Errorf("cache directory %q was not created before probing: info=%v err=%v",
wantCacheDir, info, err)
}
}
// TestResolveCacheMaxBytesDoesNotOverrideExplicitValue verifies that
// an explicitly configured value survives resolution untouched and
// that the free-space probe is never consulted for it.
func TestResolveCacheMaxBytesDoesNotOverrideExplicitValue(t *testing.T) {
yamlContent := "signing_key: " + validTestSigningKey + "\ncache_max_bytes: 1024\n"
c, err := configFromYAML(t, yamlContent)
if err != nil {
t.Fatalf("explicit cache_max_bytes must be accepted, got error: %v", err)
}
c.StateDir = t.TempDir()
probe := func(string) (uint64, error) {
t.Error("free-space probe must not be consulted for explicit values")
return 0, errors.New("probe must not be called")
}
if err := c.resolveCacheMaxBytes(discardLogger(), probe); err != nil {
t.Fatalf("resolveCacheMaxBytes returned error: %v", err)
}
if c.CacheMaxBytes != 1024 {
t.Errorf("CacheMaxBytes = %d, want explicit 1024 (no floor, no recompute)",
c.CacheMaxBytes)
}
}

View File

@@ -0,0 +1,110 @@
package config
import (
"fmt"
"log/slog"
"math"
"os"
"path/filepath"
"syscall"
)
// DefaultCacheMaxBytesFloor is the minimum computed default for the
// cache_max_bytes setting: 500 MiB. The floor applies only to the
// computed default (when the key is omitted from the configuration),
// never to explicitly configured values.
const DefaultCacheMaxBytesFloor int64 = 524288000
// cacheDirPerms is the permission mode for the cache directory created
// before probing free space, matching the state directory permissions.
const cacheDirPerms = 0o750
// freeSpaceFractionNumerator and freeSpaceFractionDenominator express
// the 75% share of free space used for the computed default limit as
// integer arithmetic (dividing before multiplying avoids overflow).
const (
freeSpaceFractionNumerator uint64 = 3
freeSpaceFractionDenominator uint64 = 4
)
// FreeSpaceProbeFunc reports the number of free bytes available on the
// filesystem containing path. It is a function type so tests can
// inject a fake probe instead of depending on the host disk.
type FreeSpaceProbeFunc func(path string) (uint64, error)
// defaultFreeSpaceProbe reports free filesystem bytes via statfs on
// the given path, as available to unprivileged processes.
func defaultFreeSpaceProbe(path string) (uint64, error) {
var stat syscall.Statfs_t
if err := syscall.Statfs(path, &stat); err != nil {
return 0, err
}
if stat.Bsize < 0 {
return 0, fmt.Errorf("statfs reported negative block size %d for %q", stat.Bsize, path)
}
blockSize := uint64(stat.Bsize) //nolint:gosec // G115: negative Bsize rejected above
return stat.Bavail * blockSize, nil
}
// ComputeDefaultCacheMaxBytes returns the default cache size limit for
// the filesystem containing cacheDir: 75% of the free bytes reported
// by probe, with a floor of DefaultCacheMaxBytesFloor.
func ComputeDefaultCacheMaxBytes(cacheDir string, probe FreeSpaceProbeFunc) (int64, error) {
freeBytes, err := probe(cacheDir)
if err != nil {
return 0, fmt.Errorf("config key %q: cannot determine free space for %q: %w",
"cache_max_bytes", cacheDir, err)
}
computed := freeBytes / freeSpaceFractionDenominator * freeSpaceFractionNumerator
if computed > math.MaxInt64 {
computed = math.MaxInt64
}
limit := int64(computed) //nolint:gosec // G115: clamped to MaxInt64 above
if limit < DefaultCacheMaxBytesFloor {
limit = DefaultCacheMaxBytesFloor
}
return limit, nil
}
// resolveCacheMaxBytes finalizes CacheMaxBytes after state_dir
// validation: an explicitly configured value is kept as-is (no floor
// applies), while an omitted key receives the computed default based
// on free space in <state_dir>/cache/. The cache directory is created
// first so statfs measures the filesystem that will actually hold the
// cache. The effective limit is logged either way.
func (c *Config) resolveCacheMaxBytes(log *slog.Logger, probe FreeSpaceProbeFunc) error {
if !c.cacheMaxBytesExplicit {
cacheDir := filepath.Join(c.StateDir, "cache")
if err := os.MkdirAll(cacheDir, cacheDirPerms); err != nil {
return fmt.Errorf("config key %q: cannot create cache directory %q: %w",
"cache_max_bytes", cacheDir, err)
}
limit, err := ComputeDefaultCacheMaxBytes(cacheDir, probe)
if err != nil {
return err
}
c.CacheMaxBytes = limit
log.Info("computed default cache size limit from free space",
"cache_max_bytes", limit,
"cache_dir", cacheDir,
)
}
log.Info("effective cache size limit",
"cache_max_bytes", c.CacheMaxBytes,
"cache_disabled", c.CacheMaxBytes == 0,
)
return nil
}

View File

@@ -41,6 +41,7 @@ const (
keyAllowlistHosts = "allowlist_hosts"
keyAllowHTTP = "allow_http"
keyUpstreamConnectionsPerHost = "upstream_connections_per_host"
keyCacheMaxBytes = "cache_max_bytes"
)
// Static validation errors. Each use site attaches the offending key
@@ -94,6 +95,19 @@ type Config struct {
AllowlistHosts []string // Hosts that don't require signatures
AllowHTTP bool // Allow non-TLS upstream (testing only)
UpstreamConnectionsPerHost int // Max concurrent connections per upstream host
// CacheMaxBytes is the disk cache size limit in bytes. Zero
// disables the disk cache entirely. When cache_max_bytes is
// omitted from the configuration, this holds the computed default
// (75% of free space on the filesystem containing
// <state_dir>/cache/, floored at DefaultCacheMaxBytesFloor).
CacheMaxBytes int64
// cacheMaxBytesExplicit records whether cache_max_bytes was
// explicitly set in the configuration file. Explicit values are
// used exactly as given; only an omitted key gets the computed
// default (and its floor) in resolveCacheMaxBytes.
cacheMaxBytesExplicit bool
}
// New creates a new Config instance by loading configuration from file.
@@ -120,6 +134,10 @@ func New(_ fx.Lifecycle, params Params) (*Config, error) {
return nil, err
}
if err := c.resolveCacheMaxBytes(log, defaultFreeSpaceProbe); err != nil {
return nil, err
}
if c.Debug {
params.Logger.EnableDebugLogging()
}
@@ -160,6 +178,16 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
AllowHTTP: loader.boolVal(keyAllowHTTP, false),
UpstreamConnectionsPerHost: loader.intVal(
keyUpstreamConnectionsPerHost, DefaultUpstreamConnectionsPerHost),
CacheMaxBytes: loader.int64Val(keyCacheMaxBytes, 0),
}
// The computed default for cache_max_bytes needs a validated
// state_dir, so it is resolved later (resolveCacheMaxBytes); here
// we only record whether the operator set the key explicitly.
if sc != nil {
if _, present := sc.Get(keyCacheMaxBytes); present {
c.cacheMaxBytesExplicit = true
}
}
// Build DBURL from StateDir if not explicitly set. The derived URL
@@ -266,7 +294,7 @@ func isKnownConfigKey(key string) bool {
switch key {
case keyDebug, keyMaintenanceMode, keyPort, keyStateDir, keySentryDSN,
keyDBURL, keyMetrics, keySigningKey, keyAllowlistHosts, keyAllowHTTP,
keyUpstreamConnectionsPerHost, "env":
keyUpstreamConnectionsPerHost, keyCacheMaxBytes, "env":
return true
}
@@ -339,6 +367,13 @@ func (c *Config) validate() error {
return fmt.Errorf("config key %q: %w", keyStateDir, errValueEmpty)
}
// Zero is valid (it disables the disk cache); only negative
// values are rejected. No floor applies to explicit values.
if c.CacheMaxBytes < 0 {
return fmt.Errorf("config key %q: value %d must not be negative",
"cache_max_bytes", c.CacheMaxBytes)
}
for _, host := range c.AllowlistHosts {
err := validateAllowlistHost(host)
if err != nil {
@@ -463,6 +498,19 @@ func (l *strictLoader) intVal(key string, defaultVal int) int {
return val
}
func (l *strictLoader) int64Val(key string, defaultVal int64) int64 {
if l.err != nil {
return 0
}
val, err := getInt64(l.sc, key, defaultVal)
if err != nil {
l.err = err
}
return val
}
func (l *strictLoader) boolVal(key string, defaultVal bool) bool {
if l.err != nil {
return false
@@ -545,6 +593,55 @@ func getInt(sc *smartconfig.Config, key string, defaultVal int) (int, error) {
}
}
// getInt64 returns the 64-bit integer value for key, or defaultVal if
// the key is omitted. A present value that is not a whole number, or
// is explicitly null, is an error; fractional values are never
// truncated and out-of-range values are never clamped.
func getInt64(sc *smartconfig.Config, key string, defaultVal int64) (int64, error) {
if sc == nil {
return defaultVal, nil
}
raw, ok := sc.Get(key)
if !ok {
return defaultVal, nil
}
if raw == nil {
return 0, errNullConfigValue(key)
}
switch val := raw.(type) {
case int:
return int64(val), nil
case int64:
return val, nil
case uint64:
if val > math.MaxInt64 {
return 0, fmt.Errorf("config key %q: value %d overflows a 64-bit integer",
key, val)
}
return int64(val), nil //nolint:gosec // G115: bounds checked above
case float64:
if val != math.Trunc(val) {
return 0, fmt.Errorf("config key %q: value %v is not an integer", key, val)
}
return int64(val), nil
case string:
parsed, err := strconv.ParseInt(strings.TrimSpace(val), 10, 64)
if err != nil {
return 0, fmt.Errorf("config key %q: value %q is not an integer", key, val)
}
return parsed, nil
default:
return 0, fmt.Errorf("config key %q: value %v (%T) is not an integer",
key, raw, raw)
}
}
// getBool returns the boolean value for key, or defaultVal if the key
// is omitted. A present value that is not a boolean (or a ParseBool-able
// string), or is explicitly null, is an error; numbers are not accepted